Re: [PATCH 08/11] drm/i915: Use drm_fbdev_helper_client_unregister()
Hi
Am 07.05.24 um 14:25 schrieb Rodrigo Vivi:
On Tue, May 07, 2024 at 01:58:29PM +0200, Thomas Zimmermann wrote:
Implement struct drm_client_funcs.unregister with the helpers
drm_fbdev_helper_client_unregister() and remove the custom code
from the driver. The generic helper is equivalent in functionality.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/i915/display/intel_fbdev.c | 21 ++---
1 file changed, 2 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c
b/drivers/gpu/drm/i915/display/intel_fbdev.c
index bda702c2cab8e..f1b4543bffc02 100644
--- a/drivers/gpu/drm/i915/display/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
@@ -38,7 +38,6 @@
#include
#include
-#include
#include
#include
#include
@@ -580,25 +579,9 @@ static int intel_fbdev_restore_mode(struct
drm_i915_private *dev_priv)
}
/*
- * Fbdev client and struct drm_client_funcs
+ * struct drm_client_funcs
*/
-static void intel_fbdev_client_unregister(struct drm_client_dev *client)
-{
- struct drm_fb_helper *fb_helper = drm_fb_helper_from_client(client);
- struct drm_device *dev = fb_helper->dev;
- struct pci_dev *pdev = to_pci_dev(dev->dev);
-
- if (fb_helper->info) {
- vga_switcheroo_client_fb_set(pdev, NULL);
- drm_fb_helper_unregister_info(fb_helper);
- } else {
- drm_fb_helper_unprepare(fb_helper);
- drm_client_release(_helper->client);
The only real difference is that these 2 calls are inverted in the new
drm_fbdev_helper_client_unregister.
The condition in this if statement is different for some drivers, but
not i915. The setup code first does a _prepare and then an
_init+_register, hence the _release goes first and then comes the
_unprepare.
I feel that the releasing after the unprepare sounds more logical,
but if there's no actual issue with that and it is working for
everybody, let's do that.
It should make no difference in practice, but doing the release first is
the inverse of the setup; hence conceptually cleaner.
Reviewed-by: Rodrigo Vivi
Acked-by: Rodrigo Vivi
(to get through drm-misc with everything else if you prefer)
Thanks.
Best regards
Thomas
- kfree(fb_helper);
- }
-}
-
static int intel_fbdev_client_restore(struct drm_client_dev *client)
{
struct drm_i915_private *dev_priv = to_i915(client->dev);
@@ -644,7 +627,7 @@ static int intel_fbdev_client_hotplug(struct drm_client_dev
*client)
static const struct drm_client_funcs intel_fbdev_client_funcs = {
.owner = THIS_MODULE,
- .unregister = intel_fbdev_client_unregister,
+ .unregister = drm_fbdev_helper_client_unregister,
.restore= intel_fbdev_client_restore,
.hotplug= intel_fbdev_client_hotplug,
};
--
2.44.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)
Re: [PATCH 08/11] drm/i915: Use drm_fbdev_helper_client_unregister()
On Tue, May 07, 2024 at 01:58:29PM +0200, Thomas Zimmermann wrote:
> Implement struct drm_client_funcs.unregister with the helpers
> drm_fbdev_helper_client_unregister() and remove the custom code
> from the driver. The generic helper is equivalent in functionality.
>
> Signed-off-by: Thomas Zimmermann
> ---
> drivers/gpu/drm/i915/display/intel_fbdev.c | 21 ++---
> 1 file changed, 2 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c
> b/drivers/gpu/drm/i915/display/intel_fbdev.c
> index bda702c2cab8e..f1b4543bffc02 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
> @@ -38,7 +38,6 @@
> #include
>
> #include
> -#include
> #include
> #include
> #include
> @@ -580,25 +579,9 @@ static int intel_fbdev_restore_mode(struct
> drm_i915_private *dev_priv)
> }
>
> /*
> - * Fbdev client and struct drm_client_funcs
> + * struct drm_client_funcs
> */
>
> -static void intel_fbdev_client_unregister(struct drm_client_dev *client)
> -{
> - struct drm_fb_helper *fb_helper = drm_fb_helper_from_client(client);
> - struct drm_device *dev = fb_helper->dev;
> - struct pci_dev *pdev = to_pci_dev(dev->dev);
> -
> - if (fb_helper->info) {
> - vga_switcheroo_client_fb_set(pdev, NULL);
> - drm_fb_helper_unregister_info(fb_helper);
> - } else {
> - drm_fb_helper_unprepare(fb_helper);
> - drm_client_release(_helper->client);
The only real difference is that these 2 calls are inverted in the new
drm_fbdev_helper_client_unregister.
I feel that the releasing after the unprepare sounds more logical,
but if there's no actual issue with that and it is working for
everybody, let's do that.
Reviewed-by: Rodrigo Vivi
Acked-by: Rodrigo Vivi
(to get through drm-misc with everything else if you prefer)
> - kfree(fb_helper);
> - }
> -}
> -
> static int intel_fbdev_client_restore(struct drm_client_dev *client)
> {
> struct drm_i915_private *dev_priv = to_i915(client->dev);
> @@ -644,7 +627,7 @@ static int intel_fbdev_client_hotplug(struct
> drm_client_dev *client)
>
> static const struct drm_client_funcs intel_fbdev_client_funcs = {
> .owner = THIS_MODULE,
> - .unregister = intel_fbdev_client_unregister,
> + .unregister = drm_fbdev_helper_client_unregister,
> .restore= intel_fbdev_client_restore,
> .hotplug= intel_fbdev_client_hotplug,
> };
> --
> 2.44.0
>
[PATCH 08/11] drm/i915: Use drm_fbdev_helper_client_unregister()
Implement struct drm_client_funcs.unregister with the helpers
drm_fbdev_helper_client_unregister() and remove the custom code
from the driver. The generic helper is equivalent in functionality.
Signed-off-by: Thomas Zimmermann
---
drivers/gpu/drm/i915/display/intel_fbdev.c | 21 ++---
1 file changed, 2 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c
b/drivers/gpu/drm/i915/display/intel_fbdev.c
index bda702c2cab8e..f1b4543bffc02 100644
--- a/drivers/gpu/drm/i915/display/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
@@ -38,7 +38,6 @@
#include
#include
-#include
#include
#include
#include
@@ -580,25 +579,9 @@ static int intel_fbdev_restore_mode(struct
drm_i915_private *dev_priv)
}
/*
- * Fbdev client and struct drm_client_funcs
+ * struct drm_client_funcs
*/
-static void intel_fbdev_client_unregister(struct drm_client_dev *client)
-{
- struct drm_fb_helper *fb_helper = drm_fb_helper_from_client(client);
- struct drm_device *dev = fb_helper->dev;
- struct pci_dev *pdev = to_pci_dev(dev->dev);
-
- if (fb_helper->info) {
- vga_switcheroo_client_fb_set(pdev, NULL);
- drm_fb_helper_unregister_info(fb_helper);
- } else {
- drm_fb_helper_unprepare(fb_helper);
- drm_client_release(_helper->client);
- kfree(fb_helper);
- }
-}
-
static int intel_fbdev_client_restore(struct drm_client_dev *client)
{
struct drm_i915_private *dev_priv = to_i915(client->dev);
@@ -644,7 +627,7 @@ static int intel_fbdev_client_hotplug(struct drm_client_dev
*client)
static const struct drm_client_funcs intel_fbdev_client_funcs = {
.owner = THIS_MODULE,
- .unregister = intel_fbdev_client_unregister,
+ .unregister = drm_fbdev_helper_client_unregister,
.restore= intel_fbdev_client_restore,
.hotplug= intel_fbdev_client_hotplug,
};
--
2.44.0
