| Message ID | 20240924204222.246862-12-rodrigo.vivi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Reconcile i915's and xe's display power mgt sequences | expand |
-----Original Message----- From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Rodrigo Vivi Sent: Tuesday, September 24, 2024 1:36 PM To: intel-gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org Cc: Deak, Imre <imre.deak@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com> Subject: [PATCH 11/31] drm/{i915,xe}: Consolidate display resume functions > > Same sequences are currently executed by i915 and xe during > system resume. Consolidate them in the intel_display_driver > side. > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > --- > .../drm/i915/display/intel_display_driver.c | 16 ++++++++++++++++ > drivers/gpu/drm/i915/i915_driver.c | 17 ----------------- > drivers/gpu/drm/xe/display/xe_display.c | 18 +----------------- > 3 files changed, 17 insertions(+), 34 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c b/drivers/gpu/drm/i915/display/intel_display_driver.c > index 42624bf80f91..86d6ebf0c82f 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_driver.c > +++ b/drivers/gpu/drm/i915/display/intel_display_driver.c > @@ -752,6 +752,10 @@ void intel_display_driver_resume(struct drm_i915_private *i915) > if (!HAS_DISPLAY(i915)) > return; > > + intel_display_driver_resume_access(i915); > + > + intel_hpd_init(i915); > + > /* MST sideband requires HPD interrupts enabled */ > intel_dp_mst_resume(i915); > > @@ -781,6 +785,18 @@ void intel_display_driver_resume(struct drm_i915_private *i915) > "Restoring old state failed with %i\n", ret); > if (state) > drm_atomic_state_put(state); > + > + intel_display_driver_enable_user_access(i915); > + > + drm_kms_helper_poll_enable(&i915->drm); > + > + intel_hpd_poll_disable(i915); > + > + intel_opregion_resume(&i915->display); > + > + intel_fbdev_set_suspend(&i915->drm, FBINFO_STATE_RUNNING, false); > + > + intel_power_domains_enable(i915); Hmm... It looks like everything here is being gated by HAS_DISPLAY, when it wasn't before in i915_drm_resume or xe_display_pm_resume. Is this correct behavior? I'll trust your judgement on this. Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com> -Jonathan Cavitt > } > > void intel_display_driver_shutdown(struct drm_i915_private *i915) > diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c > index ac8bf00458b5..ce2dd03136d3 100644 > --- a/drivers/gpu/drm/i915/i915_driver.c > +++ b/drivers/gpu/drm/i915/i915_driver.c > @@ -1165,25 +1165,8 @@ static int i915_drm_resume(struct drm_device *dev) > > intel_clock_gating_init(dev_priv); > > - if (HAS_DISPLAY(dev_priv)) > - intel_display_driver_resume_access(dev_priv); > - > - intel_hpd_init(dev_priv); > - > intel_display_driver_resume(dev_priv); > > - if (HAS_DISPLAY(dev_priv)) { > - intel_display_driver_enable_user_access(dev_priv); > - drm_kms_helper_poll_enable(dev); > - } > - intel_hpd_poll_disable(dev_priv); > - > - intel_opregion_resume(display); > - > - intel_fbdev_set_suspend(dev, FBINFO_STATE_RUNNING, false); > - > - intel_power_domains_enable(dev_priv); > - > intel_gvt_resume(dev_priv); > > enable_rpm_wakeref_asserts(&dev_priv->runtime_pm); > diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c > index a4705a452adb..98ff603a1f84 100644 > --- a/drivers/gpu/drm/xe/display/xe_display.c > +++ b/drivers/gpu/drm/xe/display/xe_display.c > @@ -447,23 +447,7 @@ void xe_display_pm_resume(struct xe_device *xe) > > intel_display_driver_init_hw(xe); > > - if (has_display(xe)) > - intel_display_driver_resume_access(xe); > - > - intel_hpd_init(xe); > - > - if (has_display(xe)) { > - intel_display_driver_resume(xe); > - drm_kms_helper_poll_enable(&xe->drm); > - intel_display_driver_enable_user_access(xe); > - intel_hpd_poll_disable(xe); > - } > - > - intel_opregion_resume(display); > - > - intel_fbdev_set_suspend(&xe->drm, FBINFO_STATE_RUNNING, false); > - > - intel_power_domains_enable(xe); > + intel_display_driver_resume(xe); > } > > void xe_display_pm_runtime_resume(struct xe_device *xe) > -- > 2.46.0 > >
On Mon, Oct 07, 2024 at 05:02:04PM -0400, Cavitt, Jonathan wrote: > -----Original Message----- > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Rodrigo Vivi > Sent: Tuesday, September 24, 2024 1:36 PM > To: intel-gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org > Cc: Deak, Imre <imre.deak@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com> > Subject: [PATCH 11/31] drm/{i915,xe}: Consolidate display resume functions > > > > Same sequences are currently executed by i915 and xe during > > system resume. Consolidate them in the intel_display_driver > > side. > > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > --- > > .../drm/i915/display/intel_display_driver.c | 16 ++++++++++++++++ > > drivers/gpu/drm/i915/i915_driver.c | 17 ----------------- > > drivers/gpu/drm/xe/display/xe_display.c | 18 +----------------- > > 3 files changed, 17 insertions(+), 34 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c b/drivers/gpu/drm/i915/display/intel_display_driver.c > > index 42624bf80f91..86d6ebf0c82f 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_driver.c > > +++ b/drivers/gpu/drm/i915/display/intel_display_driver.c > > @@ -752,6 +752,10 @@ void intel_display_driver_resume(struct drm_i915_private *i915) > > if (!HAS_DISPLAY(i915)) > > return; > > > > + intel_display_driver_resume_access(i915); > > + > > + intel_hpd_init(i915); > > + > > /* MST sideband requires HPD interrupts enabled */ > > intel_dp_mst_resume(i915); > > > > @@ -781,6 +785,18 @@ void intel_display_driver_resume(struct drm_i915_private *i915) > > "Restoring old state failed with %i\n", ret); > > if (state) > > drm_atomic_state_put(state); > > + > > + intel_display_driver_enable_user_access(i915); > > + > > + drm_kms_helper_poll_enable(&i915->drm); > > + > > + intel_hpd_poll_disable(i915); > > + > > + intel_opregion_resume(&i915->display); > > + > > + intel_fbdev_set_suspend(&i915->drm, FBINFO_STATE_RUNNING, false); > > + > > + intel_power_domains_enable(i915); > > Hmm... It looks like everything here is being gated by HAS_DISPLAY, when > it wasn't before in i915_drm_resume or xe_display_pm_resume. Is this > correct behavior? This is a very good question. I really dislike the sprinkled if HAS_DISPLAY s, and I hope this is okay... > > I'll trust your judgement on this. > Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com> > -Jonathan Cavitt > > > } > > > > void intel_display_driver_shutdown(struct drm_i915_private *i915) > > diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c > > index ac8bf00458b5..ce2dd03136d3 100644 > > --- a/drivers/gpu/drm/i915/i915_driver.c > > +++ b/drivers/gpu/drm/i915/i915_driver.c > > @@ -1165,25 +1165,8 @@ static int i915_drm_resume(struct drm_device *dev) > > > > intel_clock_gating_init(dev_priv); > > > > - if (HAS_DISPLAY(dev_priv)) > > - intel_display_driver_resume_access(dev_priv); > > - > > - intel_hpd_init(dev_priv); > > - > > intel_display_driver_resume(dev_priv); > > > > - if (HAS_DISPLAY(dev_priv)) { > > - intel_display_driver_enable_user_access(dev_priv); > > - drm_kms_helper_poll_enable(dev); > > - } > > - intel_hpd_poll_disable(dev_priv); > > - > > - intel_opregion_resume(display); > > - > > - intel_fbdev_set_suspend(dev, FBINFO_STATE_RUNNING, false); > > - > > - intel_power_domains_enable(dev_priv); > > - > > intel_gvt_resume(dev_priv); > > > > enable_rpm_wakeref_asserts(&dev_priv->runtime_pm); > > diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c > > index a4705a452adb..98ff603a1f84 100644 > > --- a/drivers/gpu/drm/xe/display/xe_display.c > > +++ b/drivers/gpu/drm/xe/display/xe_display.c > > @@ -447,23 +447,7 @@ void xe_display_pm_resume(struct xe_device *xe) > > > > intel_display_driver_init_hw(xe); > > > > - if (has_display(xe)) > > - intel_display_driver_resume_access(xe); > > - > > - intel_hpd_init(xe); > > - > > - if (has_display(xe)) { > > - intel_display_driver_resume(xe); > > - drm_kms_helper_poll_enable(&xe->drm); > > - intel_display_driver_enable_user_access(xe); > > - intel_hpd_poll_disable(xe); > > - } > > - > > - intel_opregion_resume(display); > > - > > - intel_fbdev_set_suspend(&xe->drm, FBINFO_STATE_RUNNING, false); > > - > > - intel_power_domains_enable(xe); > > + intel_display_driver_resume(xe); > > } > > > > void xe_display_pm_runtime_resume(struct xe_device *xe) > > -- > > 2.46.0 > > > >
diff --git a/drivers/gpu/drm/i915/display/intel_display_driver.c b/drivers/gpu/drm/i915/display/intel_display_driver.c index 42624bf80f91..86d6ebf0c82f 100644 --- a/drivers/gpu/drm/i915/display/intel_display_driver.c +++ b/drivers/gpu/drm/i915/display/intel_display_driver.c @@ -752,6 +752,10 @@ void intel_display_driver_resume(struct drm_i915_private *i915) if (!HAS_DISPLAY(i915)) return; + intel_display_driver_resume_access(i915); + + intel_hpd_init(i915); + /* MST sideband requires HPD interrupts enabled */ intel_dp_mst_resume(i915); @@ -781,6 +785,18 @@ void intel_display_driver_resume(struct drm_i915_private *i915) "Restoring old state failed with %i\n", ret); if (state) drm_atomic_state_put(state); + + intel_display_driver_enable_user_access(i915); + + drm_kms_helper_poll_enable(&i915->drm); + + intel_hpd_poll_disable(i915); + + intel_opregion_resume(&i915->display); + + intel_fbdev_set_suspend(&i915->drm, FBINFO_STATE_RUNNING, false); + + intel_power_domains_enable(i915); } void intel_display_driver_shutdown(struct drm_i915_private *i915) diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c index ac8bf00458b5..ce2dd03136d3 100644 --- a/drivers/gpu/drm/i915/i915_driver.c +++ b/drivers/gpu/drm/i915/i915_driver.c @@ -1165,25 +1165,8 @@ static int i915_drm_resume(struct drm_device *dev) intel_clock_gating_init(dev_priv); - if (HAS_DISPLAY(dev_priv)) - intel_display_driver_resume_access(dev_priv); - - intel_hpd_init(dev_priv); - intel_display_driver_resume(dev_priv); - if (HAS_DISPLAY(dev_priv)) { - intel_display_driver_enable_user_access(dev_priv); - drm_kms_helper_poll_enable(dev); - } - intel_hpd_poll_disable(dev_priv); - - intel_opregion_resume(display); - - intel_fbdev_set_suspend(dev, FBINFO_STATE_RUNNING, false); - - intel_power_domains_enable(dev_priv); - intel_gvt_resume(dev_priv); enable_rpm_wakeref_asserts(&dev_priv->runtime_pm); diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c index a4705a452adb..98ff603a1f84 100644 --- a/drivers/gpu/drm/xe/display/xe_display.c +++ b/drivers/gpu/drm/xe/display/xe_display.c @@ -447,23 +447,7 @@ void xe_display_pm_resume(struct xe_device *xe) intel_display_driver_init_hw(xe); - if (has_display(xe)) - intel_display_driver_resume_access(xe); - - intel_hpd_init(xe); - - if (has_display(xe)) { - intel_display_driver_resume(xe); - drm_kms_helper_poll_enable(&xe->drm); - intel_display_driver_enable_user_access(xe); - intel_hpd_poll_disable(xe); - } - - intel_opregion_resume(display); - - intel_fbdev_set_suspend(&xe->drm, FBINFO_STATE_RUNNING, false); - - intel_power_domains_enable(xe); + intel_display_driver_resume(xe); } void xe_display_pm_runtime_resume(struct xe_device *xe)
Same sequences are currently executed by i915 and xe during system resume. Consolidate them in the intel_display_driver side. Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> --- .../drm/i915/display/intel_display_driver.c | 16 ++++++++++++++++ drivers/gpu/drm/i915/i915_driver.c | 17 ----------------- drivers/gpu/drm/xe/display/xe_display.c | 18 +----------------- 3 files changed, 17 insertions(+), 34 deletions(-)