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@xxxxxxxxxxxxxx> 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 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
