[PATCH 08/11] drm/i915: Use drm_fbdev_helper_client_unregister()

[Thomas Zimmermann]

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 <tzimmermann at suse.de>
>> ---
>>   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 <linux/vga_switcheroo.h>
>>   
>>   #include <drm/drm_crtc.h>
>> -#include <drm/drm_crtc_helper.h>
>>   #include <drm/drm_fb_helper.h>
>>   #include <drm/drm_fourcc.h>
>>   #include <drm/drm_gem_framebuffer_helper.h>
>> @@ -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(&fb_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 <rodrigo.vivi at intel.com>
>
> Acked-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> (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)