[PATCH v3 0/4] drm/msm: add RGB101010 pixel format and fix 10-bit DSC timing

[PATCH v3 0/4] drm/msm: add RGB101010 pixel format and fix 10-bit DSC timing

[Alexander Koskovich]

This series adds support for the RGB101010 (30bpp) pixel format and
fixes a DSC timing bug exposed by non 8 bit panels.

Tested on the BOE BF068MWM-TD0 panel (10 bit DSC) on the Nothing
Phone (3a).

Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
---
Changes in v3:
- Fix typo for MSM8998 DSI version name (V2_2 -> V_2_0)
- Add msm_dsi_host_version_is_gt per Konrad and use for RGB101010 check
- Fix up comment & commit message for video mode DSC INTF timing width change per Neil/Konrad
- Link to v2: https://lore.kernel.org/r/20260318-dsi-rgb101010-support-v2-0-698b7612eaeb@pm.me

Changes in v2:
- Only allow RGB101010 if MSM_DSI_6G_VER >= V2.1.0
- Link to v1: https://lore.kernel.org/r/20260318-dsi-rgb101010-support-v1-0-6021eb79e796@pm.me

---
Alexander Koskovich (4):
      drm/msm/dsi: rename MSM8998 DSI version from V2_2_0 to V2_0_0
      drm/msm/dsi: add DSI version >= comparison helper
      drm/msm/dsi: Add support for RGB101010 pixel format
      drm/msm/dpu: fix video mode DSC INTF timing width calculation

 .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c    |  9 ++++-----
 drivers/gpu/drm/msm/dsi/dsi_cfg.c                   |  4 ++--
 drivers/gpu/drm/msm/dsi/dsi_cfg.h                   |  2 +-
 drivers/gpu/drm/msm/dsi/dsi_host.c                  | 21 +++++++++++++++++++--
 drivers/gpu/drm/msm/registers/display/dsi.xml       |  5 ++++-
 5 files changed, 30 insertions(+), 11 deletions(-)
---
base-commit: f338e77383789c0cae23ca3d48adcc5e9e137e3c
change-id: 20260318-dsi-rgb101010-support-4956b1cd8657

Best regards,
-- 
Alexander Koskovich <akoskovich@pm.me>

[PATCH v3 1/4] drm/msm/dsi: rename MSM8998 DSI version from V2_2_0 to V2_0_0

[Alexander Koskovich]

The MSM8998 DSI controller is v2.0.0 as stated in commit 7b8c9e203039
("drm/msm/dsi: Add support for MSM8998 DSI controller"). The value was
always correct just the name was wrong.

Rename and reorder to maintain version sorting.

Fixes: 7b8c9e203039 ("drm/msm/dsi: Add support for MSM8998 DSI controller")
Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
---
 drivers/gpu/drm/msm/dsi/dsi_cfg.c | 4 ++--
 drivers/gpu/drm/msm/dsi/dsi_cfg.h | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.c b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
index bd3c51c350e7..da3fe6824495 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_cfg.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
@@ -317,10 +317,10 @@ static const struct msm_dsi_cfg_handler dsi_cfg_handlers[] = {
 		&msm8996_dsi_cfg, &msm_dsi_6g_host_ops},
 	{MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V1_4_2,
 		&msm8976_dsi_cfg, &msm_dsi_6g_host_ops},
+	{MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V2_0_0,
+		&msm8998_dsi_cfg, &msm_dsi_6g_v2_host_ops},
 	{MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V2_1_0,
 		&sdm660_dsi_cfg, &msm_dsi_6g_v2_host_ops},
-	{MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V2_2_0,
-		&msm8998_dsi_cfg, &msm_dsi_6g_v2_host_ops},
 	{MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V2_2_1,
 		&sdm845_dsi_cfg, &msm_dsi_6g_v2_host_ops},
 	{MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V2_3_0,
diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.h b/drivers/gpu/drm/msm/dsi/dsi_cfg.h
index 5dc812028bd5..ccf06679608e 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_cfg.h
+++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.h
@@ -19,8 +19,8 @@
 #define MSM_DSI_6G_VER_MINOR_V1_3_1	0x10030001
 #define MSM_DSI_6G_VER_MINOR_V1_4_1	0x10040001
 #define MSM_DSI_6G_VER_MINOR_V1_4_2	0x10040002
+#define MSM_DSI_6G_VER_MINOR_V2_0_0	0x20000000
 #define MSM_DSI_6G_VER_MINOR_V2_1_0	0x20010000
-#define MSM_DSI_6G_VER_MINOR_V2_2_0	0x20000000
 #define MSM_DSI_6G_VER_MINOR_V2_2_1	0x20020001
 #define MSM_DSI_6G_VER_MINOR_V2_3_0	0x20030000
 #define MSM_DSI_6G_VER_MINOR_V2_3_1	0x20030001

-- 
2.53.0

Re: [PATCH v3 1/4] drm/msm/dsi: rename MSM8998 DSI version from V2_2_0 to V2_0_0

[Konrad Dybcio]

On 3/19/26 12:57 PM, Alexander Koskovich wrote:
> The MSM8998 DSI controller is v2.0.0 as stated in commit 7b8c9e203039
> ("drm/msm/dsi: Add support for MSM8998 DSI controller"). The value was
> always correct just the name was wrong.
> 
> Rename and reorder to maintain version sorting.
> 
> Fixes: 7b8c9e203039 ("drm/msm/dsi: Add support for MSM8998 DSI controller")
> Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

Re: [PATCH v3 1/4] drm/msm/dsi: rename MSM8998 DSI version from V2_2_0 to V2_0_0

[Dmitry Baryshkov]

On Thu, Mar 19, 2026 at 11:57:47AM +0000, Alexander Koskovich wrote:
> The MSM8998 DSI controller is v2.0.0 as stated in commit 7b8c9e203039
> ("drm/msm/dsi: Add support for MSM8998 DSI controller"). The value was
> always correct just the name was wrong.
> 
> Rename and reorder to maintain version sorting.
> 
> Fixes: 7b8c9e203039 ("drm/msm/dsi: Add support for MSM8998 DSI controller")
> Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
> ---
>  drivers/gpu/drm/msm/dsi/dsi_cfg.c | 4 ++--
>  drivers/gpu/drm/msm/dsi/dsi_cfg.h | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

[PATCH v3 2/4] drm/msm/dsi: add DSI version >= comparison helper

[Alexander Koskovich]

Add a helper for checking if the DSI hardware version is greater
than or equal to a given version, for use in a future change.

Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
---
 drivers/gpu/drm/msm/dsi/dsi_host.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index db6da99375a1..6fad9a612d4d 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -782,13 +782,20 @@ static void dsi_ctrl_disable(struct msm_dsi_host *msm_host)
 	dsi_write(msm_host, REG_DSI_CTRL, 0);
 }
 
