[weston] libweston/compositor-drm: Reset repaint scheduled status when setting DPMS level to off

Submitted by Marius-cristian Vlad on March 7, 2018, 5:36 p.m.

Details

Message ID 20180307173646.22820-1-marius-cristian.vlad@nxp.com
State New
Headers show
Series "libweston/compositor-drm: Reset repaint scheduled status when setting DPMS level to off" ( rev: 1 ) in Wayland

Not browsing as part of any series.

Commit Message

Marius-cristian Vlad March 7, 2018, 5:36 p.m.
Otherwise when setting dpms level DPMS_ON, weston_output_schedule_repaint()
will bail out early and never get a chance to wake up the output.

Arguably this could also be done in drm_set_dpms() when setting dpms_off_pending
but I figure it better to do it when deferred.

Signed-off-by: Marius Vlad <marius-cristian.vlad@nxp.com>
CC: Daniel Stone <daniel@fooishbar.org>
CC: Pekka Paalanen <ppaalanen@gmail.com>
---
 libweston/compositor-drm.c | 6 ++++++
 libweston/compositor.c     | 2 +-
 libweston/compositor.h     | 2 ++
 3 files changed, 9 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 9594425..a53086e 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -1439,6 +1439,12 @@  drm_output_update_complete(struct drm_output *output, uint32_t flags,
 	} else if (output->dpms_off_pending) {
 		struct drm_pending_state *pending = drm_pending_state_alloc(b);
 		output->dpms_off_pending = 0;
+		/* reset repaint status so that weston_output_schedule_repaint()
+		 * will start the repaint_loop when DPMS level is ON */
+#ifdef DEBUG
+		weston_log("Reseting repaint status for output %s\n", output->base.name);
+#endif
+		weston_output_schedule_repaint_reset(&output->base);
 		drm_output_get_disable_state(pending, output);
 		drm_pending_state_apply_sync(pending);
 		return;
diff --git a/libweston/compositor.c b/libweston/compositor.c
index 274a22d..79c8d21 100644
--- a/libweston/compositor.c
+++ b/libweston/compositor.c
@@ -2360,7 +2360,7 @@  weston_output_repaint(struct weston_output *output, void *repaint_data)
 	return r;
 }
 
