* [PATCH] drm/fourcc: document modifier uniqueness requirements @ 2020-05-27 8:52 Simon Ser 2020-05-27 8:55 ` Daniel Vetter 0 siblings, 1 reply; 5+ messages in thread From: Simon Ser @ 2020-05-27 8:52 UTC (permalink / raw) To: dri-devel; +Cc: Marek Olšák There have suggestions to bake pitch alignment, address alignement, contiguous memory or other placement (hidden VRAM, GTT/BAR, etc) constraints into modifiers. Last time this was brought up it seemed like the consensus was to not allow this. Document this in drm_fourcc.h. There are several reasons for this. - Encoding all of these constraints in the modifiers would explode the search space pretty quickly (we only have 64 bits to work with). - Modifiers need to be unambiguous: a buffer can only have a single modifier. - Modifier users aren't expected to parse modifiers. Signed-off-by: Simon Ser <contact@emersion.fr> Cc: Daniel Vetter <daniel@ffwll.ch> Cc: Daniel Stone <daniel@fooishbar.org> Cc: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl> Cc: Dave Airlie <airlied@gmail.com> Cc: Marek Olšák <maraeo@gmail.com> --- include/uapi/drm/drm_fourcc.h | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h index 490143500a50..97eb0f1cf9f8 100644 --- a/include/uapi/drm/drm_fourcc.h +++ b/include/uapi/drm/drm_fourcc.h @@ -58,6 +58,17 @@ extern "C" { * may preserve meaning - such as number of planes - from the fourcc code, * whereas others may not. * + * Modifiers must uniquely encode buffer layout. In other words, a buffer must + * match only a single modifier. A modifier must not be a subset of layouts of + * another modifier. For instance, it's incorrect to encode pitch alignment in + * a modifier: a buffer may match a 64-pixel aligned modifier and a 32-pixel + * aligned modifier. That said, modifiers can have implicit minimal + * requirements. + * + * Users see modifiers as opaque tokens they can check for equality and + * intersect. Users musn't need to know to reason about the modifier value + * (i.e. users are not expected to extract information out of the modifier). + * * Vendors should document their modifier usage in as much detail as * possible, to ensure maximum compatibility across devices, drivers and * applications. -- 2.26.2 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/fourcc: document modifier uniqueness requirements 2020-05-27 8:52 [PATCH] drm/fourcc: document modifier uniqueness requirements Simon Ser @ 2020-05-27 8:55 ` Daniel Vetter 2020-06-01 10:35 ` Brian Starkey 0 siblings, 1 reply; 5+ messages in thread From: Daniel Vetter @ 2020-05-27 8:55 UTC (permalink / raw) To: Simon Ser; +Cc: Marek Olšák, dri-devel On Wed, May 27, 2020 at 08:52:00AM +0000, Simon Ser wrote: > There have suggestions to bake pitch alignment, address alignement, > contiguous memory or other placement (hidden VRAM, GTT/BAR, etc) > constraints into modifiers. Last time this was brought up it seemed > like the consensus was to not allow this. Document this in drm_fourcc.h. > > There are several reasons for this. > > - Encoding all of these constraints in the modifiers would explode the > search space pretty quickly (we only have 64 bits to work with). > - Modifiers need to be unambiguous: a buffer can only have a single > modifier. > - Modifier users aren't expected to parse modifiers. > > Signed-off-by: Simon Ser <contact@emersion.fr> > Cc: Daniel Vetter <daniel@ffwll.ch> > Cc: Daniel Stone <daniel@fooishbar.org> > Cc: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl> > Cc: Dave Airlie <airlied@gmail.com> > Cc: Marek Olšák <maraeo@gmail.com> > --- > include/uapi/drm/drm_fourcc.h | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h > index 490143500a50..97eb0f1cf9f8 100644 > --- a/include/uapi/drm/drm_fourcc.h > +++ b/include/uapi/drm/drm_fourcc.h > @@ -58,6 +58,17 @@ extern "C" { > * may preserve meaning - such as number of planes - from the fourcc code, > * whereas others may not. > * > + * Modifiers must uniquely encode buffer layout. In other words, a buffer must > + * match only a single modifier. A modifier must not be a subset of layouts of > + * another modifier. For instance, it's incorrect to encode pitch alignment in > + * a modifier: a buffer may match a 64-pixel aligned modifier and a 32-pixel > + * aligned modifier. That said, modifiers can have implicit minimal > + * requirements. I think we should also add the aliasing when the fourcc codes are involved, with afbc as example. Maybe: For modifiers where the combination of fourcc code and modifier can alias, a canonical pair needs to be defined and used by all drivers. An example is afbc, where both argb and abgr have the exact same compressed layout. With something like that added: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > + * > + * Users see modifiers as opaque tokens they can check for equality and > + * intersect. Users musn't need to know to reason about the modifier value > + * (i.e. users are not expected to extract information out of the modifier). > + * > * Vendors should document their modifier usage in as much detail as > * possible, to ensure maximum compatibility across devices, drivers and > * applications. > -- > 2.26.2 > > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/fourcc: document modifier uniqueness requirements 2020-05-27 8:55 ` Daniel Vetter @ 2020-06-01 10:35 ` Brian Starkey 2020-06-01 10:46 ` Simon Ser 2020-06-02 13:30 ` Daniel Vetter 0 siblings, 2 replies; 5+ messages in thread From: Brian Starkey @ 2020-06-01 10:35 UTC (permalink / raw) To: Daniel Vetter; +Cc: nd, dri-devel, Marek Olšák On Wed, May 27, 2020 at 10:55:34AM +0200, Daniel Vetter wrote: > On Wed, May 27, 2020 at 08:52:00AM +0000, Simon Ser wrote: > > There have suggestions to bake pitch alignment, address alignement, > > contiguous memory or other placement (hidden VRAM, GTT/BAR, etc) > > constraints into modifiers. Last time this was brought up it seemed > > like the consensus was to not allow this. Document this in drm_fourcc.h. > > > > There are several reasons for this. > > > > - Encoding all of these constraints in the modifiers would explode the > > search space pretty quickly (we only have 64 bits to work with). > > - Modifiers need to be unambiguous: a buffer can only have a single > > modifier. > > - Modifier users aren't expected to parse modifiers. > > > > Signed-off-by: Simon Ser <contact@emersion.fr> > > Cc: Daniel Vetter <daniel@ffwll.ch> > > Cc: Daniel Stone <daniel@fooishbar.org> > > Cc: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl> > > Cc: Dave Airlie <airlied@gmail.com> > > Cc: Marek Olšák <maraeo@gmail.com> > > --- > > include/uapi/drm/drm_fourcc.h | 11 +++++++++++ > > 1 file changed, 11 insertions(+) > > > > diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h > > index 490143500a50..97eb0f1cf9f8 100644 > > --- a/include/uapi/drm/drm_fourcc.h > > +++ b/include/uapi/drm/drm_fourcc.h > > @@ -58,6 +58,17 @@ extern "C" { > > * may preserve meaning - such as number of planes - from the fourcc code, > > * whereas others may not. > > * > > + * Modifiers must uniquely encode buffer layout. In other words, a buffer must > > + * match only a single modifier. A modifier must not be a subset of layouts of > > + * another modifier. For instance, it's incorrect to encode pitch alignment in > > + * a modifier: a buffer may match a 64-pixel aligned modifier and a 32-pixel > > + * aligned modifier. That said, modifiers can have implicit minimal > > + * requirements. > > I think we should also add the aliasing when the fourcc codes are > involved, with afbc as example. Maybe: > > For modifiers where the combination of fourcc code and modifier can alias, > a canonical pair needs to be defined and used by all drivers. An example > is afbc, where both argb and abgr have the exact same compressed layout. That's actually explicitly _not_ the case for AFBC, which was one of the things I was trying to document in afbc.rst. > > With something like that added: > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > > > + * > > + * Users see modifiers as opaque tokens they can check for equality and > > + * intersect. Users musn't need to know to reason about the modifier value > > + * (i.e. users are not expected to extract information out of the modifier). > > + * Doesn't this conflict with "implicit minimal requirements" above? Certainly for a bunch of different AFBC modifiers, the allocator would need to understand some fields in order to properly align-up the buffer size. Thanks, -Brian > > * Vendors should document their modifier usage in as much detail as > > * possible, to ensure maximum compatibility across devices, drivers and > > * applications. > > -- > > 2.26.2 > > > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/fourcc: document modifier uniqueness requirements 2020-06-01 10:35 ` Brian Starkey @ 2020-06-01 10:46 ` Simon Ser 2020-06-02 13:30 ` Daniel Vetter 1 sibling, 0 replies; 5+ messages in thread From: Simon Ser @ 2020-06-01 10:46 UTC (permalink / raw) To: Brian Starkey; +Cc: dri-devel, nd, Marek Olšák > > > + * Users see modifiers as opaque tokens they can check for equality and > > > + * intersect. Users musn't need to know to reason about the modifier value > > > + * (i.e. users are not expected to extract information out of the modifier). > > > + * > > Doesn't this conflict with "implicit minimal requirements" above? Ah, when I wrote "users", I meant "non-driver users": programs like compositors and user-space applications. Of course kernel and user-space drivers can extract information out of the modifiers. Is this why there's some confusion here? > Certainly for a bunch of different AFBC modifiers, the allocator would > need to understand some fields in order to properly align-up the > buffer size. It's probably a little early to speculate on the future allocator design. For instance, instead of having a monolithic allocator, the kernel driver (and other buffer consumers) could advertise a list of constraints for each format/modifier. That way no-one would need to extract information out of modifiers to figure out alignment (but maybe drivers would still want to, to reject bad imports for instance). But again, I'm just throwing ideas around at this point. _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/fourcc: document modifier uniqueness requirements 2020-06-01 10:35 ` Brian Starkey 2020-06-01 10:46 ` Simon Ser @ 2020-06-02 13:30 ` Daniel Vetter 1 sibling, 0 replies; 5+ messages in thread From: Daniel Vetter @ 2020-06-02 13:30 UTC (permalink / raw) To: Brian Starkey; +Cc: dri-devel, nd, Marek Olšák On Mon, Jun 01, 2020 at 11:35:54AM +0100, Brian Starkey wrote: > On Wed, May 27, 2020 at 10:55:34AM +0200, Daniel Vetter wrote: > > On Wed, May 27, 2020 at 08:52:00AM +0000, Simon Ser wrote: > > > There have suggestions to bake pitch alignment, address alignement, > > > contiguous memory or other placement (hidden VRAM, GTT/BAR, etc) > > > constraints into modifiers. Last time this was brought up it seemed > > > like the consensus was to not allow this. Document this in drm_fourcc.h. > > > > > > There are several reasons for this. > > > > > > - Encoding all of these constraints in the modifiers would explode the > > > search space pretty quickly (we only have 64 bits to work with). > > > - Modifiers need to be unambiguous: a buffer can only have a single > > > modifier. > > > - Modifier users aren't expected to parse modifiers. > > > > > > Signed-off-by: Simon Ser <contact@emersion.fr> > > > Cc: Daniel Vetter <daniel@ffwll.ch> > > > Cc: Daniel Stone <daniel@fooishbar.org> > > > Cc: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl> > > > Cc: Dave Airlie <airlied@gmail.com> > > > Cc: Marek Olšák <maraeo@gmail.com> > > > --- > > > include/uapi/drm/drm_fourcc.h | 11 +++++++++++ > > > 1 file changed, 11 insertions(+) > > > > > > diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h > > > index 490143500a50..97eb0f1cf9f8 100644 > > > --- a/include/uapi/drm/drm_fourcc.h > > > +++ b/include/uapi/drm/drm_fourcc.h > > > @@ -58,6 +58,17 @@ extern "C" { > > > * may preserve meaning - such as number of planes - from the fourcc code, > > > * whereas others may not. > > > * > > > + * Modifiers must uniquely encode buffer layout. In other words, a buffer must > > > + * match only a single modifier. A modifier must not be a subset of layouts of > > > + * another modifier. For instance, it's incorrect to encode pitch alignment in > > > + * a modifier: a buffer may match a 64-pixel aligned modifier and a 32-pixel > > > + * aligned modifier. That said, modifiers can have implicit minimal > > > + * requirements. > > > > I think we should also add the aliasing when the fourcc codes are > > involved, with afbc as example. Maybe: > > > > For modifiers where the combination of fourcc code and modifier can alias, > > a canonical pair needs to be defined and used by all drivers. An example > > is afbc, where both argb and abgr have the exact same compressed layout. > > That's actually explicitly _not_ the case for AFBC, which was one of > the things I was trying to document in afbc.rst. I guess I wasn't clear: I wanted to highlight afbc as one modifier where we discussed this aliasing problem, and worded the entire spec to make sure the aliasing doesn't happen. > > > > > With something like that added: > > > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > > > > > > + * > > > + * Users see modifiers as opaque tokens they can check for equality and > > > + * intersect. Users musn't need to know to reason about the modifier value > > > + * (i.e. users are not expected to extract information out of the modifier). > > > + * > > Doesn't this conflict with "implicit minimal requirements" above? > > Certainly for a bunch of different AFBC modifiers, the allocator would > need to understand some fields in order to properly align-up the > buffer size. There's kinda two side to the modifier coin: - one is how this all looks to the higher levels sitting on top of egl/kms/... For those modifiers should be opaque things. And we really don't care how much they alias, since worst case it's just a bunch more modifiers to compare and pass around. - the other side is the implement. For that side it very much matters that modifiers don't alias badly, so that we can avoid cases where the hw supports a common format, but the drivers use different aliases to discribe it, preventing buffer sharing in an efficient format. Finally we never let higher levels allocate the buffers, it's always just some low-level allocator that does that (gbm, egl, ...). And those lower levels obviously should understand the implementation and alignment constraints of the modifiers involved. Should we make this split a bit clearer of how this is supposed to work in userspace? -Daniel > > Thanks, > -Brian > > > > * Vendors should document their modifier usage in as much detail as > > > * possible, to ensure maximum compatibility across devices, drivers and > > > * applications. > > > -- > > > 2.26.2 > > > > > > > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch > > _______________________________________________ > > 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 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-06-02 13:30 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-27 8:52 [PATCH] drm/fourcc: document modifier uniqueness requirements Simon Ser 2020-05-27 8:55 ` Daniel Vetter 2020-06-01 10:35 ` Brian Starkey 2020-06-01 10:46 ` Simon Ser 2020-06-02 13:30 ` Daniel Vetter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).