[Intel-gfx] [PATCH v3] drm/i915/edp: Ignore short pulse when panel powered off

[Shankar, Uma]

> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On Behalf Of Jani Nikula
> Sent: Wednesday, March 18, 2020 3:09 PM
> To: Gupta, Anshuman <anshuman.gupta at intel.com>; intel-
> gfx at lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH v3] drm/i915/edp: Ignore short pulse when panel
> powered off
> 
> On Wed, 18 Mar 2020, Anshuman Gupta <anshuman.gupta at intel.com> wrote:
> > Few edp panels like Sharp is triggering short and long hpd pulse after
> > panel is getting powered off.
> > Currently driver is already ignoring long pulse for eDP panel but in
> > order to process the short pulse, it turns on the VDD which requires
> > panel power_cycle_delay + panel_power_on_delay these delay on Sharp
> > panel introduced the responsiveness overhead of 800ms in the modeset
> > sequence and as well is in suspend sequence.
> > Ignoring any short pulse if panel is powered off.
> >
> > FIXME: It requires to wait for panel_power_off delay in order to check
> > the panel power status due to pps_lock because panel triggers short
> > pulse immediately after writing PP_OFF to PP_CTRL register and
> > wait_panel_off waits for panel_power_off delay with pps_lock held.
> > This still creates responsiveness overhead of panel_power_off delay.
> >
> > v2:
> > - checking vdd along with panel power to ignore the hpd. [Jani,Ville]
> > v3:
> > - safer side check to ignore the long hpd when eDP have power,
> >   adding type of hpd to debug log. [Jani]
> >
> > Signed-off-by: Anshuman Gupta <anshuman.gupta at intel.com>
> 
> Thanks,
> 
> Reviewed-by: Jani Nikula <jani.nikula at intel.com>

Pushed to dinq, thanks for the patch and review.

Regards,
Uma Shankar

> > ---
> >  drivers/gpu/drm/i915/display/intel_dp.c | 23 +++++++++++++++++++----
> >  1 file changed, 19 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> > b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 0a417cd2af2b..38e74195101a 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -6763,19 +6763,34 @@ static const struct drm_encoder_funcs
> intel_dp_enc_funcs = {
> >  	.destroy = intel_dp_encoder_destroy,  };
> >
> > +static bool intel_edp_have_power(struct intel_dp *intel_dp) {
> > +	intel_wakeref_t wakeref;
> > +	bool have_power = false;
> > +
> > +	with_pps_lock(intel_dp, wakeref) {
> > +		have_power = edp_have_panel_power(intel_dp) &&
> > +						  edp_have_panel_vdd(intel_dp);
> > +	}
> > +
> > +	return have_power;
> > +}
> > +
> >  enum irqreturn
> >  intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool
> > long_hpd)  {
> >  	struct intel_dp *intel_dp = &intel_dig_port->dp;
> >
> > -	if (long_hpd && intel_dig_port->base.type == INTEL_OUTPUT_EDP) {
> > +	if (intel_dig_port->base.type == INTEL_OUTPUT_EDP &&
> > +	    (long_hpd || !intel_edp_have_power(intel_dp))) {
> >  		/*
> > -		 * vdd off can generate a long pulse on eDP which
> > +		 * vdd off can generate a long/short pulse on eDP which
> >  		 * would require vdd on to handle it, and thus we
> >  		 * would end up in an endless cycle of
> > -		 * "vdd off -> long hpd -> vdd on -> detect -> vdd off -> ..."
> > +		 * "vdd off -> long/short hpd -> vdd on -> detect -> vdd off -> ..."
> >  		 */
> > -		DRM_DEBUG_KMS("ignoring long hpd on eDP
> [ENCODER:%d:%s]\n",
> > +		DRM_DEBUG_KMS("ignoring %s hpd on eDP [ENCODER:%d:%s]\n",
> > +			      long_hpd ? "long" : "short",
> >  			      intel_dig_port->base.base.base.id,
> >  			      intel_dig_port->base.base.name);
> >  		return IRQ_HANDLED;
> 
> --
> Jani Nikula, Intel Open Source Graphics Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx