| Message ID | 1554828284-17776-9-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 |
On Tue, Apr 09, 2019 at 10:14:42PM +0530, Uma Shankar wrote: > This patch enables modeset whenever HDR metadata > needs to be updated to sink. > > v2: Addressed Shashank's review comments. > > v3: Added Shashank's RB. > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > Reviewed-by: Shashank Sharma <shashank.sharma@intel.com> > --- > drivers/gpu/drm/i915/intel_atomic.c | 14 +++++++++++++- > drivers/gpu/drm/i915/intel_hdmi.c | 26 ++++++++++++++++++++++++++ > 2 files changed, 39 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c > index 8c8fae3..e8b5f84 100644 > --- a/drivers/gpu/drm/i915/intel_atomic.c > +++ b/drivers/gpu/drm/i915/intel_atomic.c > @@ -104,6 +104,16 @@ int intel_digital_connector_atomic_set_property(struct drm_connector *connector, > return -EINVAL; > } > > +static bool blob_equal(const struct drm_property_blob *a, > + const struct drm_property_blob *b) > +{ > + if (a && b) > + return a->length == b->length && > + !memcmp(a->data, b->data, a->length); > + > + return !a == !b; > +} I have a feeling the memcmp() is overkill. We could just check for whether the blob is the same or not. If userspace is an idiot and creates a new blob with identical content so be it. > + > int intel_digital_connector_atomic_check(struct drm_connector *conn, > struct drm_connector_state *new_state) > { > @@ -131,7 +141,9 @@ int intel_digital_connector_atomic_check(struct drm_connector *conn, > new_conn_state->base.colorspace != old_conn_state->base.colorspace || > new_conn_state->base.picture_aspect_ratio != old_conn_state->base.picture_aspect_ratio || > new_conn_state->base.content_type != old_conn_state->base.content_type || > - new_conn_state->base.scaling_mode != old_conn_state->base.scaling_mode) > + new_conn_state->base.scaling_mode != old_conn_state->base.scaling_mode || > + !blob_equal(new_conn_state->base.hdr_output_metadata_blob_ptr, > + old_conn_state->base.hdr_output_metadata_blob_ptr)) > crtc_state->mode_changed = true; > > return 0; > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c > index 0ecfda0..85333a7 100644 > --- a/drivers/gpu/drm/i915/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > @@ -799,6 +799,20 @@ void intel_read_infoframe(struct intel_encoder *encoder, > return true; > } > > +static bool is_eotf_supported(u8 output_eotf, u8 sink_eotf) > +{ > + if (output_eotf == 0) > + return (sink_eotf & (1 << 0)); > + if (output_eotf == 1) > + return (sink_eotf & (1 << 1)); > + if (output_eotf == 2) > + return (sink_eotf & (1 << 2)); > + if (output_eotf == 3) > + return (sink_eotf & (1 << 3)); > + > + return false; return sink_eotf & BIT(output_eotf); > +} > + > static bool > intel_hdmi_compute_drm_infoframe(struct intel_encoder *encoder, > struct intel_crtc_state *crtc_state, > @@ -806,11 +820,23 @@ void intel_read_infoframe(struct intel_encoder *encoder, > { > struct hdmi_drm_infoframe *frame = &crtc_state->infoframes.drm.drm; > struct hdr_output_metadata *hdr_metadata; > + struct drm_connector *connector = conn_state->connector; > int ret; > > + if (!conn_state->hdr_output_metadata_blob_ptr || > + conn_state->hdr_output_metadata_blob_ptr->length == 0) > + return true; > + > hdr_metadata = (struct hdr_output_metadata *) > conn_state->hdr_output_metadata_blob_ptr->data; > > + /* Sink EOTF is Bit map while infoframe is absolute values */ > + if (!is_eotf_supported(hdr_metadata->hdmi_metadata_type1.eotf, > + connector->hdr_sink_metadata.hdmi_type1.eotf)) { > + DRM_ERROR("EOTF Not Supported\n"); > + return true; > + } > + > ret = drm_hdmi_infoframe_set_hdr_metadata(frame, hdr_metadata); > if (ret < 0) { > DRM_ERROR("couldn't set HDR metadata in infoframe\n"); > -- > 1.9.1
>-----Original Message----- >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] >Sent: Tuesday, May 7, 2019 5:32 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; >brian.starkey@arm.com; dcastagna@chromium.org; seanpaul@chromium.org; >Roper, Matthew D <matthew.d.roper@intel.com>; jonas@kwiboo.se >Subject: Re: [v8 08/10] drm/i915:Enabled Modeset when HDR Infoframe changes > >On Tue, Apr 09, 2019 at 10:14:42PM +0530, Uma Shankar wrote: >> This patch enables modeset whenever HDR metadata needs to be updated >> to sink. >> >> v2: Addressed Shashank's review comments. >> >> v3: Added Shashank's RB. >> >> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> >> Signed-off-by: Uma Shankar <uma.shankar@intel.com> >> Reviewed-by: Shashank Sharma <shashank.sharma@intel.com> >> --- >> drivers/gpu/drm/i915/intel_atomic.c | 14 +++++++++++++- >> drivers/gpu/drm/i915/intel_hdmi.c | 26 ++++++++++++++++++++++++++ >> 2 files changed, 39 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_atomic.c >> b/drivers/gpu/drm/i915/intel_atomic.c >> index 8c8fae3..e8b5f84 100644 >> --- a/drivers/gpu/drm/i915/intel_atomic.c >> +++ b/drivers/gpu/drm/i915/intel_atomic.c >> @@ -104,6 +104,16 @@ int intel_digital_connector_atomic_set_property(struct >drm_connector *connector, >> return -EINVAL; >> } >> >> +static bool blob_equal(const struct drm_property_blob *a, >> + const struct drm_property_blob *b) { >> + if (a && b) >> + return a->length == b->length && >> + !memcmp(a->data, b->data, a->length); >> + >> + return !a == !b; >> +} > >I have a feeling the memcmp() is overkill. We could just check for whether the blob is >the same or not. If userspace is an idiot and creates a new blob with identical content >so be it. I feel it's good to have such check in kernel and avoid extra infoframe programming. More like how we suppress modeset request if same mode is requested as part of modeset. Hope its ok. >> + >> int intel_digital_connector_atomic_check(struct drm_connector *conn, >> struct drm_connector_state *new_state) { >@@ -131,7 +141,9 @@ >> int intel_digital_connector_atomic_check(struct drm_connector *conn, >> new_conn_state->base.colorspace != old_conn_state->base.colorspace || >> new_conn_state->base.picture_aspect_ratio != old_conn_state- >>base.picture_aspect_ratio || >> new_conn_state->base.content_type != old_conn_state- >>base.content_type || >> - new_conn_state->base.scaling_mode != old_conn_state- >>base.scaling_mode) >> + new_conn_state->base.scaling_mode != old_conn_state- >>base.scaling_mode || >> + !blob_equal(new_conn_state->base.hdr_output_metadata_blob_ptr, >> + old_conn_state->base.hdr_output_metadata_blob_ptr)) >> crtc_state->mode_changed = true; >> >> return 0; >> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c >> b/drivers/gpu/drm/i915/intel_hdmi.c >> index 0ecfda0..85333a7 100644 >> --- a/drivers/gpu/drm/i915/intel_hdmi.c >> +++ b/drivers/gpu/drm/i915/intel_hdmi.c >> @@ -799,6 +799,20 @@ void intel_read_infoframe(struct intel_encoder *encoder, >> return true; >> } >> >> +static bool is_eotf_supported(u8 output_eotf, u8 sink_eotf) { >> + if (output_eotf == 0) >> + return (sink_eotf & (1 << 0)); >> + if (output_eotf == 1) >> + return (sink_eotf & (1 << 1)); >> + if (output_eotf == 2) >> + return (sink_eotf & (1 << 2)); >> + if (output_eotf == 3) >> + return (sink_eotf & (1 << 3)); >> + >> + return false; > >return sink_eotf & BIT(output_eotf); Nice suggestion. Will update this. Regards, Uma Shankar >> +} >> + >> static bool >> intel_hdmi_compute_drm_infoframe(struct intel_encoder *encoder, >> struct intel_crtc_state *crtc_state, @@ -806,11 >+820,23 @@ void >> intel_read_infoframe(struct intel_encoder *encoder, { >> struct hdmi_drm_infoframe *frame = &crtc_state->infoframes.drm.drm; >> struct hdr_output_metadata *hdr_metadata; >> + struct drm_connector *connector = conn_state->connector; >> int ret; >> >> + if (!conn_state->hdr_output_metadata_blob_ptr || >> + conn_state->hdr_output_metadata_blob_ptr->length == 0) >> + return true; >> + >> hdr_metadata = (struct hdr_output_metadata *) >> conn_state->hdr_output_metadata_blob_ptr->data; >> >> + /* Sink EOTF is Bit map while infoframe is absolute values */ >> + if (!is_eotf_supported(hdr_metadata->hdmi_metadata_type1.eotf, >> + connector->hdr_sink_metadata.hdmi_type1.eotf)) { >> + DRM_ERROR("EOTF Not Supported\n"); >> + return true; >> + } >> + >> ret = drm_hdmi_infoframe_set_hdr_metadata(frame, hdr_metadata); >> if (ret < 0) { >> DRM_ERROR("couldn't set HDR metadata in infoframe\n"); >> -- >> 1.9.1 > >-- >Ville Syrjälä >Intel
diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c index 8c8fae3..e8b5f84 100644 --- a/drivers/gpu/drm/i915/intel_atomic.c +++ b/drivers/gpu/drm/i915/intel_atomic.c @@ -104,6 +104,16 @@ int intel_digital_connector_atomic_set_property(struct drm_connector *connector, return -EINVAL; } +static bool blob_equal(const struct drm_property_blob *a, + const struct drm_property_blob *b) +{ + if (a && b) + return a->length == b->length && + !memcmp(a->data, b->data, a->length); + + return !a == !b; +} + int intel_digital_connector_atomic_check(struct drm_connector *conn, struct drm_connector_state *new_state) { @@ -131,7 +141,9 @@ int intel_digital_connector_atomic_check(struct drm_connector *conn, new_conn_state->base.colorspace != old_conn_state->base.colorspace || new_conn_state->base.picture_aspect_ratio != old_conn_state->base.picture_aspect_ratio || new_conn_state->base.content_type != old_conn_state->base.content_type || - new_conn_state->base.scaling_mode != old_conn_state->base.scaling_mode) + new_conn_state->base.scaling_mode != old_conn_state->base.scaling_mode || + !blob_equal(new_conn_state->base.hdr_output_metadata_blob_ptr, + old_conn_state->base.hdr_output_metadata_blob_ptr)) crtc_state->mode_changed = true; return 0; diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 0ecfda0..85333a7 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -799,6 +799,20 @@ void intel_read_infoframe(struct intel_encoder *encoder, return true; } +static bool is_eotf_supported(u8 output_eotf, u8 sink_eotf) +{ + if (output_eotf == 0) + return (sink_eotf & (1 << 0)); + if (output_eotf == 1) + return (sink_eotf & (1 << 1)); + if (output_eotf == 2) + return (sink_eotf & (1 << 2)); + if (output_eotf == 3) + return (sink_eotf & (1 << 3)); + + return false; +} + static bool intel_hdmi_compute_drm_infoframe(struct intel_encoder *encoder, struct intel_crtc_state *crtc_state, @@ -806,11 +820,23 @@ void intel_read_infoframe(struct intel_encoder *encoder, { struct hdmi_drm_infoframe *frame = &crtc_state->infoframes.drm.drm; struct hdr_output_metadata *hdr_metadata; + struct drm_connector *connector = conn_state->connector; int ret; + if (!conn_state->hdr_output_metadata_blob_ptr || + conn_state->hdr_output_metadata_blob_ptr->length == 0) + return true; + hdr_metadata = (struct hdr_output_metadata *) conn_state->hdr_output_metadata_blob_ptr->data; + /* Sink EOTF is Bit map while infoframe is absolute values */ + if (!is_eotf_supported(hdr_metadata->hdmi_metadata_type1.eotf, + connector->hdr_sink_metadata.hdmi_type1.eotf)) { + DRM_ERROR("EOTF Not Supported\n"); + return true; + } + ret = drm_hdmi_infoframe_set_hdr_metadata(frame, hdr_metadata); if (ret < 0) { DRM_ERROR("couldn't set HDR metadata in infoframe\n");