| Message ID | 1553078906-5991-6-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, 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 <uma.shankar@intel.com> 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 = &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_output_metadata_property) { > + ret = drm_atomic_replace_property_blob_from_id(dev, > + &state->hdr_output_metadata_blob_ptr, > + val, > + -1, sizeof(struct hdr_static_metadata), > + &replaced); > + 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 >
>-----Original Message----- >From: Brian Starkey [mailto:Brian.Starkey@arm.com] >Sent: Thursday, March 21, 2019 5:17 PM >To: Shankar, Uma <uma.shankar@intel.com> >Cc: intel-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Lankhorst, >Maarten <maarten.lankhorst@intel.com>; Syrjala, Ville <ville.syrjala@intel.com>; >Sharma, Shashank <shashank.sharma@intel.com>; emil.l.velikov@gmail.com; Liviu >Dudau <Liviu.Dudau@arm.com>; nd <nd@arm.com> >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 <uma.shankar@intel.com> > >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 = &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_output_metadata_property) { >> + ret = drm_atomic_replace_property_blob_from_id(dev, >> + &state->hdr_output_metadata_blob_ptr, >> + val, >> + -1, sizeof(struct hdr_static_metadata), >> + &replaced); >> + 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 >>
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 = &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_output_metadata_property) { + ret = drm_atomic_replace_property_blob_from_id(dev, + &state->hdr_output_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) { @@ -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) {
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 <uma.shankar@intel.com> --- drivers/gpu/drm/drm_atomic.c | 2 ++ drivers/gpu/drm/drm_atomic_uapi.c | 13 +++++++++++++ 2 files changed, 15 insertions(+)