RE: [v9 3/3] drm/i915: Attach colorspace property and enable modeset

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




>-----Original Message-----
>From: dri-devel [mailto:dri-devel-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf Of
>Maarten Lankhorst
>Sent: Wednesday, January 30, 2019 3:27 PM
>To: Shankar, Uma <uma.shankar@xxxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx;
>dri-devel@xxxxxxxxxxxxxxxxxxxxx
>Cc: emil.l.velikov@xxxxxxxxx; Syrjala, Ville <ville.syrjala@xxxxxxxxx>; Lankhorst,
>Maarten <maarten.lankhorst@xxxxxxxxx>
>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.shankar@xxxxxxxxx>
>> ---
>>  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.lankhorst@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxx
>https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux