[PATCH v2 1/4] drm/i915/dp: Assume panel power is off if runtime suspended

[Cavitt, Jonathan]

-----Original Message-----
From: Intel-xe <intel-xe-bounces at lists.freedesktop.org> On Behalf Of Imre Deak
Sent: Wednesday, October 9, 2024 12:44 PM
To: intel-xe at lists.freedesktop.org; intel-gfx at lists.freedesktop.org
Subject: [PATCH v2 1/4] drm/i915/dp: Assume panel power is off if runtime suspended
> 
> If the device is runtime suspended the eDP panel power is also off.
> Ignore a short HPD on eDP if the device is suspended accordingly,
> instead of checking the panel power state via the PPS registers for the
> same purpose. The latter involves runtime resuming the device
> unnecessarily, in a frequent scenario where the panel generates a
> spurious short HPD after disabling the panel power and the device is
> runtime suspended.
> 
> Signed-off-by: Imre Deak <imre.deak at intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c                   | 5 ++++-
>  drivers/gpu/drm/i915/intel_runtime_pm.h                   | 8 +++++++-
>  drivers/gpu/drm/xe/compat-i915-headers/intel_runtime_pm.h | 8 ++++++++
>  3 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index fbb096be02ade..3eff35dd59b8a 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -85,6 +85,7 @@
>  #include "intel_pch_display.h"
>  #include "intel_pps.h"
>  #include "intel_psr.h"
> +#include "intel_runtime_pm.h"
>  #include "intel_quirks.h"
>  #include "intel_tc.h"
>  #include "intel_vdsc.h"
> @@ -6054,7 +6055,9 @@ intel_dp_hpd_pulse(struct intel_digital_port *dig_port, bool long_hpd)
>  	u8 dpcd[DP_RECEIVER_CAP_SIZE];
>  
>  	if (dig_port->base.type == INTEL_OUTPUT_EDP &&
> -	    (long_hpd || !intel_pps_have_panel_power_or_vdd(intel_dp))) {
> +	    (long_hpd ||
> +	     intel_runtime_pm_suspended(&i915->runtime_pm) ||
> +	     !intel_pps_have_panel_power_or_vdd(intel_dp))) {

>From what I'm reading, I'm fairly certain that
"i915->runtime_pm->kdev" is equivalent to "i915->drm.dev".
At least, this seems to be the case according to this comment on
the intel_runtime_pm struct in intel_runtime_pm.h:

"	struct device *kdev; /* points to i915->drm.dev */"

So, "intel_runtime_pm_suspended(&i915->runtime_pm)" seems
to be logically equivalent to
"pm_runtime_suspended(i915->drm.dev)", which would mean we
wouldn't need to declare the new helper function
"intel_runtime_pm_suspended" as both want to operate
pm_runtime_suspended on the same relative path for their target
drm device.

Though, perhaps I'm missing some other reasons we would want
the additional helper function besides, so I won't block on this:

Reviewed-by: Jonathan Cavitt <jonathan.cavitt at intel.com>
-Jonathan Cavitt

>  		/*
>  		 * vdd off can generate a long/short pulse on eDP which
>  		 * would require vdd on to handle it, and thus we
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.h b/drivers/gpu/drm/i915/intel_runtime_pm.h
> index 126f8320f86eb..e22669d61e954 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.h
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.h
> @@ -96,10 +96,16 @@ intel_rpm_wakelock_count(int wakeref_count)
>  	return wakeref_count >> INTEL_RPM_WAKELOCK_SHIFT;
>  }
>  
> +static inline bool
> +intel_runtime_pm_suspended(struct intel_runtime_pm *rpm)
> +{
> +	return pm_runtime_suspended(rpm->kdev);
> +}
> +
>  static inline void
>  assert_rpm_device_not_suspended(struct intel_runtime_pm *rpm)
>  {
> -	WARN_ONCE(pm_runtime_suspended(rpm->kdev),
> +	WARN_ONCE(intel_runtime_pm_suspended(rpm),
>  		  "Device suspended during HW access\n");
>  }
>  
> diff --git a/drivers/gpu/drm/xe/compat-i915-headers/intel_runtime_pm.h b/drivers/gpu/drm/xe/compat-i915-headers/intel_runtime_pm.h
> index cba587ceba1b6..274042bff1bec 100644
> --- a/drivers/gpu/drm/xe/compat-i915-headers/intel_runtime_pm.h
> +++ b/drivers/gpu/drm/xe/compat-i915-headers/intel_runtime_pm.h
> @@ -20,6 +20,14 @@ static inline void enable_rpm_wakeref_asserts(void *rpm)
>  {
>  }
>  
> +static inline bool
> +intel_runtime_pm_suspended(struct xe_runtime_pm *pm)
> +{
> +	struct xe_device *xe = container_of(pm, struct xe_device, runtime_pm);
> +
> +	return pm_runtime_suspended(xe->drm.dev);
> +}
> +
>  static inline intel_wakeref_t intel_runtime_pm_get(struct xe_runtime_pm *pm)
>  {
>  	struct xe_device *xe = container_of(pm, struct xe_device, runtime_pm);
> -- 
> 2.44.2
> 
>