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@xxxxxxxxxxxxxxx>
> Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@xxxxxxxxx>
> ---
> 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
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
