Thank you for the comment.  I am not aware of the Errata of DSC 1.1.  Then this 
patch does not seem applicable to the revised calculation of the parameter 
first_line_bpg_offset.
Maybe a new patch should be created for the correction, in order to fix the 
problem which is not found on the SOC with display in the version older than 13.

-----Original Message-----
From: Kandpal, Suraj <[email protected]> 
Sent: Thursday, August 3, 2023 9:00 PM
To: Tseng, William <[email protected]>; [email protected]
Cc: Nautiyal, Ankit K <[email protected]>; Heikkila, Juha-pekka 
<[email protected]>
Subject: RE: [PATCH v3] drm/i915/xelpd: Calculate first_line_bpg_offset for DSC 
1.1

> This change is required for DSC 1.1 because the current calculation is 
> for DSC
> 1.2 and may get a calculated value which is not recommended by DSC 
> 1.1, for example, the calculated value at 8bpp becomes 15, not the 
> value of 12 recommened by DSC 1.1.
> 

Hi Tseng,
Please find the comments below

> v2:
> - change the if-condition from minor version 2 to 1.
> - add comment about first_line_bpg_offset for DSC 1.1.
> v3:
> - change variable name from rc to vdsc_cfg.
> - add Cc
> 
> Cc: Suraj Kandpal <[email protected]>
> Cc: Ankit Nautiyal <[email protected]>
> Cc: Juha-Pekka Heikkil <[email protected]>
> Signed-off-by: William Tseng <[email protected]>
> ---
>  drivers/gpu/drm/i915/display/intel_vdsc.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c
> b/drivers/gpu/drm/i915/display/intel_vdsc.c
> index bd9116d2cd76..c7db3bc94246 100644
> --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> @@ -72,11 +72,19 @@ calculate_rc_params(struct drm_dsc_config
> *vdsc_cfg)
>       int qp_bpc_modifier = (bpc - 8) * 2;
>       u32 res, buf_i, bpp_i;
> 
> -     if (vdsc_cfg->slice_height >= 8)
> -             vdsc_cfg->first_line_bpg_offset =
> -                     12 + DIV_ROUND_UP((9 * min(34, vdsc_cfg-
> >slice_height - 8)), 100);
> -     else
> -             vdsc_cfg->first_line_bpg_offset = 2 * (vdsc_cfg->slice_height -
> 1);
> +     if (vdsc_cfg->dsc_version_minor == 1) {
> +             /* The recommended and required Values from Table E-2 for
> DSC1.1 */
> +             if (bpp == 8)
> +                     vdsc_cfg->first_line_bpg_offset = 12;
> +             else
> +                     vdsc_cfg->first_line_bpg_offset = 15;
> +     }

This code block wont be needed check comment below

else {
> +             if (vdsc_cfg->slice_height >= 8)
> +                     vdsc_cfg->first_line_bpg_offset =
> +                             12 + DIV_ROUND_UP((9 * min(34, vdsc_cfg-
> >slice_height - 8)), 100);

I think you missed the comment from Ankit but if you check the Errata of DSC 
1.1 they remove the ask for using 12 as first_line_bpg_offset when bpp is 8 and 
use the formula first_line_bpg_offset = 12 + floor(0.09 * MIN (34, slice_height 
- 8))for slice_height ≥ 8 which currently is incorrect as we are using 
DIV_ROUND_UP.
Your issue should be solved if you replace DIV_ROUND_UP with floor or a simple 
typecast.

Regards,
Suraj Kandpal

> +             else
> +                     vdsc_cfg->first_line_bpg_offset = 2 * (vdsc_cfg-
> >slice_height - 1);
> +     }
> 
>       /* Our hw supports only 444 modes as of today */
>       if (bpp >= 12)
> --
> 2.34.1

Reply via email to