Re: [PATCH v2 5/7] drm/i915/display: Fill in native_420 field

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

 




> -----Original Message-----
> From: Kandpal, Suraj <suraj.kandpal@xxxxxxxxx>
> Sent: Friday, March 3, 2023 11:05 AM
> To: dri-devel@xxxxxxxxxxxxxxxxxxxxx; intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Shankar, Uma <uma.shankar@xxxxxxxxx>; Nautiyal, Ankit K
> <ankit.k.nautiyal@xxxxxxxxx>; Kandpal, Suraj <suraj.kandpal@xxxxxxxxx>; Jani Nikula
> <jani.nikula@xxxxxxxxxxxxxxx>
> Subject: [PATCH v2 5/7] drm/i915/display: Fill in native_420 field

I asked to add "display" but please change to "/drm/i915/dsc" as that is more relevant.
Use that for whole series.

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

> Now that we have laid the groundwork for YUV420 Enablement we fill up native_420
> field in vdsc_cfg and add appropriate checks wherever required.
> 
> ---v2
> -adding native_422 field as 0 [Vandita]
> -filling in second_line_bpg_offset, second_line_offset_adj and nsl_bpg_offset in
> vds_cfg when native_420 is true
> 
> ---v3
> -adding display version check to solve igt issue
> 
> --v7
> -remove is_pipe_dsc check as its always true for D14 [Jani]
> 
> --v10
> -keep sink capability check [Jani]
> -move from !(x == y  || w == z) to x !=y && w != z [Jani]
> 
> --v11
> -avoid native_420 computation if not gen14 [Uma]
> 
> --v12
> -fix state mismatch issue of compressed_bpp
> 
> Cc: Uma Shankar <uma.shankar@xxxxxxxxx>
> Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>
> Signed-off-by: Suraj Kandpal <suraj.kandpal@xxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/display/icl_dsi.c    |  2 -
>  drivers/gpu/drm/i915/display/intel_dp.c   | 16 +++-
>  drivers/gpu/drm/i915/display/intel_vdsc.c | 98 ++++++++++++++++++++---
>  3 files changed, 100 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c
> b/drivers/gpu/drm/i915/display/icl_dsi.c
> index b5316715bb3b..fb7efab8e9e6 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -1539,8 +1539,6 @@ static int gen11_dsi_dsc_compute_config(struct
> intel_encoder *encoder,
>  	if (crtc_state->dsc.slice_count > 1)
>  		crtc_state->dsc.dsc_split = true;
> 
> -	vdsc_cfg->convert_rgb = true;
> -
>  	/* FIXME: initialize from VBT */
>  	vdsc_cfg->rc_model_size = DSC_RC_MODEL_SIZE_CONST;
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 2649e8b8ef57..86b9348b74bb 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1467,9 +1467,10 @@ static int intel_dp_dsc_compute_params(struct
> intel_encoder *encoder,
>  	vdsc_cfg->dsc_version_minor =
>  		min(intel_dp_source_dsc_version_minor(intel_dp),
>  		    intel_dp_sink_dsc_version_minor(intel_dp));
> -
> -	vdsc_cfg->convert_rgb = intel_dp-
> >dsc_dpcd[DP_DSC_DEC_COLOR_FORMAT_CAP - DP_DSC_SUPPORT] &
> -		DP_DSC_RGB;
> +	if (vdsc_cfg->convert_rgb)
> +		vdsc_cfg->convert_rgb =
> +			intel_dp->dsc_dpcd[DP_DSC_DEC_COLOR_FORMAT_CAP -
> DP_DSC_SUPPORT] &
> +			DP_DSC_RGB;
> 
>  	line_buf_depth = drm_dp_dsc_sink_line_buf_depth(intel_dp->dsc_dpcd);
>  	if (!line_buf_depth) {
> @@ -1587,6 +1588,15 @@ int intel_dp_dsc_compute_config(struct intel_dp
> *intel_dp,
>  							    pipe_config-
> >bigjoiner_pipes,
>  							    pipe_bpp,
>  							    timeslots);
> +			/*
> +			 * According to DSC 1.2a Section 4.1.1 Table 4.1 the
> maximum
> +			 * supported PPS value can be 63.9375 and with the further
> +			 * mention that bpp should be programmed double the
> target bpp
> +			 * restricting our target bpp to be 31.9375 at max
> +			 */
> +			if (pipe_config->output_format ==
> INTEL_OUTPUT_FORMAT_YCBCR420)
> +				dsc_max_output_bpp = min_t(u16,
> dsc_max_output_bpp, 31 << 4);
> +
>  			if (!dsc_max_output_bpp) {
>  				drm_dbg_kms(&dev_priv->drm,
>  					    "Compressed BPP not supported\n"); diff
> --git a/drivers/gpu/drm/i915/display/intel_vdsc.c
> b/drivers/gpu/drm/i915/display/intel_vdsc.c
> index ed16f63d6355..44243fc81b46 100644
> --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> @@ -460,14 +460,50 @@ int intel_dsc_compute_params(struct intel_crtc_state
> *pipe_config)
>  	vdsc_cfg->pic_width = pipe_config->hw.adjusted_mode.crtc_hdisplay;
>  	vdsc_cfg->slice_width = DIV_ROUND_UP(vdsc_cfg->pic_width,
>  					     pipe_config->dsc.slice_count);
> -
> -	/* Gen 11 does not support YCbCr */
> +	/*
> +	 * According to DSC 1.2 specs if colorspace is YCbCr then convert_rgb is 0
> +	 * else 1
> +	 */
> +	vdsc_cfg->convert_rgb = pipe_config->output_format !=
> INTEL_OUTPUT_FORMAT_YCBCR420 &&
> +				pipe_config->output_format !=
> INTEL_OUTPUT_FORMAT_YCBCR444;
> +
> +	if (DISPLAY_VER(dev_priv) >= 14 &&
> +	    pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420)
> +		vdsc_cfg->native_420 = true;
> +	/* We do not support YcBCr422 as of now */
> +	vdsc_cfg->native_422 = false;
>  	vdsc_cfg->simple_422 = false;
>  	/* Gen 11 does not support VBR */
>  	vdsc_cfg->vbr_enable = false;
> 
>  	/* Gen 11 only supports integral values of bpp */
>  	vdsc_cfg->bits_per_pixel = compressed_bpp << 4;
> +
> +	/*
> +	 * According to DSC 1.2 specs in Section 4.1 if native_420 is set:
> +	 * -We need to double the current bpp.
> +	 * -second_line_bpg_offset is 12 in general and equal to 2*(slice_height-1) if
> slice
> +	 * height < 8.
> +	 * -second_line_offset_adj is 512 as shown by emperical values to yeild best
> chroma
> +	 * preservation in second line.
> +	 * -nsl_bpg_offset is calculated as second_line_offset/slice_height -1 then
> rounded
> +	 * up to 16 fractional bits, we left shift second line offset by 11 to preserve
> 11
> +	 * fractional bits.
> +	 */
> +	if (vdsc_cfg->native_420) {
> +		vdsc_cfg->bits_per_pixel <<= 1;
> +
> +		if (vdsc_cfg->slice_height >= 8)
> +			vdsc_cfg->second_line_bpg_offset = 12;
> +		else
> +			vdsc_cfg->second_line_bpg_offset =
> +				2 * (vdsc_cfg->slice_height - 1);
> +
> +		vdsc_cfg->second_line_offset_adj = 512;
> +		vdsc_cfg->nsl_bpg_offset = DIV_ROUND_UP(vdsc_cfg-
> >second_line_bpg_offset << 11,
> +							vdsc_cfg->slice_height - 1);
> +	}
> +
>  	vdsc_cfg->bits_per_component = pipe_config->pipe_bpp / 3;
> 
>  	for (i = 0; i < DSC_NUM_BUF_RANGES - 1; i++) { @@ -594,8 +630,13 @@
> static void intel_dsc_pps_configure(const struct intel_crtc_state *crtc_state)
>  		DSC_VER_MIN_SHIFT |
>  		vdsc_cfg->bits_per_component << DSC_BPC_SHIFT |
>  		vdsc_cfg->line_buf_depth << DSC_LINE_BUF_DEPTH_SHIFT;
> -	if (vdsc_cfg->dsc_version_minor == 2)
> +	if (vdsc_cfg->dsc_version_minor == 2) {
>  		pps_val |= DSC_ALT_ICH_SEL;
> +		if (vdsc_cfg->native_420)
> +			pps_val |= DSC_NATIVE_420_ENABLE;
> +		if (vdsc_cfg->native_422)
> +			pps_val |= DSC_NATIVE_422_ENABLE;
> +	}
>  	if (vdsc_cfg->block_pred_enable)
>  		pps_val |= DSC_BLOCK_PREDICTION;
>  	if (vdsc_cfg->convert_rgb)
> @@ -906,6 +947,33 @@ static void intel_dsc_pps_configure(const struct
> intel_crtc_state *crtc_state)
>  				       pps_val);
>  	}
> 
> +	if (DISPLAY_VER(dev_priv) >= 14) {
> +		/* Populate PICTURE_PARAMETER_SET_17 registers */
> +		pps_val = 0;
> +		pps_val |= DSC_SL_BPG_OFFSET(vdsc_cfg-
> >second_line_bpg_offset);
> +		drm_dbg_kms(&dev_priv->drm, "PPS17 = 0x%08x\n", pps_val);
> +		intel_de_write(dev_priv,
> +			       MTL_DSC0_PICTURE_PARAMETER_SET_17(pipe),
> +			       pps_val);
> +		if (crtc_state->dsc.dsc_split)
> +			intel_de_write(dev_priv,
> +
> MTL_DSC1_PICTURE_PARAMETER_SET_17(pipe),
> +				       pps_val);
> +
> +		/* Populate PICTURE_PARAMETER_SET_18 registers */
> +		pps_val = 0;
> +		pps_val |= DSC_NSL_BPG_OFFSET(vdsc_cfg->nsl_bpg_offset) |
> +			   DSC_SL_OFFSET_ADJ(vdsc_cfg->second_line_offset_adj);
> +		drm_dbg_kms(&dev_priv->drm, "PPS18 = 0x%08x\n", pps_val);
> +		intel_de_write(dev_priv,
> +			       MTL_DSC0_PICTURE_PARAMETER_SET_18(pipe),
> +			       pps_val);
> +		if (crtc_state->dsc.dsc_split)
> +			intel_de_write(dev_priv,
> +
> MTL_DSC1_PICTURE_PARAMETER_SET_18(pipe),
> +				       pps_val);
> +	}
> +
>  	/* Populate the RC_BUF_THRESH registers */
>  	memset(rc_buf_thresh_dword, 0, sizeof(rc_buf_thresh_dword));
>  	for (i = 0; i < DSC_NUM_BUF_RANGES - 1; i++) { @@ -1180,7 +1248,7 @@
> void intel_dsc_get_config(struct intel_crtc_state *crtc_state)
>  	enum pipe pipe = crtc->pipe;
>  	enum intel_display_power_domain power_domain;
>  	intel_wakeref_t wakeref;
> -	u32 dss_ctl1, dss_ctl2, val;
> +	u32 dss_ctl1, dss_ctl2, pps0 = 0, pps1 = 0;
> 
>  	if (!intel_dsc_source_support(crtc_state))
>  		return;
> @@ -1203,13 +1271,21 @@ void intel_dsc_get_config(struct intel_crtc_state
> *crtc_state)
> 
>  	/* FIXME: add more state readout as needed */
> 
> -	/* PPS1 */
> -	if (!is_pipe_dsc(crtc, cpu_transcoder))
> -		val = intel_de_read(dev_priv, DSCA_PICTURE_PARAMETER_SET_1);
> -	else
> -		val = intel_de_read(dev_priv,
> -				    ICL_DSC0_PICTURE_PARAMETER_SET_1(pipe));
> -	vdsc_cfg->bits_per_pixel = val;
> +	/* PPS0 & PPS1 */
> +	if (!is_pipe_dsc(crtc, cpu_transcoder)) {
> +		pps1 = intel_de_read(dev_priv,
> DSCA_PICTURE_PARAMETER_SET_1);
> +	} else {
> +		pps0 = intel_de_read(dev_priv,
> +				     ICL_DSC0_PICTURE_PARAMETER_SET_0(pipe));
> +		pps1 = intel_de_read(dev_priv,
> +				     ICL_DSC0_PICTURE_PARAMETER_SET_1(pipe));
> +	}
> +
> +	vdsc_cfg->bits_per_pixel = pps1;
> +
> +	if (pps0 & DSC_NATIVE_420_ENABLE)
> +		vdsc_cfg->bits_per_pixel >>= 1;
> +
>  	crtc_state->dsc.compressed_bpp = vdsc_cfg->bits_per_pixel >> 4;
>  out:
>  	intel_display_power_put(dev_priv, power_domain, wakeref);
> --
> 2.25.1





[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux