Hi Andrzej:
On Fri, Dec 13, 2019 at 04:58:36PM +0100, Andrzej Pietrasiewicz wrote:
> Extend the size-checking special function to handle afbc.
>
> Signed-off-by: Andrzej Pietrasiewicz <andrze...@collabora.com>
> ---
> drivers/gpu/drm/drm_fourcc.c | 10 +++-
> drivers/gpu/drm/drm_framebuffer.c | 3 +
> drivers/gpu/drm/drm_gem_framebuffer_helper.c | 60 ++++++++++++++++++--
> include/drm/drm_gem_framebuffer_helper.h | 16 ++++++
> 4 files changed, 82 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index d14dd7c86020..9ac2175c5bee 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -323,8 +323,14 @@ drm_get_format_info(struct drm_device *dev,
> {
> const struct drm_format_info *info = NULL;
>
> - if (dev->mode_config.funcs->get_format_info)
> - info = dev->mode_config.funcs->get_format_info(mode_cmd);
> + /* bypass driver callback if afbc */
> + if (!drm_is_afbc(mode_cmd->modifier[0]))
> + if (dev->mode_config.funcs->get_format_info) {
> + const struct drm_mode_config_funcs *funcs;
> +
> + funcs = dev->mode_config.funcs;
> + info = funcs->get_format_info(mode_cmd);
> + }
>
> if (!info)
> info = drm_format_info(mode_cmd->pixel_format);
> diff --git a/drivers/gpu/drm/drm_framebuffer.c
> b/drivers/gpu/drm/drm_framebuffer.c
> index 57564318ceea..33b741cc73e8 100644
> --- a/drivers/gpu/drm/drm_framebuffer.c
> +++ b/drivers/gpu/drm/drm_framebuffer.c
> @@ -204,6 +204,9 @@ static int framebuffer_check(struct drm_device *dev,
> unsigned int block_size = info->char_per_block[i];
> u64 min_pitch = drm_format_info_min_pitch(info, i, width);
>
> + if (drm_is_afbc(r->modifier[i]))
> + block_size = 0;
> +
> if (!block_size && (r->modifier[i] == DRM_FORMAT_MOD_LINEAR)) {
> DRM_DEBUG_KMS("Format requires non-linear modifier for
> plane %d\n", i);
> return -EINVAL;
> diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> index 4201dc1f32a5..e20f4d00b0a5 100644
> --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> @@ -21,6 +21,11 @@
> #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_SUPERBLOCK_PIXELS 256
> +#define AFBC_SUPERBLOCK_ALIGNMENT 128
> +
> /**
> * DOC: overview
> *
> @@ -200,6 +205,40 @@ int drm_gem_fb_lookup(struct drm_device *dev,
> }
> EXPORT_SYMBOL_GPL(drm_gem_fb_lookup);
>
> +static int drm_gem_afbc_min_size(struct drm_device *dev,
> + const struct drm_mode_fb_cmd2 *mode_cmd,
> + struct drm_afbc *afbc)
> +{
> + u32 n_blocks;
> +
> + if (!drm_afbc_get_superblock_wh(mode_cmd->modifier[0],
> + &afbc->block_width,
> + &afbc->block_height)) {
> + return -EINVAL;
> + }
> +
> + /* tiled header afbc */
> + if (mode_cmd->modifier[0] & AFBC_FORMAT_MOD_TILED) {
> + afbc->block_width *= AFBC_TH_LAYOUT_ALIGNMENT;
> + afbc->block_height *= AFBC_TH_LAYOUT_ALIGNMENT;
> + }
> +
> + afbc->aligned_width = ALIGN(mode_cmd->width, afbc->block_width);
> + afbc->aligned_height = ALIGN(mode_cmd->height, afbc->block_height);
> + afbc->offset = mode_cmd->offsets[0];
> +
> + n_blocks = (afbc->aligned_width * afbc->aligned_height)
> + / AFBC_SUPERBLOCK_PIXELS;
> + afbc->offset_payload = ALIGN(n_blocks * AFBC_HEADER_SIZE,
> + afbc->alignment_header);
After check the references in malidp, rockchip and komeda, seems this
afbc->alignment_header is dedicated for komeda only.
This is not true. Per afbc HW spec alignment is essential for
all afbc usage. according to the spec the requiremnt are:
AFBC1.0/1.1: 64 byte alignment both for header and body buffer.
AFBC1.2 (tiled header enabled): 4096 alignment.
So this alignement is not a vendor specific value, but afbc feature
requirement, can be determined by afbc modifier.
(malidp and komeda obeys this spec, not sure about Rockchip, but I
think it should be)
But you may see, komeda uses 1024 (not 64) for none-tiled-header afbc,
that's because GPU(MALI) changed this value to 1024 for bus
performance (sorry I don't know the detail), and komeda changed to
1024 to follow.
Back to display alignment, I think we can just follow the spec, use 64
for none-tiled-header, 4096 for tiled-header, but no need to caller to
pass a value.
> +
> + afbc->afbc_size = afbc->offset_payload + n_blocks *
> + ALIGN(afbc->bpp * AFBC_SUPERBLOCK_PIXELS / 8,
> + AFBC_SUPERBLOCK_ALIGNMENT);
> +
> + return 0;
> +}
> +
> /**
> * drm_gem_fb_size_check_special() - Helper function for use in
> * &drm_mode_config_funcs.fb_create
> @@ -218,6 +257,7 @@ int drm_gem_fb_size_check_special(struct drm_device *dev,
> const struct drm_size_check *check,
> struct drm_gem_object **objs)
> {
> +#define CHECK_HAS(field) (check && check->field)
> const struct drm_format_info *info;
> int i;
>
> @@ -231,24 +271,34 @@ int drm_gem_fb_size_check_special(struct drm_device
> *dev,
> unsigned int min_size;
> u32 pitch = mode_cmd->pitches[i];
>
> - if (check && check->use_pitch_multiplier)
> + if (CHECK_HAS(use_pitch_multiplier))
> if ((pitch * check->pitch_multiplier[i]) %
> check->pitch_modulo)
> return -EINVAL;
>
> - if (check && check->use_min_size)
> + if (CHECK_HAS(use_min_size)) {
> min_size = check->min_size[i];
> - else
> + } else if (CHECK_HAS(data) &&
> + drm_is_afbc(mode_cmd->modifier[0])) {
> + struct drm_afbc *afbc;
> + int ret;
> +
> + afbc = check->data;
> + ret = drm_gem_afbc_min_size(dev, mode_cmd, afbc);
> + if (ret < 0)
> + return ret;
> + min_size = ret;
> + } else {
> min_size = (height - 1) * pitch
> + drm_format_info_min_pitch(info, i, width)
> + mode_cmd->offsets[i];
> -
> + }
> if (objs[i]->size < min_size)
> return -EINVAL;
> }
>
> return 0;
> -
> +#undef CHECK_HAS
> }
> EXPORT_SYMBOL_GPL(drm_gem_fb_size_check_special);
>
> diff --git a/include/drm/drm_gem_framebuffer_helper.h
> b/include/drm/drm_gem_framebuffer_helper.h
> index 74304a268694..3d6015194b3c 100644
> --- a/include/drm/drm_gem_framebuffer_helper.h
> +++ b/include/drm/drm_gem_framebuffer_helper.h
> @@ -22,6 +22,22 @@ struct drm_size_check {
> u32 pitch_multiplier[4];
> u32 pitch_modulo;
> bool use_pitch_multiplier;
> + void *data;
> +};
> +
> +/**
> + * struct drm_afbc - AFBC-specific data.
> + */
> +struct drm_afbc {
> + u32 block_width;
> + u32 block_height;
> + u32 aligned_width;
> + u32 aligned_height;
> + u32 offset;
> + u32 alignment_header;
> + u32 afbc_size;
> + u32 offset_payload;
> + u32 bpp;
Seems we can remove this bpp if we set the block_size (4x4) for these afbc
only formats like DRM_FORMAT_YUV420_8BIT/10BIT in drm_format_info, and then
we can calculate the bpp like the pitch calculation, then no need
caller to specify bpp anymore, and vendor specific get_bpp() likes in malidp
and komeda also can be removed.
for this change maybe we can put it into a new series.
Thanks
James
> };
>
> struct drm_gem_object *drm_gem_fb_get_obj(struct drm_framebuffer *fb,
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
