On Wed, 05 Sep 2018 at 10:37:39 +0100 Eric Engestrom wrote: > Hey Karsten, thanks for the patch! I don't know if it's correct or not, > but I'll comment on a few other things. > > The title of the commit should be a description of what your commit does; > in this case you should probably keep the title that git gave you when > you did `git revert`, as well as the first line: > > Revert "drm/atomic: Handling the case when setting old crtc for plane" > > This reverts commit fc2a69f3903dfd97cd47f593e642b47918c949df. > After that comes your explanation of why this commit should be reverted, > which you've done here: > > On Wednesday, 2018-09-05 00:26:27 +0200, Karsten Hohmeier wrote: > > Original commit causes "plane A assertion failure" on lid close/lid open > > with older HP Compaq 6720s laptops (Intel Mobile GME965/GLE960). > > The "Fixes" line that Chris gave you should got at the end of the commit > message, after the explanation. > > > Full bug report at "https://bugs.freedesktop.org/show_bug.cgi?id=107827";. > > The format for bug report references is (on its own line): > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107827 > > > --- > > drivers/gpu/drm/drm_atomic.c | 4 +--- > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > > index 895741e9cd7d..a59ded403657 100644 > > --- a/drivers/gpu/drm/drm_atomic.c > > +++ b/drivers/gpu/drm/drm_atomic.c > > @@ -1433,9 +1433,7 @@ drm_atomic_set_crtc_for_plane(struct > > drm_plane_state *plane_state, > > It looks like thunderbird mangled the patch when sending the email; it's > recommended to use `git send-email` instead, as it does the right thing: > https://git-scm.com/docs/git-send-email > > > { > > struct drm_plane *plane = plane_state->plane; > > struct drm_crtc_state *crtc_state; > > - /* Nothing to do for same crtc*/ > > - if (plane_state->crtc == crtc) > > - return 0; > > + > > if (plane_state->crtc) { > > crtc_state = drm_atomic_get_crtc_state(plane_state->state, > > plane_state->crtc); > > -- Hi Karsten, Thanks for the patch. However, your own comments at the below link are contradictory https://bugs.freedesktop.org/show_bug.cgi?id=107827 Here, you mention that the commit 82bcb0e7a45d was still in place and screen corruption is gone and logs are clean. > Karsten Hohmeier 2018-09-05 17:11:14 UTC > I cherry-picked commit 05c72e77ccda ("drm/i915: Nuke the LVDS lid notifier") on top of 4.18.5 as you suggested. > Yes, it works. > The screen corruption is gone and the logs are clean. The 82bcb0e7a45d ("drm/atomic: Handling the case when setting old crtc for plane") change was still in place. >From above comments it looks like this patch was not the reason for screen corruption. Please clarify and let me know if I am missing something here. Thanks -Satendra
