| Message ID | 1553078906-5991-2-git-send-email-uma.shankar@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add HDR Metadata Parsing and handling in DRM layer | expand |
Hi Uma, On Wed, Mar 20, 2019 at 04:18:14PM +0530, Uma Shankar wrote: > This patch adds a blob property to get HDR metadata > information from userspace. This will be send as part > of AVI Infoframe to panel. > > v2: Rebase and modified the metadata structure elements > as per Ville's POC changes. > > v3: No Change > > v4: Addressed Shashank's review comments > > v5: Rebase. > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > --- > drivers/gpu/drm/drm_connector.c | 6 ++++++ > include/drm/drm_connector.h | 10 ++++++++++ > include/drm/drm_mode_config.h | 6 ++++++ > include/linux/hdmi.h | 10 ++++++++++ > include/uapi/drm/drm_mode.h | 16 ++++++++++++++++ > 5 files changed, 48 insertions(+) > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > index 2355124..0bdae90 100644 > --- a/drivers/gpu/drm/drm_connector.c > +++ b/drivers/gpu/drm/drm_connector.c > @@ -1058,6 +1058,12 @@ int drm_connector_create_standard_properties(struct drm_device *dev) > return -ENOMEM; > dev->mode_config.non_desktop_property = prop; > > + prop = drm_property_create(dev, DRM_MODE_PROP_BLOB, > + "HDR_OUTPUT_METADATA", 0); > + if (!prop) > + return -ENOMEM; > + dev->mode_config.hdr_output_metadata_property = prop; > + > return 0; > } > > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h > index c806199..29388bd 100644 > --- a/include/drm/drm_connector.h > +++ b/include/drm/drm_connector.h > @@ -561,6 +561,13 @@ struct drm_connector_state { > * and the connector bpc limitations obtained from edid. > */ > u8 max_bpc; > + > + /** > + * @metadata_blob_ptr: > + * DRM blob property for HDR output metadata > + */ > + struct drm_property_blob *hdr_output_metadata_blob_ptr; > + u8 hdr_metadata_changed : 1; > }; > > /** > @@ -1201,6 +1208,9 @@ struct drm_connector { > * &drm_mode_config.connector_free_work. > */ > struct llist_node free_node; > + > + /* HDR metdata */ > + struct hdr_static_metadata hdr_metadata; > }; > > #define obj_to_connector(x) container_of(x, struct drm_connector, base) > diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h > index 7f60e8e..ef2656b 100644 > --- a/include/drm/drm_mode_config.h > +++ b/include/drm/drm_mode_config.h > @@ -836,6 +836,12 @@ struct drm_mode_config { > */ > struct drm_property *writeback_out_fence_ptr_property; > > + /* > + * hdr_metadata_property: Connector property containing hdr metatda > + * This will be provided by userspace compositors based on HDR content > + */ > + struct drm_property *hdr_output_metadata_property; > + > /* dumb ioctl parameters */ > uint32_t preferred_depth, prefer_shadow; > > diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h > index 927ad64..a065194 100644 > --- a/include/linux/hdmi.h > +++ b/include/linux/hdmi.h > @@ -152,6 +152,16 @@ enum hdmi_content_type { > HDMI_CONTENT_TYPE_GAME, > }; > > +enum hdmi_metadata_type { > + HDMI_STATIC_METADATA_TYPE1 = 1, > +}; > + > +enum hdmi_eotf { > + HDMI_EOTF_TRADITIONAL_GAMMA_SDR, > + HDMI_EOTF_TRADITIONAL_GAMMA_HDR, > + HDMI_EOTF_SMPTE_ST2084, > +}; > + > struct hdmi_avi_infoframe { > enum hdmi_infoframe_type type; > unsigned char version; > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h > index a439c2e..5012af2 100644 > --- a/include/uapi/drm/drm_mode.h > +++ b/include/uapi/drm/drm_mode.h > @@ -630,6 +630,22 @@ struct drm_color_lut { > __u16 reserved; > }; > > +/* HDR Metadata */ If this is meant to exactly match the layout in the CTA spec, maybe it's worth a note here saying that, to make it clear it isn't arbitrary. > +struct hdr_static_metadata { > + uint8_t eotf; > + uint8_t metadata_type; > + struct { > + uint16_t x, y; > + } display_primaries[3]; > + struct { > + uint16_t x, y; > + } white_point; > + uint16_t max_mastering_display_luminance; > + uint16_t min_mastering_display_luminance; > + uint16_t max_fall; > + uint16_t max_cll; In my copy of CTA-861, fall and cll are the other way around. cll in bytes 23/24, fall in 25/26. > +}; The types should be __u8 etc in this kernel header. It occurred to me that we're likely to want to support dynamic metadata before too long, and I'm sure there'll be other new HDR metadata types too. So, perhaps we should add a "header" of sorts to the HDR_OUTPUT_METADATA blob, to let it be re-used for stuff which isn't "STATIC_METADATA_TYPE1" in the future. Something like: struct hdr_output_metadata { __u32 metadata_type; union { struct hdr_static_metadata hdmi_type1; }; }; Not sure how DRM feels about unions in ioctl arguments. Thanks, -Brian > + > #define DRM_MODE_PAGE_FLIP_EVENT 0x01 > #define DRM_MODE_PAGE_FLIP_ASYNC 0x02 > #define DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4 > -- > 1.9.1 >
>-----Original Message----- >From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf Of Brian >Starkey >Sent: Thursday, March 21, 2019 5:01 PM >To: Shankar, Uma <uma.shankar@intel.com> >Cc: Syrjala, Ville <ville.syrjala@intel.com>; Liviu Dudau <Liviu.Dudau@arm.com>; >intel-gfx@lists.freedesktop.org; emil.l.velikov@gmail.com; dri- >devel@lists.freedesktop.org; nd <nd@arm.com>; Lankhorst, Maarten ><maarten.lankhorst@intel.com> >Subject: Re: [v6 01/13] drm: Add HDR source metadata property > >Hi Uma, > >On Wed, Mar 20, 2019 at 04:18:14PM +0530, Uma Shankar wrote: >> This patch adds a blob property to get HDR metadata information from >> userspace. This will be send as part of AVI Infoframe to panel. >> >> v2: Rebase and modified the metadata structure elements as per Ville's >> POC changes. >> >> v3: No Change >> >> v4: Addressed Shashank's review comments >> >> v5: Rebase. >> >> Signed-off-by: Uma Shankar <uma.shankar@intel.com> >> --- >> drivers/gpu/drm/drm_connector.c | 6 ++++++ >> include/drm/drm_connector.h | 10 ++++++++++ >> include/drm/drm_mode_config.h | 6 ++++++ >> include/linux/hdmi.h | 10 ++++++++++ >> include/uapi/drm/drm_mode.h | 16 ++++++++++++++++ >> 5 files changed, 48 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_connector.c >> b/drivers/gpu/drm/drm_connector.c index 2355124..0bdae90 100644 >> --- a/drivers/gpu/drm/drm_connector.c >> +++ b/drivers/gpu/drm/drm_connector.c >> @@ -1058,6 +1058,12 @@ int drm_connector_create_standard_properties(struct >drm_device *dev) >> return -ENOMEM; >> dev->mode_config.non_desktop_property = prop; >> >> + prop = drm_property_create(dev, DRM_MODE_PROP_BLOB, >> + "HDR_OUTPUT_METADATA", 0); >> + if (!prop) >> + return -ENOMEM; >> + dev->mode_config.hdr_output_metadata_property = prop; >> + >> return 0; >> } >> >> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h >> index c806199..29388bd 100644 >> --- a/include/drm/drm_connector.h >> +++ b/include/drm/drm_connector.h >> @@ -561,6 +561,13 @@ struct drm_connector_state { >> * and the connector bpc limitations obtained from edid. >> */ >> u8 max_bpc; >> + >> + /** >> + * @metadata_blob_ptr: >> + * DRM blob property for HDR output metadata >> + */ >> + struct drm_property_blob *hdr_output_metadata_blob_ptr; >> + u8 hdr_metadata_changed : 1; >> }; >> >> /** >> @@ -1201,6 +1208,9 @@ struct drm_connector { >> * &drm_mode_config.connector_free_work. >> */ >> struct llist_node free_node; >> + >> + /* HDR metdata */ >> + struct hdr_static_metadata hdr_metadata; >> }; >> >> #define obj_to_connector(x) container_of(x, struct drm_connector, >> base) diff --git a/include/drm/drm_mode_config.h >> b/include/drm/drm_mode_config.h index 7f60e8e..ef2656b 100644 >> --- a/include/drm/drm_mode_config.h >> +++ b/include/drm/drm_mode_config.h >> @@ -836,6 +836,12 @@ struct drm_mode_config { >> */ >> struct drm_property *writeback_out_fence_ptr_property; >> >> + /* >> + * hdr_metadata_property: Connector property containing hdr metatda >> + * This will be provided by userspace compositors based on HDR content >> + */ >> + struct drm_property *hdr_output_metadata_property; >> + >> /* dumb ioctl parameters */ >> uint32_t preferred_depth, prefer_shadow; >> >> diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h index >> 927ad64..a065194 100644 >> --- a/include/linux/hdmi.h >> +++ b/include/linux/hdmi.h >> @@ -152,6 +152,16 @@ enum hdmi_content_type { >> HDMI_CONTENT_TYPE_GAME, >> }; >> >> +enum hdmi_metadata_type { >> + HDMI_STATIC_METADATA_TYPE1 = 1, >> +}; >> + >> +enum hdmi_eotf { >> + HDMI_EOTF_TRADITIONAL_GAMMA_SDR, >> + HDMI_EOTF_TRADITIONAL_GAMMA_HDR, >> + HDMI_EOTF_SMPTE_ST2084, >> +}; >> + >> struct hdmi_avi_infoframe { >> enum hdmi_infoframe_type type; >> unsigned char version; >> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h >> index a439c2e..5012af2 100644 >> --- a/include/uapi/drm/drm_mode.h >> +++ b/include/uapi/drm/drm_mode.h >> @@ -630,6 +630,22 @@ struct drm_color_lut { >> __u16 reserved; >> }; >> >> +/* HDR Metadata */ > >If this is meant to exactly match the layout in the CTA spec, maybe it's worth a note >here saying that, to make it clear it isn't arbitrary. > >> +struct hdr_static_metadata { >> + uint8_t eotf; >> + uint8_t metadata_type; >> + struct { >> + uint16_t x, y; >> + } display_primaries[3]; >> + struct { >> + uint16_t x, y; >> + } white_point; >> + uint16_t max_mastering_display_luminance; >> + uint16_t min_mastering_display_luminance; >> + uint16_t max_fall; >> + uint16_t max_cll; > >In my copy of CTA-861, fall and cll are the other way around. cll in bytes 23/24, fall in >25/26. Sure Brian, will re-order these. >> +}; > >The types should be __u8 etc in this kernel header. Ok will update them. >It occurred to me that we're likely to want to support dynamic metadata before too >long, and I'm sure there'll be other new HDR metadata types too. So, perhaps we >should add a "header" of sorts to the HDR_OUTPUT_METADATA blob, to let it be re- >used for stuff which isn't "STATIC_METADATA_TYPE1" in the future. > >Something like: > >struct hdr_output_metadata { > __u32 metadata_type; > union { > struct hdr_static_metadata hdmi_type1; > }; >}; > >Not sure how DRM feels about unions in ioctl arguments. Sounds good, I will update like this and will try to test this out. Thanks & Regards, Uma Shankar >Thanks, >-Brian > >> + >> #define DRM_MODE_PAGE_FLIP_EVENT 0x01 #define >> DRM_MODE_PAGE_FLIP_ASYNC 0x02 #define >> DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4 >> -- >> 1.9.1 >> >_______________________________________________ >dri-devel mailing list >dri-devel@lists.freedesktop.org >https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 2355124..0bdae90 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -1058,6 +1058,12 @@ int drm_connector_create_standard_properties(struct drm_device *dev) return -ENOMEM; dev->mode_config.non_desktop_property = prop; + prop = drm_property_create(dev, DRM_MODE_PROP_BLOB, + "HDR_OUTPUT_METADATA", 0); + if (!prop) + return -ENOMEM; + dev->mode_config.hdr_output_metadata_property = prop; + return 0; } diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index c806199..29388bd 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -561,6 +561,13 @@ struct drm_connector_state { * and the connector bpc limitations obtained from edid. */ u8 max_bpc; + + /** + * @metadata_blob_ptr: + * DRM blob property for HDR output metadata + */ + struct drm_property_blob *hdr_output_metadata_blob_ptr; + u8 hdr_metadata_changed : 1; }; /** @@ -1201,6 +1208,9 @@ struct drm_connector { * &drm_mode_config.connector_free_work. */ struct llist_node free_node; + + /* HDR metdata */ + struct hdr_static_metadata hdr_metadata; }; #define obj_to_connector(x) container_of(x, struct drm_connector, base) diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h index 7f60e8e..ef2656b 100644 --- a/include/drm/drm_mode_config.h +++ b/include/drm/drm_mode_config.h @@ -836,6 +836,12 @@ struct drm_mode_config { */ struct drm_property *writeback_out_fence_ptr_property; + /* + * hdr_metadata_property: Connector property containing hdr metatda + * This will be provided by userspace compositors based on HDR content + */ + struct drm_property *hdr_output_metadata_property; + /* dumb ioctl parameters */ uint32_t preferred_depth, prefer_shadow; diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h index 927ad64..a065194 100644 --- a/include/linux/hdmi.h +++ b/include/linux/hdmi.h @@ -152,6 +152,16 @@ enum hdmi_content_type { HDMI_CONTENT_TYPE_GAME, }; +enum hdmi_metadata_type { + HDMI_STATIC_METADATA_TYPE1 = 1, +}; + +enum hdmi_eotf { + HDMI_EOTF_TRADITIONAL_GAMMA_SDR, + HDMI_EOTF_TRADITIONAL_GAMMA_HDR, + HDMI_EOTF_SMPTE_ST2084, +}; + struct hdmi_avi_infoframe { enum hdmi_infoframe_type type; unsigned char version; diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index a439c2e..5012af2 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -630,6 +630,22 @@ struct drm_color_lut { __u16 reserved; }; +/* HDR Metadata */ +struct hdr_static_metadata { + uint8_t eotf; + uint8_t metadata_type; + struct { + uint16_t x, y; + } display_primaries[3]; + struct { + uint16_t x, y; + } white_point; + uint16_t max_mastering_display_luminance; + uint16_t min_mastering_display_luminance; + uint16_t max_fall; + uint16_t max_cll; +}; + #define DRM_MODE_PAGE_FLIP_EVENT 0x01 #define DRM_MODE_PAGE_FLIP_ASYNC 0x02 #define DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4
This patch adds a blob property to get HDR metadata information from userspace. This will be send as part of AVI Infoframe to panel. v2: Rebase and modified the metadata structure elements as per Ville's POC changes. v3: No Change v4: Addressed Shashank's review comments v5: Rebase. Signed-off-by: Uma Shankar <uma.shankar@intel.com> --- drivers/gpu/drm/drm_connector.c | 6 ++++++ include/drm/drm_connector.h | 10 ++++++++++ include/drm/drm_mode_config.h | 6 ++++++ include/linux/hdmi.h | 10 ++++++++++ include/uapi/drm/drm_mode.h | 16 ++++++++++++++++ 5 files changed, 48 insertions(+)