* [PATCH v4 01/10] dt-bindings: display: mediatek: dsc: Add MT8196 compatible
2026-07-13 14:27 [PATCH v4 00/10] drm/mediatek: Add DSC, WDMA, MT8189/96 DSI support AngeloGioacchino Del Regno
@ 2026-07-13 14:27 ` AngeloGioacchino Del Regno
2026-07-13 14:27 ` [PATCH v4 02/10] drm/mediatek: Implement Display Stream Compression support AngeloGioacchino Del Regno
` (8 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: AngeloGioacchino Del Regno @ 2026-07-13 14:27 UTC (permalink / raw)
To: chunkuang.hu
Cc: p.zabel, maarten.lankhorst, mripard, tzimmermann, airlied, simona,
robh, krzk+dt, conor+dt, matthias.bgg, angelogioacchino.delregno,
jitao.shi, dri-devel, linux-mediatek, devicetree, linux-kernel,
linux-arm-kernel, kernel, justin.yeh, jason-jh.lin
Add compatible for the Display Stream Compression (DSC) IP found
in the display controller of the MT8196 SoC.
This IP is compatible with the one found in MT8195.
Acked-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
.../devicetree/bindings/display/mediatek/mediatek,dsc.yaml | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dsc.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dsc.yaml
index a5b88eb97e3b..c8b3e86943e4 100644
--- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dsc.yaml
+++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dsc.yaml
@@ -23,7 +23,9 @@ properties:
- enum:
- mediatek,mt8195-disp-dsc
- items:
- - const: mediatek,mt8188-disp-dsc
+ - enum:
+ - mediatek,mt8188-disp-dsc
+ - mediatek,mt8196-disp-dsc
- const: mediatek,mt8195-disp-dsc
reg:
--
2.54.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v4 02/10] drm/mediatek: Implement Display Stream Compression support
2026-07-13 14:27 [PATCH v4 00/10] drm/mediatek: Add DSC, WDMA, MT8189/96 DSI support AngeloGioacchino Del Regno
2026-07-13 14:27 ` [PATCH v4 01/10] dt-bindings: display: mediatek: dsc: Add MT8196 compatible AngeloGioacchino Del Regno
@ 2026-07-13 14:27 ` AngeloGioacchino Del Regno
2026-07-13 14:41 ` sashiko-bot
2026-07-13 14:27 ` [PATCH v4 03/10] dt-bindings: display: mediatek: dsi: Document MT8189 and MT8196 AngeloGioacchino Del Regno
` (7 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: AngeloGioacchino Del Regno @ 2026-07-13 14:27 UTC (permalink / raw)
To: chunkuang.hu
Cc: p.zabel, maarten.lankhorst, mripard, tzimmermann, airlied, simona,
robh, krzk+dt, conor+dt, matthias.bgg, angelogioacchino.delregno,
jitao.shi, dri-devel, linux-mediatek, devicetree, linux-kernel,
linux-arm-kernel, kernel, justin.yeh, jason-jh.lin
Add a real driver for the Display Stream Compression (DSC) Display
Controller IP, implementing support for DSC v1.1 to v1.2.
In order to do this, it was necessary to remove the basic DSC IP
bypass setup from mtk_ddp_comp: this functionality is retained in
the new mtk_disp_dsc driver, which checks if DSC was actually
requested by other components (with the only one that currently
supports this being DSI) and, if not, it will set BYPASS mode in
the DSC IP.
Like before, the BYPASS mode is set before starting the DSC IP,
but unlike before, this is being done in the component start
callback instead of the config one.
Notably, the config callback is called by mtk_crtc always
immediately before the calling start callback, so the order of
register writes is retained.
The only real difference is that now this is being done through
CPU writes instead of CMDQ, but since that's called only once
and since it's just three registers, the performance impact will
not be minimal and not even measurable.
As anticipated, DSC handling was also introduced in the mtk_dsi
driver: when performing dsi_host_attach, the driver now checks
if the DSI panel adds the DSC configuration structure to the
mipi_dsi_device structure and, if it does, it will store a
pointer in the driver-local mtk_dsi structure's `dsc` member.
The DSI driver will then check whether the DSC configuration
that comes from the panel is valid (in regard to MediaTek DSI)
and will call the DRM API's DSC helpers to calculate and set
all of the const and RC parameters for the actual DSC setup.
For the time being, even though the latest MediaTek SoCs do
support DSC v1.2, only DSC v1.1 pre-scr support is implemented
as an initial contribution (which is rather big, and 1.2 would
make it even bigger - but that can anyway be implemented later).
As a last step for validation of DSC parameters in DSI, a check
for the hdisplay against DSC slice sidth and one for vdisplay
against DSC slice height was added to the mode_valid callback,
making sure that H/V are, as expected, multiples of slice W/H.
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
drivers/gpu/drm/mediatek/Makefile | 1 +
drivers/gpu/drm/mediatek/mtk_crtc.c | 21 ++
drivers/gpu/drm/mediatek/mtk_ddp_comp.c | 43 +--
drivers/gpu/drm/mediatek/mtk_ddp_comp.h | 9 +
drivers/gpu/drm/mediatek/mtk_disp_drv.h | 9 +
drivers/gpu/drm/mediatek/mtk_disp_dsc.c | 455 ++++++++++++++++++++++++
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 2 +
drivers/gpu/drm/mediatek/mtk_drm_drv.h | 1 +
drivers/gpu/drm/mediatek/mtk_dsi.c | 151 +++++++-
9 files changed, 642 insertions(+), 50 deletions(-)
create mode 100644 drivers/gpu/drm/mediatek/mtk_disp_dsc.c
diff --git a/drivers/gpu/drm/mediatek/Makefile b/drivers/gpu/drm/mediatek/Makefile
index 952d294642fb..03b3470ea5b5 100644
--- a/drivers/gpu/drm/mediatek/Makefile
+++ b/drivers/gpu/drm/mediatek/Makefile
@@ -5,6 +5,7 @@ mediatek-drm-y := mtk_crtc.o \
mtk_disp_aal.o \
mtk_disp_ccorr.o \
mtk_disp_color.o \
+ mtk_disp_dsc.o \
mtk_disp_gamma.o \
mtk_disp_merge.o \
mtk_disp_ovl.o \
diff --git a/drivers/gpu/drm/mediatek/mtk_crtc.c b/drivers/gpu/drm/mediatek/mtk_crtc.c
index 8e552cdc3b53..d4be40b2574a 100644
--- a/drivers/gpu/drm/mediatek/mtk_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_crtc.c
@@ -22,6 +22,7 @@
#include "mtk_crtc.h"
#include "mtk_ddp_comp.h"
+#include "mtk_disp_drv.h"
#include "mtk_drm_drv.h"
#include "mtk_plane.h"
@@ -344,6 +345,8 @@ static int mtk_crtc_ddp_hw_init(struct mtk_crtc *mtk_crtc)
struct drm_connector *connector;
struct drm_encoder *encoder;
struct drm_connector_list_iter conn_iter;
+ struct mtk_ddp_comp *comp_dsi = NULL, *comp_dsc = NULL;
+ struct drm_dsc_config *dsc_cfg;
struct drm_device *dev = mtk_crtc->base.dev;
unsigned int width, height, vrefresh, bpc = MTK_MAX_BPC;
int ret;
@@ -398,6 +401,17 @@ static int mtk_crtc_ddp_hw_init(struct mtk_crtc *mtk_crtc)
if (!mtk_ddp_comp_add(mtk_crtc->ddp_comp[i], mtk_crtc->mutex))
mtk_mutex_add_comp(mtk_crtc->mutex,
mtk_crtc->ddp_comp[i]->id);
+
+ /* For now, only single DSI is supported */
+ if (mtk_crtc->ddp_comp[i]->id >= DDP_COMPONENT_DSI0 &&
+ mtk_crtc->ddp_comp[i]->id <= DDP_COMPONENT_DSI3)
+ if (!comp_dsi)
+ comp_dsi = mtk_crtc->ddp_comp[i];
+
+ if (mtk_crtc->ddp_comp[i]->id == DDP_COMPONENT_DSC0 ||
+ mtk_crtc->ddp_comp[i]->id == DDP_COMPONENT_DSC1)
+ if (!comp_dsc)
+ comp_dsc = mtk_crtc->ddp_comp[i];
}
if (!mtk_ddp_comp_add(mtk_crtc->ddp_comp[i], mtk_crtc->mutex))
mtk_mutex_add_comp(mtk_crtc->mutex, mtk_crtc->ddp_comp[i]->id);
@@ -413,6 +427,13 @@ static int mtk_crtc_ddp_hw_init(struct mtk_crtc *mtk_crtc)
mtk_ddp_comp_start(comp);
}
+ /* Setup the DSC if present, with the config coming from DSI */
+ if (comp_dsc && comp_dsi) {
+ dsc_cfg = mtk_dsi_get_dsc_config(comp_dsi->dev);
+ if (dsc_cfg)
+ mtk_ddp_comp_dsc_setup(comp_dsc, dsc_cfg);
+ }
+
/* Initially configure all planes */
for (i = 0; i < mtk_crtc->layer_nr; i++) {
struct drm_plane *plane = &mtk_crtc->planes[i];
diff --git a/drivers/gpu/drm/mediatek/mtk_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_ddp_comp.c
index 9672ea1f91a2..13aaf12ecbe5 100644
--- a/drivers/gpu/drm/mediatek/mtk_ddp_comp.c
+++ b/drivers/gpu/drm/mediatek/mtk_ddp_comp.c
@@ -40,12 +40,6 @@
#define DITHER_LSB_ERR_SHIFT_G(x) (((x) & 0x7) << 12)
#define DITHER_ADD_LSHIFT_G(x) (((x) & 0x7) << 4)
-#define DISP_REG_DSC_CON 0x0000
-#define DSC_EN BIT(0)
-#define DSC_DUAL_INOUT BIT(2)
-#define DSC_BYPASS BIT(4)
-#define DSC_UFOE_SEL BIT(16)
-
#define DISP_REG_OD_EN 0x0000
#define DISP_REG_OD_CFG 0x0020
#define OD_RELAYMODE BIT(0)
@@ -187,36 +181,6 @@ static void mtk_dither_set(struct device *dev, unsigned int bpc,
DISP_DITHERING, cmdq_pkt);
}
-static void mtk_dsc_config(struct device *dev, unsigned int w,
- unsigned int h, unsigned int vrefresh,
- unsigned int bpc, struct cmdq_pkt *cmdq_pkt)
-{
- struct mtk_ddp_comp_dev *priv = dev_get_drvdata(dev);
-
- /* dsc bypass mode */
- mtk_ddp_write_mask(cmdq_pkt, DSC_BYPASS, &priv->cmdq_reg, priv->regs,
- DISP_REG_DSC_CON, DSC_BYPASS);
- mtk_ddp_write_mask(cmdq_pkt, DSC_UFOE_SEL, &priv->cmdq_reg, priv->regs,
- DISP_REG_DSC_CON, DSC_UFOE_SEL);
- mtk_ddp_write_mask(cmdq_pkt, DSC_DUAL_INOUT, &priv->cmdq_reg, priv->regs,
- DISP_REG_DSC_CON, DSC_DUAL_INOUT);
-}
-
-static void mtk_dsc_start(struct device *dev)
-{
- struct mtk_ddp_comp_dev *priv = dev_get_drvdata(dev);
-
- /* write with mask to reserve the value set in mtk_dsc_config */
- mtk_ddp_write_mask(NULL, DSC_EN, &priv->cmdq_reg, priv->regs, DISP_REG_DSC_CON, DSC_EN);
-}
-
-static void mtk_dsc_stop(struct device *dev)
-{
- struct mtk_ddp_comp_dev *priv = dev_get_drvdata(dev);
-
- writel_relaxed(0x0, priv->regs + DISP_REG_DSC_CON);
-}
-
static void mtk_od_config(struct device *dev, unsigned int w,
unsigned int h, unsigned int vrefresh,
unsigned int bpc, struct cmdq_pkt *cmdq_pkt)
@@ -309,9 +273,9 @@ static const struct mtk_ddp_comp_funcs ddp_dpi = {
};
static const struct mtk_ddp_comp_funcs ddp_dsc = {
- .clk_enable = mtk_ddp_clk_enable,
- .clk_disable = mtk_ddp_clk_disable,
- .config = mtk_dsc_config,
+ .clk_enable = mtk_dsc_clk_enable,
+ .clk_disable = mtk_dsc_clk_disable,
+ .dsc_setup = mtk_dsc_setup,
.start = mtk_dsc_start,
.stop = mtk_dsc_stop,
};
@@ -671,6 +635,7 @@ int mtk_ddp_comp_init(struct device *dev, struct device_node *node, struct mtk_d
type == MTK_DISP_BLS ||
type == MTK_DISP_CCORR ||
type == MTK_DISP_COLOR ||
+ type == MTK_DISP_DSC ||
type == MTK_DISP_GAMMA ||
type == MTK_DISP_MERGE ||
type == MTK_DISP_OVL ||
diff --git a/drivers/gpu/drm/mediatek/mtk_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_ddp_comp.h
index 3f3d43f4330d..99bf1e1015da 100644
--- a/drivers/gpu/drm/mediatek/mtk_ddp_comp.h
+++ b/drivers/gpu/drm/mediatek/mtk_ddp_comp.h
@@ -20,6 +20,7 @@ struct drm_crtc;
struct drm_device;
struct mtk_plane_state;
struct drm_crtc_state;
+struct drm_dsc_config;
enum mtk_ddp_comp_type {
MTK_DISP_AAL,
@@ -56,6 +57,7 @@ struct mtk_ddp_comp_funcs {
void (*config)(struct device *dev, unsigned int w,
unsigned int h, unsigned int vrefresh,
unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
+ void (*dsc_setup)(struct device *dev, struct drm_dsc_config *dsc);
void (*start)(struct device *dev);
void (*stop)(struct device *dev);
void (*register_vblank_cb)(struct device *dev,
@@ -149,6 +151,13 @@ static inline void mtk_ddp_comp_config(struct mtk_ddp_comp *comp,
comp->funcs->config(comp->dev, w, h, vrefresh, bpc, cmdq_pkt);
}
+static inline void mtk_ddp_comp_dsc_setup(struct mtk_ddp_comp *comp,
+ struct drm_dsc_config *dsc)
+{
+ if (comp->funcs && comp->funcs->dsc_setup)
+ comp->funcs->dsc_setup(comp->dev, dsc);
+}
+
static inline void mtk_ddp_comp_start(struct mtk_ddp_comp *comp)
{
if (comp->funcs && comp->funcs->start)
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
index 679d413bf10b..5e2d8748120a 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
+++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
@@ -12,6 +12,8 @@
#include "mtk_mdp_rdma.h"
#include "mtk_plane.h"
+struct drm_dsc_config;
+
int mtk_aal_clk_enable(struct device *dev);
void mtk_aal_clk_disable(struct device *dev);
void mtk_aal_config(struct device *dev, unsigned int w,
@@ -47,9 +49,16 @@ void mtk_dpi_start(struct device *dev);
void mtk_dpi_stop(struct device *dev);
unsigned int mtk_dpi_encoder_index(struct device *dev);
+int mtk_dsc_clk_enable(struct device *dev);
+void mtk_dsc_clk_disable(struct device *dev);
+void mtk_dsc_setup(struct device *dev, struct drm_dsc_config *dsc_cfg);
+void mtk_dsc_start(struct device *dev);
+void mtk_dsc_stop(struct device *dev);
+
void mtk_dsi_ddp_start(struct device *dev);
void mtk_dsi_ddp_stop(struct device *dev);
unsigned int mtk_dsi_encoder_index(struct device *dev);
+struct drm_dsc_config *mtk_dsi_get_dsc_config(struct device *dev);
int mtk_gamma_clk_enable(struct device *dev);
void mtk_gamma_clk_disable(struct device *dev);
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_dsc.c b/drivers/gpu/drm/mediatek/mtk_disp_dsc.c
new file mode 100644
index 000000000000..bed6b77bf9a9
--- /dev/null
+++ b/drivers/gpu/drm/mediatek/mtk_disp_dsc.c
@@ -0,0 +1,455 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2021 MediaTek Inc.
+ * Copyright (c) 2025 Collabora Ltd
+ * AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/component.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/soc/mediatek/mtk-cmdq.h>
+
+#include <drm/display/drm_dsc.h>
+#include <drm/display/drm_dsc_helper.h>
+
+#include "mtk_crtc.h"
+#include "mtk_ddp_comp.h"
+#include "mtk_disp_drv.h"
+
+#define DISP_REG_DSC_CON 0x0
+# define DSC_EN BIT(0)
+# define DSC_DUAL_INOUT BIT(2)
+# define DSC_IN_SRC_SEL BIT(3)
+# define DSC_BYPASS BIT(4)
+# define DSC_RELAY BIT(5)
+# define DSC_V1_1_EXT BIT(6)
+# define DSC_PT_MEM_EN BIT(7)
+# define DSC_SW_RESET BIT(8)
+# define DSC_EMPTY_FLAG_SEL GENMASK(15, 14)
+ # define DSC_EMPTY_FLAG_NORMAL_DET 0
+ # define DSC_EMPTY_FLAG_ALWAYS_HIGH 1
+ # define DSC_EMPTY_FLAG_ALWAYS_LOW 2
+ # define DSC_EMPTY_FLAG_DO_NOT_SEND 3
+# define DSC_UFOE_SEL BIT(16)
+# define DSC_OUTPUT_SWAP BIT(18)
+# define DSC_ZERO_FIFO_STALL_DISABLE BIT(20)
+
+#define DISP_REG_DSC_SPR 0x14
+#define DISP_REG_DSC_PIC_W 0x18
+# define DSC_PIC_WIDTH GENMASK(15, 0)
+# define DSC_PIC_GROUP_WIDTH_M1 GENMASK(31, 16)
+
+#define DISP_REG_DSC_PIC_H 0x1c
+# define DSC_PIC_HEIGHT GENMASK(15, 0)
+# define DSC_PIC_HEIGHT_EXT_M1 GENMASK(31, 16)
+
+#define DISP_REG_DSC_SLICE_W 0x20
+# define DSC_SLICE_WIDTH GENMASK(15, 0)
+# define DSC_SLICE_GROUP_WIDTH_M1 GENMASK(31, 16)
+
+#define DISP_REG_DSC_SLICE_H 0x24
+# define DSC_SLICE_HEIGHT_M1 GENMASK(15, 0)
+# define DSC_SLICE_NUM_M1 GENMASK(29, 16)
+# define DSC_SLICE_WIDTH_MOD3 GENMASK(31, 30)
+
+#define DISP_REG_DSC_CHUNK_SIZE 0x28
+
+#define DISP_REG_DSC_BUF_SIZE 0x2c
+# define DISP_DSC_BUF_SIZE_MASK GENMASK(23, 0)
+
+#define DISP_REG_DSC_MODE 0x30
+# define DSC_RGB_SWAP BIT(2)
+# define DSC_INIT_DELAY_HEIGHT GENMASK(11, 8)
+
+#define DISP_REG_DSC_CFG 0x34
+# define DSC_CFG_FLATNESS_DET_THRES GENMASK(4, 0)
+# define DSC_CFG_FLATNESS_8BITS 2
+# define DSC_CFG_FLATNESS_10BITS 8
+# define DSC_CFG_ICH_EN BIT(5)
+# define DSC_CFG_ICH_LINE_CLEAR GENMASK(7, 6)
+# define DSC_CFG_V1P1 BIT(8)
+# define DSC_CFG_IDLE_MODE BIT(9)
+# define DSC_CFG_CRC_EN BIT(12)
+# define DSC_CFG_DSC12_BUGFIX BIT(14)
+# define DSC_CFG_CORE_CHECKSUM BIT(15)
+
+#define DISP_REG_DSC_PAD 0x38
+# define DSC_PAD_NUMBER GENMASK(2, 0)
+
+#define DISP_REG_DSC_ENC_WIDTH 0x3c
+# define DSC_ENC_WIDTH_SLICE GENMASK(15, 0)
+# define DSC_ENC_WIDTH_PIC GENMASK(31, 16)
+
+#define DISP_REG_DSC_PIC_PRE_PAD_SIZE 0x40
+# define DSC_PIC_PREPAD_HEIGHT GENMASK(15, 0)
+# define DSC_PIC_PREPAD_WIDTH GENMASK(31, 16)
+
+#define DISP_REG_DSC_DBG_CON 0x60
+# define DSC_CKSM_CAL_EN BIT(9)
+
+#define DISP_REG_DSC_OUTBUF 0x70
+# define DSC_OBUF_SIZE GENMASK(11, 0)
+
+#define DISP_REG_DSC_PPS(x) (0x80 + (x * 4)) /* 0..19 */
+# define DSC_P0_UP_LINE_BUF_DEPTH GENMASK(3, 0)
+# define DSC_P0_BPC GENMASK(7, 4)
+# define DSC_P0_BPP GENMASK(17, 8)
+# define DSC_P0_RCT_ON BIT(18)
+# define DSC_P0_BLOCK_PRED_EN BIT(19)
+# define DSC_P1_INITIAL_XMIT_DELAY GENMASK(15, 0)
+# define DSC_P1_INITIAL_DEC_DELAY GENMASK(31, 16)
+# define DSC_P2_INITIAL_SCALE_VALUE GENMASK(15, 0)
+# define DSC_P2_SCALE_INCR_INTERVAL GENMASK(31, 16)
+# define DSC_P3_SCALE_DECR_INTERVAL GENMASK(15, 0)
+# define DSC_P3_FIRST_LINE_BPG_OFFSET GENMASK(31, 16)
+# define DSC_P4_NFL_BPG_OFFSET GENMASK(15, 0)
+# define DSC_P4_SLICE_BPG_OFFSET GENMASK(31, 16)
+# define DSC_P5_INITIAL_OFFSET GENMASK(15, 0)
+# define DSC_P5_FINAL_OFFSET GENMASK(31, 16)
+# define DSC_P6_FLATNESS_MIN_QP GENMASK(4, 0)
+# define DSC_P6_FLATNESS_MAX_QP GENMASK(12, 8)
+# define DSC_P6_RC_MODEL_SIZE GENMASK(31, 16)
+# define DSC_P7_RC_EDGE_FACTOR GENMASK(7, 0)
+# define DSC_P7_RC_QUANT_INCR_LIMIT0 GENMASK(12, 8)
+# define DSC_P7_RC_QUANT_INCR_LIMIT1 GENMASK(20, 16)
+# define DSC_P7_RC_TGT_OFFSET_HI GENMASK(27, 24)
+# define DSC_P7_RC_TGT_OFFSET_LO GENMASK(31, 28)
+# define DSC_P8_RC_BUF_THR_X GENMASK(7, 0)
+# define DSC_P12_RC_RANGE_MIN_QP GENMASK(4, 0)
+# define DSC_P12_RC_RANGE_MAX_QP GENMASK(9, 5)
+# define DSC_P12_RC_RANGE_BPG_OFFSET GENMASK(15, 10)
+
+#define DISP_REG_DSC_SHADOW 0x200
+# define DSC_FORCE_COMMIT BIT(0)
+# define DSC_BYPASS_SHADOW BIT(1)
+# define DSC_READ_WORKING BIT(2)
+# define DSC_SHADOW_DSC_VERSION_MINOR GENMASK(8, 5)
+
+struct mtk_dsc {
+ struct clk *clk;
+ void __iomem *reg;
+ struct cmdq_client_reg cmdq_reg;
+ bool dsc_config_done;
+};
+
+int mtk_dsc_clk_enable(struct device *dev)
+{
+ struct mtk_dsc *disp_dsc = dev_get_drvdata(dev);
+
+ return clk_prepare_enable(disp_dsc->clk);
+}
+
+void mtk_dsc_clk_disable(struct device *dev)
+{
+ struct mtk_dsc *disp_dsc = dev_get_drvdata(dev);
+
+ clk_disable_unprepare(disp_dsc->clk);
+}
+
+static void mtk_dsc_pps_setup(struct mtk_dsc *disp_dsc, struct drm_dsc_config *dsc_cfg)
+{
+ struct drm_dsc_rc_range_parameters *rcrp = dsc_cfg->rc_range_params;
+ u16 *rbt = dsc_cfg->rc_buf_thresh;
+ u32 data;
+ int i, j;
+
+ /* PPS 0 - Note: Fractional BPP is not supported! */
+ data = FIELD_PREP(DSC_P0_UP_LINE_BUF_DEPTH, dsc_cfg->line_buf_depth);
+ data |= FIELD_PREP(DSC_P0_BPC, dsc_cfg->bits_per_component);
+ data |= FIELD_PREP(DSC_P0_BPP, dsc_cfg->bits_per_pixel);
+ data |= DSC_P0_RCT_ON | DSC_P0_BLOCK_PRED_EN;
+ writel(data, disp_dsc->reg + DISP_REG_DSC_PPS(0));
+
+ /* PPS 1 */
+ data = FIELD_PREP(DSC_P1_INITIAL_XMIT_DELAY, dsc_cfg->initial_xmit_delay);
+ data |= FIELD_PREP(DSC_P1_INITIAL_DEC_DELAY, dsc_cfg->initial_dec_delay);
+ writel(data, disp_dsc->reg + DISP_REG_DSC_PPS(1));
+
+ /* PPS 2 */
+ data = FIELD_PREP(DSC_P2_INITIAL_SCALE_VALUE, dsc_cfg->initial_scale_value);
+ data |= FIELD_PREP(DSC_P2_SCALE_INCR_INTERVAL, dsc_cfg->scale_increment_interval);
+ writel(data, disp_dsc->reg + DISP_REG_DSC_PPS(2));
+
+ /* PPS 3 */
+ data = FIELD_PREP(DSC_P3_SCALE_DECR_INTERVAL, dsc_cfg->scale_decrement_interval);
+ data |= FIELD_PREP(DSC_P3_FIRST_LINE_BPG_OFFSET, dsc_cfg->first_line_bpg_offset);
+ writel(data, disp_dsc->reg + DISP_REG_DSC_PPS(3));
+
+ /* PPS 4 */
+ data = FIELD_PREP(DSC_P4_NFL_BPG_OFFSET, dsc_cfg->nfl_bpg_offset);
+ data |= FIELD_PREP(DSC_P4_SLICE_BPG_OFFSET, dsc_cfg->slice_bpg_offset);
+ writel(data, disp_dsc->reg + DISP_REG_DSC_PPS(4));
+
+ /* PPS 5 */
+ data = FIELD_PREP(DSC_P5_INITIAL_OFFSET, dsc_cfg->initial_offset);
+ data |= FIELD_PREP(DSC_P5_FINAL_OFFSET, dsc_cfg->final_offset);
+ writel(data, disp_dsc->reg + DISP_REG_DSC_PPS(5));
+
+ /* PPS 6 */
+ data = FIELD_PREP(DSC_P6_FLATNESS_MIN_QP, dsc_cfg->flatness_min_qp);
+ data |= FIELD_PREP(DSC_P6_FLATNESS_MAX_QP, dsc_cfg->flatness_max_qp);
+ writel(data, disp_dsc->reg + DISP_REG_DSC_PPS(6));
+
+ /* PPS 7 */
+ data = FIELD_PREP(DSC_P7_RC_EDGE_FACTOR, dsc_cfg->rc_edge_factor);
+ data |= FIELD_PREP(DSC_P7_RC_QUANT_INCR_LIMIT0, dsc_cfg->rc_quant_incr_limit0);
+ data |= FIELD_PREP(DSC_P7_RC_QUANT_INCR_LIMIT1, dsc_cfg->rc_quant_incr_limit1);
+ data |= FIELD_PREP(DSC_P7_RC_TGT_OFFSET_HI, dsc_cfg->rc_tgt_offset_high);
+ data |= FIELD_PREP(DSC_P7_RC_TGT_OFFSET_LO, dsc_cfg->rc_tgt_offset_low);
+ writel(data, disp_dsc->reg + DISP_REG_DSC_PPS(7));
+
+ /* PPS 8..11 - Each register holds 4 RC buffer thresholds (PPS 11 has two) */
+ for (i = 0; i < 4; i++) {
+ u8 block_num = i * 4;
+ data = 0;
+
+ for (j = 0; j < 4; j++) {
+ u8 buf_index = block_num + j;
+ u8 data_shift = j * 8;
+
+ /* rc_buf_thresh holds 14 elements in total */
+ if (buf_index > 13)
+ break;
+
+ data |= (rbt[buf_index] & DSC_P8_RC_BUF_THR_X) << data_shift;
+ }
+ writel(data, disp_dsc->reg + DISP_REG_DSC_PPS(8 + i));
+ }
+
+ /* PPS 12..18 - Each register holds two sets of RC range parameters */
+ for (i = 0; i < 7; i++) {
+ u8 block_num = i * 2;
+ data = 0;
+
+ for (j = 0; j < 2; j++) {
+ u8 buf_index = block_num + j;
+ u8 data_shift = j * 16;
+ u32 range_data;
+
+ range_data = FIELD_PREP(DSC_P12_RC_RANGE_MIN_QP,
+ rcrp[buf_index].range_min_qp);
+ range_data |= FIELD_PREP(DSC_P12_RC_RANGE_MAX_QP,
+ rcrp[buf_index].range_max_qp);
+ range_data |= FIELD_PREP(DSC_P12_RC_RANGE_BPG_OFFSET,
+ rcrp[buf_index].range_bpg_offset);
+ data |= range_data << data_shift;
+
+ /* rc_range_params holds 15 elements in total */
+ if (buf_index == 13)
+ break;
+ }
+ writel(data, disp_dsc->reg + DISP_REG_DSC_PPS(12 + i));
+ }
+
+ /* PPS 19 - Last RC range register, holds only one set of parameters */
+ writel(FIELD_PREP(DSC_P12_RC_RANGE_MIN_QP, rcrp[14].range_min_qp) |
+ FIELD_PREP(DSC_P12_RC_RANGE_MAX_QP, rcrp[14].range_max_qp) |
+ FIELD_PREP(DSC_P12_RC_RANGE_BPG_OFFSET, rcrp[14].range_bpg_offset),
+ disp_dsc->reg + DISP_REG_DSC_PPS(19));
+}
+
+void mtk_dsc_setup(struct device *dev, struct drm_dsc_config *dsc_cfg)
+{
+ struct mtk_dsc *disp_dsc = dev_get_drvdata(dev);
+ u32 dsc_slice_w, dsc_slice_h, dsc_mode, dsc_cfg_rval, dsc_shadow;
+ u32 dsc_dbg_con, dsc_con, dsc_enc_width, dsc_pic_w, dsc_pic_h;
+ u32 pic_group_width, pic_height_ext_num, slice_group_width;
+ u32 chunk_size, dsc_pad_num, dsc_pre_pad_sz;
+ bool dsc_en_bit;
+
+ pic_height_ext_num = dsc_cfg->pic_height + dsc_cfg->slice_height - 1;
+ pic_group_width = dsc_cfg->slice_width * 4;
+ pic_group_width /= 3;
+
+ slice_group_width = dsc_cfg->slice_width + 2;
+ slice_group_width /= 3;
+
+ if (dsc_cfg->slice_chunk_size)
+ chunk_size = dsc_cfg->slice_chunk_size;
+ else
+ chunk_size = dsc_cfg->slice_width * dsc_cfg->bits_per_pixel / 8 / 16;
+
+ dsc_enc_width = FIELD_PREP(DSC_ENC_WIDTH_PIC, dsc_cfg->pic_width) |
+ FIELD_PREP(DSC_ENC_WIDTH_SLICE, dsc_cfg->slice_width);
+
+ dsc_pic_w = FIELD_PREP(DSC_PIC_GROUP_WIDTH_M1, pic_group_width - 1);
+ dsc_pic_w |= FIELD_PREP(DSC_PIC_WIDTH, dsc_cfg->pic_width);
+ dsc_pic_h = FIELD_PREP(DSC_PIC_HEIGHT_EXT_M1, pic_height_ext_num - 1);
+ dsc_pic_h |= FIELD_PREP(DSC_PIC_HEIGHT, dsc_cfg->pic_height - 1);
+
+ dsc_slice_w = FIELD_PREP(DSC_SLICE_GROUP_WIDTH_M1, slice_group_width - 1);
+ dsc_slice_w |= FIELD_PREP(DSC_SLICE_WIDTH, dsc_cfg->slice_width);
+ dsc_slice_h = FIELD_PREP(DSC_SLICE_WIDTH_MOD3, dsc_cfg->slice_width % 3);
+ dsc_slice_h |= FIELD_PREP(DSC_SLICE_NUM_M1,
+ (pic_height_ext_num / dsc_cfg->slice_height) - 1);
+ dsc_slice_h |= FIELD_PREP(DSC_SLICE_HEIGHT_M1, dsc_cfg->slice_height - 1);
+
+ dsc_pad_num = (3 - ((chunk_size * 2) % 3)) % 3;
+ dsc_pad_num = FIELD_PREP(DSC_PAD_NUMBER, dsc_pad_num);
+
+ dsc_pre_pad_sz = FIELD_PREP(DSC_PIC_PREPAD_HEIGHT, dsc_cfg->pic_height);
+ dsc_pre_pad_sz |= FIELD_PREP(DSC_PIC_PREPAD_WIDTH, dsc_cfg->pic_width);
+
+ dsc_mode = FIELD_PREP(DSC_INIT_DELAY_HEIGHT, 4);
+ dsc_mode |= FIELD_PREP(DSC_RGB_SWAP, 0);
+
+ /* Must enable checksum calc in DBG if enabling core checksum in CFG */
+ dsc_cfg_rval = DSC_CFG_ICH_EN | DSC_CFG_CRC_EN | DSC_CFG_DSC12_BUGFIX |
+ DSC_CFG_CORE_CHECKSUM;
+ dsc_dbg_con = DSC_CKSM_CAL_EN;
+
+ if (dsc_cfg->bits_per_component == 8)
+ dsc_cfg_rval |= FIELD_PREP_CONST(DSC_CFG_FLATNESS_DET_THRES,
+ DSC_CFG_FLATNESS_8BITS);
+ else
+ dsc_cfg_rval |= FIELD_PREP_CONST(DSC_CFG_FLATNESS_DET_THRES,
+ DSC_CFG_FLATNESS_10BITS);
+
+ dsc_shadow = FIELD_PREP(DSC_SHADOW_DSC_VERSION_MINOR,
+ dsc_cfg->dsc_version_minor);
+ dsc_shadow |= DSC_FORCE_COMMIT | DSC_BYPASS_SHADOW;
+
+ /* If DSC is currently enabled, disable it before setup and re-enable after */
+ dsc_con = readl(disp_dsc->reg + DISP_REG_DSC_CON);
+ if (dsc_con & DSC_EN) {
+ dsc_en_bit = true;
+ writel(dsc_con & ~DSC_EN, disp_dsc->reg + DISP_REG_DSC_CON);
+ } else {
+ dsc_en_bit = false;
+ }
+
+ writel(0, disp_dsc->reg + DISP_REG_DSC_SPR);
+ writel(dsc_enc_width, disp_dsc->reg + DISP_REG_DSC_ENC_WIDTH);
+ writel(dsc_pic_w, disp_dsc->reg + DISP_REG_DSC_PIC_W);
+ writel(dsc_pic_h, disp_dsc->reg + DISP_REG_DSC_PIC_H);
+ writel(dsc_slice_w, disp_dsc->reg + DISP_REG_DSC_SLICE_W);
+ writel(dsc_slice_h, disp_dsc->reg + DISP_REG_DSC_SLICE_H);
+ writel(((chunk_size * 4) / 3) << 16 | chunk_size,
+ disp_dsc->reg + DISP_REG_DSC_CHUNK_SIZE);
+ writel(dsc_pre_pad_sz, disp_dsc->reg + DISP_REG_DSC_PIC_PRE_PAD_SIZE);
+ writel(dsc_pad_num, disp_dsc->reg + DISP_REG_DSC_PAD);
+ writel(FIELD_PREP(DISP_DSC_BUF_SIZE_MASK, chunk_size * dsc_cfg->slice_height),
+ disp_dsc->reg + DISP_REG_DSC_BUF_SIZE);
+ writel(dsc_mode, disp_dsc->reg + DISP_REG_DSC_MODE);
+ writel(dsc_cfg_rval, disp_dsc->reg + DISP_REG_DSC_CFG);
+ writel(dsc_dbg_con, disp_dsc->reg + DISP_REG_DSC_DBG_CON);
+ writel(FIELD_PREP_CONST(DSC_OBUF_SIZE, 1040), disp_dsc->reg + DISP_REG_DSC_OUTBUF);
+ writel(dsc_shadow, disp_dsc->reg + DISP_REG_DSC_SHADOW);
+
+ /* Set PPS registers configuration */
+ mtk_dsc_pps_setup(disp_dsc, dsc_cfg);
+
+ dsc_con = FIELD_PREP_CONST(DSC_EMPTY_FLAG_SEL, DSC_EMPTY_FLAG_ALWAYS_LOW);
+ dsc_con |= DSC_V1_1_EXT | DSC_UFOE_SEL | DSC_PT_MEM_EN;
+ dsc_con |= DSC_ZERO_FIFO_STALL_DISABLE;
+
+ if (dsc_en_bit)
+ dsc_con |= DSC_EN;
+
+ writel(dsc_con, disp_dsc->reg + DISP_REG_DSC_CON);
+
+ disp_dsc->dsc_config_done = true;
+}
+
+void mtk_dsc_start(struct device *dev)
+{
+ struct mtk_dsc *disp_dsc = dev_get_drvdata(dev);
+
+ /* If no DSC or config not done, set bypass mode */
+ if (!disp_dsc->dsc_config_done) {
+ mtk_ddp_write_mask(NULL, DSC_BYPASS, &disp_dsc->cmdq_reg,
+ disp_dsc->reg, DISP_REG_DSC_CON, DSC_BYPASS);
+ mtk_ddp_write_mask(NULL, DSC_UFOE_SEL, &disp_dsc->cmdq_reg,
+ disp_dsc->reg, DISP_REG_DSC_CON, DSC_UFOE_SEL);
+ mtk_ddp_write_mask(NULL, DSC_DUAL_INOUT, &disp_dsc->cmdq_reg,
+ disp_dsc->reg, DISP_REG_DSC_CON, DSC_DUAL_INOUT);
+ }
+
+ mtk_ddp_write_mask(NULL, DSC_EN, &disp_dsc->cmdq_reg,
+ disp_dsc->reg, DISP_REG_DSC_CON, DSC_EN);
+}
+
+void mtk_dsc_stop(struct device *dev)
+{
+ struct mtk_dsc *disp_dsc = dev_get_drvdata(dev);
+
+ writel(0, disp_dsc->reg + DISP_REG_DSC_CON);
+}
+
+static int mtk_dsc_bind(struct device *dev, struct device *master, void *data)
+{
+ return 0;
+}
+
+static void mtk_dsc_unbind(struct device *dev, struct device *master, void *data)
+{
+}
+
+static const struct component_ops mtk_dsc_component_ops = {
+ .bind = mtk_dsc_bind,
+ .unbind = mtk_dsc_unbind,
+};
+
+static int mtk_dsc_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct mtk_dsc *priv;
+ struct resource *res;
+ int ret;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(priv->clk))
+ return dev_err_probe(dev, PTR_ERR(priv->clk),
+ "failed to get clk\n");
+
+ priv->reg = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
+ if (IS_ERR(priv->reg))
+ return dev_err_probe(dev, PTR_ERR(priv->reg),
+ "failed to do ioremap\n");
+
+#if IS_REACHABLE(CONFIG_MTK_CMDQ)
+ ret = cmdq_dev_get_client_reg(dev, &priv->cmdq_reg, 0);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to get gce client reg\n");
+#endif
+
+ platform_set_drvdata(pdev, priv);
+
+ ret = devm_pm_runtime_enable(dev);
+ if (ret)
+ return ret;
+
+ ret = component_add(dev, &mtk_dsc_component_ops);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to add component\n");
+
+ return 0;
+}
+
+static void mtk_dsc_remove(struct platform_device *pdev)
+{
+ component_del(&pdev->dev, &mtk_dsc_component_ops);
+}
+
+static const struct of_device_id mtk_dsc_driver_dt_match[] = {
+ { .compatible = "mediatek,mt8195-disp-dsc" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mtk_dsc_driver_dt_match);
+
+struct platform_driver mtk_disp_dsc_driver = {
+ .probe = mtk_dsc_probe,
+ .remove = mtk_dsc_remove,
+ .driver = {
+ .name = "mediatek-disp-dsc",
+ .of_match_table = mtk_dsc_driver_dt_match,
+ },
+};
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index c86a3f54f35b..bd2d17017bd2 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -1164,6 +1164,7 @@ static int mtk_drm_probe(struct platform_device *pdev)
if (comp_type == MTK_DISP_AAL ||
comp_type == MTK_DISP_CCORR ||
comp_type == MTK_DISP_COLOR ||
+ comp_type == MTK_DISP_DSC ||
comp_type == MTK_DISP_GAMMA ||
comp_type == MTK_DISP_MERGE ||
comp_type == MTK_DISP_OVL ||
@@ -1272,6 +1273,7 @@ static struct platform_driver * const mtk_drm_drivers[] = {
&mtk_disp_aal_driver,
&mtk_disp_ccorr_driver,
&mtk_disp_color_driver,
+ &mtk_disp_dsc_driver,
&mtk_disp_gamma_driver,
&mtk_disp_merge_driver,
&mtk_disp_ovl_adaptor_driver,
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
index d48da019a4a3..43aac2d956e7 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
@@ -71,6 +71,7 @@ struct mtk_drm_private {
extern struct platform_driver mtk_disp_aal_driver;
extern struct platform_driver mtk_disp_ccorr_driver;
extern struct platform_driver mtk_disp_color_driver;
+extern struct platform_driver mtk_disp_dsc_driver;
extern struct platform_driver mtk_disp_gamma_driver;
extern struct platform_driver mtk_disp_merge_driver;
extern struct platform_driver mtk_disp_ovl_adaptor_driver;
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 3f3f56eed3f9..8ab5c3431dbb 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -18,6 +18,8 @@
#include <video/mipi_display.h>
#include <video/videomode.h>
+#include <drm/display/drm_dsc.h>
+#include <drm/display/drm_dsc_helper.h>
#include <drm/drm_atomic_helper.h>
#include <drm/drm_bridge.h>
#include <drm/drm_bridge_connector.h>
@@ -71,11 +73,12 @@
#define DSI_PSCTRL 0x1c
#define DSI_PS_WC GENMASK(13, 0)
-#define DSI_PS_SEL GENMASK(17, 16)
+#define DSI_PS_SEL GENMASK(19, 16)
#define PACKED_PS_16BIT_RGB565 0
#define PACKED_PS_18BIT_RGB666 1
#define LOOSELY_PS_24BIT_RGB666 2
#define PACKED_PS_24BIT_RGB888 3
+#define COMPRESSED_PS_DSC 5
#define DSI_VSA_NL 0x20
#define DSI_VBP_NL 0x24
@@ -203,6 +206,7 @@ struct mtk_dsi {
struct drm_bridge bridge;
struct drm_bridge *next_bridge;
struct drm_connector *connector;
+ struct drm_dsc_config *dsc;
struct phy *phy;
void __iomem *regs;
@@ -393,9 +397,35 @@ static void mtk_dsi_rxtx_control(struct mtk_dsi *dsi)
writel(regval, dsi->regs + DSI_TXRX_CTRL);
}
-static void mtk_dsi_ps_control(struct mtk_dsi *dsi, bool config_vact)
+static void mtk_dsi_ps_control_dsc(struct mtk_dsi *dsi, bool config_vact)
+{
+ const struct mtk_dsi_driver_data *data = dsi->driver_data;
+ const u16 *reg_main = dsi->driver_data->reg_main;
+ const short dsi_buf_bpp = 3;
+ u32 ps_wc;
+
+ /* Word count */
+ ps_wc = FIELD_PREP(DSI_PS_WC, 2 * dsi->dsc->slice_chunk_size);
+
+ if (config_vact) {
+ writel(FIELD_PREP(VACT_NL, dsi->vm.vactive),
+ dsi->regs + reg_main[DSI_VACT_NL]);
+ writel(ps_wc, dsi->regs + reg_main[DSI_HSTX_CKL_WC]);
+ }
+
+ /* Always use DSC Pixel Stream type */
+ writel(ps_wc | FIELD_PREP(DSI_PS_SEL, COMPRESSED_PS_DSC),
+ dsi->regs + reg_main[DSI_PSCTRL]);
+
+ if (data->has_size_ctl)
+ writel(FIELD_PREP(DSI_HEIGHT, dsi->vm.vactive) |
+ FIELD_PREP(DSI_WIDTH, (ps_wc + dsi_buf_bpp - 1) / dsi_buf_bpp),
+ dsi->regs + reg_main[DSI_SIZE_CON]);
+}
+
+static void mtk_dsi_ps_control_uncompressed(struct mtk_dsi *dsi, bool config_vact)
{
- u32 dsi_buf_bpp, ps_val, ps_wc, vact_nl;
+ u32 dsi_buf_bpp, ps_val, ps_wc, size_val, vact_nl;
if (dsi->format == MIPI_DSI_FMT_RGB565)
dsi_buf_bpp = 2;
@@ -430,6 +460,21 @@ static void mtk_dsi_ps_control(struct mtk_dsi *dsi, bool config_vact)
writel(ps_wc, dsi->regs + DSI_HSTX_CKL_WC);
}
writel(ps_val, dsi->regs + DSI_PSCTRL);
+
+ if (dsi->driver_data->has_size_ctl) {
+ size_val = FIELD_PREP(DSI_HEIGHT, dsi->vm.vactive);
+ size_val |= FIELD_PREP(DSI_WIDTH, dsi->vm.hactive);
+
+ writel(size_val, dsi->regs + DSI_SIZE_CON);
+ }
+}
+
+static void mtk_dsi_ps_control(struct mtk_dsi *dsi, bool config_vact)
+{
+ if (dsi->dsc)
+ mtk_dsi_ps_control_dsc(dsi, config_vact);
+ else
+ mtk_dsi_ps_control_uncompressed(dsi, config_vact);
}
static void mtk_dsi_config_vdo_timing_per_frame_lp(struct mtk_dsi *dsi)
@@ -565,26 +610,68 @@ static void mtk_dsi_config_vdo_timing_per_line_lp(struct mtk_dsi *dsi)
writel(horizontal_frontporch_byte, dsi->regs + DSI_HFP_WC);
}
-static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi)
+static int mtk_dsi_set_dsc_params(struct mtk_dsi *dsi)
+{
+ struct drm_dsc_config *dsc = dsi->dsc;
+ struct device *dev = dsi->host.dev;
+ int ret;
+
+ if (dsc->bits_per_pixel & GENMASK(3, 0)) {
+ dev_err(dev, "Fractional bits_per_pixel not supported\n");
+ return -EINVAL;
+ }
+
+ if (dsc->bits_per_component != 8) {
+ dev_err(dev, "%u bits per component is not supported\n",
+ dsc->bits_per_component);
+ return -EINVAL;
+ }
+
+ dsc->simple_422 = false;
+ dsc->convert_rgb = true;
+ dsc->vbr_enable = false;
+
+ drm_dsc_set_const_params(dsc);
+ drm_dsc_set_rc_buf_thresh(dsc);
+
+ ret = drm_dsc_setup_rc_params(dsc, DRM_DSC_1_1_PRE_SCR);
+ if (ret) {
+ dev_err(dev, "Cannot find DSC RC params\n");
+ return ret;
+ }
+
+ dsc->initial_scale_value = drm_dsc_initial_scale_value(dsc);
+ dsc->line_buf_depth = dsc->bits_per_component + 1;
+
+ return drm_dsc_compute_rc_parameters(dsc);
+}
+
+static int mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi)
{
struct videomode *vm = &dsi->vm;
+ int ret;
writel(vm->vsync_len, dsi->regs + DSI_VSA_NL);
writel(vm->vback_porch, dsi->regs + DSI_VBP_NL);
writel(vm->vfront_porch, dsi->regs + DSI_VFP_NL);
writel(vm->vactive, dsi->regs + DSI_VACT_NL);
- if (dsi->driver_data->has_size_ctl)
- writel(FIELD_PREP(DSI_HEIGHT, vm->vactive) |
- FIELD_PREP(DSI_WIDTH, vm->hactive),
- dsi->regs + DSI_SIZE_CON);
-
if (dsi->driver_data->support_per_frame_lp)
mtk_dsi_config_vdo_timing_per_frame_lp(dsi);
else
mtk_dsi_config_vdo_timing_per_line_lp(dsi);
- mtk_dsi_ps_control(dsi, false);
+ if (dsi->dsc) {
+ ret = mtk_dsi_set_dsc_params(dsi);
+ if (ret)
+ return ret;
+
+ mtk_dsi_ps_control(dsi, true);
+ } else {
+ mtk_dsi_ps_control(dsi, false);
+ }
+
+ return 0;
}
static void mtk_dsi_start(struct mtk_dsi *dsi)
@@ -741,12 +828,19 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
mtk_dsi_ps_control(dsi, true);
mtk_dsi_set_vm_cmd(dsi);
- mtk_dsi_config_vdo_timing(dsi);
+ ret = mtk_dsi_config_vdo_timing(dsi);
+ if (ret)
+ goto err_disable_dsi_and_digital_clk;
+
mtk_dsi_set_interrupt_enable(dsi);
mtk_dsi_lane_ready(dsi);
mtk_dsi_clk_hs_mode(dsi, 1);
return 0;
+
+err_disable_dsi_and_digital_clk:
+ mtk_dsi_disable(dsi);
+ clk_disable_unprepare(dsi->digital_clk);
err_disable_engine_clk:
clk_disable_unprepare(dsi->engine_clk);
err_phy_power_off:
@@ -883,6 +977,28 @@ mtk_dsi_bridge_mode_valid(struct drm_bridge *bridge,
if (mode->clock * bpp / dsi->lanes > 1500000)
return MODE_CLOCK_HIGH;
+ if (dsi->dsc) {
+ if (dsi->dsc->slice_width == 0 || dsi->dsc->slice_height == 0) {
+ dev_err(dsi->host.dev,
+ "DSC: Slice width %u height %u not valid!\n",
+ dsi->dsc->slice_width, dsi->dsc->slice_height);
+ return MODE_BAD;
+ }
+
+ if (mode->hdisplay % dsi->dsc->slice_width) {
+ dev_dbg(dsi->host.dev,
+ "DSC: hdisplay %u is not a multiple of slice width %u\n",
+ dsi->dsc->slice_width, mode->hdisplay);
+ return MODE_H_ILLEGAL;
+ }
+ if (mode->vdisplay % dsi->dsc->slice_height) {
+ dev_dbg(dsi->host.dev,
+ "DSC: vdisplay %u is not a multiple of slice height %u\n",
+ dsi->dsc->slice_height, mode->vdisplay);
+ return MODE_V_ILLEGAL;
+ }
+ }
+
return MODE_OK;
}
@@ -913,6 +1029,13 @@ void mtk_dsi_ddp_stop(struct device *dev)
mtk_dsi_poweroff(dsi);
}
+struct drm_dsc_config *mtk_dsi_get_dsc_config(struct device *dev)
+{
+ struct mtk_dsi *dsi = dev_get_drvdata(dev);
+
+ return dsi->dsc;
+}
+
static int mtk_dsi_encoder_init(struct drm_device *drm, struct mtk_dsi *dsi)
{
int ret;
@@ -1006,12 +1129,16 @@ static int mtk_dsi_host_attach(struct mipi_dsi_host *host,
return PTR_ERR(dsi->next_bridge);
}
+ if (device->dsc)
+ dsi->dsc = device->dsc;
+
drm_bridge_add(&dsi->bridge);
ret = component_add(host->dev, &mtk_dsi_component_ops);
if (ret) {
drm_err(drm, "failed to add dsi_host component: %d\n", ret);
drm_bridge_remove(&dsi->bridge);
+ dsi->dsc = NULL;
return ret;
}
@@ -1025,6 +1152,8 @@ static int mtk_dsi_host_detach(struct mipi_dsi_host *host,
component_del(host->dev, &mtk_dsi_component_ops);
drm_bridge_remove(&dsi->bridge);
+ dsi->dsc = NULL;
+
return 0;
}
--
2.54.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v4 02/10] drm/mediatek: Implement Display Stream Compression support
2026-07-13 14:27 ` [PATCH v4 02/10] drm/mediatek: Implement Display Stream Compression support AngeloGioacchino Del Regno
@ 2026-07-13 14:41 ` sashiko-bot
2026-07-14 8:09 ` AngeloGioacchino Del Regno
0 siblings, 1 reply; 20+ messages in thread
From: sashiko-bot @ 2026-07-13 14:41 UTC (permalink / raw)
To: AngeloGioacchino Del Regno; +Cc: robh, devicetree, conor+dt, dri-devel
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Truncation of RC buffer thresholds due to incorrect bitwise masking instead of right-shifting.
- [High] Hardcoded and conflicting slice count assumptions without panel validation.
- [High] Component discovery loop skips DSI component, preventing DSC initialization.
- [High] DSI timing calculation ignores compression and overwrites D-PHY clock lane register.
- [Medium] State retention bug where `dsc_config_done` is never reset to false.
--
--- Patch [2]: [PATCH v4 02/10] drm/mediatek: Implement Display Stream Compression support ---
commit 94686398e3eedc1a9e39476763b6747a4bcfb4c9
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Subject: drm/mediatek: Implement Display Stream Compression support
This commit introduces a new driver for the Display Stream Compression (DSC)
Display Controller IP, implementing support for DSC v1.1 to v1.2. It adds
DSC handling into the DSI driver and integrates the bypass setup via CPU
writes instead of CMDQ.
> diff --git a/drivers/gpu/drm/mediatek/mtk_crtc.c b/drivers/gpu/drm/mediatek/mtk_crtc.c
> index 8e552cdc3b53b..d4be40b2574a1 100644
> --- a/drivers/gpu/drm/mediatek/mtk_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_crtc.c
[ ... ]
> @@ -398,6 +401,17 @@ static int mtk_crtc_ddp_hw_init(struct mtk_crtc *mtk_crtc)
> if (!mtk_ddp_comp_add(mtk_crtc->ddp_comp[i], mtk_crtc->mutex))
> mtk_mutex_add_comp(mtk_crtc->mutex,
> mtk_crtc->ddp_comp[i]->id);
> +
> + /* For now, only single DSI is supported */
> + if (mtk_crtc->ddp_comp[i]->id >= DDP_COMPONENT_DSI0 &&
> + mtk_crtc->ddp_comp[i]->id <= DDP_COMPONENT_DSI3)
> + if (!comp_dsi)
> + comp_dsi = mtk_crtc->ddp_comp[i];
> +
[Severity: High]
Does this loop terminate one iteration early, causing the DSI component
to be skipped?
Because the loop condition is i < mtk_crtc->ddp_comp_nr - 1, and the DSI
interface is typically the final component in the display pipeline, it
appears comp_dsi will remain NULL here. This would cause the subsequent
DSC initialization check to silently fail and bypass the DSC setup entirely.
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_dsc.c b/drivers/gpu/drm/mediatek/mtk_disp_dsc.c
> new file mode 100644
> index 0000000000000..bed6b77bf9a9e
> --- /dev/null
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_dsc.c
[ ... ]
> +static void mtk_dsc_pps_setup(struct mtk_dsc *disp_dsc, struct drm_dsc_config *dsc_cfg)
> +{
[ ... ]
> + /* PPS 8..11 - Each register holds 4 RC buffer thresholds (PPS 11 has two) */
> + for (i = 0; i < 4; i++) {
> + u8 block_num = i * 4;
> + data = 0;
> +
> + for (j = 0; j < 4; j++) {
> + u8 buf_index = block_num + j;
> + u8 data_shift = j * 8;
> +
> + /* rc_buf_thresh holds 14 elements in total */
> + if (buf_index > 13)
> + break;
> +
> + data |= (rbt[buf_index] & DSC_P8_RC_BUF_THR_X) << data_shift;
[Severity: High]
Does this code truncate the RC buffer thresholds by masking with 0xFF
instead of shifting right?
The VESA DSC specification requires programming the Picture Parameter Set
registers with RC buffer thresholds divided by 64. Using a bitwise AND mask
instead of shifting right by 6 appears to write truncated values to the PPS
registers, which could break rate control.
[ ... ]
> +void mtk_dsc_setup(struct device *dev, struct drm_dsc_config *dsc_cfg)
> +{
[ ... ]
> + pic_height_ext_num = dsc_cfg->pic_height + dsc_cfg->slice_height - 1;
> + pic_group_width = dsc_cfg->slice_width * 4;
> + pic_group_width /= 3;
[Severity: High]
Does this calculation hardcode the picture group width to assume exactly
four slices per line?
If a panel requests a different slice count, this could conflict with the
payload size assumptions made elsewhere in the pipeline and potentially
cause display corruption.
[ ... ]
> + if (dsc_en_bit)
> + dsc_con |= DSC_EN;
> +
> + writel(dsc_con, disp_dsc->reg + DISP_REG_DSC_CON);
> +
> + disp_dsc->dsc_config_done = true;
> +}
[Severity: Medium]
Can this state retention cause issues on subsequent modesets?
Since the dsc_config_done flag is never reset to false on teardown, will a
subsequent non-DSC modeset bypass the check below in mtk_dsc_start() and
leave the DSC IP active with stale parameters?
[ ... ]
> +void mtk_dsc_start(struct device *dev)
> +{
> + struct mtk_dsc *disp_dsc = dev_get_drvdata(dev);
> +
> + /* If no DSC or config not done, set bypass mode */
> + if (!disp_dsc->dsc_config_done) {
[ ... ]
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index 3f3f56eed3f92..8ab5c3431dbbc 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
[ ... ]
> +static void mtk_dsi_ps_control_dsc(struct mtk_dsi *dsi, bool config_vact)
> +{
> + const struct mtk_dsi_driver_data *data = dsi->driver_data;
> + const u16 *reg_main = dsi->driver_data->reg_main;
> + const short dsi_buf_bpp = 3;
> + u32 ps_wc;
> +
> + /* Word count */
> + ps_wc = FIELD_PREP(DSI_PS_WC, 2 * dsi->dsc->slice_chunk_size);
[Severity: High]
Does this hardcode the DSI payload to assume exactly two slices per line?
This appears to conflict with the assumption of four slices per line inside
mtk_dsc_setup(). Is it possible to validate the panel's requested slice
count against these assumptions during the mode_valid check?
> +
> + if (config_vact) {
> + writel(FIELD_PREP(VACT_NL, dsi->vm.vactive),
> + dsi->regs + reg_main[DSI_VACT_NL]);
> + writel(ps_wc, dsi->regs + reg_main[DSI_HSTX_CKL_WC]);
[Severity: High]
Will writing the compressed payload size here overwrite the correctly
calculated D-PHY High-Speed Clock Lane Word Count?
[ ... ]
> -static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi)
> +static int mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi)
> {
> struct videomode *vm = &dsi->vm;
> + int ret;
>
> writel(vm->vsync_len, dsi->regs + DSI_VSA_NL);
> writel(vm->vback_porch, dsi->regs + DSI_VBP_NL);
> writel(vm->vfront_porch, dsi->regs + DSI_VFP_NL);
> writel(vm->vactive, dsi->regs + DSI_VACT_NL);
>
> - if (dsi->driver_data->has_size_ctl)
> - writel(FIELD_PREP(DSI_HEIGHT, vm->vactive) |
> - FIELD_PREP(DSI_WIDTH, vm->hactive),
> - dsi->regs + DSI_SIZE_CON);
> -
> if (dsi->driver_data->support_per_frame_lp)
> mtk_dsi_config_vdo_timing_per_frame_lp(dsi);
> else
> mtk_dsi_config_vdo_timing_per_line_lp(dsi);
>
> - mtk_dsi_ps_control(dsi, false);
> + if (dsi->dsc) {
> + ret = mtk_dsi_set_dsc_params(dsi);
> + if (ret)
> + return ret;
> +
> + mtk_dsi_ps_control(dsi, true);
[Severity: High]
Do the DSI lane timings calculated earlier in
mtk_dsi_config_vdo_timing_per_frame_lp() incorrectly use the uncompressed
pixel count?
By passing config_vact = true into mtk_dsi_ps_control() for DSC streams, it
looks like the raw compressed payload size will subsequently overwrite the
timing synchronized register DSI_HSTX_CKL_WC.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260713142746.40259-1-angelogioacchino.delregno@collabora.com?part=2
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v4 02/10] drm/mediatek: Implement Display Stream Compression support
2026-07-13 14:41 ` sashiko-bot
@ 2026-07-14 8:09 ` AngeloGioacchino Del Regno
0 siblings, 0 replies; 20+ messages in thread
From: AngeloGioacchino Del Regno @ 2026-07-14 8:09 UTC (permalink / raw)
To: sashiko-reviews; +Cc: robh, devicetree, conor+dt, dri-devel
On 7/13/26 16:41, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
> - [High] Truncation of RC buffer thresholds due to incorrect bitwise masking instead of right-shifting.
> - [High] Hardcoded and conflicting slice count assumptions without panel validation.
> - [High] Component discovery loop skips DSI component, preventing DSC initialization.
> - [High] DSI timing calculation ignores compression and overwrites D-PHY clock lane register.
> - [Medium] State retention bug where `dsc_config_done` is never reset to false.
> --
>
> --- Patch [2]: [PATCH v4 02/10] drm/mediatek: Implement Display Stream Compression support ---
> commit 94686398e3eedc1a9e39476763b6747a4bcfb4c9
> Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> Subject: drm/mediatek: Implement Display Stream Compression support
>
> This commit introduces a new driver for the Display Stream Compression (DSC)
> Display Controller IP, implementing support for DSC v1.1 to v1.2. It adds
> DSC handling into the DSI driver and integrates the bypass setup via CPU
> writes instead of CMDQ.
>
>> diff --git a/drivers/gpu/drm/mediatek/mtk_crtc.c b/drivers/gpu/drm/mediatek/mtk_crtc.c
>> index 8e552cdc3b53b..d4be40b2574a1 100644
>> --- a/drivers/gpu/drm/mediatek/mtk_crtc.c
>> +++ b/drivers/gpu/drm/mediatek/mtk_crtc.c
> [ ... ]
>> @@ -398,6 +401,17 @@ static int mtk_crtc_ddp_hw_init(struct mtk_crtc *mtk_crtc)
>> if (!mtk_ddp_comp_add(mtk_crtc->ddp_comp[i], mtk_crtc->mutex))
>> mtk_mutex_add_comp(mtk_crtc->mutex,
>> mtk_crtc->ddp_comp[i]->id);
>> +
>> + /* For now, only single DSI is supported */
>> + if (mtk_crtc->ddp_comp[i]->id >= DDP_COMPONENT_DSI0 &&
>> + mtk_crtc->ddp_comp[i]->id <= DDP_COMPONENT_DSI3)
>> + if (!comp_dsi)
>> + comp_dsi = mtk_crtc->ddp_comp[i];
>> +
>
> [Severity: High]
> Does this loop terminate one iteration early, causing the DSI component
> to be skipped?
>
> Because the loop condition is i < mtk_crtc->ddp_comp_nr - 1, and the DSI
> interface is typically the final component in the display pipeline, it
> appears comp_dsi will remain NULL here. This would cause the subsequent
> DSC initialization check to silently fail and bypass the DSC setup entirely.
>
>> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_dsc.c b/drivers/gpu/drm/mediatek/mtk_disp_dsc.c
>> new file mode 100644
>> index 0000000000000..bed6b77bf9a9e
>> --- /dev/null
>> +++ b/drivers/gpu/drm/mediatek/mtk_disp_dsc.c
> [ ... ]
>> +static void mtk_dsc_pps_setup(struct mtk_dsc *disp_dsc, struct drm_dsc_config *dsc_cfg)
>> +{
> [ ... ]
>> + /* PPS 8..11 - Each register holds 4 RC buffer thresholds (PPS 11 has two) */
>> + for (i = 0; i < 4; i++) {
>> + u8 block_num = i * 4;
>> + data = 0;
>> +
>> + for (j = 0; j < 4; j++) {
>> + u8 buf_index = block_num + j;
>> + u8 data_shift = j * 8;
>> +
>> + /* rc_buf_thresh holds 14 elements in total */
>> + if (buf_index > 13)
>> + break;
>> +
>> + data |= (rbt[buf_index] & DSC_P8_RC_BUF_THR_X) << data_shift;
>
> [Severity: High]
> Does this code truncate the RC buffer thresholds by masking with 0xFF
> instead of shifting right?
>
This is not truncating anything, and it is the equivalent of
dsc->dsc_cfg.rc_buf_thresh[idx+3] << 24 |
dsc->dsc_cfg.rc_buf_thresh[idx+2] << 16 |
dsc->dsc_cfg.rc_buf_thresh[idx+1] << 8 |
dsc->dsc_cfg.rc_buf_thresh[idx+0]);
So this is fine.
> The VESA DSC specification requires programming the Picture Parameter Set
> registers with RC buffer thresholds divided by 64. Using a bitwise AND mask
> instead of shifting right by 6 appears to write truncated values to the PPS
> registers, which could break rate control.
>
> [ ... ]
>> +void mtk_dsc_setup(struct device *dev, struct drm_dsc_config *dsc_cfg)
>> +{
> [ ... ]
>> + pic_height_ext_num = dsc_cfg->pic_height + dsc_cfg->slice_height - 1;
>> + pic_group_width = dsc_cfg->slice_width * 4;
>> + pic_group_width /= 3;
>
> [Severity: High]
> Does this calculation hardcode the picture group width to assume exactly
> four slices per line?
>
No, that makes the hardware process 1+1/4 pic width per iteration.
> If a panel requests a different slice count, this could conflict with the
> payload size assumptions made elsewhere in the pipeline and potentially
> cause display corruption.
>
> [ ... ]
>> + if (dsc_en_bit)
>> + dsc_con |= DSC_EN;
>> +
>> + writel(dsc_con, disp_dsc->reg + DISP_REG_DSC_CON);
>> +
>> + disp_dsc->dsc_config_done = true;
>> +}
>
> [Severity: Medium]
> Can this state retention cause issues on subsequent modesets?
>
> Since the dsc_config_done flag is never reset to false on teardown, will a
> subsequent non-DSC modeset bypass the check below in mtk_dsc_start() and
> leave the DSC IP active with stale parameters?
>
The dynamic DSC -> non-DSC modesets can be supported only after the mediatek-drm
restructuring. As of now, only all-compressed or all-uncompressed modes displays
are supported.
> [ ... ]
>> +void mtk_dsc_start(struct device *dev)
>> +{
>> + struct mtk_dsc *disp_dsc = dev_get_drvdata(dev);
>> +
>> + /* If no DSC or config not done, set bypass mode */
>> + if (!disp_dsc->dsc_config_done) {
>
> [ ... ]
>> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
>> index 3f3f56eed3f92..8ab5c3431dbbc 100644
>> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
>> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> [ ... ]
>> +static void mtk_dsi_ps_control_dsc(struct mtk_dsi *dsi, bool config_vact)
>> +{
>> + const struct mtk_dsi_driver_data *data = dsi->driver_data;
>> + const u16 *reg_main = dsi->driver_data->reg_main;
>> + const short dsi_buf_bpp = 3;
>> + u32 ps_wc;
>> +
>> + /* Word count */
>> + ps_wc = FIELD_PREP(DSI_PS_WC, 2 * dsi->dsc->slice_chunk_size);
>
> [Severity: High]
> Does this hardcode the DSI payload to assume exactly two slices per line?
>
> This appears to conflict with the assumption of four slices per line inside
> mtk_dsc_setup(). Is it possible to validate the panel's requested slice
> count against these assumptions during the mode_valid check?
>
No, that's another thing entirely, it's the per-iteration DSI word count and
the dsc_setup is not hardcoding four slices per line.
>> +
>> + if (config_vact) {
>> + writel(FIELD_PREP(VACT_NL, dsi->vm.vactive),
>> + dsi->regs + reg_main[DSI_VACT_NL]);
>> + writel(ps_wc, dsi->regs + reg_main[DSI_HSTX_CKL_WC]);
>
> [Severity: High]
> Will writing the compressed payload size here overwrite the correctly
> calculated D-PHY High-Speed Clock Lane Word Count?
>
What!??! No!!!
> [ ... ]
>> -static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi)
>> +static int mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi)
>> {
>> struct videomode *vm = &dsi->vm;
>> + int ret;
>>
>> writel(vm->vsync_len, dsi->regs + DSI_VSA_NL);
>> writel(vm->vback_porch, dsi->regs + DSI_VBP_NL);
>> writel(vm->vfront_porch, dsi->regs + DSI_VFP_NL);
>> writel(vm->vactive, dsi->regs + DSI_VACT_NL);
>>
>> - if (dsi->driver_data->has_size_ctl)
>> - writel(FIELD_PREP(DSI_HEIGHT, vm->vactive) |
>> - FIELD_PREP(DSI_WIDTH, vm->hactive),
>> - dsi->regs + DSI_SIZE_CON);
>> -
>> if (dsi->driver_data->support_per_frame_lp)
>> mtk_dsi_config_vdo_timing_per_frame_lp(dsi);
>> else
>> mtk_dsi_config_vdo_timing_per_line_lp(dsi);
>>
>> - mtk_dsi_ps_control(dsi, false);
>> + if (dsi->dsc) {
>> + ret = mtk_dsi_set_dsc_params(dsi);
>> + if (ret)
>> + return ret;
>> +
>> + mtk_dsi_ps_control(dsi, true);
>
> [Severity: High]
> Do the DSI lane timings calculated earlier in
> mtk_dsi_config_vdo_timing_per_frame_lp() incorrectly use the uncompressed
> pixel count?
>
> By passing config_vact = true into mtk_dsi_ps_control() for DSC streams, it
> looks like the raw compressed payload size will subsequently overwrite the
> timing synchronized register DSI_HSTX_CKL_WC.
>
It's done on purpose.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4 03/10] dt-bindings: display: mediatek: dsi: Document MT8189 and MT8196
2026-07-13 14:27 [PATCH v4 00/10] drm/mediatek: Add DSC, WDMA, MT8189/96 DSI support AngeloGioacchino Del Regno
2026-07-13 14:27 ` [PATCH v4 01/10] dt-bindings: display: mediatek: dsc: Add MT8196 compatible AngeloGioacchino Del Regno
2026-07-13 14:27 ` [PATCH v4 02/10] drm/mediatek: Implement Display Stream Compression support AngeloGioacchino Del Regno
@ 2026-07-13 14:27 ` AngeloGioacchino Del Regno
2026-07-13 14:27 ` [PATCH v4 04/10] drm/mediatek: mtk_dsi: Enable interrupt at component bind time AngeloGioacchino Del Regno
` (6 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: AngeloGioacchino Del Regno @ 2026-07-13 14:27 UTC (permalink / raw)
To: chunkuang.hu
Cc: p.zabel, maarten.lankhorst, mripard, tzimmermann, airlied, simona,
robh, krzk+dt, conor+dt, matthias.bgg, angelogioacchino.delregno,
jitao.shi, dri-devel, linux-mediatek, devicetree, linux-kernel,
linux-arm-kernel, kernel, justin.yeh, jason-jh.lin,
Krzysztof Kozlowski
Add compatible strings for MT8189 and MT8196 SoCs.
These are compatible with the DSI IPs found in the MT8188/95 and
others, but with differences making them not fully compatible.
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
.../devicetree/bindings/display/mediatek/mediatek,dsi.yaml | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.yaml
index 27ffbccc2a08..b5cdfe0eaca4 100644
--- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.yaml
+++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.yaml
@@ -30,6 +30,8 @@ properties:
- mediatek,mt8183-dsi
- mediatek,mt8186-dsi
- mediatek,mt8188-dsi
+ - mediatek,mt8189-dsi
+ - mediatek,mt8196-dsi
- items:
- enum:
- mediatek,mt6795-dsi
--
2.54.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v4 04/10] drm/mediatek: mtk_dsi: Enable interrupt at component bind time
2026-07-13 14:27 [PATCH v4 00/10] drm/mediatek: Add DSC, WDMA, MT8189/96 DSI support AngeloGioacchino Del Regno
` (2 preceding siblings ...)
2026-07-13 14:27 ` [PATCH v4 03/10] dt-bindings: display: mediatek: dsi: Document MT8189 and MT8196 AngeloGioacchino Del Regno
@ 2026-07-13 14:27 ` AngeloGioacchino Del Regno
2026-07-13 14:43 ` sashiko-bot
2026-07-13 14:27 ` [PATCH v4 05/10] drm/mediatek: mtk_dsi: Transfer register offsets to per-SoC const AngeloGioacchino Del Regno
` (5 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: AngeloGioacchino Del Regno @ 2026-07-13 14:27 UTC (permalink / raw)
To: chunkuang.hu
Cc: p.zabel, maarten.lankhorst, mripard, tzimmermann, airlied, simona,
robh, krzk+dt, conor+dt, matthias.bgg, angelogioacchino.delregno,
jitao.shi, dri-devel, linux-mediatek, devicetree, linux-kernel,
linux-arm-kernel, kernel, justin.yeh, jason-jh.lin
Having the DSI interrupt enabled before actually binding the DSI
component to the display controller driver is both useless and
dangerous: the main purpose of this interrupt is to signal CMD
done, LP RX data ready, or VideoMode done, or to reset the HW
engine if this doesn't come.
Should this interrupt come too late (during probe), the HW will
be reset only at the next occurrence of a timeout, which slows
down boot and may render artifacts to the DSI display.
Moreover, clearing the DSI interrupt while the display controller
is not ready yet, may result in an interrupt storm.
In order to prevent this from happening, request the interrupt
with IRQF_NO_AUTOEN, and enable it only when binding DSI to its
display controller component master.
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
drivers/gpu/drm/mediatek/mtk_dsi.c | 30 ++++++++++++++++++------------
1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 8ab5c3431dbb..e24e7b91a361 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -225,6 +225,7 @@ struct mtk_dsi {
int refcount;
bool enabled;
bool lanes_ready;
+ int irq;
u32 irq_data;
wait_queue_head_t irq_wait_queue;
const struct mtk_dsi_driver_data *driver_data;
@@ -1090,7 +1091,13 @@ static int mtk_dsi_bind(struct device *dev, struct device *master, void *data)
if (ret)
return ret;
- return device_reset_optional(dev);
+ ret = device_reset_optional(dev);
+ if (ret)
+ return ret;
+
+ enable_irq(dsi->irq);
+
+ return 0;
}
static void mtk_dsi_unbind(struct device *dev, struct device *master,
@@ -1098,6 +1105,8 @@ static void mtk_dsi_unbind(struct device *dev, struct device *master,
{
struct mtk_dsi *dsi = dev_get_drvdata(dev);
+ disable_irq(dsi->irq);
+
drm_encoder_cleanup(&dsi->encoder);
}
@@ -1331,7 +1340,6 @@ static int mtk_dsi_probe(struct platform_device *pdev)
{
struct mtk_dsi *dsi;
struct device *dev = &pdev->dev;
- int irq_num;
int ret;
dsi = devm_drm_bridge_alloc(dev, struct mtk_dsi, bridge,
@@ -1364,9 +1372,9 @@ static int mtk_dsi_probe(struct platform_device *pdev)
if (IS_ERR(dsi->phy))
return dev_err_probe(dev, PTR_ERR(dsi->phy), "Failed to get MIPI-DPHY\n");
- irq_num = platform_get_irq(pdev, 0);
- if (irq_num < 0)
- return irq_num;
+ dsi->irq = platform_get_irq(pdev, 0);
+ if (dsi->irq < 0)
+ return dsi->irq;
dsi->host.ops = &mtk_dsi_ops;
dsi->host.dev = dev;
@@ -1375,17 +1383,15 @@ static int mtk_dsi_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, dsi);
+ ret = devm_request_irq(&pdev->dev, dsi->irq, mtk_dsi_irq,
+ IRQF_NO_AUTOEN, dev_name(&pdev->dev), dsi);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret, "Failed to request DSI irq\n");
+
ret = mipi_dsi_host_register(&dsi->host);
if (ret < 0)
return dev_err_probe(dev, ret, "Failed to register DSI host\n");
- ret = devm_request_irq(&pdev->dev, irq_num, mtk_dsi_irq,
- IRQF_TRIGGER_NONE, dev_name(&pdev->dev), dsi);
- if (ret) {
- mipi_dsi_host_unregister(&dsi->host);
- return dev_err_probe(&pdev->dev, ret, "Failed to request DSI irq\n");
- }
-
dsi->bridge.of_node = dev->of_node;
dsi->bridge.type = DRM_MODE_CONNECTOR_DSI;
--
2.54.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v4 04/10] drm/mediatek: mtk_dsi: Enable interrupt at component bind time
2026-07-13 14:27 ` [PATCH v4 04/10] drm/mediatek: mtk_dsi: Enable interrupt at component bind time AngeloGioacchino Del Regno
@ 2026-07-13 14:43 ` sashiko-bot
2026-07-14 8:17 ` AngeloGioacchino Del Regno
0 siblings, 1 reply; 20+ messages in thread
From: sashiko-bot @ 2026-07-13 14:43 UTC (permalink / raw)
To: AngeloGioacchino Del Regno; +Cc: robh, dri-devel, devicetree, conor+dt
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing cleanup of DRM encoder when device_reset_optional() fails in mtk_dsi_bind().
- [High] Read-modify-write data race on `dsi->irq_data` between process context and hardirq context.
--
--- Patch [4]: [PATCH v4 04/10] drm/mediatek: mtk_dsi: Enable interrupt at component bind time ---
commit 0607d3868a389c4103df3225d2bc7a47d44df3b0
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
drm/mediatek: mtk_dsi: Enable interrupt at component bind time
This patch requests the DSI interrupt with IRQF_NO_AUTOEN and enables it only
when binding the DSI to its display controller component master. This prevents
interrupt storms and delays caused by timeouts during probe.
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index 8ab5c3431dbbc..e24e7b91a3618 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
[ ... ]
> @@ -1090,7 +1091,13 @@ static int mtk_dsi_bind(struct device *dev, struct device *master, void *data)
> if (ret)
> return ret;
>
> - return device_reset_optional(dev);
> + ret = device_reset_optional(dev);
> + if (ret)
> + return ret;
[Severity: High]
This isn't a bug introduced by this patch, but does this error path leak
the DRM encoder?
If device_reset_optional(dev) fails, it returns without calling
drm_encoder_cleanup(&dsi->encoder). Because the component bind operation
fails, the component framework will not invoke mtk_dsi_unbind(), meaning
the encoder is never cleaned up.
If the DSI driver is subsequently unbound or removed, its locally managed
memory is freed while the encoder is still linked in the DRM core's list,
which could lead to a use-after-free or kernel panic when the list is
traversed.
> +
> + enable_irq(dsi->irq);
> +
> + return 0;
> }
[ ... ]
> @@ -1375,17 +1383,15 @@ static int mtk_dsi_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, dsi);
>
> + ret = devm_request_irq(&pdev->dev, dsi->irq, mtk_dsi_irq,
> + IRQF_NO_AUTOEN, dev_name(&pdev->dev), dsi);
[Severity: High]
This is a pre-existing issue, but is there a read-modify-write data race on
dsi->irq_data between process context and hardirq context?
mtk_dsi_irq_data_clear() modifies dsi->irq_data using &= in process context
(from mtk_dsi_host_transfer()) without disabling interrupts or using atomic
bitwise operations. Concurrently, the hardirq handler mtk_dsi_irq() modifies
the same variable using |=.
If a DSI interrupt fires during the process-context read-modify-write
sequence, won't the flag set by the interrupt handler be silently
overwritten and lost when the process context stores its computed value?
This could lead to hardware interrupt events being lost, causing timeouts
in mtk_dsi_wait_for_irq_done() and potentially breaking display
initialization or frame updates.
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret, "Failed to request DSI irq\n");
> +
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260713142746.40259-1-angelogioacchino.delregno@collabora.com?part=4
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v4 04/10] drm/mediatek: mtk_dsi: Enable interrupt at component bind time
2026-07-13 14:43 ` sashiko-bot
@ 2026-07-14 8:17 ` AngeloGioacchino Del Regno
0 siblings, 0 replies; 20+ messages in thread
From: AngeloGioacchino Del Regno @ 2026-07-14 8:17 UTC (permalink / raw)
To: sashiko-reviews; +Cc: robh, dri-devel, devicetree, conor+dt
On 7/13/26 16:43, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Missing cleanup of DRM encoder when device_reset_optional() fails in mtk_dsi_bind().
> - [High] Read-modify-write data race on `dsi->irq_data` between process context and hardirq context.
> --
>
> --- Patch [4]: [PATCH v4 04/10] drm/mediatek: mtk_dsi: Enable interrupt at component bind time ---
> commit 0607d3868a389c4103df3225d2bc7a47d44df3b0
> Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>
> drm/mediatek: mtk_dsi: Enable interrupt at component bind time
>
> This patch requests the DSI interrupt with IRQF_NO_AUTOEN and enables it only
> when binding the DSI to its display controller component master. This prevents
> interrupt storms and delays caused by timeouts during probe.
>
>> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
>> index 8ab5c3431dbbc..e24e7b91a3618 100644
>> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
>> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> [ ... ]
>> @@ -1090,7 +1091,13 @@ static int mtk_dsi_bind(struct device *dev, struct device *master, void *data)
>> if (ret)
>> return ret;
>>
>> - return device_reset_optional(dev);
>> + ret = device_reset_optional(dev);
>> + if (ret)
>> + return ret;
>
> [Severity: High]
> This isn't a bug introduced by this patch, but does this error path leak
> the DRM encoder?
>
That's right. I will send a patch to fix this outside of this series.
This series shall not be blocked by issues that were already present before.
> If device_reset_optional(dev) fails, it returns without calling
> drm_encoder_cleanup(&dsi->encoder). Because the component bind operation
> fails, the component framework will not invoke mtk_dsi_unbind(), meaning
> the encoder is never cleaned up.
>
> If the DSI driver is subsequently unbound or removed, its locally managed
> memory is freed while the encoder is still linked in the DRM core's list,
> which could lead to a use-after-free or kernel panic when the list is
> traversed.
>
>> +
>> + enable_irq(dsi->irq);
>> +
>> + return 0;
>> }
> [ ... ]
>> @@ -1375,17 +1383,15 @@ static int mtk_dsi_probe(struct platform_device *pdev)
>>
>> platform_set_drvdata(pdev, dsi);
>>
>> + ret = devm_request_irq(&pdev->dev, dsi->irq, mtk_dsi_irq,
>> + IRQF_NO_AUTOEN, dev_name(&pdev->dev), dsi);
>
> [Severity: High]
> This is a pre-existing issue, but is there a read-modify-write data race on
> dsi->irq_data between process context and hardirq context?
>
> mtk_dsi_irq_data_clear() modifies dsi->irq_data using &= in process context
> (from mtk_dsi_host_transfer()) without disabling interrupts or using atomic
> bitwise operations. Concurrently, the hardirq handler mtk_dsi_irq() modifies
> the same variable using |=.
>
> If a DSI interrupt fires during the process-context read-modify-write
> sequence, won't the flag set by the interrupt handler be silently
> overwritten and lost when the process context stores its computed value?
>
> This could lead to hardware interrupt events being lost, causing timeouts
> in mtk_dsi_wait_for_irq_done() and potentially breaking display
> initialization or frame updates.
>
That doesn't really happen for now, but it's a fair point. Will send a patch
to fix outside of this series.
This was not introduced by this series and shall not block it.
>> + if (ret)
>> + return dev_err_probe(&pdev->dev, ret, "Failed to request DSI irq\n");
>> +
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4 05/10] drm/mediatek: mtk_dsi: Transfer register offsets to per-SoC const
2026-07-13 14:27 [PATCH v4 00/10] drm/mediatek: Add DSC, WDMA, MT8189/96 DSI support AngeloGioacchino Del Regno
` (3 preceding siblings ...)
2026-07-13 14:27 ` [PATCH v4 04/10] drm/mediatek: mtk_dsi: Enable interrupt at component bind time AngeloGioacchino Del Regno
@ 2026-07-13 14:27 ` AngeloGioacchino Del Regno
2026-07-13 14:27 ` [PATCH v4 06/10] drm/mediatek: mtk_dsi: Add support for MT8189 AngeloGioacchino Del Regno
` (4 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: AngeloGioacchino Del Regno @ 2026-07-13 14:27 UTC (permalink / raw)
To: chunkuang.hu
Cc: p.zabel, maarten.lankhorst, mripard, tzimmermann, airlied, simona,
robh, krzk+dt, conor+dt, matthias.bgg, angelogioacchino.delregno,
jitao.shi, dri-devel, linux-mediatek, devicetree, linux-kernel,
linux-arm-kernel, kernel, justin.yeh, jason-jh.lin
As of now, the all of the supported SoCs have small differences in
the register offsets for their version of the DSI IP, at least for
the VM_CMD_CON, SHADOW_DEBUG and CMDQ offsets.
As a preparation for introducing support for newer generation DSI
IPs, having even more differences in the register offsets (but not
in the layout of their fields, nor in the actual programming), as
found on Dimensity 9400 MT6991, Kompanio Ultra MT8196 and Genio
Pro 5100 MT8894, transfer all the register offsets to two const
arrays, splitting the DSI IP version specific registers from the
SoC specific ones (as those depend on interfacing with CMDQ and
other IPs external to DSI, but internal to the SoC, and embedded
in DSI).
This change brings no functional difference, as it only changes
how the register offsets are retrieved and nothing else.
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
drivers/gpu/drm/mediatek/mtk_dsi.c | 378 +++++++++++++++++++----------
1 file changed, 253 insertions(+), 125 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index e24e7b91a361..bfb640f3f5b1 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -1,6 +1,8 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
* Copyright (c) 2015 MediaTek Inc.
+ * Copyright (c) 2026 Collabora Ltd.
+ * AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
*/
#include <linux/bitfield.h>
@@ -34,11 +36,11 @@
#include "mtk_disp_drv.h"
#include "mtk_drm_drv.h"
-#define DSI_START 0x00
+/* DSI_START */
-#define DSI_INTEN 0x08
+/* DSI_INTEN */
-#define DSI_INTSTA 0x0c
+/* DSI_INTSTA */
#define LPRX_RD_RDY_INT_FLAG BIT(0)
#define CMD_DONE_INT_FLAG BIT(1)
#define TE_RDY_INT_FLAG BIT(2)
@@ -46,12 +48,13 @@
#define EXT_TE_RDY_INT_FLAG BIT(4)
#define DSI_BUSY BIT(31)
-#define DSI_CON_CTRL 0x10
+/* DSI_CON_CTRL */
#define DSI_RESET BIT(0)
#define DSI_EN BIT(1)
#define DPHY_RESET BIT(2)
+#define CMDMODE_WAIT_DATA_EVERY_LINE_EN BIT(24)
-#define DSI_MODE_CTRL 0x14
+/* DSI_MODE_CTRL */
#define MODE (3)
#define CMD_MODE 0
#define SYNC_PULSE_MODE 1
@@ -60,7 +63,7 @@
#define FRM_MODE BIT(16)
#define MIX_MODE BIT(17)
-#define DSI_TXRX_CTRL 0x18
+/* DSI_TXRX_CTRL */
#define VC_NUM BIT(1)
#define LANE_NUM GENMASK(5, 2)
#define DIS_EOT BIT(6)
@@ -71,7 +74,7 @@
#define MAX_RTN_SIZE GENMASK(15, 12)
#define HSTX_CKLP_EN BIT(16)
-#define DSI_PSCTRL 0x1c
+/* DSI_PSCTRL */
#define DSI_PS_WC GENMASK(13, 0)
#define DSI_PS_SEL GENMASK(19, 16)
#define PACKED_PS_16BIT_RGB565 0
@@ -80,64 +83,64 @@
#define PACKED_PS_24BIT_RGB888 3
#define COMPRESSED_PS_DSC 5
-#define DSI_VSA_NL 0x20
-#define DSI_VBP_NL 0x24
-#define DSI_VFP_NL 0x28
-#define DSI_VACT_NL 0x2C
+/* DSI_VSA_NL */
+/* DSI_VBP_NL */
+/* DSI_VFP_NL */
+/* DSI_VACT_NL */
#define VACT_NL GENMASK(14, 0)
-#define DSI_SIZE_CON 0x38
-#define DSI_HEIGHT GENMASK(30, 16)
-#define DSI_WIDTH GENMASK(14, 0)
-#define DSI_HSA_WC 0x50
-#define DSI_HBP_WC 0x54
-#define DSI_HFP_WC 0x58
-#define HFP_HS_VB_PS_WC GENMASK(30, 16)
+/* DSI_SIZE_CON */
+#define DSI_HEIGHT GENMASK(30, 16)
+#define DSI_WIDTH GENMASK(14, 0)
+/* DSI_HSA_WC */
+/* DSI_HBP_WC */
+/* DSI_HFP_WC */
+#define HFP_HS_VB_PS_WC GENMASK(30, 16)
#define HFP_HS_EN BIT(31)
-#define DSI_CMDQ_SIZE 0x60
+/* DSI_CMDQ_SIZE */
#define CMDQ_SIZE 0x3f
-#define CMDQ_SIZE_SEL BIT(15)
+#define CMDQ_SIZE_SEL BIT(15)
-#define DSI_HSTX_CKL_WC 0x64
+/* DSI_HSTX_CKL_WC */
#define HSTX_CKL_WC GENMASK(15, 2)
-#define DSI_RX_DATA0 0x74
-#define DSI_RX_DATA1 0x78
-#define DSI_RX_DATA2 0x7c
-#define DSI_RX_DATA3 0x80
+/* DSI_RX_DATA0 */
+/* DSI_RX_DATA1 */
+/* DSI_RX_DATA2 */
+/* DSI_RX_DATA3 */
-#define DSI_RACK 0x84
+/* DSI_RACK */
#define RACK BIT(0)
-#define DSI_PHY_LCCON 0x104
+/* DSI_PHY_LCCON */
#define LC_HS_TX_EN BIT(0)
#define LC_ULPM_EN BIT(1)
#define LC_WAKEUP_EN BIT(2)
-#define DSI_PHY_LD0CON 0x108
+/* DSI_PHY_LD0CON */
#define LD0_HS_TX_EN BIT(0)
#define LD0_ULPM_EN BIT(1)
#define LD0_WAKEUP_EN BIT(2)
-#define DSI_PHY_TIMECON0 0x110
+/* DSI_PHY_TIMECON0 */
#define LPX GENMASK(7, 0)
#define HS_PREP GENMASK(15, 8)
#define HS_ZERO GENMASK(23, 16)
#define HS_TRAIL GENMASK(31, 24)
-#define DSI_PHY_TIMECON1 0x114
+/* DSI_PHY_TIMECON1 */
#define TA_GO GENMASK(7, 0)
#define TA_SURE GENMASK(15, 8)
#define TA_GET GENMASK(23, 16)
#define DA_HS_EXIT GENMASK(31, 24)
-#define DSI_PHY_TIMECON2 0x118
+/* DSI_PHY_TIMECON2 */
#define CONT_DET GENMASK(7, 0)
#define DA_HS_SYNC GENMASK(15, 8)
#define CLK_ZERO GENMASK(23, 16)
#define CLK_TRAIL GENMASK(31, 24)
-#define DSI_PHY_TIMECON3 0x11c
+/* DSI_PHY_TIMECON3 */
#define CLK_HS_PREP GENMASK(7, 0)
#define CLK_HS_POST GENMASK(15, 8)
#define CLK_HS_EXIT GENMASK(23, 16)
@@ -168,6 +171,45 @@
(type == MIPI_DSI_GENERIC_READ_REQUEST_2_PARAM) || \
(type == MIPI_DSI_DCS_READ))
+enum mtk_dsi_main_regidx {
+ DSI_START,
+ DSI_INTEN,
+ DSI_INTSTA,
+ DSI_CON_CTRL,
+ DSI_MODE_CTRL,
+ DSI_TXRX_CTRL,
+ DSI_PSCTRL,
+ DSI_VSA_NL,
+ DSI_VBP_NL,
+ DSI_VFP_NL,
+ DSI_VACT_NL,
+ DSI_SIZE_CON,
+ DSI_HSA_WC,
+ DSI_HBP_WC,
+ DSI_HFP_WC,
+ DSI_CMDQ_SIZE,
+ DSI_HSTX_CKL_WC,
+ DSI_RX_DATA0,
+ DSI_RX_DATA1,
+ DSI_RX_DATA2,
+ DSI_RX_DATA3,
+ DSI_RACK,
+ DSI_PHY_LCCON,
+ DSI_PHY_LD0CON,
+ DSI_PHY_TIMECON0,
+ DSI_PHY_TIMECON1,
+ DSI_PHY_TIMECON2,
+ DSI_PHY_TIMECON3,
+ DSI_MAIN_REG_MAX
+};
+
+enum mtk_dsi_adv_regidx {
+ DSI_VM_CMD_CON,
+ DSI_SHADOW_DEBUG,
+ DSI_CMDQ,
+ DSI_ADV_REG_MAX
+};
+
struct mtk_phy_timing {
u32 lpx;
u32 da_hs_prepare;
@@ -190,10 +232,9 @@ struct mtk_phy_timing {
struct phy;
struct mtk_dsi_driver_data {
- const u32 reg_cmdq_off;
- const u32 reg_vm_cmd_off;
- const u32 reg_shadow_dbg_off;
- bool has_shadow_ctl;
+ const u16 *reg_main;
+ const u16 *reg_adv;
+
bool has_size_ctl;
bool cmdq_long_packet_ctl;
bool support_per_frame_lp;
@@ -231,6 +272,61 @@ struct mtk_dsi {
const struct mtk_dsi_driver_data *driver_data;
};
+static const u16 mtk_dsi_regs_main_v1[DSI_MAIN_REG_MAX] = {
+ [DSI_START] = 0x00,
+ [DSI_INTEN] = 0x08,
+ [DSI_INTSTA] = 0x0c,
+ [DSI_CON_CTRL] = 0x10,
+ [DSI_MODE_CTRL] = 0x14,
+ [DSI_TXRX_CTRL] = 0x18,
+ [DSI_PSCTRL] = 0x1c,
+ [DSI_VSA_NL] = 0x20,
+ [DSI_VBP_NL] = 0x24,
+ [DSI_VFP_NL] = 0x28,
+ [DSI_VACT_NL] = 0x2c,
+ [DSI_SIZE_CON] = 0x38,
+ [DSI_HSA_WC] = 0x50,
+ [DSI_HBP_WC] = 0x54,
+ [DSI_HFP_WC] = 0x58,
+ [DSI_CMDQ_SIZE] = 0x60,
+ [DSI_HSTX_CKL_WC] = 0x64,
+ [DSI_RX_DATA0] = 0x74,
+ [DSI_RX_DATA1] = 0x78,
+ [DSI_RX_DATA2] = 0x7c,
+ [DSI_RX_DATA3] = 0x80,
+ [DSI_RACK] = 0x84,
+ [DSI_PHY_LCCON] = 0x104,
+ [DSI_PHY_LD0CON] = 0x108,
+ [DSI_PHY_TIMECON0] = 0x110,
+ [DSI_PHY_TIMECON1] = 0x114,
+ [DSI_PHY_TIMECON2] = 0x118,
+ [DSI_PHY_TIMECON3] = 0x11c,
+};
+
+static const u16 mtk_dsi_regs_mt2701[DSI_ADV_REG_MAX] = {
+ [DSI_VM_CMD_CON] = 0x130,
+ [DSI_SHADOW_DEBUG] = 0,
+ [DSI_CMDQ] = 0x180,
+};
+
+static const u16 mtk_dsi_regs_mt8173[DSI_ADV_REG_MAX] = {
+ [DSI_VM_CMD_CON] = 0x130,
+ [DSI_SHADOW_DEBUG] = 0,
+ [DSI_CMDQ] = 0x200,
+};
+
+static const u16 mtk_dsi_regs_mt8183[DSI_ADV_REG_MAX] = {
+ [DSI_VM_CMD_CON] = 0x130,
+ [DSI_SHADOW_DEBUG] = 0x190,
+ [DSI_CMDQ] = 0x200,
+};
+
+static const u16 mtk_dsi_regs_mt8186[DSI_ADV_REG_MAX] = {
+ [DSI_VM_CMD_CON] = 0x200,
+ [DSI_SHADOW_DEBUG] = 0xc00,
+ [DSI_CMDQ] = 0xd00,
+};
+
static inline struct mtk_dsi *bridge_to_dsi(struct drm_bridge *b)
{
return container_of(b, struct mtk_dsi, bridge);
@@ -253,6 +349,7 @@ static void mtk_dsi_phy_timconfig(struct mtk_dsi *dsi)
u32 timcon0, timcon1, timcon2, timcon3;
u32 data_rate_mhz = DIV_ROUND_UP(dsi->data_rate, HZ_PER_MHZ);
struct mtk_phy_timing *timing = &dsi->phy_timing;
+ const u16 *reg_main = dsi->driver_data->reg_main;
timing->lpx = (60 * data_rate_mhz / (8 * 1000)) + 1;
timing->da_hs_prepare = (80 * data_rate_mhz + 4 * 1000) / 8000;
@@ -289,76 +386,97 @@ static void mtk_dsi_phy_timconfig(struct mtk_dsi *dsi)
FIELD_PREP(CLK_HS_POST, timing->clk_hs_post) |
FIELD_PREP(CLK_HS_EXIT, timing->clk_hs_exit);
- writel(timcon0, dsi->regs + DSI_PHY_TIMECON0);
- writel(timcon1, dsi->regs + DSI_PHY_TIMECON1);
- writel(timcon2, dsi->regs + DSI_PHY_TIMECON2);
- writel(timcon3, dsi->regs + DSI_PHY_TIMECON3);
+ writel(timcon0, dsi->regs + reg_main[DSI_PHY_TIMECON0]);
+ writel(timcon1, dsi->regs + reg_main[DSI_PHY_TIMECON1]);
+ writel(timcon2, dsi->regs + reg_main[DSI_PHY_TIMECON2]);
+ writel(timcon3, dsi->regs + reg_main[DSI_PHY_TIMECON3]);
}
static void mtk_dsi_enable(struct mtk_dsi *dsi)
{
- mtk_dsi_mask(dsi, DSI_CON_CTRL, DSI_EN, DSI_EN);
+ const u16 *reg_main = dsi->driver_data->reg_main;
+
+ mtk_dsi_mask(dsi, reg_main[DSI_CON_CTRL], DSI_EN, DSI_EN);
}
static void mtk_dsi_disable(struct mtk_dsi *dsi)
{
- mtk_dsi_mask(dsi, DSI_CON_CTRL, DSI_EN, 0);
+ const u16 *reg_main = dsi->driver_data->reg_main;
+
+ mtk_dsi_mask(dsi, reg_main[DSI_CON_CTRL], DSI_EN, 0);
}
static void mtk_dsi_reset_engine(struct mtk_dsi *dsi)
{
- mtk_dsi_mask(dsi, DSI_CON_CTRL, DSI_RESET, DSI_RESET);
- mtk_dsi_mask(dsi, DSI_CON_CTRL, DSI_RESET, 0);
+ const u16 *reg_main = dsi->driver_data->reg_main;
+
+ mtk_dsi_mask(dsi, reg_main[DSI_CON_CTRL], DSI_RESET, DSI_RESET);
+ mtk_dsi_mask(dsi, reg_main[DSI_CON_CTRL], DSI_RESET, 0);
}
static void mtk_dsi_reset_dphy(struct mtk_dsi *dsi)
{
- mtk_dsi_mask(dsi, DSI_CON_CTRL, DPHY_RESET, DPHY_RESET);
- mtk_dsi_mask(dsi, DSI_CON_CTRL, DPHY_RESET, 0);
+ const u16 *reg_main = dsi->driver_data->reg_main;
+
+ mtk_dsi_mask(dsi, reg_main[DSI_CON_CTRL], DPHY_RESET, DPHY_RESET);
+ mtk_dsi_mask(dsi, reg_main[DSI_CON_CTRL], DPHY_RESET, 0);
}
static void mtk_dsi_clk_ulp_mode_enter(struct mtk_dsi *dsi)
{
- mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_HS_TX_EN, 0);
- mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_ULPM_EN, 0);
+ const u16 *reg_main = dsi->driver_data->reg_main;
+
+ mtk_dsi_mask(dsi, reg_main[DSI_PHY_LCCON], LC_HS_TX_EN, 0);
+ mtk_dsi_mask(dsi, reg_main[DSI_PHY_LCCON], LC_ULPM_EN, 0);
}
static void mtk_dsi_clk_ulp_mode_leave(struct mtk_dsi *dsi)
{
- mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_ULPM_EN, 0);
- mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_WAKEUP_EN, LC_WAKEUP_EN);
- mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_WAKEUP_EN, 0);
+ const u16 *reg_main = dsi->driver_data->reg_main;
+
+ mtk_dsi_mask(dsi, reg_main[DSI_PHY_LCCON], LC_ULPM_EN, 0);
+ mtk_dsi_mask(dsi, reg_main[DSI_PHY_LCCON], LC_WAKEUP_EN, LC_WAKEUP_EN);
+ mtk_dsi_mask(dsi, reg_main[DSI_PHY_LCCON], LC_WAKEUP_EN, 0);
}
static void mtk_dsi_lane0_ulp_mode_enter(struct mtk_dsi *dsi)
{
- mtk_dsi_mask(dsi, DSI_PHY_LD0CON, LD0_HS_TX_EN, 0);
- mtk_dsi_mask(dsi, DSI_PHY_LD0CON, LD0_ULPM_EN, 0);
+ const u16 *reg_main = dsi->driver_data->reg_main;
+
+ mtk_dsi_mask(dsi, reg_main[DSI_PHY_LD0CON], LD0_HS_TX_EN, 0);
+ mtk_dsi_mask(dsi, reg_main[DSI_PHY_LD0CON], LD0_ULPM_EN, 0);
}
static void mtk_dsi_lane0_ulp_mode_leave(struct mtk_dsi *dsi)
{
- mtk_dsi_mask(dsi, DSI_PHY_LD0CON, LD0_ULPM_EN, 0);
- mtk_dsi_mask(dsi, DSI_PHY_LD0CON, LD0_WAKEUP_EN, LD0_WAKEUP_EN);
- mtk_dsi_mask(dsi, DSI_PHY_LD0CON, LD0_WAKEUP_EN, 0);
+ const u16 *reg_main = dsi->driver_data->reg_main;
+
+ mtk_dsi_mask(dsi, reg_main[DSI_PHY_LD0CON], LD0_ULPM_EN, 0);
+ mtk_dsi_mask(dsi, reg_main[DSI_PHY_LD0CON], LD0_WAKEUP_EN, LD0_WAKEUP_EN);
+ mtk_dsi_mask(dsi, reg_main[DSI_PHY_LD0CON], LD0_WAKEUP_EN, 0);
}
static bool mtk_dsi_clk_hs_state(struct mtk_dsi *dsi)
{
- return readl(dsi->regs + DSI_PHY_LCCON) & LC_HS_TX_EN;
+ const u16 *regoff = dsi->driver_data->reg_main;
+
+ return readl(dsi->regs + regoff[DSI_PHY_LCCON]) & LC_HS_TX_EN;
}
static void mtk_dsi_clk_hs_mode(struct mtk_dsi *dsi, bool enter)
{
+ const u16 *reg_main = dsi->driver_data->reg_main;
+
if (enter && !mtk_dsi_clk_hs_state(dsi))
- mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_HS_TX_EN, LC_HS_TX_EN);
+ mtk_dsi_mask(dsi, reg_main[DSI_PHY_LCCON], LC_HS_TX_EN, LC_HS_TX_EN);
else if (!enter && mtk_dsi_clk_hs_state(dsi))
- mtk_dsi_mask(dsi, DSI_PHY_LCCON, LC_HS_TX_EN, 0);
+ mtk_dsi_mask(dsi, reg_main[DSI_PHY_LCCON], LC_HS_TX_EN, 0);
}
static void mtk_dsi_set_mode(struct mtk_dsi *dsi)
{
- u32 vid_mode = CMD_MODE;
+ const u16 *reg_main = dsi->driver_data->reg_main;
+ u32 vid_mode;
if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO) {
if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST)
@@ -367,19 +485,24 @@ static void mtk_dsi_set_mode(struct mtk_dsi *dsi)
vid_mode = SYNC_PULSE_MODE;
else
vid_mode = SYNC_EVENT_MODE;
+ } else {
+ vid_mode = CMD_MODE;
}
- writel(vid_mode, dsi->regs + DSI_MODE_CTRL);
+ writel(vid_mode, dsi->regs + reg_main[DSI_MODE_CTRL]);
}
static void mtk_dsi_set_vm_cmd(struct mtk_dsi *dsi)
{
- mtk_dsi_mask(dsi, dsi->driver_data->reg_vm_cmd_off, VM_CMD_EN, VM_CMD_EN);
- mtk_dsi_mask(dsi, dsi->driver_data->reg_vm_cmd_off, TS_VFP_EN, TS_VFP_EN);
+ const u16 *reg_adv = dsi->driver_data->reg_adv;
+
+ mtk_dsi_mask(dsi, reg_adv[DSI_VM_CMD_CON], VM_CMD_EN, VM_CMD_EN);
+ mtk_dsi_mask(dsi, reg_adv[DSI_VM_CMD_CON], TS_VFP_EN, TS_VFP_EN);
}
static void mtk_dsi_rxtx_control(struct mtk_dsi *dsi)
{
+ const u16 *reg_main = dsi->driver_data->reg_main;
u32 regval, tmp_reg = 0;
u8 i;
@@ -395,7 +518,7 @@ static void mtk_dsi_rxtx_control(struct mtk_dsi *dsi)
if (dsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET)
regval |= DIS_EOT;
- writel(regval, dsi->regs + DSI_TXRX_CTRL);
+ writel(regval, dsi->regs + reg_main[DSI_TXRX_CTRL]);
}
static void mtk_dsi_ps_control_dsc(struct mtk_dsi *dsi, bool config_vact)
@@ -426,6 +549,8 @@ static void mtk_dsi_ps_control_dsc(struct mtk_dsi *dsi, bool config_vact)
static void mtk_dsi_ps_control_uncompressed(struct mtk_dsi *dsi, bool config_vact)
{
+ const struct mtk_dsi_driver_data *data = dsi->driver_data;
+ const u16 *reg_main = dsi->driver_data->reg_main;
u32 dsi_buf_bpp, ps_val, ps_wc, size_val, vact_nl;
if (dsi->format == MIPI_DSI_FMT_RGB565)
@@ -457,16 +582,16 @@ static void mtk_dsi_ps_control_uncompressed(struct mtk_dsi *dsi, bool config_vac
if (config_vact) {
vact_nl = FIELD_PREP(VACT_NL, dsi->vm.vactive);
- writel(vact_nl, dsi->regs + DSI_VACT_NL);
- writel(ps_wc, dsi->regs + DSI_HSTX_CKL_WC);
+ writel(vact_nl, dsi->regs + reg_main[DSI_VACT_NL]);
+ writel(ps_wc, dsi->regs + reg_main[DSI_HSTX_CKL_WC]);
}
- writel(ps_val, dsi->regs + DSI_PSCTRL);
+ writel(ps_val, dsi->regs + reg_main[DSI_PSCTRL]);
- if (dsi->driver_data->has_size_ctl) {
+ if (data->has_size_ctl) {
size_val = FIELD_PREP(DSI_HEIGHT, dsi->vm.vactive);
size_val |= FIELD_PREP(DSI_WIDTH, dsi->vm.hactive);
- writel(size_val, dsi->regs + DSI_SIZE_CON);
+ writel(size_val, dsi->regs + reg_main[DSI_SIZE_CON]);
}
}
@@ -480,6 +605,7 @@ static void mtk_dsi_ps_control(struct mtk_dsi *dsi, bool config_vact)
static void mtk_dsi_config_vdo_timing_per_frame_lp(struct mtk_dsi *dsi)
{
+ const u16 *reg_main = dsi->driver_data->reg_main;
u32 horizontal_sync_active_byte;
u32 horizontal_backporch_byte;
u32 horizontal_frontporch_byte;
@@ -534,20 +660,21 @@ static void mtk_dsi_config_vdo_timing_per_frame_lp(struct mtk_dsi *dsi)
ps_wc), dsi->lanes) + da_hs_trail + 1) * dsi->lanes / 6 - 1;
hstx_cklp_wc = FIELD_PREP(HSTX_CKL_WC, (hstx_cklp_wc_min + hstx_cklp_wc_max) / 2);
- writel(hstx_cklp_wc, dsi->regs + DSI_HSTX_CKL_WC);
+ writel(hstx_cklp_wc, dsi->regs + reg_main[DSI_HSTX_CKL_WC]);
hs_vb_ps_wc = ps_wc - (dsi->phy_timing.lpx + dsi->phy_timing.da_hs_exit +
dsi->phy_timing.da_hs_prepare + dsi->phy_timing.da_hs_zero + 2) * dsi->lanes;
horizontal_frontporch_byte |= FIELD_PREP(HFP_HS_EN, 1) |
FIELD_PREP(HFP_HS_VB_PS_WC, hs_vb_ps_wc);
- writel(horizontal_sync_active_byte, dsi->regs + DSI_HSA_WC);
- writel(horizontal_backporch_byte, dsi->regs + DSI_HBP_WC);
- writel(horizontal_frontporch_byte, dsi->regs + DSI_HFP_WC);
+ writel(horizontal_sync_active_byte, dsi->regs + reg_main[DSI_HSA_WC]);
+ writel(horizontal_backporch_byte, dsi->regs + reg_main[DSI_HBP_WC]);
+ writel(horizontal_frontporch_byte, dsi->regs + reg_main[DSI_HFP_WC]);
}
static void mtk_dsi_config_vdo_timing_per_line_lp(struct mtk_dsi *dsi)
{
+ const u16 *reg_main = dsi->driver_data->reg_main;
u32 horizontal_sync_active_byte;
u32 horizontal_backporch_byte;
u32 horizontal_frontporch_byte;
@@ -606,9 +733,9 @@ static void mtk_dsi_config_vdo_timing_per_line_lp(struct mtk_dsi *dsi)
(vm->hactive * dsi_tmp_buf_bpp + 2) % dsi->lanes;
}
- writel(horizontal_sync_active_byte, dsi->regs + DSI_HSA_WC);
- writel(horizontal_backporch_byte, dsi->regs + DSI_HBP_WC);
- writel(horizontal_frontporch_byte, dsi->regs + DSI_HFP_WC);
+ writel(horizontal_sync_active_byte, dsi->regs + reg_main[DSI_HSA_WC]);
+ writel(horizontal_backporch_byte, dsi->regs + reg_main[DSI_HBP_WC]);
+ writel(horizontal_frontporch_byte, dsi->regs + reg_main[DSI_HFP_WC]);
}
static int mtk_dsi_set_dsc_params(struct mtk_dsi *dsi)
@@ -649,15 +776,17 @@ static int mtk_dsi_set_dsc_params(struct mtk_dsi *dsi)
static int mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi)
{
+ const struct mtk_dsi_driver_data *data = dsi->driver_data;
+ const u16 *reg_main = data->reg_main;
struct videomode *vm = &dsi->vm;
int ret;
- writel(vm->vsync_len, dsi->regs + DSI_VSA_NL);
- writel(vm->vback_porch, dsi->regs + DSI_VBP_NL);
- writel(vm->vfront_porch, dsi->regs + DSI_VFP_NL);
- writel(vm->vactive, dsi->regs + DSI_VACT_NL);
+ writel(vm->vsync_len, dsi->regs + reg_main[DSI_VSA_NL]);
+ writel(vm->vback_porch, dsi->regs + reg_main[DSI_VBP_NL]);
+ writel(vm->vfront_porch, dsi->regs + reg_main[DSI_VFP_NL]);
+ writel(vm->vactive, dsi->regs + reg_main[DSI_VACT_NL]);
- if (dsi->driver_data->support_per_frame_lp)
+ if (data->support_per_frame_lp)
mtk_dsi_config_vdo_timing_per_frame_lp(dsi);
else
mtk_dsi_config_vdo_timing_per_line_lp(dsi);
@@ -677,25 +806,25 @@ static int mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi)
static void mtk_dsi_start(struct mtk_dsi *dsi)
{
- writel(0, dsi->regs + DSI_START);
- writel(1, dsi->regs + DSI_START);
+ writel(0, dsi->regs + dsi->driver_data->reg_main[DSI_START]);
+ writel(1, dsi->regs + dsi->driver_data->reg_main[DSI_START]);
}
static void mtk_dsi_stop(struct mtk_dsi *dsi)
{
- writel(0, dsi->regs + DSI_START);
+ writel(0, dsi->regs + dsi->driver_data->reg_main[DSI_START]);
}
static void mtk_dsi_set_cmd_mode(struct mtk_dsi *dsi)
{
- writel(CMD_MODE, dsi->regs + DSI_MODE_CTRL);
+ writel(CMD_MODE, dsi->regs + dsi->driver_data->reg_main[DSI_MODE_CTRL]);
}
static void mtk_dsi_set_interrupt_enable(struct mtk_dsi *dsi)
{
u32 inten = LPRX_RD_RDY_INT_FLAG | CMD_DONE_INT_FLAG | VM_DONE_INT_FLAG;
- writel(inten, dsi->regs + DSI_INTEN);
+ writel(inten, dsi->regs + dsi->driver_data->reg_main[DSI_INTEN]);
}
static void mtk_dsi_irq_data_set(struct mtk_dsi *dsi, u32 irq_bit)
@@ -730,19 +859,20 @@ static s32 mtk_dsi_wait_for_irq_done(struct mtk_dsi *dsi, u32 irq_flag,
static irqreturn_t mtk_dsi_irq(int irq, void *dev_id)
{
- struct mtk_dsi *dsi = dev_id;
u32 status, tmp;
- u32 flag = LPRX_RD_RDY_INT_FLAG | CMD_DONE_INT_FLAG | VM_DONE_INT_FLAG;
+ struct mtk_dsi *dsi = dev_id;
+ const u16 *reg_main = dsi->driver_data->reg_main;
+ const u32 flag = LPRX_RD_RDY_INT_FLAG | CMD_DONE_INT_FLAG | VM_DONE_INT_FLAG;
- status = readl(dsi->regs + DSI_INTSTA) & flag;
+ status = readl(dsi->regs + dsi->driver_data->reg_main[DSI_INTSTA]) & flag;
if (status) {
do {
- mtk_dsi_mask(dsi, DSI_RACK, RACK, RACK);
- tmp = readl(dsi->regs + DSI_INTSTA);
+ mtk_dsi_mask(dsi, reg_main[DSI_RACK], RACK, RACK);
+ tmp = readl(dsi->regs + reg_main[DSI_INTSTA]);
} while (tmp & DSI_BUSY);
- mtk_dsi_mask(dsi, DSI_INTSTA, status, 0);
+ mtk_dsi_mask(dsi, reg_main[DSI_INTSTA], status, 0);
mtk_dsi_irq_data_set(dsi, status);
wake_up_interruptible(&dsi->irq_wait_queue);
}
@@ -781,9 +911,10 @@ static void mtk_dsi_lane_ready(struct mtk_dsi *dsi)
static int mtk_dsi_poweron(struct mtk_dsi *dsi)
{
+ const struct mtk_dsi_driver_data *data = dsi->driver_data;
struct device *dev = dsi->host.dev;
- int ret;
u32 bit_per_pixel;
+ int ret;
if (++dsi->refcount != 1)
return 0;
@@ -820,9 +951,10 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
mtk_dsi_enable(dsi);
- if (dsi->driver_data->has_shadow_ctl)
+ /* Bypass shadow and force commit only if the register is present */
+ if (data->reg_adv[DSI_SHADOW_DEBUG])
writel(FORCE_COMMIT | BYPASS_SHADOW,
- dsi->regs + dsi->driver_data->reg_shadow_dbg_off);
+ dsi->regs + data->reg_adv[DSI_SHADOW_DEBUG]);
mtk_dsi_reset_engine(dsi);
mtk_dsi_phy_timconfig(dsi);
@@ -873,7 +1005,7 @@ static void mtk_dsi_poweroff(struct mtk_dsi *dsi)
mtk_dsi_lane0_ulp_mode_enter(dsi);
mtk_dsi_clk_ulp_mode_enter(dsi);
/* set the lane number as 0 to pull down mipi */
- writel(0, dsi->regs + DSI_TXRX_CTRL);
+ writel(0, dsi->regs + dsi->driver_data->reg_main[DSI_TXRX_CTRL]);
mtk_dsi_disable(dsi);
@@ -1171,9 +1303,10 @@ static void mtk_dsi_wait_for_idle(struct mtk_dsi *dsi)
int ret;
u32 val;
struct drm_device *drm = dsi->bridge.dev;
+ const struct mtk_dsi_driver_data *data = dsi->driver_data;
- ret = readl_poll_timeout(dsi->regs + DSI_INTSTA, val, !(val & DSI_BUSY),
- 4, 2000000);
+ ret = readl_poll_timeout(dsi->regs + data->reg_main[DSI_INTSTA],
+ val, !(val & DSI_BUSY), 4, 2000000);
if (ret) {
drm_warn(drm, "polling dsi wait not busy timeout!\n");
@@ -1207,10 +1340,12 @@ static u32 mtk_dsi_recv_cnt(u8 type, u8 *read_data)
static void mtk_dsi_cmdq(struct mtk_dsi *dsi, const struct mipi_dsi_msg *msg)
{
+ const struct mtk_dsi_driver_data *data = dsi->driver_data;
const char *tx_buf = msg->tx_buf;
- u8 config, cmdq_size, cmdq_off, type = msg->type;
+ const u8 type = msg->type;
u32 reg_val, cmdq_mask, i;
- u32 reg_cmdq_off = dsi->driver_data->reg_cmdq_off;
+ u8 cmdq_size, cmdq_off;
+ u8 config;
if (MTK_DSI_HOST_IS_READ(type))
config = BTA;
@@ -1233,15 +1368,15 @@ static void mtk_dsi_cmdq(struct mtk_dsi *dsi, const struct mipi_dsi_msg *msg)
}
for (i = 0; i < msg->tx_len; i++)
- mtk_dsi_mask(dsi, (reg_cmdq_off + cmdq_off + i) & (~0x3U),
+ mtk_dsi_mask(dsi, (data->reg_adv[DSI_CMDQ] + cmdq_off + i) & (~0x3U),
(0xffUL << (((i + cmdq_off) & 3U) * 8U)),
tx_buf[i] << (((i + cmdq_off) & 3U) * 8U));
- mtk_dsi_mask(dsi, reg_cmdq_off, cmdq_mask, reg_val);
- mtk_dsi_mask(dsi, DSI_CMDQ_SIZE, CMDQ_SIZE, cmdq_size);
- if (dsi->driver_data->cmdq_long_packet_ctl) {
+ mtk_dsi_mask(dsi, data->reg_adv[DSI_CMDQ], cmdq_mask, reg_val);
+ mtk_dsi_mask(dsi, data->reg_main[DSI_CMDQ_SIZE], CMDQ_SIZE, cmdq_size);
+ if (data->cmdq_long_packet_ctl) {
/* Disable setting cmdq_size automatically for long packets */
- mtk_dsi_mask(dsi, DSI_CMDQ_SIZE, CMDQ_SIZE_SEL, CMDQ_SIZE_SEL);
+ mtk_dsi_mask(dsi, data->reg_main[DSI_CMDQ_SIZE], CMDQ_SIZE_SEL, CMDQ_SIZE_SEL);
}
}
@@ -1271,7 +1406,7 @@ static ssize_t mtk_dsi_host_transfer(struct mipi_dsi_host *host,
u32 dsi_mode;
int ret, i;
- dsi_mode = readl(dsi->regs + DSI_MODE_CTRL);
+ dsi_mode = readl(dsi->regs + dsi->driver_data->reg_main[DSI_MODE_CTRL]);
if (dsi_mode & MODE) {
mtk_dsi_stop(dsi);
ret = mtk_dsi_switch_to_cmd_mode(dsi, VM_DONE_INT_FLAG, 500);
@@ -1300,7 +1435,8 @@ static ssize_t mtk_dsi_host_transfer(struct mipi_dsi_host *host,
}
for (i = 0; i < 16; i++)
- *(read_data + i) = readb(dsi->regs + DSI_RX_DATA0 + i);
+ *(read_data + i) = readb(dsi->regs +
+ dsi->driver_data->reg_main[DSI_RX_DATA0] + i);
recv_cnt = mtk_dsi_recv_cnt(read_data[0], read_data);
@@ -1407,38 +1543,30 @@ static void mtk_dsi_remove(struct platform_device *pdev)
}
static const struct mtk_dsi_driver_data mt8173_dsi_driver_data = {
- .reg_cmdq_off = 0x200,
- .reg_vm_cmd_off = 0x130,
- .reg_shadow_dbg_off = 0x190
+ .reg_main = mtk_dsi_regs_main_v1,
+ .reg_adv = mtk_dsi_regs_mt8173,
};
static const struct mtk_dsi_driver_data mt2701_dsi_driver_data = {
- .reg_cmdq_off = 0x180,
- .reg_vm_cmd_off = 0x130,
- .reg_shadow_dbg_off = 0x190
+ .reg_main = mtk_dsi_regs_main_v1,
+ .reg_adv = mtk_dsi_regs_mt2701,
};
static const struct mtk_dsi_driver_data mt8183_dsi_driver_data = {
- .reg_cmdq_off = 0x200,
- .reg_vm_cmd_off = 0x130,
- .reg_shadow_dbg_off = 0x190,
- .has_shadow_ctl = true,
+ .reg_main = mtk_dsi_regs_main_v1,
+ .reg_adv = mtk_dsi_regs_mt8183,
.has_size_ctl = true,
};
static const struct mtk_dsi_driver_data mt8186_dsi_driver_data = {
- .reg_cmdq_off = 0xd00,
- .reg_vm_cmd_off = 0x200,
- .reg_shadow_dbg_off = 0xc00,
- .has_shadow_ctl = true,
+ .reg_main = mtk_dsi_regs_main_v1,
+ .reg_adv = mtk_dsi_regs_mt8186,
.has_size_ctl = true,
};
static const struct mtk_dsi_driver_data mt8188_dsi_driver_data = {
- .reg_cmdq_off = 0xd00,
- .reg_vm_cmd_off = 0x200,
- .reg_shadow_dbg_off = 0xc00,
- .has_shadow_ctl = true,
+ .reg_main = mtk_dsi_regs_main_v1,
+ .reg_adv = mtk_dsi_regs_mt8186,
.has_size_ctl = true,
.cmdq_long_packet_ctl = true,
.support_per_frame_lp = true,
--
2.54.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v4 06/10] drm/mediatek: mtk_dsi: Add support for MT8189
2026-07-13 14:27 [PATCH v4 00/10] drm/mediatek: Add DSC, WDMA, MT8189/96 DSI support AngeloGioacchino Del Regno
` (4 preceding siblings ...)
2026-07-13 14:27 ` [PATCH v4 05/10] drm/mediatek: mtk_dsi: Transfer register offsets to per-SoC const AngeloGioacchino Del Regno
@ 2026-07-13 14:27 ` AngeloGioacchino Del Regno
2026-07-13 14:27 ` [PATCH v4 07/10] drm/mediatek: mtk_dsi: Add support for MT8196 AngeloGioacchino Del Regno
` (3 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: AngeloGioacchino Del Regno @ 2026-07-13 14:27 UTC (permalink / raw)
To: chunkuang.hu
Cc: p.zabel, maarten.lankhorst, mripard, tzimmermann, airlied, simona,
robh, krzk+dt, conor+dt, matthias.bgg, angelogioacchino.delregno,
jitao.shi, dri-devel, linux-mediatek, devicetree, linux-kernel,
linux-arm-kernel, kernel, justin.yeh, jason-jh.lin
Add support for the DSI IP found in the MT8189 SoC: this one is
similar to the IPs found in the most recent already supported SoCs
with the only difference being that it supports a higher link rate
of 2.5Gbps instead of 1.5Gbps.
To support the higher rate, add a new "max_link_rate_mbps" member
to the mtk_dsi_driver_data, assign the correct one to all of the
supported SoCs, other than to the newly introduced MT8189, and
use it in the .mode_valid() callback in place of the hardcoded
1.5Gbps value.
Since .mode_valid() is supposed to run just for resolution changes
the link rate was expressed in Mbps and gets multiplied on the fly
to save some bits.
As a note, starting with MT8189, higher maximum pixel clock rates
are achievable: while at it, also make sure that calculations are
not overflowing by casting to u64 where needed.
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
drivers/gpu/drm/mediatek/mtk_dsi.c | 29 +++++++++++++++++++++++++++--
1 file changed, 27 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index bfb640f3f5b1..5f0674934928 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -235,6 +235,8 @@ struct mtk_dsi_driver_data {
const u16 *reg_main;
const u16 *reg_adv;
+ const u16 max_link_rate_mbps;
+
bool has_size_ctl;
bool cmdq_long_packet_ctl;
bool support_per_frame_lp;
@@ -926,7 +928,7 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
}
bit_per_pixel = ret;
- dsi->data_rate = DIV_ROUND_UP_ULL(dsi->vm.pixelclock * bit_per_pixel,
+ dsi->data_rate = DIV_ROUND_UP_ULL((u64)dsi->vm.pixelclock * bit_per_pixel,
dsi->lanes);
ret = clk_set_rate(dsi->hs_clk, dsi->data_rate);
@@ -1101,13 +1103,21 @@ mtk_dsi_bridge_mode_valid(struct drm_bridge *bridge,
const struct drm_display_mode *mode)
{
struct mtk_dsi *dsi = bridge_to_dsi(bridge);
+ const struct mtk_dsi_driver_data *data = dsi->driver_data;
+ u64 wanted_link_rate, max_link_rate;
int bpp;
bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
if (bpp < 0)
return MODE_ERROR;
- if (mode->clock * bpp / dsi->lanes > 1500000)
+ wanted_link_rate = mode->clock;
+ wanted_link_rate *= bpp;
+ max_link_rate = data->max_link_rate_mbps;
+ max_link_rate *= dsi->lanes;
+ max_link_rate *= KILO;
+
+ if (wanted_link_rate > max_link_rate)
return MODE_CLOCK_HIGH;
if (dsi->dsc) {
@@ -1545,28 +1555,42 @@ static void mtk_dsi_remove(struct platform_device *pdev)
static const struct mtk_dsi_driver_data mt8173_dsi_driver_data = {
.reg_main = mtk_dsi_regs_main_v1,
.reg_adv = mtk_dsi_regs_mt8173,
+ .max_link_rate_mbps = 1500,
};
static const struct mtk_dsi_driver_data mt2701_dsi_driver_data = {
.reg_main = mtk_dsi_regs_main_v1,
.reg_adv = mtk_dsi_regs_mt2701,
+ .max_link_rate_mbps = 1500,
};
static const struct mtk_dsi_driver_data mt8183_dsi_driver_data = {
.reg_main = mtk_dsi_regs_main_v1,
.reg_adv = mtk_dsi_regs_mt8183,
+ .max_link_rate_mbps = 1500,
.has_size_ctl = true,
};
static const struct mtk_dsi_driver_data mt8186_dsi_driver_data = {
.reg_main = mtk_dsi_regs_main_v1,
.reg_adv = mtk_dsi_regs_mt8186,
+ .max_link_rate_mbps = 1500,
.has_size_ctl = true,
};
static const struct mtk_dsi_driver_data mt8188_dsi_driver_data = {
.reg_main = mtk_dsi_regs_main_v1,
.reg_adv = mtk_dsi_regs_mt8186,
+ .max_link_rate_mbps = 1500,
+ .has_size_ctl = true,
+ .cmdq_long_packet_ctl = true,
+ .support_per_frame_lp = true,
+};
+
+static const struct mtk_dsi_driver_data mt8189_dsi_driver_data = {
+ .reg_main = mtk_dsi_regs_main_v1,
+ .reg_adv = mtk_dsi_regs_mt8186,
+ .max_link_rate_mbps = 2500,
.has_size_ctl = true,
.cmdq_long_packet_ctl = true,
.support_per_frame_lp = true,
@@ -1579,6 +1603,7 @@ static const struct of_device_id mtk_dsi_of_match[] = {
{ .compatible = "mediatek,mt8183-dsi", .data = &mt8183_dsi_driver_data },
{ .compatible = "mediatek,mt8186-dsi", .data = &mt8186_dsi_driver_data },
{ .compatible = "mediatek,mt8188-dsi", .data = &mt8188_dsi_driver_data },
+ { .compatible = "mediatek,mt8189-dsi", .data = &mt8189_dsi_driver_data },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, mtk_dsi_of_match);
--
2.54.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v4 07/10] drm/mediatek: mtk_dsi: Add support for MT8196
2026-07-13 14:27 [PATCH v4 00/10] drm/mediatek: Add DSC, WDMA, MT8189/96 DSI support AngeloGioacchino Del Regno
` (5 preceding siblings ...)
2026-07-13 14:27 ` [PATCH v4 06/10] drm/mediatek: mtk_dsi: Add support for MT8189 AngeloGioacchino Del Regno
@ 2026-07-13 14:27 ` AngeloGioacchino Del Regno
2026-07-13 14:39 ` sashiko-bot
2026-07-13 14:27 ` [PATCH v4 08/10] drm/mediatek: mtk_dsi: Enable PM Runtime on probe AngeloGioacchino Del Regno
` (2 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: AngeloGioacchino Del Regno @ 2026-07-13 14:27 UTC (permalink / raw)
To: chunkuang.hu
Cc: p.zabel, maarten.lankhorst, mripard, tzimmermann, airlied, simona,
robh, krzk+dt, conor+dt, matthias.bgg, angelogioacchino.delregno,
jitao.shi, dri-devel, linux-mediatek, devicetree, linux-kernel,
linux-arm-kernel, kernel, justin.yeh, jason-jh.lin
Add support for the new DSI IP found in the Kompanio Ultra MT8196
SoC and its Dimensity and Genio variants.
Differently from the older DSI IPs, the one from MT8196 requires
the initialization of all of the QoS parameters and can make use
of a DSI SRAM reserved buffer (present also on older SoCs but not
mandatory on those).
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 2 +
drivers/gpu/drm/mediatek/mtk_dsi.c | 211 +++++++++++++++++++++++++
2 files changed, 213 insertions(+)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index bd2d17017bd2..9a4c59849c4b 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -826,6 +826,8 @@ static const struct of_device_id mtk_ddp_comp_dt_ids[] = {
.data = (void *)MTK_DSI },
{ .compatible = "mediatek,mt8188-dsi",
.data = (void *)MTK_DSI },
+ { .compatible = "mediatek,mt8196-dsi",
+ .data = (void *)MTK_DSI },
{ }
};
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 5f0674934928..93bc507d77e2 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -153,6 +153,18 @@
#define FORCE_COMMIT BIT(0)
#define BYPASS_SHADOW BIT(1)
+/* DSI_VDE */
+#define VDE_BLOCK_ULTRA BIT(29)
+
+/* DSI_BUF_CON0 */
+#define DSI_QOS_BUF_EN BIT(0)
+
+/* DSI_BUF_CON1 */
+#define BUF_OUT_VALID_THRESH GENMASK(14, 0)
+
+/* DSI_BUF_SODI_HIGH, SODI_LOW and other BUF registers */
+#define BUF_THRESHOLD_PARAM GENMASK(19, 0)
+
/* CMDQ related bits */
#define CONFIG GENMASK(7, 0)
#define SHORT_PACKET 0
@@ -165,6 +177,17 @@
#define NS_TO_CYCLE(n, c) ((n) / (c) + (((n) % (c)) ? 1 : 0))
+/* HW QoS and Anti-Latency Buffer related bits */
+#define MTK_DSI_MAX_FIFO_BYTES 1554
+#define MTK_DSI_DEFAULT_QOS_VALID_FIFO_US 25
+#define MTK_DSI_DEFAULT_QOS_SODI_LO_OVERHEAD (23 + 5)
+#define MTK_DSI_DEFAULT_QOS_PREULTRA_HI_US 36
+#define MTK_DSI_DEFAULT_QOS_PREULTRA_LO_US 35
+#define MTK_DSI_DEFAULT_QOS_ULTRA_HI_US 26
+#define MTK_DSI_DEFAULT_QOS_ULTRA_LO_US 25
+#define MTK_DSI_DEFAULT_QOS_URGENT_LO_US 11
+#define MTK_DSI_DEFAULT_QOS_URGENT_HI_US 12
+
#define MTK_DSI_HOST_IS_READ(type) \
((type == MIPI_DSI_GENERIC_READ_REQUEST_0_PARAM) || \
(type == MIPI_DSI_GENERIC_READ_REQUEST_1_PARAM) || \
@@ -207,9 +230,26 @@ enum mtk_dsi_adv_regidx {
DSI_VM_CMD_CON,
DSI_SHADOW_DEBUG,
DSI_CMDQ,
+ DSI_VDE,
DSI_ADV_REG_MAX
};
+enum mtk_dsi_qos_regidx {
+ DSI_QOS_BUF_CON0,
+ DSI_QOS_BUF_CON1,
+ DSI_QOS_TX_BUF_RW_TIMES,
+ DSI_QOS_SODI_HIGH,
+ DSI_QOS_SODI_LOW,
+ DSI_QOS_PREULTRA_HIGH,
+ DSI_QOS_PREULTRA_LOW,
+ DSI_QOS_ULTRA_HIGH,
+ DSI_QOS_ULTRA_LOW,
+ DSI_QOS_URGENT_HIGH,
+ DSI_QOS_URGENT_LOW,
+ DSI_QOS_PREURGENT_HIGH,
+ DSI_QOS_REG_MAX
+};
+
struct mtk_phy_timing {
u32 lpx;
u32 da_hs_prepare;
@@ -234,8 +274,12 @@ struct phy;
struct mtk_dsi_driver_data {
const u16 *reg_main;
const u16 *reg_adv;
+ const u16 *reg_qos;
const u16 max_link_rate_mbps;
+ const u8 dsi_sram_bytes;
+ const u8 pixels_per_iter;
+ const u8 num_burst_lines;
bool has_size_ctl;
bool cmdq_long_packet_ctl;
@@ -329,6 +373,59 @@ static const u16 mtk_dsi_regs_mt8186[DSI_ADV_REG_MAX] = {
[DSI_CMDQ] = 0xd00,
};
+static const u16 mtk_dsi_regs_main_v2[DSI_MAIN_REG_MAX] = {
+ [DSI_START] = 0x00,
+ [DSI_INTEN] = 0x08,
+ [DSI_INTSTA] = 0x0c,
+ [DSI_CON_CTRL] = 0x30,
+ [DSI_MODE_CTRL] = 0x34,
+ [DSI_TXRX_CTRL] = 0x38,
+ [DSI_PSCTRL] = 0x3c,
+ [DSI_VSA_NL] = 0x60,
+ [DSI_VBP_NL] = 0x64,
+ [DSI_VFP_NL] = 0x68,
+ [DSI_VACT_NL] = 0x6c,
+ [DSI_SIZE_CON] = 0x2c,
+ [DSI_HSA_WC] = 0x80,
+ [DSI_HBP_WC] = 0x84,
+ [DSI_HFP_WC] = 0x88,
+ [DSI_CMDQ_SIZE] = 0x44,
+ [DSI_HSTX_CKL_WC] = 0x100,
+ [DSI_RX_DATA0] = 0xa4,
+ [DSI_RX_DATA1] = 0xa8,
+ [DSI_RX_DATA2] = 0xac,
+ [DSI_RX_DATA3] = 0xb0,
+ [DSI_RACK] = 0xb4,
+ [DSI_PHY_LCCON] = 0x7d0,
+ [DSI_PHY_LD0CON] = 0x7d4,
+ [DSI_PHY_TIMECON0] = 0x600,
+ [DSI_PHY_TIMECON1] = 0x604,
+ [DSI_PHY_TIMECON2] = 0x608,
+ [DSI_PHY_TIMECON3] = 0x60c,
+};
+
+static const u16 mtk_dsi_regs_qos_v2[DSI_QOS_REG_MAX] = {
+ [DSI_QOS_BUF_CON0] = 0x300,
+ [DSI_QOS_BUF_CON1] = 0x304,
+ [DSI_QOS_TX_BUF_RW_TIMES] = 0x310,
+ [DSI_QOS_SODI_HIGH] = 0x314,
+ [DSI_QOS_SODI_LOW] = 0x318,
+ [DSI_QOS_PREULTRA_HIGH] = 0x324,
+ [DSI_QOS_PREULTRA_LOW] = 0x328,
+ [DSI_QOS_ULTRA_HIGH] = 0x32c,
+ [DSI_QOS_ULTRA_LOW] = 0x330,
+ [DSI_QOS_URGENT_HIGH] = 0x334,
+ [DSI_QOS_URGENT_LOW] = 0x338,
+ [DSI_QOS_PREURGENT_HIGH] = 0x33c
+};
+
+static const u16 mtk_dsi_regs_mt8196[DSI_ADV_REG_MAX] = {
+ [DSI_VM_CMD_CON] = 0x110,
+ [DSI_SHADOW_DEBUG] = 0xd0,
+ [DSI_CMDQ] = 0x400,
+ [DSI_VDE] = 0x3f8,
+};
+
static inline struct mtk_dsi *bridge_to_dsi(struct drm_bridge *b)
{
return container_of(b, struct mtk_dsi, bridge);
@@ -776,6 +873,102 @@ static int mtk_dsi_set_dsc_params(struct mtk_dsi *dsi)
return drm_dsc_compute_rc_parameters(dsc);
}
+static void mtk_dsi_config_hw_buffers(struct mtk_dsi *dsi)
+{
+ const struct mtk_dsi_driver_data *data = dsi->driver_data;
+ const u16 *reg_qos = data->reg_qos;
+ u32 buffer_unit, sram_unit, num_hw_buffers;
+ u32 preultra_hi, preultra_lo;
+ u32 urgent_hi, urgent_lo;
+ u32 ultra_hi, ultra_lo;
+ u32 sodi_hi, sodi_lo;
+ u32 data_rate_per_buf;
+ u32 out_valid_thresh;
+ u32 dsi_buf_bpp;
+ u32 fill_rate;
+ u32 pclk_mhz;
+ u32 rw_times;
+ u32 val;
+ u64 tmp;
+
+ /*
+ * At the time of writing, only MT8196 is implemented and, for this SoC,
+ * the buffer unit is equal to the SRAM bytes.
+ *
+ * There are other SoCs already out in the wild that do support the HW
+ * buffers and that have different sizes, so keep the calculation as-is!
+ */
+ buffer_unit = data->dsi_sram_bytes;
+ sram_unit = data->dsi_sram_bytes;
+ num_hw_buffers = sram_unit / buffer_unit;
+
+ if (data->support_per_frame_lp)
+ val = CMDMODE_WAIT_DATA_EVERY_LINE_EN;
+ else
+ val = 0;
+
+ mtk_dsi_mask(dsi, data->reg_main[DSI_CON_CTRL],
+ CMDMODE_WAIT_DATA_EVERY_LINE_EN, val);
+
+ /* Read as: [Data rate (MHz)] * [Number of DSI lanes] / [8 buffer blocks] */
+ tmp = (u64)dsi->data_rate * dsi->lanes;
+ data_rate_per_buf = div_u64(tmp, 8 * buffer_unit * HZ_PER_MHZ);
+
+ /*
+ * Anti-latency buffer output threshold for absolute timer mode: this
+ * parameter controls the maximum amount of output data that the FIFO
+ * can hold before running out of buffer space.
+ *
+ * The data will therefore be sent either when the DSI IP0s internal
+ * vblank vs bus QoS timer expires or when it reaches the amount of
+ * buffers set in BUF_OUT_VALID_THRESHOLD (regardless of QoS) to avoid
+ * partially, or entirely, losing frame(s).
+ */
+ out_valid_thresh = MTK_DSI_DEFAULT_QOS_VALID_FIFO_US * data_rate_per_buf;
+ out_valid_thresh = min(out_valid_thresh, MTK_DSI_MAX_FIFO_BYTES - 1);
+ mtk_dsi_mask(dsi, reg_qos[DSI_QOS_BUF_CON1], BUF_OUT_VALID_THRESH, out_valid_thresh);
+
+ /* Enable ULTRA signal trigger between SOF and VACT */
+ mtk_dsi_mask(dsi, data->reg_adv[DSI_VDE], VDE_BLOCK_ULTRA, 0);
+
+ /* Calculate fill rate with line counter mode for DSI Video Mode */
+ if (dsi->format == MIPI_DSI_FMT_RGB565)
+ dsi_buf_bpp = 2;
+ else
+ dsi_buf_bpp = 3;
+
+ pclk_mhz = dsi->vm.pixelclock / HZ_PER_MHZ;
+ fill_rate = div_u64((u64)pclk_mhz * data->pixels_per_iter * dsi_buf_bpp,
+ buffer_unit);
+
+ /* Calculate QoS Anti-Latency parameters */
+ sodi_hi = MTK_DSI_MAX_FIFO_BYTES * num_hw_buffers;
+ sodi_hi -= (fill_rate - data_rate_per_buf) * 12 / 10;
+ sodi_lo = MTK_DSI_DEFAULT_QOS_SODI_LO_OVERHEAD * data_rate_per_buf;
+ preultra_hi = MTK_DSI_DEFAULT_QOS_PREULTRA_HI_US * data_rate_per_buf;
+ preultra_lo = MTK_DSI_DEFAULT_QOS_PREULTRA_LO_US * data_rate_per_buf;
+ ultra_hi = MTK_DSI_DEFAULT_QOS_ULTRA_HI_US * data_rate_per_buf;
+ ultra_lo = MTK_DSI_DEFAULT_QOS_ULTRA_LO_US * data_rate_per_buf;
+ urgent_hi = MTK_DSI_DEFAULT_QOS_URGENT_HI_US * data_rate_per_buf;
+ urgent_lo = MTK_DSI_DEFAULT_QOS_URGENT_LO_US * data_rate_per_buf;
+ rw_times = dsi->vm.vactive * dsi_buf_bpp;
+ rw_times /= data->num_burst_lines * data->pixels_per_iter;
+
+ /* Write all QoS parameters: Screen On Deep Idle, (pre)Ultra, Urgent, RW times */
+ mtk_dsi_mask(dsi, reg_qos[DSI_QOS_SODI_HIGH], BUF_THRESHOLD_PARAM, sodi_hi);
+ mtk_dsi_mask(dsi, reg_qos[DSI_QOS_SODI_LOW], BUF_THRESHOLD_PARAM, sodi_lo);
+ mtk_dsi_mask(dsi, reg_qos[DSI_QOS_PREULTRA_HIGH], BUF_THRESHOLD_PARAM, preultra_hi);
+ mtk_dsi_mask(dsi, reg_qos[DSI_QOS_PREULTRA_LOW], BUF_THRESHOLD_PARAM, preultra_lo);
+ mtk_dsi_mask(dsi, reg_qos[DSI_QOS_ULTRA_HIGH], BUF_THRESHOLD_PARAM, ultra_hi);
+ mtk_dsi_mask(dsi, reg_qos[DSI_QOS_ULTRA_LOW], BUF_THRESHOLD_PARAM, ultra_lo);
+ mtk_dsi_mask(dsi, reg_qos[DSI_QOS_URGENT_HIGH], BUF_THRESHOLD_PARAM, urgent_hi);
+ mtk_dsi_mask(dsi, reg_qos[DSI_QOS_URGENT_LOW], BUF_THRESHOLD_PARAM, urgent_lo);
+ writel(rw_times, dsi->regs + reg_qos[DSI_QOS_TX_BUF_RW_TIMES]);
+
+ /* Finally, activate internal line-buffering */
+ mtk_dsi_mask(dsi, reg_qos[DSI_QOS_BUF_CON0], DSI_QOS_BUF_EN, DSI_QOS_BUF_EN);
+}
+
static int mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi)
{
const struct mtk_dsi_driver_data *data = dsi->driver_data;
@@ -961,6 +1154,10 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
mtk_dsi_reset_engine(dsi);
mtk_dsi_phy_timconfig(dsi);
+ /* Setup HW FIFO if DSI supports QoS Anti-Latency buffers */
+ if (data->dsi_sram_bytes && data->reg_qos)
+ mtk_dsi_config_hw_buffers(dsi);
+
mtk_dsi_ps_control(dsi, true);
mtk_dsi_set_vm_cmd(dsi);
ret = mtk_dsi_config_vdo_timing(dsi);
@@ -1596,6 +1793,19 @@ static const struct mtk_dsi_driver_data mt8189_dsi_driver_data = {
.support_per_frame_lp = true,
};
+static const struct mtk_dsi_driver_data mt8196_dsi_driver_data = {
+ .reg_main = mtk_dsi_regs_main_v2,
+ .reg_qos = mtk_dsi_regs_qos_v2,
+ .reg_adv = mtk_dsi_regs_mt8196,
+ .max_link_rate_mbps = 2000,
+ .dsi_sram_bytes = 32,
+ .pixels_per_iter = 2,
+ .num_burst_lines = 8,
+ .has_size_ctl = true,
+ .cmdq_long_packet_ctl = true,
+ .support_per_frame_lp = true,
+};
+
static const struct of_device_id mtk_dsi_of_match[] = {
{ .compatible = "mediatek,mt2701-dsi", .data = &mt2701_dsi_driver_data },
{ .compatible = "mediatek,mt8167-dsi", .data = &mt2701_dsi_driver_data },
@@ -1604,6 +1814,7 @@ static const struct of_device_id mtk_dsi_of_match[] = {
{ .compatible = "mediatek,mt8186-dsi", .data = &mt8186_dsi_driver_data },
{ .compatible = "mediatek,mt8188-dsi", .data = &mt8188_dsi_driver_data },
{ .compatible = "mediatek,mt8189-dsi", .data = &mt8189_dsi_driver_data },
+ { .compatible = "mediatek,mt8196-dsi", .data = &mt8196_dsi_driver_data },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, mtk_dsi_of_match);
--
2.54.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v4 07/10] drm/mediatek: mtk_dsi: Add support for MT8196
2026-07-13 14:27 ` [PATCH v4 07/10] drm/mediatek: mtk_dsi: Add support for MT8196 AngeloGioacchino Del Regno
@ 2026-07-13 14:39 ` sashiko-bot
0 siblings, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-07-13 14:39 UTC (permalink / raw)
To: AngeloGioacchino Del Regno; +Cc: devicetree, conor+dt, robh, dri-devel
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Dimensional unit mismatch in HW QoS calculations mixing 'bytes' and 'buffer units' leads to FIFO overflow misconfiguration
- [High] Incorrect use of `vactive` instead of `hactive` for horizontal TX buffer read/write times calculation
--
--- Patch [7]: [PATCH v4 07/10] drm/mediatek: mtk_dsi: Add support for MT8196 ---
commit 53b751cd3a037b336029e926c7c021a5e451f750
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
drm/mediatek: mtk_dsi: Add support for MT8196
Add support for the new DSI IP found in the Kompanio Ultra MT8196 SoC.
It initializes QoS parameters and makes use of a DSI SRAM reserved
buffer.
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index 5f06749349289..93bc507d77e20 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
[ ... ]
> @@ -776,6 +873,102 @@ static int mtk_dsi_set_dsc_params(struct mtk_dsi *dsi)
> return drm_dsc_compute_rc_parameters(dsc);
> }
>
> +static void mtk_dsi_config_hw_buffers(struct mtk_dsi *dsi)
> +{
[ ... ]
> + /* Read as: [Data rate (MHz)] * [Number of DSI lanes] / [8 buffer blocks] */
> + tmp = (u64)dsi->data_rate * dsi->lanes;
> + data_rate_per_buf = div_u64(tmp, 8 * buffer_unit * HZ_PER_MHZ);
> +
> + /*
> + * Anti-latency buffer output threshold for absolute timer mode: this
> + * parameter controls the maximum amount of output data that the FIFO
> + * can hold before running out of buffer space.
> + *
> + * The data will therefore be sent either when the DSI IP0s internal
> + * vblank vs bus QoS timer expires or when it reaches the amount of
> + * buffers set in BUF_OUT_VALID_THRESHOLD (regardless of QoS) to avoid
> + * partially, or entirely, losing frame(s).
> + */
> + out_valid_thresh = MTK_DSI_DEFAULT_QOS_VALID_FIFO_US * data_rate_per_buf;
> + out_valid_thresh = min(out_valid_thresh, MTK_DSI_MAX_FIFO_BYTES - 1);
[Severity: High]
Does this incorrectly cap a value in buffer blocks against a constant in bytes?
Since data_rate_per_buf is computed in buffer blocks per microsecond,
out_valid_thresh is calculated in buffer blocks. Capping it against
MTK_DSI_MAX_FIFO_BYTES - 1 (1553) allows up to 1553 blocks. This cap
evaluates to 49,696 bytes, which defeats the safety limit for the
1554-byte physical FIFO limit.
> + mtk_dsi_mask(dsi, reg_qos[DSI_QOS_BUF_CON1], BUF_OUT_VALID_THRESH, out_valid_thresh);
> +
> + /* Enable ULTRA signal trigger between SOF and VACT */
> + mtk_dsi_mask(dsi, data->reg_adv[DSI_VDE], VDE_BLOCK_ULTRA, 0);
> +
> + /* Calculate fill rate with line counter mode for DSI Video Mode */
> + if (dsi->format == MIPI_DSI_FMT_RGB565)
> + dsi_buf_bpp = 2;
> + else
> + dsi_buf_bpp = 3;
> +
> + pclk_mhz = dsi->vm.pixelclock / HZ_PER_MHZ;
> + fill_rate = div_u64((u64)pclk_mhz * data->pixels_per_iter * dsi_buf_bpp,
> + buffer_unit);
> +
> + /* Calculate QoS Anti-Latency parameters */
> + sodi_hi = MTK_DSI_MAX_FIFO_BYTES * num_hw_buffers;
> + sodi_hi -= (fill_rate - data_rate_per_buf) * 12 / 10;
[Severity: High]
Does this mix incompatible units?
sodi_hi initializes as bytes (MTK_DSI_MAX_FIFO_BYTES * num_hw_buffers),
but then subtracts a value in buffer blocks per microsecond
((fill_rate - data_rate_per_buf) * 1.2). This mixes incompatible units
and may result in mathematically meaningless values being programmed
into the hardware.
> + sodi_lo = MTK_DSI_DEFAULT_QOS_SODI_LO_OVERHEAD * data_rate_per_buf;
> + preultra_hi = MTK_DSI_DEFAULT_QOS_PREULTRA_HI_US * data_rate_per_buf;
> + preultra_lo = MTK_DSI_DEFAULT_QOS_PREULTRA_LO_US * data_rate_per_buf;
> + ultra_hi = MTK_DSI_DEFAULT_QOS_ULTRA_HI_US * data_rate_per_buf;
> + ultra_lo = MTK_DSI_DEFAULT_QOS_ULTRA_LO_US * data_rate_per_buf;
> + urgent_hi = MTK_DSI_DEFAULT_QOS_URGENT_HI_US * data_rate_per_buf;
> + urgent_lo = MTK_DSI_DEFAULT_QOS_URGENT_LO_US * data_rate_per_buf;
> + rw_times = dsi->vm.vactive * dsi_buf_bpp;
> + rw_times /= data->num_burst_lines * data->pixels_per_iter;
[Severity: High]
Should this use dsi->vm.hactive instead of vactive?
Multiplying screen height (vactive) by bytes per pixel seems physically
meaningless for calculating horizontal line buffering burst characteristics.
Using hactive (horizontal pixels per line) might be the intended metric
to compute horizontal burst timings correctly.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260713142746.40259-1-angelogioacchino.delregno@collabora.com?part=7
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4 08/10] drm/mediatek: mtk_dsi: Enable PM Runtime on probe
2026-07-13 14:27 [PATCH v4 00/10] drm/mediatek: Add DSC, WDMA, MT8189/96 DSI support AngeloGioacchino Del Regno
` (6 preceding siblings ...)
2026-07-13 14:27 ` [PATCH v4 07/10] drm/mediatek: mtk_dsi: Add support for MT8196 AngeloGioacchino Del Regno
@ 2026-07-13 14:27 ` AngeloGioacchino Del Regno
2026-07-13 14:43 ` sashiko-bot
2026-07-13 14:27 ` [PATCH v4 09/10] dt-bindings: display: mediatek: wdma: Add compatibles for more SoCs AngeloGioacchino Del Regno
2026-07-13 14:27 ` [PATCH v4 10/10] drm/mediatek: Add Write DMA (WDMA) Engine for Writeback support AngeloGioacchino Del Regno
9 siblings, 1 reply; 20+ messages in thread
From: AngeloGioacchino Del Regno @ 2026-07-13 14:27 UTC (permalink / raw)
To: chunkuang.hu
Cc: p.zabel, maarten.lankhorst, mripard, tzimmermann, airlied, simona,
robh, krzk+dt, conor+dt, matthias.bgg, angelogioacchino.delregno,
jitao.shi, dri-devel, linux-mediatek, devicetree, linux-kernel,
linux-arm-kernel, kernel, justin.yeh, jason-jh.lin
Enable PM Runtime at the end of mtk_dsi_probe() so that the
mediatek-drm mtk_ddp_comp_power_on() function is able to do
pm_runtime_resume_and_get() and, analogously, the function
mtk_ddp_comp_power_off() to pm_runtime_put() for the DSI
device.
This makes sure that the power domains for the DSI IP are
turned on and off when necessary.
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
drivers/gpu/drm/mediatek/mtk_dsi.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 93bc507d77e2..1726f5a2683c 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -1731,6 +1731,10 @@ static int mtk_dsi_probe(struct platform_device *pdev)
if (ret)
return dev_err_probe(&pdev->dev, ret, "Failed to request DSI irq\n");
+ ret = devm_pm_runtime_enable(dev);
+ if (ret)
+ return ret;
+
ret = mipi_dsi_host_register(&dsi->host);
if (ret < 0)
return dev_err_probe(dev, ret, "Failed to register DSI host\n");
--
2.54.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v4 08/10] drm/mediatek: mtk_dsi: Enable PM Runtime on probe
2026-07-13 14:27 ` [PATCH v4 08/10] drm/mediatek: mtk_dsi: Enable PM Runtime on probe AngeloGioacchino Del Regno
@ 2026-07-13 14:43 ` sashiko-bot
2026-07-14 8:13 ` AngeloGioacchino Del Regno
0 siblings, 1 reply; 20+ messages in thread
From: sashiko-bot @ 2026-07-13 14:43 UTC (permalink / raw)
To: AngeloGioacchino Del Regno; +Cc: conor+dt, dri-devel, robh, devicetree
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Enabling PM runtime leaves the DSI hardware permanently suspended, resulting in system crashes during unpowered MMIO register accesses.
- [High] The IRQ handler accesses hardware registers without verifying the device's PM runtime state.
--
--- Patch [8]: [PATCH v4 08/10] drm/mediatek: mtk_dsi: Enable PM Runtime on probe ---
commit 7ce79b5636ac6fccf8b21d5b0f6a67f31c6b10eb
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
drm/mediatek: mtk_dsi: Enable PM Runtime on probe
This commit enables PM runtime at the end of mtk_dsi_probe() to allow the
power domains for the DSI IP to be turned on and off when necessary. The
commit message assumes mtk_ddp_comp_power_on() will resume the device.
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index 93bc507d77e20..1726f5a2683cf 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -1731,6 +1731,10 @@ static int mtk_dsi_probe(struct platform_device *pdev)
> if (ret)
> return dev_err_probe(&pdev->dev, ret, "Failed to request DSI irq\n");
>
> + ret = devm_pm_runtime_enable(dev);
> + if (ret)
> + return ret;
> +
[Severity: High]
The commit message states that mtk_ddp_comp_power_on() will resume the
device. Is this assumption correct?
Looking at mtk_crtc_atomic_enable() in drivers/gpu/drm/mediatek/mtk_crtc.c,
mtk_ddp_comp_power_on() is only called for the first component in the display
pipeline (comp[0]), bypassing the DSI component entirely.
Furthermore, even if mtk_ddp_comp_power_on() iterated over all components, it
executes after mtk_dsi_bridge_atomic_pre_enable(). The pre-enable hook calls
mtk_dsi_poweron(), which directly accesses hardware registers:
drivers/gpu/drm/mediatek/mtk_dsi.c:mtk_dsi_poweron() {
...
mtk_dsi_enable(dsi);
/* Bypass shadow and force commit only if the register is present */
if (data->reg_adv[DSI_SHADOW_DEBUG])
writel(FORCE_COMMIT | BYPASS_SHADOW,
dsi->regs + data->reg_adv[DSI_SHADOW_DEBUG]);
...
}
Since the PM domain is still powered off at this point, will these direct
MMIO accesses cause a bus hang or system crash?
[Severity: High]
Now that the device supports PM runtime, can the IRQ handler run while the
device is in the RPM_SUSPENDED state?
The interrupt is unconditionally enabled in mtk_dsi_bind() and remains
enabled at the IRQ controller level even when the device is suspended. If a
spurious or delayed interrupt fires, mtk_dsi_irq() directly reads the
hardware register:
drivers/gpu/drm/mediatek/mtk_dsi.c:mtk_dsi_irq() {
...
status = readl(dsi->regs + dsi->driver_data->reg_main[DSI_INTSTA]) & flag;
...
}
Should the IRQ handler use pm_runtime_get_if_active() before accessing
hardware registers to avoid touching unpowered hardware?
> ret = mipi_dsi_host_register(&dsi->host);
> if (ret < 0)
> return dev_err_probe(dev, ret, "Failed to register DSI host\n");
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260713142746.40259-1-angelogioacchino.delregno@collabora.com?part=8
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v4 08/10] drm/mediatek: mtk_dsi: Enable PM Runtime on probe
2026-07-13 14:43 ` sashiko-bot
@ 2026-07-14 8:13 ` AngeloGioacchino Del Regno
0 siblings, 0 replies; 20+ messages in thread
From: AngeloGioacchino Del Regno @ 2026-07-14 8:13 UTC (permalink / raw)
To: sashiko-reviews; +Cc: conor+dt, dri-devel, robh, devicetree
On 7/13/26 16:43, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Enabling PM runtime leaves the DSI hardware permanently suspended, resulting in system crashes during unpowered MMIO register accesses.
> - [High] The IRQ handler accesses hardware registers without verifying the device's PM runtime state.
> --
>
> --- Patch [8]: [PATCH v4 08/10] drm/mediatek: mtk_dsi: Enable PM Runtime on probe ---
> commit 7ce79b5636ac6fccf8b21d5b0f6a67f31c6b10eb
> Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>
> drm/mediatek: mtk_dsi: Enable PM Runtime on probe
>
> This commit enables PM runtime at the end of mtk_dsi_probe() to allow the
> power domains for the DSI IP to be turned on and off when necessary. The
> commit message assumes mtk_ddp_comp_power_on() will resume the device.
>
>> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
>> index 93bc507d77e20..1726f5a2683cf 100644
>> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
>> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
>> @@ -1731,6 +1731,10 @@ static int mtk_dsi_probe(struct platform_device *pdev)
>> if (ret)
>> return dev_err_probe(&pdev->dev, ret, "Failed to request DSI irq\n");
>>
>> + ret = devm_pm_runtime_enable(dev);
>> + if (ret)
>> + return ret;
>> +
>
> [Severity: High]
> The commit message states that mtk_ddp_comp_power_on() will resume the
> device. Is this assumption correct?
>
> Looking at mtk_crtc_atomic_enable() in drivers/gpu/drm/mediatek/mtk_crtc.c,
> mtk_ddp_comp_power_on() is only called for the first component in the display
> pipeline (comp[0]), bypassing the DSI component entirely.
>
> Furthermore, even if mtk_ddp_comp_power_on() iterated over all components, it
> executes after mtk_dsi_bridge_atomic_pre_enable(). The pre-enable hook calls
> mtk_dsi_poweron(), which directly accesses hardware registers:
>
> drivers/gpu/drm/mediatek/mtk_dsi.c:mtk_dsi_poweron() {
> ...
> mtk_dsi_enable(dsi);
>
> /* Bypass shadow and force commit only if the register is present */
> if (data->reg_adv[DSI_SHADOW_DEBUG])
> writel(FORCE_COMMIT | BYPASS_SHADOW,
> dsi->regs + data->reg_adv[DSI_SHADOW_DEBUG]);
> ...
> }
>
> Since the PM domain is still powered off at this point, will these direct
> MMIO accesses cause a bus hang or system crash?
>
Before the mediatek-drm restructuring, the power domains and clocks will be always
on because the poweron op will be called at startup time.
After the restructuring, power is OFF when DSI unused, and ON when DSI used.
The power domains will therefore be ON when needed in both cases.
> [Severity: High]
> Now that the device supports PM runtime, can the IRQ handler run while the
> device is in the RPM_SUSPENDED state?
>
> The interrupt is unconditionally enabled in mtk_dsi_bind() and remains
> enabled at the IRQ controller level even when the device is suspended. If a
> spurious or delayed interrupt fires, mtk_dsi_irq() directly reads the
> hardware register:
>
> drivers/gpu/drm/mediatek/mtk_dsi.c:mtk_dsi_irq() {
> ...
> status = readl(dsi->regs + dsi->driver_data->reg_main[DSI_INTSTA]) & flag;
> ...
> }
>
> Should the IRQ handler use pm_runtime_get_if_active() before accessing
> hardware registers to avoid touching unpowered hardware?
>
It's guaranteed to be powered. Anyway, even if it is, that will read zero and
will not crash.
>> ret = mipi_dsi_host_register(&dsi->host);
>> if (ret < 0)
>> return dev_err_probe(dev, ret, "Failed to register DSI host\n");
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4 09/10] dt-bindings: display: mediatek: wdma: Add compatibles for more SoCs
2026-07-13 14:27 [PATCH v4 00/10] drm/mediatek: Add DSC, WDMA, MT8189/96 DSI support AngeloGioacchino Del Regno
` (7 preceding siblings ...)
2026-07-13 14:27 ` [PATCH v4 08/10] drm/mediatek: mtk_dsi: Enable PM Runtime on probe AngeloGioacchino Del Regno
@ 2026-07-13 14:27 ` AngeloGioacchino Del Regno
2026-07-13 14:42 ` sashiko-bot
2026-07-13 14:27 ` [PATCH v4 10/10] drm/mediatek: Add Write DMA (WDMA) Engine for Writeback support AngeloGioacchino Del Regno
9 siblings, 1 reply; 20+ messages in thread
From: AngeloGioacchino Del Regno @ 2026-07-13 14:27 UTC (permalink / raw)
To: chunkuang.hu
Cc: p.zabel, maarten.lankhorst, mripard, tzimmermann, airlied, simona,
robh, krzk+dt, conor+dt, matthias.bgg, angelogioacchino.delregno,
jitao.shi, dri-devel, linux-mediatek, devicetree, linux-kernel,
linux-arm-kernel, kernel, justin.yeh, jason-jh.lin,
Krzysztof Kozlowski
Multiple MediaTek SoCs have the Write DMA hardware in their Display
Controller: add the missing compatibles for Dimensity 1200 (MT6893)
and for Kompanio 500/520/820/1200 (MT8183/MT8186/MT8192/MT8195).
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
.../devicetree/bindings/display/mediatek/mediatek,wdma.yaml | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,wdma.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,wdma.yaml
index 97d052b0fb61..c3ed867d058d 100644
--- a/Documentation/devicetree/bindings/display/mediatek/mediatek,wdma.yaml
+++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,wdma.yaml
@@ -22,11 +22,16 @@ properties:
compatible:
oneOf:
- enum:
+ - mediatek,mt6893-disp-wdma
- mediatek,mt8173-disp-wdma
- items:
- enum:
- mediatek,mt6795-disp-wdma
- mediatek,mt8167-disp-wdma
+ - mediatek,mt8183-disp-wdma
+ - mediatek,mt8186-disp-wdma
+ - mediatek,mt8192-disp-wdma
+ - mediatek,mt8195-disp-wdma
- const: mediatek,mt8173-disp-wdma
reg:
--
2.54.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v4 09/10] dt-bindings: display: mediatek: wdma: Add compatibles for more SoCs
2026-07-13 14:27 ` [PATCH v4 09/10] dt-bindings: display: mediatek: wdma: Add compatibles for more SoCs AngeloGioacchino Del Regno
@ 2026-07-13 14:42 ` sashiko-bot
0 siblings, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-07-13 14:42 UTC (permalink / raw)
To: AngeloGioacchino Del Regno; +Cc: conor+dt, robh, devicetree, dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The compatible string `mediatek,mt6893-disp-wdma` is incorrectly added to the standalone enum instead of the fallback items list.
--
--- Patch [9]: [PATCH v4 09/10] dt-bindings: display: mediatek: wdma: Add compatibles for more SoCs ---
commit 8001f83833ae9c684be67bc719c7aaa056f90763
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
dt-bindings: display: mediatek: wdma: Add compatibles for more SoCs
Multiple MediaTek SoCs have the Write DMA hardware in their Display
Controller. This commit adds the missing compatibles for Dimensity 1200
(MT6893) and for Kompanio 500/520/820/1200.
> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,wdma.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,wdma.yaml
> index 97d052b0fb614..c3ed867d058d9 100644
> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,wdma.yaml
> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,wdma.yaml
> @@ -22,11 +22,16 @@ properties:
> compatible:
> oneOf:
> - enum:
> + - mediatek,mt6893-disp-wdma
[Severity: Medium]
Does this incorrectly place mediatek,mt6893-disp-wdma as a standalone
compatible string rather than in the items list requiring a fallback?
If a device tree uses compatible = "mediatek,mt6893-disp-wdma" to satisfy
this schema, the DRM driver will not probe the device since it only matches
mediatek,mt8173-disp-wdma. If the developer instead provides the fallback
compatible = "mediatek,mt6893-disp-wdma", "mediatek,mt8173-disp-wdma" so the
driver can probe it, won't the device tree fail schema validation here?
> - mediatek,mt8173-disp-wdma
> - items:
> - enum:
> - mediatek,mt6795-disp-wdma
> - mediatek,mt8167-disp-wdma
> + - mediatek,mt8183-disp-wdma
> + - mediatek,mt8186-disp-wdma
> + - mediatek,mt8192-disp-wdma
> + - mediatek,mt8195-disp-wdma
> - const: mediatek,mt8173-disp-wdma
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260713142746.40259-1-angelogioacchino.delregno@collabora.com?part=9
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4 10/10] drm/mediatek: Add Write DMA (WDMA) Engine for Writeback support
2026-07-13 14:27 [PATCH v4 00/10] drm/mediatek: Add DSC, WDMA, MT8189/96 DSI support AngeloGioacchino Del Regno
` (8 preceding siblings ...)
2026-07-13 14:27 ` [PATCH v4 09/10] dt-bindings: display: mediatek: wdma: Add compatibles for more SoCs AngeloGioacchino Del Regno
@ 2026-07-13 14:27 ` AngeloGioacchino Del Regno
2026-07-13 14:47 ` sashiko-bot
9 siblings, 1 reply; 20+ messages in thread
From: AngeloGioacchino Del Regno @ 2026-07-13 14:27 UTC (permalink / raw)
To: chunkuang.hu
Cc: p.zabel, maarten.lankhorst, mripard, tzimmermann, airlied, simona,
robh, krzk+dt, conor+dt, matthias.bgg, angelogioacchino.delregno,
jitao.shi, dri-devel, linux-mediatek, devicetree, linux-kernel,
linux-arm-kernel, kernel, justin.yeh, jason-jh.lin
Add a basic driver for the Write DMA Engine and initial compatible
for the MediaTek MT8173 and MediaTek Dimensity 1200 MT6893, and
hook it up to the mtk_ddp_comp and mtk_drm_drv in order for it to
probe.
This display controller component is used to enable the writeback
engine that can be used for faster display image capturing in the
userspace (for example, screenshots and screen recording).
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
drivers/gpu/drm/mediatek/Makefile | 1 +
drivers/gpu/drm/mediatek/mtk_ddp_comp.c | 21 +-
drivers/gpu/drm/mediatek/mtk_disp_drv.h | 20 +
drivers/gpu/drm/mediatek/mtk_disp_wdma.c | 622 +++++++++++++++++++++++
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 4 +
drivers/gpu/drm/mediatek/mtk_drm_drv.h | 1 +
6 files changed, 667 insertions(+), 2 deletions(-)
create mode 100644 drivers/gpu/drm/mediatek/mtk_disp_wdma.c
diff --git a/drivers/gpu/drm/mediatek/Makefile b/drivers/gpu/drm/mediatek/Makefile
index 03b3470ea5b5..8079962597c8 100644
--- a/drivers/gpu/drm/mediatek/Makefile
+++ b/drivers/gpu/drm/mediatek/Makefile
@@ -11,6 +11,7 @@ mediatek-drm-y := mtk_crtc.o \
mtk_disp_ovl.o \
mtk_disp_ovl_adaptor.o \
mtk_disp_rdma.o \
+ mtk_disp_wdma.o \
mtk_drm_drv.o \
mtk_dsi.o \
mtk_dpi.o \
diff --git a/drivers/gpu/drm/mediatek/mtk_ddp_comp.c b/drivers/gpu/drm/mediatek/mtk_ddp_comp.c
index 13aaf12ecbe5..94b356da6de7 100644
--- a/drivers/gpu/drm/mediatek/mtk_ddp_comp.c
+++ b/drivers/gpu/drm/mediatek/mtk_ddp_comp.c
@@ -357,6 +357,22 @@ static const struct mtk_ddp_comp_funcs ddp_rdma = {
.get_num_formats = mtk_rdma_get_num_formats,
};
+static const struct mtk_ddp_comp_funcs ddp_wdma = {
+ .clk_enable = mtk_wdma_clk_enable,
+ .clk_disable = mtk_wdma_clk_disable,
+ .config = mtk_wdma_config,
+ .start = mtk_wdma_start,
+ .stop = mtk_wdma_stop,
+ .register_vblank_cb = mtk_wdma_register_vblank_cb,
+ .unregister_vblank_cb = mtk_wdma_unregister_vblank_cb,
+ .enable_vblank = mtk_wdma_enable_vblank,
+ .disable_vblank = mtk_wdma_disable_vblank,
+ .layer_nr = mtk_wdma_layer_nr,
+ .layer_config = mtk_wdma_layer_config,
+ .get_formats = mtk_wdma_get_formats,
+ .get_num_formats = mtk_wdma_get_num_formats,
+};
+
static const struct mtk_ddp_comp_funcs ddp_ufoe = {
.clk_enable = mtk_ddp_clk_enable,
.clk_disable = mtk_ddp_clk_disable,
@@ -460,8 +476,8 @@ static const struct mtk_ddp_comp_match mtk_ddp_matches[DDP_COMPONENT_DRM_ID_MAX]
[DDP_COMPONENT_RDMA2] = { MTK_DISP_RDMA, 2, &ddp_rdma },
[DDP_COMPONENT_RDMA4] = { MTK_DISP_RDMA, 4, &ddp_rdma },
[DDP_COMPONENT_UFOE] = { MTK_DISP_UFOE, 0, &ddp_ufoe },
- [DDP_COMPONENT_WDMA0] = { MTK_DISP_WDMA, 0, NULL },
- [DDP_COMPONENT_WDMA1] = { MTK_DISP_WDMA, 1, NULL },
+ [DDP_COMPONENT_WDMA0] = { MTK_DISP_WDMA, 0, &ddp_wdma },
+ [DDP_COMPONENT_WDMA1] = { MTK_DISP_WDMA, 1, &ddp_wdma },
};
static bool mtk_ddp_comp_find(struct device *dev,
@@ -642,6 +658,7 @@ int mtk_ddp_comp_init(struct device *dev, struct device_node *node, struct mtk_d
type == MTK_DISP_OVL_2L ||
type == MTK_DISP_PWM ||
type == MTK_DISP_RDMA ||
+ type == MTK_DISP_WDMA ||
type == MTK_DPI ||
type == MTK_DP_INTF ||
type == MTK_DSI)
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
index 5e2d8748120a..e0c30c6c7cc8 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
+++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
@@ -181,6 +181,26 @@ void mtk_mdp_rdma_config(struct device *dev, struct mtk_mdp_rdma_cfg *cfg,
const u32 *mtk_mdp_rdma_get_formats(struct device *dev);
size_t mtk_mdp_rdma_get_num_formats(struct device *dev);
+int mtk_wdma_clk_enable(struct device *dev);
+void mtk_wdma_clk_disable(struct device *dev);
+void mtk_wdma_config(struct device *dev, unsigned int width,
+ unsigned int height, unsigned int vrefresh,
+ unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
+unsigned int mtk_wdma_layer_nr(struct device *dev);
+void mtk_wdma_layer_config(struct device *dev, unsigned int idx,
+ struct mtk_plane_state *state,
+ struct cmdq_pkt *cmdq_pkt);
+void mtk_wdma_start(struct device *dev);
+void mtk_wdma_stop(struct device *dev);
+void mtk_wdma_register_vblank_cb(struct device *dev,
+ void (*vblank_cb)(void *),
+ void *vblank_cb_data);
+void mtk_wdma_unregister_vblank_cb(struct device *dev);
+void mtk_wdma_enable_vblank(struct device *dev);
+void mtk_wdma_disable_vblank(struct device *dev);
+const u32 *mtk_wdma_get_formats(struct device *dev);
+size_t mtk_wdma_get_num_formats(struct device *dev);
+
int mtk_padding_clk_enable(struct device *dev);
void mtk_padding_clk_disable(struct device *dev);
void mtk_padding_start(struct device *dev);
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_wdma.c b/drivers/gpu/drm/mediatek/mtk_disp_wdma.c
new file mode 100644
index 000000000000..7c3df4c1445d
--- /dev/null
+++ b/drivers/gpu/drm/mediatek/mtk_disp_wdma.c
@@ -0,0 +1,622 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2021 MediaTek Inc.
+ * Copyright (c) 2025 Collabora Ltd
+ * AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
+ */
+
+#include <drm/drm_atomic.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_connector.h>
+#include <drm/drm_edid.h>
+#include <drm/drm_fourcc.h>
+#include <drm/drm_framebuffer.h>
+#include <drm/drm_gem_dma_helper.h>
+#include <drm/drm_probe_helper.h>
+#include <drm/drm_writeback.h>
+
+#include <linux/align.h>
+#include <linux/clk.h>
+#include <linux/component.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/soc/mediatek/mtk-cmdq.h>
+#include <linux/wordpart.h>
+
+#include "mtk_crtc.h"
+#include "mtk_ddp_comp.h"
+#include "mtk_disp_drv.h"
+#include "mtk_drm_drv.h"
+
+#define DISP_REG_WDMA_INT_ENABLE 0x000
+ #define WDMA_FRAME_COMPLETE_INT BIT(0)
+#define DISP_REG_WDMA_INT_STATUS 0x004
+#define DISP_REG_WDMA_EN 0x008
+ #define WDMA_ENGINE_EN BIT(0)
+#define DISP_REG_WDMA_CFG 0x014
+ #define WDMA_CFG_OUT_FMT GENMASK(7, 4)
+ #define WDMA_OUT_FMT_RGB565 0
+ #define WDMA_OUT_FMT_RGB888 1
+ #define WDMA_OUT_FMT_RGBA8888 2
+ #define WDMA_OUT_FMT_ARGB8888 3
+ #define WDMA_OUT_FMT_UYVY 4
+ #define WDMA_OUT_FMT_YUY2 5
+ #define WDMA_OUT_FMT_P010 6
+ #define WDMA_OUT_FMT_Y_ONLY 7
+ #define WDMA_OUT_FMT_I420 8
+ #define WDMA_OUT_FMT_ARGB2101010 11
+ #define WDMA_OUT_FMT_NV12 12
+ #define WDMA_CT_EN BIT(11)
+ #define WDMA_CFG_SWAP BIT(16)
+ #define WDMA_UFO_DCP_ENABLE BIT(17)
+ #define WDMA_INT_MTX_SEL GENMASK(27, 23)
+ #define WDMA_CT_COEF_RGB_TO_JPEG 0
+ #define WDMA_CT_COEF_JPEG_TO_RGB 4
+#define DISP_REG_WDMA_SRC_SIZE 0x018
+#define DISP_REG_WDMA_CLIP_SIZE 0x01c
+ #define WDMA_HEIGHT_PX GENMASK(29, 16)
+ #define WDMA_WIDTH_PX GENMASK(13, 0)
+#define DISP_REG_WDMA_CLIP_COORD 0x020
+ #define WDMA_CLIP_Y_COORD GENMASK(29, 16)
+ #define WDMA_CLIP_X_COORD GENMASK(13, 0)
+#define DISP_REG_WDMA_SHADOW_CTRL 0x024
+ #define WDMA_FORCE_COMMIT BIT(0)
+ #define WDMA_BYPASS_SHADOW BIT(1)
+#define DISP_REG_WDMA_DST_W_IN_BYTE 0x028
+#define DISP_REG_WDMA_DST_UV_PITCH 0x078
+ #define WDMA_UV_DST_W_IN_BYTE GENMASK(15, 0)
+#define DISP_REG_WDMA_DST_ADDR_LSB 0xf00
+#define DISP_REG_WDMA_DST_ADDR_MSB_MT6893 0xf20
+#define DISP_REG_WDMA_DST_ADDRX(r, x) (r + (x * 0x4))
+
+static const u32 mtk_wdma_wb_output_formats[] = {
+ DRM_FORMAT_RGB888
+};
+
+static const u32 mt6893_formats[] = {
+ DRM_FORMAT_XRGB8888,
+ DRM_FORMAT_ARGB8888,
+ DRM_FORMAT_BGRX8888,
+ DRM_FORMAT_BGRA8888,
+ DRM_FORMAT_ABGR8888,
+ DRM_FORMAT_XBGR8888,
+ DRM_FORMAT_RGB888,
+ DRM_FORMAT_BGR888,
+ DRM_FORMAT_RGB565,
+ DRM_FORMAT_YUV420,
+ DRM_FORMAT_YVU420,
+ DRM_FORMAT_UYVY,
+ DRM_FORMAT_YUYV,
+};
+
+struct mtk_disp_wdma_data {
+ u32 reg_wdma_dst_addr0_msb;
+ const u32 *formats;
+ size_t num_formats;
+};
+
+struct mtk_disp_wdma {
+ struct device *dev;
+ struct clk *clk;
+ void __iomem *regs;
+ struct cmdq_client_reg cmdq_reg;
+ const struct mtk_disp_wdma_data *data;
+ void (*vblank_cb)(void *data);
+ void *vblank_cb_data;
+ int irq;
+ struct drm_writeback_connector wb_connector;
+ bool wb_pending;
+};
+
+static inline struct mtk_disp_wdma *connector_to_wdma(struct drm_connector *connector)
+{
+ return container_of(connector, struct mtk_disp_wdma, wb_connector.base);
+}
+
+static irqreturn_t mtk_disp_wdma_irq_handler(int irq, void *dev_id)
+{
+ struct mtk_disp_wdma *wdma = dev_id;
+
+ /* Clear frame completion interrupt */
+ writel(0x0, wdma->regs + DISP_REG_WDMA_INT_STATUS);
+
+ if (wdma->vblank_cb)
+ wdma->vblank_cb(wdma->vblank_cb_data);
+
+ /* TODO: Move completion signaling to CMDQ interrupt callback */
+ if (wdma->wb_pending) {
+ drm_writeback_signal_completion(&wdma->wb_connector, 0);
+ wdma->wb_pending = false;
+ }
+
+ return IRQ_HANDLED;
+}
+
+static void wdma_update_bits(struct device *dev, unsigned int reg,
+ unsigned int mask, unsigned int val)
+{
+ struct mtk_disp_wdma *wdma = dev_get_drvdata(dev);
+ unsigned int tmp = readl(wdma->regs + reg);
+
+ tmp = (tmp & ~mask) | (val & mask);
+ writel(tmp, wdma->regs + reg);
+}
+
+void mtk_wdma_register_vblank_cb(struct device *dev,
+ void (*vblank_cb)(void *),
+ void *vblank_cb_data)
+{
+ struct mtk_disp_wdma *wdma = dev_get_drvdata(dev);
+
+ wdma->vblank_cb = vblank_cb;
+ wdma->vblank_cb_data = vblank_cb_data;
+}
+
+void mtk_wdma_unregister_vblank_cb(struct device *dev)
+{
+ struct mtk_disp_wdma *wdma = dev_get_drvdata(dev);
+
+ wdma->vblank_cb = NULL;
+ wdma->vblank_cb_data = NULL;
+}
+
+void mtk_wdma_enable_vblank(struct device *dev)
+{
+ wdma_update_bits(dev, DISP_REG_WDMA_INT_ENABLE, WDMA_FRAME_COMPLETE_INT,
+ WDMA_FRAME_COMPLETE_INT);
+}
+
+void mtk_wdma_disable_vblank(struct device *dev)
+{
+ wdma_update_bits(dev, DISP_REG_WDMA_INT_ENABLE, WDMA_FRAME_COMPLETE_INT, 0);
+}
+
+const u32 *mtk_wdma_get_formats(struct device *dev)
+{
+ struct mtk_disp_wdma *wdma = dev_get_drvdata(dev);
+
+ return wdma->data->formats;
+}
+
+size_t mtk_wdma_get_num_formats(struct device *dev)
+{
+ struct mtk_disp_wdma *wdma = dev_get_drvdata(dev);
+
+ return wdma->data->num_formats;
+}
+
+int mtk_wdma_clk_enable(struct device *dev)
+{
+ struct mtk_disp_wdma *wdma = dev_get_drvdata(dev);
+
+ return clk_prepare_enable(wdma->clk);
+}
+
+void mtk_wdma_clk_disable(struct device *dev)
+{
+ struct mtk_disp_wdma *wdma = dev_get_drvdata(dev);
+
+ clk_disable_unprepare(wdma->clk);
+}
+
+void mtk_wdma_start(struct device *dev)
+{
+ wdma_update_bits(dev, DISP_REG_WDMA_EN, WDMA_ENGINE_EN,
+ WDMA_ENGINE_EN);
+}
+
+void mtk_wdma_stop(struct device *dev)
+{
+ struct mtk_disp_wdma *wdma = dev_get_drvdata(dev);
+
+ if (wdma->wb_pending) {
+ drm_writeback_signal_completion(&wdma->wb_connector, 0);
+ wdma->wb_pending = false;
+ }
+
+ wdma_update_bits(dev, DISP_REG_WDMA_EN, WDMA_ENGINE_EN, 0);
+}
+
+void mtk_wdma_config(struct device *dev, unsigned int width,
+ unsigned int height, unsigned int vrefresh,
+ unsigned int bpc, struct cmdq_pkt *cmdq_pkt)
+{
+ struct mtk_disp_wdma *wdma = dev_get_drvdata(dev);
+
+ writel(WDMA_FORCE_COMMIT | WDMA_BYPASS_SHADOW,
+ wdma->regs + DISP_REG_WDMA_SHADOW_CTRL);
+}
+
+static u32 wdma_fmt_convert(unsigned int fmt)
+{
+ switch (fmt) {
+ default:
+ case DRM_FORMAT_RGB565:
+ return FIELD_PREP(WDMA_CFG_OUT_FMT, WDMA_OUT_FMT_RGB565);
+ case DRM_FORMAT_BGR565:
+ return FIELD_PREP(WDMA_CFG_OUT_FMT, WDMA_OUT_FMT_RGB565) | WDMA_CFG_SWAP;
+ case DRM_FORMAT_RGB888:
+ return FIELD_PREP(WDMA_CFG_OUT_FMT, WDMA_OUT_FMT_RGB888);
+ case DRM_FORMAT_BGR888:
+ return FIELD_PREP(WDMA_CFG_OUT_FMT, WDMA_OUT_FMT_RGB888) | WDMA_CFG_SWAP;
+ case DRM_FORMAT_RGBX8888:
+ case DRM_FORMAT_RGBA8888:
+ return FIELD_PREP(WDMA_CFG_OUT_FMT, WDMA_OUT_FMT_RGBA8888);
+ case DRM_FORMAT_BGRX8888:
+ case DRM_FORMAT_BGRA8888:
+ return FIELD_PREP(WDMA_CFG_OUT_FMT, WDMA_OUT_FMT_RGBA8888) | WDMA_CFG_SWAP;
+ case DRM_FORMAT_XRGB8888:
+ case DRM_FORMAT_ARGB8888:
+ return FIELD_PREP(WDMA_CFG_OUT_FMT, WDMA_OUT_FMT_ARGB8888);
+ case DRM_FORMAT_XBGR8888:
+ case DRM_FORMAT_ABGR8888:
+ return FIELD_PREP(WDMA_CFG_OUT_FMT, WDMA_OUT_FMT_ARGB8888) | WDMA_CFG_SWAP;
+ case DRM_FORMAT_UYVY:
+ return FIELD_PREP(WDMA_CFG_OUT_FMT, WDMA_OUT_FMT_UYVY);
+ case DRM_FORMAT_YUYV:
+ return FIELD_PREP(WDMA_CFG_OUT_FMT, WDMA_OUT_FMT_YUY2);
+ case DRM_FORMAT_YUV420:
+ return FIELD_PREP(WDMA_CFG_OUT_FMT, WDMA_OUT_FMT_I420);
+ case DRM_FORMAT_YVU420:
+ return FIELD_PREP(WDMA_CFG_OUT_FMT, WDMA_OUT_FMT_I420) | WDMA_CFG_SWAP;
+ }
+}
+
+unsigned int mtk_wdma_layer_nr(struct device *dev)
+{
+ return 1;
+}
+
+static void mtk_wdma_ddp_write_dst_addr(struct cmdq_pkt *cmdq_pkt, u64 val,
+ u8 reg_id, struct mtk_disp_wdma *wdma)
+{
+ mtk_ddp_write(cmdq_pkt, lower_32_bits(val), &wdma->cmdq_reg, wdma->regs,
+ DISP_REG_WDMA_DST_ADDRX(DISP_REG_WDMA_DST_ADDR_LSB, reg_id));
+
+ if (wdma->data->reg_wdma_dst_addr0_msb == 0)
+ return;
+
+ mtk_ddp_write(cmdq_pkt, upper_32_bits(val), &wdma->cmdq_reg, wdma->regs,
+ DISP_REG_WDMA_DST_ADDRX(wdma->data->reg_wdma_dst_addr0_msb, reg_id));
+}
+
+static void mtk_wdma_format_config(struct mtk_disp_wdma *wdma,
+ struct mtk_plane_pending_state *pending,
+ const struct drm_format_info *fmt_info,
+ struct cmdq_pkt *cmdq_pkt)
+{
+ unsigned int u_off, u_stride, u_size, v_off;
+ u32 val;
+
+ /*
+ * For RGB formats, this sets the image destination address;
+ * For YUV formats, this sets the Y component destination address.
+ */
+ mtk_wdma_ddp_write_dst_addr(cmdq_pkt, pending->addr, 0, wdma);
+
+ if (!fmt_info->is_yuv) {
+ /* Disable color transform matrix and data compression */
+ mtk_ddp_write_mask(cmdq_pkt, 0, &wdma->cmdq_reg, wdma->regs,
+ DISP_REG_WDMA_CFG,
+ WDMA_UFO_DCP_ENABLE | WDMA_CT_EN);
+ return;
+ }
+
+ /* Additional format config required only for 420 sampling */
+ if (!drm_format_info_is_yuv_sampling_420(fmt_info))
+ return;
+
+ u_off = pending->pitch * pending->height;
+ u_stride = pending->pitch / 2;
+
+ if (drm_format_info_is_yuv_planar(fmt_info)) {
+ /* YUV420 or YVU420 */
+ u_stride = ALIGN(u_stride, 16);
+ u_size = u_stride * pending->height / 2;
+ v_off = u_off + u_size;
+ } else {
+ /* NV12 or NV21 */
+ u_size = u_stride * pending->height / 2;
+ v_off = 0;
+ }
+
+ /* Set U and V components destination addresses */
+ mtk_wdma_ddp_write_dst_addr(cmdq_pkt, pending->addr + u_off, 1, wdma);
+ mtk_wdma_ddp_write_dst_addr(cmdq_pkt, pending->addr + v_off, 2, wdma);
+
+ mtk_ddp_write(cmdq_pkt, FIELD_PREP(WDMA_UV_DST_W_IN_BYTE, u_stride),
+ &wdma->cmdq_reg, wdma->regs, DISP_REG_WDMA_DST_UV_PITCH);
+
+ /* Color transform coefficient selection */
+ val = FIELD_PREP_CONST(WDMA_INT_MTX_SEL, WDMA_CT_COEF_JPEG_TO_RGB);
+ mtk_ddp_write_mask(cmdq_pkt, val, &wdma->cmdq_reg, wdma->regs,
+ DISP_REG_WDMA_CFG, WDMA_INT_MTX_SEL);
+
+ /* Enable color transform matrix, disable data compression */
+ mtk_ddp_write_mask(cmdq_pkt, WDMA_CT_EN, &wdma->cmdq_reg, wdma->regs,
+ DISP_REG_WDMA_CFG, WDMA_UFO_DCP_ENABLE | WDMA_CT_EN);
+}
+
+void mtk_wdma_layer_config(struct device *dev, unsigned int idx,
+ struct mtk_plane_state *state,
+ struct cmdq_pkt *cmdq_pkt)
+{
+ struct mtk_disp_wdma *wdma = dev_get_drvdata(dev);
+ struct mtk_plane_pending_state *pending = &state->pending;
+ unsigned int pitch = pending->pitch & 0xffff;
+ unsigned int fmt = pending->format;
+ unsigned int con = wdma_fmt_convert(fmt);
+ const struct drm_format_info *fmt_info = drm_format_info(fmt);
+ u16 clip_sz_h = pending->height;
+ u16 clip_sz_w = pending->width;
+ u32 val;
+
+ val = FIELD_PREP(WDMA_HEIGHT_PX, pending->height);
+ val |= FIELD_PREP(WDMA_WIDTH_PX, pending->width);
+ mtk_ddp_write(cmdq_pkt, val, &wdma->cmdq_reg, wdma->regs,
+ DISP_REG_WDMA_SRC_SIZE);
+
+ val = FIELD_PREP(WDMA_HEIGHT_PX, pending->y);
+ val |= FIELD_PREP(WDMA_WIDTH_PX, pending->x);
+ mtk_ddp_write(cmdq_pkt, val, &wdma->cmdq_reg, wdma->regs,
+ DISP_REG_WDMA_CLIP_COORD);
+
+ if (fmt_info->is_yuv) {
+ if ((pending->y + pending->height) % 2)
+ clip_sz_h--;
+
+ if ((pending->x + pending->width) % 2)
+ clip_sz_w--;
+ }
+ val = FIELD_PREP(WDMA_HEIGHT_PX, clip_sz_h);
+ val |= FIELD_PREP(WDMA_WIDTH_PX, clip_sz_w);
+ mtk_ddp_write(cmdq_pkt, val, &wdma->cmdq_reg, wdma->regs,
+ DISP_REG_WDMA_CLIP_SIZE);
+
+ mtk_ddp_write(cmdq_pkt, con, &wdma->cmdq_reg, wdma->regs,
+ DISP_REG_WDMA_CFG);
+ mtk_ddp_write(cmdq_pkt, pitch, &wdma->cmdq_reg, wdma->regs,
+ DISP_REG_WDMA_DST_W_IN_BYTE);
+
+ mtk_wdma_format_config(wdma, pending, fmt_info, cmdq_pkt);
+
+ drm_writeback_queue_job(&wdma->wb_connector, wdma->wb_connector.base.state);
+}
+
+static enum drm_connector_status
+mtk_wdma_wb_connector_detect(struct drm_connector *connector, bool force)
+{
+ return connector_status_connected;
+}
+
+static const struct drm_connector_funcs mtk_wdma_wb_connector_funcs = {
+ .detect = mtk_wdma_wb_connector_detect,
+ .fill_modes = drm_helper_probe_single_connector_modes,
+ .destroy = drm_connector_cleanup,
+ .reset = drm_atomic_helper_connector_reset,
+ .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+ .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
+};
+
+static int mtk_wdma_wb_atomic_check(struct drm_encoder *encoder,
+ struct drm_crtc_state *crtc_state,
+ struct drm_connector_state *conn_state)
+{
+ const struct drm_display_mode *mode = &crtc_state->mode;
+ struct drm_framebuffer *fb;
+ int i;
+
+ if (!conn_state->writeback_job || !conn_state->writeback_job->fb)
+ return -EINVAL;
+
+ fb = conn_state->writeback_job->fb;
+ if (fb->width != mode->hdisplay || fb->height != mode->vdisplay)
+ return -EINVAL;
+
+ for (i = 0; i < ARRAY_SIZE(mtk_wdma_wb_output_formats); i++) {
+ if (fb->format->format == mtk_wdma_wb_output_formats[i])
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
+static const struct drm_encoder_helper_funcs mtk_wdma_wb_encoder_helper_funcs = {
+ .atomic_check = mtk_wdma_wb_atomic_check,
+};
+
+static int mtk_wdma_wb_connector_get_modes(struct drm_connector *connector)
+{
+ struct drm_device *dev = connector->dev;
+
+ return drm_add_modes_noedid(connector, dev->mode_config.max_width,
+ dev->mode_config.max_height);
+}
+
+static enum drm_mode_status
+mtk_wdma_wb_connector_mode_valid(struct drm_connector *connector,
+ const struct drm_display_mode *mode)
+{
+ struct drm_device *dev = connector->dev;
+ struct drm_mode_config *mode_config = &dev->mode_config;
+ int w = mode->hdisplay, h = mode->vdisplay;
+
+ if (w < mode_config->min_width || w > mode_config->max_width)
+ return MODE_BAD_HVALUE;
+
+ if (h < mode_config->min_height || h > mode_config->max_height)
+ return MODE_BAD_VVALUE;
+
+ return MODE_OK;
+}
+
+static void mtk_wdma_wb_connector_atomic_commit(struct drm_connector *connector,
+ struct drm_atomic_commit *state)
+{
+ struct drm_connector_state *conn_state =
+ drm_atomic_get_new_connector_state(state, connector);
+ struct mtk_disp_wdma *wdma = connector_to_wdma(connector);
+ struct drm_framebuffer *fb;
+ struct drm_gem_object *gem;
+ struct drm_gem_dma_object *dma_obj;
+ dma_addr_t addr;
+
+ if (WARN_ON(!conn_state->writeback_job))
+ return;
+
+ fb = conn_state->writeback_job->fb;
+ if (!fb)
+ return;
+
+ gem = fb->obj[0];
+ dma_obj = to_drm_gem_dma_obj(gem);
+ addr = dma_obj->dma_addr;
+
+ /* Store writeback pending state before queuing the job */
+ wdma->wb_pending = true;
+
+ mtk_wdma_ddp_write_dst_addr(NULL, addr, 0, wdma);
+ drm_writeback_queue_job(&wdma->wb_connector, conn_state);
+}
+
+static const struct drm_connector_helper_funcs mtk_wdma_wb_connector_helper_funcs = {
+ .get_modes = mtk_wdma_wb_connector_get_modes,
+ .mode_valid = mtk_wdma_wb_connector_mode_valid,
+ .atomic_commit = mtk_wdma_wb_connector_atomic_commit,
+};
+
+static int mtk_disp_wdma_bind(struct device *dev, struct device *master,
+ void *data)
+{
+ struct mtk_disp_wdma *wdma = dev_get_drvdata(dev);
+ struct drm_device *drm_dev = data;
+ int crtcs, ret;
+
+ crtcs = mtk_find_possible_crtcs(drm_dev, wdma->dev);
+ if (crtcs < 0)
+ return crtcs;
+
+ drm_connector_helper_add(&wdma->wb_connector.base,
+ &mtk_wdma_wb_connector_helper_funcs);
+
+ ret = drm_writeback_connector_init(drm_dev, &wdma->wb_connector,
+ &mtk_wdma_wb_connector_funcs,
+ &mtk_wdma_wb_encoder_helper_funcs,
+ mtk_wdma_wb_output_formats,
+ ARRAY_SIZE(mtk_wdma_wb_output_formats),
+ crtcs);
+ if (ret)
+ return ret;
+
+ ret = pm_runtime_get_sync(dev);
+ if (ret)
+ dev_warn(dev, "Could not get PM Runtime!\n");
+
+ /*
+ * Disable and clear pending interrupts - write unconditionally as no
+ * crash (SError, etc) will happen as if unpowered/unclocked this is
+ * simply going to be ignored (and the registers will be 0 at reset).
+ */
+ writel(0x0, wdma->regs + DISP_REG_WDMA_INT_ENABLE);
+ writel(0x0, wdma->regs + DISP_REG_WDMA_INT_STATUS);
+
+ /* Put RPM only if refcount was incremented before to avoid unbalancing */
+ if (ret == 0)
+ pm_runtime_put(dev);
+
+ enable_irq(wdma->irq);
+ return 0;
+}
+
+static void mtk_disp_wdma_unbind(struct device *dev, struct device *master,
+ void *data)
+{
+ struct mtk_disp_wdma *wdma = dev_get_drvdata(dev);
+
+ disable_irq(wdma->irq);
+}
+
+static const struct component_ops mtk_disp_wdma_component_ops = {
+ .bind = mtk_disp_wdma_bind,
+ .unbind = mtk_disp_wdma_unbind,
+};
+
+static int mtk_disp_wdma_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct mtk_disp_wdma *priv;
+ struct resource *res;
+ int ret;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->dev = dev;
+
+ priv->irq = platform_get_irq(pdev, 0);
+ if (priv->irq < 0)
+ return priv->irq;
+
+ priv->clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(priv->clk))
+ return dev_err_probe(dev, PTR_ERR(priv->clk),
+ "failed to get wdma clk\n");
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ priv->regs = devm_ioremap_resource(dev, res);
+ if (IS_ERR(priv->regs))
+ return dev_err_probe(dev, PTR_ERR(priv->regs),
+ "failed to ioremap wdma\n");
+#if IS_REACHABLE(CONFIG_MTK_CMDQ)
+ ret = cmdq_dev_get_client_reg(dev, &priv->cmdq_reg, 0);
+ if (ret)
+ dev_dbg(dev, "get mediatek,gce-client-reg fail!\n");
+#endif
+
+ priv->data = of_device_get_match_data(dev);
+ platform_set_drvdata(pdev, priv);
+
+ ret = devm_request_irq(dev, priv->irq, mtk_disp_wdma_irq_handler,
+ IRQF_NO_AUTOEN, dev_name(dev), priv);
+ if (ret < 0)
+ return dev_err_probe(dev, ret, "Failed to request irq\n");
+
+ ret = devm_pm_runtime_enable(dev);
+ if (ret)
+ return ret;
+
+ ret = component_add(dev, &mtk_disp_wdma_component_ops);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to add component\n");
+
+ return 0;
+}
+
+static const struct mtk_disp_wdma_data mt6893_wdma_driver_data = {
+ .reg_wdma_dst_addr0_msb = DISP_REG_WDMA_DST_ADDR_MSB_MT6893,
+ .formats = mt6893_formats,
+ .num_formats = ARRAY_SIZE(mt6893_formats),
+};
+
+static const struct mtk_disp_wdma_data mt8173_wdma_driver_data = {
+ .formats = mt6893_formats,
+ .num_formats = ARRAY_SIZE(mt6893_formats),
+};
+
+static const struct of_device_id mtk_disp_wdma_driver_dt_match[] = {
+ { .compatible = "mediatek,mt6893-disp-wdma", .data = &mt6893_wdma_driver_data },
+ { .compatible = "mediatek,mt8173-disp-wdma", .data = &mt8173_wdma_driver_data },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mtk_disp_wdma_driver_dt_match);
+
+struct platform_driver mtk_disp_wdma_driver = {
+ .probe = mtk_disp_wdma_probe,
+ .driver = {
+ .name = "mediatek-disp-wdma",
+ .of_match_table = mtk_disp_wdma_driver_dt_match,
+ },
+};
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index 9a4c59849c4b..e54f0654f2f9 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -796,6 +796,8 @@ static const struct of_device_id mtk_ddp_comp_dt_ids[] = {
.data = (void *)MTK_DISP_RDMA },
{ .compatible = "mediatek,mt8173-disp-ufoe",
.data = (void *)MTK_DISP_UFOE },
+ { .compatible = "mediatek,mt6893-disp-wdma",
+ .data = (void *)MTK_DISP_WDMA },
{ .compatible = "mediatek,mt8173-disp-wdma",
.data = (void *)MTK_DISP_WDMA },
{ .compatible = "mediatek,mt2701-dpi",
@@ -1173,6 +1175,7 @@ static int mtk_drm_probe(struct platform_device *pdev)
comp_type == MTK_DISP_OVL_2L ||
comp_type == MTK_DISP_OVL_ADAPTOR ||
comp_type == MTK_DISP_RDMA ||
+ comp_type == MTK_DISP_WDMA ||
comp_type == MTK_DP_INTF ||
comp_type == MTK_DPI ||
comp_type == MTK_DSI) {
@@ -1281,6 +1284,7 @@ static struct platform_driver * const mtk_drm_drivers[] = {
&mtk_disp_ovl_adaptor_driver,
&mtk_disp_ovl_driver,
&mtk_disp_rdma_driver,
+ &mtk_disp_wdma_driver,
&mtk_dpi_driver,
&mtk_drm_platform_driver,
&mtk_dsi_driver,
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
index 43aac2d956e7..8b5c51d93f72 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
@@ -77,6 +77,7 @@ extern struct platform_driver mtk_disp_merge_driver;
extern struct platform_driver mtk_disp_ovl_adaptor_driver;
extern struct platform_driver mtk_disp_ovl_driver;
extern struct platform_driver mtk_disp_rdma_driver;
+extern struct platform_driver mtk_disp_wdma_driver;
extern struct platform_driver mtk_dpi_driver;
extern struct platform_driver mtk_dsi_driver;
extern struct platform_driver mtk_ethdr_driver;
--
2.54.0
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v4 10/10] drm/mediatek: Add Write DMA (WDMA) Engine for Writeback support
2026-07-13 14:27 ` [PATCH v4 10/10] drm/mediatek: Add Write DMA (WDMA) Engine for Writeback support AngeloGioacchino Del Regno
@ 2026-07-13 14:47 ` sashiko-bot
0 siblings, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-07-13 14:47 UTC (permalink / raw)
To: AngeloGioacchino Del Regno; +Cc: devicetree, conor+dt, dri-devel, robh
Thank you for your contribution! Sashiko AI review found 12 potential issue(s) to consider:
- [High] WDMA hardware configuration is misplaced in a plane callback, leaving hardware registers uninitialized.
- [High] Bypass of CMDQ synchronization via raw CPU MMIO writes causes pipeline tearing.
- [High] Unprotected TOCTOU race condition on `wb_pending` can permanently hang the writeback connector or trigger warnings.
- [High] The writeback destination DMA address calculation ignores the framebuffer's plane offset.
- [High] WDMA writeback destination address is incorrectly assigned the source plane address.
- [High] Writeback jobs hang forever because the WDMA frame completion interrupt is never enabled.
- [High] Atomic check breaks standard display updates when no writeback job is queued.
- [High] Missing `.remove` callback in platform driver leads to leaked component and Use-After-Free.
- [High] Unprotected check of `wdma->vblank_cb` can lead to a NULL pointer dereference.
- [Medium] Improper handling of pm_runtime_get_sync() return values leads to a PM usage counter leak.
- [Medium] WDMA writeback format is artificially restricted to RGB888 despite robust hardware support.
- [Medium] `mtk_wdma_stop()` falsely signals pending writeback jobs with success (0) instead of an error code.
--
--- Patch [10]: [PATCH v4 10/10] drm/mediatek: Add Write DMA (WDMA) Engine for Writeback support ---
commit 5da1257f7edfc5dd259de5b087094d68961d4d72
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
drm/mediatek: Add Write DMA (WDMA) Engine for Writeback support
Add a basic driver for the Write DMA Engine and initial compatible for the
MediaTek MT8173 and MediaTek Dimensity 1200 MT6893. This hooks the engine up
to the mtk_ddp_comp and mtk_drm_drv to enable writeback for capturing display
images to userspace.
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_wdma.c b/drivers/gpu/drm/mediatek/mtk_disp_wdma.c
> new file mode 100644
> index 0000000000000..7c3df4c1445d0
> --- /dev/null
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_wdma.c
[ ... ]
> +static irqreturn_t mtk_disp_wdma_irq_handler(int irq, void *dev_id)
> +{
> + struct mtk_disp_wdma *wdma = dev_id;
> +
> + /* Clear frame completion interrupt */
> + writel(0x0, wdma->regs + DISP_REG_WDMA_INT_STATUS);
> +
> + if (wdma->vblank_cb)
> + wdma->vblank_cb(wdma->vblank_cb_data);
[Severity: High]
Can this unprotected check of wdma->vblank_cb result in a NULL pointer
dereference?
If mtk_wdma_unregister_vblank_cb() sets these pointers to NULL in process
context concurrently with this interrupt handler, the compiler might reload
the pointer between the check and execution without locking or memory
barriers.
> +
> + /* TODO: Move completion signaling to CMDQ interrupt callback */
> + if (wdma->wb_pending) {
> + drm_writeback_signal_completion(&wdma->wb_connector, 0);
> + wdma->wb_pending = false;
> + }
> +
> + return IRQ_HANDLED;
> +}
[Severity: High]
Can this race condition between the atomic commit and the IRQ handler hang
the writeback connector?
In mtk_wdma_wb_connector_atomic_commit(), wdma->wb_pending is set to true
before the job is actually queued to the list. If an interrupt fires in that
window, this IRQ handler could see the flag, call signal completion with no
job present, and clear the flag. The job would subsequently be queued but
never signaled.
[ ... ]
> +void mtk_wdma_enable_vblank(struct device *dev)
> +{
> + wdma_update_bits(dev, DISP_REG_WDMA_INT_ENABLE, WDMA_FRAME_COMPLETE_INT,
> + WDMA_FRAME_COMPLETE_INT);
> +}
[Severity: High]
Are the WDMA frame completion interrupts ever enabled?
The mtk_crtc_enable_vblank() function exclusively invokes the enable_vblank
hook for the first component in the display path (comp[0]). Because WDMA is
at the end of the pipeline, it seems this hook is never called, which would
leave writeback jobs hanging indefinitely waiting for completion.
[ ... ]
> +void mtk_wdma_stop(struct device *dev)
> +{
> + struct mtk_disp_wdma *wdma = dev_get_drvdata(dev);
> +
> + if (wdma->wb_pending) {
> + drm_writeback_signal_completion(&wdma->wb_connector, 0);
> + wdma->wb_pending = false;
> + }
> +
> + wdma_update_bits(dev, DISP_REG_WDMA_EN, WDMA_ENGINE_EN, 0);
> +}
[Severity: Medium]
Should an aborted writeback job signal an error code here instead of 0?
Passing 0 (success) on abort could mislead userspace into processing
incomplete, stale, or uninitialized data from the writeback buffer. Signaling
an error like -ECANCELED would properly inform userspace of the aborted
capture.
[ ... ]
> +static void mtk_wdma_format_config(struct mtk_disp_wdma *wdma,
> + struct mtk_plane_pending_state *pending,
> + const struct drm_format_info *fmt_info,
> + struct cmdq_pkt *cmdq_pkt)
> +{
> + unsigned int u_off, u_stride, u_size, v_off;
> + u32 val;
> +
> + /*
> + * For RGB formats, this sets the image destination address;
> + * For YUV formats, this sets the Y component destination address.
> + */
> + mtk_wdma_ddp_write_dst_addr(cmdq_pkt, pending->addr, 0, wdma);
[Severity: High]
Does this assign the source plane address to the writeback destination
address register?
Using pending->addr sets the destination address to the source image being
read by the CRTC plane. This appears to cause the WDMA engine to overwrite
the active framebuffer instead of the intended writeback destination buffer.
[ ... ]
> +void mtk_wdma_layer_config(struct device *dev, unsigned int idx,
> + struct mtk_plane_state *state,
> + struct cmdq_pkt *cmdq_pkt)
> +{
> + struct mtk_disp_wdma *wdma = dev_get_drvdata(dev);
[Severity: High]
Is this configuration callback ever executed?
Since WDMA is at the end of the display pipeline, mtk_crtc_num_comp_planes()
evaluates to 0 for it. Consequently, mtk_ddp_comp_for_plane() never
associates a plane with the WDMA component, and this layer_config callback
is never reached. This would leave the WDMA engine capturing garbage with
uninitialized hardware registers.
[ ... ]
> +static int mtk_wdma_wb_atomic_check(struct drm_encoder *encoder,
> + struct drm_crtc_state *crtc_state,
> + struct drm_connector_state *conn_state)
> +{
> + const struct drm_display_mode *mode = &crtc_state->mode;
> + struct drm_framebuffer *fb;
> + int i;
> +
> + if (!conn_state->writeback_job || !conn_state->writeback_job->fb)
> + return -EINVAL;
[Severity: High]
Does returning -EINVAL here prevent standard screen updates that do not
include a writeback job?
Userspace frequently performs atomic display updates on the CRTC without
queueing a writeback job on every frame. Returning an error rather than 0
in the absence of a writeback job breaks normal DRM display operations.
> +
> + fb = conn_state->writeback_job->fb;
> + if (fb->width != mode->hdisplay || fb->height != mode->vdisplay)
> + return -EINVAL;
> +
> + for (i = 0; i < ARRAY_SIZE(mtk_wdma_wb_output_formats); i++) {
> + if (fb->format->format == mtk_wdma_wb_output_formats[i])
> + return 0;
> + }
> +
> + return -EINVAL;
> +}
[Severity: Medium]
Is the writeback format intentionally restricted to just RGB888?
The mtk_wdma_wb_output_formats array contains exactly one element. The driver
also contains the mt6893_formats array with 13 different supported formats and
a full wdma_fmt_convert() switch statement, which are rendered dead code by
this restriction.
[ ... ]
> +static void mtk_wdma_wb_connector_atomic_commit(struct drm_connector *connector,
> + struct drm_atomic_commit *state)
> +{
> + struct drm_connector_state *conn_state =
> + drm_atomic_get_new_connector_state(state, connector);
> + struct mtk_disp_wdma *wdma = connector_to_wdma(connector);
> + struct drm_framebuffer *fb;
> + struct drm_gem_object *gem;
> + struct drm_gem_dma_object *dma_obj;
> + dma_addr_t addr;
> +
> + if (WARN_ON(!conn_state->writeback_job))
> + return;
> +
> + fb = conn_state->writeback_job->fb;
> + if (!fb)
> + return;
> +
> + gem = fb->obj[0];
> + dma_obj = to_drm_gem_dma_obj(gem);
> + addr = dma_obj->dma_addr;
[Severity: High]
Should the destination DMA address calculation account for the framebuffer's
plane offset?
Ignoring fb->offsets[0] here causes captured image data to be written to the
wrong location within the GEM buffer whenever userspace specifies a non-zero
offset during framebuffer creation.
> +
> + /* Store writeback pending state before queuing the job */
> + wdma->wb_pending = true;
> +
> + mtk_wdma_ddp_write_dst_addr(NULL, addr, 0, wdma);
[Severity: High]
Could passing NULL for cmdq_pkt bypass the CMDQ synchronization and cause
hardware tearing during active scanout?
Explicitly passing a NULL cmdq_pkt to mtk_wdma_ddp_write_dst_addr() forces
a synchronous CPU writel() to the hardware state immediately in process
context, rather than synchronizing the write with VBLANK via the CMDQ.
> + drm_writeback_queue_job(&wdma->wb_connector, conn_state);
> +}
[ ... ]
> +static int mtk_disp_wdma_bind(struct device *dev, struct device *master,
> + void *data)
> +{
[ ... ]
> + ret = pm_runtime_get_sync(dev);
> + if (ret)
> + dev_warn(dev, "Could not get PM Runtime!\n");
> +
> + /*
> + * Disable and clear pending interrupts - write unconditionally as no
> + * crash (SError, etc) will happen as if unpowered/unclocked this is
> + * simply going to be ignored (and the registers will be 0 at reset).
> + */
> + writel(0x0, wdma->regs + DISP_REG_WDMA_INT_ENABLE);
> + writel(0x0, wdma->regs + DISP_REG_WDMA_INT_STATUS);
> +
> + /* Put RPM only if refcount was incremented before to avoid unbalancing */
> + if (ret == 0)
> + pm_runtime_put(dev);
[Severity: Medium]
Does this evaluation of pm_runtime_get_sync() leak the PM usage counter?
The pm_runtime_get_sync() function returns 1 if the device was already
active, and a negative error code on failure. In both cases, the PM core
still increments the usage counter. Checking for exactly 0 means the
pm_runtime_put(dev) is skipped for ret == 1 or ret < 0, resulting in a
permanent usage counter leak that prevents the device from suspending.
[ ... ]
> +struct platform_driver mtk_disp_wdma_driver = {
> + .probe = mtk_disp_wdma_probe,
> + .driver = {
> + .name = "mediatek-disp-wdma",
> + .of_match_table = mtk_disp_wdma_driver_dt_match,
> + },
> +};
[Severity: High]
Could the missing .remove callback lead to a component leak and
use-after-free when the device is unbound?
The mtk_disp_wdma_probe() function uses component_add(), but the driver
lacks a corresponding .remove callback to invoke component_del(). If the
module is unloaded or the device is unbound, the component framework retains
a dangling pointer to the removed component and its ops.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260713142746.40259-1-angelogioacchino.delregno@collabora.com?part=10
^ permalink raw reply [flat|nested] 20+ messages in thread