| Message ID | 20210319190607.2903545-1-l.stach@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/fourcc: add Vivante TS modifiers | expand |
On Fri, Mar 19, 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 Vivante color buffer tiling modifiers, so they are kind of a modifier > modifier. If a TS modifier is present we have a additional plane for the > TS buffer and the modifier defines the layout of this TS buffer. > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > --- > include/uapi/drm/drm_fourcc.h | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h > index f76de49c768f..76df2a932637 100644 > --- a/include/uapi/drm/drm_fourcc.h > +++ b/include/uapi/drm/drm_fourcc.h > @@ -623,6 +623,22 @@ extern "C" { > */ > #define DRM_FORMAT_MOD_VIVANTE_SPLIT_SUPER_TILED fourcc_mod_code(VIVANTE, 4) > > +/* > + * 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. Might be worth it to go into alignment/rounding/stride requirements here too, if you know them. Either way lgtm. Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch> > + * 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) > + > /* NVIDIA frame buffer modifiers */ > > /* > -- > 2.29.2 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
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?
Hi Lucas Am Fr., 19. März 2021 um 20:06 Uhr schrieb Lucas Stach <l.stach@pengutronix.de>: > > Vivante TS (tile-status) buffer modifiers. They can be combined with all of > the Vivante color buffer tiling modifiers, so they are kind of a modifier > modifier. If a TS modifier is present we have a additional plane for the > TS buffer and the modifier defines the layout of this TS buffer. > I am unsure why you want to have the TS modifiers in drm_fourcc.h. Can you share some insight on this?
On Sat, Mar 20, 2021 at 10:28 AM Christian Gmeiner <christian.gmeiner@gmail.com> wrote: > > Hi Lucas > > Am Fr., 19. März 2021 um 20:06 Uhr schrieb Lucas Stach <l.stach@pengutronix.de>: > > > > Vivante TS (tile-status) buffer modifiers. They can be combined with all of > > the Vivante color buffer tiling modifiers, so they are kind of a modifier > > modifier. If a TS modifier is present we have a additional plane for the > > TS buffer and the modifier defines the layout of this TS buffer. > > > > I am unsure why you want to have the TS modifiers in drm_fourcc.h. Can > you share some insight on this? It's the official registry for drm_fourcc codes and drm modifiers. Whether the kernel ever uses it or not doesn't matter. -Daniel
Am Sa., 20. März 2021 um 20:11 Uhr schrieb Daniel Vetter <daniel@ffwll.ch>: > > On Sat, Mar 20, 2021 at 10:28 AM Christian Gmeiner > <christian.gmeiner@gmail.com> wrote: > > > > Hi Lucas > > > > Am Fr., 19. März 2021 um 20:06 Uhr schrieb Lucas Stach <l.stach@pengutronix.de>: > > > > > > Vivante TS (tile-status) buffer modifiers. They can be combined with all of > > > the Vivante color buffer tiling modifiers, so they are kind of a modifier > > > modifier. If a TS modifier is present we have a additional plane for the > > > TS buffer and the modifier defines the layout of this TS buffer. > > > > > > > I am unsure why you want to have the TS modifiers in drm_fourcc.h. Can > > you share some insight on this? > > It's the official registry for drm_fourcc codes and drm modifiers. > Whether the kernel ever uses it or not doesn't matter. Fair point.. but I do not see any usage of these TS modifiers in mesa - that's why I am asking.
Hi Christian, Am Montag, dem 22.03.2021 um 09:54 +0100 schrieb Christian Gmeiner: > Am Sa., 20. März 2021 um 20:11 Uhr schrieb Daniel Vetter <daniel@ffwll.ch>: > > > > On Sat, Mar 20, 2021 at 10:28 AM Christian Gmeiner > > <christian.gmeiner@gmail.com> wrote: > > > > > > Hi Lucas > > > > > > Am Fr., 19. März 2021 um 20:06 Uhr schrieb Lucas Stach <l.stach@pengutronix.de>: > > > > > > > > Vivante TS (tile-status) buffer modifiers. They can be combined with all of > > > > the Vivante color buffer tiling modifiers, so they are kind of a modifier > > > > modifier. If a TS modifier is present we have a additional plane for the > > > > TS buffer and the modifier defines the layout of this TS buffer. > > > > > > > > > > I am unsure why you want to have the TS modifiers in drm_fourcc.h. Can > > > you share some insight on this? > > > > It's the official registry for drm_fourcc codes and drm modifiers. > > Whether the kernel ever uses it or not doesn't matter. > > Fair point.. but I do not see any usage of these TS modifiers in mesa > - that's why I am asking. I have a Mesa series using those modifiers, which I'm currently rebasing and will be posted shortly. However, the way things work is: first get the modifier into drm_fourcc.h, then merge any code using the new modifiers, so I figured it would be fair game to post this patch before I fully finished reworking the Mesa series. Regards, Lucas
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
On Mon, Mar 22, 2021 at 10:20:45AM +0100, Lucas Stach wrote: > Hi Christian, > > Am Montag, dem 22.03.2021 um 09:54 +0100 schrieb Christian Gmeiner: > > Am Sa., 20. März 2021 um 20:11 Uhr schrieb Daniel Vetter <daniel@ffwll.ch>: > > > > > > On Sat, Mar 20, 2021 at 10:28 AM Christian Gmeiner > > > <christian.gmeiner@gmail.com> wrote: > > > > > > > > Hi Lucas > > > > > > > > Am Fr., 19. März 2021 um 20:06 Uhr schrieb Lucas Stach <l.stach@pengutronix.de>: > > > > > > > > > > Vivante TS (tile-status) buffer modifiers. They can be combined with all of > > > > > the Vivante color buffer tiling modifiers, so they are kind of a modifier > > > > > modifier. If a TS modifier is present we have a additional plane for the > > > > > TS buffer and the modifier defines the layout of this TS buffer. > > > > > > > > > > > > > I am unsure why you want to have the TS modifiers in drm_fourcc.h. Can > > > > you share some insight on this? > > > > > > It's the official registry for drm_fourcc codes and drm modifiers. > > > Whether the kernel ever uses it or not doesn't matter. > > > > Fair point.. but I do not see any usage of these TS modifiers in mesa > > - that's why I am asking. > > I have a Mesa series using those modifiers, which I'm currently > rebasing and will be posted shortly. However, the way things work is: > first get the modifier into drm_fourcc.h, then merge any code using the > new modifiers, so I figured it would be fair game to post this patch > before I fully finished reworking the Mesa series. Generally post poth sides, and then _merge_ them in the order you described. Christian has a good point that generally for modifiers that mesa is expected to use, we want to have the mesa code as demonstration that they work. Especially for r/e'ed drivers where there's not authoritative spec. I was just assuming that this has happened already. -Daniel PS: Since this comes up all the time, relevant part of the upstream docs: "The kernel patch can only be merged after all the above requirements are met, but it must be merged to either drm-next or drm-misc-next before the userspace patches land. uAPI always flows from the kernel, doing things the other way round risks divergence of the uAPI definitions and header files." https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#open-source-userspace-requirements
Am Montag, dem 22.03.2021 um 10:20 +0100 schrieb Lucas Stach: > Hi Christian, > > Am Montag, dem 22.03.2021 um 09:54 +0100 schrieb Christian Gmeiner: > > Am Sa., 20. März 2021 um 20:11 Uhr schrieb Daniel Vetter <daniel@ffwll.ch>: > > > > > > On Sat, Mar 20, 2021 at 10:28 AM Christian Gmeiner > > > <christian.gmeiner@gmail.com> wrote: > > > > > > > > Hi Lucas > > > > > > > > Am Fr., 19. März 2021 um 20:06 Uhr schrieb Lucas Stach <l.stach@pengutronix.de>: > > > > > > > > > > Vivante TS (tile-status) buffer modifiers. They can be combined with all of > > > > > the Vivante color buffer tiling modifiers, so they are kind of a modifier > > > > > modifier. If a TS modifier is present we have a additional plane for the > > > > > TS buffer and the modifier defines the layout of this TS buffer. > > > > > > > > > > > > > I am unsure why you want to have the TS modifiers in drm_fourcc.h. Can > > > > you share some insight on this? > > > > > > It's the official registry for drm_fourcc codes and drm modifiers. > > > Whether the kernel ever uses it or not doesn't matter. > > > > Fair point.. but I do not see any usage of these TS modifiers in mesa > > - that's why I am asking. > > I have a Mesa series using those modifiers, which I'm currently > rebasing and will be posted shortly. However, the way things work is: > first get the modifier into drm_fourcc.h, then merge any code using the > new modifiers, so I figured it would be fair game to post this patch > before I fully finished reworking the Mesa series. Rebasing and re-testing did take a bit more time than I expected, but userspace bits are now available at: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/9780 I also have patches for DCSS to enable compression support all the way to the display on i.MX8M, but those also need rebasing and re-testing. I'll send them out when ready. Regards, Lucas
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h index f76de49c768f..76df2a932637 100644 --- a/include/uapi/drm/drm_fourcc.h +++ b/include/uapi/drm/drm_fourcc.h @@ -623,6 +623,22 @@ extern "C" { */ #define DRM_FORMAT_MOD_VIVANTE_SPLIT_SUPER_TILED fourcc_mod_code(VIVANTE, 4) +/* + * 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) + /* NVIDIA frame buffer modifiers */ /*
Vivante TS (tile-status) buffer modifiers. They can be combined with all of the Vivante color buffer tiling modifiers, so they are kind of a modifier modifier. If a TS modifier is present we have a additional plane for the TS buffer and the modifier defines the layout of this TS buffer. Signed-off-by: Lucas Stach <l.stach@pengutronix.de> --- include/uapi/drm/drm_fourcc.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)