[PATCH 0/8] drm/{amdgpu,nouveau}: Remove old fbdev hooks

[PATCH 0/8] drm/{amdgpu,nouveau}: Remove old fbdev hooks

[Thomas Zimmermann]

After switching all drivers' fbdev emulation to DRM client, the
old fbdev hooks are now obsolete. Only amgdpu and nouveau still use
them in a several places. Remove the hooks from the drivers and the
DRM core.

The series would ideally be merged at once via drm-misc-next.

Thomas Zimmermann (8):
  drm/fbdev-helper: Do delayed switcheroo in lastclose helper
  drm/amdgpu: Do not set struct drm_driver.lastclose
  drm/nouveau: Do not set struct drm_driver.lastclose
  drm/nouveau: Do not set struct
    drm_mode_config_funcs.output_poll_changed
  drm/nouveau: Implement switcheroo reprobe with
    drm_client_dev_hotplug()
  drm/fbdev-helper: Remove drm_fb_helper_output_poll_changed()
  drm: Remove struct drm_driver.lastclose
  drm: Remove struct drm_mode_config_funcs.output_poll_changed

 drivers/gpu/drm/amd/amdgpu/amdgpu.h       |  1 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   |  2 --
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c   | 17 -----------
 drivers/gpu/drm/drm_drv.c                 |  5 ++-
 drivers/gpu/drm/drm_fb_helper.c           | 37 ++++++-----------------
 drivers/gpu/drm/drm_file.c                | 35 +++++----------------
 drivers/gpu/drm/drm_internal.h            |  1 -
 drivers/gpu/drm/drm_probe_helper.c        | 10 +-----
 drivers/gpu/drm/nouveau/dispnv50/disp.c   |  1 -
 drivers/gpu/drm/nouveau/nouveau_display.c |  1 -
 drivers/gpu/drm/nouveau/nouveau_drm.c     |  1 -
 drivers/gpu/drm/nouveau/nouveau_vga.c     | 10 ++----
 drivers/gpu/drm/nouveau/nouveau_vga.h     |  1 -
 include/drm/drm_client.h                  |  7 +++--
 include/drm/drm_drv.h                     | 28 -----------------
 include/drm/drm_fb_helper.h               |  6 ----
 include/drm/drm_mode_config.h             | 16 ----------
 17 files changed, 25 insertions(+), 154 deletions(-)

-- 
2.46.0

[PATCH 1/8] drm/fbdev-helper: Do delayed switcheroo in lastclose helper

[Thomas Zimmermann]

Amdgpu and nouveau call vga_switcheroo_process_delayed_switch() from
their lastclose callbacks. Call it from the fbdev lastclose helper,
so that the driver functions can finally be removed.

The fbdev call is part of all lastclose handling that restores the
DRM fbcon terminal. Only PCI devices with enabled switcheroo do the
delayed switching. The call has no effect on other drivers.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_fb_helper.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 3f7da78849e4..f6667dfba8a2 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2009,6 +2009,10 @@ EXPORT_SYMBOL(drm_fb_helper_hotplug_event);
 void drm_fb_helper_lastclose(struct drm_device *dev)
 {
 	drm_fb_helper_restore_fbdev_mode_unlocked(dev->fb_helper);
+
+	if (dev_is_pci(dev->dev))
+		vga_switcheroo_process_delayed_switch();
+
 }
 EXPORT_SYMBOL(drm_fb_helper_lastclose);
 
-- 
2.46.0

Re: [PATCH 1/8] drm/fbdev-helper: Do delayed switcheroo in lastclose helper

[Daniel Vetter]

On Wed, Aug 07, 2024 at 10:41:33AM +0200, Thomas Zimmermann wrote:
> Amdgpu and nouveau call vga_switcheroo_process_delayed_switch() from
> their lastclose callbacks. Call it from the fbdev lastclose helper,
> so that the driver functions can finally be removed.
> 
> The fbdev call is part of all lastclose handling that restores the
> DRM fbcon terminal. Only PCI devices with enabled switcheroo do the
> delayed switching. The call has no effect on other drivers.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/drm_fb_helper.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 3f7da78849e4..f6667dfba8a2 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -2009,6 +2009,10 @@ EXPORT_SYMBOL(drm_fb_helper_hotplug_event);
>  void drm_fb_helper_lastclose(struct drm_device *dev)
>  {
>  	drm_fb_helper_restore_fbdev_mode_unlocked(dev->fb_helper);
> +
> +	if (dev_is_pci(dev->dev))
> +		vga_switcheroo_process_delayed_switch();

I think if you want to move this, it needs to be in drm core. Otherwise
the vgaswitcheroo delayed switching stops working if you disable fbdev
support. Which doesn't make much sense.
-Sima

> +
>  }
>  EXPORT_SYMBOL(drm_fb_helper_lastclose);
>  
> -- 
> 2.46.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 1/8] drm/fbdev-helper: Do delayed switcheroo in lastclose helper

