[Lima] [PATCH v2] drm/fourcc: add ARM GPU tile format

[Qiang Yu]

On Sun, Mar 10, 2019 at 10:08 AM Alyssa Rosenzweig <alyssa at rosenzweig.io> wrote:
>
> > +/*
> > + * Arm Device code
> > + *
> > + * Arm has multiple devices which do not share buffer format,
> > + * so add a device field at the MSB of the format field to seperate
> > + * each device's encoding.
> > + */
> > +#define DRM_FORMAT_MOD_ARM_DEVICE_AFBC 0x00
> > +#define DRM_FORMAT_MOD_ARM_DEVICE_GPU  0x01
> > +
> > +#define DRM_FORMAT_MOD_ARM_CODE(device, val) \
> > +     fourcc_mod_code(ARM, ((__u64)device << 48) | ((val) & 0x0000ffffffffffffULL))
> > +
> >  /*
> >   * Arm Framebuffer Compression (AFBC) modifiers
> >   *
> > @@ -615,7 +628,8 @@ extern "C" {
> >   * Further information on the use of AFBC modifiers can be found in
> >   * Documentation/gpu/afbc.rst
> >   */
> > -#define DRM_FORMAT_MOD_ARM_AFBC(__afbc_mode) fourcc_mod_code(ARM, __afbc_mode)
> > +#define DRM_FORMAT_MOD_ARM_AFBC(__afbc_mode) \
> > +     DRM_FORMAT_MOD_ARM_CODE(DRM_FORMAT_MOD_ARM_DEVICE_AFBC, __afbc_mode)
> >
> >  /*
> >   * AFBC superblock size
> > @@ -709,6 +723,21 @@ extern "C" {
> >   */
> >  #define AFBC_FORMAT_MOD_BCH     (1ULL << 11)
> >
> > +/*
> > + * Arm GPU modifiers
> > + */
> > +#define DRM_FORMAT_MOD_ARM_GPU(mode) \
> > +     DRM_FORMAT_MOD_ARM_CODE(DRM_FORMAT_MOD_ARM_DEVICE_GPU, mode)
> > +
>
> Although Utgard does not support AFBC, both Midgard and Bifrost natively
> support AFBC for both texturing and rendering. Separating "AFBC" from
> "GPU" is incorrect.
>
> It's okay that lima does not support all Arm format codes, but it is
> confusing to create a new "device" to describe that.

We need a field in MSB of the format field to distinguish AFBC formats
and Utgard formats. AFBC is bit based format discretion, so I can't add
some number to that in LSB.

If you think Midgard/Bifrost is compatible with AFBC and don't have it's
own format, and name "device" is improper,  I can rename
DRM_FORMAT_MOD_ARM_DEVICE_AFBC to DRM_FORMAT_MOD_ARM_TYPE_AFBC
DRM_FORMAT_MOD_ARM_DEVICE_GPU to DRM_FORMAT_MOD_ARM_TYPE_UTGARD

You may add Midgard/Bifrost spec formats latter if need by another type:
DRM_FORMAT_MOD_ARM_TYPE_MIDGARD

How about it?

>
> These hunks are therefore:
>
> NAKed-by: Alyssa Rosenzweig <alyssa at rosenzweig.io>
>
> > +/*
> > + * Arm GPU tiled format
> > + *
> > + * This is used by ARM Mali Utgard/Midgard GPU. It divides buffer into
> > + * 16x16 pixel blocks. Blocks are stored linearly in order, but pixels
> > + * in the block are reordered.
> > + */
> > +#define DRM_FORMAT_MOD_ARM_GPU_TILED DRM_FORMAT_MOD_ARM_GPU(1)
>
> Per above, "DRM_FORMAT_MOD_ARM_GPU(1)" should be "fourcc_mod_code(ARM,
> ...)" for a suitable number. Besides that, this hunk is:
>
> Reviewed-by: Alyssa Rosenzweig <alyssa at rosenzweig.io>
>
> ---
>
> Does lima import/export tiled BOs? Panfrost does not, because AFBC is
> our preferred format for shared buffers. As far as I know, Midgard
> cannot render into the tiled format, only linear or AFBC.  I'm curious
> what your use case is.

Yes, lima need this when xserver/client share window buffer. Client may
render into a tiled window buffer to be presented to xserver, so need a
modifier to describe the buffer format. And Utgard can render into both
tiled and linear buffers.

Regards,
Qiang