Re: [Intel-gfx] [PATCH v2 09/12] drm/i915/dp: Add support for "4:2:0 also" modes for DP

["Shankar, Uma" <uma.shankar@intel.com>]

> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville Syrjala
> Sent: Tuesday, March 22, 2022 5:30 PM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH v2 09/12] drm/i915/dp: Add support for "4:2:0 also"
> modes for DP
> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Currently we only support "4:2:0 also" modes on native HDMI.
> Extend that support for DP as well.
> 
> With all the HDMI DFP TMDS clock handling sorted out this is now going to work for
> both native DP and DP->HDMI converters. As with native HDMI we first check if RGB
> output is possible, and if not we try YCbCr 4:2:0 instead.

Looks Good to me.
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 67 ++++++++++++++++++++++---
>  1 file changed, 59 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 436d0b0f0b76..3dbb68fa4e51 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -895,6 +895,8 @@ intel_dp_mode_valid_downstream(struct intel_connector
> *connector,  {
>  	struct intel_dp *intel_dp = intel_attached_dp(connector);
>  	const struct drm_display_info *info = &connector->base.display_info;
> +	enum drm_mode_status status;
> +	bool ycbcr_420_only;
> 
>  	/* If PCON supports FRL MODE, check FRL bandwidth constraints */
>  	if (intel_dp->dfp.pcon_max_frl_bw) {
> @@ -919,9 +921,25 @@ intel_dp_mode_valid_downstream(struct intel_connector
> *connector,
>  	    target_clock > intel_dp->dfp.max_dotclock)
>  		return MODE_CLOCK_HIGH;
> 
> +	ycbcr_420_only = drm_mode_is_420_only(info, mode);
> +
>  	/* Assume 8bpc for the DP++/HDMI/DVI TMDS clock check */
> -	return intel_dp_tmds_clock_valid(intel_dp, target_clock, 8,
> -					 drm_mode_is_420_only(info, mode));
> +	status = intel_dp_tmds_clock_valid(intel_dp, target_clock,
> +					   8, ycbcr_420_only);
> +
> +	if (status != MODE_OK) {
> +		if (ycbcr_420_only ||
> +		    !connector->base.ycbcr_420_allowed ||
> +		    !drm_mode_is_420_also(info, mode))
> +			return status;
> +
> +		status = intel_dp_tmds_clock_valid(intel_dp, target_clock,
> +						   8, true);
> +		if (status != MODE_OK)
> +			return status;
> +	}
> +
> +	return MODE_OK;
>  }
> 
>  static bool intel_dp_need_bigjoiner(struct intel_dp *intel_dp, @@ -1829,6 +1847,43
> @@ static bool intel_dp_has_audio(struct intel_encoder *encoder,
>  		return intel_conn_state->force_audio == HDMI_AUDIO_ON;  }
> 
> +static int
> +intel_dp_compute_output_format(struct intel_encoder *encoder,
> +			       struct intel_crtc_state *crtc_state,
> +			       struct drm_connector_state *conn_state) {
> +	struct drm_i915_private *i915 = to_i915(encoder->base.dev);
> +	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> +	struct intel_connector *connector = intel_dp->attached_connector;
> +	const struct drm_display_info *info = &connector->base.display_info;
> +	const struct drm_display_mode *adjusted_mode = &crtc_state-
> >hw.adjusted_mode;
> +	bool ycbcr_420_only;
> +	int ret;
> +
> +	ycbcr_420_only = drm_mode_is_420_only(info, adjusted_mode);
> +
> +	crtc_state->output_format = intel_dp_output_format(connector,
> +ycbcr_420_only);
> +
> +	if (ycbcr_420_only && !intel_dp_is_ycbcr420(intel_dp, crtc_state)) {
> +		drm_dbg_kms(&i915->drm,
> +			    "YCbCr 4:2:0 mode but YCbCr 4:2:0 output not possible.
> Falling back to RGB.\n");
> +		crtc_state->output_format = INTEL_OUTPUT_FORMAT_RGB;
> +	}
> +
> +	ret = intel_dp_compute_link_config(encoder, crtc_state, conn_state);
> +	if (ret) {
> +		if (intel_dp_is_ycbcr420(intel_dp, crtc_state) ||
> +		    !connector->base.ycbcr_420_allowed ||
> +		    !drm_mode_is_420_also(info, adjusted_mode))
> +			return ret;
> +
> +		crtc_state->output_format = intel_dp_output_format(connector,
> true);
> +		ret = intel_dp_compute_link_config(encoder, crtc_state,
> conn_state);
> +	}
> +
> +	return ret;
> +}
> +
>  int
>  intel_dp_compute_config(struct intel_encoder *encoder,
>  			struct intel_crtc_state *pipe_config, @@ -1839,7 +1894,6
> @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>  	const struct drm_display_mode *fixed_mode;
>  	struct intel_connector *connector = intel_dp->attached_connector;
> -	const struct drm_display_info *info = &connector->base.display_info;
>  	bool constant_n = drm_dp_has_quirk(&intel_dp->desc,
> DP_DPCD_QUIRK_CONSTANT_N);
>  	int ret = 0, output_bpp;
> 
> @@ -1868,11 +1922,8 @@ intel_dp_compute_config(struct intel_encoder
> *encoder,
>  	if (intel_dp_hdisplay_bad(dev_priv, adjusted_mode->crtc_hdisplay))
>  		return -EINVAL;
> 
> -	pipe_config->output_format =
> -		intel_dp_output_format(connector, drm_mode_is_420_only(info,
> adjusted_mode));
> -
> -	ret = intel_dp_compute_link_config(encoder, pipe_config, conn_state);
> -	if (ret < 0)
> +	ret = intel_dp_compute_output_format(encoder, pipe_config, conn_state);
> +	if (ret)
>  		return ret;
> 
>  	if ((intel_dp_is_edp(intel_dp) && fixed_mode) ||
> --
> 2.34.1