Re: [PATCH v6 5/6] drm/i915/dp: Change a link bandwidth computation for DP

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, May 08, 2019 at 11:17:56AM +0300, Gwan-gyeong Mun wrote:
> Data M/N calculations were assumed a bpp as RGB format. But when we are
> using YCbCr 4:2:0 output format on DP, we should change bpp calculations
> as YCbCr 4:2:0 format. The pipe_bpp value was assumed RGB format,
> therefore, it was multiplied with 3. But YCbCr 4:2:0 requires a multiplier
> value to 1.5.
> Therefore we need to divide pipe_bpp to 2 while DP output uses YCbCr4:2:0
> format.
>  - RGB format bpp = bpc x 3
>  - YCbCr 4:2:0 format bpp = bpc x 1.5
> 
> But Link M/N values are calculated and applied based on the Full Clock for
> YCbCr 4:2:0. And DP YCbCr 4:2:0 does not need to pixel clock double for
> a dotclock caluation. Only for HDMI YCbCr 4:2:0 needs to pixel clock double
> for a dot clock calculation.
> 
> And it adds missed bpc values for a programming of VSC Header.
> It only affects dp and edp port which use YCbCr 4:2:0 output format.
> And for now, it does not consider a use case of DSC + YCbCr 4:2:0.
> 
> v2:
>   Addressed review comments from Ville.
>   Remove a changing of pipe_bpp on intel_ddi_set_pipe_settings().
>   Because the pipe is running at the full bpp, keep pipe_bpp as RGB
>   even though YCbCr 4:2:0 output format is used.
>   Add a link bandwidth computation for YCbCr4:2:0 output format.
> 
> v3:
>   Addressed reivew comments from Ville.
>   In order to make codes simple, it adds and uses intel_dp_output_bpp()
>   function.
> 
> v6:
>   Link M/N values are calculated and applied based on the Full Clock for
>   YCbCr420. The Bit per Pixel needs to be adjusted for YUV420 mode as it
>   requires only half of the RGB case.
>     - Link M/N values are calculated and applied based on the Full Clock
>     - Data M/N values needs to be calculated considering the data is half
>       due to subsampling
>   Remove a doubling of pixel clock on a dot clock calculator for
>   DP YCbCr 4:2:0.
>   Rebase and remove a duplicate setting of vsc_sdp.DB17.
>   Add a setting of dynamic range bit to  vsc_sdp.DB17.
>   Change Content Type bit to "Graphics" from "Not defined".
>   Change a dividing of pipe_bpp to muliplying to constant values on a
>   switch-case statement.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c |  3 ++-
>  drivers/gpu/drm/i915/intel_dp.c  | 42 +++++++++++++++++++++++++++++---
>  2 files changed, 41 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 4441c5ba71fb..e22a0898b957 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1457,7 +1457,8 @@ static void ddi_dotclock_get(struct intel_crtc_state *pipe_config)
>  	else
>  		dotclock = pipe_config->port_clock;
>  
> -	if (pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420)
> +	if (pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 &&
> +	    !intel_crtc_has_dp_encoder(pipe_config))
>  		dotclock *= 2;
>  
>  	if (pipe_config->pixel_multiplier)
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 74aad8830a80..c75e2bbe612a 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1842,6 +1842,19 @@ intel_dp_adjust_compliance_config(struct intel_dp *intel_dp,
>  	}
>  }
>  
> +static int intel_dp_output_bpp(const struct intel_crtc_state *crtc_state, int bpp)
> +{
> +	/*
> +	 * bpp value was assumed to RGB format. And YCbCr 4:2:0 output
> +	 * format of the number of bytes per pixel will be half the number
> +	 * of bytes of RGB pixel.
> +	 */
> +	if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420)
> +		bpp /= 2;
> +
> +	return bpp;
> +}
> +
>  /* Optimize link config in order: max bpp, min clock, min lanes */
>  static int
>  intel_dp_compute_link_config_wide(struct intel_dp *intel_dp,
> @@ -2212,7 +2225,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  	if (pipe_config->dsc_params.compression_enable)
>  		output_bpp = pipe_config->dsc_params.compressed_bpp;
>  	else
> -		output_bpp = pipe_config->pipe_bpp;
> +		output_bpp = intel_dp_output_bpp(pipe_config, pipe_config->pipe_bpp);
>  
>  	intel_link_compute_m_n(output_bpp,
>  			       pipe_config->lane_count,
> @@ -4439,7 +4452,30 @@ intel_pixel_encoding_setup_vsc(struct intel_dp *intel_dp,
>  	 * 011b = 12bpc.
>  	 * 100b = 16bpc.
>  	 */
> -	vsc_sdp.DB17 = 0x1;
> +	switch (crtc_state->pipe_bpp) {
> +	case 24: /* 8bpc */
> +		vsc_sdp.DB17 = 0x1;
> +		break;
> +	case 30: /* 10bpc */
> +		vsc_sdp.DB17 = 0x2;
> +		break;
> +	case 36: /* 12bpc */
> +		vsc_sdp.DB17 = 0x3;
> +		break;
> +	case 48: /* 16bpc */
> +		vsc_sdp.DB17 = 0x4;
> +		break;
> +	default:
> +		DRM_DEBUG_KMS("Invalid bpp value '%d'\n", crtc_state->pipe_bpp);
> +		break;
> +	}
> +
> +	/*
> +	 * Dynamic Range (Bit 7)
> +	 * 0 = VESA range, 1 = CTA range.
> +	 * all YCbCr are always limited range
> +	 */
> +	vsc_sdp.DB17 |= 0x80;

Ah, it was all here already. Never mind then.

>  
>  	/*
>  	 * Content Type (Bits 2:0)
> @@ -4452,7 +4488,7 @@ intel_pixel_encoding_setup_vsc(struct intel_dp *intel_dp,
>  	 * Note: See CTA-861-G for the definition and expected
>  	 * processing by a stream sink for the above contect types.
>  	 */
> -	vsc_sdp.DB18 = 0;
> +	vsc_sdp.DB18 = 0x1;

Why is this here?

>  
>  	intel_dig_port->write_infoframe(&intel_dig_port->base,
>  			crtc_state, DP_SDP_VSC, &vsc_sdp, sizeof(vsc_sdp));
> -- 
> 2.21.0

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx