diff mbox series

[v4,15/16] drm/i915: Let PCON convert from RGB to YUV if it can

Message ID 20201208075145.17389-16-ankit.k.nautiyal@intel.com (mailing list archive)
State New, archived
Headers show
Series Add support for DP-HDMI2.1 PCON | expand

Commit Message

Nautiyal, Ankit K Dec. 8, 2020, 7:51 a.m. UTC
If PCON has capability to convert RGB->YUV colorspace and also
to 444->420 downsampling then for any YUV420 only mode, we can
let the PCON do all the conversion.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 .../drm/i915/display/intel_display_types.h    |  1 +
 drivers/gpu/drm/i915/display/intel_dp.c       | 37 +++++++++++++------
 2 files changed, 26 insertions(+), 12 deletions(-)

Comments

Shankar, Uma Dec. 13, 2020, 7:23 a.m. UTC | #1
> -----Original Message-----
> From: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>
> Sent: Tuesday, December 8, 2020 1:22 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org; Shankar, Uma <uma.shankar@intel.com>;
> airlied@linux.ie; jani.nikula@linux.intel.com; ville.syrjala@linux.intel.com;
> Kulkarni, Vandita <vandita.kulkarni@intel.com>; Sharma, Swati2
> <swati2.sharma@intel.com>
> Subject: [PATCH v4 15/16] drm/i915: Let PCON convert from RGB to YUV if it can

Make it drm/i915/display:

> If PCON has capability to convert RGB->YUV colorspace and also to 444->420
> downsampling then for any YUV420 only mode, we can let the PCON do all the
> conversion.
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  .../drm/i915/display/intel_display_types.h    |  1 +
>  drivers/gpu/drm/i915/display/intel_dp.c       | 37 +++++++++++++------
>  2 files changed, 26 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index b41de41759a0..4150108bdc6d 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1460,6 +1460,7 @@ struct intel_dp {
>  		int pcon_max_frl_bw, sink_max_frl_bw;
>  		u8 max_bpc;
>  		bool ycbcr_444_to_420;
> +		bool rgb_to_ycbcr;
>  	} dfp;
> 
>  	/* Display stream compression testing */ diff --git
> a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 30c76ba63232..b3f1190d8150 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -651,6 +651,10 @@ intel_dp_output_format(struct drm_connector
> *connector,
>  	    !drm_mode_is_420_only(info, mode))
>  		return INTEL_OUTPUT_FORMAT_RGB;
> 
> +	if (intel_dp->dfp.rgb_to_ycbcr &&
> +	    intel_dp->dfp.ycbcr_444_to_420)
> +		return INTEL_OUTPUT_FORMAT_RGB;
> +
>  	if (intel_dp->dfp.ycbcr_444_to_420)
>  		return INTEL_OUTPUT_FORMAT_YCBCR444;
>  	else
> @@ -4365,13 +4369,12 @@ void intel_dp_configure_protocol_converter(struct
> intel_dp *intel_dp)
>  			    "Failed to set protocol converter YCbCr 4:2:0
> conversion mode to %s\n",
>  			    enableddisabled(intel_dp->dfp.ycbcr_444_to_420));
> 
> -	tmp = 0;
> -
> -	if (drm_dp_dpcd_writeb(&intel_dp->aux,
> -			       DP_PROTOCOL_CONVERTER_CONTROL_2, tmp) <= 0)
> +	tmp = intel_dp->dfp.rgb_to_ycbcr ?
> +		DP_CONVERSION_BT601_RGB_YCBCR_ENABLE : 0;

There are other combinations also possible like BT709 and BT2020. Handle those as well here.
If no colorspace data is provided we can make BT601 as default, but if user chooses a colorspace
and pcon supports it, then we should go for that option. We can get that info from connector
colorspace properties.


