| Message ID | 20230804083737.3844575-1-suraj.kandpal@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/vdsc: Fix first_line_bpg_offset calculation | expand |
On Fri, 04 Aug 2023, Suraj Kandpal <suraj.kandpal@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@intel.com> > Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> Should this be: Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/9102 > --- > 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:
> > On Fri, 04 Aug 2023, Suraj Kandpal <suraj.kandpal@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@intel.com> > > Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@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
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: