Re: [Intel-gfx] [PATCH 1/2] drm/i915/display/vlv_dsi: Do not skip panel_pwr_cycle_delay when disabling the panel

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Apr 06, 2021 at 03:57:32PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 3/25/21 12:48 PM, Hans de Goede wrote:
> > After the recently added commit fe0f1e3bfdfe ("drm/i915: Shut down
> > displays gracefully on reboot"), the DSI panel on a Cherry Trail based
> > Predia Basic tablet would no longer properly light up after reboot.
> > 
> > I've managed to reproduce this without rebooting by doing:
> > chvt 3; echo 1 > /sys/class/graphics/fb0/blank;\
> > echo 0 > /sys/class/graphics/fb0/blank
> > 
> > Which rapidly turns the panel off and back on again.
> > 
> > The vlv_dsi.c code uses an intel_dsi_msleep() helper for the various delays
> > used for panel on/off, since starting with MIPI-sequences version >= 3 the
> > delays are already included inside the MIPI-sequences.
> > 
> > The problems exposed by the "Shut down displays gracefully on reboot"
> > change, show that using this helper for the panel_pwr_cycle_delay is
> > not the right thing to do. This has not been noticed until now because
> > normally the panel never is cycled off and directly on again in quick
> > succession.
> > 
> > Change the msleep for the panel_pwr_cycle_delay to a normal msleep()
> > call to avoid the panel staying black after a quick off + on cycle.
> > 
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Fixes: fe0f1e3bfdfe ("drm/i915: Shut down displays gracefully on reboot")
> > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> 
> Ping? Ville AFAICT this is ready for merging, can you review this please so that I can push it to drm-intel-next ?

Didn't get the original mail, but lgtm.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> 
> Regards,
> 
> Hans
> 
> 
> > ---
> >  drivers/gpu/drm/i915/display/vlv_dsi.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c
> > index d5a3f69c5df3..38d5a1f3ded5 100644
> > --- a/drivers/gpu/drm/i915/display/vlv_dsi.c
> > +++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
> > @@ -996,14 +996,14 @@ static void intel_dsi_post_disable(struct intel_atomic_state *state,
> >  	 * FIXME As we do with eDP, just make a note of the time here
> >  	 * and perform the wait before the next panel power on.
> >  	 */
> > -	intel_dsi_msleep(intel_dsi, intel_dsi->panel_pwr_cycle_delay);
> > +	msleep(intel_dsi->panel_pwr_cycle_delay);
> >  }
> >  
> >  static void intel_dsi_shutdown(struct intel_encoder *encoder)
> >  {
> >  	struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
> >  
> > -	intel_dsi_msleep(intel_dsi, intel_dsi->panel_pwr_cycle_delay);
> > +	msleep(intel_dsi->panel_pwr_cycle_delay);
> >  }
> >  
> >  static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
> > 

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx