| Message ID | 20220831103724.14839-1-shawn.c.lee@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/display: refine eDP power off sequence | expand |
On Wed, Aug 31, 2022 at 06:37:24PM +0800, Lee Shawn C wrote: > The current eDP disable sequence like this. > > disable plane > disable backlight (include T9, the delay > from backlight disable to end of valid video data) > > disalbe transcoder/pipe > disable eDP power > > Found abnormal pixel output after plane off sometimes. > It did not cause any issue but impact user experience. > So we modify the eDP disable flow to turn backlight off > earlier to avoid abnormal display. NAK. Planes can be disable at any time by userspace. We need to find out what is causing the glitch. > > disable backlight > disable plane > disalbe transcoder/pipe > > disable eDP power > > Cc: Shankar Uma <uma.shankar@intel.com> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index 72e2091d9fcb..d08927036350 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -2045,10 +2045,8 @@ static void hsw_crtc_disable(struct intel_atomic_state *state, > * FIXME collapse everything to one hook. > * Need care with mst->ddi interactions. > */ > - if (!intel_crtc_is_bigjoiner_slave(old_crtc_state)) { > - intel_encoders_disable(state, crtc); > + if (!intel_crtc_is_bigjoiner_slave(old_crtc_state)) > intel_encoders_post_disable(state, crtc); > - } > } > > static void i9xx_pfit_enable(const struct intel_crtc_state *crtc_state) > @@ -7224,6 +7222,10 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state) > continue; > > intel_pre_plane_update(state, crtc); > + > + if (!intel_crtc_is_bigjoiner_slave(old_crtc_state)) > + intel_encoders_disable(state, crtc); > + > intel_crtc_disable_planes(state, crtc); > } > > -- > 2.31.1
On Wed, Aug 31, 2022 at 06:49, Ville Syrjälä wrote: >On Wed, Aug 31, 2022 at 06:37:24PM +0800, Lee Shawn C wrote: >> The current eDP disable sequence like this. >> >> disable plane > disable backlight (include T9, the delay from >> backlight disable to end of valid video data) > disalbe >> transcoder/pipe > disable eDP power >> >> Found abnormal pixel output after plane off sometimes. >> It did not cause any issue but impact user experience. >> So we modify the eDP disable flow to turn backlight off earlier to >> avoid abnormal display. > >NAK. Planes can be disable at any time by userspace. >We need to find out what is causing the glitch. > Hi Ville, thanks for comment! I uploaded a patch earlier to fix the problem. https://patchwork.freedesktop.org/patch/496067/ It pass the review by Uma. Unfortunately, the change is not able to pass CI. With that change, FIFO underrun always be found during CI testing. Best regards, Shawn >> >> disable backlight > disable plane > disalbe transcoder/pipe >> > disable eDP power >> >> Cc: Shankar Uma <uma.shankar@intel.com> >> Cc: Jani Nikula <jani.nikula@linux.intel.com> >> Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com> >> --- >> drivers/gpu/drm/i915/display/intel_display.c | 8 +++++--- >> 1 file changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_display.c >> b/drivers/gpu/drm/i915/display/intel_display.c >> index 72e2091d9fcb..d08927036350 100644 >> --- a/drivers/gpu/drm/i915/display/intel_display.c >> +++ b/drivers/gpu/drm/i915/display/intel_display.c >> @@ -2045,10 +2045,8 @@ static void hsw_crtc_disable(struct intel_atomic_state *state, >> * FIXME collapse everything to one hook. >> * Need care with mst->ddi interactions. >> */ >> - if (!intel_crtc_is_bigjoiner_slave(old_crtc_state)) { >> - intel_encoders_disable(state, crtc); >> + if (!intel_crtc_is_bigjoiner_slave(old_crtc_state)) >> intel_encoders_post_disable(state, crtc); >> - } >> } >> >> static void i9xx_pfit_enable(const struct intel_crtc_state >> *crtc_state) @@ -7224,6 +7222,10 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state) >> continue; >> >> intel_pre_plane_update(state, crtc); >> + >> + if (!intel_crtc_is_bigjoiner_slave(old_crtc_state)) >> + intel_encoders_disable(state, crtc); >> + >> intel_crtc_disable_planes(state, crtc); >> } >> >> -- >> 2.31.1 > >-- >Ville Syrjälä >Intel
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 72e2091d9fcb..d08927036350 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -2045,10 +2045,8 @@ static void hsw_crtc_disable(struct intel_atomic_state *state, * FIXME collapse everything to one hook. * Need care with mst->ddi interactions. */ - if (!intel_crtc_is_bigjoiner_slave(old_crtc_state)) { - intel_encoders_disable(state, crtc); + if (!intel_crtc_is_bigjoiner_slave(old_crtc_state)) intel_encoders_post_disable(state, crtc); - } } static void i9xx_pfit_enable(const struct intel_crtc_state *crtc_state) @@ -7224,6 +7222,10 @@ static void intel_commit_modeset_disables(struct intel_atomic_state *state) continue; intel_pre_plane_update(state, crtc); + + if (!intel_crtc_is_bigjoiner_slave(old_crtc_state)) + intel_encoders_disable(state, crtc); + intel_crtc_disable_planes(state, crtc); }