[PATCHv7 2/6] drm/core: Add drm_afbc_framebuffer and a corresponding helper
Daniel Vetter
daniel at ffwll.ch
Mon Mar 30 17:01:25 UTC 2020
On Wed, Mar 11, 2020 at 3:55 PM Andrzej Pietrasiewicz
<andrzej.p at collabora.com> wrote:
>
> The new struct contains afbc-specific data.
>
> The new function can be used by drivers which support afbc to complete
> the preparation of struct drm_afbc_framebuffer. It must be called after
> allocating the said struct and calling drm_gem_fb_init_with_funcs().
>
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p at collabora.com>
> ---
> Documentation/gpu/todo.rst | 15 +++
> drivers/gpu/drm/drm_gem_framebuffer_helper.c | 108 +++++++++++++++++++
> include/drm/drm_framebuffer.h | 45 ++++++++
> include/drm/drm_gem_framebuffer_helper.h | 10 ++
> 4 files changed, 178 insertions(+)
>
> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> index 439656f55c5d..37a3a023c114 100644
> --- a/Documentation/gpu/todo.rst
> +++ b/Documentation/gpu/todo.rst
> @@ -404,6 +404,21 @@ Contact: Laurent Pinchart, respective driver maintainers
>
> Level: Intermediate
>
> +Encode cpp properly in malidp
> +-----------------------------
> +
> +cpp (chars per pixel) is not encoded properly in malidp, zero is
> +used instead. afbc implementation needs bpp or cpp, but if it is
> +zero it needs to be provided elsewhere, and so the bpp field exists
> +in struct drm_afbc_framebuffer.
> +
> +Properly encode cpp in malidp and remove the bpp field in struct
> +drm_afbc_framebuffer.
> +
> +Contact: malidp maintainers
> +
> +Level: Intermediate
Just stumbled over this todo, which is really surprising. Also
definitely not something I wanted to ack, earlier versions at least
didn't have this.
Why is this needed? drm_afbc_framebuffer contains a drm_framebuffer,
which has a pointer to drm_format_info, which we're always setting
(the core does that for all drivers, both for addfb and addfb2). Why
is that not good enough to get at cpp for everyone?
Cheers, Daniel
> +
> Core refactorings
> =================
>
> diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> index 86c1907c579a..7e3982c36baa 100644
> --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> @@ -21,6 +21,13 @@
> #include <drm/drm_modeset_helper.h>
> #include <drm/drm_simple_kms_helper.h>
>
> +#define AFBC_HEADER_SIZE 16
> +#define AFBC_TH_LAYOUT_ALIGNMENT 8
> +#define AFBC_HDR_ALIGN 64
> +#define AFBC_SUPERBLOCK_PIXELS 256
> +#define AFBC_SUPERBLOCK_ALIGNMENT 128
> +#define AFBC_TH_BODY_START_ALIGNMENT 4096
> +
> /**
> * DOC: overview
> *
> @@ -302,6 +309,107 @@ drm_gem_fb_create_with_dirty(struct drm_device *dev, struct drm_file *file,
> }
> EXPORT_SYMBOL_GPL(drm_gem_fb_create_with_dirty);
>
> +static int drm_gem_afbc_min_size(struct drm_device *dev,
> + const struct drm_mode_fb_cmd2 *mode_cmd,
> + struct drm_afbc_framebuffer *afbc_fb)
> +{
> + const struct drm_format_info *info;
> + __u32 n_blocks, w_alignment, h_alignment, hdr_alignment;
> + /* remove bpp when all users properly encode cpp in drm_format_info */
> + __u32 bpp;
> +
> + switch (mode_cmd->modifier[0] & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK) {
> + case AFBC_FORMAT_MOD_BLOCK_SIZE_16x16:
> + afbc_fb->block_width = 16;
> + afbc_fb->block_height = 16;
> + break;
> + case AFBC_FORMAT_MOD_BLOCK_SIZE_32x8:
> + afbc_fb->block_width = 32;
> + afbc_fb->block_height = 8;
> + break;
> + /* no user exists yet - fall through */
> + case AFBC_FORMAT_MOD_BLOCK_SIZE_64x4:
> + case AFBC_FORMAT_MOD_BLOCK_SIZE_32x8_64x4:
> + default:
> + DRM_DEBUG_KMS("Invalid AFBC_FORMAT_MOD_BLOCK_SIZE: %lld.\n",
> + mode_cmd->modifier[0]
> + & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK);
> + return -EINVAL;
> + }
> +
> + /* tiled header afbc */
> + w_alignment = afbc_fb->block_width;
> + h_alignment = afbc_fb->block_height;
> + hdr_alignment = AFBC_HDR_ALIGN;
> + if (mode_cmd->modifier[0] & AFBC_FORMAT_MOD_TILED) {
> + w_alignment *= AFBC_TH_LAYOUT_ALIGNMENT;
> + h_alignment *= AFBC_TH_LAYOUT_ALIGNMENT;
> + hdr_alignment = AFBC_TH_BODY_START_ALIGNMENT;
> + }
> +
> + afbc_fb->aligned_width = ALIGN(mode_cmd->width, w_alignment);
> + afbc_fb->aligned_height = ALIGN(mode_cmd->height, h_alignment);
> + afbc_fb->offset = mode_cmd->offsets[0];
> +
> + info = drm_get_format_info(dev, mode_cmd);
> + /*
> + * Change to always using info->cpp[0]
> + * when all users properly encode it
> + */
> + bpp = info->cpp[0] ? info->cpp[0] * 8 : afbc_fb->bpp;
> +
> + n_blocks = (afbc_fb->aligned_width * afbc_fb->aligned_height)
> + / AFBC_SUPERBLOCK_PIXELS;
> + afbc_fb->afbc_size = ALIGN(n_blocks * AFBC_HEADER_SIZE, hdr_alignment);
> + afbc_fb->afbc_size += n_blocks * ALIGN(bpp * AFBC_SUPERBLOCK_PIXELS / 8,
> + AFBC_SUPERBLOCK_ALIGNMENT);
> +
> + return 0;
> +}
> +
> +/**
> + * drm_gem_fb_afbc_init() - Helper function for drivers using afbc to
> + * fill and validate all the afbc-specific
> + * struct drm_afbc_framebuffer members
> + *
> + * @dev: DRM device
> + * @afbc_fb: afbc-specific framebuffer
> + * @mode_cmd: Metadata from the userspace framebuffer creation request
> + * @afbc_fb: afbc framebuffer
> + *
> + * This function can be used by drivers which support afbc to complete
> + * the preparation of struct drm_afbc_framebuffer. It must be called after
> + * allocating the said struct and calling drm_gem_fb_init_with_funcs().
> + * It is caller's responsibility to put afbc_fb->base.obj objects in case
> + * the call is unsuccessful.
> + *
> + * Returns:
> + * Zero on success or a negative error value on failure.
> + */
> +int drm_gem_fb_afbc_init(struct drm_device *dev,
> + const struct drm_mode_fb_cmd2 *mode_cmd,
> + struct drm_afbc_framebuffer *afbc_fb)
> +{
> + const struct drm_format_info *info;
> + struct drm_gem_object **objs;
> + int ret;
> +
> + objs = afbc_fb->base.obj;
> + info = drm_get_format_info(dev, mode_cmd);
> + if (!info)
> + return -EINVAL;
> +
> + ret = drm_gem_afbc_min_size(dev, mode_cmd, afbc_fb);
> + if (ret < 0)
> + return ret;
> +
> + if (objs[0]->size < afbc_fb->afbc_size)
> + return -EINVAL;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(drm_gem_fb_afbc_init);
> +
> /**
> * drm_gem_fb_prepare_fb() - Prepare a GEM backed framebuffer
> * @plane: Plane
> diff --git a/include/drm/drm_framebuffer.h b/include/drm/drm_framebuffer.h
> index c0e0256e3e98..e9f1b0e2968d 100644
> --- a/include/drm/drm_framebuffer.h
> +++ b/include/drm/drm_framebuffer.h
> @@ -297,4 +297,49 @@ int drm_framebuffer_plane_width(int width,
> int drm_framebuffer_plane_height(int height,
> const struct drm_framebuffer *fb, int plane);
>
> +/**
> + * struct drm_afbc_framebuffer - a special afbc frame buffer object
> + *
> + * A derived class of struct drm_framebuffer, dedicated for afbc use cases.
> + */
> +struct drm_afbc_framebuffer {
> + /**
> + * @base: base framebuffer structure.
> + */
> + struct drm_framebuffer base;
> + /**
> + * @block_widht: width of a single afbc block
> + */
> + u32 block_width;
> + /**
> + * @block_widht: height of a single afbc block
> + */
> + u32 block_height;
> + /**
> + * @aligned_width: aligned frame buffer width
> + */
> + u32 aligned_width;
> + /**
> + * @aligned_height: aligned frame buffer height
> + */
> + u32 aligned_height;
> + /**
> + * @offset: offset of the first afbc header
> + */
> + u32 offset;
> + /**
> + * @afbc_size: minimum size of afbc buffer
> + */
> + u32 afbc_size;
> + /**
> + * @bpp: bpp value for this afbc buffer
> + * To be removed when users such as malidp
> + * properly store the cpp in drm_format_info.
> + * New users should not start using this field.
> + */
> + u32 bpp;
> +};
> +
> +#define fb_to_afbc_fb(x) container_of(x, struct drm_afbc_framebuffer, base)
> +
> #endif
> diff --git a/include/drm/drm_gem_framebuffer_helper.h b/include/drm/drm_gem_framebuffer_helper.h
> index c029c1618661..6b013154911d 100644
> --- a/include/drm/drm_gem_framebuffer_helper.h
> +++ b/include/drm/drm_gem_framebuffer_helper.h
> @@ -1,6 +1,7 @@
> #ifndef __DRM_GEM_FB_HELPER_H__
> #define __DRM_GEM_FB_HELPER_H__
>
> +struct drm_afbc_framebuffer;
> struct drm_device;
> struct drm_fb_helper_surface_size;
> struct drm_file;
> @@ -12,6 +13,8 @@ struct drm_plane;
> struct drm_plane_state;
> struct drm_simple_display_pipe;
>
> +#define AFBC_VENDOR_AND_TYPE_MASK GENMASK_ULL(63, 52)
> +
> struct drm_gem_object *drm_gem_fb_get_obj(struct drm_framebuffer *fb,
> unsigned int plane);
> void drm_gem_fb_destroy(struct drm_framebuffer *fb);
> @@ -34,6 +37,13 @@ struct drm_framebuffer *
> drm_gem_fb_create_with_dirty(struct drm_device *dev, struct drm_file *file,
> const struct drm_mode_fb_cmd2 *mode_cmd);
>
> +#define drm_is_afbc(modifier) \
> + (((modifier) & AFBC_VENDOR_AND_TYPE_MASK) == DRM_FORMAT_MOD_ARM_AFBC(0))
> +
> +int drm_gem_fb_afbc_init(struct drm_device *dev,
> + const struct drm_mode_fb_cmd2 *mode_cmd,
> + struct drm_afbc_framebuffer *afbc_fb);
> +
> int drm_gem_fb_prepare_fb(struct drm_plane *plane,
> struct drm_plane_state *state);
> int drm_gem_fb_simple_display_pipe_prepare_fb(struct drm_simple_display_pipe *pipe,
> --
> 2.17.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
More information about the dri-devel
mailing list