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

2023-03-08 Thread Shankar, Uma



> -Original Message-
> From: Kandpal, Suraj 
> Sent: Friday, March 3, 2023 11:05 AM
> To: dri-devel@lists.freedesktop.org; intel-...@lists.freedesktop.org
> Cc: Shankar, Uma ; Nautiyal, Ankit K
> ; Kandpal, Suraj ; Jani 
> Nikula
> 
> 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 

> 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 
> Cc: Jani Nikula 
> Signed-off-by: Suraj Kandpal 
> ---
>  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(_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

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

2023-03-02 Thread Suraj Kandpal
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 
Cc: Jani Nikula 
Signed-off-by: Suraj Kandpal 
---
 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(_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