> +	if (drm_dp_pcon_convert_rgb_to_ycbcr(&intel_dp->aux, tmp) <= 0)
>  		drm_dbg_kms(&i915->drm,
> -			    "Failed to set protocol converter YCbCr 4:2:2
> conversion mode to %s\n",
> -			    enableddisabled(false));
> +			    "Failed to set protocol converter RGB->YCbCr
> conversion mode to %s\n",
> +			    enableddisabled(intel_dp->dfp.rgb_to_ycbcr));
>  }
> 
>  static void intel_enable_dp(struct intel_atomic_state *state, @@ -6897,7
> +6900,7 @@ intel_dp_update_420(struct intel_dp *intel_dp)  {
>  	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
>  	struct intel_connector *connector = intel_dp->attached_connector;
> -	bool is_branch, ycbcr_420_passthrough, ycbcr_444_to_420;
> +	bool is_branch, ycbcr_420_passthrough, ycbcr_444_to_420, rgb_to_ycbcr;
> 
>  	/* No YCbCr output support on gmch platforms */
>  	if (HAS_GMCH(i915))
> @@ -6919,14 +6922,23 @@ intel_dp_update_420(struct intel_dp *intel_dp)
>  		dp_to_dig_port(intel_dp)->lspcon.active ||
>  		drm_dp_downstream_444_to_420_conversion(intel_dp->dpcd,
>  							intel_dp-
> >downstream_ports);
> +	rgb_to_ycbcr = drm_dp_downstream_rgb_to_ycbcr_conversion(intel_dp-
> >dpcd,
> +							intel_dp-
> >downstream_ports);
> 
>  	if (INTEL_GEN(i915) >= 11) {
> +		/* Let PCON convert from RGB->YCbCr if possible */
> +		if (is_branch && rgb_to_ycbcr && ycbcr_444_to_420) {
> +			intel_dp->dfp.rgb_to_ycbcr = true;
> +			intel_dp->dfp.ycbcr_444_to_420 = true;
> +			connector->base.ycbcr_420_allowed = true;
> +		} else {
>  		/* Prefer 4:2:0 passthrough over 4:4:4->4:2:0 conversion */
> -		intel_dp->dfp.ycbcr_444_to_420 =
> -			ycbcr_444_to_420 && !ycbcr_420_passthrough;
> +			intel_dp->dfp.ycbcr_444_to_420 =
> +				ycbcr_444_to_420 && !ycbcr_420_passthrough;
> 
> -		connector->base.ycbcr_420_allowed =
> -			!is_branch || ycbcr_444_to_420 ||
> ycbcr_420_passthrough;
> +			connector->base.ycbcr_420_allowed =
> +				!is_branch || ycbcr_444_to_420 ||
> ycbcr_420_passthrough;
> +		}
>  	} else {
>  		/* 4:4:4->4:2:0 conversion is the only way */
>  		intel_dp->dfp.ycbcr_444_to_420 = ycbcr_444_to_420; @@ -
> 6935,8 +6947,9 @@ intel_dp_update_420(struct intel_dp *intel_dp)
>  	}
> 
>  	drm_dbg_kms(&i915->drm,
> -		    "[CONNECTOR:%d:%s] YCbCr 4:2:0 allowed? %s, YCbCr 4:4:4-
> >4:2:0 conversion? %s\n",
> +		    "[CONNECTOR:%d:%s] RGB->YcbCr conversion? %s, YCbCr 4:2:0
> +allowed? %s, YCbCr 4:4:4->4:2:0 conversion? %s\n",
>  		    connector->base.base.id, connector->base.name,
> +		    yesno(intel_dp->dfp.rgb_to_ycbcr),
>  		    yesno(connector->base.ycbcr_420_allowed),
>  		    yesno(intel_dp->dfp.ycbcr_444_to_420));
>  }
> --
> 2.17.1
Nautiyal, Ankit K Dec. 14, 2020, 1:27 p.m. UTC | #2
On 12/13/2020 12:53 PM, Shankar, Uma wrote:
>
>> -----Original Message-----
>> From: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>
>> Sent: Tuesday, December 8, 2020 1:22 PM
>> To: intel-gfx@lists.freedesktop.org
>> Cc: dri-devel@lists.freedesktop.org; Shankar, Uma <uma.shankar@intel.com>;
>> airlied@linux.ie; jani.nikula@linux.intel.com; ville.syrjala@linux.intel.com;
>> Kulkarni, Vandita <vandita.kulkarni@intel.com>; Sharma, Swati2
>> <swati2.sharma@intel.com>
>> Subject: [PATCH v4 15/16] drm/i915: Let PCON convert from RGB to YUV if it can
> Make it drm/i915/display:
Noted. Will change in next version of the patch
>
>> If PCON has capability to convert RGB->YUV colorspace and also to 444->420
>> downsampling then for any YUV420 only mode, we can let the PCON do all the
>> conversion.
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   .../drm/i915/display/intel_display_types.h    |  1 +
>>   drivers/gpu/drm/i915/display/intel_dp.c       | 37 +++++++++++++------
>>   2 files changed, 26 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
>> b/drivers/gpu/drm/i915/display/intel_display_types.h
>> index b41de41759a0..4150108bdc6d 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
>> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
>> @@ -1460,6 +1460,7 @@ struct intel_dp {
>>   		int pcon_max_frl_bw, sink_max_frl_bw;
>>   		u8 max_bpc;
>>   		bool ycbcr_444_to_420;
>> +		bool rgb_to_ycbcr;
>>   	} dfp;
>>
>>   	/* Display stream compression testing */ diff --git
>> a/drivers/gpu/drm/i915/display/intel_dp.c
>> b/drivers/gpu/drm/i915/display/intel_dp.c
>> index 30c76ba63232..b3f1190d8150 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -651,6 +651,10 @@ intel_dp_output_format(struct drm_connector
>> *connector,
>>   	    !drm_mode_is_420_only(info, mode))
>>   		return INTEL_OUTPUT_FORMAT_RGB;
>>
>> +	if (intel_dp->dfp.rgb_to_ycbcr &&
>> +	    intel_dp->dfp.ycbcr_444_to_420)
>> +		return INTEL_OUTPUT_FORMAT_RGB;
>> +
>>   	if (intel_dp->dfp.ycbcr_444_to_420)
>>   		return INTEL_OUTPUT_FORMAT_YCBCR444;
>>   	else
>> @@ -4365,13 +4369,12 @@ void intel_dp_configure_protocol_converter(struct
>> intel_dp *intel_dp)
>>   			    "Failed to set protocol converter YCbCr 4:2:0
>> conversion mode to %s\n",
>>   			    enableddisabled(intel_dp->dfp.ycbcr_444_to_420));
>>
>> -	tmp = 0;
>> -
>> -	if (drm_dp_dpcd_writeb(&intel_dp->aux,
>> -			       DP_PROTOCOL_CONVERTER_CONTROL_2, tmp) <= 0)
>> +	tmp = intel_dp->dfp.rgb_to_ycbcr ?
>> +		DP_CONVERSION_BT601_RGB_YCBCR_ENABLE : 0;
> There are other combinations also possible like BT709 and BT2020. Handle those as well here.
> If no colorspace data is provided we can make BT601 as default, but if user chooses a colorspace
> and pcon supports it, then we should go for that option. We can get that info from connector
> colorspace properties.

