[PATCH] drm/fourcc: add Vivante TS modifiers

[Lucas Stach]

Hi Simon,

Am Freitag, dem 19.03.2021 um 19:52 +0000 schrieb Simon Ser:
> On Friday, March 19th, 2021 at 8:06 PM, Lucas Stach <l.stach at pengutronix.de> wrote:
> 
> > +/*
> > + * Vivante TS (tile-status) buffer modifiers. They can be combined with all of
> > + * the color buffer tiling modifiers defined above. When TS is present it's a
> > + * separate buffer containing the clear/compression status of each tile. The
> > + * modifiers are defined as VIVANTE_MOD_TS_c_s, where c is the color buffer tile
> > + * size in bytes covered by one entry in the status buffer and s is the number
> > + * of status bits per entry.
> > + * We reserve the top 8bits of the Vivante modifier space for TS modifiers, as
> > + * future cores might add some more TS layout variations.
> > + */
> > +#define VIVANTE_MOD_TS_64_4               (1ULL << 48)
> > +#define VIVANTE_MOD_TS_64_2               (2ULL << 48)
> > +#define VIVANTE_MOD_TS_128_4              (3ULL << 48)
> > +#define VIVANTE_MOD_TS_256_4              (4ULL << 48)
> > +#define VIVANTE_MOD_TS_MASK               (0xffULL  << 48)
> 
> Hm, I think it's the first time we have values you can OR with modifiers to
> get a new modifiers. This sounds a little bit dangerous, because all of the
> fields don't get through the fourcc_mod_code mask.
> 
> Maybe it would be better to define something like this:
> 
>     #define DRM_FORMAT_MOD_VIVANTE_TS(color_tiling, ts) \
>         fourcc_mod_code(VIVANTE, (color_tiling & 0xFF) | (ts & 0xFF << 48))
> 
> And then have defines for all of the possible values for color tiling and ts?

While I agree that this requires some attention when working with those
values, I specifically designed them in such a way that one can combine
them with the regular color buffer modifiers by OR'ing them together,
as that makes the code using them much simpler.

Regards,
Lucas