[Thomas Zimmermann]

Hi

Am 09.08.24 um 10:44 schrieb Daniel Vetter:
> On Wed, Aug 07, 2024 at 10:41:33AM +0200, Thomas Zimmermann wrote:
>> Amdgpu and nouveau call vga_switcheroo_process_delayed_switch() from
>> their lastclose callbacks. Call it from the fbdev lastclose helper,
>> so that the driver functions can finally be removed.
>>
>> The fbdev call is part of all lastclose handling that restores the
>> DRM fbcon terminal. Only PCI devices with enabled switcheroo do the
>> delayed switching. The call has no effect on other drivers.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>   drivers/gpu/drm/drm_fb_helper.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
>> index 3f7da78849e4..f6667dfba8a2 100644
>> --- a/drivers/gpu/drm/drm_fb_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> @@ -2009,6 +2009,10 @@ EXPORT_SYMBOL(drm_fb_helper_hotplug_event);
>>   void drm_fb_helper_lastclose(struct drm_device *dev)
>>   {
>>   	drm_fb_helper_restore_fbdev_mode_unlocked(dev->fb_helper);
>> +
>> +	if (dev_is_pci(dev->dev))
>> +		vga_switcheroo_process_delayed_switch();
> I think if you want to move this, it needs to be in drm core. Otherwise
> the vgaswitcheroo delayed switching stops working if you disable fbdev
> support. Which doesn't make much sense.

Thanks for reviewing. This call can then go directly into 
drm_lastclose(), which will stay around.

Best regards
Thomas

> -Sima
>
>> +
>>   }
>>   EXPORT_SYMBOL(drm_fb_helper_lastclose);
>>   
>> -- 
>> 2.46.0
>>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

[PATCH 2/8] drm/amdgpu: Do not set struct drm_driver.lastclose

[Thomas Zimmermann]

Remove the implementation of struct drm_driver.lastclose. The hook
was only necessary before in-kernel DRM clients existed, but is now
obsolete.

The code within amdgpu_driver_lastclose_kms() is performed elsewhere
in the DRM fbdev emulation.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  1 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  2 --
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 17 -----------------
 3 files changed, 20 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 137a88b8de45..4baeb6519fda 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1484,7 +1484,6 @@ extern const int amdgpu_max_kms_ioctl;
 
 int amdgpu_driver_load_kms(struct amdgpu_device *adev, unsigned long flags);
 void amdgpu_driver_unload_kms(struct drm_device *dev);
