[Intel-gfx] [PATCH] drm/i915/vdsc: Fix first_line_bpg_offset calculation

[Kandpal, Suraj]

> 
> On Fri, 04 Aug 2023, Suraj Kandpal <suraj.kandpal at intel.com> wrote:
> > On checking DSC1.1 Errata and DSC 1.2 spec the current formula we were
> > using was incorrect to calculate first_line_bpg_offset.
> > The new fixed formula is derived from C model.
> >
> > --v2
> > -Use clamp function in linux/minmax.h [Ankit]
> >
> > --v3
> > -remove linux/minmax.h header
> >
> > Signed-off-by: Suraj Kandpal <suraj.kandpal at intel.com>
> > Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal at intel.com>
> 
> Should this be:
> 
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/9102
> 

Hi Jani,
Actually this does not close the issue when we were looking into this issue we found this
discrepancy between cmodel and our code  fixed it and hoped it would even fix the above issue
but it seems the first_line_bpg_offset value clamps to 14 now instead of 12 which would be 
required in the above bug to work.
I am actually looking for your input on another patch I have floated that fixes this issue.
I have added the conditional for first_line_bpg_offset to be 12 incase dsc version is 1.1 and made
It dsi but not sure if that is the right approach, had to do this as it seems some panels were made with
the first DSC 1.1 spec where first_line_bpg_offset recommended value was 12 in mind but then this was
changed in the Errata more info in the patch link below
https://patchwork.freedesktop.org/series/122108/

Regards,
Suraj Kandpal

> > ---
> >  drivers/gpu/drm/i915/display/intel_vdsc.c | 12 +++++++++---
> >  1 file changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c
> > b/drivers/gpu/drm/i915/display/intel_vdsc.c
> > index 9d76c2756784..e4c395b4dc46 100644
> > --- a/drivers/gpu/drm/i915/display/intel_vdsc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c
> > @@ -80,13 +80,19 @@ calculate_rc_params(struct drm_dsc_config
> *vdsc_cfg)
> >  	int bpc = vdsc_cfg->bits_per_component;
> >  	int bpp = vdsc_cfg->bits_per_pixel >> 4;
> >  	int qp_bpc_modifier = (bpc - 8) * 2;
> > +	int uncompressed_bpg_rate;
> > +	int first_line_bpg_offset;
> >  	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);
> > +		first_line_bpg_offset =
> > +			12 + (9 * min(34, vdsc_cfg->slice_height - 8)) / 100;
> >  	else
> > -		vdsc_cfg->first_line_bpg_offset = 2 * (vdsc_cfg->slice_height -
> 1);
> > +		first_line_bpg_offset = 2 * (vdsc_cfg->slice_height - 1);
> > +
> > +	uncompressed_bpg_rate = (3 * bpc + (vdsc_cfg->convert_rgb ? 0 : 2))
> * 3;
> > +	vdsc_cfg->first_line_bpg_offset = clamp(first_line_bpg_offset, 0,
> > +						uncompressed_bpg_rate - 3 *
> bpp);
> >
> >  	/*
> >  	 * According to DSC 1.2 spec in Section 4.1 if native_420 is set:
> 
> --
> Jani Nikula, Intel Open Source Graphics Center