>-----Original Message-----
>From: dri-devel [mailto:dri-devel-boun...@lists.freedesktop.org] On Behalf Of
>Maarten Lankhorst
>Sent: Wednesday, January 30, 2019 3:27 PM
>To: Shankar, Uma <uma.shan...@intel.com>; intel-...@lists.freedesktop.org;
>dri-devel@lists.freedesktop.org
>Cc: emil.l.veli...@gmail.com; Syrjala, Ville <ville.syrj...@intel.com>;
>Lankhorst,
>Maarten <maarten.lankho...@intel.com>
>Subject: Re: [v9 3/3] drm/i915: Attach colorspace property and enable modeset
>
>Op 29-01-2019 om 19:50 schreef Uma Shankar:
>> This patch attaches the colorspace connector property to the hdmi
>> connector. Based on colorspace change, modeset will be triggered to
>> switch to new colorspace.
>>
>> Based on colorspace property value create an infoframe with
>> appropriate colorspace. This can be used to send an infoframe packet
>> with proper colorspace value set which will help to enable wider color
>> gamut like BT2020 on sink.
>>
>> This patch attaches and enables HDMI colorspace, DP will be taken care
>> separately.
>>
>> v2: Merged the changes of creating infoframe as well to this patch as
>> per Maarten's suggestion.
>>
>> v3: Addressed review comments from Shashank. Separated HDMI and DP
>> colorspaces as suggested by Ville and Maarten.
>>
>> v4: Addressed Chris and Ville's review comments, and created a common
>> colorspace property for DP and HDMI, filtered the list based on the
>> colorspaces supported by the respective protocol standard. Handle the
>> default case properly.
>>
>> v5: Merged the DP handling along with platform colorspace handling as
>> per Shashank's comments.
>>
>> v6: Addressed Maarten's review comment and limited this currently to
>> non lspcon based devices.
>>
>> v7: Reverted to old design of exposing all colorspaces to userspace as
>> per Ville's review comment
>>
>> Signed-off-by: Uma Shankar <uma.shan...@intel.com>
>> ---
>> drivers/gpu/drm/i915/intel_atomic.c | 1 +
>> drivers/gpu/drm/i915/intel_connector.c | 8 ++++++++
>> drivers/gpu/drm/i915/intel_drv.h | 1 +
>> drivers/gpu/drm/i915/intel_hdmi.c | 25 +++++++++++++++++++++++++
>> 4 files changed, 35 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_atomic.c
>> b/drivers/gpu/drm/i915/intel_atomic.c
>> index 16263ad..76b7114 100644
>> --- a/drivers/gpu/drm/i915/intel_atomic.c
>> +++ b/drivers/gpu/drm/i915/intel_atomic.c
>> @@ -124,6 +124,7 @@ int intel_digital_connector_atomic_check(struct
>drm_connector *conn,
>> */
>> if (new_conn_state->force_audio != old_conn_state->force_audio ||
>> new_conn_state->broadcast_rgb != old_conn_state->broadcast_rgb
>> ||
>> + new_state->colorspace != old_state->colorspace ||
>
>I only care if there's a new version, but can you use xxx_conn_state->base ?
>
>Can fixup if patch is ready otherwise.
Sure, I can fix this. Will send out the next version along with your RB and
Jani's ack.
Thanks for the review.
Regards,
Uma Shankar
>
>Otherwise series looks good, so
>
>Reviewed-by: Maarten Lankhorst <maarten.lankho...@linux.intel.com> for the
>whole series. :)
>
>> 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)
>> diff --git a/drivers/gpu/drm/i915/intel_connector.c
>> b/drivers/gpu/drm/i915/intel_connector.c
>> index ee16758..ac2aed7 100644
>> --- a/drivers/gpu/drm/i915/intel_connector.c
>> +++ b/drivers/gpu/drm/i915/intel_connector.c
>> @@ -265,3 +265,11 @@ int intel_ddc_get_modes(struct drm_connector
>*connector,
>> connector->dev->mode_config.aspect_ratio_property,
>> DRM_MODE_PICTURE_ASPECT_NONE);
>> }
>> +
>> +void
>> +intel_attach_colorspace_property(struct drm_connector *connector) {
>> + if (!drm_mode_create_colorspace_property(connector))
>> + drm_object_attach_property(&connector->base,
>> + connector->colorspace_property, 0); }
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h
>> b/drivers/gpu/drm/i915/intel_drv.h
>> index 85b913e..5178a9a 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -1783,6 +1783,7 @@ int intel_connector_update_modes(struct
>> drm_connector *connector, void
>> intel_attach_force_audio_property(struct drm_connector *connector);
>> void intel_attach_broadcast_rgb_property(struct drm_connector
>> *connector); void intel_attach_aspect_ratio_property(struct
>> drm_connector *connector);
>> +void intel_attach_colorspace_property(struct drm_connector
>> +*connector);
>>
>> /* intel_csr.c */
>> void intel_csr_ucode_init(struct drm_i915_private *); diff --git
>> a/drivers/gpu/drm/i915/intel_hdmi.c
>> b/drivers/gpu/drm/i915/intel_hdmi.c
>> index 97a98e1..5c5009d 100644
>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>> @@ -498,6 +498,24 @@ static void intel_hdmi_set_avi_infoframe(struct
>intel_encoder *encoder,
>> else
>> frame.avi.colorspace = HDMI_COLORSPACE_RGB;
>>
>> + if (conn_state->colorspace == DRM_MODE_COLORIMETRY_DEFAULT) {
>> + /* Set ITU 709 as default for HDMI */
>> + frame.avi.colorimetry = DRM_MODE_COLORIMETRY_ITU_709;
>> + } else if (conn_state->colorspace <
>DRM_MODE_COLORIMETRY_XV_YCC_601) {
>> + frame.avi.colorimetry = conn_state->colorspace;
>> + } else {
>> + frame.avi.colorimetry = HDMI_COLORIMETRY_EXTENDED;
>> + /*
>> + * Starting from extended list where COLORIMETRY_XV_YCC_601
>> + * is the first extended mode and its value is 0 as per HDMI
>> + * specification.
>> + * TODO: This needs to be extended for LSPCON implementation
>> + * as well. Will be implemented separately.
>> + */
>> + frame.avi.extended_colorimetry = conn_state->colorspace -
>> + DRM_MODE_COLORIMETRY_XV_YCC_601;
>> + }
>> +
>> drm_hdmi_avi_infoframe_quant_range(&frame.avi,
>> conn_state->connector,
>> adjusted_mode,
>> @@ -2143,10 +2161,17 @@ static void intel_hdmi_destroy(struct
>> drm_connector *connector) intel_hdmi_add_properties(struct intel_hdmi
>> *intel_hdmi, struct drm_connector *connector) {
>> struct drm_i915_private *dev_priv = to_i915(connector->dev);
>> + struct intel_digital_port *intel_dig_port =
>> + hdmi_to_dig_port(intel_hdmi);
>>
>> intel_attach_force_audio_property(connector);
>> intel_attach_broadcast_rgb_property(connector);
>> intel_attach_aspect_ratio_property(connector);
>> +
>> + /* Attach Colorspace property for Non LSPCON based device */
>> + if (!intel_dig_port->lspcon.active)
>> + intel_attach_colorspace_property(connector);
>> +
>> drm_connector_attach_content_type_property(connector);
>> connector->state->picture_aspect_ratio =
>HDMI_PICTURE_ASPECT_NONE;
>>
>
>
>_______________________________________________
>dri-devel mailing list
>dri-devel@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
