[PATCH 6/9] drm/fb-helper: Provide callback to create fbdev dumb buffers

[Javier Martinez Canillas]

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 at suse.de>
> ---
>  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 at redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat