diff mbox series

[v2,07/14] drm: Implement HDR source metadata set and get property handling

Message ID 1544560702-16447-8-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

Commit Message

Shankar, Uma Dec. 11, 2018, 8:38 p.m. UTC
HDR source metadata set and get property implemented in this
patch. 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

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/drm_atomic.c      |  2 ++
 drivers/gpu/drm/drm_atomic_uapi.c | 13 +++++++++++++
 2 files changed, 15 insertions(+)

Comments

Sharma, Shashank Dec. 20, 2018, 6:33 p.m. UTC | #1
Regards

Shashank


On 12/12/2018 2:08 AM, Uma Shankar wrote:
> HDR source metadata set and get property implemented in this
> patch.
Again, HDR output metadata ? How about re-arranging the line like "This 
patch implements get() and set() functions for HDR output metadata 
property" just to make it more clear ?
> 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
>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>   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..4e71c6b 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);
Alignment
>   
>   	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 c408898..b721b12 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 = &dev->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_source_metadata_property) {
> +		ret = drm_atomic_replace_property_blob_from_id(dev,
> +				&state->hdr_source_metadata_blob_ptr,
> +				val,
> +				-1, sizeof(struct hdr_static_metadata),
> +				&replaced);
Alignment, but I know it might be difficult to contain the params within 
80 chars.
> +		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) {
> @@ -816,6 +826,9 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
>   		*val = state->content_type;
>   	} else if (property == connector->scaling_mode_property) {
>   		*val = state->scaling_mode;
> +	} else if (property == config->hdr_source_metadata_property) {
> +		*val = (state->hdr_source_metadata_blob_ptr) ?
> +			state->hdr_source_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) {
- Shashank
Shankar, Uma Jan. 8, 2019, 6:48 a.m. UTC | #2
>-----Original Message-----
>From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf Of
>Sharma, Shashank
>Sent: Friday, December 21, 2018 12:04 AM
>To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org;
>dri-devel@lists.freedesktop.org
>Cc: Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
><maarten.lankhorst@intel.com>
>Subject: Re: [v2 07/14] drm: Implement HDR source metadata set and get
>property handling
>
>Regards
>
>Shashank
>
>
>On 12/12/2018 2:08 AM, Uma Shankar wrote:
>> HDR source metadata set and get property implemented in this patch.
>Again, HDR output metadata ? How about re-arranging the line like "This patch
>implements get() and set() functions for HDR output metadata property" just to
>make it more clear ?

Sure, will update this.

>> 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
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>>   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..4e71c6b 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);
>Alignment

Ok, will update.

>>
>>   	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 c408898..b721b12 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 = &dev->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_source_metadata_property) {
>> +		ret = drm_atomic_replace_property_blob_from_id(dev,
>> +				&state->hdr_source_metadata_blob_ptr,
>> +				val,
>> +				-1, sizeof(struct hdr_static_metadata),
>> +				&replaced);
>Alignment, but I know it might be difficult to contain the params within
>80 chars.

Yeah, not sure what is higher priority - 80 character limit or the
alignment here. Currently went with 80 character limit.

Regards,
Uma Shankar

>> +		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) { @@ -816,6
>> +826,9 @@ static int drm_atomic_connector_set_property(struct
>drm_connector *connector,
>>   		*val = state->content_type;
>>   	} else if (property == connector->scaling_mode_property) {
>>   		*val = state->scaling_mode;
>> +	} else if (property == config->hdr_source_metadata_property) {
>> +		*val = (state->hdr_source_metadata_blob_ptr) ?
>> +			state->hdr_source_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) {
>- Shashank
>_______________________________________________
>dri-devel mailing list
>dri-devel@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 5eb4013..4e71c6b 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 c408898..b721b12 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 = &dev->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_source_metadata_property) {
+		ret = drm_atomic_replace_property_blob_from_id(dev,
+				&state->hdr_source_metadata_blob_ptr,
+				val,
+				-1, sizeof(struct hdr_static_metadata),
+				&replaced);
+		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) {
@@ -816,6 +826,9 @@  static int drm_atomic_connector_set_property(struct drm_connector *connector,
 		*val = state->content_type;
 	} else if (property == connector->scaling_mode_property) {
 		*val = state->scaling_mode;
+	} else if (property == config->hdr_source_metadata_property) {
+		*val = (state->hdr_source_metadata_blob_ptr) ?
+			state->hdr_source_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) {