[PATCH v2 5/5] drm/omapdrm: Implement fbdev emulation as in-kernel client

[Thomas Zimmermann]

Hi

Am 03.04.23 um 16:27 schrieb Emil Velikov:
> On Mon, 3 Apr 2023 at 11:41, Thomas Zimmermann <tzimmermann at suse.de> wrote:
>>
>> Move code from ad-hoc fbdev callbacks into DRM client functions
>> and remove the old callbacks. The functions instruct the client
>> to poll for changed output or restore the display. The DRM core
>> calls both, the old callbacks and the new client helpers, from
>> the same places. The new functions perform the same operation as
>> before, so there's no change in functionality.
>>
>> Replace all code that initializes or releases fbdev emulation
>> throughout the driver. Instead initialize the fbdev client by a
>> single call to omapdrm_fbdev_setup() after omapdrm has registered
>> its DRM device. As in most drivers, omapdrm's fbdev emulation now
>> acts like a regular DRM client.
>>
>> The fbdev client setup consists of the initial preparation and the
>> hot-plugging of the display. The latter creates the fbdev device
>> and sets up the fbdev framebuffer. The setup performs display
>> hot-plugging once. If no display can be detected, DRM probe helpers
>> re-run the detection on each hotplug event.
>>
>> A call to drm_dev_unregister() releases the client automatically.
>> No further action is required within omapdrm. If the fbdev
>> framebuffer has been fully set up, struct fb_ops.fb_destroy
>> implements the release. For partially initialized emulation, the
>> fbdev client reverts the initial setup.
>>
>> v2:
>>          * init drm_client in this patch (Tomi)
>>          * don't handle non-atomic modesetting (Tomi)
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
>> Reviewed-by: Tomi Valkeinen <tomi.valkeinen at ideasonboard.com>
>> ---
>>   drivers/gpu/drm/omapdrm/omap_drv.c   |  11 +--
>>   drivers/gpu/drm/omapdrm/omap_fbdev.c | 132 +++++++++++++++++----------
>>   drivers/gpu/drm/omapdrm/omap_fbdev.h |   9 +-
>>   3 files changed, 90 insertions(+), 62 deletions(-)
>>
> 
>> +static void omap_fbdev_fb_destroy(struct fb_info *info)
>> +{
>> +       struct drm_fb_helper *helper = info->par;
>> +       struct drm_framebuffer *fb = helper->fb;
>> +       struct drm_gem_object *bo = drm_gem_fb_get_obj(fb, 0);
>> +       struct omap_fbdev *fbdev = to_omap_fbdev(helper);
>> +
>> +       DBG();
>> +
> 
> What a lovely little surprise. It's a pre-existing "feature", so let's
> ignore that for now ;-)

I have no idea what you're talking about. This code was in the original 
clean-up function. If it is not supposed to be here, I can remove it.

> 
>> +       drm_fb_helper_fini(helper);
>> +
>> +       omap_gem_unpin(bo);
>> +       drm_framebuffer_remove(fb);
>> +
>> +       drm_client_release(&helper->client);
>> +       drm_fb_helper_unprepare(helper);
>> +       kfree(fbdev);
>> +}
> 
> 
>> -void omap_fbdev_fini(struct drm_device *dev)
>> +static const struct drm_client_funcs omap_fbdev_client_funcs = {
>> +       .owner          = THIS_MODULE,
>> +       .unregister     = omap_fbdev_client_unregister,
>> +       .restore        = omap_fbdev_client_restore,
>> +       .hotplug        = omap_fbdev_client_hotplug,
> 
> AFAICT the fbdev client helpers above are identical to the generic
> ones in drm_fbdev_generic.c. Why aren't we reusing those but
> copy/pasting them in the driver?

The code in drm_fbdev_generic.c (and other fbdev files) might be 
shareable at some point when I know what exactly I need.  I already plan 
to move some of the damage handling from drm_fb_helper.c to 
drm_fbdev_generic.c and that will affect the helpers that are currently 
identical. There's little point in code sharing right now.

I know that the fbdev emulation is convoluted and confusing at times. 
It's the result of various redesigns and false starts. Things are 
getting better, though.

Best regards
Thomas

> 
> Thanks
> Emil

-- 
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/dri-devel/attachments/20230403/146cea3c/attachment-0001.sig>