On 2020-03-04 at 20:45:20 +0200, Ville Syrjälä wrote:
> On Wed, Mar 04, 2020 at 03:33:03PM +0200, Jani Nikula wrote:
> > On Wed, 04 Mar 2020, Anshuman Gupta <anshuman.gupta@xxxxxxxxx> 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 once panel is powered off.
> > >
> > > FIXME: It requires to wait for panel_power_off_delay in order
> > > to check the panel status, as panel triggers short pulse immediately
> > > after writing PP_OFF to PP_CTRL register.
> > >
> > > Signed-off-by: Anshuman Gupta <anshuman.gupta@xxxxxxxxx>
> > > ---
> > > drivers/gpu/drm/i915/display/intel_dp.c | 19 +++++++++++++++++++
> > > 1 file changed, 19 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > > index 0a417cd2af2b..93de015f5322 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > > @@ -6763,10 +6763,24 @@ static const struct drm_encoder_funcs intel_dp_enc_funcs = {
> > > .destroy = intel_dp_encoder_destroy,
> > > };
> > >
> > > +static bool is_edp_powered_off(struct intel_dp *intel_dp)
> >
> > We have a number of existing edp_ prefixed functions, with intel_
> > prefixed wrappers. Please make this intel_edp_have_panel_power(). Early
> > return false for non-eDP.
> >
> > Also handle intel_dp_is_edp() in the caller so it's clear what's being
> > done.
> >
> > > +{
> > > + intel_wakeref_t wakeref;
> > > + bool powerd_off = false;
> > > +
> > > + if (intel_dp_is_edp(intel_dp)) {
> > > + with_pps_lock(intel_dp, wakeref)
> > > + powerd_off = !edp_have_panel_power(intel_dp);
> > > + }
> > > +
> > > + return powerd_off;
> > > +}
> > > +
> > > 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;
> > > + struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> > >
> > > if (long_hpd && intel_dig_port->base.type == INTEL_OUTPUT_EDP) {
> > > /*
> > > @@ -6810,6 +6824,11 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
> > > if (!intel_dp->is_mst) {
> > > bool handled;
> > >
> > > + if (is_edp_powered_off(intel_dp)) {
> >
> > I would move this to the beginning of the function in the same if
> > statement as the long_hpd handling:
> >
> > if (intel_dp_is_edp(intel_dp) &&
> > (long_hpd || !intel_edp_have_panel_power(intel_dp)))
> >
> > But makes me wonder if that should be changed to ignore all hpd from eDP
> > if there's no panel power nor vdd. Ville?
>
> From what I've seen for eDP no vdd implies HPD being deasserted, hence
> there is no way to signal short HPDs after vdd has been removed (apart
> from some glitchy HPD lines I guess). This case sounds like there could
> be a glitch of some sort on the HPD line while it is being deasserted
> and that triggers a short HPD. Either that or the panel is just stupid.
Hi Ville ,
I have observed that, similar panel doesn't trigger any short/long pulse on ICL but on TGL
it triggers both short and long pulse.
Thanks,
Anshuman.
>
> Anyways, vdd and panel power are the same thing from the panel POV, so
> checking both would make sense. I can't actually imagine this even
> working correctly otherwise, unless there is an actual bug in our code
> and the spurious HPD only happens when we disable vdd via the state
> machine rather than the override bit. So which is it?
>
> >
> > > + drm_info(&i915->drm, "edp panel is off, ignoring the short pulse\n");
> >
> > drm_dbg_kms() will be enough.
> >
> > > + return IRQ_HANDLED;
> > > + }
> > > +
> > > handled = intel_dp_short_pulse(intel_dp);
> > >
> > > if (!handled)
> >
> > --
> > Jani Nikula, Intel Open Source Graphics Center
>
> --
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
