[PATCH 1/2] drm/fb-helper: Remove drm_fb_helper_defio_init() and update docs
Thomas Zimmermann
tzimmermann at suse.de
Fri Oct 25 17:26:54 UTC 2019
Hi
Am 25.10.19 um 17:46 schrieb Noralf Trønnes:
>
>
> Den 25.10.2019 11.27, skrev Thomas Zimmermann:
>> There are no users of drm_fb_helper_defio_init(), so we can remove
>> it. The documenation around defio support is a bit misleading and
>> should mention compatibility issues with SHMEM helpers. Clarify this.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
>> ---
>> drivers/gpu/drm/drm_fb_helper.c | 61 +++++++--------------------------
>> include/drm/drm_fb_helper.h | 1 -
>> 2 files changed, 13 insertions(+), 49 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
>> index b75ae8555baf..8ebeccdeed23 100644
>> --- a/drivers/gpu/drm/drm_fb_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> @@ -92,9 +92,12 @@ static DEFINE_MUTEX(kernel_fb_helper_lock);
>> *
>> * Drivers that support a dumb buffer with a virtual address and mmap support,
>> * should try out the generic fbdev emulation using drm_fbdev_generic_setup().
>> + * It will automatically set up deferred I/O if the driver requires a shadow
>> + * buffer.
>> *
>> - * Setup fbdev emulation by calling drm_fb_helper_fbdev_setup() and tear it
>> - * down by calling drm_fb_helper_fbdev_teardown().
>> + * For other drivers, setup fbdev emulation by calling
>> + * drm_fb_helper_fbdev_setup() and tear it down by calling
>> + * drm_fb_helper_fbdev_teardown().
>> *
>> * At runtime drivers should restore the fbdev console by using
>> * drm_fb_helper_lastclose() as their &drm_driver.lastclose callback.
>> @@ -127,8 +130,10 @@ static DEFINE_MUTEX(kernel_fb_helper_lock);
>> * always run in process context since the fb_*() function could be running in
>> * atomic context. If drm_fb_helper_deferred_io() is used as the deferred_io
>> * callback it will also schedule dirty_work with the damage collected from the
>> - * mmap page writes. Drivers can use drm_fb_helper_defio_init() to setup
>> - * deferred I/O (coupled with drm_fb_helper_fbdev_teardown()).
>> + * mmap page writes.
>> + *
>> + * Deferred I/O is not compatible with SHMEM. Such drivers should request an
>> + * fbdev shadow buffer and call drm_fbdev_generic_setup() instead.
>> */
>>
>> static void drm_fb_helper_restore_lut_atomic(struct drm_crtc *crtc)
>> @@ -679,49 +684,6 @@ void drm_fb_helper_deferred_io(struct fb_info *info,
>> }
>> EXPORT_SYMBOL(drm_fb_helper_deferred_io);
>>
>> -/**
>> - * drm_fb_helper_defio_init - fbdev deferred I/O initialization
>> - * @fb_helper: driver-allocated fbdev helper
>> - *
>> - * This function allocates &fb_deferred_io, sets callback to
>> - * drm_fb_helper_deferred_io(), delay to 50ms and calls fb_deferred_io_init().
>> - * It should be called from the &drm_fb_helper_funcs->fb_probe callback.
>> - * drm_fb_helper_fbdev_teardown() cleans up deferred I/O.
>> - *
>> - * NOTE: A copy of &fb_ops is made and assigned to &info->fbops. This is done
>> - * because fb_deferred_io_cleanup() clears &fbops->fb_mmap and would thereby
>> - * affect other instances of that &fb_ops.
>> - *
>> - * Returns:
>> - * 0 on success or a negative error code on failure.
>> - */
>> -int drm_fb_helper_defio_init(struct drm_fb_helper *fb_helper)
>> -{
>> - struct fb_info *info = fb_helper->fbdev;
>> - struct fb_deferred_io *fbdefio;
>> - struct fb_ops *fbops;
>> -
>> - fbdefio = kzalloc(sizeof(*fbdefio), GFP_KERNEL);
>> - fbops = kzalloc(sizeof(*fbops), GFP_KERNEL);
>> - if (!fbdefio || !fbops) {
>> - kfree(fbdefio);
>> - kfree(fbops);
>> - return -ENOMEM;
>> - }
>> -
>> - info->fbdefio = fbdefio;
>> - fbdefio->delay = msecs_to_jiffies(50);
>> - fbdefio->deferred_io = drm_fb_helper_deferred_io;
>> -
>> - *fbops = *info->fbops;
>> - info->fbops = fbops;
>> -
>> - fb_deferred_io_init(info);
>> -
>> - return 0;
>> -}
>> -EXPORT_SYMBOL(drm_fb_helper_defio_init);
>> -
>
> With this gone you can remove the defio cleanup part from
> drm_fb_helper_fbdev_teardown() as well.
>
> And I see that there's no users left of that function (the same with
> *_seup()). Would be nice if you just removed them. I made them before I
> embarked on the generic fbdev solution. I think it's better to try and
> make the generic emulation usable for "everyone" and avoid the need for
> drivers to have to do their own special stuff.
The last user was vboxvideo, which I converted ~2 weeks ago. I haven't
removed them yet, as there's a TODO item to convert drivers over to them.
From a quick 'git grep':
Most drivers that uses drm_fb_helper_sys_*() in its fb_ops could
probably be converted over to generic fbdev. That's hibmc (I have some
untested patches for it), msm, omapdrm, tegra, and udl (currently being
converted).
Only nouveau and gma500 have some form of HW acceleration.
The rest of the drivers (10) uses drm_fb_helper_cfb_*() functions. Are
these strictly required, or can they be made available within generic fbdev?
Best regards
Thomas
> code to avoid the shadow buffering would aid in that as Daniel have
> talked about.
>
> Either way you choose, this patch is:
>
> Reviewed-by: Noralf Trønnes <noralf at tronnes.org>
>
>> /**
>> * drm_fb_helper_sys_read - wrapper around fb_sys_read
>> * @info: fb_info struct pointer
>> @@ -2356,7 +2318,10 @@ static const struct drm_client_funcs drm_fbdev_client_funcs = {
>> *
>> * Drivers that set the dirty callback on their framebuffer will get a shadow
>> * fbdev buffer that is blitted onto the real buffer. This is done in order to
>> - * make deferred I/O work with all kinds of buffers.
>> + * make deferred I/O work with all kinds of buffers. A shadow buffer can be
>> + * requested explicitly by setting struct drm_mode_config.prefer_shadow or
>> + * struct drm_mode_config.prefer_shadow_fbdev to true beforehand. This is
>> + * required to use generic fbdev emulation with SHMEM helpers.
>> *
>> * This function is safe to call even when there are no connectors present.
>> * Setup will be retried on the next hotplug event.
>> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
>> index 8dcc012ccbc8..2338e9f94a03 100644
>> --- a/include/drm/drm_fb_helper.h
>> +++ b/include/drm/drm_fb_helper.h
>> @@ -235,7 +235,6 @@ void drm_fb_helper_unlink_fbi(struct drm_fb_helper *fb_helper);
>>
>> void drm_fb_helper_deferred_io(struct fb_info *info,
>> struct list_head *pagelist);
>> -int drm_fb_helper_defio_init(struct drm_fb_helper *fb_helper);
>>
>> ssize_t drm_fb_helper_sys_read(struct fb_info *info, char __user *buf,
>> size_t count, loff_t *ppos);
>>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20191025/52793edc/attachment.sig>
More information about the dri-devel
mailing list