| Message ID | 20230802101541.10045-1-william.tseng@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/xelpd: Calculate first_line_bpg_offset for DSC 1.1 | expand |
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@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 | 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- > >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 Regards, Suraj Kandpal > > /* Our hw supports only 444 modes as of today */ > if (bpp >= 12) > -- > 2.34.1
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@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 | 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
Hi William, kernel test robot noticed the following build errors: [auto build test ERROR on drm-intel/for-linux-next-fixes] [also build test ERROR on linus/master v6.5-rc4 next-20230802] [cannot apply to drm-tip/drm-tip drm-intel/for-linux-next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/William-Tseng/drm-i915-xelpd-Calculate-first_line_bpg_offset-for-DSC-1-1/20230802-181626 base: git://anongit.freedesktop.org/drm-intel for-linux-next-fixes patch link: https://lore.kernel.org/r/20230802101541.10045-1-william.tseng%40intel.com patch subject: [Intel-gfx] [PATCH] drm/i915/xelpd: Calculate first_line_bpg_offset for DSC 1.1 config: x86_64-buildonly-randconfig-r002-20230731 (https://download.01.org/0day-ci/archive/20230802/202308022035.wnMRWpdC-lkp@intel.com/config) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce: (https://download.01.org/0day-ci/archive/20230802/202308022035.wnMRWpdC-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202308022035.wnMRWpdC-lkp@intel.com/ All errors (new ones prefixed by >>): drivers/gpu/drm/i915/display/intel_vdsc.c: In function 'calculate_rc_params': >> drivers/gpu/drm/i915/display/intel_vdsc.c:83:25: error: 'rc' undeclared (first use in this function); did you mean 'rq'? 83 | rc->first_line_bpg_offset = 12; | ^~ | rq drivers/gpu/drm/i915/display/intel_vdsc.c:83:25: note: each undeclared identifier is reported only once for each function it appears in vim +83 drivers/gpu/drm/i915/display/intel_vdsc.c 54 55 static void 56 calculate_rc_params(struct drm_dsc_config *vdsc_cfg) 57 { 58 int bpc = vdsc_cfg->bits_per_component; 59 int bpp = vdsc_cfg->bits_per_pixel >> 4; 60 static const s8 ofs_und6[] = { 61 0, -2, -2, -4, -6, -6, -8, -8, -8, -10, -10, -12, -12, -12, -12 62 }; 63 static const s8 ofs_und8[] = { 64 2, 0, 0, -2, -4, -6, -8, -8, -8, -10, -10, -10, -12, -12, -12 65 }; 66 static const s8 ofs_und12[] = { 67 2, 0, 0, -2, -4, -6, -8, -8, -8, -10, -10, -10, -12, -12, -12 68 }; 69 static const s8 ofs_und15[] = { 70 10, 8, 6, 4, 2, 0, -2, -4, -6, -8, -10, -10, -12, -12, -12 71 }; 72 int qp_bpc_modifier = (bpc - 8) * 2; 73 u32 res, buf_i, bpp_i; 74 75 if (vdsc_cfg->dsc_version_minor == 2) { 76 if (vdsc_cfg->slice_height >= 8) 77 vdsc_cfg->first_line_bpg_offset = 78 12 + DIV_ROUND_UP((9 * min(34, vdsc_cfg->slice_height - 8)), 100); 79 else 80 vdsc_cfg->first_line_bpg_offset = 2 * (vdsc_cfg->slice_height - 1); 81 } else { 82 if (bpp == 8) > 83 rc->first_line_bpg_offset = 12; 84 else 85 rc->first_line_bpg_offset = 15; 86 } 87 88 /* Our hw supports only 444 modes as of today */ 89 if (bpp >= 12) 90 vdsc_cfg->initial_offset = 2048; 91 else if (bpp >= 10) 92 vdsc_cfg->initial_offset = 5632 - DIV_ROUND_UP(((bpp - 10) * 3584), 2); 93 else if (bpp >= 8) 94 vdsc_cfg->initial_offset = 6144 - DIV_ROUND_UP(((bpp - 8) * 512), 2); 95 else 96 vdsc_cfg->initial_offset = 6144; 97 98 /* initial_xmit_delay = rc_model_size/2/compression_bpp */ 99 vdsc_cfg->initial_xmit_delay = DIV_ROUND_UP(DSC_RC_MODEL_SIZE_CONST, 2 * bpp); 100 101 vdsc_cfg->flatness_min_qp = 3 + qp_bpc_modifier; 102 vdsc_cfg->flatness_max_qp = 12 + qp_bpc_modifier; 103 104 vdsc_cfg->rc_quant_incr_limit0 = 11 + qp_bpc_modifier; 105 vdsc_cfg->rc_quant_incr_limit1 = 11 + qp_bpc_modifier; 106 107 bpp_i = (2 * (bpp - 6)); 108 for (buf_i = 0; buf_i < DSC_NUM_BUF_RANGES; buf_i++) { 109 u8 range_bpg_offset; 110 111 /* Read range_minqp and range_max_qp from qp tables */ 112 vdsc_cfg->rc_range_params[buf_i].range_min_qp = 113 intel_lookup_range_min_qp(bpc, buf_i, bpp_i, vdsc_cfg->native_420); 114 vdsc_cfg->rc_range_params[buf_i].range_max_qp = 115 intel_lookup_range_max_qp(bpc, buf_i, bpp_i, vdsc_cfg->native_420); 116 117 /* Calculate range_bpg_offset */ 118 if (bpp <= 6) { 119 range_bpg_offset = ofs_und6[buf_i]; 120 } else if (bpp <= 8) { 121 res = DIV_ROUND_UP(((bpp - 6) * (ofs_und8[buf_i] - ofs_und6[buf_i])), 2); 122 range_bpg_offset = ofs_und6[buf_i] + res; 123 } else if (bpp <= 12) { 124 range_bpg_offset = ofs_und8[buf_i]; 125 } else if (bpp <= 15) { 126 res = DIV_ROUND_UP(((bpp - 12) * (ofs_und15[buf_i] - ofs_und12[buf_i])), 3); 127 range_bpg_offset = ofs_und12[buf_i] + res; 128 } else { 129 range_bpg_offset = ofs_und15[buf_i]; 130 } 131 132 vdsc_cfg->rc_range_params[buf_i].range_bpg_offset = 133 range_bpg_offset & DSC_RANGE_BPG_OFFSET_MASK; 134 } 135 } 136
Hi William, kernel test robot noticed the following build errors: [auto build test ERROR on drm-intel/for-linux-next-fixes] [also build test ERROR on linus/master v6.5-rc4 next-20230802] [cannot apply to drm-tip/drm-tip drm-intel/for-linux-next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/William-Tseng/drm-i915-xelpd-Calculate-first_line_bpg_offset-for-DSC-1-1/20230802-181626 base: git://anongit.freedesktop.org/drm-intel for-linux-next-fixes patch link: https://lore.kernel.org/r/20230802101541.10045-1-william.tseng%40intel.com patch subject: [Intel-gfx] [PATCH] drm/i915/xelpd: Calculate first_line_bpg_offset for DSC 1.1 config: x86_64-randconfig-r016-20230731 (https://download.01.org/0day-ci/archive/20230802/202308022109.Mp7MKSId-lkp@intel.com/config) compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07) reproduce: (https://download.01.org/0day-ci/archive/20230802/202308022109.Mp7MKSId-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202308022109.Mp7MKSId-lkp@intel.com/ All errors (new ones prefixed by >>): >> drivers/gpu/drm/i915/display/intel_vdsc.c:83:4: error: use of undeclared identifier 'rc' rc->first_line_bpg_offset = 12; ^ drivers/gpu/drm/i915/display/intel_vdsc.c:85:4: error: use of undeclared identifier 'rc' rc->first_line_bpg_offset = 15; ^ 2 errors generated. vim +/rc +83 drivers/gpu/drm/i915/display/intel_vdsc.c 54 55 static void 56 calculate_rc_params(struct drm_dsc_config *vdsc_cfg) 57 { 58 int bpc = vdsc_cfg->bits_per_component; 59 int bpp = vdsc_cfg->bits_per_pixel >> 4; 60 static const s8 ofs_und6[] = { 61 0, -2, -2, -4, -6, -6, -8, -8, -8, -10, -10, -12, -12, -12, -12 62 }; 63 static const s8 ofs_und8[] = { 64 2, 0, 0, -2, -4, -6, -8, -8, -8, -10, -10, -10, -12, -12, -12 65 }; 66 static const s8 ofs_und12[] = { 67 2, 0, 0, -2, -4, -6, -8, -8, -8, -10, -10, -10, -12, -12, -12 68 }; 69 static const s8 ofs_und15[] = { 70 10, 8, 6, 4, 2, 0, -2, -4, -6, -8, -10, -10, -12, -12, -12 71 }; 72 int qp_bpc_modifier = (bpc - 8) * 2; 73 u32 res, buf_i, bpp_i; 74 75 if (vdsc_cfg->dsc_version_minor == 2) { 76 if (vdsc_cfg->slice_height >= 8) 77 vdsc_cfg->first_line_bpg_offset = 78 12 + DIV_ROUND_UP((9 * min(34, vdsc_cfg->slice_height - 8)), 100); 79 else 80 vdsc_cfg->first_line_bpg_offset = 2 * (vdsc_cfg->slice_height - 1); 81 } else { 82 if (bpp == 8) > 83 rc->first_line_bpg_offset = 12; 84 else 85 rc->first_line_bpg_offset = 15; 86 } 87 88 /* Our hw supports only 444 modes as of today */ 89 if (bpp >= 12) 90 vdsc_cfg->initial_offset = 2048; 91 else if (bpp >= 10) 92 vdsc_cfg->initial_offset = 5632 - DIV_ROUND_UP(((bpp - 10) * 3584), 2); 93 else if (bpp >= 8) 94 vdsc_cfg->initial_offset = 6144 - DIV_ROUND_UP(((bpp - 8) * 512), 2); 95 else 96 vdsc_cfg->initial_offset = 6144; 97 98 /* initial_xmit_delay = rc_model_size/2/compression_bpp */ 99 vdsc_cfg->initial_xmit_delay = DIV_ROUND_UP(DSC_RC_MODEL_SIZE_CONST, 2 * bpp); 100 101 vdsc_cfg->flatness_min_qp = 3 + qp_bpc_modifier; 102 vdsc_cfg->flatness_max_qp = 12 + qp_bpc_modifier; 103 104 vdsc_cfg->rc_quant_incr_limit0 = 11 + qp_bpc_modifier; 105 vdsc_cfg->rc_quant_incr_limit1 = 11 + qp_bpc_modifier; 106 107 bpp_i = (2 * (bpp - 6)); 108 for (buf_i = 0; buf_i < DSC_NUM_BUF_RANGES; buf_i++) { 109 u8 range_bpg_offset; 110 111 /* Read range_minqp and range_max_qp from qp tables */ 112 vdsc_cfg->rc_range_params[buf_i].range_min_qp = 113 intel_lookup_range_min_qp(bpc, buf_i, bpp_i, vdsc_cfg->native_420); 114 vdsc_cfg->rc_range_params[buf_i].range_max_qp = 115 intel_lookup_range_max_qp(bpc, buf_i, bpp_i, vdsc_cfg->native_420); 116 117 /* Calculate range_bpg_offset */ 118 if (bpp <= 6) { 119 range_bpg_offset = ofs_und6[buf_i]; 120 } else if (bpp <= 8) { 121 res = DIV_ROUND_UP(((bpp - 6) * (ofs_und8[buf_i] - ofs_und6[buf_i])), 2); 122 range_bpg_offset = ofs_und6[buf_i] + res; 123 } else if (bpp <= 12) { 124 range_bpg_offset = ofs_und8[buf_i]; 125 } else if (bpp <= 15) { 126 res = DIV_ROUND_UP(((bpp - 12) * (ofs_und15[buf_i] - ofs_und12[buf_i])), 3); 127 range_bpg_offset = ofs_und12[buf_i] + res; 128 } else { 129 range_bpg_offset = ofs_und15[buf_i]; 130 } 131 132 vdsc_cfg->rc_range_params[buf_i].range_bpg_offset = 133 range_bpg_offset & DSC_RANGE_BPG_OFFSET_MASK; 134 } 135 } 136
Thank you for the comments. I have made the corresponding changes to the revised patch in version No.3. -----Original Message----- From: Kandpal, Suraj <suraj.kandpal@intel.com> Sent: Wednesday, August 2, 2023 7:15 PM To: Tseng, William <william.tseng@intel.com>; intel-gfx@lists.freedesktop.org Cc: Heikkila, Juha-pekka <juha-pekka.heikkila@intel.com>; Tseng, William <william.tseng@intel.com>; Nautiyal, Ankit K <ankit.k.nautiyal@intel.com> Subject: RE: [Intel-gfx] [PATCH] drm/i915/xelpd: Calculate first_line_bpg_offset for DSC 1.1 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@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 | 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- > >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 Regards, Suraj Kandpal > > /* Our hw supports only 444 modes as of today */ > if (bpp >= 12) > -- > 2.34.1
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. -----Original Message----- From: Nautiyal, Ankit K <ankit.k.nautiyal@intel.com> Sent: Wednesday, August 2, 2023 7:39 PM To: Kandpal, Suraj <suraj.kandpal@intel.com>; Tseng, William <william.tseng@intel.com>; intel-gfx@lists.freedesktop.org Cc: Heikkila, Juha-pekka <juha-pekka.heikkila@intel.com> Subject: Re: [Intel-gfx] [PATCH] drm/i915/xelpd: Calculate first_line_bpg_offset for DSC 1.1 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@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 | 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
> 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@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 | 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
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->slice_height - 8)), 100); - else - vdsc_cfg->first_line_bpg_offset = 2 * (vdsc_cfg->slice_height - 1); + if (vdsc_cfg->dsc_version_minor == 2) { + 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; + } /* 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. 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 | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-)