-static void
+WL_EXPORT void
 weston_output_schedule_repaint_reset(struct weston_output *output)
 {
 	output->repaint_status = REPAINT_NOT_SCHEDULED;
diff --git a/libweston/compositor.h b/libweston/compositor.h
index 010f1fa..afd49f5 100644
--- a/libweston/compositor.h
+++ b/libweston/compositor.h
@@ -1461,6 +1461,8 @@  weston_output_finish_frame(struct weston_output *output,
 void
 weston_output_schedule_repaint(struct weston_output *output);
 void
+weston_output_schedule_repaint_reset(struct weston_output *output);
+void
 weston_output_damage(struct weston_output *output);
 void
 weston_compositor_schedule_repaint(struct weston_compositor *compositor);

Comments

Hi Marius,

On 7 March 2018 at 17:36, Marius Vlad <marius-cristian.vlad@nxp.com> wrote:
> Otherwise when setting dpms level DPMS_ON, weston_output_schedule_repaint()
> will bail out early and never get a chance to wake up the output.
>
> Arguably this could also be done in drm_set_dpms() when setting dpms_off_pending
> but I figure it better to do it when deferred.

Thanks, that's a good catch, but I do wonder how it wasn't getting
tripped up before. In previous releases, we'd call drm_set_dpms() from
anywhere, which would block on the update completing and then set the
DPMS level. I wonder if it's because we would receive a flip-done
event anyway and call weston_output_finish_frame() after the DPMS had
completed, which would drive us into repaint-idle.

I suspect we should be calling finish_frame() to match that behaviour,
to indicate that the previous update has completed, and then
synchronously applying DPMS state. Pretending that
disable/destroy/DPMS-off can be called at any time is really biting
though. :(

Cheers,
Daniel
On Fri, 2018-03-09 at 09:35 +0000, Daniel Stone wrote:
> Hi Marius,

> 

> On 7 March 2018 at 17:36, Marius Vlad <marius-cristian.vlad@nxp.com>

> wrote:

> > Otherwise when setting dpms level DPMS_ON,

> > weston_output_schedule_repaint()

> > will bail out early and never get a chance to wake up the output.

> > 

> > Arguably this could also be done in drm_set_dpms() when setting

> > dpms_off_pending

> > but I figure it better to do it when deferred.

> 

> Thanks, that's a good catch, but I do wonder how it wasn't getting

> tripped up before. In previous releases, we'd call drm_set_dpms()

> from

> anywhere, which would block on the update completing and then set the

> DPMS level. I wonder if it's because we would receive a flip-done

> event anyway and call weston_output_finish_frame() after the DPMS had

> completed, which would drive us into repaint-idle.


Indeed, I've submitted a previous patch because for timeline
instrumentation because of this. weston_output_finish_frame() is being
called with a NULL timestamp.

> 

> I suspect we should be calling finish_frame() to match that

> behaviour,

> to indicate that the previous update has completed, and then

> synchronously applying DPMS state. Pretending that

> disable/destroy/DPMS-off can be called at any time is really biting

> though. :(


I don't know enough about the repaint machinery, should this be done
from drm_set_dpms() or from drm_output_update_complete()?. I'm hitting
some asserts in both cases, maybe I'm doing it wrong :/.



> Cheers,

> Daniel
> On 7 March 2018 at 17:36, Marius Vlad <marius-cristian.vlad at nxp.com 
> <https://lists.freedesktop.org/mailman/listinfo/wayland-devel>> wrote:
> /
>> /Otherwise when setting dpms level DPMS_ON, 
>> weston_output_schedule_repaint() //will bail out early and never get a chance to wake up the output. ////Arguably this could also be done in drm_set_dpms() when setting 
>> dpms_off_pending //but I figure it better to do it when deferred./
> /
> Thanks, that's a good catch, but I do wonder how it wasn't getting
> tripped up before. In previous releases, we'd call drm_set_dpms() from
> anywhere, which would block on the update completing and then set the
> DPMS level. I wonder if it's because we would receive a flip-done
> event anyway and call weston_output_finish_frame() after the DPMS had
> completed, which would drive us into repaint-idle.
Hi! I think I might have tripped that up before.

Or a different DPMS bug?
Does "wake up the output" here mean DPMS wake, or starting drawing to that output again?

One of my monitors, which is quite slow to wake up (AOC U2879VF) pretty often becomes frozen after waking up from DPMS.
While on the other monitor (an old NEC MultiSync) this problem never happened.
So like I could move the mouse and see the picture change on that monitor, but the first one was still displaying the frozen picture from before going to sleep.

The log lines were:

|[00:42:10.740] queueing pageflip failed: m [00:42:10.740] Couldn't apply 
state for output DP-2|
On Fri, 2018-03-23 at 21:30 +0300, Greg V wrote:
> > On 7 March 2018 at 17:36, Marius Vlad <marius-cristian.vlad at

> > nxp.com 

> > <https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2

> > Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fwayland-

> > devel&data=02%7C01%7Cmarius-

> > cristian.vlad%40nxp.com%7Cf1acbc5280754c8936a908d590ec293a%7C686ea1

> > d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C636574266340740999&sdata=Q60NW

> > AAtygRpT0s55ToG%2Fkz5DloXqOM5Bhy5xIEO6PM%3D&reserved=0>> wrote:

> > /

> > > /Otherwise when setting dpms level DPMS_ON, 

> > > weston_output_schedule_repaint() //will bail out early and never

> > > get a chance to wake up the output. ////Arguably this could also

> > > be done in drm_set_dpms() when setting 

> > > dpms_off_pending //but I figure it better to do it when

> > > deferred./

> > 

> > /

> > Thanks, that's a good catch, but I do wonder how it wasn't getting

> > tripped up before. In previous releases, we'd call drm_set_dpms()

> > from

> > anywhere, which would block on the update completing and then set

> > the

> > DPMS level. I wonder if it's because we would receive a flip-done

> > event anyway and call weston_output_finish_frame() after the DPMS

> > had

> > completed, which would drive us into repaint-idle.

> 

> Hi! I think I might have tripped that up before.

> 

> Or a different DPMS bug?

> Does "wake up the output" here mean DPMS wake, or starting drawing to

> that output again?

Hi,

When the monitor wakes up it will eventually start drawing. From your
description seems to be a different issue. This (patch) relates to
Daniels' basic atomic modesetting patches that have landed in a few
weeks ago, and to me it seems you are (still) using the legacy KMS page
flip ioctl.


> One of my monitors, which is quite slow to wake up (AOC U2879VF)

> pretty often becomes frozen after waking up from DPMS.

> While on the other monitor (an old NEC MultiSync) this problem never

> happened.

> So like I could move the mouse and see the picture change on that

> monitor, but the first one was still displaying the frozen picture

> from before going to sleep.

> 

> The log lines were:

> 

> > [00:42:10.740] queueing pageflip failed: m [00:42:10.740] Couldn't

> > apply 

> 

> state for output DP-2|


Yes, failing to queue page flips will lead a frozen image on the
screen. 

>