| Message ID | 20230803124913.210370-1-william.tseng@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] drm/i915/xelpd: Calculate first_line_bpg_offset for DSC 1.1 | expand |
> 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 <suraj.kandpal@intel.com> > Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com> > Cc: Juha-Pekka Heikkil <juha-pekka.heikkila@intel.com> > Signed-off-by: William Tseng <william.tseng@intel.com> > --- > 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
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 <suraj.kandpal@intel.com> Sent: Thursday, August 3, 2023 9:00 PM To: Tseng, William <william.tseng@intel.com>; intel-gfx@lists.freedesktop.org Cc: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; Heikkila, Juha-pekka <juha-pekka.heikkila@intel.com> 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 <suraj.kandpal@intel.com> > Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com> > Cc: Juha-Pekka Heikkil <juha-pekka.heikkila@intel.com> > Signed-off-by: William Tseng <william.tseng@intel.com> > --- > 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
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; + } 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); + 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)
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. 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 <suraj.kandpal@intel.com> Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com> Cc: Juha-Pekka Heikkil <juha-pekka.heikkila@intel.com> Signed-off-by: William Tseng <william.tseng@intel.com> --- drivers/gpu/drm/i915/display/intel_vdsc.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)