-void amdgpu_driver_lastclose_kms(struct drm_device *dev);
 int amdgpu_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv);
 void amdgpu_driver_postclose_kms(struct drm_device *dev,
 				 struct drm_file *file_priv);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 094498a0964b..5dd39e6c6223 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2953,7 +2953,6 @@ static const struct drm_driver amdgpu_kms_driver = {
 	    DRIVER_SYNCOBJ_TIMELINE,
 	.open = amdgpu_driver_open_kms,
 	.postclose = amdgpu_driver_postclose_kms,
-	.lastclose = amdgpu_driver_lastclose_kms,
 	.ioctls = amdgpu_ioctls_kms,
 	.num_ioctls = ARRAY_SIZE(amdgpu_ioctls_kms),
 	.dumb_create = amdgpu_mode_dumb_create,
@@ -2980,7 +2979,6 @@ const struct drm_driver amdgpu_partition_driver = {
 	    DRIVER_SYNCOBJ_TIMELINE,
 	.open = amdgpu_driver_open_kms,
 	.postclose = amdgpu_driver_postclose_kms,
-	.lastclose = amdgpu_driver_lastclose_kms,
 	.ioctls = amdgpu_ioctls_kms,
 	.num_ioctls = ARRAY_SIZE(amdgpu_ioctls_kms),
 	.dumb_create = amdgpu_mode_dumb_create,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 66782be5917b..0a799942343d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -1269,23 +1269,6 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
 	return 0;
 }
 
-
-/*
- * Outdated mess for old drm with Xorg being in charge (void function now).
- */
-/**
- * amdgpu_driver_lastclose_kms - drm callback for last close
- *
- * @dev: drm dev pointer
- *
- * Switch vga_switcheroo state after last close (all asics).
- */
-void amdgpu_driver_lastclose_kms(struct drm_device *dev)
-{
-	drm_fb_helper_lastclose(dev);
-	vga_switcheroo_process_delayed_switch();
-}
-
 /**
  * amdgpu_driver_open_kms - drm callback for open
  *
-- 
2.46.0

[PATCH 3/8] drm/nouveau: Do not set struct drm_driver.lastclose

[Thomas Zimmermann]

Remove the implementation of struct drm_driver.lastclose. The hook
was only necessary before in-kernel DRM clients existed, but is now
obsolete.

The code within nouveau_vga_lastclose() is performed elsewhere in the
DRM fbdev emulation.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/nouveau/nouveau_drm.c | 1 -
 drivers/gpu/drm/nouveau/nouveau_vga.c | 7 -------
 drivers/gpu/drm/nouveau/nouveau_vga.h | 1 -
 3 files changed, 9 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
index ac7c60fb14d3..4a9a9b9c3935 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -1303,7 +1303,6 @@ driver_stub = {
 			   DRIVER_RENDER,
 	.open = nouveau_drm_open,
 	.postclose = nouveau_drm_postclose,
-	.lastclose = nouveau_vga_lastclose,
 
 #if defined(CONFIG_DEBUG_FS)
 	.debugfs_init = nouveau_drm_debugfs_init,
diff --git a/drivers/gpu/drm/nouveau/nouveau_vga.c b/drivers/gpu/drm/nouveau/nouveau_vga.c
index 2525e08938b3..ee637f1fe03d 100644
--- a/drivers/gpu/drm/nouveau/nouveau_vga.c
+++ b/drivers/gpu/drm/nouveau/nouveau_vga.c
@@ -127,10 +127,3 @@ nouveau_vga_fini(struct nouveau_drm *drm)
 	if (runtime && nouveau_is_v1_dsm() && !nouveau_is_optimus())
 		vga_switcheroo_fini_domain_pm_ops(drm->dev->dev);
 }
-
-
-void
-nouveau_vga_lastclose(struct drm_device *dev)
-{
-	vga_switcheroo_process_delayed_switch();
-}
diff --git a/drivers/gpu/drm/nouveau/nouveau_vga.h b/drivers/gpu/drm/nouveau/nouveau_vga.h
index 951a83f984dd..63be415d2a44 100644
--- a/drivers/gpu/drm/nouveau/nouveau_vga.h
+++ b/drivers/gpu/drm/nouveau/nouveau_vga.h
@@ -4,6 +4,5 @@
 
 void nouveau_vga_init(struct nouveau_drm *);
 void nouveau_vga_fini(struct nouveau_drm *);
-void nouveau_vga_lastclose(struct drm_device *dev);
 
 #endif
-- 
2.46.0

[PATCH 4/8] drm/nouveau: Do not set struct drm_mode_config_funcs.output_poll_changed

[Thomas Zimmermann]

The output_poll_changed hook was only necessary before in-kernel
DRM clients existed, but is now obsolete. The client code handles
display otplugging internally.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c   | 1 -
 drivers/gpu/drm/nouveau/nouveau_display.c | 1 -
 2 files changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index e4c8ce6dd40a..eed579a6c858 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -2648,7 +2648,6 @@ nv50_disp_atomic_state_alloc(struct drm_device *dev)
 static const struct drm_mode_config_funcs
 nv50_disp_func = {
 	.fb_create = nouveau_user_framebuffer_create,
-	.output_poll_changed = drm_fb_helper_output_poll_changed,
 	.atomic_check = nv50_disp_atomic_check,
 	.atomic_commit = nv50_disp_atomic_commit,
 	.atomic_state_alloc = nv50_disp_atomic_state_alloc,
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
index 8a87e9697a42..e2fd561cd23f 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -391,7 +391,6 @@ nouveau_user_framebuffer_create(struct drm_device *dev,
 
 static const struct drm_mode_config_funcs nouveau_mode_config_funcs = {
 	.fb_create = nouveau_user_framebuffer_create,
-	.output_poll_changed = drm_fb_helper_output_poll_changed,
 };
 
 
-- 
2.46.0

Re: [PATCH 4/8] drm/nouveau: Do not set struct drm_mode_config_funcs.output_poll_changed

[Daniel Vetter]

On Wed, Aug 07, 2024 at 10:41:36AM +0200, Thomas Zimmermann wrote:
> The output_poll_changed hook was only necessary before in-kernel
> DRM clients existed, but is now obsolete. The client code handles
> display otplugging internally.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/nouveau/dispnv50/disp.c   | 1 -
>  drivers/gpu/drm/nouveau/nouveau_display.c | 1 -
>  2 files changed, 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> index e4c8ce6dd40a..eed579a6c858 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> @@ -2648,7 +2648,6 @@ nv50_disp_atomic_state_alloc(struct drm_device *dev)
>  static const struct drm_mode_config_funcs
>  nv50_disp_func = {
>  	.fb_create = nouveau_user_framebuffer_create,
> -	.output_poll_changed = drm_fb_helper_output_poll_changed,
>  	.atomic_check = nv50_disp_atomic_check,
>  	.atomic_commit = nv50_disp_atomic_commit,
>  	.atomic_state_alloc = nv50_disp_atomic_state_alloc,
> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
> index 8a87e9697a42..e2fd561cd23f 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_display.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_display.c
> @@ -391,7 +391,6 @@ nouveau_user_framebuffer_create(struct drm_device *dev,
>  
>  static const struct drm_mode_config_funcs nouveau_mode_config_funcs = {
>  	.fb_create = nouveau_user_framebuffer_create,
> -	.output_poll_changed = drm_fb_helper_output_poll_changed,
>  };
>  
>  
> -- 
> 2.46.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH 5/8] drm/nouveau: Implement switcheroo reprobe with drm_client_dev_hotplug()

[Thomas Zimmermann]

Replace the callto drm_fb_helper_output_poll_changed() with a call
to drm_client_dev_hotplug(). It's equivalent in functionality, but
use the DRM client infrastructure.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/nouveau/nouveau_vga.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_vga.c b/drivers/gpu/drm/nouveau/nouveau_vga.c
index ee637f1fe03d..ab4e11dc0b8a 100644
--- a/drivers/gpu/drm/nouveau/nouveau_vga.c
+++ b/drivers/gpu/drm/nouveau/nouveau_vga.c
@@ -58,8 +58,9 @@ static void
 nouveau_switcheroo_reprobe(struct pci_dev *pdev)
 {
 	struct nouveau_drm *drm = pci_get_drvdata(pdev);
+	struct drm_device *dev = drm->dev;
 
-	drm_fb_helper_output_poll_changed(drm->dev);
+	drm_client_dev_hotplug(dev);
 }
 
 static bool
-- 
2.46.0

Re: [PATCH 5/8] drm/nouveau: Implement switcheroo reprobe with drm_client_dev_hotplug()

[Daniel Vetter]

On Wed, Aug 07, 2024 at 10:41:37AM +0200, Thomas Zimmermann wrote:
> Replace the callto drm_fb_helper_output_poll_changed() with a call
> to drm_client_dev_hotplug(). It's equivalent in functionality, but
> use the DRM client infrastructure.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/nouveau/nouveau_vga.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_vga.c b/drivers/gpu/drm/nouveau/nouveau_vga.c
> index ee637f1fe03d..ab4e11dc0b8a 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_vga.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_vga.c
> @@ -58,8 +58,9 @@ static void
>  nouveau_switcheroo_reprobe(struct pci_dev *pdev)
>  {
>  	struct nouveau_drm *drm = pci_get_drvdata(pdev);
> +	struct drm_device *dev = drm->dev;
>  
> -	drm_fb_helper_output_poll_changed(drm->dev);
> +	drm_client_dev_hotplug(dev);
>  }
>  
>  static bool
> -- 
> 2.46.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH 6/8] drm/fbdev-helper: Remove drm_fb_helper_output_poll_changed()

[Thomas Zimmermann]

The function is unused. Remove it.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_fb_helper.c | 15 ---------------
 include/drm/drm_fb_helper.h     |  6 ------
 2 files changed, 21 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index f6667dfba8a2..3cafb28236f7 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2015,18 +2015,3 @@ void drm_fb_helper_lastclose(struct drm_device *dev)
 
 }
 EXPORT_SYMBOL(drm_fb_helper_lastclose);
-
-/**
- * drm_fb_helper_output_poll_changed - DRM mode config \.output_poll_changed
- *                                     helper for fbdev emulation
- * @dev: DRM device
- *
- * This function can be used as the
- * &drm_mode_config_funcs.output_poll_changed callback for drivers that only
- * need to call drm_fbdev.hotplug_event().
- */
-void drm_fb_helper_output_poll_changed(struct drm_device *dev)
-{
-	drm_fb_helper_hotplug_event(dev->fb_helper);
-}
-EXPORT_SYMBOL(drm_fb_helper_output_poll_changed);
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index 375737fd6c36..699f2790b9ac 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -271,9 +271,7 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper);
 int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper);
 int drm_fb_helper_debug_enter(struct fb_info *info);
 int drm_fb_helper_debug_leave(struct fb_info *info);
