RE: [v7 1/9] drm: Add HDR source metadata property
>-Original Message-
>From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf Of
>Jonas
>Karlman
>Sent: Monday, April 8, 2019 3:51 PM
>To: Shankar, Uma ; intel-...@lists.freedesktop.org; dri-
>de...@lists.freedesktop.org
>Cc: seanp...@chromium.org; emil.l.veli...@gmail.com; dcasta...@chromium.org;
>Lankhorst, Maarten ; Syrjala, Ville
>
>Subject: Re: [v7 1/9] drm: Add HDR source metadata property
>
>On 2019-04-02 22:20, 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.
>>
>> It also implements get() and set() functions for HDR output metadata
>> property.The blob data is received from userspace and saved in
>> connector state, the same is returned as blob in get property call to
>> userspace.
>>
>> 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.
>>
>> v6: Addressed Brian Starkey's review comments, defined new structure
>> with header for dynamic metadata scalability.
>> Merge get/set property functions for metadata in this patch.
>>
>> Signed-off-by: Uma Shankar
>> ---
>> drivers/gpu/drm/drm_atomic.c | 2 ++
>> drivers/gpu/drm/drm_atomic_uapi.c | 13 +
>> 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 | 22 ++
>> 7 files changed, 69 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic.c
>> b/drivers/gpu/drm/drm_atomic.c index 5eb4013..8b9c126 100644
>> --- a/drivers/gpu/drm/drm_atomic.c
>> +++ b/drivers/gpu/drm/drm_atomic.c
>> @@ -881,6 +881,8 @@ static void
>> drm_atomic_connector_print_state(struct drm_printer *p,
>>
>> drm_printf(p, "connector[%u]: %s\n", connector->base.id, connector-
>>name);
>> drm_printf(p, "\tcrtc=%s\n", state->crtc ? state->crtc->name :
>> "(null)");
>> +drm_printf(p, "\thdr_metadata_changed=%d\n",
>> + state->hdr_metadata_changed);
>>
>> if (connector->connector_type == DRM_MODE_CONNECTOR_WRITEBACK)
>> if (state->writeback_job && state->writeback_job->fb) diff --git
>> a/drivers/gpu/drm/drm_atomic_uapi.c
>> b/drivers/gpu/drm/drm_atomic_uapi.c
>> index ea797d4..6d0d161 100644
>> --- a/drivers/gpu/drm/drm_atomic_uapi.c
>> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
>> @@ -673,6 +673,8 @@ static int
>> drm_atomic_connector_set_property(struct drm_connector *connector, {
>> struct drm_device *dev = connector->dev;
>> struct drm_mode_config *config = >mode_config;
>> +bool replaced = false;
>> +int ret;
>>
>> if (property == config->prop_crtc_id) {
>> struct drm_crtc *crtc = drm_crtc_find(dev, NULL, val); @@ -721,6
>> +723,14 @@ static int drm_atomic_connector_set_property(struct drm_connector
>*connector,
>> */
>> if (state->link_status != DRM_LINK_STATUS_GOOD)
>> state->link_status = val;
>> +} else if (property == config->hdr_output_metadata_property) {
>> +ret = drm_atomic_replace_property_blob_from_id(dev,
>> +>hdr_output_metadata_blob_ptr,
>> +val,
>> +-1, sizeof(struct hdr_output_metadata),
>> +);
>> +state->hdr_metadata_changed |= replaced;
>> +return ret;
>> } else if (property == config->aspect_ratio_property) {
>> state->picture_aspect_ratio = val;
>> } else if (property == config->content_type_property) { @@ -807,6
>> +817,9 @@ static int drm_atomic_connector_set_property(struct drm_connector
>*connector,
>> *val = state->colorspace;
>> } else if (property == connector->scaling_mode_property) {
>> *val = state->scaling_mode;
>> +} else if (property == config->hdr_output_metadata_property) {
>> +*val = (state->hdr_output_metadata_blob_ptr) ?
>> +state->hdr_output_metadata_blob_ptr->base.id : 0;
>> } else if (property == connector->conten
Re: [v7 1/9] drm: Add HDR source metadata property
On 2019-04-02 22:20, 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.
>
> It also implements get() and set() functions for HDR output
> metadata property.The blob data is received from userspace and
> saved in connector state, the same is returned as blob in get
> property call to userspace.
>
> 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.
>
> v6: Addressed Brian Starkey's review comments, defined
> new structure with header for dynamic metadata scalability.
> Merge get/set property functions for metadata in this patch.
>
> Signed-off-by: Uma Shankar
> ---
> drivers/gpu/drm/drm_atomic.c | 2 ++
> drivers/gpu/drm/drm_atomic_uapi.c | 13 +
> 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 | 22 ++
> 7 files changed, 69 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 5eb4013..8b9c126 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -881,6 +881,8 @@ static void drm_atomic_connector_print_state(struct
> drm_printer *p,
>
> drm_printf(p, "connector[%u]: %s\n", connector->base.id,
> connector->name);
> drm_printf(p, "\tcrtc=%s\n", state->crtc ? state->crtc->name :
> "(null)");
> + drm_printf(p, "\thdr_metadata_changed=%d\n",
> +state->hdr_metadata_changed);
>
> if (connector->connector_type == DRM_MODE_CONNECTOR_WRITEBACK)
> if (state->writeback_job && state->writeback_job->fb)
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c
> b/drivers/gpu/drm/drm_atomic_uapi.c
> index ea797d4..6d0d161 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -673,6 +673,8 @@ static int drm_atomic_connector_set_property(struct
> drm_connector *connector,
> {
> struct drm_device *dev = connector->dev;
> struct drm_mode_config *config = >mode_config;
> + bool replaced = false;
> + int ret;
>
> if (property == config->prop_crtc_id) {
> struct drm_crtc *crtc = drm_crtc_find(dev, NULL, val);
> @@ -721,6 +723,14 @@ static int drm_atomic_connector_set_property(struct
> drm_connector *connector,
>*/
> if (state->link_status != DRM_LINK_STATUS_GOOD)
> state->link_status = val;
> + } else if (property == config->hdr_output_metadata_property) {
> + ret = drm_atomic_replace_property_blob_from_id(dev,
> + >hdr_output_metadata_blob_ptr,
> + val,
> + -1, sizeof(struct hdr_output_metadata),
> + );
> + state->hdr_metadata_changed |= replaced;
> + return ret;
> } else if (property == config->aspect_ratio_property) {
> state->picture_aspect_ratio = val;
> } else if (property == config->content_type_property) {
> @@ -807,6 +817,9 @@ static int drm_atomic_connector_set_property(struct
> drm_connector *connector,
> *val = state->colorspace;
> } else if (property == connector->scaling_mode_property) {
> *val = state->scaling_mode;
> + } else if (property == config->hdr_output_metadata_property) {
> + *val = (state->hdr_output_metadata_blob_ptr) ?
> + state->hdr_output_metadata_blob_ptr->base.id : 0;
> } else if (property == connector->content_protection_property) {
> *val = state->content_protection;
> } else if (property == config->writeback_fb_id_property) {
> 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 02a1312..9e3a39c 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -599,6 +599,13 @@ struct drm_connector_state {
>* and the connector bpc limitations obtained from edid.
>*/
> u8 max_bpc;
> +
> + /**
> + *
Re: [v7 1/9] drm: Add HDR source metadata property
Hi Uma.
Some kerneldoc nits below.
Sam
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -599,6 +599,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;
The kerneldoc comment uses the wrong name.
> + u8 hdr_metadata_changed : 1;
kerneldoc is missing.
> };
>
> /**
> @@ -1239,6 +1246,9 @@ struct drm_connector {
>* _mode_config.connector_free_work.
>*/
> struct llist_node free_node;
> +
> + /* HDR metdata */
> + struct hdr_output_metadata hdr_metadata;
Please format as kerneldoc.
> };
>
> #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;
> +
kerneldoc comment uses wrong name
> /* dumb ioctl parameters */
> uint32_t preferred_depth, prefer_shadow;
Please format as kerneldoc.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
