| Message ID | 20230428100115.9802-2-contact@emersion.fr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] drm: fix code style for embedded structs in hdr_metadata_infoframe | expand |
On Fri, 28 Apr 2023 10:01:29 +0000 Simon Ser <contact@emersion.fr> wrote: > This avoids hard-coding magic values in user-space, and makes our > documentation clearer. > > Signed-off-by: Simon Ser <contact@emersion.fr> > Cc: Harry Wentland <harry.wentland@amd.com> > Cc: Daniel Vetter <daniel@ffwll.ch> > Cc: Sebastian Wick <sebastian.wick@redhat.com> > Cc: Joshua Ashton <joshua@froggi.es> > Cc: Pekka Paalanen <pekka.paalanen@collabora.com> > --- > include/uapi/drm/drm_mode.h | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h > index 997d23fb2d68..c0c40dc9e2f1 100644 > --- a/include/uapi/drm/drm_mode.h > +++ b/include/uapi/drm/drm_mode.h > @@ -849,6 +849,16 @@ struct drm_color_lut { > __u16 reserved; > }; > > +/** > + * enum drm_hdr_metadata_type - HDR metadata descriptor ID. > + */ > +enum drm_hdr_metadata_type { > + /** > + * @DRM_HDR_STATIC_METADATA_TYPE1: Static Metadata Type 1. > + */ > + DRM_HDR_STATIC_METADATA_TYPE1 = 0, > +}; Hi, the subject says "define" but this is an enum. No big deal, but the thing I started wondering is how I am going to use these in userspace. There is no #define I could test to know if I need to provide a fallback definition. What's the migration plan for userspace to start using this and patch 3? Nevertheless, these are good to have. Oh, is it a libdrm version check I should use? In that case, patches 2 and 3 are Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.com> Thanks, pq > + > /** > * struct hdr_metadata_infoframe - HDR Metadata Infoframe Data. > * > @@ -866,6 +876,7 @@ struct hdr_metadata_infoframe { > __u8 eotf; > /** > * @metadata_type: Static_Metadata_Descriptor_ID. > + * This must be &DRM_HDR_STATIC_METADATA_TYPE1. > */ > __u8 metadata_type; > /** > @@ -925,6 +936,7 @@ struct hdr_metadata_infoframe { > struct hdr_output_metadata { > /** > * @metadata_type: Static_Metadata_Descriptor_ID. > + * This must be &DRM_HDR_STATIC_METADATA_TYPE1. > */ > __u32 metadata_type; > /**
On Friday, April 28th, 2023 at 15:12, Pekka Paalanen <ppaalanen@gmail.com> wrote: > the subject says "define" but this is an enum. No big deal, but the > thing I started wondering is how I am going to use these in userspace. > There is no #define I could test to know if I need to provide a > fallback definition. What's the migration plan for userspace to start > using this and patch 3? > > Nevertheless, these are good to have. > > Oh, is it a libdrm version check I should use? Yeah, that's one way to do it. Another way would be cc.has_header_symbol(). The same kind of situation happened when struct hdr_output_metadata was introduced. I chose to use enums because they better group the different values and document the intent. If I had gone for #defines, the docs would need to either list each individual #define or refer to "DRM_HDR_EOTF_*" which doesn't get linkified.
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 997d23fb2d68..c0c40dc9e2f1 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -849,6 +849,16 @@ struct drm_color_lut { __u16 reserved; }; +/** + * enum drm_hdr_metadata_type - HDR metadata descriptor ID. + */ +enum drm_hdr_metadata_type { + /** + * @DRM_HDR_STATIC_METADATA_TYPE1: Static Metadata Type 1. + */ + DRM_HDR_STATIC_METADATA_TYPE1 = 0, +}; + /** * struct hdr_metadata_infoframe - HDR Metadata Infoframe Data. * @@ -866,6 +876,7 @@ struct hdr_metadata_infoframe { __u8 eotf; /** * @metadata_type: Static_Metadata_Descriptor_ID. + * This must be &DRM_HDR_STATIC_METADATA_TYPE1. */ __u8 metadata_type; /** @@ -925,6 +936,7 @@ struct hdr_metadata_infoframe { struct hdr_output_metadata { /** * @metadata_type: Static_Metadata_Descriptor_ID. + * This must be &DRM_HDR_STATIC_METADATA_TYPE1. */ __u32 metadata_type; /**
This avoids hard-coding magic values in user-space, and makes our documentation clearer. Signed-off-by: Simon Ser <contact@emersion.fr> Cc: Harry Wentland <harry.wentland@amd.com> Cc: Daniel Vetter <daniel@ffwll.ch> Cc: Sebastian Wick <sebastian.wick@redhat.com> Cc: Joshua Ashton <joshua@froggi.es> Cc: Pekka Paalanen <pekka.paalanen@collabora.com> --- include/uapi/drm/drm_mode.h | 12 ++++++++++++ 1 file changed, 12 insertions(+)