RE: [v6 05/13] drm: Implement HDR output metadata set and get property handling
>-Original Message-
>From: Brian Starkey [mailto:brian.star...@arm.com]
>Sent: Thursday, March 21, 2019 5:17 PM
>To: Shankar, Uma
>Cc: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
>Lankhorst,
>Maarten ; Syrjala, Ville
>;
>Sharma, Shashank ; emil.l.veli...@gmail.com; Liviu
>Dudau ; nd
>Subject: Re: [v6 05/13] drm: Implement HDR output metadata set and get property
>handling
>
>Hi,
>
>On Wed, Mar 20, 2019 at 04:18:18PM +0530, Uma Shankar wrote:
>> This patch 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 added Ville's POC changes
>>
>> v3: No Change
>>
>> v4: Addressed Shashank's review comments
>>
>> Signed-off-by: Uma Shankar
>
>This looks like a candidate to be squashed into patch 1.
Ok, will merged it with 1.
>> ---
>> drivers/gpu/drm/drm_atomic.c | 2 ++
>> drivers/gpu/drm/drm_atomic_uapi.c | 13 +
>> 2 files changed, 15 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 4eb81f1..18c8b81f 100644
>> --- a/drivers/gpu/drm/drm_atomic_uapi.c
>> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
>> @@ -686,6 +686,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); @@ -734,6
>> +736,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_static_metadata),
>> +);
>> +state->hdr_metadata_changed |= replaced;
>
>I've said the same about other flags: Do we really need them?
>It seems easy enough to just compare the blob pointers or their IDs
This was just an extension of how gamma luts are used. It's just to have this
check
here and later use it where ever needed, since we may require it at multiple
places for
certain checks.
Regards,
Uma Shankar
>
>Thanks,
>-Brian
>
>> +return ret;
>> } else if (property == config->aspect_ratio_property) {
>> state->picture_aspect_ratio = val;
>> } else if (property == config->content_type_property) { @@ -820,6
>> +830,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) {
>> --
>> 1.9.1
>>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [v6 05/13] drm: Implement HDR output metadata set and get property handling
Hi,
On Wed, Mar 20, 2019 at 04:18:18PM +0530, Uma Shankar wrote:
> This patch 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 added Ville's POC changes
>
> v3: No Change
>
> v4: Addressed Shashank's review comments
>
> Signed-off-by: Uma Shankar
This looks like a candidate to be squashed into patch 1.
> ---
> drivers/gpu/drm/drm_atomic.c | 2 ++
> drivers/gpu/drm/drm_atomic_uapi.c | 13 +
> 2 files changed, 15 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 4eb81f1..18c8b81f 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -686,6 +686,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);
> @@ -734,6 +736,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_static_metadata),
> + );
> + state->hdr_metadata_changed |= replaced;
I've said the same about other flags: Do we really need them?
It seems easy enough to just compare the blob pointers or their IDs
Thanks,
-Brian
> + return ret;
> } else if (property == config->aspect_ratio_property) {
> state->picture_aspect_ratio = val;
> } else if (property == config->content_type_property) {
> @@ -820,6 +830,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) {
> --
> 1.9.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[v6 05/13] drm: Implement HDR output metadata set and get property handling
This patch 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 added Ville's POC changes
v3: No Change
v4: Addressed Shashank's review comments
Signed-off-by: Uma Shankar
---
drivers/gpu/drm/drm_atomic.c | 2 ++
drivers/gpu/drm/drm_atomic_uapi.c | 13 +
2 files changed, 15 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 4eb81f1..18c8b81f 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -686,6 +686,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);
@@ -734,6 +736,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_static_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) {
@@ -820,6 +830,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) {
--
1.9.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