+static bool msm_dsi_host_version_ge(struct msm_dsi_host *msm_host,
+				    u32 major, u32 minor)
+{
+	return msm_host->cfg_hnd->major == major &&
+	       msm_host->cfg_hnd->minor >= minor;
+}
+
 bool msm_dsi_host_is_wide_bus_enabled(struct mipi_dsi_host *host)
 {
 	struct msm_dsi_host *msm_host = to_msm_dsi_host(host);
 
 	return msm_host->dsc &&
-		(msm_host->cfg_hnd->major == MSM_DSI_VER_MAJOR_6G &&
-		 msm_host->cfg_hnd->minor >= MSM_DSI_6G_VER_MINOR_V2_5_0);
+		msm_dsi_host_version_ge(msm_host, MSM_DSI_VER_MAJOR_6G,
+					MSM_DSI_6G_VER_MINOR_V2_5_0);
 }
 
 static void dsi_ctrl_enable(struct msm_dsi_host *msm_host,

-- 
2.53.0

Re: [PATCH v3 2/4] drm/msm/dsi: add DSI version >= comparison helper

[Konrad Dybcio]

On 3/19/26 12:57 PM, Alexander Koskovich wrote:
> Add a helper for checking if the DSI hardware version is greater
> than or equal to a given version, for use in a future change.
> 
> Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
> ---
>  drivers/gpu/drm/msm/dsi/dsi_host.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index db6da99375a1..6fad9a612d4d 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -782,13 +782,20 @@ static void dsi_ctrl_disable(struct msm_dsi_host *msm_host)
>  	dsi_write(msm_host, REG_DSI_CTRL, 0);
>  }
>  
> +static bool msm_dsi_host_version_ge(struct msm_dsi_host *msm_host,

I think that "geq" is more common/obvious, but anyway

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

Re: [PATCH v3 2/4] drm/msm/dsi: add DSI version >= comparison helper

[Dmitry Baryshkov]

On Thu, Mar 19, 2026 at 11:57:52AM +0000, Alexander Koskovich wrote:
> Add a helper for checking if the DSI hardware version is greater
> than or equal to a given version, for use in a future change.
> 
> Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
> ---
>  drivers/gpu/drm/msm/dsi/dsi_host.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

Re: [PATCH v3 2/4] drm/msm/dsi: add DSI version >= comparison helper

[Dmitry Baryshkov]

On Thu, Mar 19, 2026 at 11:57:52AM +0000, Alexander Koskovich wrote:
> Add a helper for checking if the DSI hardware version is greater
> than or equal to a given version, for use in a future change.
> 
> Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
> ---
>  drivers/gpu/drm/msm/dsi/dsi_host.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index db6da99375a1..6fad9a612d4d 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -782,13 +782,20 @@ static void dsi_ctrl_disable(struct msm_dsi_host *msm_host)
>  	dsi_write(msm_host, REG_DSI_CTRL, 0);
>  }
>  
> +static bool msm_dsi_host_version_ge(struct msm_dsi_host *msm_host,
> +				    u32 major, u32 minor)
> +{
> +	return msm_host->cfg_hnd->major == major &&
> +	       msm_host->cfg_hnd->minor >= minor;

I r-b'ed it, but there should be ... || msm_host->cfg_hnd->major > major.

> +}
> +
>  bool msm_dsi_host_is_wide_bus_enabled(struct mipi_dsi_host *host)
>  {
>  	struct msm_dsi_host *msm_host = to_msm_dsi_host(host);
>  
>  	return msm_host->dsc &&
> -		(msm_host->cfg_hnd->major == MSM_DSI_VER_MAJOR_6G &&
> -		 msm_host->cfg_hnd->minor >= MSM_DSI_6G_VER_MINOR_V2_5_0);
> +		msm_dsi_host_version_ge(msm_host, MSM_DSI_VER_MAJOR_6G,
> +					MSM_DSI_6G_VER_MINOR_V2_5_0);
>  }
>  
>  static void dsi_ctrl_enable(struct msm_dsi_host *msm_host,
> 
> -- 
> 2.53.0
> 
> 

-- 
With best wishes
Dmitry

[PATCH v3 3/4] drm/msm/dsi: Add support for RGB101010 pixel format

[Alexander Koskovich]

Add video and command mode destination format mappings for RGB101010,
and extend the VID_CFG0 DST_FORMAT bitfield to 3 bits to accommodate
the new format value.

Make sure this is guarded behind MSM_DSI_6G_VER >= V2.1.0 as anything
older does not support this.

Required for 10 bit panels such as the BOE BF068MWM-TD0.

Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
---
 drivers/gpu/drm/msm/dsi/dsi_host.c            | 10 ++++++++++
 drivers/gpu/drm/msm/registers/display/dsi.xml |  5 ++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 6fad9a612d4d..65c5b0e904ee 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -757,6 +757,7 @@ static inline enum dsi_vid_dst_format
 dsi_get_vid_fmt(const enum mipi_dsi_pixel_format mipi_fmt)
 {
 	switch (mipi_fmt) {
+	case MIPI_DSI_FMT_RGB101010:	return VID_DST_FORMAT_RGB101010;
 	case MIPI_DSI_FMT_RGB888:	return VID_DST_FORMAT_RGB888;
 	case MIPI_DSI_FMT_RGB666:	return VID_DST_FORMAT_RGB666_LOOSE;
 	case MIPI_DSI_FMT_RGB666_PACKED:	return VID_DST_FORMAT_RGB666;
@@ -769,6 +770,7 @@ static inline enum dsi_cmd_dst_format
 dsi_get_cmd_fmt(const enum mipi_dsi_pixel_format mipi_fmt)
 {
 	switch (mipi_fmt) {
+	case MIPI_DSI_FMT_RGB101010:	return CMD_DST_FORMAT_RGB101010;
 	case MIPI_DSI_FMT_RGB888:	return CMD_DST_FORMAT_RGB888;
 	case MIPI_DSI_FMT_RGB666_PACKED:
 	case MIPI_DSI_FMT_RGB666:	return CMD_DST_FORMAT_RGB666;
@@ -1705,6 +1707,14 @@ static int dsi_host_attach(struct mipi_dsi_host *host,
 	if (dsi->lanes > msm_host->num_data_lanes)
 		return -EINVAL;
 
+	if (dsi->format == MIPI_DSI_FMT_RGB101010 &&
+	    !msm_dsi_host_version_ge(msm_host, MSM_DSI_VER_MAJOR_6G,
+				     MSM_DSI_6G_VER_MINOR_V2_1_0)) {
+		DRM_DEV_ERROR(&msm_host->pdev->dev,
+			      "RGB101010 not supported on this DSI controller\n");
+		return -EINVAL;
+	}
+
 	msm_host->channel = dsi->channel;
 	msm_host->lanes = dsi->lanes;
 	msm_host->format = dsi->format;
diff --git a/drivers/gpu/drm/msm/registers/display/dsi.xml b/drivers/gpu/drm/msm/registers/display/dsi.xml
index c7a7b633d747..e40125f75175 100644
--- a/drivers/gpu/drm/msm/registers/display/dsi.xml
+++ b/drivers/gpu/drm/msm/registers/display/dsi.xml
@@ -15,6 +15,7 @@ xsi:schemaLocation="https://gitlab.freedesktop.org/freedreno/ rules-fd.xsd">
 		<value name="VID_DST_FORMAT_RGB666" value="1"/>
 		<value name="VID_DST_FORMAT_RGB666_LOOSE" value="2"/>
 		<value name="VID_DST_FORMAT_RGB888" value="3"/>
+		<value name="VID_DST_FORMAT_RGB101010" value="4"/>
 	</enum>
 	<enum name="dsi_rgb_swap">
 		<value name="SWAP_RGB" value="0"/>
@@ -39,6 +40,7 @@ xsi:schemaLocation="https://gitlab.freedesktop.org/freedreno/ rules-fd.xsd">
 		<value name="CMD_DST_FORMAT_RGB565" value="6"/>
 		<value name="CMD_DST_FORMAT_RGB666" value="7"/>
 		<value name="CMD_DST_FORMAT_RGB888" value="8"/>
+		<value name="CMD_DST_FORMAT_RGB101010" value="9"/>
 	</enum>
 	<enum name="dsi_lane_swap">
 		<value name="LANE_SWAP_0123" value="0"/>
@@ -142,7 +144,8 @@ xsi:schemaLocation="https://gitlab.freedesktop.org/freedreno/ rules-fd.xsd">
 	</reg32>
 	<reg32 offset="0x0000c" name="VID_CFG0">
 		<bitfield name="VIRT_CHANNEL" low="0" high="1" type="uint"/>  <!-- always zero? -->
-		<bitfield name="DST_FORMAT" low="4" high="5" type="dsi_vid_dst_format"/>
+		<!-- high was 5 before DSI 6G 2.1.0 -->
+		<bitfield name="DST_FORMAT" low="4" high="6" type="dsi_vid_dst_format"/>
 		<bitfield name="TRAFFIC_MODE" low="8" high="9" type="dsi_traffic_mode"/>
 		<bitfield name="BLLP_POWER_STOP" pos="12" type="boolean"/>
 		<bitfield name="EOF_BLLP_POWER_STOP" pos="15" type="boolean"/>

-- 
2.53.0

Re: [PATCH v3 3/4] drm/msm/dsi: Add support for RGB101010 pixel format

[Konrad Dybcio]

On 3/19/26 12:57 PM, Alexander Koskovich wrote:
> Add video and command mode destination format mappings for RGB101010,
> and extend the VID_CFG0 DST_FORMAT bitfield to 3 bits to accommodate
> the new format value.
> 
> Make sure this is guarded behind MSM_DSI_6G_VER >= V2.1.0 as anything
> older does not support this.
> 
> Required for 10 bit panels such as the BOE BF068MWM-TD0.
> 
> Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

Re: [PATCH v3 3/4] drm/msm/dsi: Add support for RGB101010 pixel format

[Dmitry Baryshkov]

On Thu, Mar 19, 2026 at 11:57:56AM +0000, Alexander Koskovich wrote:
> Add video and command mode destination format mappings for RGB101010,
> and extend the VID_CFG0 DST_FORMAT bitfield to 3 bits to accommodate
> the new format value.
> 
> Make sure this is guarded behind MSM_DSI_6G_VER >= V2.1.0 as anything
> older does not support this.
> 
> Required for 10 bit panels such as the BOE BF068MWM-TD0.
> 
> Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
> ---
>  drivers/gpu/drm/msm/dsi/dsi_host.c            | 10 ++++++++++
>  drivers/gpu/drm/msm/registers/display/dsi.xml |  5 ++++-
>  2 files changed, 14 insertions(+), 1 deletion(-)

For the patch itself:

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

Note, you've dropped the patch adding RGB101010 to
include/drm/drm_mipi_dsi.h without declaring a dependency on any
external patchset, so this can't be merged.

Also, there was a report from LKP that you need to fix the meson driver
while adding new MIPI define (we should not be introducing known
warnings to the kernel).

-- 
With best wishes
Dmitry

Re: [PATCH v3 3/4] drm/msm/dsi: Add support for RGB101010 pixel format

[Alexander Koskovich]

On Thursday, March 19th, 2026 at 3:00 PM, Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> wrote:

> On Thu, Mar 19, 2026 at 11:57:56AM +0000, Alexander Koskovich wrote:
> > Add video and command mode destination format mappings for RGB101010,
> > and extend the VID_CFG0 DST_FORMAT bitfield to 3 bits to accommodate
> > the new format value.
> >
> > Make sure this is guarded behind MSM_DSI_6G_VER >= V2.1.0 as anything
> > older does not support this.
> >
> > Required for 10 bit panels such as the BOE BF068MWM-TD0.
> >
> > Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
> > ---
> >  drivers/gpu/drm/msm/dsi/dsi_host.c            | 10 ++++++++++
> >  drivers/gpu/drm/msm/registers/display/dsi.xml |  5 ++++-
> >  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> For the patch itself:
> 
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> 
> Note, you've dropped the patch adding RGB101010 to
> include/drm/drm_mipi_dsi.h without declaring a dependency on any
> external patchset, so this can't be merged.

Oops, guess I skipped over that while rebasing. Will address in v4.

> 
> Also, there was a report from LKP that you need to fix the meson driver
> while adding new MIPI define (we should not be introducing known
> warnings to the kernel).

Will address that

> 
> --
> With best wishes
> Dmitry
> 
> 

Thanks,
Alex

Re: [PATCH v3 3/4] drm/msm/dsi: Add support for RGB101010 pixel format

[kernel test robot]

Hi Alexander,

kernel test robot noticed the following build errors:

[auto build test ERROR on f338e77383789c0cae23ca3d48adcc5e9e137e3c]

url:    https://github.com/intel-lab-lkp/linux/commits/Alexander-Koskovich/drm-msm-dsi-rename-MSM8998-DSI-version-from-V2_2_0-to-V2_0_0/20260320-011528
base:   f338e77383789c0cae23ca3d48adcc5e9e137e3c
patch link:    https://lore.kernel.org/r/20260319-dsi-rgb101010-support-v3-3-85b99df2d090%40pm.me
patch subject: [PATCH v3 3/4] drm/msm/dsi: Add support for RGB101010 pixel format
config: sparc64-allmodconfig (https://download.01.org/0day-ci/archive/20260320/202603201719.MxZJCZoY-lkp@intel.com/config)
compiler: clang version 23.0.0git (https://github.com/llvm/llvm-project 4abb927bacf37f18f6359a41639a6d1b3bffffb5)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260320/202603201719.MxZJCZoY-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/202603201719.MxZJCZoY-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/msm/dsi/dsi_host.c:760:7: error: use of undeclared identifier 'MIPI_DSI_FMT_RGB101010'
     760 |         case MIPI_DSI_FMT_RGB101010:    return VID_DST_FORMAT_RGB101010;
         |              ^~~~~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/msm/dsi/dsi_host.c:773:7: error: use of undeclared identifier 'MIPI_DSI_FMT_RGB101010'
     773 |         case MIPI_DSI_FMT_RGB101010:    return CMD_DST_FORMAT_RGB101010;
         |              ^~~~~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/msm/dsi/dsi_host.c:1710:21: error: use of undeclared identifier 'MIPI_DSI_FMT_RGB101010'
    1710 |         if (dsi->format == MIPI_DSI_FMT_RGB101010 &&
         |                            ^~~~~~~~~~~~~~~~~~~~~~
   3 errors generated.


vim +/MIPI_DSI_FMT_RGB101010 +760 drivers/gpu/drm/msm/dsi/dsi_host.c

   755	
   756	static inline enum dsi_vid_dst_format
   757	dsi_get_vid_fmt(const enum mipi_dsi_pixel_format mipi_fmt)
   758	{
   759		switch (mipi_fmt) {
 > 760		case MIPI_DSI_FMT_RGB101010:	return VID_DST_FORMAT_RGB101010;
   761		case MIPI_DSI_FMT_RGB888:	return VID_DST_FORMAT_RGB888;
   762		case MIPI_DSI_FMT_RGB666:	return VID_DST_FORMAT_RGB666_LOOSE;
   763		case MIPI_DSI_FMT_RGB666_PACKED:	return VID_DST_FORMAT_RGB666;
   764		case MIPI_DSI_FMT_RGB565:	return VID_DST_FORMAT_RGB565;
   765		default:			return VID_DST_FORMAT_RGB888;
   766		}
   767	}
   768	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v3 3/4] drm/msm/dsi: Add support for RGB101010 pixel format

[kernel test robot]

Hi Alexander,

kernel test robot noticed the following build errors:

[auto build test ERROR on f338e77383789c0cae23ca3d48adcc5e9e137e3c]

url:    https://github.com/intel-lab-lkp/linux/commits/Alexander-Koskovich/drm-msm-dsi-rename-MSM8998-DSI-version-from-V2_2_0-to-V2_0_0/20260320-011528
base:   f338e77383789c0cae23ca3d48adcc5e9e137e3c
patch link:    https://lore.kernel.org/r/20260319-dsi-rgb101010-support-v3-3-85b99df2d090%40pm.me
patch subject: [PATCH v3 3/4] drm/msm/dsi: Add support for RGB101010 pixel format
config: arc-allyesconfig (https://download.01.org/0day-ci/archive/20260321/202603210139.i7hnShAJ-lkp@intel.com/config)
compiler: arc-linux-gcc (GCC) 15.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260321/202603210139.i7hnShAJ-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/202603210139.i7hnShAJ-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/gpu/drm/msm/dsi/dsi_host.c: In function 'dsi_get_vid_fmt':
>> drivers/gpu/drm/msm/dsi/dsi_host.c:760:14: error: 'MIPI_DSI_FMT_RGB101010' undeclared (first use in this function); did you mean 'MIPI_DSI_FMT_RGB565'?
     760 |         case MIPI_DSI_FMT_RGB101010:    return VID_DST_FORMAT_RGB101010;
         |              ^~~~~~~~~~~~~~~~~~~~~~
         |              MIPI_DSI_FMT_RGB565
   drivers/gpu/drm/msm/dsi/dsi_host.c:760:14: note: each undeclared identifier is reported only once for each function it appears in
   drivers/gpu/drm/msm/dsi/dsi_host.c: In function 'dsi_get_cmd_fmt':
   drivers/gpu/drm/msm/dsi/dsi_host.c:773:14: error: 'MIPI_DSI_FMT_RGB101010' undeclared (first use in this function); did you mean 'MIPI_DSI_FMT_RGB565'?
     773 |         case MIPI_DSI_FMT_RGB101010:    return CMD_DST_FORMAT_RGB101010;
         |              ^~~~~~~~~~~~~~~~~~~~~~
         |              MIPI_DSI_FMT_RGB565
   drivers/gpu/drm/msm/dsi/dsi_host.c: In function 'dsi_host_attach':
   drivers/gpu/drm/msm/dsi/dsi_host.c:1710:28: error: 'MIPI_DSI_FMT_RGB101010' undeclared (first use in this function); did you mean 'MIPI_DSI_FMT_RGB565'?
    1710 |         if (dsi->format == MIPI_DSI_FMT_RGB101010 &&
         |                            ^~~~~~~~~~~~~~~~~~~~~~
         |                            MIPI_DSI_FMT_RGB565


vim +760 drivers/gpu/drm/msm/dsi/dsi_host.c

   755	
   756	static inline enum dsi_vid_dst_format
   757	dsi_get_vid_fmt(const enum mipi_dsi_pixel_format mipi_fmt)
   758	{
   759		switch (mipi_fmt) {
 > 760		case MIPI_DSI_FMT_RGB101010:	return VID_DST_FORMAT_RGB101010;
   761		case MIPI_DSI_FMT_RGB888:	return VID_DST_FORMAT_RGB888;
   762		case MIPI_DSI_FMT_RGB666:	return VID_DST_FORMAT_RGB666_LOOSE;
   763		case MIPI_DSI_FMT_RGB666_PACKED:	return VID_DST_FORMAT_RGB666;
   764		case MIPI_DSI_FMT_RGB565:	return VID_DST_FORMAT_RGB565;
   765		default:			return VID_DST_FORMAT_RGB888;
   766		}
   767	}
   768	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH v3 4/4] drm/msm/dpu: fix video mode DSC INTF timing width calculation

[Alexander Koskovich]

Using bits_per_component * 3 as the divisor for the compressed INTF
timing width produces constant FIFO errors for the BOE BF068MWM-TD0
panel due to bits_per_component being 10 which results in a divisor
of 30 instead of 24.

Regardless of the compression ratio and pixel depth, 24 bits of
compressed data are transferred per pclk, so the divisor should
always be 24.

Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
index 0ba777bda253..5419ef0be137 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
@@ -122,19 +122,18 @@ static void drm_mode_to_intf_timing_params(
 	}
 
 	/*
-	 * for DSI, if compression is enabled, then divide the horizonal active
-	 * timing parameters by compression ratio. bits of 3 components(R/G/B)
-	 * is compressed into bits of 1 pixel.
+	 * For DSI, if DSC is enabled, 24 bits of compressed data are
+	 * transferred per pclk regardless of the source pixel depth.
 	 */
 	if (phys_enc->hw_intf->cap->type != INTF_DP && timing->compression_en) {
 		struct drm_dsc_config *dsc =
 		       dpu_encoder_get_dsc_config(phys_enc->parent);
+
 		/*
 		 * TODO: replace drm_dsc_get_bpp_int with logic to handle
 		 * fractional part if there is fraction
 		 */
-		timing->width = timing->width * drm_dsc_get_bpp_int(dsc) /
-				(dsc->bits_per_component * 3);
+		timing->width = timing->width * drm_dsc_get_bpp_int(dsc) / 24;
 		timing->xres = timing->width;
 	}
 }

-- 
2.53.0

Re: [PATCH v3 4/4] drm/msm/dpu: fix video mode DSC INTF timing width calculation

[Neil Armstrong]

Hi,

On 3/19/26 12:58, Alexander Koskovich wrote:
> Using bits_per_component * 3 as the divisor for the compressed INTF
> timing width produces constant FIFO errors for the BOE BF068MWM-TD0
> panel due to bits_per_component being 10 which results in a divisor
> of 30 instead of 24.
> 
> Regardless of the compression ratio and pixel depth, 24 bits of
> compressed data are transferred per pclk, so the divisor should
> always be 24.

Not true with widebus, specify why 24 and because DSI widebus is not implemented yet.

> 
> Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 9 ++++-----
>   1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> index 0ba777bda253..5419ef0be137 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> @@ -122,19 +122,18 @@ static void drm_mode_to_intf_timing_params(
>   	}
>   
>   	/*
> -	 * for DSI, if compression is enabled, then divide the horizonal active
> -	 * timing parameters by compression ratio. bits of 3 components(R/G/B)
> -	 * is compressed into bits of 1 pixel.
> +	 * For DSI, if DSC is enabled, 24 bits of compressed data are
> +	 * transferred per pclk regardless of the source pixel depth.
>   	 */
>   	if (phys_enc->hw_intf->cap->type != INTF_DP && timing->compression_en) {
>   		struct drm_dsc_config *dsc =
>   		       dpu_encoder_get_dsc_config(phys_enc->parent);
> +
Drop this change

>   		/*
>   		 * TODO: replace drm_dsc_get_bpp_int with logic to handle
>   		 * fractional part if there is fraction
>   		 */
> -		timing->width = timing->width * drm_dsc_get_bpp_int(dsc) /
> -				(dsc->bits_per_component * 3);
> +		timing->width = timing->width * drm_dsc_get_bpp_int(dsc) / 24;

It would be helpful to somehow show that 24 is 8 * 3, 8 being the byte width and 3 the compression ratio.

>   		timing->xres = timing->width;
>   	}
>   }
> 

Re: [PATCH v3 4/4] drm/msm/dpu: fix video mode DSC INTF timing width calculation

[Alexander Koskovich]

On Thursday, March 19th, 2026 at 10:13 AM, Neil Armstrong <neil.armstrong@linaro.org> wrote:

> Hi,
> 
> On 3/19/26 12:58, Alexander Koskovich wrote:
> > Using bits_per_component * 3 as the divisor for the compressed INTF
> > timing width produces constant FIFO errors for the BOE BF068MWM-TD0
> > panel due to bits_per_component being 10 which results in a divisor
> > of 30 instead of 24.
> >
> > Regardless of the compression ratio and pixel depth, 24 bits of
> > compressed data are transferred per pclk, so the divisor should
> > always be 24.
> 
> Not true with widebus, specify why 24 and because DSI widebus is not implemented yet.
> 
> >
> > Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
> > ---
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 9 ++++-----
> >   1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> > index 0ba777bda253..5419ef0be137 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> > @@ -122,19 +122,18 @@ static void drm_mode_to_intf_timing_params(
> >   	}
> >
> >   	/*
> > -	 * for DSI, if compression is enabled, then divide the horizonal active
> > -	 * timing parameters by compression ratio. bits of 3 components(R/G/B)
> > -	 * is compressed into bits of 1 pixel.
> > +	 * For DSI, if DSC is enabled, 24 bits of compressed data are
> > +	 * transferred per pclk regardless of the source pixel depth.
> >   	 */
> >   	if (phys_enc->hw_intf->cap->type != INTF_DP && timing->compression_en) {
> >   		struct drm_dsc_config *dsc =
> >   		       dpu_encoder_get_dsc_config(phys_enc->parent);
> > +
> Drop this change
> 
> >   		/*
> >   		 * TODO: replace drm_dsc_get_bpp_int with logic to handle
> >   		 * fractional part if there is fraction
> >   		 */
> > -		timing->width = timing->width * drm_dsc_get_bpp_int(dsc) /
> > -				(dsc->bits_per_component * 3);
> > +		timing->width = timing->width * drm_dsc_get_bpp_int(dsc) / 24;
> 
> It would be helpful to somehow show that 24 is 8 * 3, 8 being the byte width and 3 the compression ratio.

Can you clarify what the 3 represents? My panel should have a 3.75:1 compression
ratio (30/8) so the final divisor of 24 would be wrong for my panel if its the
compression ratio?

> 
> >   		timing->xres = timing->width;
> >   	}
> >   }
> >
> 
> 
> 

Thanks,
Alex

Re: [PATCH v3 4/4] drm/msm/dpu: fix video mode DSC INTF timing width calculation

[Neil Armstrong]

On 3/19/26 15:40, Alexander Koskovich wrote:
> On Thursday, March 19th, 2026 at 10:13 AM, Neil Armstrong <neil.armstrong@linaro.org> wrote:
> 
>> Hi,
>>
>> On 3/19/26 12:58, Alexander Koskovich wrote:
>>> Using bits_per_component * 3 as the divisor for the compressed INTF
>>> timing width produces constant FIFO errors for the BOE BF068MWM-TD0
>>> panel due to bits_per_component being 10 which results in a divisor
>>> of 30 instead of 24.
>>>
>>> Regardless of the compression ratio and pixel depth, 24 bits of
>>> compressed data are transferred per pclk, so the divisor should
>>> always be 24.
>>
>> Not true with widebus, specify why 24 and because DSI widebus is not implemented yet.
>>
>>>
>>> Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
>>> ---
>>>    drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 9 ++++-----
>>>    1 file changed, 4 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
>>> index 0ba777bda253..5419ef0be137 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
>>> @@ -122,19 +122,18 @@ static void drm_mode_to_intf_timing_params(
>>>    	}
>>>
>>>    	/*
>>> -	 * for DSI, if compression is enabled, then divide the horizonal active
>>> -	 * timing parameters by compression ratio. bits of 3 components(R/G/B)
>>> -	 * is compressed into bits of 1 pixel.
>>> +	 * For DSI, if DSC is enabled, 24 bits of compressed data are
>>> +	 * transferred per pclk regardless of the source pixel depth.
>>>    	 */
>>>    	if (phys_enc->hw_intf->cap->type != INTF_DP && timing->compression_en) {
>>>    		struct drm_dsc_config *dsc =
>>>    		       dpu_encoder_get_dsc_config(phys_enc->parent);
>>> +
>> Drop this change
>>
>>>    		/*
>>>    		 * TODO: replace drm_dsc_get_bpp_int with logic to handle
>>>    		 * fractional part if there is fraction
>>>    		 */
>>> -		timing->width = timing->width * drm_dsc_get_bpp_int(dsc) /
>>> -				(dsc->bits_per_component * 3);
>>> +		timing->width = timing->width * drm_dsc_get_bpp_int(dsc) / 24;
>>
>> It would be helpful to somehow show that 24 is 8 * 3, 8 being the byte width and 3 the compression ratio.
> 
> Can you clarify what the 3 represents? My panel should have a 3.75:1 compression
> ratio (30/8) so the final divisor of 24 would be wrong for my panel if its the
> compression ratio?

So my guess is that while the exact ratio on the DSI lanes is 3.75:1, the ratio
used to calculate the INTF timings is 3, then the DSC encoder probably does the
magic to feed 10bpp into a 3.75:1 ratio over the DSI lanes.

In dsi_adjust_pclk_for_compression, the pclk is adjusted to take in account bits_per_component,
so I presume the actual DSI pclk _is_  timing->width * drm_dsc_get_bpp_int(dsc) / (dsc->bits_per_component * 3),
which is your 3.75:1, but the INTF needs to generate timing->width * drm_dsc_get_bpp_int(dsc) / (8 * 3) pixels
to the DSC encoder which will emit timing->width * drm_dsc_get_bpp_int(dsc) / (dsc->bits_per_component * 3) pixels.

In any case, 24 _is_ 3 * 8, 3 being the DSC compression ratio on the INTF side.

Dmitry, Konrad, could you help confirming this ?

Neil

> 
>>
>>>    		timing->xres = timing->width;
>>>    	}
>>>    }
>>>
>>
>>
>>
> 
> Thanks,
> Alex

Re: [PATCH v3 4/4] drm/msm/dpu: fix video mode DSC INTF timing width calculation

[Jonathan Marek]

On 3/19/26 10:54 AM, Neil Armstrong wrote:
> On 3/19/26 15:40, Alexander Koskovich wrote:
>> On Thursday, March 19th, 2026 at 10:13 AM, Neil Armstrong 
>> <neil.armstrong@linaro.org> wrote:
>>
>>> Hi,
>>>
>>> On 3/19/26 12:58, Alexander Koskovich wrote:
>>>> Using bits_per_component * 3 as the divisor for the compressed INTF
>>>> timing width produces constant FIFO errors for the BOE BF068MWM-TD0
>>>> panel due to bits_per_component being 10 which results in a divisor
>>>> of 30 instead of 24.
>>>>
>>>> Regardless of the compression ratio and pixel depth, 24 bits of
>>>> compressed data are transferred per pclk, so the divisor should
>>>> always be 24.
>>>
>>> Not true with widebus, specify why 24 and because DSI widebus is not 
>>> implemented yet.
>>>

DSI widebus is implemented, and always used when available. The 
adjustment for DSI widebus just doesn't happen in this function.

>>>>
>>>> Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
>>>> ---
>>>>    drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 9 ++++-----
>>>>    1 file changed, 4 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c 
>>>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
>>>> index 0ba777bda253..5419ef0be137 100644
>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
>>>> @@ -122,19 +122,18 @@ static void drm_mode_to_intf_timing_params(
>>>>        }
>>>>
>>>>        /*
>>>> -     * for DSI, if compression is enabled, then divide the 
>>>> horizonal active
>>>> -     * timing parameters by compression ratio. bits of 3 
>>>> components(R/G/B)
>>>> -     * is compressed into bits of 1 pixel.
>>>> +     * For DSI, if DSC is enabled, 24 bits of compressed data are
>>>> +     * transferred per pclk regardless of the source pixel depth.
>>>>         */
>>>>        if (phys_enc->hw_intf->cap->type != INTF_DP && 
>>>> timing->compression_en) {
>>>>            struct drm_dsc_config *dsc =
>>>>                   dpu_encoder_get_dsc_config(phys_enc->parent);
>>>> +
>>> Drop this change
>>>
>>>>            /*
>>>>             * TODO: replace drm_dsc_get_bpp_int with logic to handle
>>>>             * fractional part if there is fraction
>>>>             */
>>>> -        timing->width = timing->width * drm_dsc_get_bpp_int(dsc) /
>>>> -                (dsc->bits_per_component * 3);
>>>> +        timing->width = timing->width * drm_dsc_get_bpp_int(dsc) / 24;
>>>
>>> It would be helpful to somehow show that 24 is 8 * 3, 8 being the 
>>> byte width and 3 the compression ratio.
>>
>> Can you clarify what the 3 represents? My panel should have a 3.75:1 
>> compression
>> ratio (30/8) so the final divisor of 24 would be wrong for my panel if 
>> its the
>> compression ratio?
> 
> So my guess is that while the exact ratio on the DSI lanes is 3.75:1, 
> the ratio
> used to calculate the INTF timings is 3, then the DSC encoder probably 
> does the
> magic to feed 10bpp into a 3.75:1 ratio over the DSI lanes.
> 

That's not how it works. INTF (which feeds DSI) is after DSC compression.

INTF timings are always in RGB888 (24-bit) units. Ignoring widebus 
details, the INTF timing should match what is programmed on the DSI side 
(hdisplay, which is calculated as bytes per line / 3).

(fwiw, the current "timing->width = ..." calculation here blames to me, 
but what I wrote originally was just "timing->width = timing->width / 3" 
with a comment about being incomplete.)

> In dsi_adjust_pclk_for_compression, the pclk is adjusted to take in 
> account bits_per_component,
> so I presume the actual DSI pclk _is_  timing->width * 
> drm_dsc_get_bpp_int(dsc) / (dsc->bits_per_component * 3),
> which is your 3.75:1, but the INTF needs to generate timing->width * 
> drm_dsc_get_bpp_int(dsc) / (8 * 3) pixels
> to the DSC encoder which will emit timing->width * 
> drm_dsc_get_bpp_int(dsc) / (dsc->bits_per_component * 3) pixels.
> 

The hdisplay calculation in dsi_adjust_pclk_for_compression (which only 
affects the clock rate) seems to be wrong, and I think Alexander's panel 
must be running at a 20% lower clock because of it. dsi_timing_setup has 
the right hdisplay adjustment.

> In any case, 24 _is_ 3 * 8, 3 being the DSC compression ratio on the 
> INTF side.
> 
> Dmitry, Konrad, could you help confirming this ?
> 
> Neil
> 
>>
>>>
>>>>            timing->xres = timing->width;
>>>>        }
>>>>    }
>>>>
>>>
>>>
>>>
>>
>> Thanks,
>> Alex
> 

Re: [PATCH v3 4/4] drm/msm/dpu: fix video mode DSC INTF timing width calculation

[Alexander Koskovich]

On Thursday, March 19th, 2026 at 1:23 PM, Jonathan Marek <jonathan@marek.ca> wrote:

> On 3/19/26 10:54 AM, Neil Armstrong wrote:
> > On 3/19/26 15:40, Alexander Koskovich wrote:
> >> On Thursday, March 19th, 2026 at 10:13 AM, Neil Armstrong
> >> <neil.armstrong@linaro.org> wrote:
> >>
> >>> Hi,
> >>>
> >>> On 3/19/26 12:58, Alexander Koskovich wrote:
> >>>> Using bits_per_component * 3 as the divisor for the compressed INTF
> >>>> timing width produces constant FIFO errors for the BOE BF068MWM-TD0
> >>>> panel due to bits_per_component being 10 which results in a divisor
> >>>> of 30 instead of 24.
> >>>>
> >>>> Regardless of the compression ratio and pixel depth, 24 bits of
> >>>> compressed data are transferred per pclk, so the divisor should
> >>>> always be 24.
> >>>
> >>> Not true with widebus, specify why 24 and because DSI widebus is not
> >>> implemented yet.
> >>>
> 
> DSI widebus is implemented, and always used when available. The
> adjustment for DSI widebus just doesn't happen in this function.
> 
> >>>>
> >>>> Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
> >>>> ---
> >>>>    drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 9 ++++-----
> >>>>    1 file changed, 4 insertions(+), 5 deletions(-)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> >>>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> >>>> index 0ba777bda253..5419ef0be137 100644
> >>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> >>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> >>>> @@ -122,19 +122,18 @@ static void drm_mode_to_intf_timing_params(
> >>>>        }
> >>>>
> >>>>        /*
> >>>> -     * for DSI, if compression is enabled, then divide the
> >>>> horizonal active
> >>>> -     * timing parameters by compression ratio. bits of 3
> >>>> components(R/G/B)
> >>>> -     * is compressed into bits of 1 pixel.
> >>>> +     * For DSI, if DSC is enabled, 24 bits of compressed data are
> >>>> +     * transferred per pclk regardless of the source pixel depth.
> >>>>         */
> >>>>        if (phys_enc->hw_intf->cap->type != INTF_DP &&
> >>>> timing->compression_en) {
> >>>>            struct drm_dsc_config *dsc =
> >>>>                   dpu_encoder_get_dsc_config(phys_enc->parent);
> >>>> +
> >>> Drop this change
> >>>
> >>>>            /*
> >>>>             * TODO: replace drm_dsc_get_bpp_int with logic to handle
> >>>>             * fractional part if there is fraction
> >>>>             */
> >>>> -        timing->width = timing->width * drm_dsc_get_bpp_int(dsc) /
> >>>> -                (dsc->bits_per_component * 3);
> >>>> +        timing->width = timing->width * drm_dsc_get_bpp_int(dsc) / 24;
> >>>
> >>> It would be helpful to somehow show that 24 is 8 * 3, 8 being the
> >>> byte width and 3 the compression ratio.
> >>
> >> Can you clarify what the 3 represents? My panel should have a 3.75:1
> >> compression
> >> ratio (30/8) so the final divisor of 24 would be wrong for my panel if
> >> its the
> >> compression ratio?
> >
> > So my guess is that while the exact ratio on the DSI lanes is 3.75:1,
> > the ratio
> > used to calculate the INTF timings is 3, then the DSC encoder probably
> > does the
> > magic to feed 10bpp into a 3.75:1 ratio over the DSI lanes.
> >
> 
> That's not how it works. INTF (which feeds DSI) is after DSC compression.
> 
> INTF timings are always in RGB888 (24-bit) units. Ignoring widebus
> details, the INTF timing should match what is programmed on the DSI side
> (hdisplay, which is calculated as bytes per line / 3).
> 
> (fwiw, the current "timing->width = ..." calculation here blames to me,
> but what I wrote originally was just "timing->width = timing->width / 3"
> with a comment about being incomplete.)
> 
> > In dsi_adjust_pclk_for_compression, the pclk is adjusted to take in
> > account bits_per_component,
> > so I presume the actual DSI pclk _is_  timing->width *
> > drm_dsc_get_bpp_int(dsc) / (dsc->bits_per_component * 3),
> > which is your 3.75:1, but the INTF needs to generate timing->width *
> > drm_dsc_get_bpp_int(dsc) / (8 * 3) pixels
> > to the DSC encoder which will emit timing->width *
> > drm_dsc_get_bpp_int(dsc) / (dsc->bits_per_component * 3) pixels.
> >
> 
> The hdisplay calculation in dsi_adjust_pclk_for_compression (which only
> affects the clock rate) seems to be wrong, and I think Alexander's panel
> must be running at a 20% lower clock because of it. dsi_timing_setup has
> the right hdisplay adjustment.

Checked against downstream and the clocks seem to match more or less:

downstream:
pclk: 110070156
byte: 103190771

upstream:
pclk: 110073457
byte: 103193865


> 
> > In any case, 24 _is_ 3 * 8, 3 being the DSC compression ratio on the
> > INTF side.
> >
> > Dmitry, Konrad, could you help confirming this ?
> >
> > Neil
> >
> >>
> >>>
> >>>>            timing->xres = timing->width;
> >>>>        }
> >>>>    }
> >>>>
> >>>
> >>>
> >>>
> >>
> >> Thanks,
> >> Alex
> >
>

Re: [PATCH v3 4/4] drm/msm/dpu: fix video mode DSC INTF timing width calculation

[Jonathan Marek]

On 3/19/26 1:31 PM, Alexander Koskovich wrote:
> On Thursday, March 19th, 2026 at 1:23 PM, Jonathan Marek <jonathan@marek.ca> wrote:
> 
...>>
>> The hdisplay calculation in dsi_adjust_pclk_for_compression (which only
>> affects the clock rate) seems to be wrong, and I think Alexander's panel
>> must be running at a 20% lower clock because of it. dsi_timing_setup has
>> the right hdisplay adjustment.
> 
> Checked against downstream and the clocks seem to match more or less:
> 
> downstream:
> pclk: 110070156
> byte: 103190771
> 
> upstream:
> pclk: 110073457
> byte: 103193865
> 
I was curious about this and looked into it a bit (without testing any HW):

- using MIPI_DSI_FMT_RGB101010 dsi_byte_clk_get_rate cancels the effect 
of adjusting with bits_per_component for the byte clk, so the byte clock 
ends up being right
- using DST_FORMAT_RGB101010 DSI pclk is in 30-bit units instead of 
24-bit units, so the pclk ends up being right too (but that only works 
if widebus is enabled)

a recent commit (ac47870fd795) changed the hdisplay calculation in 
dsi_timing_setup to match that in dsi_adjust_pclk_for_compression, but 
only when widebus is enabled.

So things work out if widebus is enabled and MIPI_DSI_FMT_RGB101010 is 
used (note: looks like the only upstream 10-bit panel uses 
MIPI_DSI_FMT_RGB888), otherwise its broken.

AFAICT if you revert ac47870fd795, use MIPI_DSI_FMT_RGB888, and change 
dsi_adjust_pclk_for_compression to divide by 24 instead of 30, then it 
should also work (and won't be dependent on widebus).

Re: [PATCH v3 4/4] drm/msm/dpu: fix video mode DSC INTF timing width calculation

[Dmitry Baryshkov]

On Thu, Mar 19, 2026 at 01:23:03PM -0400, Jonathan Marek wrote:
> On 3/19/26 10:54 AM, Neil Armstrong wrote:
> > On 3/19/26 15:40, Alexander Koskovich wrote:
> > > On Thursday, March 19th, 2026 at 10:13 AM, Neil Armstrong
> > > <neil.armstrong@linaro.org> wrote:
> > > 
> > > > Hi,
> > > > 
> > > > On 3/19/26 12:58, Alexander Koskovich wrote:
> > > > > Using bits_per_component * 3 as the divisor for the compressed INTF
> > > > > timing width produces constant FIFO errors for the BOE BF068MWM-TD0
> > > > > panel due to bits_per_component being 10 which results in a divisor
> > > > > of 30 instead of 24.
> > > > > 
> > > > > Regardless of the compression ratio and pixel depth, 24 bits of
> > > > > compressed data are transferred per pclk, so the divisor should
> > > > > always be 24.
> > > > 
> > > > Not true with widebus, specify why 24 and because DSI widebus is
> > > > not implemented yet.
> > > > 
> 
> DSI widebus is implemented, and always used when available. The adjustment
> for DSI widebus just doesn't happen in this function.
> 
> > > > > 
> > > > > Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
> > > > > ---
> > > > >    drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 9 ++++-----
> > > > >    1 file changed, 4 insertions(+), 5 deletions(-)
> > > > > 
> > > > > diff --git
> > > > > a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> > > > > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> > > > > index 0ba777bda253..5419ef0be137 100644
> > > > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> > > > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> > > > > @@ -122,19 +122,18 @@ static void drm_mode_to_intf_timing_params(
> > > > >        }
> > > > > 
> > > > >        /*
> > > > > -     * for DSI, if compression is enabled, then divide the
> > > > > horizonal active
> > > > > -     * timing parameters by compression ratio. bits of 3
> > > > > components(R/G/B)
> > > > > -     * is compressed into bits of 1 pixel.
> > > > > +     * For DSI, if DSC is enabled, 24 bits of compressed data are
> > > > > +     * transferred per pclk regardless of the source pixel depth.
> > > > >         */
> > > > >        if (phys_enc->hw_intf->cap->type != INTF_DP &&
> > > > > timing->compression_en) {
> > > > >            struct drm_dsc_config *dsc =
> > > > >                   dpu_encoder_get_dsc_config(phys_enc->parent);
> > > > > +
> > > > Drop this change
> > > > 
> > > > >            /*
> > > > >             * TODO: replace drm_dsc_get_bpp_int with logic to handle
> > > > >             * fractional part if there is fraction
> > > > >             */
> > > > > -        timing->width = timing->width * drm_dsc_get_bpp_int(dsc) /
> > > > > -                (dsc->bits_per_component * 3);
> > > > > +        timing->width = timing->width * drm_dsc_get_bpp_int(dsc) / 24;
> > > > 
> > > > It would be helpful to somehow show that 24 is 8 * 3, 8 being
> > > > the byte width and 3 the compression ratio.
> > > 
> > > Can you clarify what the 3 represents? My panel should have a 3.75:1
> > > compression
> > > ratio (30/8) so the final divisor of 24 would be wrong for my panel
> > > if its the
> > > compression ratio?
> > 
> > So my guess is that while the exact ratio on the DSI lanes is 3.75:1,
> > the ratio
> > used to calculate the INTF timings is 3, then the DSC encoder probably
> > does the
> > magic to feed 10bpp into a 3.75:1 ratio over the DSI lanes.
> > 
> 
> That's not how it works. INTF (which feeds DSI) is after DSC compression.
> 
> INTF timings are always in RGB888 (24-bit) units. Ignoring widebus details,
> the INTF timing should match what is programmed on the DSI side (hdisplay,
> which is calculated as bytes per line / 3).
> 
> (fwiw, the current "timing->width = ..." calculation here blames to me, but
> what I wrote originally was just "timing->width = timing->width / 3" with a
> comment about being incomplete.)
> 
Okay. After reading the docs (sorry, it took a while).

- When widebus is not enabled, the transfer is always 24 bit of
  compressed data. Thus if it is not in play, pclk and timing->width
  should be scaled by source_pixel_depth / compression_ratio / 24. In
  case of the code it is 'drm_dsc_get_bpp_int(dsc) / 24'.

  For RGB101010 / 8bpp DSC this should result in the PCLK being lowered
  by the factor of 3 (= 24 / (30 / 3.75))

- When widebus is in play (MDSS 6.x+, DSI 2.4+), the transfer takes
  more than 24 bits. In this case the PCLK and timing->width should be
  scaled exactly by the DSC compression ratio, which is
  'drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component).

So, this piece of code needs to be adjusted to check for the widebus
being enabled or not.

> > In dsi_adjust_pclk_for_compression, the pclk is adjusted to take in
> > account bits_per_component,
> > so I presume the actual DSI pclk _is_  timing->width *
> > drm_dsc_get_bpp_int(dsc) / (dsc->bits_per_component * 3),
> > which is your 3.75:1, but the INTF needs to generate timing->width *
> > drm_dsc_get_bpp_int(dsc) / (8 * 3) pixels
> > to the DSC encoder which will emit timing->width *
> > drm_dsc_get_bpp_int(dsc) / (dsc->bits_per_component * 3) pixels.
> > 
> 
> The hdisplay calculation in dsi_adjust_pclk_for_compression (which only
> affects the clock rate) seems to be wrong, and I think Alexander's panel
> must be running at a 20% lower clock because of it. dsi_timing_setup has the
> right hdisplay adjustment.

That function also needs to be adjusted accordingly. I think only the
dsi_timing_setup() is correct at this point. Note, widebus / not-widebus
cases should be handled separately.

> > In any case, 24 _is_ 3 * 8, 3 being the DSC compression ratio on the
> > INTF side.

In this case DSC compression ratio is 3.75, so it's not true.

-- 
With best wishes
Dmitry

Re: [PATCH v3 4/4] drm/msm/dpu: fix video mode DSC INTF timing width calculation

[Alexander Koskovich]

On Thursday, March 19th, 2026 at 9:45 PM, Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> wrote:

> On Thu, Mar 19, 2026 at 01:23:03PM -0400, Jonathan Marek wrote:
> > On 3/19/26 10:54 AM, Neil Armstrong wrote:
> > > On 3/19/26 15:40, Alexander Koskovich wrote:
> > > > On Thursday, March 19th, 2026 at 10:13 AM, Neil Armstrong
> > > > <neil.armstrong@linaro.org> wrote:
> > > >
> > > > > Hi,
> > > > >
> > > > > On 3/19/26 12:58, Alexander Koskovich wrote:
> > > > > > Using bits_per_component * 3 as the divisor for the compressed INTF
> > > > > > timing width produces constant FIFO errors for the BOE BF068MWM-TD0
> > > > > > panel due to bits_per_component being 10 which results in a divisor
> > > > > > of 30 instead of 24.
> > > > > >
> > > > > > Regardless of the compression ratio and pixel depth, 24 bits of
> > > > > > compressed data are transferred per pclk, so the divisor should
> > > > > > always be 24.
> > > > >
> > > > > Not true with widebus, specify why 24 and because DSI widebus is
> > > > > not implemented yet.
> > > > >
> >
> > DSI widebus is implemented, and always used when available. The adjustment
> > for DSI widebus just doesn't happen in this function.
> >
> > > > > >
> > > > > > Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
> > > > > > ---
> > > > > >    drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 9 ++++-----
> > > > > >    1 file changed, 4 insertions(+), 5 deletions(-)
> > > > > >
> > > > > > diff --git
> > > > > > a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> > > > > > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> > > > > > index 0ba777bda253..5419ef0be137 100644
> > > > > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> > > > > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> > > > > > @@ -122,19 +122,18 @@ static void drm_mode_to_intf_timing_params(
> > > > > >        }
> > > > > >
> > > > > >        /*
> > > > > > -     * for DSI, if compression is enabled, then divide the
> > > > > > horizonal active
> > > > > > -     * timing parameters by compression ratio. bits of 3
> > > > > > components(R/G/B)
> > > > > > -     * is compressed into bits of 1 pixel.
> > > > > > +     * For DSI, if DSC is enabled, 24 bits of compressed data are
> > > > > > +     * transferred per pclk regardless of the source pixel depth.
> > > > > >         */
> > > > > >        if (phys_enc->hw_intf->cap->type != INTF_DP &&
> > > > > > timing->compression_en) {
> > > > > >            struct drm_dsc_config *dsc =
> > > > > >                   dpu_encoder_get_dsc_config(phys_enc->parent);
> > > > > > +
> > > > > Drop this change
> > > > >
> > > > > >            /*
> > > > > >             * TODO: replace drm_dsc_get_bpp_int with logic to handle
> > > > > >             * fractional part if there is fraction
> > > > > >             */
> > > > > > -        timing->width = timing->width * drm_dsc_get_bpp_int(dsc) /
> > > > > > -                (dsc->bits_per_component * 3);
> > > > > > +        timing->width = timing->width * drm_dsc_get_bpp_int(dsc) / 24;
> > > > >
> > > > > It would be helpful to somehow show that 24 is 8 * 3, 8 being
> > > > > the byte width and 3 the compression ratio.
> > > >
> > > > Can you clarify what the 3 represents? My panel should have a 3.75:1
> > > > compression
> > > > ratio (30/8) so the final divisor of 24 would be wrong for my panel
> > > > if its the
> > > > compression ratio?
> > >
> > > So my guess is that while the exact ratio on the DSI lanes is 3.75:1,
> > > the ratio
> > > used to calculate the INTF timings is 3, then the DSC encoder probably
> > > does the
> > > magic to feed 10bpp into a 3.75:1 ratio over the DSI lanes.
> > >
> >
> > That's not how it works. INTF (which feeds DSI) is after DSC compression.
> >
> > INTF timings are always in RGB888 (24-bit) units. Ignoring widebus details,
> > the INTF timing should match what is programmed on the DSI side (hdisplay,
> > which is calculated as bytes per line / 3).
> >
> > (fwiw, the current "timing->width = ..." calculation here blames to me, but
> > what I wrote originally was just "timing->width = timing->width / 3" with a
> > comment about being incomplete.)
> >
> Okay. After reading the docs (sorry, it took a while).
> 
> - When widebus is not enabled, the transfer is always 24 bit of
>   compressed data. Thus if it is not in play, pclk and timing->width
>   should be scaled by source_pixel_depth / compression_ratio / 24. In
>   case of the code it is 'drm_dsc_get_bpp_int(dsc) / 24'.
> 
>   For RGB101010 / 8bpp DSC this should result in the PCLK being lowered
>   by the factor of 3 (= 24 / (30 / 3.75))
> 
> - When widebus is in play (MDSS 6.x+, DSI 2.4+), the transfer takes
>   more than 24 bits. In this case the PCLK and timing->width should be
>   scaled exactly by the DSC compression ratio, which is
>   'drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component).
> 
> So, this piece of code needs to be adjusted to check for the widebus
> being enabled or not.
> 

Modified drm_mode_to_intf_timing_params & dsi_adjust_pclk_for_compression to account for widebus, but the hdisplay I get is different from stock with widebus factored in. Getting 288 instead of 360 now which produces constant FIFO errors.

This is with widebus enabled, using 3 * dsc->bits_per_component. Should it be 24 regardless of widebus?

> > > In dsi_adjust_pclk_for_compression, the pclk is adjusted to take in
> > > account bits_per_component,
> > > so I presume the actual DSI pclk _is_  timing->width *
> > > drm_dsc_get_bpp_int(dsc) / (dsc->bits_per_component * 3),
> > > which is your 3.75:1, but the INTF needs to generate timing->width *
> > > drm_dsc_get_bpp_int(dsc) / (8 * 3) pixels
> > > to the DSC encoder which will emit timing->width *
> > > drm_dsc_get_bpp_int(dsc) / (dsc->bits_per_component * 3) pixels.
> > >
> >
> > The hdisplay calculation in dsi_adjust_pclk_for_compression (which only
> > affects the clock rate) seems to be wrong, and I think Alexander's panel
> > must be running at a 20% lower clock because of it. dsi_timing_setup has the
> > right hdisplay adjustment.
> 
> That function also needs to be adjusted accordingly. I think only the
> dsi_timing_setup() is correct at this point. Note, widebus / not-widebus
> cases should be handled separately.
> 
> > > In any case, 24 _is_ 3 * 8, 3 being the DSC compression ratio on the
> > > INTF side.
> 
> In this case DSC compression ratio is 3.75, so it's not true.
> 
> --
> With best wishes
> Dmitry
> 

Thanks,
Alex

Re: [PATCH v3 4/4] drm/msm/dpu: fix video mode DSC INTF timing width calculation

[Jonathan Marek]

On 3/19/26 9:45 PM, Dmitry Baryshkov wrote:
> On Thu, Mar 19, 2026 at 01:23:03PM -0400, Jonathan Marek wrote:
...
>>
>> That's not how it works. INTF (which feeds DSI) is after DSC compression.
>>
>> INTF timings are always in RGB888 (24-bit) units. Ignoring widebus details,
>> the INTF timing should match what is programmed on the DSI side (hdisplay,
>> which is calculated as bytes per line / 3).
>>
>> (fwiw, the current "timing->width = ..." calculation here blames to me, but
>> what I wrote originally was just "timing->width = timing->width / 3" with a
>> comment about being incomplete.)
>>
> Okay. After reading the docs (sorry, it took a while).
> 
> - When widebus is not enabled, the transfer is always 24 bit of
>    compressed data. Thus if it is not in play, pclk and timing->width
>    should be scaled by source_pixel_depth / compression_ratio / 24. In
>    case of the code it is 'drm_dsc_get_bpp_int(dsc) / 24'.
> 
>    For RGB101010 / 8bpp DSC this should result in the PCLK being lowered
>    by the factor of 3 (= 24 / (30 / 3.75))
> 
> - When widebus is in play (MDSS 6.x+, DSI 2.4+), the transfer takes
>    more than 24 bits. In this case the PCLK and timing->width should be
>    scaled exactly by the DSC compression ratio, which is
>    'drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component).
> 
> So, this piece of code needs to be adjusted to check for the widebus
> being enabled or not.
> 

The widebus adjustment on the MDP/INTF side is already in 
dpu_hw_intf_setup_timing_engine: the "data width" is divided by 2 for 
48-bit widebus instead of 24-bit. there shouldn't be any other 
adjustment (downstream doesn't have any other adjustment).

a relevant downstream comment: "In DATABUS-WIDEN mode, MDP always sends 
out 48-bit compressed data per pclk and on average, DSI consumes an 
amount of compressed data equivalent to the uncompressed pixel depth per 
pclk."

Based on that comment, this patch is correct, and the 
''drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component)' adjustment 
only applies to DSI. (note: newer downstream looks like it would divide 
by 3.75 here, which doesn't make sense. older downstream would divide by 
3 here. I guess downstream is broken now and video mode + 10-bit dsc 
doesn't get tested?)

on DSI side, "uncompressed pixel depth" shouldn't matter either: DSI 
only sees the compressed data. But based on that comment, when widebus 
is enabled, by setting DSI_VID_CFG0_DST_FORMAT(?) to 30bpp, then the DSI 
pclk is in 30-bit units instead of 24-bits. And with this series DSI 
side ends up with the right result if 30bpp format and widebus is enabled.

Re: [PATCH v3 4/4] drm/msm/dpu: fix video mode DSC INTF timing width calculation

[Alexander Koskovich]

On Friday, March 20th, 2026 at 12:25 AM, Jonathan Marek <jonathan@marek.ca> wrote:

> On 3/19/26 9:45 PM, Dmitry Baryshkov wrote:
> > On Thu, Mar 19, 2026 at 01:23:03PM -0400, Jonathan Marek wrote:
> ...
> >>
> >> That's not how it works. INTF (which feeds DSI) is after DSC compression.
> >>
> >> INTF timings are always in RGB888 (24-bit) units. Ignoring widebus details,
> >> the INTF timing should match what is programmed on the DSI side (hdisplay,
> >> which is calculated as bytes per line / 3).
> >>
> >> (fwiw, the current "timing->width = ..." calculation here blames to me, but
> >> what I wrote originally was just "timing->width = timing->width / 3" with a
> >> comment about being incomplete.)
> >>
> > Okay. After reading the docs (sorry, it took a while).
> >
> > - When widebus is not enabled, the transfer is always 24 bit of
> >    compressed data. Thus if it is not in play, pclk and timing->width
> >    should be scaled by source_pixel_depth / compression_ratio / 24. In
> >    case of the code it is 'drm_dsc_get_bpp_int(dsc) / 24'.
> >
> >    For RGB101010 / 8bpp DSC this should result in the PCLK being lowered
> >    by the factor of 3 (= 24 / (30 / 3.75))
> >
> > - When widebus is in play (MDSS 6.x+, DSI 2.4+), the transfer takes
> >    more than 24 bits. In this case the PCLK and timing->width should be
> >    scaled exactly by the DSC compression ratio, which is
> >    'drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component).
> >
> > So, this piece of code needs to be adjusted to check for the widebus
> > being enabled or not.
> >
> 
> The widebus adjustment on the MDP/INTF side is already in
> dpu_hw_intf_setup_timing_engine: the "data width" is divided by 2 for
> 48-bit widebus instead of 24-bit. there shouldn't be any other
> adjustment (downstream doesn't have any other adjustment).
> 
> a relevant downstream comment: "In DATABUS-WIDEN mode, MDP always sends
> out 48-bit compressed data per pclk and on average, DSI consumes an
> amount of compressed data equivalent to the uncompressed pixel depth per
> pclk."
> 
> Based on that comment, this patch is correct, and the
> ''drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component)' adjustment
> only applies to DSI. 

If I keep the INTF side at /24 and change the DSI side to:

if (wide_bus)
        new_hdisplay = DIV_ROUND_UP(mode->hdisplay * drm_dsc_get_bpp_int(dsc), dsc->bits_per_component * 3);
else
        new_hdisplay = DIV_ROUND_UP(mode->hdisplay * drm_dsc_get_bpp_int(dsc), 24);

This also works on my panel.

Should I send this in a v4 for this series or just leave it for a seperate
change as panel seems to work with /24 here anyways?

> (note: newer downstream looks like it would divide
> by 3.75 here, which doesn't make sense. older downstream would divide by
> 3 here. I guess downstream is broken now and video mode + 10-bit dsc
> doesn't get tested?)
> 
> on DSI side, "uncompressed pixel depth" shouldn't matter either: DSI
> only sees the compressed data. But based on that comment, when widebus
> is enabled, by setting DSI_VID_CFG0_DST_FORMAT(?) to 30bpp, then the DSI
> pclk is in 30-bit units instead of 24-bits. And with this series DSI
> side ends up with the right result if 30bpp format and widebus is enabled.
> 
> 

Thanks,
Alex

Re: [PATCH v3 4/4] drm/msm/dpu: fix video mode DSC INTF timing width calculation

[Dmitry Baryshkov]

On Fri, Mar 20, 2026 at 04:46:02AM +0000, Alexander Koskovich wrote:
> On Friday, March 20th, 2026 at 12:25 AM, Jonathan Marek <jonathan@marek.ca> wrote:
> 
> > On 3/19/26 9:45 PM, Dmitry Baryshkov wrote:
> > > On Thu, Mar 19, 2026 at 01:23:03PM -0400, Jonathan Marek wrote:
> > ...
> > >>
> > >> That's not how it works. INTF (which feeds DSI) is after DSC compression.
> > >>
> > >> INTF timings are always in RGB888 (24-bit) units. Ignoring widebus details,
> > >> the INTF timing should match what is programmed on the DSI side (hdisplay,
> > >> which is calculated as bytes per line / 3).
> > >>
> > >> (fwiw, the current "timing->width = ..." calculation here blames to me, but
> > >> what I wrote originally was just "timing->width = timing->width / 3" with a
> > >> comment about being incomplete.)
> > >>
> > > Okay. After reading the docs (sorry, it took a while).
> > >
> > > - When widebus is not enabled, the transfer is always 24 bit of
> > >    compressed data. Thus if it is not in play, pclk and timing->width
> > >    should be scaled by source_pixel_depth / compression_ratio / 24. In
> > >    case of the code it is 'drm_dsc_get_bpp_int(dsc) / 24'.
> > >
> > >    For RGB101010 / 8bpp DSC this should result in the PCLK being lowered
> > >    by the factor of 3 (= 24 / (30 / 3.75))
> > >
> > > - When widebus is in play (MDSS 6.x+, DSI 2.4+), the transfer takes
> > >    more than 24 bits. In this case the PCLK and timing->width should be
> > >    scaled exactly by the DSC compression ratio, which is
> > >    'drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component).
> > >
> > > So, this piece of code needs to be adjusted to check for the widebus
> > > being enabled or not.
> > >
> > 
> > The widebus adjustment on the MDP/INTF side is already in
> > dpu_hw_intf_setup_timing_engine: the "data width" is divided by 2 for
> > 48-bit widebus instead of 24-bit. there shouldn't be any other
> > adjustment (downstream doesn't have any other adjustment).
> > 
> > a relevant downstream comment: "In DATABUS-WIDEN mode, MDP always sends
> > out 48-bit compressed data per pclk and on average, DSI consumes an
> > amount of compressed data equivalent to the uncompressed pixel depth per
> > pclk."
> > 
> > Based on that comment, this patch is correct, and the
> > ''drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component)' adjustment
> > only applies to DSI. 
> 
> If I keep the INTF side at /24 and change the DSI side to:
> 
> if (wide_bus)
>         new_hdisplay = DIV_ROUND_UP(mode->hdisplay * drm_dsc_get_bpp_int(dsc), dsc->bits_per_component * 3);
> else
>         new_hdisplay = DIV_ROUND_UP(mode->hdisplay * drm_dsc_get_bpp_int(dsc), 24);

Please check the actual fps (I'm usually using a vblank-synced GL, e.g.
kmscube).

At least this matches my expectations.

> 
> This also works on my panel.
> 
> Should I send this in a v4 for this series or just leave it for a seperate
> change as panel seems to work with /24 here anyways?
> 
> > (note: newer downstream looks like it would divide
> > by 3.75 here, which doesn't make sense. older downstream would divide by
> > 3 here. I guess downstream is broken now and video mode + 10-bit dsc
> > doesn't get tested?)
> > 
> > on DSI side, "uncompressed pixel depth" shouldn't matter either: DSI
> > only sees the compressed data. But based on that comment, when widebus
> > is enabled, by setting DSI_VID_CFG0_DST_FORMAT(?) to 30bpp, then the DSI
> > pclk is in 30-bit units instead of 24-bits. And with this series DSI
> > side ends up with the right result if 30bpp format and widebus is enabled.
> > 
> > 
> 
> Thanks,
> Alex

-- 
With best wishes
Dmitry

Re: [PATCH v3 4/4] drm/msm/dpu: fix video mode DSC INTF timing width calculation

[Alexander Koskovich]

On Friday, March 20th, 2026 at 2:38 AM, Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> wrote:

> On Fri, Mar 20, 2026 at 04:46:02AM +0000, Alexander Koskovich wrote:
> > On Friday, March 20th, 2026 at 12:25 AM, Jonathan Marek <jonathan@marek.ca> wrote:
> >
> > > On 3/19/26 9:45 PM, Dmitry Baryshkov wrote:
> > > > On Thu, Mar 19, 2026 at 01:23:03PM -0400, Jonathan Marek wrote:
> > > ...
> > > >>
> > > >> That's not how it works. INTF (which feeds DSI) is after DSC compression.
> > > >>
> > > >> INTF timings are always in RGB888 (24-bit) units. Ignoring widebus details,
> > > >> the INTF timing should match what is programmed on the DSI side (hdisplay,
> > > >> which is calculated as bytes per line / 3).
> > > >>
> > > >> (fwiw, the current "timing->width = ..." calculation here blames to me, but
> > > >> what I wrote originally was just "timing->width = timing->width / 3" with a
> > > >> comment about being incomplete.)
> > > >>
> > > > Okay. After reading the docs (sorry, it took a while).
> > > >
> > > > - When widebus is not enabled, the transfer is always 24 bit of
> > > >    compressed data. Thus if it is not in play, pclk and timing->width
> > > >    should be scaled by source_pixel_depth / compression_ratio / 24. In
> > > >    case of the code it is 'drm_dsc_get_bpp_int(dsc) / 24'.
> > > >
> > > >    For RGB101010 / 8bpp DSC this should result in the PCLK being lowered
> > > >    by the factor of 3 (= 24 / (30 / 3.75))
> > > >
> > > > - When widebus is in play (MDSS 6.x+, DSI 2.4+), the transfer takes
> > > >    more than 24 bits. In this case the PCLK and timing->width should be
> > > >    scaled exactly by the DSC compression ratio, which is
> > > >    'drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component).
> > > >
> > > > So, this piece of code needs to be adjusted to check for the widebus
> > > > being enabled or not.
> > > >
> > >
> > > The widebus adjustment on the MDP/INTF side is already in
> > > dpu_hw_intf_setup_timing_engine: the "data width" is divided by 2 for
> > > 48-bit widebus instead of 24-bit. there shouldn't be any other
> > > adjustment (downstream doesn't have any other adjustment).
> > >
> > > a relevant downstream comment: "In DATABUS-WIDEN mode, MDP always sends
> > > out 48-bit compressed data per pclk and on average, DSI consumes an
> > > amount of compressed data equivalent to the uncompressed pixel depth per
> > > pclk."
> > >
> > > Based on that comment, this patch is correct, and the
> > > ''drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component)' adjustment
> > > only applies to DSI.
> >
> > If I keep the INTF side at /24 and change the DSI side to:
> >
> > if (wide_bus)
> >         new_hdisplay = DIV_ROUND_UP(mode->hdisplay * drm_dsc_get_bpp_int(dsc), dsc->bits_per_component * 3);
> > else
> >         new_hdisplay = DIV_ROUND_UP(mode->hdisplay * drm_dsc_get_bpp_int(dsc), 24);
> 
> Please check the actual fps (I'm usually using a vblank-synced GL, e.g.
> kmscube).
> 
> At least this matches my expectations.

Hmmm with kmscube I am getting 120FPS with 24 and 100FPS with 30. So I guess that's a no go

> 
> >
> > This also works on my panel.
> >
> > Should I send this in a v4 for this series or just leave it for a seperate
> > change as panel seems to work with /24 here anyways?
> >
> > > (note: newer downstream looks like it would divide
> > > by 3.75 here, which doesn't make sense. older downstream would divide by
> > > 3 here. I guess downstream is broken now and video mode + 10-bit dsc
> > > doesn't get tested?)
> > >
> > > on DSI side, "uncompressed pixel depth" shouldn't matter either: DSI
> > > only sees the compressed data. But based on that comment, when widebus
> > > is enabled, by setting DSI_VID_CFG0_DST_FORMAT(?) to 30bpp, then the DSI
> > > pclk is in 30-bit units instead of 24-bits. And with this series DSI
> > > side ends up with the right result if 30bpp format and widebus is enabled.
> > >
> > >
> >
> > Thanks,
> > Alex
> 
> --
> With best wishes
> Dmitry
>

Re: [PATCH v3 4/4] drm/msm/dpu: fix video mode DSC INTF timing width calculation

[Alexander Koskovich]

On Friday, March 20th, 2026 at 2:50 AM, Alexander Koskovich <akoskovich@pm.me> wrote:

> On Friday, March 20th, 2026 at 2:38 AM, Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> wrote:
> 
> > On Fri, Mar 20, 2026 at 04:46:02AM +0000, Alexander Koskovich wrote:
> > > On Friday, March 20th, 2026 at 12:25 AM, Jonathan Marek <jonathan@marek.ca> wrote:
> > >
> > > > On 3/19/26 9:45 PM, Dmitry Baryshkov wrote:
> > > > > On Thu, Mar 19, 2026 at 01:23:03PM -0400, Jonathan Marek wrote:
> > > > ...
> > > > >>
> > > > >> That's not how it works. INTF (which feeds DSI) is after DSC compression.
> > > > >>
> > > > >> INTF timings are always in RGB888 (24-bit) units. Ignoring widebus details,
> > > > >> the INTF timing should match what is programmed on the DSI side (hdisplay,
> > > > >> which is calculated as bytes per line / 3).
> > > > >>
> > > > >> (fwiw, the current "timing->width = ..." calculation here blames to me, but
> > > > >> what I wrote originally was just "timing->width = timing->width / 3" with a
> > > > >> comment about being incomplete.)
> > > > >>
> > > > > Okay. After reading the docs (sorry, it took a while).
> > > > >
> > > > > - When widebus is not enabled, the transfer is always 24 bit of
> > > > >    compressed data. Thus if it is not in play, pclk and timing->width
> > > > >    should be scaled by source_pixel_depth / compression_ratio / 24. In
> > > > >    case of the code it is 'drm_dsc_get_bpp_int(dsc) / 24'.
> > > > >
> > > > >    For RGB101010 / 8bpp DSC this should result in the PCLK being lowered
> > > > >    by the factor of 3 (= 24 / (30 / 3.75))
> > > > >
> > > > > - When widebus is in play (MDSS 6.x+, DSI 2.4+), the transfer takes
> > > > >    more than 24 bits. In this case the PCLK and timing->width should be
> > > > >    scaled exactly by the DSC compression ratio, which is
> > > > >    'drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component).
> > > > >
> > > > > So, this piece of code needs to be adjusted to check for the widebus
> > > > > being enabled or not.
> > > > >
> > > >
> > > > The widebus adjustment on the MDP/INTF side is already in
> > > > dpu_hw_intf_setup_timing_engine: the "data width" is divided by 2 for
> > > > 48-bit widebus instead of 24-bit. there shouldn't be any other
> > > > adjustment (downstream doesn't have any other adjustment).
> > > >
> > > > a relevant downstream comment: "In DATABUS-WIDEN mode, MDP always sends
> > > > out 48-bit compressed data per pclk and on average, DSI consumes an
> > > > amount of compressed data equivalent to the uncompressed pixel depth per
> > > > pclk."
> > > >
> > > > Based on that comment, this patch is correct, and the
> > > > ''drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component)' adjustment
> > > > only applies to DSI.
> > >
> > > If I keep the INTF side at /24 and change the DSI side to:
> > >
> > > if (wide_bus)
> > >         new_hdisplay = DIV_ROUND_UP(mode->hdisplay * drm_dsc_get_bpp_int(dsc), dsc->bits_per_component * 3);
> > > else
> > >         new_hdisplay = DIV_ROUND_UP(mode->hdisplay * drm_dsc_get_bpp_int(dsc), 24);
> >
> > Please check the actual fps (I'm usually using a vblank-synced GL, e.g.
> > kmscube).
> >
> > At least this matches my expectations.
> 
> Hmmm with kmscube I am getting 120FPS with 24 and 100FPS with 30. So I guess that's a no go

Although it was using dsc->bits_per_component * 3 regardless before for
dsi_adjust_pclk_for_compression so I guess that's what Jonathan was
referring to earlier...

> 
> >
> > >
> > > This also works on my panel.
> > >
> > > Should I send this in a v4 for this series or just leave it for a seperate
> > > change as panel seems to work with /24 here anyways?
> > >
> > > > (note: newer downstream looks like it would divide
> > > > by 3.75 here, which doesn't make sense. older downstream would divide by
> > > > 3 here. I guess downstream is broken now and video mode + 10-bit dsc
> > > > doesn't get tested?)
> > > >
> > > > on DSI side, "uncompressed pixel depth" shouldn't matter either: DSI
> > > > only sees the compressed data. But based on that comment, when widebus
> > > > is enabled, by setting DSI_VID_CFG0_DST_FORMAT(?) to 30bpp, then the DSI
> > > > pclk is in 30-bit units instead of 24-bits. And with this series DSI
> > > > side ends up with the right result if 30bpp format and widebus is enabled.
> > > >
> > > >
> > >
> > > Thanks,
> > > Alex
> >
> > --
> > With best wishes
> > Dmitry
> >
> 
>

Re: [PATCH v3 4/4] drm/msm/dpu: fix video mode DSC INTF timing width calculation

[Dmitry Baryshkov]

On Fri, Mar 20, 2026 at 07:02:54AM +0000, Alexander Koskovich wrote:
> On Friday, March 20th, 2026 at 2:50 AM, Alexander Koskovich <akoskovich@pm.me> wrote:
> 
> > On Friday, March 20th, 2026 at 2:38 AM, Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> wrote:
> > 
> > > On Fri, Mar 20, 2026 at 04:46:02AM +0000, Alexander Koskovich wrote:
> > > > On Friday, March 20th, 2026 at 12:25 AM, Jonathan Marek <jonathan@marek.ca> wrote:
> > > >
> > > > > On 3/19/26 9:45 PM, Dmitry Baryshkov wrote:
> > > > > > On Thu, Mar 19, 2026 at 01:23:03PM -0400, Jonathan Marek wrote:
> > > > > ...
> > > > > >>
> > > > > >> That's not how it works. INTF (which feeds DSI) is after DSC compression.
> > > > > >>
> > > > > >> INTF timings are always in RGB888 (24-bit) units. Ignoring widebus details,
> > > > > >> the INTF timing should match what is programmed on the DSI side (hdisplay,
> > > > > >> which is calculated as bytes per line / 3).
> > > > > >>
> > > > > >> (fwiw, the current "timing->width = ..." calculation here blames to me, but
> > > > > >> what I wrote originally was just "timing->width = timing->width / 3" with a
> > > > > >> comment about being incomplete.)
> > > > > >>
> > > > > > Okay. After reading the docs (sorry, it took a while).
> > > > > >
> > > > > > - When widebus is not enabled, the transfer is always 24 bit of
> > > > > >    compressed data. Thus if it is not in play, pclk and timing->width
> > > > > >    should be scaled by source_pixel_depth / compression_ratio / 24. In
> > > > > >    case of the code it is 'drm_dsc_get_bpp_int(dsc) / 24'.
> > > > > >
> > > > > >    For RGB101010 / 8bpp DSC this should result in the PCLK being lowered
> > > > > >    by the factor of 3 (= 24 / (30 / 3.75))
> > > > > >
> > > > > > - When widebus is in play (MDSS 6.x+, DSI 2.4+), the transfer takes
> > > > > >    more than 24 bits. In this case the PCLK and timing->width should be
> > > > > >    scaled exactly by the DSC compression ratio, which is
> > > > > >    'drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component).
> > > > > >
> > > > > > So, this piece of code needs to be adjusted to check for the widebus
> > > > > > being enabled or not.
> > > > > >
> > > > >
> > > > > The widebus adjustment on the MDP/INTF side is already in
> > > > > dpu_hw_intf_setup_timing_engine: the "data width" is divided by 2 for
> > > > > 48-bit widebus instead of 24-bit. there shouldn't be any other
> > > > > adjustment (downstream doesn't have any other adjustment).
> > > > >
> > > > > a relevant downstream comment: "In DATABUS-WIDEN mode, MDP always sends
> > > > > out 48-bit compressed data per pclk and on average, DSI consumes an
> > > > > amount of compressed data equivalent to the uncompressed pixel depth per
> > > > > pclk."
> > > > >
> > > > > Based on that comment, this patch is correct, and the
> > > > > ''drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component)' adjustment
> > > > > only applies to DSI.
> > > >
> > > > If I keep the INTF side at /24 and change the DSI side to:
> > > >
> > > > if (wide_bus)
> > > >         new_hdisplay = DIV_ROUND_UP(mode->hdisplay * drm_dsc_get_bpp_int(dsc), dsc->bits_per_component * 3);
> > > > else
> > > >         new_hdisplay = DIV_ROUND_UP(mode->hdisplay * drm_dsc_get_bpp_int(dsc), 24);
> > >
> > > Please check the actual fps (I'm usually using a vblank-synced GL, e.g.
> > > kmscube).
> > >
> > > At least this matches my expectations.
> > 
> > Hmmm with kmscube I am getting 120FPS with 24 and 100FPS with 30. So I guess that's a no go
> 
> Although it was using dsc->bits_per_component * 3 regardless before for
> dsi_adjust_pclk_for_compression so I guess that's what Jonathan was
> referring to earlier...

Do you have any of the patches by Marijn or Pengyu?

- https://lore.kernel.org/linux-arm-msm/20260311-dsi-dsc-regresses-again-v1-1-6a422141eeea@somainline.org/

- https://lore.kernel.org/linux-arm-msm/20260307111250.105772-1-mitltlatltl@gmail.com/

-- 
With best wishes
Dmitry

Re: [PATCH v3 4/4] drm/msm/dpu: fix video mode DSC INTF timing width calculation

[Alexander Koskovich]

On Friday, March 20th, 2026 at 3:36 AM, Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> wrote:

> On Fri, Mar 20, 2026 at 07:02:54AM +0000, Alexander Koskovich wrote:
> > On Friday, March 20th, 2026 at 2:50 AM, Alexander Koskovich <akoskovich@pm.me> wrote:
> >
> > > On Friday, March 20th, 2026 at 2:38 AM, Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> wrote:
> > >
> > > > On Fri, Mar 20, 2026 at 04:46:02AM +0000, Alexander Koskovich wrote:
> > > > > On Friday, March 20th, 2026 at 12:25 AM, Jonathan Marek <jonathan@marek.ca> wrote:
> > > > >
> > > > > > On 3/19/26 9:45 PM, Dmitry Baryshkov wrote:
> > > > > > > On Thu, Mar 19, 2026 at 01:23:03PM -0400, Jonathan Marek wrote:
> > > > > > ...
> > > > > > >>
> > > > > > >> That's not how it works. INTF (which feeds DSI) is after DSC compression.
> > > > > > >>
> > > > > > >> INTF timings are always in RGB888 (24-bit) units. Ignoring widebus details,
> > > > > > >> the INTF timing should match what is programmed on the DSI side (hdisplay,
> > > > > > >> which is calculated as bytes per line / 3).
> > > > > > >>
> > > > > > >> (fwiw, the current "timing->width = ..." calculation here blames to me, but
> > > > > > >> what I wrote originally was just "timing->width = timing->width / 3" with a
> > > > > > >> comment about being incomplete.)
> > > > > > >>
> > > > > > > Okay. After reading the docs (sorry, it took a while).
> > > > > > >
> > > > > > > - When widebus is not enabled, the transfer is always 24 bit of
> > > > > > >    compressed data. Thus if it is not in play, pclk and timing->width
> > > > > > >    should be scaled by source_pixel_depth / compression_ratio / 24. In
> > > > > > >    case of the code it is 'drm_dsc_get_bpp_int(dsc) / 24'.
> > > > > > >
> > > > > > >    For RGB101010 / 8bpp DSC this should result in the PCLK being lowered
> > > > > > >    by the factor of 3 (= 24 / (30 / 3.75))
> > > > > > >
> > > > > > > - When widebus is in play (MDSS 6.x+, DSI 2.4+), the transfer takes
> > > > > > >    more than 24 bits. In this case the PCLK and timing->width should be
> > > > > > >    scaled exactly by the DSC compression ratio, which is
> > > > > > >    'drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component).
> > > > > > >
> > > > > > > So, this piece of code needs to be adjusted to check for the widebus
> > > > > > > being enabled or not.
> > > > > > >
> > > > > >
> > > > > > The widebus adjustment on the MDP/INTF side is already in
> > > > > > dpu_hw_intf_setup_timing_engine: the "data width" is divided by 2 for
> > > > > > 48-bit widebus instead of 24-bit. there shouldn't be any other
> > > > > > adjustment (downstream doesn't have any other adjustment).
> > > > > >
> > > > > > a relevant downstream comment: "In DATABUS-WIDEN mode, MDP always sends
> > > > > > out 48-bit compressed data per pclk and on average, DSI consumes an
> > > > > > amount of compressed data equivalent to the uncompressed pixel depth per
> > > > > > pclk."
> > > > > >
> > > > > > Based on that comment, this patch is correct, and the
> > > > > > ''drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component)' adjustment
> > > > > > only applies to DSI.
> > > > >
> > > > > If I keep the INTF side at /24 and change the DSI side to:
> > > > >
> > > > > if (wide_bus)
> > > > >         new_hdisplay = DIV_ROUND_UP(mode->hdisplay * drm_dsc_get_bpp_int(dsc), dsc->bits_per_component * 3);
> > > > > else
> > > > >         new_hdisplay = DIV_ROUND_UP(mode->hdisplay * drm_dsc_get_bpp_int(dsc), 24);
> > > >
> > > > Please check the actual fps (I'm usually using a vblank-synced GL, e.g.
> > > > kmscube).
> > > >
> > > > At least this matches my expectations.
> > >
> > > Hmmm with kmscube I am getting 120FPS with 24 and 100FPS with 30. So I guess that's a no go
> >
> > Although it was using dsc->bits_per_component * 3 regardless before for
> > dsi_adjust_pclk_for_compression so I guess that's what Jonathan was
> > referring to earlier...
> 
> Do you have any of the patches by Marijn or Pengyu?
> 
> - https://lore.kernel.org/linux-arm-msm/20260311-dsi-dsc-regresses-again-v1-1-6a422141eeea@somainline.org/
> 
> - https://lore.kernel.org/linux-arm-msm/20260307111250.105772-1-mitltlatltl@gmail.com/

Nope, I can test with them though if you'd like

> 
> --
> With best wishes
> Dmitry
>

Re: [PATCH v3 4/4] drm/msm/dpu: fix video mode DSC INTF timing width calculation

[Alexander Koskovich]

On Friday, March 20th, 2026 at 3:48 AM, Alexander Koskovich <akoskovich@pm.me> wrote:

> 
> On Friday, March 20th, 2026 at 3:36 AM, Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> wrote:
> 
> > On Fri, Mar 20, 2026 at 07:02:54AM +0000, Alexander Koskovich wrote:
> > > On Friday, March 20th, 2026 at 2:50 AM, Alexander Koskovich <akoskovich@pm.me> wrote:
> > >
> > > > On Friday, March 20th, 2026 at 2:38 AM, Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> wrote:
> > > >
> > > > > On Fri, Mar 20, 2026 at 04:46:02AM +0000, Alexander Koskovich wrote:
> > > > > > On Friday, March 20th, 2026 at 12:25 AM, Jonathan Marek <jonathan@marek.ca> wrote:
> > > > > >
> > > > > > > On 3/19/26 9:45 PM, Dmitry Baryshkov wrote:
> > > > > > > > On Thu, Mar 19, 2026 at 01:23:03PM -0400, Jonathan Marek wrote:
> > > > > > > ...
> > > > > > > >>
> > > > > > > >> That's not how it works. INTF (which feeds DSI) is after DSC compression.
> > > > > > > >>
> > > > > > > >> INTF timings are always in RGB888 (24-bit) units. Ignoring widebus details,
> > > > > > > >> the INTF timing should match what is programmed on the DSI side (hdisplay,
> > > > > > > >> which is calculated as bytes per line / 3).
> > > > > > > >>
> > > > > > > >> (fwiw, the current "timing->width = ..." calculation here blames to me, but
> > > > > > > >> what I wrote originally was just "timing->width = timing->width / 3" with a
> > > > > > > >> comment about being incomplete.)
> > > > > > > >>
> > > > > > > > Okay. After reading the docs (sorry, it took a while).
> > > > > > > >
> > > > > > > > - When widebus is not enabled, the transfer is always 24 bit of
> > > > > > > >    compressed data. Thus if it is not in play, pclk and timing->width
> > > > > > > >    should be scaled by source_pixel_depth / compression_ratio / 24. In
> > > > > > > >    case of the code it is 'drm_dsc_get_bpp_int(dsc) / 24'.
> > > > > > > >
> > > > > > > >    For RGB101010 / 8bpp DSC this should result in the PCLK being lowered
> > > > > > > >    by the factor of 3 (= 24 / (30 / 3.75))
> > > > > > > >
> > > > > > > > - When widebus is in play (MDSS 6.x+, DSI 2.4+), the transfer takes
> > > > > > > >    more than 24 bits. In this case the PCLK and timing->width should be
> > > > > > > >    scaled exactly by the DSC compression ratio, which is
> > > > > > > >    'drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component).
> > > > > > > >
> > > > > > > > So, this piece of code needs to be adjusted to check for the widebus
> > > > > > > > being enabled or not.
> > > > > > > >
> > > > > > >
> > > > > > > The widebus adjustment on the MDP/INTF side is already in
> > > > > > > dpu_hw_intf_setup_timing_engine: the "data width" is divided by 2 for
> > > > > > > 48-bit widebus instead of 24-bit. there shouldn't be any other
> > > > > > > adjustment (downstream doesn't have any other adjustment).
> > > > > > >
> > > > > > > a relevant downstream comment: "In DATABUS-WIDEN mode, MDP always sends
> > > > > > > out 48-bit compressed data per pclk and on average, DSI consumes an
> > > > > > > amount of compressed data equivalent to the uncompressed pixel depth per
> > > > > > > pclk."
> > > > > > >
> > > > > > > Based on that comment, this patch is correct, and the
> > > > > > > ''drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component)' adjustment
> > > > > > > only applies to DSI.
> > > > > >
> > > > > > If I keep the INTF side at /24 and change the DSI side to:
> > > > > >
> > > > > > if (wide_bus)
> > > > > >         new_hdisplay = DIV_ROUND_UP(mode->hdisplay * drm_dsc_get_bpp_int(dsc), dsc->bits_per_component * 3);
> > > > > > else
> > > > > >         new_hdisplay = DIV_ROUND_UP(mode->hdisplay * drm_dsc_get_bpp_int(dsc), 24);
> > > > >
> > > > > Please check the actual fps (I'm usually using a vblank-synced GL, e.g.
> > > > > kmscube).
> > > > >
> > > > > At least this matches my expectations.
> > > >
> > > > Hmmm with kmscube I am getting 120FPS with 24 and 100FPS with 30. So I guess that's a no go
> > >
> > > Although it was using dsc->bits_per_component * 3 regardless before for
> > > dsi_adjust_pclk_for_compression so I guess that's what Jonathan was
> > > referring to earlier...
> >
> > Do you have any of the patches by Marijn or Pengyu?
> >
> > - https://lore.kernel.org/linux-arm-msm/20260311-dsi-dsc-regresses-again-v1-1-6a422141eeea@somainline.org/
> >
> > - https://lore.kernel.org/linux-arm-msm/20260307111250.105772-1-mitltlatltl@gmail.com/
> 
> Nope, I can test with them though if you'd like

Tested the following 3 patches on top of this series:
drm/msm/dsi: fix hdisplay calculation when programming dsi registers
drm/msm/dsi: fix bits_per_pclk
drm/msm/dsi: fix hdisplay calculation for CMD mode panel

Getting constant FIFO errors with them applied:
[   64.851635] dsi_err_worker: status=4
[   64.851692] dsi_err_worker: status=4
[   64.851730] dsi_err_worker: status=4
[   64.851766] dsi_err_worker: status=4
[   64.851802] dsi_err_worker: status=4
[   64.851837] dsi_err_worker: status=4
[   64.851903] dsi_err_worker: status=4
[   64.851940] dsi_err_worker: status=4
[   64.851976] dsi_err_worker: status=4
[   64.852011] dsi_err_worker: status=4


> 
> >
> > --
> > With best wishes
> > Dmitry
> >
> 
>

Re: [PATCH v3 4/4] drm/msm/dpu: fix video mode DSC INTF timing width calculation

[Dmitry Baryshkov]

On Fri, Mar 20, 2026 at 12:25:00AM -0400, Jonathan Marek wrote:
> On 3/19/26 9:45 PM, Dmitry Baryshkov wrote:
> > On Thu, Mar 19, 2026 at 01:23:03PM -0400, Jonathan Marek wrote:
> ...
> > > 
> > > That's not how it works. INTF (which feeds DSI) is after DSC compression.
> > > 
> > > INTF timings are always in RGB888 (24-bit) units. Ignoring widebus details,
> > > the INTF timing should match what is programmed on the DSI side (hdisplay,
> > > which is calculated as bytes per line / 3).
> > > 
> > > (fwiw, the current "timing->width = ..." calculation here blames to me, but
> > > what I wrote originally was just "timing->width = timing->width / 3" with a
> > > comment about being incomplete.)
> > > 
> > Okay. After reading the docs (sorry, it took a while).
> > 
> > - When widebus is not enabled, the transfer is always 24 bit of
> >    compressed data. Thus if it is not in play, pclk and timing->width
> >    should be scaled by source_pixel_depth / compression_ratio / 24. In
> >    case of the code it is 'drm_dsc_get_bpp_int(dsc) / 24'.
> > 
> >    For RGB101010 / 8bpp DSC this should result in the PCLK being lowered
> >    by the factor of 3 (= 24 / (30 / 3.75))
> > 
> > - When widebus is in play (MDSS 6.x+, DSI 2.4+), the transfer takes
> >    more than 24 bits. In this case the PCLK and timing->width should be
> >    scaled exactly by the DSC compression ratio, which is
> >    'drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component).
> > 
> > So, this piece of code needs to be adjusted to check for the widebus
> > being enabled or not.
> > 
> 
> The widebus adjustment on the MDP/INTF side is already in
> dpu_hw_intf_setup_timing_engine: the "data width" is divided by 2 for 48-bit
> widebus instead of 24-bit. there shouldn't be any other adjustment
> (downstream doesn't have any other adjustment).
> 
> a relevant downstream comment: "In DATABUS-WIDEN mode, MDP always sends out
> 48-bit compressed data per pclk and on average, DSI consumes an amount of
> compressed data equivalent to the uncompressed pixel depth per pclk."
> 
> Based on that comment, this patch is correct, and the
> ''drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component)' adjustment only
> applies to DSI. (note: newer downstream looks like it would divide by 3.75
> here, which doesn't make sense. older downstream would divide by 3 here. I
> guess downstream is broken now and video mode + 10-bit dsc doesn't get
> tested?)

I guess, the downstream might be broken wrt. the widebus being enabled
or not.

> 
> on DSI side, "uncompressed pixel depth" shouldn't matter either: DSI only
> sees the compressed data. But based on that comment, when widebus is
> enabled, by setting DSI_VID_CFG0_DST_FORMAT(?) to 30bpp, then the DSI pclk
> is in 30-bit units instead of 24-bits. And with this series DSI side ends up
> with the right result if 30bpp format and widebus is enabled.

-- 
With best wishes
Dmitry

Re: [PATCH v3 4/4] drm/msm/dpu: fix video mode DSC INTF timing width calculation

[Dmitry Baryshkov]

On Fri, Mar 20, 2026 at 12:25:00AM -0400, Jonathan Marek wrote:
> On 3/19/26 9:45 PM, Dmitry Baryshkov wrote:
> > On Thu, Mar 19, 2026 at 01:23:03PM -0400, Jonathan Marek wrote:
> ...
> > > 
> > > That's not how it works. INTF (which feeds DSI) is after DSC compression.
> > > 
> > > INTF timings are always in RGB888 (24-bit) units. Ignoring widebus details,
> > > the INTF timing should match what is programmed on the DSI side (hdisplay,
> > > which is calculated as bytes per line / 3).
> > > 
> > > (fwiw, the current "timing->width = ..." calculation here blames to me, but
> > > what I wrote originally was just "timing->width = timing->width / 3" with a
> > > comment about being incomplete.)
> > > 
> > Okay. After reading the docs (sorry, it took a while).
> > 
> > - When widebus is not enabled, the transfer is always 24 bit of
> >    compressed data. Thus if it is not in play, pclk and timing->width
> >    should be scaled by source_pixel_depth / compression_ratio / 24. In
> >    case of the code it is 'drm_dsc_get_bpp_int(dsc) / 24'.
> > 
> >    For RGB101010 / 8bpp DSC this should result in the PCLK being lowered
> >    by the factor of 3 (= 24 / (30 / 3.75))
> > 
> > - When widebus is in play (MDSS 6.x+, DSI 2.4+), the transfer takes
> >    more than 24 bits. In this case the PCLK and timing->width should be
> >    scaled exactly by the DSC compression ratio, which is
> >    'drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component).
> > 
> > So, this piece of code needs to be adjusted to check for the widebus
> > being enabled or not.
> > 
> 
> The widebus adjustment on the MDP/INTF side is already in
> dpu_hw_intf_setup_timing_engine: the "data width" is divided by 2 for 48-bit
> widebus instead of 24-bit. there shouldn't be any other adjustment
> (downstream doesn't have any other adjustment).
> 
> a relevant downstream comment: "In DATABUS-WIDEN mode, MDP always sends out
> 48-bit compressed data per pclk and on average, DSI consumes an amount of
> compressed data equivalent to the uncompressed pixel depth per pclk."
> 
> Based on that comment, this patch is correct, and the
> ''drm_dsc_get_bpp_int(dsc) / (3 * dsc->bits_per_component)' adjustment only
> applies to DSI. (note: newer downstream looks like it would divide by 3.75
> here, which doesn't make sense. older downstream would divide by 3 here. I
> guess downstream is broken now and video mode + 10-bit dsc doesn't get
> tested?)
> 

For the reference, the commit in question is [1].

> on DSI side, "uncompressed pixel depth" shouldn't matter either: DSI only
> sees the compressed data. But based on that comment, when widebus is
> enabled, by setting DSI_VID_CFG0_DST_FORMAT(?) to 30bpp, then the DSI pclk
> is in 30-bit units instead of 24-bits. And with this series DSI side ends up
> with the right result if 30bpp format and widebus is enabled.

Nevertheless, this matches the docs that I'm looking at.

[1] https://git.codelinaro.org/clo/la/platform/vendor/opensource/display-drivers/-/commit/7b4616f157e67e593fae13684237f96da351e877

-- 
With best wishes
Dmitry

Re: [PATCH v3 4/4] drm/msm/dpu: fix video mode DSC INTF timing width calculation

[Dmitry Baryshkov]

On Thu, Mar 19, 2026 at 03:09:13PM +0100, Neil Armstrong wrote:
> Hi,
> 
> On 3/19/26 12:58, Alexander Koskovich wrote:
> > Using bits_per_component * 3 as the divisor for the compressed INTF
> > timing width produces constant FIFO errors for the BOE BF068MWM-TD0
> > panel due to bits_per_component being 10 which results in a divisor
> > of 30 instead of 24.
> > 
> > Regardless of the compression ratio and pixel depth, 24 bits of
> > compressed data are transferred per pclk, so the divisor should
> > always be 24.
> 
> Not true with widebus, specify why 24 and because DSI widebus is not implemented yet.

Support for the widebus is implemented and enable for DSI >= 2.5.0

> 
> > 
> > Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
> > ---
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 9 ++++-----
> >   1 file changed, 4 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> > index 0ba777bda253..5419ef0be137 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> > @@ -122,19 +122,18 @@ static void drm_mode_to_intf_timing_params(
> >   	}
> >   	/*
> > -	 * for DSI, if compression is enabled, then divide the horizonal active
> > -	 * timing parameters by compression ratio. bits of 3 components(R/G/B)
> > -	 * is compressed into bits of 1 pixel.
> > +	 * For DSI, if DSC is enabled, 24 bits of compressed data are
> > +	 * transferred per pclk regardless of the source pixel depth.
> >   	 */
> >   	if (phys_enc->hw_intf->cap->type != INTF_DP && timing->compression_en) {
> >   		struct drm_dsc_config *dsc =
> >   		       dpu_encoder_get_dsc_config(phys_enc->parent);
> > +
> Drop this change
> 
> >   		/*
> >   		 * TODO: replace drm_dsc_get_bpp_int with logic to handle
> >   		 * fractional part if there is fraction
> >   		 */
> > -		timing->width = timing->width * drm_dsc_get_bpp_int(dsc) /
> > -				(dsc->bits_per_component * 3);
> > +		timing->width = timing->width * drm_dsc_get_bpp_int(dsc) / 24;
> 
> It would be helpful to somehow show that 24 is 8 * 3, 8 being the byte width and 3 the compression ratio.

Otherwise I'd have assumed that it is bus width.

> 
> >   		timing->xres = timing->width;
> >   	}
> >   }
> > 
> 

-- 
With best wishes
Dmitry