Agreed. I will take into account color space in version of the patch.

For DP we set vsc.colorspace in pipe_config from connector colorspace, 
so I can use that information to take a call if its rec 709 or BT2020.

As suggested will use BT601 as default.

Thanks & Regards,

Ankit

>
>> +	if (drm_dp_pcon_convert_rgb_to_ycbcr(&intel_dp->aux, tmp) <= 0)
>>   		drm_dbg_kms(&i915->drm,
>> -			    "Failed to set protocol converter YCbCr 4:2:2
>> conversion mode to %s\n",
>> -			    enableddisabled(false));
>> +			    "Failed to set protocol converter RGB->YCbCr
>> conversion mode to %s\n",
>> +			    enableddisabled(intel_dp->dfp.rgb_to_ycbcr));
>>   }
>>
>>   static void intel_enable_dp(struct intel_atomic_state *state, @@ -6897,7
>> +6900,7 @@ intel_dp_update_420(struct intel_dp *intel_dp)  {
>>   	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
>>   	struct intel_connector *connector = intel_dp->attached_connector;
>> -	bool is_branch, ycbcr_420_passthrough, ycbcr_444_to_420;
>> +	bool is_branch, ycbcr_420_passthrough, ycbcr_444_to_420, rgb_to_ycbcr;
>>
>>   	/* No YCbCr output support on gmch platforms */
>>   	if (HAS_GMCH(i915))
>> @@ -6919,14 +6922,23 @@ intel_dp_update_420(struct intel_dp *intel_dp)
>>   		dp_to_dig_port(intel_dp)->lspcon.active ||
>>   		drm_dp_downstream_444_to_420_conversion(intel_dp->dpcd,
>>   							intel_dp-
>>> downstream_ports);
>> +	rgb_to_ycbcr = drm_dp_downstream_rgb_to_ycbcr_conversion(intel_dp-
>>> dpcd,
>> +							intel_dp-
>>> downstream_ports);
>>   	if (INTEL_GEN(i915) >= 11) {
>> +		/* Let PCON convert from RGB->YCbCr if possible */
>> +		if (is_branch && rgb_to_ycbcr && ycbcr_444_to_420) {
>> +			intel_dp->dfp.rgb_to_ycbcr = true;
>> +			intel_dp->dfp.ycbcr_444_to_420 = true;
>> +			connector->base.ycbcr_420_allowed = true;
>> +		} else {
>>   		/* Prefer 4:2:0 passthrough over 4:4:4->4:2:0 conversion */
>> -		intel_dp->dfp.ycbcr_444_to_420 =
>> -			ycbcr_444_to_420 && !ycbcr_420_passthrough;
>> +			intel_dp->dfp.ycbcr_444_to_420 =
>> +				ycbcr_444_to_420 && !ycbcr_420_passthrough;
>>
>> -		connector->base.ycbcr_420_allowed =
>> -			!is_branch || ycbcr_444_to_420 ||
>> ycbcr_420_passthrough;
>> +			connector->base.ycbcr_420_allowed =
>> +				!is_branch || ycbcr_444_to_420 ||
>> ycbcr_420_passthrough;
>> +		}
>>   	} else {
>>   		/* 4:4:4->4:2:0 conversion is the only way */
>>   		intel_dp->dfp.ycbcr_444_to_420 = ycbcr_444_to_420; @@ -
>> 6935,8 +6947,9 @@ intel_dp_update_420(struct intel_dp *intel_dp)
>>   	}
>>
>>   	drm_dbg_kms(&i915->drm,
>> -		    "[CONNECTOR:%d:%s] YCbCr 4:2:0 allowed? %s, YCbCr 4:4:4-
>>> 4:2:0 conversion? %s\n",
>> +		    "[CONNECTOR:%d:%s] RGB->YcbCr conversion? %s, YCbCr 4:2:0
>> +allowed? %s, YCbCr 4:4:4->4:2:0 conversion? %s\n",
>>   		    connector->base.base.id, connector->base.name,
>> +		    yesno(intel_dp->dfp.rgb_to_ycbcr),
>>   		    yesno(connector->base.ycbcr_420_allowed),
>>   		    yesno(intel_dp->dfp.ycbcr_444_to_420));
>>   }
>> --
>> 2.17.1
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index b41de41759a0..4150108bdc6d 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1460,6 +1460,7 @@  struct intel_dp {
 		int pcon_max_frl_bw, sink_max_frl_bw;
 		u8 max_bpc;
 		bool ycbcr_444_to_420;
+		bool rgb_to_ycbcr;
 	} dfp;
 
 	/* Display stream compression testing */
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 30c76ba63232..b3f1190d8150 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -651,6 +651,10 @@  intel_dp_output_format(struct drm_connector *connector,
 	    !drm_mode_is_420_only(info, mode))
 		return INTEL_OUTPUT_FORMAT_RGB;
 
+	if (intel_dp->dfp.rgb_to_ycbcr &&
+	    intel_dp->dfp.ycbcr_444_to_420)
+		return INTEL_OUTPUT_FORMAT_RGB;
+
 	if (intel_dp->dfp.ycbcr_444_to_420)
 		return INTEL_OUTPUT_FORMAT_YCBCR444;
 	else
@@ -4365,13 +4369,12 @@  void intel_dp_configure_protocol_converter(struct intel_dp *intel_dp)
 			    "Failed to set protocol converter YCbCr 4:2:0 conversion mode to %s\n",
 			    enableddisabled(intel_dp->dfp.ycbcr_444_to_420));
 
