On Wed, 18 Mar 2020, Anshuman Gupta <anshuman.gu...@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.gu...@intel.com>

Thanks,

Reviewed-by: Jani Nikula <jani.nik...@intel.com>

> ---
>  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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to