| Message ID | 20200626135233.32137-1-brian.starkey@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: drm_fourcc: Add generic alias for 16_16_TILE modifier | expand |
On Fri, Jun 26, 2020 at 3:52 PM Brian Starkey <brian.starkey@arm.com> wrote: > > In cases such as DRM_FORMAT_MOD_SAMSUNG_16_16_TILE, the modifier > describes a generic pixel re-ordering which can be applicable to > multiple vendors. > > Define an alias: DRM_FORMAT_MOD_GENERIC_16_16_TILE, which can be > used to describe this layout in a vendor-neutral way, and add a > comment about the expected usage of such "generic" modifiers. > > Signed-off-by: Brian Starkey <brian.starkey@arm.com> > --- > > This is based off a suggestion from Daniel here: > https://lore.kernel.org/dri-devel/20200529115328.GC1630358@phenom.ffwll.local/ > > If there are future cases where a "generic" modifier is identified > before merging with a specific vendor code, perhaps we could use > `fourcc_mod_code(NONE, n)` or add a DRM_FORMAT_MOD_VENDOR_GENERIC. > > However, I also think we shouldn't try and "guess" what might be generic > up-front and end up polluting the generic namespace. It seems fine to > just alias vendor-specific definitions unless it's very clear-cut. I think the above two things would also be good additions to the comment. A totally different thing: I just noticed that we don't pull in all the comments for modifiers. I think we could convert them to kerneldoc, and then pull them into a separate section in drm-kms.rst. Maybe even worth to pull out into a new drm-fourcc.rst document, since these are officially shared with gl and vk standard extensions. Then this new modifier's doc could simply point at teh existing one (and the generated kerneldoc would make that a hyperlink for added awesomeness). Aside from that makes sense to me, maybe just add it to your patch series that will start making use of these. -Daniel > > I typed a version which was s/GENERIC/COMMON/, other suggestions > welcome. > > Cheers, > -Brian > > include/uapi/drm/drm_fourcc.h | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h > index 299f9ac4840b..ef78dc9c3fb0 100644 > --- a/include/uapi/drm/drm_fourcc.h > +++ b/include/uapi/drm/drm_fourcc.h > @@ -345,8 +345,27 @@ extern "C" { > * When adding a new token please document the layout with a code comment, > * similar to the fourcc codes above. drm_fourcc.h is considered the > * authoritative source for all of these. > + * > + * Generic modifier names: > + * > + * DRM_FORMAT_MOD_GENERIC_* definitions are used to provide vendor-neutral names > + * for layouts which are common across multiple vendors. To preserve > + * compatibility, in cases where a vendor-specific definition already exists and > + * a generic name for it is desired, the common name is a purely symbolic alias > + * and must use the same numerical value as the original definition. > + * > + * Note that generic names should only be used for modifiers which describe > + * generic layouts (such as pixel re-ordering), which may have > + * independently-developed support across multiple vendors. > + * > + * Generic names should not be used for cases where multiple hardware vendors > + * have implementations of the same standardised compression scheme (such as > + * AFBC). In those cases, all implementations should use the same format > + * modifier(s), reflecting the vendor of the standard. > */ > > +#define DRM_FORMAT_MOD_GENERIC_16_16_TILE DRM_FORMAT_MOD_SAMSUNG_16_16_TILE > + > /* > * Invalid Modifier > * > -- > 2.17.1 >
Hi Daniel, On Fri, Jun 26, 2020 at 04:07:43PM +0200, Daniel Vetter wrote: > On Fri, Jun 26, 2020 at 3:52 PM Brian Starkey <brian.starkey@arm.com> wrote: > > > > In cases such as DRM_FORMAT_MOD_SAMSUNG_16_16_TILE, the modifier > > describes a generic pixel re-ordering which can be applicable to > > multiple vendors. > > > > Define an alias: DRM_FORMAT_MOD_GENERIC_16_16_TILE, which can be > > used to describe this layout in a vendor-neutral way, and add a > > comment about the expected usage of such "generic" modifiers. > > > > Signed-off-by: Brian Starkey <brian.starkey@arm.com> > > --- > > > > This is based off a suggestion from Daniel here: > > https://lore.kernel.org/dri-devel/20200529115328.GC1630358@phenom.ffwll.local/ > > > > If there are future cases where a "generic" modifier is identified > > before merging with a specific vendor code, perhaps we could use > > `fourcc_mod_code(NONE, n)` or add a DRM_FORMAT_MOD_VENDOR_GENERIC. > > > > However, I also think we shouldn't try and "guess" what might be generic > > up-front and end up polluting the generic namespace. It seems fine to > > just alias vendor-specific definitions unless it's very clear-cut. > > I think the above two things would also be good additions to the comment. > > A totally different thing: I just noticed that we don't pull in all > the comments for modifiers. I think we could convert them to > kerneldoc, and then pull them into a separate section in drm-kms.rst. > Maybe even worth to pull out into a new drm-fourcc.rst document, since > these are officially shared with gl and vk standard extensions. Then > this new modifier's doc could simply point at teh existing one (and > the generated kerneldoc would make that a hyperlink for added > awesomeness). > > Aside from that makes sense to me, maybe just add it to your patch > series that will start making use of these. This is only supported by the GPU, so we wouldn't be using this on the Display side. Thanks, -Brian > -Daniel > > > > > I typed a version which was s/GENERIC/COMMON/, other suggestions > > welcome. > > > > Cheers, > > -Brian > > > > include/uapi/drm/drm_fourcc.h | 19 +++++++++++++++++++ > > 1 file changed, 19 insertions(+) > > > > diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h > > index 299f9ac4840b..ef78dc9c3fb0 100644 > > --- a/include/uapi/drm/drm_fourcc.h > > +++ b/include/uapi/drm/drm_fourcc.h > > @@ -345,8 +345,27 @@ extern "C" { > > * When adding a new token please document the layout with a code comment, > > * similar to the fourcc codes above. drm_fourcc.h is considered the > > * authoritative source for all of these. > > + * > > + * Generic modifier names: > > + * > > + * DRM_FORMAT_MOD_GENERIC_* definitions are used to provide vendor-neutral names > > + * for layouts which are common across multiple vendors. To preserve > > + * compatibility, in cases where a vendor-specific definition already exists and > > + * a generic name for it is desired, the common name is a purely symbolic alias > > + * and must use the same numerical value as the original definition. > > + * > > + * Note that generic names should only be used for modifiers which describe > > + * generic layouts (such as pixel re-ordering), which may have > > + * independently-developed support across multiple vendors. > > + * > > + * Generic names should not be used for cases where multiple hardware vendors > > + * have implementations of the same standardised compression scheme (such as > > + * AFBC). In those cases, all implementations should use the same format > > + * modifier(s), reflecting the vendor of the standard. > > */ > > > > +#define DRM_FORMAT_MOD_GENERIC_16_16_TILE DRM_FORMAT_MOD_SAMSUNG_16_16_TILE > > + > > /* > > * Invalid Modifier > > * > > -- > > 2.17.1 > > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
On Fri, Jun 26, 2020 at 4:16 PM Brian Starkey <brian.starkey@arm.com> wrote: > > Hi Daniel, > > On Fri, Jun 26, 2020 at 04:07:43PM +0200, Daniel Vetter wrote: > > On Fri, Jun 26, 2020 at 3:52 PM Brian Starkey <brian.starkey@arm.com> wrote: > > > > > > In cases such as DRM_FORMAT_MOD_SAMSUNG_16_16_TILE, the modifier > > > describes a generic pixel re-ordering which can be applicable to > > > multiple vendors. > > > > > > Define an alias: DRM_FORMAT_MOD_GENERIC_16_16_TILE, which can be > > > used to describe this layout in a vendor-neutral way, and add a > > > comment about the expected usage of such "generic" modifiers. > > > > > > Signed-off-by: Brian Starkey <brian.starkey@arm.com> > > > --- > > > > > > This is based off a suggestion from Daniel here: > > > https://lore.kernel.org/dri-devel/20200529115328.GC1630358@phenom.ffwll.local/ > > > > > > If there are future cases where a "generic" modifier is identified > > > before merging with a specific vendor code, perhaps we could use > > > `fourcc_mod_code(NONE, n)` or add a DRM_FORMAT_MOD_VENDOR_GENERIC. > > > > > > However, I also think we shouldn't try and "guess" what might be generic > > > up-front and end up polluting the generic namespace. It seems fine to > > > just alias vendor-specific definitions unless it's very clear-cut. > > > > I think the above two things would also be good additions to the comment. > > > > A totally different thing: I just noticed that we don't pull in all > > the comments for modifiers. I think we could convert them to > > kerneldoc, and then pull them into a separate section in drm-kms.rst. > > Maybe even worth to pull out into a new drm-fourcc.rst document, since > > these are officially shared with gl and vk standard extensions. Then > > this new modifier's doc could simply point at teh existing one (and > > the generated kerneldoc would make that a hyperlink for added > > awesomeness). > > > > Aside from that makes sense to me, maybe just add it to your patch > > series that will start making use of these. > > This is only supported by the GPU, so we wouldn't be using this on the > Display side. I mean the patch to add the NV15 fource, it mentions "suitable for 16 by 16 tile layout", would be nice to just put the generic modifier in that comment. -Daniel > > Thanks, > -Brian > > > -Daniel > > > > > > > > I typed a version which was s/GENERIC/COMMON/, other suggestions > > > welcome. > > > > > > Cheers, > > > -Brian > > > > > > include/uapi/drm/drm_fourcc.h | 19 +++++++++++++++++++ > > > 1 file changed, 19 insertions(+) > > > > > > diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h > > > index 299f9ac4840b..ef78dc9c3fb0 100644 > > > --- a/include/uapi/drm/drm_fourcc.h > > > +++ b/include/uapi/drm/drm_fourcc.h > > > @@ -345,8 +345,27 @@ extern "C" { > > > * When adding a new token please document the layout with a code comment, > > > * similar to the fourcc codes above. drm_fourcc.h is considered the > > > * authoritative source for all of these. > > > + * > > > + * Generic modifier names: > > > + * > > > + * DRM_FORMAT_MOD_GENERIC_* definitions are used to provide vendor-neutral names > > > + * for layouts which are common across multiple vendors. To preserve > > > + * compatibility, in cases where a vendor-specific definition already exists and > > > + * a generic name for it is desired, the common name is a purely symbolic alias > > > + * and must use the same numerical value as the original definition. > > > + * > > > + * Note that generic names should only be used for modifiers which describe > > > + * generic layouts (such as pixel re-ordering), which may have > > > + * independently-developed support across multiple vendors. > > > + * > > > + * Generic names should not be used for cases where multiple hardware vendors > > > + * have implementations of the same standardised compression scheme (such as > > > + * AFBC). In those cases, all implementations should use the same format > > > + * modifier(s), reflecting the vendor of the standard. > > > */ > > > > > > +#define DRM_FORMAT_MOD_GENERIC_16_16_TILE DRM_FORMAT_MOD_SAMSUNG_16_16_TILE > > > + > > > /* > > > * Invalid Modifier > > > * > > > -- > > > 2.17.1 > > > > > > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
On Fri, Jun 26, 2020 at 05:16:53PM +0200, Daniel Vetter wrote: > On Fri, Jun 26, 2020 at 4:16 PM Brian Starkey <brian.starkey@arm.com> wrote: > > > > Hi Daniel, > > > > On Fri, Jun 26, 2020 at 04:07:43PM +0200, Daniel Vetter wrote: > > > On Fri, Jun 26, 2020 at 3:52 PM Brian Starkey <brian.starkey@arm.com> wrote: > > > > > > > > In cases such as DRM_FORMAT_MOD_SAMSUNG_16_16_TILE, the modifier > > > > describes a generic pixel re-ordering which can be applicable to > > > > multiple vendors. > > > > > > > > Define an alias: DRM_FORMAT_MOD_GENERIC_16_16_TILE, which can be > > > > used to describe this layout in a vendor-neutral way, and add a > > > > comment about the expected usage of such "generic" modifiers. > > > > > > > > Signed-off-by: Brian Starkey <brian.starkey@arm.com> > > > > --- > > > > > > > > This is based off a suggestion from Daniel here: > > > > https://lore.kernel.org/dri-devel/20200529115328.GC1630358@phenom.ffwll.local/ > > > > > > > > If there are future cases where a "generic" modifier is identified > > > > before merging with a specific vendor code, perhaps we could use > > > > `fourcc_mod_code(NONE, n)` or add a DRM_FORMAT_MOD_VENDOR_GENERIC. > > > > > > > > However, I also think we shouldn't try and "guess" what might be generic > > > > up-front and end up polluting the generic namespace. It seems fine to > > > > just alias vendor-specific definitions unless it's very clear-cut. > > > > > > I think the above two things would also be good additions to the comment. > > > > > > A totally different thing: I just noticed that we don't pull in all > > > the comments for modifiers. I think we could convert them to > > > kerneldoc, and then pull them into a separate section in drm-kms.rst. > > > Maybe even worth to pull out into a new drm-fourcc.rst document, since > > > these are officially shared with gl and vk standard extensions. Then > > > this new modifier's doc could simply point at teh existing one (and > > > the generated kerneldoc would make that a hyperlink for added > > > awesomeness). > > > > > > Aside from that makes sense to me, maybe just add it to your patch > > > series that will start making use of these. > > > > This is only supported by the GPU, so we wouldn't be using this on the > > Display side. > > I mean the patch to add the NV15 fource, it mentions "suitable for 16 > by 16 tile layout", would be nice to just put the generic modifier in > that comment. > -Daniel Ah right. I saw that one went out in Maarten's pull request this morning. -Brian > > > > > Thanks, > > -Brian > > > > > -Daniel > > > > > > > > > > > I typed a version which was s/GENERIC/COMMON/, other suggestions > > > > welcome. > > > > > > > > Cheers, > > > > -Brian > > > > > > > > include/uapi/drm/drm_fourcc.h | 19 +++++++++++++++++++ > > > > 1 file changed, 19 insertions(+) > > > > > > > > diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h > > > > index 299f9ac4840b..ef78dc9c3fb0 100644 > > > > --- a/include/uapi/drm/drm_fourcc.h > > > > +++ b/include/uapi/drm/drm_fourcc.h > > > > @@ -345,8 +345,27 @@ extern "C" { > > > > * When adding a new token please document the layout with a code comment, > > > > * similar to the fourcc codes above. drm_fourcc.h is considered the > > > > * authoritative source for all of these. > > > > + * > > > > + * Generic modifier names: > > > > + * > > > > + * DRM_FORMAT_MOD_GENERIC_* definitions are used to provide vendor-neutral names > > > > + * for layouts which are common across multiple vendors. To preserve > > > > + * compatibility, in cases where a vendor-specific definition already exists and > > > > + * a generic name for it is desired, the common name is a purely symbolic alias > > > > + * and must use the same numerical value as the original definition. > > > > + * > > > > + * Note that generic names should only be used for modifiers which describe > > > > + * generic layouts (such as pixel re-ordering), which may have > > > > + * independently-developed support across multiple vendors. > > > > + * > > > > + * Generic names should not be used for cases where multiple hardware vendors > > > > + * have implementations of the same standardised compression scheme (such as > > > > + * AFBC). In those cases, all implementations should use the same format > > > > + * modifier(s), reflecting the vendor of the standard. > > > > */ > > > > > > > > +#define DRM_FORMAT_MOD_GENERIC_16_16_TILE DRM_FORMAT_MOD_SAMSUNG_16_16_TILE > > > > + > > > > /* > > > > * Invalid Modifier > > > > * > > > > -- > > > > 2.17.1 > > > > > > > > > > > > > -- > > > Daniel Vetter > > > Software Engineer, Intel Corporation > > > http://blog.ffwll.ch > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
On Fri, Jun 26, 2020 at 5:21 PM Brian Starkey <brian.starkey@arm.com> wrote: > > On Fri, Jun 26, 2020 at 05:16:53PM +0200, Daniel Vetter wrote: > > On Fri, Jun 26, 2020 at 4:16 PM Brian Starkey <brian.starkey@arm.com> wrote: > > > > > > Hi Daniel, > > > > > > On Fri, Jun 26, 2020 at 04:07:43PM +0200, Daniel Vetter wrote: > > > > On Fri, Jun 26, 2020 at 3:52 PM Brian Starkey <brian.starkey@arm.com> wrote: > > > > > > > > > > In cases such as DRM_FORMAT_MOD_SAMSUNG_16_16_TILE, the modifier > > > > > describes a generic pixel re-ordering which can be applicable to > > > > > multiple vendors. > > > > > > > > > > Define an alias: DRM_FORMAT_MOD_GENERIC_16_16_TILE, which can be > > > > > used to describe this layout in a vendor-neutral way, and add a > > > > > comment about the expected usage of such "generic" modifiers. > > > > > > > > > > Signed-off-by: Brian Starkey <brian.starkey@arm.com> > > > > > --- > > > > > > > > > > This is based off a suggestion from Daniel here: > > > > > https://lore.kernel.org/dri-devel/20200529115328.GC1630358@phenom.ffwll.local/ > > > > > > > > > > If there are future cases where a "generic" modifier is identified > > > > > before merging with a specific vendor code, perhaps we could use > > > > > `fourcc_mod_code(NONE, n)` or add a DRM_FORMAT_MOD_VENDOR_GENERIC. > > > > > > > > > > However, I also think we shouldn't try and "guess" what might be generic > > > > > up-front and end up polluting the generic namespace. It seems fine to > > > > > just alias vendor-specific definitions unless it's very clear-cut. > > > > > > > > I think the above two things would also be good additions to the comment. > > > > > > > > A totally different thing: I just noticed that we don't pull in all > > > > the comments for modifiers. I think we could convert them to > > > > kerneldoc, and then pull them into a separate section in drm-kms.rst. > > > > Maybe even worth to pull out into a new drm-fourcc.rst document, since > > > > these are officially shared with gl and vk standard extensions. Then > > > > this new modifier's doc could simply point at teh existing one (and > > > > the generated kerneldoc would make that a hyperlink for added > > > > awesomeness). > > > > > > > > Aside from that makes sense to me, maybe just add it to your patch > > > > series that will start making use of these. > > > > > > This is only supported by the GPU, so we wouldn't be using this on the > > > Display side. > > > > I mean the patch to add the NV15 fource, it mentions "suitable for 16 > > by 16 tile layout", would be nice to just put the generic modifier in > > that comment. > > -Daniel > > Ah right. I saw that one went out in Maarten's pull request this > morning. Oh I missed that, then maybe just amend this patch to update the other comment? -Daniel > > -Brian > > > > > > > > > Thanks, > > > -Brian > > > > > > > -Daniel > > > > > > > > > > > > > > I typed a version which was s/GENERIC/COMMON/, other suggestions > > > > > welcome. > > > > > > > > > > Cheers, > > > > > -Brian > > > > > > > > > > include/uapi/drm/drm_fourcc.h | 19 +++++++++++++++++++ > > > > > 1 file changed, 19 insertions(+) > > > > > > > > > > diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h > > > > > index 299f9ac4840b..ef78dc9c3fb0 100644 > > > > > --- a/include/uapi/drm/drm_fourcc.h > > > > > +++ b/include/uapi/drm/drm_fourcc.h > > > > > @@ -345,8 +345,27 @@ extern "C" { > > > > > * When adding a new token please document the layout with a code comment, > > > > > * similar to the fourcc codes above. drm_fourcc.h is considered the > > > > > * authoritative source for all of these. > > > > > + * > > > > > + * Generic modifier names: > > > > > + * > > > > > + * DRM_FORMAT_MOD_GENERIC_* definitions are used to provide vendor-neutral names > > > > > + * for layouts which are common across multiple vendors. To preserve > > > > > + * compatibility, in cases where a vendor-specific definition already exists and > > > > > + * a generic name for it is desired, the common name is a purely symbolic alias > > > > > + * and must use the same numerical value as the original definition. > > > > > + * > > > > > + * Note that generic names should only be used for modifiers which describe > > > > > + * generic layouts (such as pixel re-ordering), which may have > > > > > + * independently-developed support across multiple vendors. > > > > > + * > > > > > + * Generic names should not be used for cases where multiple hardware vendors > > > > > + * have implementations of the same standardised compression scheme (such as > > > > > + * AFBC). In those cases, all implementations should use the same format > > > > > + * modifier(s), reflecting the vendor of the standard. > > > > > */ > > > > > > > > > > +#define DRM_FORMAT_MOD_GENERIC_16_16_TILE DRM_FORMAT_MOD_SAMSUNG_16_16_TILE > > > > > + > > > > > /* > > > > > * Invalid Modifier > > > > > * > > > > > -- > > > > > 2.17.1 > > > > > > > > > > > > > > > > > -- > > > > Daniel Vetter > > > > Software Engineer, Intel Corporation > > > > http://blog.ffwll.ch > > > > > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch
On Fri, Jun 26, 2020 at 06:15:36PM +0200, Daniel Vetter wrote: > On Fri, Jun 26, 2020 at 5:21 PM Brian Starkey <brian.starkey@arm.com> wrote: > > > > On Fri, Jun 26, 2020 at 05:16:53PM +0200, Daniel Vetter wrote: > > > On Fri, Jun 26, 2020 at 4:16 PM Brian Starkey <brian.starkey@arm.com> wrote: > > > > > > > > Hi Daniel, > > > > > > > > On Fri, Jun 26, 2020 at 04:07:43PM +0200, Daniel Vetter wrote: > > > > > On Fri, Jun 26, 2020 at 3:52 PM Brian Starkey <brian.starkey@arm.com> wrote: > > > > > > > > > > > > In cases such as DRM_FORMAT_MOD_SAMSUNG_16_16_TILE, the modifier > > > > > > describes a generic pixel re-ordering which can be applicable to > > > > > > multiple vendors. > > > > > > > > > > > > Define an alias: DRM_FORMAT_MOD_GENERIC_16_16_TILE, which can be > > > > > > used to describe this layout in a vendor-neutral way, and add a > > > > > > comment about the expected usage of such "generic" modifiers. > > > > > > > > > > > > Signed-off-by: Brian Starkey <brian.starkey@arm.com> > > > > > > --- > > > > > > > > > > > > This is based off a suggestion from Daniel here: > > > > > > https://lore.kernel.org/dri-devel/20200529115328.GC1630358@phenom.ffwll.local/ > > > > > > > > > > > > If there are future cases where a "generic" modifier is identified > > > > > > before merging with a specific vendor code, perhaps we could use > > > > > > `fourcc_mod_code(NONE, n)` or add a DRM_FORMAT_MOD_VENDOR_GENERIC. > > > > > > > > > > > > However, I also think we shouldn't try and "guess" what might be generic > > > > > > up-front and end up polluting the generic namespace. It seems fine to > > > > > > just alias vendor-specific definitions unless it's very clear-cut. > > > > > > > > > > I think the above two things would also be good additions to the comment. > > > > > > > > > > A totally different thing: I just noticed that we don't pull in all > > > > > the comments for modifiers. I think we could convert them to > > > > > kerneldoc, and then pull them into a separate section in drm-kms.rst. > > > > > Maybe even worth to pull out into a new drm-fourcc.rst document, since > > > > > these are officially shared with gl and vk standard extensions. Then > > > > > this new modifier's doc could simply point at teh existing one (and > > > > > the generated kerneldoc would make that a hyperlink for added > > > > > awesomeness). > > > > > > > > > > Aside from that makes sense to me, maybe just add it to your patch > > > > > series that will start making use of these. > > > > > > > > This is only supported by the GPU, so we wouldn't be using this on the > > > > Display side. > > > > > > I mean the patch to add the NV15 fource, it mentions "suitable for 16 > > > by 16 tile layout", would be nice to just put the generic modifier in > > > that comment. > > > -Daniel > > > > Ah right. I saw that one went out in Maarten's pull request this > > morning. > > Oh I missed that, then maybe just amend this patch to update the other comment? It was only mentioned in the commit message there, so I guess I can't do anything about it now. I'll respin to put the extra notes into the comment. Thanks, -Brian > -Daniel > > > > > -Brian > > > > > > > > > > > > > Thanks, > > > > -Brian > > > > > > > > > -Daniel > > > > > > > > > > > > > > > > > I typed a version which was s/GENERIC/COMMON/, other suggestions > > > > > > welcome. > > > > > > > > > > > > Cheers, > > > > > > -Brian > > > > > > > > > > > > include/uapi/drm/drm_fourcc.h | 19 +++++++++++++++++++ > > > > > > 1 file changed, 19 insertions(+) > > > > > > > > > > > > diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h > > > > > > index 299f9ac4840b..ef78dc9c3fb0 100644 > > > > > > --- a/include/uapi/drm/drm_fourcc.h > > > > > > +++ b/include/uapi/drm/drm_fourcc.h > > > > > > @@ -345,8 +345,27 @@ extern "C" { > > > > > > * When adding a new token please document the layout with a code comment, > > > > > > * similar to the fourcc codes above. drm_fourcc.h is considered the > > > > > > * authoritative source for all of these. > > > > > > + * > > > > > > + * Generic modifier names: > > > > > > + * > > > > > > + * DRM_FORMAT_MOD_GENERIC_* definitions are used to provide vendor-neutral names > > > > > > + * for layouts which are common across multiple vendors. To preserve > > > > > > + * compatibility, in cases where a vendor-specific definition already exists and > > > > > > + * a generic name for it is desired, the common name is a purely symbolic alias > > > > > > + * and must use the same numerical value as the original definition. > > > > > > + * > > > > > > + * Note that generic names should only be used for modifiers which describe > > > > > > + * generic layouts (such as pixel re-ordering), which may have > > > > > > + * independently-developed support across multiple vendors. > > > > > > + * > > > > > > + * Generic names should not be used for cases where multiple hardware vendors > > > > > > + * have implementations of the same standardised compression scheme (such as > > > > > > + * AFBC). In those cases, all implementations should use the same format > > > > > > + * modifier(s), reflecting the vendor of the standard. > > > > > > */ > > > > > > > > > > > > +#define DRM_FORMAT_MOD_GENERIC_16_16_TILE DRM_FORMAT_MOD_SAMSUNG_16_16_TILE > > > > > > + > > > > > > /* > > > > > > * Invalid Modifier > > > > > > * > > > > > > -- > > > > > > 2.17.1 > > > > > > > > > > > > > > > > > > > > > -- > > > > > Daniel Vetter > > > > > Software Engineer, Intel Corporation > > > > > http://blog.ffwll.ch > > > > > > > > > > > > -- > > > Daniel Vetter > > > Software Engineer, Intel Corporation > > > http://blog.ffwll.ch > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h index 299f9ac4840b..ef78dc9c3fb0 100644 --- a/include/uapi/drm/drm_fourcc.h +++ b/include/uapi/drm/drm_fourcc.h @@ -345,8 +345,27 @@ extern "C" { * When adding a new token please document the layout with a code comment, * similar to the fourcc codes above. drm_fourcc.h is considered the * authoritative source for all of these. + * + * Generic modifier names: + * + * DRM_FORMAT_MOD_GENERIC_* definitions are used to provide vendor-neutral names + * for layouts which are common across multiple vendors. To preserve + * compatibility, in cases where a vendor-specific definition already exists and + * a generic name for it is desired, the common name is a purely symbolic alias + * and must use the same numerical value as the original definition. + * + * Note that generic names should only be used for modifiers which describe + * generic layouts (such as pixel re-ordering), which may have + * independently-developed support across multiple vendors. + * + * Generic names should not be used for cases where multiple hardware vendors + * have implementations of the same standardised compression scheme (such as + * AFBC). In those cases, all implementations should use the same format + * modifier(s), reflecting the vendor of the standard. */ +#define DRM_FORMAT_MOD_GENERIC_16_16_TILE DRM_FORMAT_MOD_SAMSUNG_16_16_TILE + /* * Invalid Modifier *
In cases such as DRM_FORMAT_MOD_SAMSUNG_16_16_TILE, the modifier describes a generic pixel re-ordering which can be applicable to multiple vendors. Define an alias: DRM_FORMAT_MOD_GENERIC_16_16_TILE, which can be used to describe this layout in a vendor-neutral way, and add a comment about the expected usage of such "generic" modifiers. Signed-off-by: Brian Starkey <brian.starkey@arm.com> --- This is based off a suggestion from Daniel here: https://lore.kernel.org/dri-devel/20200529115328.GC1630358@phenom.ffwll.local/ If there are future cases where a "generic" modifier is identified before merging with a specific vendor code, perhaps we could use `fourcc_mod_code(NONE, n)` or add a DRM_FORMAT_MOD_VENDOR_GENERIC. However, I also think we shouldn't try and "guess" what might be generic up-front and end up polluting the generic namespace. It seems fine to just alias vendor-specific definitions unless it's very clear-cut. I typed a version which was s/GENERIC/COMMON/, other suggestions welcome. Cheers, -Brian include/uapi/drm/drm_fourcc.h | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)