-
 void drm_fb_helper_lastclose(struct drm_device *dev);
-void drm_fb_helper_output_poll_changed(struct drm_device *dev);
 #else
 static inline void drm_fb_helper_prepare(struct drm_device *dev,
 					 struct drm_fb_helper *helper,
@@ -401,10 +399,6 @@ static inline int drm_fb_helper_debug_leave(struct fb_info *info)
 static inline void drm_fb_helper_lastclose(struct drm_device *dev)
 {
 }
-
-static inline void drm_fb_helper_output_poll_changed(struct drm_device *dev)
-{
-}
 #endif
 
 #endif
-- 
2.46.0

Re: [PATCH 6/8] drm/fbdev-helper: Remove drm_fb_helper_output_poll_changed()

[Daniel Vetter]

On Wed, Aug 07, 2024 at 10:41:38AM +0200, Thomas Zimmermann wrote:
> The function is unused. Remove it.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Without the next patch to remove ->lastclose there's some confusion text
left in the DOC: section in drm_fb_helper.c, but no point to split that up
perfectly imo. Was just trying to find it and didn't find it only looking
at the poll_changed patches ...
-Sima

> ---
>  drivers/gpu/drm/drm_fb_helper.c | 15 ---------------
>  include/drm/drm_fb_helper.h     |  6 ------
>  2 files changed, 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index f6667dfba8a2..3cafb28236f7 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -2015,18 +2015,3 @@ void drm_fb_helper_lastclose(struct drm_device *dev)
>  
>  }
>  EXPORT_SYMBOL(drm_fb_helper_lastclose);
> -
> -/**
> - * drm_fb_helper_output_poll_changed - DRM mode config \.output_poll_changed
> - *                                     helper for fbdev emulation
> - * @dev: DRM device
> - *
> - * This function can be used as the
> - * &drm_mode_config_funcs.output_poll_changed callback for drivers that only
> - * need to call drm_fbdev.hotplug_event().
> - */
> -void drm_fb_helper_output_poll_changed(struct drm_device *dev)
> -{
> -	drm_fb_helper_hotplug_event(dev->fb_helper);
> -}
> -EXPORT_SYMBOL(drm_fb_helper_output_poll_changed);
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index 375737fd6c36..699f2790b9ac 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -271,9 +271,7 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper);
>  int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper);
>  int drm_fb_helper_debug_enter(struct fb_info *info);
>  int drm_fb_helper_debug_leave(struct fb_info *info);
> -
>  void drm_fb_helper_lastclose(struct drm_device *dev);
> -void drm_fb_helper_output_poll_changed(struct drm_device *dev);
>  #else
>  static inline void drm_fb_helper_prepare(struct drm_device *dev,
>  					 struct drm_fb_helper *helper,
> @@ -401,10 +399,6 @@ static inline int drm_fb_helper_debug_leave(struct fb_info *info)
>  static inline void drm_fb_helper_lastclose(struct drm_device *dev)
>  {
>  }
> -
> -static inline void drm_fb_helper_output_poll_changed(struct drm_device *dev)
> -{
> -}
>  #endif
>  
>  #endif
> -- 
> 2.46.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH 7/8] drm: Remove struct drm_driver.lastclose

