| Message ID | 20241009194358.1321200-2-imre.deak@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/xe: Fix HPD interrupt enabling during runtime resume | expand |
-----Original Message----- From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of Imre Deak Sent: Wednesday, October 9, 2024 12:44 PM To: intel-xe@lists.freedesktop.org; intel-gfx@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@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@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 > >
On Wed, Oct 09, 2024 at 11:35:56PM +0300, Cavitt, Jonathan wrote: > -----Original Message----- > From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of Imre Deak > Sent: Wednesday, October 9, 2024 12:44 PM > To: intel-xe@lists.freedesktop.org; intel-gfx@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@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, Yes, I was surprised too but drivers/gpu/drm/i915/intel_runtime_pm.h is not included by xe, even when drivers/gpu/drm/i915/display is built for it. IIUC for this and other headers the xe specific ones will be included instead from drivers/gpu/drm/xe/compat-i915-headers. Some of these in turn like i915_irq.h will revert back including the original one from drivers/gpu/drm/i915. > so I won't block on this: > > Reviewed-by: Jonathan Cavitt <jonathan.cavitt@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 > > > >
-----Original Message----- From: Deak, Imre <imre.deak@intel.com> Sent: Wednesday, October 9, 2024 2:26 PM To: Cavitt, Jonathan <jonathan.cavitt@intel.com> Cc: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org Subject: Re: [PATCH v2 1/4] drm/i915/dp: Assume panel power is off if runtime suspended > > On Wed, Oct 09, 2024 at 11:35:56PM +0300, Cavitt, Jonathan wrote: > > -----Original Message----- > > From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of Imre Deak > > Sent: Wednesday, October 9, 2024 12:44 PM > > To: intel-xe@lists.freedesktop.org; intel-gfx@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@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, > > Yes, I was surprised too but drivers/gpu/drm/i915/intel_runtime_pm.h is > not included by xe, even when drivers/gpu/drm/i915/display is built for > it. IIUC for this and other headers the xe specific ones will be > included instead from drivers/gpu/drm/xe/compat-i915-headers. Some of > these in turn like i915_irq.h will revert back including the original > one from drivers/gpu/drm/i915. Sorry, let me clarify. When I said "perhaps I'm missing some other reasons we would want the additional helper function", I was referring to intel_runtime_pm_suspended as a whole, not just the mirror in compat-i915-headers. Basically, my question was why we use intel_runtime_pm_suspended, when pm_runtime_suspended, at least at first glance, would also work by itself? -Jonathan Cavitt > > > so I won't block on this: > > > > Reviewed-by: Jonathan Cavitt <jonathan.cavitt@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 > > > > > > >
On Thu, Oct 10, 2024 at 12:59:43AM +0300, Cavitt, Jonathan wrote: > -----Original Message----- > From: Deak, Imre <imre.deak@intel.com> > Sent: Wednesday, October 9, 2024 2:26 PM > To: Cavitt, Jonathan <jonathan.cavitt@intel.com> > Cc: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org > Subject: Re: [PATCH v2 1/4] drm/i915/dp: Assume panel power is off if runtime suspended > > > > On Wed, Oct 09, 2024 at 11:35:56PM +0300, Cavitt, Jonathan wrote: > > > -----Original Message----- > > > From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of Imre Deak > > > Sent: Wednesday, October 9, 2024 12:44 PM > > > To: intel-xe@lists.freedesktop.org; intel-gfx@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@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, > > > > Yes, I was surprised too but drivers/gpu/drm/i915/intel_runtime_pm.h is > > not included by xe, even when drivers/gpu/drm/i915/display is built for > > it. IIUC for this and other headers the xe specific ones will be > > included instead from drivers/gpu/drm/xe/compat-i915-headers. Some of > > these in turn like i915_irq.h will revert back including the original > > one from drivers/gpu/drm/i915. > > Sorry, let me clarify. When I said "perhaps I'm missing some other > reasons we would want the additional helper function", I was > referring to intel_runtime_pm_suspended as a whole, not just the > mirror in compat-i915-headers. > > Basically, my question was why we use intel_runtime_pm_suspended, > when pm_runtime_suspended, at least at first glance, would also work > by itself? I think all use of the driver's runtime PM interface - i.e. all runtime PM calls outside of intel_runtime_pm.c - should happen via the intel_runtime_pm struct pointer, which is opaque for the caller. > -Jonathan Cavitt > > > > > > so I won't block on this: > > > > > > Reviewed-by: Jonathan Cavitt <jonathan.cavitt@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 > > > > > > > > > >
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))) { /* * 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);
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@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(-)