| Message ID | 20241212170913.185939-2-tzimmermann@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/{i915,xe}: Convert to DRM client setup | expand |
On Thu, Dec 12, 2024 at 06:08:42PM +0100, Thomas Zimmermann wrote: > Implement drm_client_dev_suspend() and drm_client_dev_resume() for > i915's fbdev emulation and call the helper via DRM client interfaces. > This is required to convert i915 and xe to DRM's generic fbdev client. > No functional changes. Well, there is one functional change that is the clientlist iterator and the clientlist_mutex hold. So I would avoid the statement of no functional change. But indeed it seems there's not relevant functional changes... the code itself looks right to me: Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- > drivers/gpu/drm/i915/display/intel_fbdev.c | 16 ++++++++++++++++ > drivers/gpu/drm/i915/i915_driver.c | 9 +++++---- > drivers/gpu/drm/xe/display/xe_display.c | 9 +++++---- > 3 files changed, 26 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c > index 00852ff5b247..a51d1dfd1b58 100644 > --- a/drivers/gpu/drm/i915/display/intel_fbdev.c > +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c > @@ -642,11 +642,27 @@ static int intel_fbdev_client_hotplug(struct drm_client_dev *client) > return ret; > } > > +static int intel_fbdev_client_suspend(struct drm_client_dev *client, bool holds_console_lock) > +{ > + intel_fbdev_set_suspend(client->dev, FBINFO_STATE_SUSPENDED, true); > + > + return 0; > +} > + > +static int intel_fbdev_client_resume(struct drm_client_dev *client, bool holds_console_lock) > +{ > + intel_fbdev_set_suspend(client->dev, FBINFO_STATE_RUNNING, false); > + > + return 0; > +} > + > static const struct drm_client_funcs intel_fbdev_client_funcs = { > .owner = THIS_MODULE, > .unregister = intel_fbdev_client_unregister, > .restore = intel_fbdev_client_restore, > .hotplug = intel_fbdev_client_hotplug, > + .suspend = intel_fbdev_client_suspend, > + .resume = intel_fbdev_client_resume, > }; > > void intel_fbdev_setup(struct drm_i915_private *i915) > diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c > index eb3fcc9e77a5..e385e4947a91 100644 > --- a/drivers/gpu/drm/i915/i915_driver.c > +++ b/drivers/gpu/drm/i915/i915_driver.c > @@ -41,6 +41,8 @@ > #include <linux/vt.h> > > #include <drm/drm_atomic_helper.h> > +#include <drm/drm_client.h> > +#include <drm/drm_client_event.h> > #include <drm/drm_ioctl.h> > #include <drm/drm_managed.h> > #include <drm/drm_probe_helper.h> > @@ -55,7 +57,6 @@ > #include "display/intel_dp.h" > #include "display/intel_dpt.h" > #include "display/intel_encoder.h" > -#include "display/intel_fbdev.h" > #include "display/intel_hotplug.h" > #include "display/intel_overlay.h" > #include "display/intel_pch_refclk.h" > @@ -958,7 +959,7 @@ void i915_driver_shutdown(struct drm_i915_private *i915) > intel_runtime_pm_disable(&i915->runtime_pm); > intel_power_domains_disable(display); > > - intel_fbdev_set_suspend(&i915->drm, FBINFO_STATE_SUSPENDED, true); > + drm_client_dev_suspend(&i915->drm, false); > if (HAS_DISPLAY(i915)) { > drm_kms_helper_poll_disable(&i915->drm); > intel_display_driver_disable_user_access(display); > @@ -1041,7 +1042,7 @@ static int i915_drm_suspend(struct drm_device *dev) > /* We do a lot of poking in a lot of registers, make sure they work > * properly. */ > intel_power_domains_disable(display); > - intel_fbdev_set_suspend(dev, FBINFO_STATE_SUSPENDED, true); > + drm_client_dev_suspend(dev, false); > if (HAS_DISPLAY(dev_priv)) { > drm_kms_helper_poll_disable(dev); > intel_display_driver_disable_user_access(display); > @@ -1227,7 +1228,7 @@ static int i915_drm_resume(struct drm_device *dev) > > intel_opregion_resume(display); > > - intel_fbdev_set_suspend(dev, FBINFO_STATE_RUNNING, false); > + drm_client_dev_resume(dev, false); > > intel_power_domains_enable(display); > > diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c > index 317fa66adf18..bc73c9999c57 100644 > --- a/drivers/gpu/drm/xe/display/xe_display.c > +++ b/drivers/gpu/drm/xe/display/xe_display.c > @@ -8,6 +8,8 @@ > > #include <linux/fb.h> > > +#include <drm/drm_client.h> > +#include <drm/drm_client_event.h> > #include <drm/drm_drv.h> > #include <drm/drm_managed.h> > #include <drm/drm_probe_helper.h> > @@ -25,7 +27,6 @@ > #include "intel_dmc_wl.h" > #include "intel_dp.h" > #include "intel_encoder.h" > -#include "intel_fbdev.h" > #include "intel_hdcp.h" > #include "intel_hotplug.h" > #include "intel_opregion.h" > @@ -336,7 +337,7 @@ static void __xe_display_pm_suspend(struct xe_device *xe, bool runtime) > */ > intel_power_domains_disable(display); > if (!runtime) > - intel_fbdev_set_suspend(&xe->drm, FBINFO_STATE_SUSPENDED, true); > + drm_client_dev_suspend(&xe->drm, false); > > if (!runtime && has_display(xe)) { > drm_kms_helper_poll_disable(&xe->drm); > @@ -374,7 +375,7 @@ void xe_display_pm_shutdown(struct xe_device *xe) > return; > > intel_power_domains_disable(display); > - intel_fbdev_set_suspend(&xe->drm, FBINFO_STATE_SUSPENDED, true); > + drm_client_dev_suspend(&xe->drm, false); > if (has_display(xe)) { > drm_kms_helper_poll_disable(&xe->drm); > intel_display_driver_disable_user_access(display); > @@ -494,7 +495,7 @@ static void __xe_display_pm_resume(struct xe_device *xe, bool runtime) > intel_opregion_resume(display); > > if (!runtime) > - intel_fbdev_set_suspend(&xe->drm, FBINFO_STATE_RUNNING, false); > + drm_client_dev_resume(&xe->drm, false); > > intel_power_domains_enable(display); > } > -- > 2.47.1 >
Hi Am 11.02.25 um 21:55 schrieb Rodrigo Vivi: > On Thu, Dec 12, 2024 at 06:08:42PM +0100, Thomas Zimmermann wrote: >> Implement drm_client_dev_suspend() and drm_client_dev_resume() for >> i915's fbdev emulation and call the helper via DRM client interfaces. >> This is required to convert i915 and xe to DRM's generic fbdev client. >> No functional changes. > Well, there is one functional change that is the clientlist iterator > and the clientlist_mutex hold. So I would avoid the statement of > no functional change. But indeed it seems there's not relevant > functional changes... Fair point. I'll reword this. > > the code itself looks right to me: > > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> Thanks Best regards Thomas > >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >> --- >> drivers/gpu/drm/i915/display/intel_fbdev.c | 16 ++++++++++++++++ >> drivers/gpu/drm/i915/i915_driver.c | 9 +++++---- >> drivers/gpu/drm/xe/display/xe_display.c | 9 +++++---- >> 3 files changed, 26 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c >> index 00852ff5b247..a51d1dfd1b58 100644 >> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c >> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c >> @@ -642,11 +642,27 @@ static int intel_fbdev_client_hotplug(struct drm_client_dev *client) >> return ret; >> } >> >> +static int intel_fbdev_client_suspend(struct drm_client_dev *client, bool holds_console_lock) >> +{ >> + intel_fbdev_set_suspend(client->dev, FBINFO_STATE_SUSPENDED, true); >> + >> + return 0; >> +} >> + >> +static int intel_fbdev_client_resume(struct drm_client_dev *client, bool holds_console_lock) >> +{ >> + intel_fbdev_set_suspend(client->dev, FBINFO_STATE_RUNNING, false); >> + >> + return 0; >> +} >> + >> static const struct drm_client_funcs intel_fbdev_client_funcs = { >> .owner = THIS_MODULE, >> .unregister = intel_fbdev_client_unregister, >> .restore = intel_fbdev_client_restore, >> .hotplug = intel_fbdev_client_hotplug, >> + .suspend = intel_fbdev_client_suspend, >> + .resume = intel_fbdev_client_resume, >> }; >> >> void intel_fbdev_setup(struct drm_i915_private *i915) >> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c >> index eb3fcc9e77a5..e385e4947a91 100644 >> --- a/drivers/gpu/drm/i915/i915_driver.c >> +++ b/drivers/gpu/drm/i915/i915_driver.c >> @@ -41,6 +41,8 @@ >> #include <linux/vt.h> >> >> #include <drm/drm_atomic_helper.h> >> +#include <drm/drm_client.h> >> +#include <drm/drm_client_event.h> >> #include <drm/drm_ioctl.h> >> #include <drm/drm_managed.h> >> #include <drm/drm_probe_helper.h> >> @@ -55,7 +57,6 @@ >> #include "display/intel_dp.h" >> #include "display/intel_dpt.h" >> #include "display/intel_encoder.h" >> -#include "display/intel_fbdev.h" >> #include "display/intel_hotplug.h" >> #include "display/intel_overlay.h" >> #include "display/intel_pch_refclk.h" >> @@ -958,7 +959,7 @@ void i915_driver_shutdown(struct drm_i915_private *i915) >> intel_runtime_pm_disable(&i915->runtime_pm); >> intel_power_domains_disable(display); >> >> - intel_fbdev_set_suspend(&i915->drm, FBINFO_STATE_SUSPENDED, true); >> + drm_client_dev_suspend(&i915->drm, false); >> if (HAS_DISPLAY(i915)) { >> drm_kms_helper_poll_disable(&i915->drm); >> intel_display_driver_disable_user_access(display); >> @@ -1041,7 +1042,7 @@ static int i915_drm_suspend(struct drm_device *dev) >> /* We do a lot of poking in a lot of registers, make sure they work >> * properly. */ >> intel_power_domains_disable(display); >> - intel_fbdev_set_suspend(dev, FBINFO_STATE_SUSPENDED, true); >> + drm_client_dev_suspend(dev, false); >> if (HAS_DISPLAY(dev_priv)) { >> drm_kms_helper_poll_disable(dev); >> intel_display_driver_disable_user_access(display); >> @@ -1227,7 +1228,7 @@ static int i915_drm_resume(struct drm_device *dev) >> >> intel_opregion_resume(display); >> >> - intel_fbdev_set_suspend(dev, FBINFO_STATE_RUNNING, false); >> + drm_client_dev_resume(dev, false); >> >> intel_power_domains_enable(display); >> >> diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c >> index 317fa66adf18..bc73c9999c57 100644 >> --- a/drivers/gpu/drm/xe/display/xe_display.c >> +++ b/drivers/gpu/drm/xe/display/xe_display.c >> @@ -8,6 +8,8 @@ >> >> #include <linux/fb.h> >> >> +#include <drm/drm_client.h> >> +#include <drm/drm_client_event.h> >> #include <drm/drm_drv.h> >> #include <drm/drm_managed.h> >> #include <drm/drm_probe_helper.h> >> @@ -25,7 +27,6 @@ >> #include "intel_dmc_wl.h" >> #include "intel_dp.h" >> #include "intel_encoder.h" >> -#include "intel_fbdev.h" >> #include "intel_hdcp.h" >> #include "intel_hotplug.h" >> #include "intel_opregion.h" >> @@ -336,7 +337,7 @@ static void __xe_display_pm_suspend(struct xe_device *xe, bool runtime) >> */ >> intel_power_domains_disable(display); >> if (!runtime) >> - intel_fbdev_set_suspend(&xe->drm, FBINFO_STATE_SUSPENDED, true); >> + drm_client_dev_suspend(&xe->drm, false); >> >> if (!runtime && has_display(xe)) { >> drm_kms_helper_poll_disable(&xe->drm); >> @@ -374,7 +375,7 @@ void xe_display_pm_shutdown(struct xe_device *xe) >> return; >> >> intel_power_domains_disable(display); >> - intel_fbdev_set_suspend(&xe->drm, FBINFO_STATE_SUSPENDED, true); >> + drm_client_dev_suspend(&xe->drm, false); >> if (has_display(xe)) { >> drm_kms_helper_poll_disable(&xe->drm); >> intel_display_driver_disable_user_access(display); >> @@ -494,7 +495,7 @@ static void __xe_display_pm_resume(struct xe_device *xe, bool runtime) >> intel_opregion_resume(display); >> >> if (!runtime) >> - intel_fbdev_set_suspend(&xe->drm, FBINFO_STATE_RUNNING, false); >> + drm_client_dev_resume(&xe->drm, false); >> >> intel_power_domains_enable(display); >> } >> -- >> 2.47.1 >>
diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c index 00852ff5b247..a51d1dfd1b58 100644 --- a/drivers/gpu/drm/i915/display/intel_fbdev.c +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c @@ -642,11 +642,27 @@ static int intel_fbdev_client_hotplug(struct drm_client_dev *client) return ret; } +static int intel_fbdev_client_suspend(struct drm_client_dev *client, bool holds_console_lock) +{ + intel_fbdev_set_suspend(client->dev, FBINFO_STATE_SUSPENDED, true); + + return 0; +} + +static int intel_fbdev_client_resume(struct drm_client_dev *client, bool holds_console_lock) +{ + intel_fbdev_set_suspend(client->dev, FBINFO_STATE_RUNNING, false); + + return 0; +} + static const struct drm_client_funcs intel_fbdev_client_funcs = { .owner = THIS_MODULE, .unregister = intel_fbdev_client_unregister, .restore = intel_fbdev_client_restore, .hotplug = intel_fbdev_client_hotplug, + .suspend = intel_fbdev_client_suspend, + .resume = intel_fbdev_client_resume, }; void intel_fbdev_setup(struct drm_i915_private *i915) diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c index eb3fcc9e77a5..e385e4947a91 100644 --- a/drivers/gpu/drm/i915/i915_driver.c +++ b/drivers/gpu/drm/i915/i915_driver.c @@ -41,6 +41,8 @@ #include <linux/vt.h> #include <drm/drm_atomic_helper.h> +#include <drm/drm_client.h> +#include <drm/drm_client_event.h> #include <drm/drm_ioctl.h> #include <drm/drm_managed.h> #include <drm/drm_probe_helper.h> @@ -55,7 +57,6 @@ #include "display/intel_dp.h" #include "display/intel_dpt.h" #include "display/intel_encoder.h" -#include "display/intel_fbdev.h" #include "display/intel_hotplug.h" #include "display/intel_overlay.h" #include "display/intel_pch_refclk.h" @@ -958,7 +959,7 @@ void i915_driver_shutdown(struct drm_i915_private *i915) intel_runtime_pm_disable(&i915->runtime_pm); intel_power_domains_disable(display); - intel_fbdev_set_suspend(&i915->drm, FBINFO_STATE_SUSPENDED, true); + drm_client_dev_suspend(&i915->drm, false); if (HAS_DISPLAY(i915)) { drm_kms_helper_poll_disable(&i915->drm); intel_display_driver_disable_user_access(display); @@ -1041,7 +1042,7 @@ static int i915_drm_suspend(struct drm_device *dev) /* We do a lot of poking in a lot of registers, make sure they work * properly. */ intel_power_domains_disable(display); - intel_fbdev_set_suspend(dev, FBINFO_STATE_SUSPENDED, true); + drm_client_dev_suspend(dev, false); if (HAS_DISPLAY(dev_priv)) { drm_kms_helper_poll_disable(dev); intel_display_driver_disable_user_access(display); @@ -1227,7 +1228,7 @@ static int i915_drm_resume(struct drm_device *dev) intel_opregion_resume(display); - intel_fbdev_set_suspend(dev, FBINFO_STATE_RUNNING, false); + drm_client_dev_resume(dev, false); intel_power_domains_enable(display); diff --git a/drivers/gpu/drm/xe/display/xe_display.c b/drivers/gpu/drm/xe/display/xe_display.c index 317fa66adf18..bc73c9999c57 100644 --- a/drivers/gpu/drm/xe/display/xe_display.c +++ b/drivers/gpu/drm/xe/display/xe_display.c @@ -8,6 +8,8 @@ #include <linux/fb.h> +#include <drm/drm_client.h> +#include <drm/drm_client_event.h> #include <drm/drm_drv.h> #include <drm/drm_managed.h> #include <drm/drm_probe_helper.h> @@ -25,7 +27,6 @@ #include "intel_dmc_wl.h" #include "intel_dp.h" #include "intel_encoder.h" -#include "intel_fbdev.h" #include "intel_hdcp.h" #include "intel_hotplug.h" #include "intel_opregion.h" @@ -336,7 +337,7 @@ static void __xe_display_pm_suspend(struct xe_device *xe, bool runtime) */ intel_power_domains_disable(display); if (!runtime) - intel_fbdev_set_suspend(&xe->drm, FBINFO_STATE_SUSPENDED, true); + drm_client_dev_suspend(&xe->drm, false); if (!runtime && has_display(xe)) { drm_kms_helper_poll_disable(&xe->drm); @@ -374,7 +375,7 @@ void xe_display_pm_shutdown(struct xe_device *xe) return; intel_power_domains_disable(display); - intel_fbdev_set_suspend(&xe->drm, FBINFO_STATE_SUSPENDED, true); + drm_client_dev_suspend(&xe->drm, false); if (has_display(xe)) { drm_kms_helper_poll_disable(&xe->drm); intel_display_driver_disable_user_access(display); @@ -494,7 +495,7 @@ static void __xe_display_pm_resume(struct xe_device *xe, bool runtime) intel_opregion_resume(display); if (!runtime) - intel_fbdev_set_suspend(&xe->drm, FBINFO_STATE_RUNNING, false); + drm_client_dev_resume(&xe->drm, false); intel_power_domains_enable(display); }
Implement drm_client_dev_suspend() and drm_client_dev_resume() for i915's fbdev emulation and call the helper via DRM client interfaces. This is required to convert i915 and xe to DRM's generic fbdev client. No functional changes. Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> --- drivers/gpu/drm/i915/display/intel_fbdev.c | 16 ++++++++++++++++ drivers/gpu/drm/i915/i915_driver.c | 9 +++++---- drivers/gpu/drm/xe/display/xe_display.c | 9 +++++---- 3 files changed, 26 insertions(+), 8 deletions(-)