[Thomas Zimmermann]

The lastclose callback in struct drm_driver is unused. Remove it. Also
inline the now trivial drm_lastclose() and update documentation.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_drv.c       |  5 ++---
 drivers/gpu/drm/drm_fb_helper.c | 18 +++++------------
 drivers/gpu/drm/drm_file.c      | 35 +++++++--------------------------
 drivers/gpu/drm/drm_internal.h  |  1 -
 include/drm/drm_client.h        |  7 ++++---
 include/drm/drm_drv.h           | 28 --------------------------
 6 files changed, 18 insertions(+), 76 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 93543071a500..5859838b68cb 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -409,9 +409,8 @@ void drm_minor_release(struct drm_minor *minor)
  * drm_put_dev - Unregister and release a DRM device
  * @dev: DRM device
  *
- * Called at module unload time or when a PCI device is unplugged.
- *
- * Cleans up all DRM device, calling drm_lastclose().
+ * Called at module unload time or when a PCI device is unplugged. Cleans up all DRM
+ * device resrouces.
  *
  * Note: Use of this function is deprecated. It will eventually go away
  * completely.  Please use drm_dev_unregister() and drm_dev_put() explicitly
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 3cafb28236f7..36ac75a2ebb8 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -89,14 +89,6 @@ static DEFINE_MUTEX(kernel_fb_helper_lock);
  * interfaces. Drivers that use one of the shared memory managers, TTM, SHMEM,
  * DMA, should instead use the corresponding fbdev emulation.
  *
