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@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