-	tmp = 0;
-
-	if (drm_dp_dpcd_writeb(&intel_dp->aux,
-			       DP_PROTOCOL_CONVERTER_CONTROL_2, tmp) <= 0)
+	tmp = intel_dp->dfp.rgb_to_ycbcr ?
+		DP_CONVERSION_BT601_RGB_YCBCR_ENABLE : 0;
+	if (drm_dp_pcon_convert_rgb_to_ycbcr(&intel_dp->aux, tmp) <= 0)
 		drm_dbg_kms(&i915->drm,
-			    "Failed to set protocol converter YCbCr 4:2:2 conversion mode to %s\n",
-			    enableddisabled(false));
+			    "Failed to set protocol converter RGB->YCbCr conversion mode to %s\n",
+			    enableddisabled(intel_dp->dfp.rgb_to_ycbcr));
 }
 
 static void intel_enable_dp(struct intel_atomic_state *state,
@@ -6897,7 +6900,7 @@  intel_dp_update_420(struct intel_dp *intel_dp)
 {
 	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
 	struct intel_connector *connector = intel_dp->attached_connector;
-	bool is_branch, ycbcr_420_passthrough, ycbcr_444_to_420;
+	bool is_branch, ycbcr_420_passthrough, ycbcr_444_to_420, rgb_to_ycbcr;
 
 	/* No YCbCr output support on gmch platforms */
 	if (HAS_GMCH(i915))
@@ -6919,14 +6922,23 @@  intel_dp_update_420(struct intel_dp *intel_dp)
 		dp_to_dig_port(intel_dp)->lspcon.active ||
 		drm_dp_downstream_444_to_420_conversion(intel_dp->dpcd,
 							intel_dp->downstream_ports);
+	rgb_to_ycbcr = drm_dp_downstream_rgb_to_ycbcr_conversion(intel_dp->dpcd,
+							intel_dp->downstream_ports);
 
 	if (INTEL_GEN(i915) >= 11) {
+		/* Let PCON convert from RGB->YCbCr if possible */
+		if (is_branch && rgb_to_ycbcr && ycbcr_444_to_420) {
+			intel_dp->dfp.rgb_to_ycbcr = true;
+			intel_dp->dfp.ycbcr_444_to_420 = true;
+			connector->base.ycbcr_420_allowed = true;
+		} else {
 		/* Prefer 4:2:0 passthrough over 4:4:4->4:2:0 conversion */
-		intel_dp->dfp.ycbcr_444_to_420 =
-			ycbcr_444_to_420 && !ycbcr_420_passthrough;
+			intel_dp->dfp.ycbcr_444_to_420 =
+				ycbcr_444_to_420 && !ycbcr_420_passthrough;
 
-		connector->base.ycbcr_420_allowed =
-			!is_branch || ycbcr_444_to_420 || ycbcr_420_passthrough;
+			connector->base.ycbcr_420_allowed =
+				!is_branch || ycbcr_444_to_420 || ycbcr_420_passthrough;
+		}
 	} else {
 		/* 4:4:4->4:2:0 conversion is the only way */
 		intel_dp->dfp.ycbcr_444_to_420 = ycbcr_444_to_420;
@@ -6935,8 +6947,9 @@  intel_dp_update_420(struct intel_dp *intel_dp)
 	}
 
 	drm_dbg_kms(&i915->drm,
-		    "[CONNECTOR:%d:%s] YCbCr 4:2:0 allowed? %s, YCbCr 4:4:4->4:2:0 conversion? %s\n",
+		    "[CONNECTOR:%d:%s] RGB->YcbCr conversion? %s, YCbCr 4:2:0 allowed? %s, YCbCr 4:4:4->4:2:0 conversion? %s\n",
 		    connector->base.base.id, connector->base.name,
+		    yesno(intel_dp->dfp.rgb_to_ycbcr),
 		    yesno(connector->base.ycbcr_420_allowed),
 		    yesno(intel_dp->dfp.ycbcr_444_to_420));
 }