- * Existing fbdev implementations should restore the fbdev console by using
- * drm_fb_helper_lastclose() as their &drm_driver.lastclose callback.
- * They should also notify the fb helper code from updates to the output
- * configuration by using drm_fb_helper_output_poll_changed() as their
- * &drm_mode_config_funcs.output_poll_changed callback. New implementations
- * of fbdev should be build on top of struct &drm_client_funcs, which handles
- * this automatically. Setting the old callbacks should be avoided.
- *
  * For suspend/resume consider using drm_mode_config_helper_suspend() and
  * drm_mode_config_helper_resume() which takes care of fbdev as well.
  *
@@ -260,12 +252,12 @@ __drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper,
  * drm_fb_helper_restore_fbdev_mode_unlocked - restore fbdev configuration
  * @fb_helper: driver-allocated fbdev helper, can be NULL
  *
- * This should be called from driver's drm &drm_driver.lastclose callback
- * when implementing an fbcon on top of kms using this helper. This ensures that
- * the user isn't greeted with a black screen when e.g. X dies.
+ * This helper should be called from fbdev emulation's &drm_client_funcs.restore
+ * callback. It ensures that the user isn't greeted with a black screen when the
+ * userspace compositor releases the display device.
  *
- * RETURNS:
- * Zero if everything went ok, negative error code otherwise.
+ * Returns:
+ * 0 on success, or a negative errno code otherwise.
  */
 int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
 {
diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index 714e42b05108..8fc664974e72 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -62,15 +62,6 @@ bool drm_dev_needs_global_mutex(struct drm_device *dev)
 	if (dev->driver->load || dev->driver->unload)
 		return true;
 
-	/*
-	 * Drivers with the lastclose callback assume that it's synchronized
-	 * against concurrent opens, which again needs the BKL. The proper fix
-	 * is to use the drm_client infrastructure with proper locking for each
-	 * client.
-	 */
-	if (dev->driver->lastclose)
-		return true;
-
 	return false;
 }
 
@@ -395,26 +386,15 @@ int drm_open(struct inode *inode, struct file *filp)
 }
 EXPORT_SYMBOL(drm_open);
 
-void drm_lastclose(struct drm_device * dev)
-{
-	drm_dbg_core(dev, "\n");
-
-	if (dev->driver->lastclose)
-		dev->driver->lastclose(dev);
-	drm_dbg_core(dev, "driver lastclose completed\n");
-
-	drm_client_dev_restore(dev);
-}
-
 /**
  * drm_release - release method for DRM file
  * @inode: device inode
  * @filp: file pointer.
  *
  * This function must be used by drivers as their &file_operations.release
- * method. It frees any resources associated with the open file, and calls the
- * &drm_driver.postclose driver callback. If this is the last open file for the
- * DRM device also proceeds to call the &drm_driver.lastclose driver callback.
+ * method. It frees any resources associated with the open file. If this
+ * is the last open file for the DRM device, it also restores the active
+ * in-kernel DRM client.
  *
  * RETURNS:
  *
@@ -434,7 +414,7 @@ int drm_release(struct inode *inode, struct file *filp)
 	drm_close_helper(filp);
 
 	if (atomic_dec_and_test(&dev->open_count))
-		drm_lastclose(dev);
+		drm_client_dev_restore(dev);
 
 	if (drm_dev_needs_global_mutex(dev))
 		mutex_unlock(&drm_global_mutex);
@@ -484,9 +464,8 @@ void drm_file_update_pid(struct drm_file *filp)
  *
  * This function may be used by drivers as their &file_operations.release
  * method. It frees any resources associated with the open file prior to taking
- * the drm_global_mutex, which then calls the &drm_driver.postclose driver
- * callback. If this is the last open file for the DRM device also proceeds to
- * call the &drm_driver.lastclose driver callback.
+ * the drm_global_mutex. If this is the last open file for the DRM device, it
+ * then restores the active in-kernel DRM client.
  *
  * RETURNS:
  *
@@ -501,7 +480,7 @@ int drm_release_noglobal(struct inode *inode, struct file *filp)
 	drm_close_helper(filp);
 
 	if (atomic_dec_and_mutex_lock(&dev->open_count, &drm_global_mutex)) {
-		drm_lastclose(dev);
+		drm_client_dev_restore(dev);
 		mutex_unlock(&drm_global_mutex);
 	}
 
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index 690505a1f7a5..23c99803af44 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -53,7 +53,6 @@ extern struct mutex drm_global_mutex;
 bool drm_dev_needs_global_mutex(struct drm_device *dev);
 struct drm_file *drm_file_alloc(struct drm_minor *minor);
 void drm_file_free(struct drm_file *file);
-void drm_lastclose(struct drm_device *dev);
 
 #ifdef CONFIG_PCI
 
diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
index bc0e66f9c425..ddf38b210254 100644
--- a/include/drm/drm_client.h
+++ b/include/drm/drm_client.h
@@ -41,9 +41,10 @@ struct drm_client_funcs {
 	/**
 	 * @restore:
 	 *
-	 * Called on drm_lastclose(). The first client instance in the list that
-	 * returns zero gets the privilege to restore and no more clients are
-	 * called. This callback is not called after @unregister has been called.
+	 * Called userspace closes the final reference to the DRM device file. The
+	 * first client instance in the list that returns zero gets the privilege
+	 * to restore and no more clients are called. This callback is not called
+	 * after @unregister has been called.
 	 *
 	 * Note that the core does not guarantee exclusion against concurrent
 	 * drm_open(). Clients need to ensure this themselves, for example by
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index cd37936c3926..02ea4e3248fd 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -228,34 +228,6 @@ struct drm_driver {
 	 */
 	void (*postclose) (struct drm_device *, struct drm_file *);
 
