On 3/3/22 21:58, Thomas Zimmermann wrote:
> Provide struct drm_driver.dumb_create_fbdev, a callback hook for
> fbdev dumb buffers. Wire up fbdev and client helpers to use the new
> interface, if present.
>
> This acknowledges the fact that fbdev buffers are different. The most
> significant difference to regular GEM BOs is in mmap semantics. Fbdev
> userspace treats the pages as video memory, which makes it impossible
> to ever move the mmap'ed buffer. Hence, drivers ussually have to pin
> the BO permanently or install an intermediate shadow buffer for mmap.
>
> So far, fbdev memory came from dumb buffers and DRM drivers had no
> means of detecting this without reimplementing a good part of the fbdev
> code. With the new callback, drivers can perma-pin fbdev buffer objects
> if needed.
>
> Several drivers also require damage handling, which fbdev implements
> with its deferred I/O helpers. The new callback allows a driver's memory
> manager to set up a suitable mmap.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
> ---
> drivers/gpu/drm/drm_client.c | 14 +++++++----
> drivers/gpu/drm/drm_crtc_internal.h | 3 +++
> drivers/gpu/drm/drm_dumb_buffers.c | 36 +++++++++++++++++++++++++----
> drivers/gpu/drm/drm_fb_helper.c | 26 +++++++++++++++++----
> include/drm/drm_client.h | 3 ++-
> include/drm/drm_drv.h | 17 ++++++++++++++
> 6 files changed, 84 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> index af3b7395bf69..c964064651d5 100644
> --- a/drivers/gpu/drm/drm_client.c
> +++ b/drivers/gpu/drm/drm_client.c
> @@ -247,7 +247,8 @@ static void drm_client_buffer_delete(struct drm_client_buffer *buffer)
> }
>
> static struct drm_client_buffer *
> -drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 height, u32 format)
> +drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 height, u32 format,
> + bool fbdev)
> {
> const struct drm_format_info *info = drm_format_info(format);
> struct drm_mode_create_dumb dumb_args = { };
> @@ -265,7 +266,10 @@ drm_client_buffer_create(struct drm_client_dev *client, u32 width, u32 height, u
> dumb_args.width = width;
> dumb_args.height = height;
> dumb_args.bpp = info->cpp[0] * 8;
> - ret = drm_mode_create_dumb(dev, &dumb_args, client->file);
> + if (fbdev)
Maybe if (defined(CONFIG_DRM_FBDEV_EMULATION) && fbdev) ?
> + ret = drm_mode_create_dumb_fbdev(dev, &dumb_args, client->file);
And drm_mode_create_dumb_fbdev() could just be made a stub if
CONFIG_DRM_FBDEV_EMULATION isn't enabled.
I believe the only usage of the DRM client API currently is the fbdev
emulation layer anyways? But still makes sense I think to condtionally
compile that since drm_client.o is built in the drm.ko module and the
drm_fb_helper.o only included if fbdev emulation has been enabled.
> + else
> + ret = drm_mode_create_dumb(dev, &dumb_args, client->file);
> if (ret)
> goto err_delete;
>
> @@ -402,6 +406,7 @@ static int drm_client_buffer_addfb(struct drm_client_buffer *buffer,
> * @width: Framebuffer width
> * @height: Framebuffer height
> * @format: Buffer format
> + * @fbdev: True if the client provides an fbdev device, or false otherwise
> *
An emulated fbdev device ?
Other than those small nit,
Reviewed-by: Javier Martinez Canillas <javierm@xxxxxxxxxx>
--
Best regards,
Javier Martinez Canillas
Linux Engineering
Red Hat
