[PATCH 1/2] drm/fourcc: add ARM tiled format modifier

[Brian Starkey]

On Fri, Feb 15, 2019 at 09:48:47AM +0800, Qiang Yu wrote:
> On Thu, Feb 14, 2019 at 11:27 PM Brian Starkey <Brian.Starkey at arm.com> wrote:
> >
> > Hi,
> >
> > On Wed, Feb 06, 2019 at 09:14:56PM +0800, Qiang Yu wrote:
> > > Signed-off-by: Qiang Yu <yuq825 at gmail.com>
> > > ---
> > >  include/uapi/drm/drm_fourcc.h | 9 +++++++++
> > >  1 file changed, 9 insertions(+)
> > >
> > > diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> > > index 0b44260a5ee9..953b59eb3fd2 100644
> > > --- a/include/uapi/drm/drm_fourcc.h
> > > +++ b/include/uapi/drm/drm_fourcc.h
> > > @@ -644,6 +644,15 @@ extern "C" {
> > >   */
> > >  #define AFBC_FORMAT_MOD_SC      (1ULL <<  9)
> > >
> > > +/*
> > > + * ARM 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_TILED fourcc_mod_code(ARM, 1)
> > > +
> >
> > This conflicts with the already-defined AFBC modifiers. It has the
> > same value as:
> >
> > DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_BLOCK_SIZE_16x16);
> >
> > I would suggest we use the top bit(s) to namespace between the
> > different types of tiling formats Arm has (e.g. AFBC vs
> > non-compressed).
> Just for conformation, is the non-compressed tile format + BLOCK_SIZE_16x16
> super block format same as mali gpu used tile format?
> 
> So what name would you suggest to name this mali gpu tile format?
> Or maybe it's better that you can send out a patch to add the tile
> format (better also other ones) and how it's assembled with super block
> format in the modifier, as I don't have a whole picture of this.

Honestly, you know more about it than I do (ironic, I know). I'm not
working on Mali GPUs and I've never run anything on Utgard. All I
really know is that what you're describing here definitely _isn't_
AFBC.

I've asked internally, so if I have anything useful to add, I will
update you. If we can get a patch from Arm, I agree that would be a
good solution.

My (personal) main concern here is only that you don't conflict with
AFBC.

> 
> >
> > I think this would be an internal-only layout though. Do we need
> > modifiers for internal-only formats? I thought they were mainly for
> > sharing cross-device. I also didn't see this macro getting used
> > anywhere in the driver; I suppose at a minimum you should validate the
> > modifier value provided by userspace.
> >
> Yes we need it. It's used in the userspace mesa driver and applications
> which need to share mali driver allocated buffers across process. So
> that one process can tell another process which format to interpret the
> passed buffer.

OK, thanks to both of you for educating me :-)

Cheers,
-Brian

> 
> Thanks,
> Qiang