-	/**
-	 * @lastclose:
-	 *
-	 * Called when the last &struct drm_file has been closed and there's
-	 * currently no userspace client for the &struct drm_device.
-	 *
-	 * Modern drivers should only use this to force-restore the fbdev
-	 * framebuffer using drm_fb_helper_restore_fbdev_mode_unlocked().
-	 * Anything else would indicate there's something seriously wrong.
-	 * Modern drivers can also use this to execute delayed power switching
-	 * state changes, e.g. in conjunction with the :ref:`vga_switcheroo`
-	 * infrastructure.
-	 *
-	 * This is called after @postclose hook has been called.
-	 *
-	 * NOTE:
-	 *
-	 * All legacy drivers use this callback to de-initialize the hardware.
-	 * This is purely because of the shadow-attach model, where the DRM
-	 * kernel driver does not really own the hardware. Instead ownershipe is
-	 * handled with the help of userspace through an inheritedly racy dance
-	 * to set/unset the VT into raw mode.
-	 *
-	 * Legacy drivers initialize the hardware in the @firstopen callback,
-	 * which isn't even called for modern drivers.
-	 */
-	void (*lastclose) (struct drm_device *);
-
 	/**
 	 * @unload:
 	 *
-- 
2.46.0

[PATCH 8/8] drm: Remove struct drm_mode_config_funcs.output_poll_changed

[Thomas Zimmermann]

The output_poll_changed hook in struct drm_mode_config_funcs is
unused. Remove it. The helper drm_client_dev_hotplug() implements
the callback's functionality.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_probe_helper.c | 10 +---------
 include/drm/drm_mode_config.h      | 16 ----------------
 2 files changed, 1 insertion(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index 285290067056..92f21764246f 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -714,7 +714,7 @@ EXPORT_SYMBOL(drm_helper_probe_single_connector_modes);
  * @dev: drm_device whose connector state changed
  *
  * This function fires off the uevent for userspace and also calls the
- * output_poll_changed function, which is most commonly used to inform the fbdev
+ * client hotplug function, which is most commonly used to inform the fbdev
  * emulation code and allow it to update the fbcon output configuration.
  *
  * Drivers should call this from their hotplug handling code when a change is
@@ -730,11 +730,7 @@ EXPORT_SYMBOL(drm_helper_probe_single_connector_modes);
  */
 void drm_kms_helper_hotplug_event(struct drm_device *dev)
 {
-	/* send a uevent + call fbdev */
 	drm_sysfs_hotplug_event(dev);
-	if (dev->mode_config.funcs->output_poll_changed)
-		dev->mode_config.funcs->output_poll_changed(dev);
-
 	drm_client_dev_hotplug(dev);
 }
 EXPORT_SYMBOL(drm_kms_helper_hotplug_event);
