On Wed, Sep 07, 2022 at 07:51:05AM +0200, Takashi Iwai wrote: > On Tue, 06 Sep 2022 22:06:55 +0200, > Daniel Vetter wrote: > > > > On Tue, Aug 16, 2022 at 05:36:44PM +0200, Takashi Iwai wrote: > > > From: Thomas Zimmermann <tzimmermann@xxxxxxx> > > > > > > Restore the display mode whne resuming from suspend. Currently, the > > > display remains dark. > > > > > > On resume, the CRTC's mode does not change, but the 'active' flag > > > changes to 'true'. Taking this into account when considering a mode > > > switch restores the display mode. > > > > > > The bug is reproducable by using Gnome with udl and observing the > > > adapter's suspend/resume behavior. > > > > > > Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx> > > > Signed-off-by: Takashi Iwai <tiwai@xxxxxxx> > > > > This patch isn't great and incomplete, see > > > > https://lore.kernel.org/dri-devel/YxegiQFAv+OWjjqE@phenom.ffwll.local/ > > > > You need cc: stable and fixes: 997d33c35618 and actually just remove the > > entire check :-) > > OK, then is something like below? > > I already submitted v2 yesterday (as I overlooked your reply), so I'll > respin v3 with this (and your ack) if that's OK. > > > thanks, > > Takashi > > -- 8< -- > From: Takashi Iwai <tiwai@xxxxxxx> > Subject: [PATCH] drm/udl: Restore display mode on resume > > Restore the display mode whne resuming from suspend. Currently, the > display remains dark. > > On resume, the CRTC's mode does not change, but the 'active' flag > changes to 'true'. Taking this into account when considering a mode > switch restores the display mode. > > The bug is reproducable by using Gnome with udl and observing the > adapter's suspend/resume behavior. > > Actually, the whole check added in udl_simple_display_pipe_enable() > about the crtc_state->mode_changed was bogus. We should drop the > whole check and always apply the mode change in this function. > > [ tiwai -- Drop the mode_changed check entirely instead, per Daniel's > suggestion ] > > Fixes: 997d33c35618 ("drm/udl: Inline DPMS code into CRTC enable and disable functions") > Cc: <stable@xxxxxxxxxxxxxxx> > Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx> > Suggested-by: Daniel Vetter <daniel.vetter@xxxxxxxx> > Signed-off-by: Takashi Iwai <tiwai@xxxxxxx> Yeah I think that's the right one. But please check that it still fixes the bug you're seeing :-) Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx> > --- > drivers/gpu/drm/udl/udl_modeset.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/drivers/gpu/drm/udl/udl_modeset.c b/drivers/gpu/drm/udl/udl_modeset.c > index 169110d8fc2e..34ce5b43c5db 100644 > --- a/drivers/gpu/drm/udl/udl_modeset.c > +++ b/drivers/gpu/drm/udl/udl_modeset.c > @@ -382,9 +382,6 @@ udl_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe, > > udl_handle_damage(fb, &shadow_plane_state->data[0], 0, 0, fb->width, fb->height); > > - if (!crtc_state->mode_changed) > - return; > - > /* enable display */ > udl_crtc_write_mode_to_hw(crtc); > } > -- > 2.35.3 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
