Changes since v1:
- separate frame size and reg commit control independent patches.
- fix some return values in probe
- remove DSI_CMDW0 in "CMDQ reg address of mt8173 is different with mt2701"
Jitao Shi (5):
drm/mediatek: move mipi_dsi_host_register to probe
drm/mediatek: CMDQ reg address of mt8173 is different with mt2701
drm/mediatek: add dsi reg commit control
drm/mediatek: add frame size control
drm/mediatek: add mt8183 dsi driver support
drivers/gpu/drm/mediatek/mtk_dsi.c | 112 +++++++++++++++++++++--------
1 file changed, 83 insertions(+), 29 deletions(-)
--
2.21.0
DSI panel driver need attach function which is inculde in
mipi_dsi_host_ops.
If mipi_dsi_host_register is not in probe, dsi panel will
probe fail or more delay.
So move the mipi_dsi_host_register to probe from bind.
Signed-off-by: Jitao Shi <[email protected]>
---
drivers/gpu/drm/mediatek/mtk_dsi.c | 50 ++++++++++++++++++------------
1 file changed, 30 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index b00eb2d2e086..6c4ac37f983d 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -1045,12 +1045,6 @@ static int mtk_dsi_bind(struct device *dev, struct device *master, void *data)
return ret;
}
- ret = mipi_dsi_host_register(&dsi->host);
- if (ret < 0) {
- dev_err(dev, "failed to register DSI host: %d\n", ret);
- goto err_ddp_comp_unregister;
- }
-
ret = mtk_dsi_create_conn_enc(drm, dsi);
if (ret) {
DRM_ERROR("Encoder create failed with %d\n", ret);
@@ -1060,8 +1054,6 @@ static int mtk_dsi_bind(struct device *dev, struct device *master, void *data)
return 0;
err_unregister:
- mipi_dsi_host_unregister(&dsi->host);
-err_ddp_comp_unregister:
mtk_ddp_comp_unregister(drm, &dsi->ddp_comp);
return ret;
}
@@ -1097,31 +1089,37 @@ static int mtk_dsi_probe(struct platform_device *pdev)
dsi->host.ops = &mtk_dsi_ops;
dsi->host.dev = dev;
+ dsi->dev = dev;
+ ret = mipi_dsi_host_register(&dsi->host);
+ if (ret < 0) {
+ dev_err(dev, "failed to register DSI host: %d\n", ret);
+ return ret;
+ }
ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0,
&dsi->panel, &dsi->bridge);
if (ret)
- return ret;
+ goto err_unregister_host;
dsi->engine_clk = devm_clk_get(dev, "engine");
if (IS_ERR(dsi->engine_clk)) {
ret = PTR_ERR(dsi->engine_clk);
dev_err(dev, "Failed to get engine clock: %d\n", ret);
- return ret;
+ goto err_unregister_host;
}
dsi->digital_clk = devm_clk_get(dev, "digital");
if (IS_ERR(dsi->digital_clk)) {
ret = PTR_ERR(dsi->digital_clk);
dev_err(dev, "Failed to get digital clock: %d\n", ret);
- return ret;
+ goto err_unregister_host;
}
dsi->hs_clk = devm_clk_get(dev, "hs");
if (IS_ERR(dsi->hs_clk)) {
ret = PTR_ERR(dsi->hs_clk);
dev_err(dev, "Failed to get hs clock: %d\n", ret);
- return ret;
+ goto err_unregister_host;
}
regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -1129,33 +1127,35 @@ static int mtk_dsi_probe(struct platform_device *pdev)
if (IS_ERR(dsi->regs)) {
ret = PTR_ERR(dsi->regs);
dev_err(dev, "Failed to ioremap memory: %d\n", ret);
- return ret;
+ goto err_unregister_host;
}
dsi->phy = devm_phy_get(dev, "dphy");
if (IS_ERR(dsi->phy)) {
ret = PTR_ERR(dsi->phy);
dev_err(dev, "Failed to get MIPI-DPHY: %d\n", ret);
- return ret;
+ goto err_unregister_host;
}
comp_id = mtk_ddp_comp_get_id(dev->of_node, MTK_DSI);
if (comp_id < 0) {
dev_err(dev, "Failed to identify by alias: %d\n", comp_id);
- return comp_id;
+ ret = comp_id;
+ goto err_unregister_host;
}
ret = mtk_ddp_comp_init(dev, dev->of_node, &dsi->ddp_comp, comp_id,
&mtk_dsi_funcs);
if (ret) {
dev_err(dev, "Failed to initialize component: %d\n", ret);
- return ret;
+ goto err_unregister_host;
}
irq_num = platform_get_irq(pdev, 0);
if (irq_num < 0) {
- dev_err(&pdev->dev, "failed to request dsi irq resource\n");
- return -EPROBE_DEFER;
+ dev_err(&pdev->dev, "failed to get dsi irq_num: %d\n", irq_num);
+ ret = irq_num;
+ goto err_unregister_host;
}
irq_set_status_flags(irq_num, IRQ_TYPE_LEVEL_LOW);
@@ -1163,14 +1163,24 @@ static int mtk_dsi_probe(struct platform_device *pdev)
IRQF_TRIGGER_LOW, dev_name(&pdev->dev), dsi);
if (ret) {
dev_err(&pdev->dev, "failed to request mediatek dsi irq\n");
- return -EPROBE_DEFER;
+ goto err_unregister_host;
}
init_waitqueue_head(&dsi->irq_wait_queue);
platform_set_drvdata(pdev, dsi);
- return component_add(&pdev->dev, &mtk_dsi_component_ops);
+ ret = component_add(&pdev->dev, &mtk_dsi_component_ops);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to add component: %d\n", ret);
+ goto err_unregister_host;
+ }
+
+ return 0;
+
+err_unregister_host:
+ mipi_dsi_host_unregister(&dsi->host);
+ return ret;
}
static int mtk_dsi_remove(struct platform_device *pdev)
--
2.21.0
Add mt8183 dsi driver data. Enable size control and
reg commit control.
Signed-off-by: Jitao Shi <[email protected]>
---
drivers/gpu/drm/mediatek/mtk_dsi.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 458a700ce74c..f0b36ec38e4f 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -1104,11 +1104,19 @@ static const struct mtk_dsi_driver_data mt2701_dsi_driver_data = {
.reg_cmdq_off = 0x180,
};
+static const struct mtk_dsi_driver_data mt8183_dsi_driver_data = {
+ .reg_cmdq_off = 0x200,
+ .has_shadow_ctl = true,
+ .has_size_ctl = true,
+};
+
static const struct of_device_id mtk_dsi_of_match[] = {
{ .compatible = "mediatek,mt2701-dsi",
.data = &mt2701_dsi_driver_data },
{ .compatible = "mediatek,mt8173-dsi",
.data = &mt8173_dsi_driver_data },
+ { .compatible = "mediatek,mt8183-dsi",
+ .data = &mt8183_dsi_driver_data },
{ },
};
--
2.21.0
Config the different CMDQ reg address in driver data.
Signed-off-by: Jitao Shi <[email protected]>
---
drivers/gpu/drm/mediatek/mtk_dsi.c | 39 +++++++++++++++++++++++-------
1 file changed, 30 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 6c4ac37f983d..573e6bec6d36 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -131,7 +131,6 @@
#define VM_CMD_EN BIT(0)
#define TS_VFP_EN BIT(5)
-#define DSI_CMDQ0 0x180
#define CONFIG (0xff << 0)
#define SHORT_PACKET 0
#define LONG_PACKET 2
@@ -156,6 +155,10 @@
struct phy;
+struct mtk_dsi_driver_data {
+ const u32 reg_cmdq_off;
+};
+
struct mtk_dsi {
struct mtk_ddp_comp ddp_comp;
struct device *dev;
@@ -182,6 +185,7 @@ struct mtk_dsi {
bool enabled;
u32 irq_data;
wait_queue_head_t irq_wait_queue;
+ struct mtk_dsi_driver_data *driver_data;
};
static inline struct mtk_dsi *encoder_to_dsi(struct drm_encoder *e)
@@ -934,6 +938,7 @@ static void mtk_dsi_cmdq(struct mtk_dsi *dsi, const struct mipi_dsi_msg *msg)
const char *tx_buf = msg->tx_buf;
u8 config, cmdq_size, cmdq_off, type = msg->type;
u32 reg_val, cmdq_mask, i;
+ u32 reg_cmdq_off = dsi->driver_data->reg_cmdq_off;
if (MTK_DSI_HOST_IS_READ(type))
config = BTA;
@@ -953,9 +958,11 @@ static void mtk_dsi_cmdq(struct mtk_dsi *dsi, const struct mipi_dsi_msg *msg)
}
for (i = 0; i < msg->tx_len; i++)
- writeb(tx_buf[i], dsi->regs + DSI_CMDQ0 + cmdq_off + i);
+ mtk_dsi_mask(dsi, (reg_cmdq_off + cmdq_off + i) & (~0x3U),
+ (0xffUL << (((i + cmdq_off) & 3U) * 8U)),
+ tx_buf[i] << (((i + cmdq_off) & 3U) * 8U));
- mtk_dsi_mask(dsi, DSI_CMDQ0, cmdq_mask, reg_val);
+ mtk_dsi_mask(dsi, reg_cmdq_off, cmdq_mask, reg_val);
mtk_dsi_mask(dsi, DSI_CMDQ_SIZE, CMDQ_SIZE, cmdq_size);
}
@@ -1074,10 +1081,27 @@ static const struct component_ops mtk_dsi_component_ops = {
.unbind = mtk_dsi_unbind,
};
+static const struct mtk_dsi_driver_data mt8173_dsi_driver_data = {
+ .reg_cmdq_off = 0x200,
+};
+
+static const struct mtk_dsi_driver_data mt2701_dsi_driver_data = {
+ .reg_cmdq_off = 0x180,
+};
+
+static const struct of_device_id mtk_dsi_of_match[] = {
+ { .compatible = "mediatek,mt2701-dsi",
+ .data = &mt2701_dsi_driver_data },
+ { .compatible = "mediatek,mt8173-dsi",
+ .data = &mt8173_dsi_driver_data },
+ { },
+};
+
static int mtk_dsi_probe(struct platform_device *pdev)
{
struct mtk_dsi *dsi;
struct device *dev = &pdev->dev;
+ const struct of_device_id *of_id;
struct resource *regs;
int irq_num;
int comp_id;
@@ -1101,6 +1125,9 @@ static int mtk_dsi_probe(struct platform_device *pdev)
if (ret)
goto err_unregister_host;
+ of_id = of_match_device(mtk_dsi_of_match, &pdev->dev);
+ dsi->driver_data = of_id->data;
+
dsi->engine_clk = devm_clk_get(dev, "engine");
if (IS_ERR(dsi->engine_clk)) {
ret = PTR_ERR(dsi->engine_clk);
@@ -1193,12 +1220,6 @@ static int mtk_dsi_remove(struct platform_device *pdev)
return 0;
}
-static const struct of_device_id mtk_dsi_of_match[] = {
- { .compatible = "mediatek,mt2701-dsi" },
- { .compatible = "mediatek,mt8173-dsi" },
- { },
-};
-
struct platform_driver mtk_dsi_driver = {
.probe = mtk_dsi_probe,
.remove = mtk_dsi_remove,
--
2.21.0
New DSI IP has shadow register and working reg. The register
values are writen to shadow register. And then trigger with
commit reg, the register values will be moved working register.
Signed-off-by: Jitao Shi <[email protected]>
---
drivers/gpu/drm/mediatek/mtk_dsi.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index 573e6bec6d36..be42405a0a78 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -131,6 +131,10 @@
#define VM_CMD_EN BIT(0)
#define TS_VFP_EN BIT(5)
+#define DSI_SHADOW_DEBUG 0x190U
+#define FORCE_COMMIT BIT(0)
+#define BYPASS_SHADOW BIT(1)
+
#define CONFIG (0xff << 0)
#define SHORT_PACKET 0
#define LONG_PACKET 2
@@ -157,6 +161,7 @@ struct phy;
struct mtk_dsi_driver_data {
const u32 reg_cmdq_off;
+ bool has_shadow_ctl;
};
struct mtk_dsi {
@@ -594,6 +599,11 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
}
mtk_dsi_enable(dsi);
+
+ if (dsi->driver_data->has_shadow_ctl)
+ writel(FORCE_COMMIT | BYPASS_SHADOW,
+ dsi->regs + DSI_SHADOW_DEBUG);
+
mtk_dsi_reset_engine(dsi);
mtk_dsi_phy_timconfig(dsi);
--
2.21.0
Our new DSI chip has frame size control.
So add the driver data to control for different chips.
Signed-off-by: Jitao Shi <[email protected]>
---
drivers/gpu/drm/mediatek/mtk_dsi.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
index be42405a0a78..458a700ce74c 100644
--- a/drivers/gpu/drm/mediatek/mtk_dsi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
@@ -78,6 +78,7 @@
#define DSI_VBP_NL 0x24
#define DSI_VFP_NL 0x28
#define DSI_VACT_NL 0x2C
+#define DSI_SIZE_CON 0x38
#define DSI_HSA_WC 0x50
#define DSI_HBP_WC 0x54
#define DSI_HFP_WC 0x58
@@ -162,6 +163,7 @@ struct phy;
struct mtk_dsi_driver_data {
const u32 reg_cmdq_off;
bool has_shadow_ctl;
+ bool has_size_ctl;
};
struct mtk_dsi {
@@ -430,6 +432,9 @@ static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi)
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(vm->vactive << 16 | vm->hactive, dsi->regs + DSI_SIZE_CON);
+
horizontal_sync_active_byte = (vm->hsync_len * dsi_tmp_buf_bpp - 10);
if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE)
--
2.21.0
Hi Jitao,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v5.1-rc5 next-20190416]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Jitao-Shi/drm-mediatek-move-mipi_dsi_host_register-to-probe/20190417-100619
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=arm
All warnings (new ones prefixed by >>):
drivers/gpu//drm/mediatek/mtk_dsi.c: In function 'mtk_dsi_probe':
>> drivers/gpu//drm/mediatek/mtk_dsi.c:1129:19: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
dsi->driver_data = of_id->data;
^
vim +/const +1129 drivers/gpu//drm/mediatek/mtk_dsi.c
1099
1100 static int mtk_dsi_probe(struct platform_device *pdev)
1101 {
1102 struct mtk_dsi *dsi;
1103 struct device *dev = &pdev->dev;
1104 const struct of_device_id *of_id;
1105 struct resource *regs;
1106 int irq_num;
1107 int comp_id;
1108 int ret;
1109
1110 dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL);
1111 if (!dsi)
1112 return -ENOMEM;
1113
1114 dsi->host.ops = &mtk_dsi_ops;
1115 dsi->host.dev = dev;
1116 dsi->dev = dev;
1117 ret = mipi_dsi_host_register(&dsi->host);
1118 if (ret < 0) {
1119 dev_err(dev, "failed to register DSI host: %d\n", ret);
1120 return ret;
1121 }
1122
1123 ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0,
1124 &dsi->panel, &dsi->bridge);
1125 if (ret)
1126 goto err_unregister_host;
1127
1128 of_id = of_match_device(mtk_dsi_of_match, &pdev->dev);
> 1129 dsi->driver_data = of_id->data;
1130
1131 dsi->engine_clk = devm_clk_get(dev, "engine");
1132 if (IS_ERR(dsi->engine_clk)) {
1133 ret = PTR_ERR(dsi->engine_clk);
1134 dev_err(dev, "Failed to get engine clock: %d\n", ret);
1135 goto err_unregister_host;
1136 }
1137
1138 dsi->digital_clk = devm_clk_get(dev, "digital");
1139 if (IS_ERR(dsi->digital_clk)) {
1140 ret = PTR_ERR(dsi->digital_clk);
1141 dev_err(dev, "Failed to get digital clock: %d\n", ret);
1142 goto err_unregister_host;
1143 }
1144
1145 dsi->hs_clk = devm_clk_get(dev, "hs");
1146 if (IS_ERR(dsi->hs_clk)) {
1147 ret = PTR_ERR(dsi->hs_clk);
1148 dev_err(dev, "Failed to get hs clock: %d\n", ret);
1149 goto err_unregister_host;
1150 }
1151
1152 regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
1153 dsi->regs = devm_ioremap_resource(dev, regs);
1154 if (IS_ERR(dsi->regs)) {
1155 ret = PTR_ERR(dsi->regs);
1156 dev_err(dev, "Failed to ioremap memory: %d\n", ret);
1157 goto err_unregister_host;
1158 }
1159
1160 dsi->phy = devm_phy_get(dev, "dphy");
1161 if (IS_ERR(dsi->phy)) {
1162 ret = PTR_ERR(dsi->phy);
1163 dev_err(dev, "Failed to get MIPI-DPHY: %d\n", ret);
1164 goto err_unregister_host;
1165 }
1166
1167 comp_id = mtk_ddp_comp_get_id(dev->of_node, MTK_DSI);
1168 if (comp_id < 0) {
1169 dev_err(dev, "Failed to identify by alias: %d\n", comp_id);
1170 ret = comp_id;
1171 goto err_unregister_host;
1172 }
1173
1174 ret = mtk_ddp_comp_init(dev, dev->of_node, &dsi->ddp_comp, comp_id,
1175 &mtk_dsi_funcs);
1176 if (ret) {
1177 dev_err(dev, "Failed to initialize component: %d\n", ret);
1178 goto err_unregister_host;
1179 }
1180
1181 irq_num = platform_get_irq(pdev, 0);
1182 if (irq_num < 0) {
1183 dev_err(&pdev->dev, "failed to get dsi irq_num: %d\n", irq_num);
1184 ret = irq_num;
1185 goto err_unregister_host;
1186 }
1187
1188 irq_set_status_flags(irq_num, IRQ_TYPE_LEVEL_LOW);
1189 ret = devm_request_irq(&pdev->dev, irq_num, mtk_dsi_irq,
1190 IRQF_TRIGGER_LOW, dev_name(&pdev->dev), dsi);
1191 if (ret) {
1192 dev_err(&pdev->dev, "failed to request mediatek dsi irq\n");
1193 goto err_unregister_host;
1194 }
1195
1196 init_waitqueue_head(&dsi->irq_wait_queue);
1197
1198 platform_set_drvdata(pdev, dsi);
1199
1200 ret = component_add(&pdev->dev, &mtk_dsi_component_ops);
1201 if (ret) {
1202 dev_err(&pdev->dev, "failed to add component: %d\n", ret);
1203 goto err_unregister_host;
1204 }
1205
1206 return 0;
1207
1208 err_unregister_host:
1209 mipi_dsi_host_unregister(&dsi->host);
1210 return ret;
1211 }
1212
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Tue, Apr 16, 2019 at 2:05 PM Jitao Shi <[email protected]> wrote:
>
> Config the different CMDQ reg address in driver data.
>
> Signed-off-by: Jitao Shi <[email protected]>
> ---
> drivers/gpu/drm/mediatek/mtk_dsi.c | 39 +++++++++++++++++++++++-------
> 1 file changed, 30 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index 6c4ac37f983d..573e6bec6d36 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -131,7 +131,6 @@
> #define VM_CMD_EN BIT(0)
> #define TS_VFP_EN BIT(5)
>
> -#define DSI_CMDQ0 0x180
> #define CONFIG (0xff << 0)
> #define SHORT_PACKET 0
> #define LONG_PACKET 2
> @@ -156,6 +155,10 @@
>
> struct phy;
>
> +struct mtk_dsi_driver_data {
> + const u32 reg_cmdq_off;
> +};
> +
> struct mtk_dsi {
> struct mtk_ddp_comp ddp_comp;
> struct device *dev;
> @@ -182,6 +185,7 @@ struct mtk_dsi {
> bool enabled;
> u32 irq_data;
> wait_queue_head_t irq_wait_queue;
> + struct mtk_dsi_driver_data *driver_data;
As highlighted by kbuild, you're missing a const here.
> };
>
> static inline struct mtk_dsi *encoder_to_dsi(struct drm_encoder *e)
> @@ -934,6 +938,7 @@ static void mtk_dsi_cmdq(struct mtk_dsi *dsi, const struct mipi_dsi_msg *msg)
> const char *tx_buf = msg->tx_buf;
> u8 config, cmdq_size, cmdq_off, type = msg->type;
> u32 reg_val, cmdq_mask, i;
> + u32 reg_cmdq_off = dsi->driver_data->reg_cmdq_off;
>
> if (MTK_DSI_HOST_IS_READ(type))
> config = BTA;
> @@ -953,9 +958,11 @@ static void mtk_dsi_cmdq(struct mtk_dsi *dsi, const struct mipi_dsi_msg *msg)
> }
>
> for (i = 0; i < msg->tx_len; i++)
> - writeb(tx_buf[i], dsi->regs + DSI_CMDQ0 + cmdq_off + i);
> + mtk_dsi_mask(dsi, (reg_cmdq_off + cmdq_off + i) & (~0x3U),
> + (0xffUL << (((i + cmdq_off) & 3U) * 8U)),
> + tx_buf[i] << (((i + cmdq_off) & 3U) * 8U));
The writeb call looked significantly cleaner...
writeb(tx_buf[i], dsi->regs + reg_cmdq_off + cmdq_off + i);
should just work, right?
>
> - mtk_dsi_mask(dsi, DSI_CMDQ0, cmdq_mask, reg_val);
> + mtk_dsi_mask(dsi, reg_cmdq_off, cmdq_mask, reg_val);
> mtk_dsi_mask(dsi, DSI_CMDQ_SIZE, CMDQ_SIZE, cmdq_size);
> }
>
> @@ -1074,10 +1081,27 @@ static const struct component_ops mtk_dsi_component_ops = {
> .unbind = mtk_dsi_unbind,
> };
>
> +static const struct mtk_dsi_driver_data mt8173_dsi_driver_data = {
> + .reg_cmdq_off = 0x200,
> +};
> +
> +static const struct mtk_dsi_driver_data mt2701_dsi_driver_data = {
> + .reg_cmdq_off = 0x180,
> +};
> +
> +static const struct of_device_id mtk_dsi_of_match[] = {
> + { .compatible = "mediatek,mt2701-dsi",
> + .data = &mt2701_dsi_driver_data },
> + { .compatible = "mediatek,mt8173-dsi",
> + .data = &mt8173_dsi_driver_data },
> + { },
> +};
> +
> static int mtk_dsi_probe(struct platform_device *pdev)
> {
> struct mtk_dsi *dsi;
> struct device *dev = &pdev->dev;
> + const struct of_device_id *of_id;
> struct resource *regs;
> int irq_num;
> int comp_id;
> @@ -1101,6 +1125,9 @@ static int mtk_dsi_probe(struct platform_device *pdev)
> if (ret)
> goto err_unregister_host;
>
> + of_id = of_match_device(mtk_dsi_of_match, &pdev->dev);
> + dsi->driver_data = of_id->data;
> +
> dsi->engine_clk = devm_clk_get(dev, "engine");
> if (IS_ERR(dsi->engine_clk)) {
> ret = PTR_ERR(dsi->engine_clk);
> @@ -1193,12 +1220,6 @@ static int mtk_dsi_remove(struct platform_device *pdev)
> return 0;
> }
>
> -static const struct of_device_id mtk_dsi_of_match[] = {
> - { .compatible = "mediatek,mt2701-dsi" },
> - { .compatible = "mediatek,mt8173-dsi" },
> - { },
> -};
> -
> struct platform_driver mtk_dsi_driver = {
> .probe = mtk_dsi_probe,
> .remove = mtk_dsi_remove,
> --
> 2.21.0
>
> _______________________________________________
> dri-devel mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi, Jitao:
On Tue, 2019-04-16 at 14:04 +0800, Jitao Shi wrote:
> DSI panel driver need attach function which is inculde in
> mipi_dsi_host_ops.
>
> If mipi_dsi_host_register is not in probe, dsi panel will
> probe fail or more delay.
I think this patch just prevent delay, not to prevent dsi panel probe
fail. In [1], you mention mipi_dsi_attach() is called in
panel_simple_dsi_probe(), but panel_simple_dsi_probe() is trigger by
mipi_dsi_host_register(), so the probe would success.
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/panel/panel-simple.c?h=v5.0-rc6#n2987
>
> So move the mipi_dsi_host_register to probe from bind.
>
> Signed-off-by: Jitao Shi <[email protected]>
> ---
> drivers/gpu/drm/mediatek/mtk_dsi.c | 50 ++++++++++++++++++------------
> 1 file changed, 30 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index b00eb2d2e086..6c4ac37f983d 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -1045,12 +1045,6 @@ static int mtk_dsi_bind(struct device *dev, struct device *master, void *data)
> return ret;
> }
>
> - ret = mipi_dsi_host_register(&dsi->host);
> - if (ret < 0) {
> - dev_err(dev, "failed to register DSI host: %d\n", ret);
> - goto err_ddp_comp_unregister;
> - }
> -
> ret = mtk_dsi_create_conn_enc(drm, dsi);
> if (ret) {
> DRM_ERROR("Encoder create failed with %d\n", ret);
> @@ -1060,8 +1054,6 @@ static int mtk_dsi_bind(struct device *dev, struct device *master, void *data)
> return 0;
>
> err_unregister:
> - mipi_dsi_host_unregister(&dsi->host);
> -err_ddp_comp_unregister:
> mtk_ddp_comp_unregister(drm, &dsi->ddp_comp);
> return ret;
> }
> @@ -1097,31 +1089,37 @@ static int mtk_dsi_probe(struct platform_device *pdev)
>
> dsi->host.ops = &mtk_dsi_ops;
> dsi->host.dev = dev;
> + dsi->dev = dev;
Why do this?
Regards,
CK
> + ret = mipi_dsi_host_register(&dsi->host);
> + if (ret < 0) {
> + dev_err(dev, "failed to register DSI host: %d\n", ret);
> + return ret;
> + }
>
> ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0,
> &dsi->panel, &dsi->bridge);
> if (ret)
> - return ret;
> + goto err_unregister_host;
>
> dsi->engine_clk = devm_clk_get(dev, "engine");
> if (IS_ERR(dsi->engine_clk)) {
> ret = PTR_ERR(dsi->engine_clk);
> dev_err(dev, "Failed to get engine clock: %d\n", ret);
> - return ret;
> + goto err_unregister_host;
> }
>
> dsi->digital_clk = devm_clk_get(dev, "digital");
> if (IS_ERR(dsi->digital_clk)) {
> ret = PTR_ERR(dsi->digital_clk);
> dev_err(dev, "Failed to get digital clock: %d\n", ret);
> - return ret;
> + goto err_unregister_host;
> }
>
> dsi->hs_clk = devm_clk_get(dev, "hs");
> if (IS_ERR(dsi->hs_clk)) {
> ret = PTR_ERR(dsi->hs_clk);
> dev_err(dev, "Failed to get hs clock: %d\n", ret);
> - return ret;
> + goto err_unregister_host;
> }
>
> regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
On Tue, 2019-04-16 at 14:04 +0800, Jitao Shi wrote:
> Config the different CMDQ reg address in driver data.
>
For MT8173, you change reg_cmd_off from 0x180 to 0x200, so this patch is
a bug fix. You should add a 'Fixes' tag.
> Signed-off-by: Jitao Shi <[email protected]>
> ---
> drivers/gpu/drm/mediatek/mtk_dsi.c | 39 +++++++++++++++++++++++-------
> 1 file changed, 30 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index 6c4ac37f983d..573e6bec6d36 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -131,7 +131,6 @@
> #define VM_CMD_EN BIT(0)
> #define TS_VFP_EN BIT(5)
>
> -#define DSI_CMDQ0 0x180
> #define CONFIG (0xff << 0)
> #define SHORT_PACKET 0
> #define LONG_PACKET 2
> @@ -156,6 +155,10 @@
>
> struct phy;
>
> +struct mtk_dsi_driver_data {
> + const u32 reg_cmdq_off;
> +};
> +
> struct mtk_dsi {
> struct mtk_ddp_comp ddp_comp;
> struct device *dev;
> @@ -182,6 +185,7 @@ struct mtk_dsi {
> bool enabled;
> u32 irq_data;
> wait_queue_head_t irq_wait_queue;
> + struct mtk_dsi_driver_data *driver_data;
> };
>
> static inline struct mtk_dsi *encoder_to_dsi(struct drm_encoder *e)
> @@ -934,6 +938,7 @@ static void mtk_dsi_cmdq(struct mtk_dsi *dsi, const struct mipi_dsi_msg *msg)
> const char *tx_buf = msg->tx_buf;
> u8 config, cmdq_size, cmdq_off, type = msg->type;
> u32 reg_val, cmdq_mask, i;
> + u32 reg_cmdq_off = dsi->driver_data->reg_cmdq_off;
>
> if (MTK_DSI_HOST_IS_READ(type))
> config = BTA;
> @@ -953,9 +958,11 @@ static void mtk_dsi_cmdq(struct mtk_dsi *dsi, const struct mipi_dsi_msg *msg)
> }
>
> for (i = 0; i < msg->tx_len; i++)
> - writeb(tx_buf[i], dsi->regs + DSI_CMDQ0 + cmdq_off + i);
> + mtk_dsi_mask(dsi, (reg_cmdq_off + cmdq_off + i) & (~0x3U),
> + (0xffUL << (((i + cmdq_off) & 3U) * 8U)),
> + tx_buf[i] << (((i + cmdq_off) & 3U) * 8U));
You say you would follow Nicolas' suggestion here.
>
> - mtk_dsi_mask(dsi, DSI_CMDQ0, cmdq_mask, reg_val);
> + mtk_dsi_mask(dsi, reg_cmdq_off, cmdq_mask, reg_val);
> mtk_dsi_mask(dsi, DSI_CMDQ_SIZE, CMDQ_SIZE, cmdq_size);
> }
>
> @@ -1074,10 +1081,27 @@ static const struct component_ops mtk_dsi_component_ops = {
> .unbind = mtk_dsi_unbind,
> };
>
> +static const struct mtk_dsi_driver_data mt8173_dsi_driver_data = {
> + .reg_cmdq_off = 0x200,
> +};
> +
> +static const struct mtk_dsi_driver_data mt2701_dsi_driver_data = {
> + .reg_cmdq_off = 0x180,
> +};
> +
> +static const struct of_device_id mtk_dsi_of_match[] = {
> + { .compatible = "mediatek,mt2701-dsi",
> + .data = &mt2701_dsi_driver_data },
> + { .compatible = "mediatek,mt8173-dsi",
> + .data = &mt8173_dsi_driver_data },
> + { },
> +};
> +
> static int mtk_dsi_probe(struct platform_device *pdev)
> {
> struct mtk_dsi *dsi;
> struct device *dev = &pdev->dev;
> + const struct of_device_id *of_id;
> struct resource *regs;
> int irq_num;
> int comp_id;
> @@ -1101,6 +1125,9 @@ static int mtk_dsi_probe(struct platform_device *pdev)
> if (ret)
> goto err_unregister_host;
>
> + of_id = of_match_device(mtk_dsi_of_match, &pdev->dev);
> + dsi->driver_data = of_id->data;
Maybe use of_device_get_match_data() is a more simple way. You could
refer to [1].
[1]
https://elixir.bootlin.com/linux/v5.1/source/drivers/gpu/drm/mediatek/mtk_disp_ovl.c#L300
Regards,
CK
> +
> dsi->engine_clk = devm_clk_get(dev, "engine");
> if (IS_ERR(dsi->engine_clk)) {
> ret = PTR_ERR(dsi->engine_clk);
> @@ -1193,12 +1220,6 @@ static int mtk_dsi_remove(struct platform_device *pdev)
> return 0;
> }
>
> -static const struct of_device_id mtk_dsi_of_match[] = {
> - { .compatible = "mediatek,mt2701-dsi" },
> - { .compatible = "mediatek,mt8173-dsi" },
> - { },
> -};
> -
> struct platform_driver mtk_dsi_driver = {
> .probe = mtk_dsi_probe,
> .remove = mtk_dsi_remove,
Hi, Jitao:
On Tue, 2019-04-16 at 14:04 +0800, Jitao Shi wrote:
> New DSI IP has shadow register and working reg. The register
> values are writen to shadow register. And then trigger with
> commit reg, the register values will be moved working register.
This patch looks good, but the message is not complete. The message make
us believe you use shadow register to work, but actually, shadow
register is default turn on in new DSI IP and you want to turn off it.
Regards,
CK
>
> Signed-off-by: Jitao Shi <[email protected]>
> ---
> drivers/gpu/drm/mediatek/mtk_dsi.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index 573e6bec6d36..be42405a0a78 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -131,6 +131,10 @@
> #define VM_CMD_EN BIT(0)
> #define TS_VFP_EN BIT(5)
>
> +#define DSI_SHADOW_DEBUG 0x190U
> +#define FORCE_COMMIT BIT(0)
> +#define BYPASS_SHADOW BIT(1)
> +
> #define CONFIG (0xff << 0)
> #define SHORT_PACKET 0
> #define LONG_PACKET 2
> @@ -157,6 +161,7 @@ struct phy;
>
> struct mtk_dsi_driver_data {
> const u32 reg_cmdq_off;
> + bool has_shadow_ctl;
> };
>
> struct mtk_dsi {
> @@ -594,6 +599,11 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi)
> }
>
> mtk_dsi_enable(dsi);
> +
> + if (dsi->driver_data->has_shadow_ctl)
> + writel(FORCE_COMMIT | BYPASS_SHADOW,
> + dsi->regs + DSI_SHADOW_DEBUG);
> +
> mtk_dsi_reset_engine(dsi);
> mtk_dsi_phy_timconfig(dsi);
>
Hi, Jitao:
On Tue, 2019-04-16 at 14:05 +0800, Jitao Shi wrote:
> Our new DSI chip has frame size control.
> So add the driver data to control for different chips.
>
Reviewed-by: CK Hu <[email protected]>
> Signed-off-by: Jitao Shi <[email protected]>
> ---
> drivers/gpu/drm/mediatek/mtk_dsi.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index be42405a0a78..458a700ce74c 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -78,6 +78,7 @@
> #define DSI_VBP_NL 0x24
> #define DSI_VFP_NL 0x28
> #define DSI_VACT_NL 0x2C
> +#define DSI_SIZE_CON 0x38
> #define DSI_HSA_WC 0x50
> #define DSI_HBP_WC 0x54
> #define DSI_HFP_WC 0x58
> @@ -162,6 +163,7 @@ struct phy;
> struct mtk_dsi_driver_data {
> const u32 reg_cmdq_off;
> bool has_shadow_ctl;
> + bool has_size_ctl;
> };
>
> struct mtk_dsi {
> @@ -430,6 +432,9 @@ static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi)
> 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(vm->vactive << 16 | vm->hactive, dsi->regs + DSI_SIZE_CON);
> +
> horizontal_sync_active_byte = (vm->hsync_len * dsi_tmp_buf_bpp - 10);
>
> if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE)
Hi, Jitao:
On Tue, 2019-04-16 at 14:05 +0800, Jitao Shi wrote:
> Add mt8183 dsi driver data. Enable size control and
> reg commit control.
>
Reviewed-by: CK Hu <[email protected]>
> Signed-off-by: Jitao Shi <[email protected]>
> ---
> drivers/gpu/drm/mediatek/mtk_dsi.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> index 458a700ce74c..f0b36ec38e4f 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> @@ -1104,11 +1104,19 @@ static const struct mtk_dsi_driver_data mt2701_dsi_driver_data = {
> .reg_cmdq_off = 0x180,
> };
>
> +static const struct mtk_dsi_driver_data mt8183_dsi_driver_data = {
> + .reg_cmdq_off = 0x200,
> + .has_shadow_ctl = true,
> + .has_size_ctl = true,
> +};
> +
> static const struct of_device_id mtk_dsi_of_match[] = {
> { .compatible = "mediatek,mt2701-dsi",
> .data = &mt2701_dsi_driver_data },
> { .compatible = "mediatek,mt8173-dsi",
> .data = &mt8173_dsi_driver_data },
> + { .compatible = "mediatek,mt8183-dsi",
> + .data = &mt8183_dsi_driver_data },
> { },
> };
>
On Sun, 2019-05-19 at 17:36 +0800, Jitao Shi wrote:
> On Tue, 2019-05-07 at 17:52 +0800, CK Hu wrote:
> > Hi, Jitao:
> >
> > On Tue, 2019-04-16 at 14:04 +0800, Jitao Shi wrote:
> > > DSI panel driver need attach function which is inculde in
> > > mipi_dsi_host_ops.
> > >
> > > If mipi_dsi_host_register is not in probe, dsi panel will
> > > probe fail or more delay.
> >
> > I think this patch just prevent delay, not to prevent dsi panel probe
> > fail. In [1], you mention mipi_dsi_attach() is called in
> > panel_simple_dsi_probe(), but panel_simple_dsi_probe() is trigger by
> > mipi_dsi_host_register(), so the probe would success.
> >
> > [1]
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/panel/panel-simple.c?h=v5.0-rc6#n2987
> >
> >
>
> Yes, this just prevent delay.
>
> > >
> > > So move the mipi_dsi_host_register to probe from bind.
> > >
> > > Signed-off-by: Jitao Shi <[email protected]>
> > > ---
> > > drivers/gpu/drm/mediatek/mtk_dsi.c | 50 ++++++++++++++++++------------
> > > 1 file changed, 30 insertions(+), 20 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > > index b00eb2d2e086..6c4ac37f983d 100644
> > > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> > > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > > @@ -1045,12 +1045,6 @@ static int mtk_dsi_bind(struct device *dev, struct device *master, void *data)
> > > return ret;
> > > }
> > >
> > > - ret = mipi_dsi_host_register(&dsi->host);
> > > - if (ret < 0) {
> > > - dev_err(dev, "failed to register DSI host: %d\n", ret);
> > > - goto err_ddp_comp_unregister;
> > > - }
> > > -
> > > ret = mtk_dsi_create_conn_enc(drm, dsi);
> > > if (ret) {
> > > DRM_ERROR("Encoder create failed with %d\n", ret);
> > > @@ -1060,8 +1054,6 @@ static int mtk_dsi_bind(struct device *dev, struct device *master, void *data)
> > > return 0;
> > >
> > > err_unregister:
> > > - mipi_dsi_host_unregister(&dsi->host);
> > > -err_ddp_comp_unregister:
> > > mtk_ddp_comp_unregister(drm, &dsi->ddp_comp);
> > > return ret;
> > > }
> > > @@ -1097,31 +1089,37 @@ static int mtk_dsi_probe(struct platform_device *pdev)
> > >
> > > dsi->host.ops = &mtk_dsi_ops;
> > > dsi->host.dev = dev;
> > > + dsi->dev = dev;
> >
> > Why do this?
> >
> > Regards,
> > CK
> >
>
> There are some error message require this poweron().
So this should not be in this patch. This patch is related to the timing
of mipi_dsi_host_register().
Regards,
CK
>
> > > + ret = mipi_dsi_host_register(&dsi->host);
> > > + if (ret < 0) {
> > > + dev_err(dev, "failed to register DSI host: %d\n", ret);
> > > + return ret;
> > > + }
> > >
> > > ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0,
> > > &dsi->panel, &dsi->bridge);
> > > if (ret)
> > > - return ret;
> > > + goto err_unregister_host;
> > >
> > > dsi->engine_clk = devm_clk_get(dev, "engine");
> > > if (IS_ERR(dsi->engine_clk)) {
> > > ret = PTR_ERR(dsi->engine_clk);
> > > dev_err(dev, "Failed to get engine clock: %d\n", ret);
> > > - return ret;
> > > + goto err_unregister_host;
> > > }
> > >
> > > dsi->digital_clk = devm_clk_get(dev, "digital");
> > > if (IS_ERR(dsi->digital_clk)) {
> > > ret = PTR_ERR(dsi->digital_clk);
> > > dev_err(dev, "Failed to get digital clock: %d\n", ret);
> > > - return ret;
> > > + goto err_unregister_host;
> > > }
> > >
> > > dsi->hs_clk = devm_clk_get(dev, "hs");
> > > if (IS_ERR(dsi->hs_clk)) {
> > > ret = PTR_ERR(dsi->hs_clk);
> > > dev_err(dev, "Failed to get hs clock: %d\n", ret);
> > > - return ret;
> > > + goto err_unregister_host;
> > > }
> > >
> > > regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >
> >
> >
>
>
Hi, Jitao:
On Sun, 2019-05-19 at 17:33 +0800, Jitao Shi wrote:
> On Wed, 2019-05-08 at 10:39 +0800, CK Hu wrote:
> > On Tue, 2019-04-16 at 14:04 +0800, Jitao Shi wrote:
> > > Config the different CMDQ reg address in driver data.
> > >
> > For MT8173, you change reg_cmd_off from 0x180 to 0x200, so this patch is
> > a bug fix. You should add a 'Fixes' tag.
> >
> > > Signed-off-by: Jitao Shi <[email protected]>
> > > ---
> > > drivers/gpu/drm/mediatek/mtk_dsi.c | 39 +++++++++++++++++++++++-------
> > > 1 file changed, 30 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > > index 6c4ac37f983d..573e6bec6d36 100644
> > > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> > > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > > @@ -131,7 +131,6 @@
> > > #define VM_CMD_EN BIT(0)
> > > #define TS_VFP_EN BIT(5)
> > >
> > > -#define DSI_CMDQ0 0x180
> > > #define CONFIG (0xff << 0)
> > > #define SHORT_PACKET 0
> > > #define LONG_PACKET 2
> > > @@ -156,6 +155,10 @@
> > >
> > > struct phy;
> > >
> > > +struct mtk_dsi_driver_data {
> > > + const u32 reg_cmdq_off;
> > > +};
> > > +
> > > struct mtk_dsi {
> > > struct mtk_ddp_comp ddp_comp;
> > > struct device *dev;
> > > @@ -182,6 +185,7 @@ struct mtk_dsi {
> > > bool enabled;
> > > u32 irq_data;
> > > wait_queue_head_t irq_wait_queue;
> > > + struct mtk_dsi_driver_data *driver_data;
> > > };
> > >
> > > static inline struct mtk_dsi *encoder_to_dsi(struct drm_encoder *e)
> > > @@ -934,6 +938,7 @@ static void mtk_dsi_cmdq(struct mtk_dsi *dsi, const struct mipi_dsi_msg *msg)
> > > const char *tx_buf = msg->tx_buf;
> > > u8 config, cmdq_size, cmdq_off, type = msg->type;
> > > u32 reg_val, cmdq_mask, i;
> > > + u32 reg_cmdq_off = dsi->driver_data->reg_cmdq_off;
> > >
> > > if (MTK_DSI_HOST_IS_READ(type))
> > > config = BTA;
> > > @@ -953,9 +958,11 @@ static void mtk_dsi_cmdq(struct mtk_dsi *dsi, const struct mipi_dsi_msg *msg)
> > > }
> > >
> > > for (i = 0; i < msg->tx_len; i++)
> > > - writeb(tx_buf[i], dsi->regs + DSI_CMDQ0 + cmdq_off + i);
> > > + mtk_dsi_mask(dsi, (reg_cmdq_off + cmdq_off + i) & (~0x3U),
> > > + (0xffUL << (((i + cmdq_off) & 3U) * 8U)),
> > > + tx_buf[i] << (((i + cmdq_off) & 3U) * 8U));
> >
> > You say you would follow Nicolas' suggestion here.
> >
>
> If i replace mtk_dsi_mask with writeb, i can't get right value from
> registers. I don't know why this.
I remember that Shaoming has also meet some error about writeb(), but he
finally fix this bug. You may get some hint from him. If we can not use
writeb(), this modification should be two patches: one is changing
DSI_CMDQ0 to reg_cmdq_off, another one is changing writeb() to
mtk_dsi_mask().
Regards,
CK
>
> > >
> > > - mtk_dsi_mask(dsi, DSI_CMDQ0, cmdq_mask, reg_val);
> > > + mtk_dsi_mask(dsi, reg_cmdq_off, cmdq_mask, reg_val);
> > > mtk_dsi_mask(dsi, DSI_CMDQ_SIZE, CMDQ_SIZE, cmdq_size);
> > > }
> > >
> > > @@ -1074,10 +1081,27 @@ static const struct component_ops mtk_dsi_component_ops = {
> > > .unbind = mtk_dsi_unbind,
> > > };
> > >
> > > +static const struct mtk_dsi_driver_data mt8173_dsi_driver_data = {
> > > + .reg_cmdq_off = 0x200,
> > > +};
> > > +
> > > +static const struct mtk_dsi_driver_data mt2701_dsi_driver_data = {
> > > + .reg_cmdq_off = 0x180,
> > > +};
> > > +
> > > +static const struct of_device_id mtk_dsi_of_match[] = {
> > > + { .compatible = "mediatek,mt2701-dsi",
> > > + .data = &mt2701_dsi_driver_data },
> > > + { .compatible = "mediatek,mt8173-dsi",
> > > + .data = &mt8173_dsi_driver_data },
> > > + { },
> > > +};
> > > +
> > > static int mtk_dsi_probe(struct platform_device *pdev)
> > > {
> > > struct mtk_dsi *dsi;
> > > struct device *dev = &pdev->dev;
> > > + const struct of_device_id *of_id;
> > > struct resource *regs;
> > > int irq_num;
> > > int comp_id;
> > > @@ -1101,6 +1125,9 @@ static int mtk_dsi_probe(struct platform_device *pdev)
> > > if (ret)
> > > goto err_unregister_host;
> > >
> > > + of_id = of_match_device(mtk_dsi_of_match, &pdev->dev);
> > > + dsi->driver_data = of_id->data;
> >
> > Maybe use of_device_get_match_data() is a more simple way. You could
> > refer to [1].
> >
> > [1]
> > https://elixir.bootlin.com/linux/v5.1/source/drivers/gpu/drm/mediatek/mtk_disp_ovl.c#L300
> >
> > Regards,
> > CK
> >
>
> I'll fix it next version.
>
> > > +
> > > dsi->engine_clk = devm_clk_get(dev, "engine");
> > > if (IS_ERR(dsi->engine_clk)) {
> > > ret = PTR_ERR(dsi->engine_clk);
> > > @@ -1193,12 +1220,6 @@ static int mtk_dsi_remove(struct platform_device *pdev)
> > > return 0;
> > > }
> > >
> > > -static const struct of_device_id mtk_dsi_of_match[] = {
> > > - { .compatible = "mediatek,mt2701-dsi" },
> > > - { .compatible = "mediatek,mt8173-dsi" },
> > > - { },
> > > -};
> > > -
> > > struct platform_driver mtk_dsi_driver = {
> > > .probe = mtk_dsi_probe,
> > > .remove = mtk_dsi_remove,
> >
> >
>
>