@@ -750,11 +746,7 @@ void drm_kms_helper_connector_hotplug_event(struct drm_connector *connector)
 {
 	struct drm_device *dev = connector->dev;
 
-	/* send a uevent + call fbdev */
 	drm_sysfs_connector_hotplug_event(connector);
-	if (dev->mode_config.funcs->output_poll_changed)
-		dev->mode_config.funcs->output_poll_changed(dev);
-
 	drm_client_dev_hotplug(dev);
 }
 EXPORT_SYMBOL(drm_kms_helper_connector_hotplug_event);
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index ab0f167474b1..271765e2e9f2 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -97,22 +97,6 @@ struct drm_mode_config_funcs {
 	 */
 	const struct drm_format_info *(*get_format_info)(const struct drm_mode_fb_cmd2 *mode_cmd);
 
-	/**
-	 * @output_poll_changed:
-	 *
-	 * Callback used by helpers to inform the driver of output configuration
-	 * changes.
-	 *
-	 * Drivers implementing fbdev emulation use drm_kms_helper_hotplug_event()
-	 * to call this hook to inform the fbdev helper of output changes.
-	 *
-	 * This hook is deprecated, drivers should instead implement fbdev
-	 * support with struct drm_client, which takes care of any necessary
-	 * hotplug event forwarding already without further involvement by
-	 * the driver.
-	 */
-	void (*output_poll_changed)(struct drm_device *dev);
-
 	/**
 	 * @mode_valid:
 	 *
-- 
2.46.0

Re: [PATCH 8/8] drm: Remove struct drm_mode_config_funcs.output_poll_changed

[Daniel Vetter]

On Wed, Aug 07, 2024 at 10:41:40AM +0200, Thomas Zimmermann wrote:
> The output_poll_changed hook in struct drm_mode_config_funcs is
> unused. Remove it. The helper drm_client_dev_hotplug() implements
> the callback's functionality.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/drm_probe_helper.c | 10 +---------
>  include/drm/drm_mode_config.h      | 16 ----------------
>  2 files changed, 1 insertion(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> index 285290067056..92f21764246f 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -714,7 +714,7 @@ EXPORT_SYMBOL(drm_helper_probe_single_connector_modes);
>   * @dev: drm_device whose connector state changed
>   *
>   * This function fires off the uevent for userspace and also calls the
> - * output_poll_changed function, which is most commonly used to inform the fbdev
> + * client hotplug function, which is most commonly used to inform the fbdev
>   * emulation code and allow it to update the fbcon output configuration.
>   *
>   * Drivers should call this from their hotplug handling code when a change is
> @@ -730,11 +730,7 @@ EXPORT_SYMBOL(drm_helper_probe_single_connector_modes);
>   */
>  void drm_kms_helper_hotplug_event(struct drm_device *dev)
>  {
> -	/* send a uevent + call fbdev */
>  	drm_sysfs_hotplug_event(dev);
> -	if (dev->mode_config.funcs->output_poll_changed)
> -		dev->mode_config.funcs->output_poll_changed(dev);
> -
>  	drm_client_dev_hotplug(dev);
>  }
>  EXPORT_SYMBOL(drm_kms_helper_hotplug_event);
> @@ -750,11 +746,7 @@ void drm_kms_helper_connector_hotplug_event(struct drm_connector *connector)
>  {
>  	struct drm_device *dev = connector->dev;
>  
> -	/* send a uevent + call fbdev */
>  	drm_sysfs_connector_hotplug_event(connector);
> -	if (dev->mode_config.funcs->output_poll_changed)
> -		dev->mode_config.funcs->output_poll_changed(dev);
> -
>  	drm_client_dev_hotplug(dev);
>  }
>  EXPORT_SYMBOL(drm_kms_helper_connector_hotplug_event);
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index ab0f167474b1..271765e2e9f2 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -97,22 +97,6 @@ struct drm_mode_config_funcs {
>  	 */
>  	const struct drm_format_info *(*get_format_info)(const struct drm_mode_fb_cmd2 *mode_cmd);
>  
> -	/**
> -	 * @output_poll_changed:
> -	 *
> -	 * Callback used by helpers to inform the driver of output configuration
> -	 * changes.
> -	 *
> -	 * Drivers implementing fbdev emulation use drm_kms_helper_hotplug_event()
> -	 * to call this hook to inform the fbdev helper of output changes.
> -	 *
> -	 * This hook is deprecated, drivers should instead implement fbdev
> -	 * support with struct drm_client, which takes care of any necessary
> -	 * hotplug event forwarding already without further involvement by
> -	 * the driver.
> -	 */
> -	void (*output_poll_changed)(struct drm_device *dev);
> -
>  	/**
>  	 * @mode_valid:
>  	 *
> -- 
> 2.46.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch