[Intel-gfx] [PATCH] drm/i915/xelpd: Calculate first_line_bpg_offset for DSC 1.1

[Kandpal, Suraj]

> Thank you for the comments.  It seems the correction is for DSC 1.1 and DSC
> 1.2 at the same time.
> Can you submit a patch for the correction?  Because the original patch is
> specifically for the original specification DSC 1.1.
> 
Hi I have sent a patch fixing the dsc first_line_bpg_calculation I do not think after that fix this patch
Which is specific to DSC 1.1 will be required

Regards,
Suraj Kandpal
> 
> 
> On 8/2/2023 4:45 PM, Kandpal, Suraj wrote:
> > 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.
> >>
> >> Cc: Ankit Nautiyal <ankit.k.nautiyal at intel.com>
> >> Cc: Juha-Pekka Heikkil <juha-pekka.heikkila at intel.com>
> >> Signed-off-by: William Tseng <william.tseng at intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/display/intel_vdsc.c | 17 ++++++++++++-----
> >>   1 file changed, 12 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..9b350f5f8ebb 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> >> @@ -72,11 +72,18 @@ 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-
> 
> This seem to be incorrect as per the spec this should have been
> first_line_bpg_offset = 12 + floor(0.09 * MIN (34, slice_height - 8))for
> slice_height ≥ 8
> 
> So instead of rounding up we should have just divided.
> 
> 
> >>> slice_height - 8)), 100);
> >> -	else
> >> -		vdsc_cfg->first_line_bpg_offset = 2 * (vdsc_cfg-
> >>> slice_height - 1);
> >> +	if (vdsc_cfg->dsc_version_minor == 2) {
> > I think the check here should be for minor version 1 and move the code
> > in this block to the else block and vice versa as this 8 bpp corner
> > case is applicable only to DSC 1.1
> >
> >> +		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);
> >> +	} else {
> >> +		if (bpp == 8)
> >> +			rc->first_line_bpg_offset = 12;
> >> +		else
> >> +			rc->first_line_bpg_offset = 15;
> >> +	}
> > Add the section in DSC spec/ Cmodel from where one can verify this in
> > comments
> 
> I think this is coming from the recommended/ required value from DSC 1.1.
> 
> 
> Regards,
> 
> Ankit
> 
> >
> > Regards,
> > Suraj Kandpal
> >>   	/* Our hw supports only 444 modes as of today */
> >>   	if (bpp >= 12)
> >> --
> >> 2.34.1