[PATCH v3] drm/mediatek: Add AFBC support to Mediatek DRM driver

[Daniel Stone]

Hi Justin,

On Wed, 12 Oct 2022 at 20:12, Justin Green <greenjustin at chromium.org> wrote:

> @@ -226,6 +249,32 @@ int mtk_ovl_layer_check(struct device *dev, unsigned
> int idx,
>         if (state->fb->format->is_yuv && rotation != 0)
>                 return -EINVAL;
>
> +       if (state->fb->modifier) {
>

Please spell this out explicitly as DRM_FORMAT_MOD_LINEAR. For some reason,
we specify that 0 is explicitly block-linear, whilst INVALID ('unknown
layout, figure it out by magic') is a non-zero value. So != 0 isn't a check
for 'was a modifier explicitly specified', even if != 0 is almost always
'is this buffer non-linear'.

+               unsigned long long modifier;
> +               unsigned int fourcc;
>

u64, u32, but ...


> +               if (!ovl->data->supports_afbc)
> +                       return -EINVAL;
> +
> +               modifier = state->fb->modifier;
> +
> +               if (modifier !=
> DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_BLOCK_SIZE_32x8 |
> +
>  AFBC_FORMAT_MOD_SPLIT |
> +
>  AFBC_FORMAT_MOD_SPARSE))
> +                       return -EINVAL;
> +
> +               fourcc = state->fb->format->format;
> +               if (fourcc != DRM_FORMAT_BGRA8888 &&
> +                   fourcc != DRM_FORMAT_ABGR8888 &&
> +                   fourcc != DRM_FORMAT_ARGB8888 &&
> +                   fourcc != DRM_FORMAT_XRGB8888 &&
> +                   fourcc != DRM_FORMAT_XBGR8888 &&
> +                   fourcc != DRM_FORMAT_RGB888 &&
> +                   fourcc != DRM_FORMAT_BGR888)
> +                       return -EINVAL;


The core shouldn't allow a framebuffer to be created with a format/modifier
pair which wasn't advertised, so these checks can be dropped. Except that
it's never advertised.

mtk_plane_init() passes a set of formats and modifiers to
drm_universal_plane_init(); the AFBC modifier needs to be added here, as
well as LINEAR and INVALID. Then you'll need to set the
format_mod_supported() hook on the plane to filter out format/modifier
pairs. After that, you should see (e.g. with drm_info) that you get an
IN_FORMATS blob which advertises DRM_FORMAT_MOD_ARM_AFBC(...) as well as
linear for DRM_FORMAT_XRGB8888, but only linear for DRM_FORMAT_RGB565.

After doing that, you should see framebuffer creation fail for unsupported
pairings, e.g. RGB565 + AFBC.


>  +       bool is_afbc = pending->modifier;


... != DRM_FORMAT_MOD_LINEAR


> @@ -52,6 +53,7 @@ static void mtk_plane_reset(struct drm_plane *plane)
>
>         state->base.plane = plane;
>         state->pending.format = DRM_FORMAT_RGB565;
> +       state->pending.modifier = 0;
>

= DRM_FORMAT_MOD_LINEAR


> @@ -120,21 +122,52 @@ static void mtk_plane_update_new_state(struct
> drm_plane_state *new_state,
>         struct drm_gem_object *gem;
>         struct mtk_drm_gem_obj *mtk_gem;
>         unsigned int pitch, format;
> +       unsigned long long modifier;
>

u64


> +       if (!modifier) {
>

modifier == DRM_FORMAT_MOD_LINEAR

Cheers,
Daniel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20221013/0481a42a/attachment.htm>