According to the `/* bpc 8 */` comment below only values for a
bits_per_component of 8 are currently hardcoded in place. This is
further confirmed by downstream sources [1] containing different
constants for other BPC values (and different initial_offset too,
with an extra dependency on bits_per_pixel). Prevent future mishaps by
explicitly disallowing any other bits_per_component value until the
right parameters are put in place and tested.
[1]: https://git.codelinaro.org/clo/la/platform/vendor/opensource/display-drivers/-/blob/DISPLAY.LA.2.0.r1-08000-WAIPIO.0/msm/sde_dsc_helper.c#L110-139
Signed-off-by: Marijn Suijten <[email protected]>
---
drivers/gpu/drm/msm/dsi/dsi_host.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 68c39debc22f..7e6b7e506ae8 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -1774,6 +1774,11 @@ static int dsi_populate_dsc_params(struct drm_dsc_config *dsc)
{
int i;
+ if (dsc->bits_per_component != 8) {
+ DRM_DEV_ERROR(&msm_host->pdev->dev, "DSI does not support bits_per_component != 8 yet\n");
+ return -EOPNOTSUPP;
+ }
+
dsc->rc_model_size = 8192;
dsc->first_line_bpg_offset = 12;
dsc->rc_edge_factor = 6;
--
2.38.0
On 09/10/2022 21:51, Marijn Suijten wrote:
> According to the `/* bpc 8 */` comment below only values for a
> bits_per_component of 8 are currently hardcoded in place. This is
> further confirmed by downstream sources [1] containing different
> constants for other BPC values (and different initial_offset too,
> with an extra dependency on bits_per_pixel). Prevent future mishaps by
> explicitly disallowing any other bits_per_component value until the
> right parameters are put in place and tested.
>
> [1]: https://git.codelinaro.org/clo/la/platform/vendor/opensource/display-drivers/-/blob/DISPLAY.LA.2.0.r1-08000-WAIPIO.0/msm/sde_dsc_helper.c#L110-139
>
> Signed-off-by: Marijn Suijten <[email protected]>
Reviewed-by: Dmitry Baryshkov <[email protected]>
--
With best wishes
Dmitry
Hi Marijn,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on next-20221007]
[cannot apply to drm-misc/drm-misc-next v6.0]
[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/Marijn-Suijten/drm-msm-Fix-math-issues-in-MSM-DSC-implementation/20221010-025519
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git a6afa4199d3d038fbfdff5511f7523b0e30cb774
config: s390-allyesconfig
compiler: s390-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/919ed8d5cca6928a77890ea464052a0ff6017c34
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Marijn-Suijten/drm-msm-Fix-math-issues-in-MSM-DSC-implementation/20221010-025519
git checkout 919ed8d5cca6928a77890ea464052a0ff6017c34
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
In file included from include/drm/drm_mm.h:51,
from include/drm/drm_vma_manager.h:26,
from include/drm/drm_gem.h:40,
from drivers/gpu/drm/msm/msm_drv.h:34,
from drivers/gpu/drm/msm/dsi/dsi.h:16,
from drivers/gpu/drm/msm/dsi/dsi_host.c:27:
drivers/gpu/drm/msm/dsi/dsi_host.c: In function 'dsi_populate_dsc_params':
>> drivers/gpu/drm/msm/dsi/dsi_host.c:1756:32: error: 'msm_host' undeclared (first use in this function)
1756 | DRM_DEV_ERROR(&msm_host->pdev->dev, "DSI does not support bits_per_component != 8 yet\n");
| ^~~~~~~~
include/drm/drm_print.h:371:24: note: in definition of macro 'DRM_DEV_ERROR'
371 | drm_dev_printk(dev, KERN_ERR, "*ERROR* " fmt, ##__VA_ARGS__)
| ^~~
drivers/gpu/drm/msm/dsi/dsi_host.c:1756:32: note: each undeclared identifier is reported only once for each function it appears in
1756 | DRM_DEV_ERROR(&msm_host->pdev->dev, "DSI does not support bits_per_component != 8 yet\n");
| ^~~~~~~~
include/drm/drm_print.h:371:24: note: in definition of macro 'DRM_DEV_ERROR'
371 | drm_dev_printk(dev, KERN_ERR, "*ERROR* " fmt, ##__VA_ARGS__)
| ^~~
vim +/msm_host +1756 drivers/gpu/drm/msm/dsi/dsi_host.c
1750
1751 static int dsi_populate_dsc_params(struct drm_dsc_config *dsc)
1752 {
1753 int i;
1754
1755 if (dsc->bits_per_component != 8) {
> 1756 DRM_DEV_ERROR(&msm_host->pdev->dev, "DSI does not support bits_per_component != 8 yet\n");
1757 return -EOPNOTSUPP;
1758 }
1759
1760 dsc->rc_model_size = 8192;
1761 dsc->first_line_bpg_offset = 12;
1762 dsc->rc_edge_factor = 6;
1763 dsc->rc_tgt_offset_high = 3;
1764 dsc->rc_tgt_offset_low = 3;
1765 dsc->simple_422 = 0;
1766 dsc->convert_rgb = 1;
1767 dsc->vbr_enable = 0;
1768
1769 /* handle only bpp = bpc = 8 */
1770 for (i = 0; i < DSC_NUM_BUF_RANGES - 1 ; i++)
1771 dsc->rc_buf_thresh[i] = dsi_dsc_rc_buf_thresh[i];
1772
1773 for (i = 0; i < DSC_NUM_BUF_RANGES; i++) {
1774 dsc->rc_range_params[i].range_min_qp = min_qp[i];
1775 dsc->rc_range_params[i].range_max_qp = max_qp[i];
1776 dsc->rc_range_params[i].range_bpg_offset = bpg_offset[i];
1777 }
1778
1779 dsc->initial_offset = 6144; /* Not bpp 12 */
1780 if (dsc->bits_per_pixel != 8)
1781 dsc->initial_offset = 2048; /* bpp = 12 */
1782
1783 if (dsc->bits_per_component <= 10)
1784 dsc->mux_word_size = DSC_MUX_WORD_SIZE_8_10_BPC;
1785 else
1786 dsc->mux_word_size = DSC_MUX_WORD_SIZE_12_BPC;
1787
1788 dsc->initial_xmit_delay = 512;
1789 dsc->initial_scale_value = 32;
1790 dsc->first_line_bpg_offset = 12;
1791 dsc->line_buf_depth = dsc->bits_per_component + 1;
1792
1793 /* bpc 8 */
1794 dsc->flatness_min_qp = 3;
1795 dsc->flatness_max_qp = 12;
1796 dsc->rc_quant_incr_limit0 = 11;
1797 dsc->rc_quant_incr_limit1 = 11;
1798
1799 return drm_dsc_compute_rc_parameters(dsc);
1800 }
1801
--
0-DAY CI Kernel Test Service
https://01.org/lkp
On 10/9/2022 11:51 AM, Marijn Suijten wrote:
> According to the `/* bpc 8 */` comment below only values for a
> bits_per_component of 8 are currently hardcoded in place. This is
> further confirmed by downstream sources [1] containing different
> constants for other BPC values (and different initial_offset too,
> with an extra dependency on bits_per_pixel). Prevent future mishaps by
> explicitly disallowing any other bits_per_component value until the
> right parameters are put in place and tested.
>
> [1]: https://git.codelinaro.org/clo/la/platform/vendor/opensource/display-drivers/-/blob/DISPLAY.LA.2.0.r1-08000-WAIPIO.0/msm/sde_dsc_helper.c#L110-139
>
Seems like a valid kbot error.
https://patchwork.freedesktop.org/patch/506359/#comment_912830
> Signed-off-by: Marijn Suijten <[email protected]>
> ---
> drivers/gpu/drm/msm/dsi/dsi_host.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index 68c39debc22f..7e6b7e506ae8 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -1774,6 +1774,11 @@ static int dsi_populate_dsc_params(struct drm_dsc_config *dsc)
> {
> int i;
>
> + if (dsc->bits_per_component != 8) {
> + DRM_DEV_ERROR(&msm_host->pdev->dev, "DSI does not support bits_per_component != 8 yet\n");
> + return -EOPNOTSUPP;
> + }
> +
> dsc->rc_model_size = 8192;
> dsc->first_line_bpg_offset = 12;
> dsc->rc_edge_factor = 6;
On 2022-10-12 16:08:07, Abhinav Kumar wrote:
>
>
> On 10/9/2022 11:51 AM, Marijn Suijten wrote:
> > According to the `/* bpc 8 */` comment below only values for a
> > bits_per_component of 8 are currently hardcoded in place. This is
> > further confirmed by downstream sources [1] containing different
> > constants for other BPC values (and different initial_offset too,
> > with an extra dependency on bits_per_pixel). Prevent future mishaps by
> > explicitly disallowing any other bits_per_component value until the
> > right parameters are put in place and tested.
> >
> > [1]: https://git.codelinaro.org/clo/la/platform/vendor/opensource/display-drivers/-/blob/DISPLAY.LA.2.0.r1-08000-WAIPIO.0/msm/sde_dsc_helper.c#L110-139
> >
>
> Seems like a valid kbot error.
>
> https://patchwork.freedesktop.org/patch/506359/#comment_912830
It is correct, and I suggested in [1] to either reorder this patch 7/10
after 8/10, or pull back the msm_host pointer argument into this patch.
[1]: https://lore.kernel.org/linux-arm-msm/[email protected]/
- Marijn
> > Signed-off-by: Marijn Suijten <[email protected]>
> > ---
> > drivers/gpu/drm/msm/dsi/dsi_host.c | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
> > index 68c39debc22f..7e6b7e506ae8 100644
> > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> > @@ -1774,6 +1774,11 @@ static int dsi_populate_dsc_params(struct drm_dsc_config *dsc)
> > {
> > int i;
> >
> > + if (dsc->bits_per_component != 8) {
> > + DRM_DEV_ERROR(&msm_host->pdev->dev, "DSI does not support bits_per_component != 8 yet\n");
> > + return -EOPNOTSUPP;
> > + }
> > +
> > dsc->rc_model_size = 8192;
> > dsc->first_line_bpg_offset = 12;
> > dsc->rc_edge_factor = 6;