Re: [Intel-gfx] [PATCH v9 5/7] drm/i915: Fill in native_420 field
> On Tue, 07 Feb 2023, Suraj Kandpal wrote:
> > 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]
> >
> > Signed-off-by: Suraj Kandpal
> > ---
> > drivers/gpu/drm/i915/display/icl_dsi.c| 2 -
> > drivers/gpu/drm/i915/display/intel_dp.c | 3 -
> > drivers/gpu/drm/i915/display/intel_vdsc.c | 72
> > ++-
> > 3 files changed, 69 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c
> > b/drivers/gpu/drm/i915/display/icl_dsi.c
> > index 003cac918228..f8c999fa3242 100644
> > --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> > +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> > @@ -1534,8 +1534,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 73a7baccd7d0..250d9cdd14b8 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -1470,9 +1470,6 @@ static int intel_dp_dsc_compute_params(struct
> intel_encoder *encoder,
> > 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;
>
> Are the sink caps taken into account somewhere else?
Looking at it again I should move it after intel_dsc_compute_params and check
sink capability
If convert_rgb is set as true there
>
> > -
> > line_buf_depth = drm_dp_dsc_sink_line_buf_depth(intel_dp-
> >dsc_dpcd);
> > if (!line_buf_depth) {
> > drm_dbg_kms(>drm,
> > diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c
> > b/drivers/gpu/drm/i915/display/intel_vdsc.c
> > index ed16f63d6355..13ad853e24eb 100644
> > --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> > @@ -460,14 +460,47 @@ 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);
>
> Nitpick, IMO "format != x && format != y" reads better than "!(format == x ||
> format == y)"
>
Sure will change this in the next revision
Regards,
Suraj Kandpal
> > +
> > + if (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;
> > + /* Gen 11 does not support YCbCr422 */
> > 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 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;
> > +
Re: [Intel-gfx] [PATCH v9 5/7] drm/i915: Fill in native_420 field
On Tue, 07 Feb 2023, Suraj Kandpal wrote:
> 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]
>
> Signed-off-by: Suraj Kandpal
> ---
> drivers/gpu/drm/i915/display/icl_dsi.c| 2 -
> drivers/gpu/drm/i915/display/intel_dp.c | 3 -
> drivers/gpu/drm/i915/display/intel_vdsc.c | 72 ++-
> 3 files changed, 69 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c
> b/drivers/gpu/drm/i915/display/icl_dsi.c
> index 003cac918228..f8c999fa3242 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -1534,8 +1534,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 73a7baccd7d0..250d9cdd14b8 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1470,9 +1470,6 @@ static int intel_dp_dsc_compute_params(struct
> intel_encoder *encoder,
> 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;
Are the sink caps taken into account somewhere else?
> -
> line_buf_depth = drm_dp_dsc_sink_line_buf_depth(intel_dp->dsc_dpcd);
> if (!line_buf_depth) {
> drm_dbg_kms(>drm,
> diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c
> b/drivers/gpu/drm/i915/display/intel_vdsc.c
> index ed16f63d6355..13ad853e24eb 100644
> --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> @@ -460,14 +460,47 @@ 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);
Nitpick, IMO "format != x && format != y" reads better than "!(format ==
x || format == y)"
> +
> + if (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;
> + /* Gen 11 does not support YCbCr422 */
> 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 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
[Intel-gfx] [PATCH v9 5/7] drm/i915: Fill in native_420 field
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]
Signed-off-by: Suraj Kandpal
---
drivers/gpu/drm/i915/display/icl_dsi.c| 2 -
drivers/gpu/drm/i915/display/intel_dp.c | 3 -
drivers/gpu/drm/i915/display/intel_vdsc.c | 72 ++-
3 files changed, 69 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c
b/drivers/gpu/drm/i915/display/icl_dsi.c
index 003cac918228..f8c999fa3242 100644
--- a/drivers/gpu/drm/i915/display/icl_dsi.c
+++ b/drivers/gpu/drm/i915/display/icl_dsi.c
@@ -1534,8 +1534,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 73a7baccd7d0..250d9cdd14b8 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1470,9 +1470,6 @@ static int intel_dp_dsc_compute_params(struct
intel_encoder *encoder,
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;
-
line_buf_depth = drm_dp_dsc_sink_line_buf_depth(intel_dp->dsc_dpcd);
if (!line_buf_depth) {
drm_dbg_kms(>drm,
diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c
b/drivers/gpu/drm/i915/display/intel_vdsc.c
index ed16f63d6355..13ad853e24eb 100644
--- a/drivers/gpu/drm/i915/display/intel_vdsc.c
+++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
@@ -460,14 +460,47 @@ 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 (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;
+ /* Gen 11 does not support YCbCr422 */
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 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 +627,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)
+
