> -----Original Message-----
> From: Kandpal, Suraj <suraj.kand...@intel.com>
> Sent: Thursday, February 20, 2025 2:01 PM
> To: Tseng, William <william.ts...@intel.com>; intel-gfx@lists.freedesktop.org
> Cc: Heikkila, Juha-pekka <juha-pekka.heikk...@intel.com>; Nautiyal, Ankit K
> <ankit.k.nauti...@intel.com>
> Subject: RE: [PATCH] drm/i915/dsc: Change rc parameters calculation for DSC
> 1.1
>
>
>
> > -----Original Message-----
> > From: Tseng, William <william.ts...@intel.com>
> > Sent: Thursday, February 20, 2025 8:56 AM
> > To: intel-gfx@lists.freedesktop.org
> > Cc: Tseng, William <william.ts...@intel.com>; Kandpal, Suraj
> > <suraj.kand...@intel.com>; Heikkila, Juha-pekka <juha-
> > pekka.heikk...@intel.com>
> > Subject: [PATCH] drm/i915/dsc: Change rc parameters calculation for
> > DSC 1.1
> >
>
>
> So to start with this needs to be sent intel-xe mailing list too
>
> > When calculating dsc parameters, the rc parameters calculated by
> > calculate_rc_params() may be incorrect in the case of DSC 1.1 and
>
> "Maybe" does not work to get the patch merged we need specifically what
> param does not work in calculate rc params so needs some more debug before
> sending a patch over.
> You can compare the dsc param dump when we use calculate rc params vs
> when we Use the tables
>
Thanks, Suraj. The wording should be changed.
> > DISPLAY_VER(dev_priv) >= 13 and cause noise-like lines displayed on a
> > MIPI DSI panel supporting DSC 1.1. The calculation seems for DSC 1.2
> > only. So, instead of calculate_rc_params(), calculate the rc
> > paramerters with the function
> > drm_dsc_setup_rc_params() for DSC 1.1.
> >
> > Cc: Suraj Kandpal <suraj.kand...@intel.com>
> > Cc: Juha-Pekka Heikkil <juha-pekka.heikk...@intel.com>
> > Signed-off-by: William Tseng <william.ts...@intel.com>
> > ---
> > drivers/gpu/drm/i915/display/intel_vdsc.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c
> > b/drivers/gpu/drm/i915/display/intel_vdsc.c
> > index b355c479eda3..e3443a1d12e0 100644
> > --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> > @@ -320,7 +320,7 @@ int intel_dsc_compute_params(struct
> > intel_crtc_state
> > *pipe_config)
> > * upto uncompressed bpp-1, hence add calculations for all the rc
> > * parameters
> > */
>
> You need to amend to comment explain the additional conditions
>
According to test result, the different parameters are listed below, comparing
the results from calculate_rc_params for DSC 1.2 and drm_dsc_setup_rc_params
for DSC 1.1.
first_line_bpg_offset (14 vs 12),
vdsc_cfg->rc_range_params[1].range_max_qp (5 vs 4),
vdsc_cfg->rc_range_params[2].range_min_qp (2 vs 1),
vdsc_cfg->rc_range_params[2].range_max_qp (7 vs 5)
vdsc_cfg->rc_range_params[3].range_min_qp (2 vs 1),
vdsc_cfg->rc_range_params[3].range_max_qp (7 vs 6)
vdsc_cfg->rc_range_params[4].range_min_qp (4 vs 3),
vdsc_cfg->rc_range_params[4].range_max_qp (8 vs 7)...
...
and so on.
That is why the additional condition is needed for the issue.
> > - if (DISPLAY_VER(dev_priv) >= 13) {
> > + if (DISPLAY_VER(dev_priv) >= 13 && vdsc_cfg->dsc_version_minor ==
> > 2) {
>
> Needs to be >= 2
>
Yes. It should be >= 2.
Please let me know if you have any questions.
Thank you.
Regards
William
> Regards,
> Suraj Kandpal
>
> > calculate_rc_params(vdsc_cfg);
> > } else {
> > if ((compressed_bpp == 8 ||
> > --
> > 2.34.1
