* [PATCH v2 0/3] drm/msm: add RGB101010 pixel format and fix 10-bit DSC timing
@ 2026-03-19 3:59 Alexander Koskovich
2026-03-19 4:00 ` [PATCH v2 1/3] drm/mipi-dsi: add RGB101010 pixel format Alexander Koskovich
` (3 more replies)
0 siblings, 4 replies; 16+ messages in thread
From: Alexander Koskovich @ 2026-03-19 3:59 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten
Cc: dri-devel, linux-kernel, linux-arm-msm, freedreno,
Alexander Koskovich, Dmitry Baryshkov
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).
Note, I'd appreciate a comment on the INTF timing change from someone
at QCOM who knows the DPU hardware a bit better, this appears to be
what downstream is doing regardless of bpp, but let me know if there's
a better solution here.
Signed-off-by: Alexander Koskovich <akoskovich@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 (3):
drm/mipi-dsi: add RGB101010 pixel format
drm/msm/dsi: Add support for RGB101010 pixel format
drm/msm/dpu: fix video mode DSC INTF timing width for non 8 bit panels
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 12 +++++++-----
drivers/gpu/drm/msm/dsi/dsi_cfg.c | 8 ++++++++
drivers/gpu/drm/msm/dsi/dsi_cfg.h | 1 +
drivers/gpu/drm/msm/dsi/dsi_host.c | 9 +++++++++
drivers/gpu/drm/msm/registers/display/dsi.xml | 5 ++++-
include/drm/drm_mipi_dsi.h | 4 ++++
6 files changed, 33 insertions(+), 6 deletions(-)
---
base-commit: f338e77383789c0cae23ca3d48adcc5e9e137e3c
change-id: 20260318-dsi-rgb101010-support-4956b1cd8657
Best regards,
--
Alexander Koskovich <akoskovich@pm.me>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 1/3] drm/mipi-dsi: add RGB101010 pixel format
2026-03-19 3:59 [PATCH v2 0/3] drm/msm: add RGB101010 pixel format and fix 10-bit DSC timing Alexander Koskovich
@ 2026-03-19 4:00 ` Alexander Koskovich
2026-03-21 18:52 ` Claude review: " Claude Code Review Bot
2026-03-19 4:00 ` [PATCH v2 2/3] drm/msm/dsi: Add support for " Alexander Koskovich
` (2 subsequent siblings)
3 siblings, 1 reply; 16+ messages in thread
From: Alexander Koskovich @ 2026-03-19 4:00 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten
Cc: dri-devel, linux-kernel, linux-arm-msm, freedreno,
Alexander Koskovich, Dmitry Baryshkov
Add MIPI_DSI_FMT_RGB101010 for 30 bit (10,10,10 RGB) pixel format,
corresponding to the packed 30 bit pixel stream defined in MIPI DSI
v1.3 Section 8.8.17.
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
---
include/drm/drm_mipi_dsi.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
index 3aba7b380c8d..a822e9e876af 100644
--- a/include/drm/drm_mipi_dsi.h
+++ b/include/drm/drm_mipi_dsi.h
@@ -140,6 +140,7 @@ struct mipi_dsi_host *of_find_mipi_dsi_host_by_node(struct device_node *node);
#define MIPI_DSI_HS_PKT_END_ALIGNED BIT(12)
enum mipi_dsi_pixel_format {
+ MIPI_DSI_FMT_RGB101010,
MIPI_DSI_FMT_RGB888,
MIPI_DSI_FMT_RGB666,
MIPI_DSI_FMT_RGB666_PACKED,
@@ -235,6 +236,9 @@ extern const struct bus_type mipi_dsi_bus_type;
static inline int mipi_dsi_pixel_format_to_bpp(enum mipi_dsi_pixel_format fmt)
{
switch (fmt) {
+ case MIPI_DSI_FMT_RGB101010:
+ return 30;
+
case MIPI_DSI_FMT_RGB888:
case MIPI_DSI_FMT_RGB666:
return 24;
--
2.53.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 2/3] drm/msm/dsi: Add support for RGB101010 pixel format
2026-03-19 3:59 [PATCH v2 0/3] drm/msm: add RGB101010 pixel format and fix 10-bit DSC timing Alexander Koskovich
2026-03-19 4:00 ` [PATCH v2 1/3] drm/mipi-dsi: add RGB101010 pixel format Alexander Koskovich
@ 2026-03-19 4:00 ` Alexander Koskovich
2026-03-19 4:21 ` Dmitry Baryshkov
` (2 more replies)
2026-03-19 4:00 ` [PATCH v2 3/3] drm/msm/dpu: fix video mode DSC INTF timing width for non 8 bit panels Alexander Koskovich
2026-03-21 18:52 ` Claude review: drm/msm: add RGB101010 pixel format and fix 10-bit DSC timing Claude Code Review Bot
3 siblings, 3 replies; 16+ messages in thread
From: Alexander Koskovich @ 2026-03-19 4:00 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten
Cc: dri-devel, linux-kernel, linux-arm-msm, freedreno,
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_cfg.c | 8 ++++++++
drivers/gpu/drm/msm/dsi/dsi_cfg.h | 1 +
drivers/gpu/drm/msm/dsi/dsi_host.c | 9 +++++++++
drivers/gpu/drm/msm/registers/display/dsi.xml | 5 ++++-
4 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.c b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
index bd3c51c350e7..6a7ea2183a3b 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_cfg.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
@@ -133,6 +133,7 @@ static const struct msm_dsi_config msm8998_dsi_cfg = {
.io_start = {
{ 0xc994000, 0xc996000 },
},
+ .has_rgb30 = true,
};
static const char * const dsi_sdm660_bus_clk_names[] = {
@@ -152,6 +153,7 @@ static const struct msm_dsi_config sdm660_dsi_cfg = {
.io_start = {
{ 0xc994000, 0xc996000 },
},
+ .has_rgb30 = true,
};
static const char * const dsi_v2_4_clk_names[] = {
@@ -173,6 +175,7 @@ static const struct msm_dsi_config sdm845_dsi_cfg = {
{ 0xae94000, 0xae96000 }, /* SDM845 / SDM670 */
{ 0x5e94000 }, /* QCM2290 / SM6115 / SM6125 / SM6375 */
},
+ .has_rgb30 = true,
};
static const struct regulator_bulk_data sm8550_dsi_regulators[] = {
@@ -188,6 +191,7 @@ static const struct msm_dsi_config sm8550_dsi_cfg = {
.io_start = {
{ 0xae94000, 0xae96000 },
},
+ .has_rgb30 = true,
};
static const struct regulator_bulk_data sm8650_dsi_regulators[] = {
@@ -203,6 +207,7 @@ static const struct msm_dsi_config sm8650_dsi_cfg = {
.io_start = {
{ 0xae94000, 0xae96000 },
},
+ .has_rgb30 = true,
};
static const struct msm_dsi_config kaanapali_dsi_cfg = {
@@ -214,6 +219,7 @@ static const struct msm_dsi_config kaanapali_dsi_cfg = {
.io_start = {
{ 0x9ac0000, 0x9ac3000 },
},
+ .has_rgb30 = true,
};
static const struct regulator_bulk_data sc7280_dsi_regulators[] = {
@@ -230,6 +236,7 @@ static const struct msm_dsi_config sc7280_dsi_cfg = {
.io_start = {
{ 0xae94000, 0xae96000 },
},
+ .has_rgb30 = true,
};
static const struct regulator_bulk_data sa8775p_dsi_regulators[] = {
@@ -246,6 +253,7 @@ static const struct msm_dsi_config sa8775p_dsi_cfg = {
.io_start = {
{ 0xae94000, 0xae96000 },
},
+ .has_rgb30 = true,
};
static const struct msm_dsi_host_cfg_ops msm_dsi_v2_host_ops = {
diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.h b/drivers/gpu/drm/msm/dsi/dsi_cfg.h
index 5dc812028bd5..15cb9b46fadf 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_cfg.h
+++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.h
@@ -48,6 +48,7 @@ struct msm_dsi_config {
const char * const *bus_clk_names;
const int num_bus_clks;
const resource_size_t io_start[VARIANTS_MAX][DSI_MAX];
+ bool has_rgb30;
};
struct msm_dsi_host_cfg_ops {
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index db6da99375a1..34fd0dc5f7c7 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;
@@ -1698,6 +1700,13 @@ 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_host->cfg_hnd->cfg->has_rgb30) {
+ 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
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 3/3] drm/msm/dpu: fix video mode DSC INTF timing width for non 8 bit panels
2026-03-19 3:59 [PATCH v2 0/3] drm/msm: add RGB101010 pixel format and fix 10-bit DSC timing Alexander Koskovich
2026-03-19 4:00 ` [PATCH v2 1/3] drm/mipi-dsi: add RGB101010 pixel format Alexander Koskovich
2026-03-19 4:00 ` [PATCH v2 2/3] drm/msm/dsi: Add support for " Alexander Koskovich
@ 2026-03-19 4:00 ` Alexander Koskovich
2026-03-19 8:34 ` Neil Armstrong
2026-03-21 18:52 ` Claude review: " Claude Code Review Bot
2026-03-21 18:52 ` Claude review: drm/msm: add RGB101010 pixel format and fix 10-bit DSC timing Claude Code Review Bot
3 siblings, 2 replies; 16+ messages in thread
From: Alexander Koskovich @ 2026-03-19 4:00 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten
Cc: dri-devel, linux-kernel, linux-arm-msm, freedreno,
Alexander Koskovich
Using bits_per_component * 3 as the divisor for the compressed INTF
timing width produces constant FIFO errors for panels such as the BOE
BF068MWM-TD0 which is a 10 bit panel.
The downstream driver calculates the compressed timing width by
dividing the total compressed bytes per line by 3 which does not depend
on bits_per_component. Switch the divisor to 24 to match downstream.
Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 12 +++++++-----
1 file changed, 7 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..9b046a0e77aa 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,21 @@ 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, use a fixed divisor of 24 rather than
+ * bits_per_component * 3 when calculating the compressed timing width.
+ *
+ * This matches the downstream driver and is required for panels with
+ * bits_per_component != 8.
*/
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
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2 2/3] drm/msm/dsi: Add support for RGB101010 pixel format
2026-03-19 4:00 ` [PATCH v2 2/3] drm/msm/dsi: Add support for " Alexander Koskovich
@ 2026-03-19 4:21 ` Dmitry Baryshkov
2026-03-19 9:10 ` Konrad Dybcio
2026-03-21 18:52 ` Claude review: " Claude Code Review Bot
2 siblings, 0 replies; 16+ messages in thread
From: Dmitry Baryshkov @ 2026-03-19 4:21 UTC (permalink / raw)
To: Alexander Koskovich
Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, dri-devel, linux-kernel,
linux-arm-msm, freedreno
On Thu, Mar 19, 2026 at 04:00:05AM +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_cfg.c | 8 ++++++++
> drivers/gpu/drm/msm/dsi/dsi_cfg.h | 1 +
> drivers/gpu/drm/msm/dsi/dsi_host.c | 9 +++++++++
> drivers/gpu/drm/msm/registers/display/dsi.xml | 5 ++++-
> 4 files changed, 22 insertions(+), 1 deletion(-)
>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 3/3] drm/msm/dpu: fix video mode DSC INTF timing width for non 8 bit panels
2026-03-19 4:00 ` [PATCH v2 3/3] drm/msm/dpu: fix video mode DSC INTF timing width for non 8 bit panels Alexander Koskovich
@ 2026-03-19 8:34 ` Neil Armstrong
2026-03-19 8:48 ` Alexander Koskovich
2026-03-19 10:04 ` Konrad Dybcio
2026-03-21 18:52 ` Claude review: " Claude Code Review Bot
1 sibling, 2 replies; 16+ messages in thread
From: Neil Armstrong @ 2026-03-19 8:34 UTC (permalink / raw)
To: Alexander Koskovich, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Clark,
Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang, Sean Paul,
Marijn Suijten
Cc: dri-devel, linux-kernel, linux-arm-msm, freedreno
On 3/19/26 05:00, Alexander Koskovich wrote:
> Using bits_per_component * 3 as the divisor for the compressed INTF
> timing width produces constant FIFO errors for panels such as the BOE
> BF068MWM-TD0 which is a 10 bit panel.
>
> The downstream driver calculates the compressed timing width by
> dividing the total compressed bytes per line by 3 which does not depend
> on bits_per_component. Switch the divisor to 24 to match downstream.
>
> Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 12 +++++++-----
> 1 file changed, 7 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..9b046a0e77aa 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,21 @@ 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, use a fixed divisor of 24 rather than
> + * bits_per_component * 3 when calculating the compressed timing width.
> + *
> + * This matches the downstream driver and is required for panels with
> + * bits_per_component != 8.
> */
> 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;
@bits_per_component: Bits per component to code (8/10/12) <= how the DSC pixels are encoded in the stream
@bits_per_pixel: Target bits per pixel with 4 fractional bits, bits_per_pixel << 4 <= the target display pixels
- bits_per_component is the transport width
- bits_per_pixel is the display width
- 3 is the DSC compression ratio
So for a RGB101010 DSC display bits_per_pixel should be 10 << 4
But here you say bits_per_component should be 8 ? can you share the downstream config of your panel ?
Are you sure about the bits_per_component & bits_per_pixel values you set in the dsc parameters ?
Neil
> timing->xres = timing->width;
> }
> }
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 3/3] drm/msm/dpu: fix video mode DSC INTF timing width for non 8 bit panels
2026-03-19 8:34 ` Neil Armstrong
@ 2026-03-19 8:48 ` Alexander Koskovich
2026-03-19 10:33 ` Neil Armstrong
2026-03-19 10:04 ` Konrad Dybcio
1 sibling, 1 reply; 16+ messages in thread
From: Alexander Koskovich @ 2026-03-19 8:48 UTC (permalink / raw)
To: Neil Armstrong
Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, dri-devel, linux-kernel,
linux-arm-msm, freedreno
On Thursday, March 19th, 2026 at 4:35 AM, Neil Armstrong <neil.armstrong@linaro.org> wrote:
> On 3/19/26 05:00, Alexander Koskovich wrote:
> > Using bits_per_component * 3 as the divisor for the compressed INTF
> > timing width produces constant FIFO errors for panels such as the BOE
> > BF068MWM-TD0 which is a 10 bit panel.
> >
> > The downstream driver calculates the compressed timing width by
> > dividing the total compressed bytes per line by 3 which does not depend
> > on bits_per_component. Switch the divisor to 24 to match downstream.
> >
> > Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 12 +++++++-----
> > 1 file changed, 7 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..9b046a0e77aa 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,21 @@ 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, use a fixed divisor of 24 rather than
> > + * bits_per_component * 3 when calculating the compressed timing width.
> > + *
> > + * This matches the downstream driver and is required for panels with
> > + * bits_per_component != 8.
> > */
> > 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;
>
>
>
> @bits_per_component: Bits per component to code (8/10/12) <= how the DSC pixels are encoded in the stream
> @bits_per_pixel: Target bits per pixel with 4 fractional bits, bits_per_pixel << 4 <= the target display pixels
>
> - bits_per_component is the transport width
> - bits_per_pixel is the display width
> - 3 is the DSC compression ratio
>
> So for a RGB101010 DSC display bits_per_pixel should be 10 << 4
>
> But here you say bits_per_component should be 8 ? can you share the downstream config of your panel ?
This is what is defined downstream for this panel, they're using 8:
https://github.com/NothingOSS/android_kernel_msm-6.1_nothing_sm7635/blob/sm7635/b/mr/vendor/qcom/proprietary/display-devicetree/display/dsi-panel-rm69220-dsc-fhd-plus-120hz-vid-boe.dtsi
>
> Are you sure about the bits_per_component & bits_per_pixel values you set in the dsc parameters ?
I'm pretty sure they're correct, here's the panel driver I have atm:
https://github.com/AKoskovich/linux/blob/asteroids-6.19.y/drivers/gpu/drm/panel/panel-boe-bf068mwm-td0.c
>
> Neil
>
>
> > timing->xres = timing->width;
> > }
> > }
> >
>
>
Thanks,
Alex
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 2/3] drm/msm/dsi: Add support for RGB101010 pixel format
2026-03-19 4:00 ` [PATCH v2 2/3] drm/msm/dsi: Add support for " Alexander Koskovich
2026-03-19 4:21 ` Dmitry Baryshkov
@ 2026-03-19 9:10 ` Konrad Dybcio
2026-03-19 9:25 ` Alexander Koskovich
2026-03-21 18:52 ` Claude review: " Claude Code Review Bot
2 siblings, 1 reply; 16+ messages in thread
From: Konrad Dybcio @ 2026-03-19 9:10 UTC (permalink / raw)
To: Alexander Koskovich, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Clark,
Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang, Sean Paul,
Marijn Suijten
Cc: dri-devel, linux-kernel, linux-arm-msm, freedreno
On 3/19/26 5:00 AM, 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_cfg.c | 8 ++++++++
> drivers/gpu/drm/msm/dsi/dsi_cfg.h | 1 +
> drivers/gpu/drm/msm/dsi/dsi_host.c | 9 +++++++++
> drivers/gpu/drm/msm/registers/display/dsi.xml | 5 ++++-
> 4 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.c b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
> index bd3c51c350e7..6a7ea2183a3b 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_cfg.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
> @@ -133,6 +133,7 @@ static const struct msm_dsi_config msm8998_dsi_cfg = {
> .io_start = {
> { 0xc994000, 0xc996000 },
> },
> + .has_rgb30 = true,
I wrote a patch to determine this at runtime, and only after I was done, I
noticed that we can already achieve this:
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);
}
let's perhaps extract this to a msm_dsi_host_version_is_gt(u32 major, u32 minor)
or similar
and your assumption about >=v2.1 is corroborated by a doc I have
Konrad
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 2/3] drm/msm/dsi: Add support for RGB101010 pixel format
2026-03-19 9:10 ` Konrad Dybcio
@ 2026-03-19 9:25 ` Alexander Koskovich
2026-03-19 9:39 ` Konrad Dybcio
0 siblings, 1 reply; 16+ messages in thread
From: Alexander Koskovich @ 2026-03-19 9:25 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, dri-devel, linux-kernel,
linux-arm-msm, freedreno
On Thursday, March 19th, 2026 at 5:10 AM, Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> wrote:
> On 3/19/26 5:00 AM, 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_cfg.c | 8 ++++++++
> > drivers/gpu/drm/msm/dsi/dsi_cfg.h | 1 +
> > drivers/gpu/drm/msm/dsi/dsi_host.c | 9 +++++++++
> > drivers/gpu/drm/msm/registers/display/dsi.xml | 5 ++++-
> > 4 files changed, 22 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.c b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
> > index bd3c51c350e7..6a7ea2183a3b 100644
> > --- a/drivers/gpu/drm/msm/dsi/dsi_cfg.c
> > +++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
> > @@ -133,6 +133,7 @@ static const struct msm_dsi_config msm8998_dsi_cfg = {
> > .io_start = {
> > { 0xc994000, 0xc996000 },
> > },
> > + .has_rgb30 = true,
>
> I wrote a patch to determine this at runtime, and only after I was done, I
> noticed that we can already achieve this:
>
> 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);
> }
>
> let's perhaps extract this to a msm_dsi_host_version_is_gt(u32 major, u32 minor)
> or similar
That's what I was looking into initially, but V2_2_0 (0x20000000) is numerically less than V2_1_0 (0x20010000) so didn't think I could do that.
Do you know if msm8998 is correct? Downstream says it is 2.0 not 2.2.
>
> and your assumption about >=v2.1 is corroborated by a doc I have
>
> Konrad
>
Thanks,
Alex
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 2/3] drm/msm/dsi: Add support for RGB101010 pixel format
2026-03-19 9:25 ` Alexander Koskovich
@ 2026-03-19 9:39 ` Konrad Dybcio
0 siblings, 0 replies; 16+ messages in thread
From: Konrad Dybcio @ 2026-03-19 9:39 UTC (permalink / raw)
To: Alexander Koskovich
Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, dri-devel, linux-kernel,
linux-arm-msm, freedreno
On 3/19/26 10:25 AM, Alexander Koskovich wrote:
> On Thursday, March 19th, 2026 at 5:10 AM, Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> wrote:
>
>> On 3/19/26 5:00 AM, 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_cfg.c | 8 ++++++++
>>> drivers/gpu/drm/msm/dsi/dsi_cfg.h | 1 +
>>> drivers/gpu/drm/msm/dsi/dsi_host.c | 9 +++++++++
>>> drivers/gpu/drm/msm/registers/display/dsi.xml | 5 ++++-
>>> 4 files changed, 22 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.c b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
>>> index bd3c51c350e7..6a7ea2183a3b 100644
>>> --- a/drivers/gpu/drm/msm/dsi/dsi_cfg.c
>>> +++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
>>> @@ -133,6 +133,7 @@ static const struct msm_dsi_config msm8998_dsi_cfg = {
>>> .io_start = {
>>> { 0xc994000, 0xc996000 },
>>> },
>>> + .has_rgb30 = true,
>>
>> I wrote a patch to determine this at runtime, and only after I was done, I
>> noticed that we can already achieve this:
>>
>> 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);
>> }
>>
>> let's perhaps extract this to a msm_dsi_host_version_is_gt(u32 major, u32 minor)
>> or similar
>
> That's what I was looking into initially, but V2_2_0 (0x20000000) is numerically less than V2_1_0 (0x20010000) so didn't think I could do that.
It should be v2.0.0 (0x2(zeroes))
v2.2.0 -> sdm845v1 (unreleased)
v2.2.1 -> sdm670 and sdm845v2
Konrad
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 3/3] drm/msm/dpu: fix video mode DSC INTF timing width for non 8 bit panels
2026-03-19 8:34 ` Neil Armstrong
2026-03-19 8:48 ` Alexander Koskovich
@ 2026-03-19 10:04 ` Konrad Dybcio
1 sibling, 0 replies; 16+ messages in thread
From: Konrad Dybcio @ 2026-03-19 10:04 UTC (permalink / raw)
To: Neil Armstrong, Alexander Koskovich, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten
Cc: dri-devel, linux-kernel, linux-arm-msm, freedreno
On 3/19/26 9:34 AM, Neil Armstrong wrote:
> On 3/19/26 05:00, Alexander Koskovich wrote:
>> Using bits_per_component * 3 as the divisor for the compressed INTF
>> timing width produces constant FIFO errors for panels such as the BOE
>> BF068MWM-TD0 which is a 10 bit panel.
>>
>> The downstream driver calculates the compressed timing width by
>> dividing the total compressed bytes per line by 3 which does not depend
>> on bits_per_component. Switch the divisor to 24 to match downstream.
>>
>> Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
>> ---
>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 12 +++++++-----
>> 1 file changed, 7 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..9b046a0e77aa 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,21 @@ 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, use a fixed divisor of 24 rather than
>> + * bits_per_component * 3 when calculating the compressed timing width.
>> + *
>> + * This matches the downstream driver and is required for panels with
>> + * bits_per_component != 8.
>> */
>> 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;
>
>
>
> @bits_per_component: Bits per component to code (8/10/12) <= how the DSC pixels are encoded in the stream
> @bits_per_pixel: Target bits per pixel with 4 fractional bits, bits_per_pixel << 4 <= the target display pixels
>
> - bits_per_component is the transport width
> - bits_per_pixel is the display width
> - 3 is the DSC compression ratio
>
> So for a RGB101010 DSC display bits_per_pixel should be 10 << 4
>
> But here you say bits_per_component should be 8 ? can you share the downstream config of your panel ?
>
> Are you sure about the bits_per_component & bits_per_pixel values you set in the dsc parameters ?
The computer tells me that if widebus=off, regardless of the compression
ratio and pixel depth before compression, 24 bits of compressed data are
transferred per pclk, and then you can transfer 1/2/4 slices per xfer
(As a note, the DSC compression ratio isn't fixed)
This also impacts the byte/pixel clock calculations (but dsi_host.c seems
to have taken care of that)
Maybe Dmitry knows something more..
Konrad
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 3/3] drm/msm/dpu: fix video mode DSC INTF timing width for non 8 bit panels
2026-03-19 8:48 ` Alexander Koskovich
@ 2026-03-19 10:33 ` Neil Armstrong
0 siblings, 0 replies; 16+ messages in thread
From: Neil Armstrong @ 2026-03-19 10:33 UTC (permalink / raw)
To: Alexander Koskovich
Cc: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, dri-devel, linux-kernel,
linux-arm-msm, freedreno
On 3/19/26 09:48, Alexander Koskovich wrote:
> On Thursday, March 19th, 2026 at 4:35 AM, Neil Armstrong <neil.armstrong@linaro.org> wrote:
>
>> On 3/19/26 05:00, Alexander Koskovich wrote:
>>> Using bits_per_component * 3 as the divisor for the compressed INTF
>>> timing width produces constant FIFO errors for panels such as the BOE
>>> BF068MWM-TD0 which is a 10 bit panel.
>>>
>>> The downstream driver calculates the compressed timing width by
>>> dividing the total compressed bytes per line by 3 which does not depend
>>> on bits_per_component. Switch the divisor to 24 to match downstream.
>>>
>>> Signed-off-by: Alexander Koskovich <akoskovich@pm.me>
>>> ---
>>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 12 +++++++-----
>>> 1 file changed, 7 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..9b046a0e77aa 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,21 @@ 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, use a fixed divisor of 24 rather than
>>> + * bits_per_component * 3 when calculating the compressed timing width.
>>> + *
>>> + * This matches the downstream driver and is required for panels with
>>> + * bits_per_component != 8.
>>> */
>>> 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;
>>
>>
>>
>> @bits_per_component: Bits per component to code (8/10/12) <= how the DSC pixels are encoded in the stream
>> @bits_per_pixel: Target bits per pixel with 4 fractional bits, bits_per_pixel << 4 <= the target display pixels
>>
>> - bits_per_component is the transport width
>> - bits_per_pixel is the display width
>> - 3 is the DSC compression ratio
>>
>> So for a RGB101010 DSC display bits_per_pixel should be 10 << 4
>>
>> But here you say bits_per_component should be 8 ? can you share the downstream config of your panel ?
>
> This is what is defined downstream for this panel, they're using 8:
> https://github.com/NothingOSS/android_kernel_msm-6.1_nothing_sm7635/blob/sm7635/b/mr/vendor/qcom/proprietary/display-devicetree/display/dsi-panel-rm69220-dsc-fhd-plus-120hz-vid-boe.dtsi
>
>>
>> Are you sure about the bits_per_component & bits_per_pixel values you set in the dsc parameters ?
>
> I'm pretty sure they're correct, here's the panel driver I have atm:
> https://github.com/AKoskovich/linux/blob/asteroids-6.19.y/drivers/gpu/drm/panel/panel-boe-bf068mwm-td0.c
So I looked at downstream, and bit-per-component is not used at all for the width calculation.
Here's the full downstream calculation with your panel:
intf_width = mode->timing.h_active; => 1080
slice_per_pkt = dsc_info->slice_per_pkt; => 2
slice_per_intf = DIV_ROUND_UP(intf_width, dsc_info->config.slice_width); => 1080/540 = 2
if (slice_per_pkt > slice_per_intf)
slice_per_pkt = 1;
bpp = DSC_BPP(dsc_info->config); => 8
bytes_in_slice = DIV_ROUND_UP(dsc_info->config.slice_width * bpp, 8); => 540 * 8 / 8 = 540
dsc_info->bytes_per_pkt = bytes_in_slice * slice_per_pkt; => 540 * 2 = 1080
dsc_info->pkt_per_line = slice_per_intf / slice_per_pkt; => 2 / 2 = 1
...
phys->dce_bytes_per_line = comp_info->dsc_info.bytes_per_pkt * comp_info->dsc_info.pkt_per_line; => 1080 * 1 = 1080
...
if (p->compression_en) {
if (p->wide_bus_en)
data_width = DIV_ROUND_UP(p->dce_bytes_per_line, 6); => 1080 / 6 = 180
else
data_width = DIV_ROUND_UP(p->dce_bytes_per_line, 3); => 1080 / 3 = 360
}
So you're right, it should be a fixed (8 * 3) division, and a (8 * 6) in case of widebus, but DSI widebus is not yet handled.
Neil
>
>>
>> Neil
>>
>>
>>> timing->xres = timing->width;
>>> }
>>> }
>>>
>>
>>
>
> Thanks,
> Alex
^ permalink raw reply [flat|nested] 16+ messages in thread
* Claude review: drm/msm: add RGB101010 pixel format and fix 10-bit DSC timing
2026-03-19 3:59 [PATCH v2 0/3] drm/msm: add RGB101010 pixel format and fix 10-bit DSC timing Alexander Koskovich
` (2 preceding siblings ...)
2026-03-19 4:00 ` [PATCH v2 3/3] drm/msm/dpu: fix video mode DSC INTF timing width for non 8 bit panels Alexander Koskovich
@ 2026-03-21 18:52 ` Claude Code Review Bot
3 siblings, 0 replies; 16+ messages in thread
From: Claude Code Review Bot @ 2026-03-21 18:52 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: drm/msm: add RGB101010 pixel format and fix 10-bit DSC timing
Author: Alexander Koskovich <akoskovich@pm.me>
Patches: 12
Reviewed: 2026-03-22T04:52:39.673001
---
This 3-patch series adds MIPI DSI RGB101010 (30bpp/10-bit) pixel format support to the MSM DSI driver and fixes a DSC timing calculation for non-8-bit panels. The series is well-motivated (enabling 10-bit panels like the BOE BF068MWM-TD0) and the approach is generally sound, but there are a few issues to address.
**Key concerns:**
1. **Patch 1**: Adding the new enum value at the **beginning** of the enum rather than the end is unconventional and fragile, even though current users only switch on symbolic names.
2. **Patch 3**: A parallel formula in `dsi_host.c:591` (`dsi_adjust_pclk_for_compression`) uses the same `bits_per_component * 3` divisor but is not addressed — this should be investigated for consistency.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 16+ messages in thread
* Claude review: drm/mipi-dsi: add RGB101010 pixel format
2026-03-19 4:00 ` [PATCH v2 1/3] drm/mipi-dsi: add RGB101010 pixel format Alexander Koskovich
@ 2026-03-21 18:52 ` Claude Code Review Bot
0 siblings, 0 replies; 16+ messages in thread
From: Claude Code Review Bot @ 2026-03-21 18:52 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Enum ordering concern:** The new `MIPI_DSI_FMT_RGB101010` is inserted at the **top** of the enum:
```c
enum mipi_dsi_pixel_format {
+ MIPI_DSI_FMT_RGB101010,
MIPI_DSI_FMT_RGB888,
```
This changes the implicit numeric values of all existing members (`MIPI_DSI_FMT_RGB888` goes from 0 to 1, etc.). While current in-tree users appear to use symbolic names in switch statements rather than relying on numeric values, this is still bad practice. There are 137+ files using these enum values across the DRM subsystem. Any out-of-tree driver or userspace tool that has hardcoded the numeric values would break.
**Recommendation:** Append the new value at the end, after `MIPI_DSI_FMT_RGB565`:
```c
enum mipi_dsi_pixel_format {
MIPI_DSI_FMT_RGB888,
MIPI_DSI_FMT_RGB666,
MIPI_DSI_FMT_RGB666_PACKED,
MIPI_DSI_FMT_RGB565,
MIPI_DSI_FMT_RGB101010,
};
```
**Missing handling in `drm_mipi_dsi_get_input_bus_fmt()`:** The function at `drm_mipi_dsi.c:888` switches on all four existing formats and falls through to a `default` case. Adding `MIPI_DSI_FMT_RGB101010` without a corresponding case there will cause compiler warnings (`-Wswitch`) in some configurations, and the function will return 0 for this format. A `MEDIA_BUS_FMT_RGB101010_1X30` case should be added (or the function should explicitly handle it, even if just falling to default with a comment).
The `mipi_dsi_pixel_format_to_bpp()` addition returning 30 is correct.
Already has Reviewed-by from Dmitry Baryshkov, which is good.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 16+ messages in thread
* Claude review: drm/msm/dsi: Add support for RGB101010 pixel format
2026-03-19 4:00 ` [PATCH v2 2/3] drm/msm/dsi: Add support for " Alexander Koskovich
2026-03-19 4:21 ` Dmitry Baryshkov
2026-03-19 9:10 ` Konrad Dybcio
@ 2026-03-21 18:52 ` Claude Code Review Bot
2 siblings, 0 replies; 16+ messages in thread
From: Claude Code Review Bot @ 2026-03-21 18:52 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**`has_rgb30` flag approach:** Using a per-config boolean flag is a reasonable approach given the version-guarding requirement. The check in `dsi_host_attach()` is clean:
```c
+ if (dsi->format == MIPI_DSI_FMT_RGB101010 &&
+ !msm_host->cfg_hnd->cfg->has_rgb30) {
+ DRM_DEV_ERROR(&msm_host->pdev->dev,
+ "RGB101010 not supported on this DSI controller\n");
+ return -EINVAL;
+ }
```
**Missing configs:** The patch sets `has_rgb30 = true` for msm8998, sdm660, sdm845, sm8550, sm8650, kaanapali, sc7280, and sa8775p. However, the cover letter says this is guarded behind `MSM_DSI_6G_VER >= V2.1.0`, yet `msm8998_dsi_cfg` and `sdm660_dsi_cfg` are V1.x era configs. Are these actually V2.1.0+? If the hardware truly requires V2.1.0+, then marking msm8998 (which is 6G v1.x based) as `has_rgb30 = true` may be incorrect. The commit message and the actual flag settings should be reconciled.
**Register XML changes are correct:** Extending `VID_CFG0.DST_FORMAT` from bits [4:5] to [4:6] and adding `VID_DST_FORMAT_RGB101010 = 4` and `CMD_DST_FORMAT_RGB101010 = 9` looks appropriate. The comment `<!-- high was 5 before DSI 6G 2.1.0 -->` is a helpful annotation.
**Format mapping additions** in `dsi_get_vid_fmt()` and `dsi_get_cmd_fmt()` are straightforward and correct.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 16+ messages in thread
* Claude review: drm/msm/dpu: fix video mode DSC INTF timing width for non 8 bit panels
2026-03-19 4:00 ` [PATCH v2 3/3] drm/msm/dpu: fix video mode DSC INTF timing width for non 8 bit panels Alexander Koskovich
2026-03-19 8:34 ` Neil Armstrong
@ 2026-03-21 18:52 ` Claude Code Review Bot
1 sibling, 0 replies; 16+ messages in thread
From: Claude Code Review Bot @ 2026-03-21 18:52 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**The fix itself is mathematically correct.** The compressed INTF timing width should represent how many pixel-clock cycles are needed to output the compressed data. The INTF outputs 3 bytes (24 bits) per pixel clock, so:
```
compressed_width = (width * bpp_compressed) / 24
```
The previous formula `width * bpp_compressed / (bits_per_component * 3)` happens to equal the same thing for 8-bit panels (8×3=24) but produces wrong results for 10-bit panels (10×3=30), which explains the FIFO errors.
**Consistency concern — `dsi_adjust_pclk_for_compression()` at `dsi_host.c:590-591`:**
```c
int new_hdisplay = DIV_ROUND_UP(mode->hdisplay * drm_dsc_get_bpp_int(dsc),
dsc->bits_per_component * 3);
```
This function uses the **exact same** `bits_per_component * 3` divisor for pclk rate adjustment. If the reasoning in this patch is correct (the divisor should always be 24), then this function likely needs the same fix. Not addressing it could mean the pixel clock rate is calculated incorrectly for 10-bit DSC panels, potentially causing timing issues even after this INTF fix. At minimum, the commit message should explain why this instance is different or add a note that it will be addressed separately.
**Commit message could be improved:** The patch is described as fixing "video mode DSC INTF timing" but the rationale of "matches downstream" is somewhat weak on its own. It would benefit from a brief explanation of *why* 24 is correct (the INTF always outputs 3 bytes per clock regardless of source bit depth), rather than just deferring to downstream behavior.
**Nit:** This patch could arguably carry a `Fixes:` tag pointing to the commit that introduced the `bits_per_component * 3` formula, which would help with backporting to stable kernels.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2026-03-21 18:52 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-19 3:59 [PATCH v2 0/3] drm/msm: add RGB101010 pixel format and fix 10-bit DSC timing Alexander Koskovich
2026-03-19 4:00 ` [PATCH v2 1/3] drm/mipi-dsi: add RGB101010 pixel format Alexander Koskovich
2026-03-21 18:52 ` Claude review: " Claude Code Review Bot
2026-03-19 4:00 ` [PATCH v2 2/3] drm/msm/dsi: Add support for " Alexander Koskovich
2026-03-19 4:21 ` Dmitry Baryshkov
2026-03-19 9:10 ` Konrad Dybcio
2026-03-19 9:25 ` Alexander Koskovich
2026-03-19 9:39 ` Konrad Dybcio
2026-03-21 18:52 ` Claude review: " Claude Code Review Bot
2026-03-19 4:00 ` [PATCH v2 3/3] drm/msm/dpu: fix video mode DSC INTF timing width for non 8 bit panels Alexander Koskovich
2026-03-19 8:34 ` Neil Armstrong
2026-03-19 8:48 ` Alexander Koskovich
2026-03-19 10:33 ` Neil Armstrong
2026-03-19 10:04 ` Konrad Dybcio
2026-03-21 18:52 ` Claude review: " Claude Code Review Bot
2026-03-21 18:52 ` Claude review: drm/msm: add RGB101010 pixel format and fix 10-bit DSC timing Claude Code Review Bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox