On Mon, Oct 07, 2024 at 05:02:04PM -0400, Cavitt, Jonathan wrote: > -----Original Message----- > From: Intel-gfx <[email protected]> On Behalf Of > Rodrigo Vivi > Sent: Tuesday, September 24, 2024 1:36 PM > To: [email protected]; [email protected] > Cc: Deak, Imre <[email protected]>; Vivi, Rodrigo <[email protected]> > 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 <[email protected]> > > --- > > .../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 <[email protected]> > -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 > > > >
