[Intel-gfx] [PATCH v2 03/10] drm/fb-helper: Introduce drm_fb_helper_unprepare()
Thomas Zimmermann
tzimmermann at suse.de
Wed Jan 25 14:11:43 UTC 2023
Hi
Am 25.01.23 um 10:18 schrieb Javier Martinez Canillas:
> On 1/24/23 14:40, Thomas Zimmermann wrote:
>> Move the fb-helper clean-up code into drm_fb_helper_unprepare(). No
>> functional changes.
>>
>> v2:
>> * declare as static inline (kernel test robot)
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
>> ---
>> drivers/gpu/drm/drm_fb_helper.c | 14 +++++++++++++-
>> include/drm/drm_fb_helper.h | 5 +++++
>> 2 files changed, 18 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
>> index c5c13e192b64..4379bcd7718b 100644
>> --- a/drivers/gpu/drm/drm_fb_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> @@ -435,6 +435,18 @@ void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper,
>> }
>> EXPORT_SYMBOL(drm_fb_helper_prepare);
>>
>> +/**
>> + * drm_fb_helper_unprepare - clean up a drm_fb_helper structure
>> + * @fb_helper: driver-allocated fbdev helper structure to set up
>> + *
>> + * Cleans up the framebuffer helper. Inverse of drm_fb_helper_prepare().
>> + */
>> +void drm_fb_helper_unprepare(struct drm_fb_helper *fb_helper)
>> +{
>> + mutex_destroy(&fb_helper->lock);
>> +}
>
> I like that we have an _unprepare that is the inverse of the _prepare, but
> since is only destroying the mutex, maybe is an unneeded indirection level?
>
> Or do you plan to add more cleanup to that _unprepare function? Otherwise I
> would just make it an inline function.
There could possibly be more; not sure yet. With patch 04, the call is
being invoked from drm_fbdev_generic.c. And I have prototype patches
that convert a number of driver-internal fbdevs to use struct
drm_client. They all follow the same design/structure/pattern that is
being layed out in generic-fbdev in this patchset. In the end
drm_fb_helper_unprepare will definitely be a public interface of the
fbdev helpers.
Best regards
Thomas
>
>> +EXPORT_SYMBOL(drm_fb_helper_unprepare);
>> +
>
> Does it have to be an exported symbol? AFAICT the only user for now is the
> drm_fb_helper_fini() function, so the function could be a static inline.
>
> [...]
>
>> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
>> index f443e1f11654..39710c570a04 100644
>> --- a/include/drm/drm_fb_helper.h
>> +++ b/include/drm/drm_fb_helper.h
>> @@ -230,6 +230,7 @@ drm_fb_helper_from_client(struct drm_client_dev *client)
>> #ifdef CONFIG_DRM_FBDEV_EMULATION
>> void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper,
>> const struct drm_fb_helper_funcs *funcs);
>> +void drm_fb_helper_unprepare(struct drm_fb_helper *fb_helper);
>> int drm_fb_helper_init(struct drm_device *dev, struct drm_fb_helper *helper);
>> void drm_fb_helper_fini(struct drm_fb_helper *helper);
>> int drm_fb_helper_blank(int blank, struct fb_info *info);
>> @@ -296,6 +297,10 @@ static inline void drm_fb_helper_prepare(struct drm_device *dev,
>> {
>> }
>>
>> +static inline void drm_fb_helper_unprepare(struct drm_fb_helper *fb_helper)
>> +{
>> +}
>> +
>
> And you should be able to remove this stub if you limit the scope of the helper.
>
> No strong opinion though. So if you prefer to keep it as is, feel free to add:
>
> Reviewed-by: Javier Martinez Canillas <javierm at redhat.com>
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/intel-gfx/attachments/20230125/0bd2fd70/attachment.sig>
More information about the Intel-gfx
mailing list