>-----Original Message----- >From: dri-devel [mailto:dri-devel-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf Of Brian >Starkey >Sent: Thursday, March 21, 2019 5:01 PM >To: Shankar, Uma <uma.shankar@xxxxxxxxx> >Cc: Syrjala, Ville <ville.syrjala@xxxxxxxxx>; Liviu Dudau <Liviu.Dudau@xxxxxxx>; >intel-gfx@xxxxxxxxxxxxxxxxxxxxx; emil.l.velikov@xxxxxxxxx; dri- >devel@xxxxxxxxxxxxxxxxxxxxx; nd <nd@xxxxxxx>; Lankhorst, Maarten ><maarten.lankhorst@xxxxxxxxx> >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@xxxxxxxxx> >> --- >> 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@xxxxxxxxxxxxxxxxxxxxx >https://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
