[PATCH 08/43] drm/fbdev: Add fbdev-shmem

[Thomas Zimmermann]

Hi Geert

Am 12.03.24 um 17:14 schrieb Geert Uytterhoeven:
> Hi Thomas,
>
> On Tue, Mar 12, 2024 at 4:48 PM Thomas Zimmermann <tzimmermann at suse.de> wrote:
>> Add an fbdev emulation for SHMEM-based memory managers. The code is
>> similar to fbdev-generic, but does not require an addition shadow
>> buffer for mmap(). Fbdev-shmem operates directly on the buffer object's
>> SHMEM pages. Fbdev's deferred-I/O mechanism updates the hardware state
>> on write operations.
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
> Thanks for your patch!
>
>> --- /dev/null
>> +++ b/drivers/gpu/drm/drm_fbdev_shmem.c
>> +static int drm_fbdev_shmem_helper_fb_probe(struct drm_fb_helper *fb_helper,
>> +                                          struct drm_fb_helper_surface_size *sizes)
>> +{
>> +       struct drm_client_dev *client = &fb_helper->client;
>> +       struct drm_device *dev = fb_helper->dev;
>> +       struct drm_client_buffer *buffer;
>> +       struct drm_gem_shmem_object *shmem;
>> +       struct drm_framebuffer *fb;
>> +       struct fb_info *info;
>> +       u32 format;
>> +       struct iosys_map map;
>> +       int ret;
>> +
>> +       drm_dbg_kms(dev, "surface width(%d), height(%d) and bpp(%d)\n",
>> +                   sizes->surface_width, sizes->surface_height,
>> +                   sizes->surface_bpp);
>> +
>> +       format = drm_mode_legacy_fb_format(sizes->surface_bpp, sizes->surface_depth);
> Oops, one more caller of the imprecise
> let's-guess-the-format-from-bpp-and-depth machinery to get rid of...

Right, that has been discussed in another thread. I'll change this call 
to the drm_driver_() function.

>
>> +       buffer = drm_client_framebuffer_create(client, sizes->surface_width,
>> +                                              sizes->surface_height, format);
> [...]
>
>> +}
>> +/**
>> + * drm_fbdev_shmem_setup() - Setup fbdev emulation for GEM SHMEM helpers
>> + * @dev: DRM device
>> + * @preferred_bpp: Preferred bits per pixel for the device.
>> + *                 32 is used if this is zero.
>> + *
>> + * This function sets up fbdev emulation for GEM DMA drivers that support
>> + * dumb buffers with a virtual address and that can be mmap'ed.
>> + * drm_fbdev_shmem_setup() shall be called after the DRM driver registered
>> + * the new DRM device with drm_dev_register().
>> + *
>> + * Restore, hotplug events and teardown are all taken care of. Drivers that do
>> + * suspend/resume need to call drm_fb_helper_set_suspend_unlocked() themselves.
>> + * Simple drivers might use drm_mode_config_helper_suspend().
>> + *
>> + * This function is safe to call even when there are no connectors present.
>> + * Setup will be retried on the next hotplug event.
>> + *
>> + * The fbdev is destroyed by drm_dev_unregister().
>> + */
>> +void drm_fbdev_shmem_setup(struct drm_device *dev, unsigned int preferred_bpp)
> As this is a completely new function, can we please get a
> preferred_format parameter instead?

An understandable question. But as it is, the patchset has a trivial 
change in each driver. And the preferred_bpp parameter has the same 
meaning as the bpp value in the video= parameter. So it's ok-ish for now.

Using a format parameter here is really a much larger update and touches 
the internals of the fbdev emulation. I'm not even sure that we should 
have a parameter at all. Since in-kernel clients should behave like 
userspace clients, we could try to figure out the format from the 
driver's primary planes. That's a patchset of its own.

Best regards
Thomas

>
>> +{
>> +       struct drm_fb_helper *fb_helper;
>> +       int ret;
>> +
>> +       drm_WARN(dev, !dev->registered, "Device has not been registered.\n");
>> +       drm_WARN(dev, dev->fb_helper, "fb_helper is already set!\n");
>> +
>> +       fb_helper = kzalloc(sizeof(*fb_helper), GFP_KERNEL);
>> +       if (!fb_helper)
>> +               return;
>> +       drm_fb_helper_prepare(dev, fb_helper, preferred_bpp, &drm_fbdev_shmem_helper_funcs);
>> +
>> +       ret = drm_client_init(dev, &fb_helper->client, "fbdev", &drm_fbdev_shmem_client_funcs);
>> +       if (ret) {
>> +               drm_err(dev, "Failed to register client: %d\n", ret);
>> +               goto err_drm_client_init;
>> +       }
>> +
>> +       drm_client_register(&fb_helper->client);
>> +
>> +       return;
>> +
>> +err_drm_client_init:
>> +       drm_fb_helper_unprepare(fb_helper);
>> +       kfree(fb_helper);
>> +}
>> +EXPORT_SYMBOL(drm_fbdev_shmem_setup);
>
> Gr{oetje,eeting}s,
>
>                          Geert
>

-- 
--
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)