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 jitao.shi@mediatek.com --- drivers/gpu/drm/mediatek/mtk_dsi.c | 49 ++++++++++++++++++------------ 1 file changed, 30 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index 27b507eb4a99..93fa255b4aad 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 -EPROBE_DEFER; + }
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; + ret = -EPROBE_DEFER; + goto err_unregister_host; }
irq_set_status_flags(irq_num, IRQ_TYPE_LEVEL_LOW); @@ -1163,14 +1163,25 @@ 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; + ret = -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) { + ret = -EPROBE_DEFER; + 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)
Config the different CMDQ reg address in driver data.
Signed-off-by: Jitao Shi jitao.shi@mediatek.com --- drivers/gpu/drm/mediatek/mtk_dsi.c | 39 ++++++++++++++++++++++++------ 1 file changed, 31 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index 93fa255b4aad..80db02a25cb0 100644 --- a/drivers/gpu/drm/mediatek/mtk_dsi.c +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c @@ -156,6 +156,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 +186,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 +939,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 +959,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 +1082,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 +1126,10 @@ 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 = (struct mtk_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); @@ -1194,12 +1223,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,
On Thu, Feb 14, 2019 at 12:42 PM Jitao Shi jitao.shi@mediatek.com wrote:
Config the different CMDQ reg address in driver data.
Signed-off-by: Jitao Shi jitao.shi@mediatek.com
drivers/gpu/drm/mediatek/mtk_dsi.c | 39 ++++++++++++++++++++++++------ 1 file changed, 31 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index 93fa255b4aad..80db02a25cb0 100644 --- a/drivers/gpu/drm/mediatek/mtk_dsi.c +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c @@ -156,6 +156,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 +186,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 +939,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 +959,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));
I found the writeb call _much_ clearer ... Either switch back to that, or create a new mtk_disk_mask_byte function maybe?
mtk_dsi_mask(dsi, DSI_CMDQ0, cmdq_mask, reg_val);
mtk_dsi_mask(dsi, reg_cmdq_off, cmdq_mask, reg_val);
You're removing DSI_CMDQ0 usage in this patch, so remove the #define in this patch too (instead of doing that in 3/3).
mtk_dsi_mask(dsi, DSI_CMDQ_SIZE, CMDQ_SIZE, cmdq_size);
}
@@ -1074,10 +1082,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 +1126,10 @@ 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 = (struct mtk_dsi_driver_data *)
of_id->data;
This fits in 80 chars. Also, of_id->data is a void*, so no cast needed.
dsi->engine_clk = devm_clk_get(dev, "engine"); if (IS_ERR(dsi->engine_clk)) { ret = PTR_ERR(dsi->engine_clk);
@@ -1194,12 +1223,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" },
{ },
-};
Any reason you moved this up?
struct platform_driver mtk_dsi_driver = { .probe = mtk_dsi_probe, .remove = mtk_dsi_remove, -- 2.20.1
On Thu, 2019-02-14 at 13:48 +0800, Nicolas Boichat wrote:
On Thu, Feb 14, 2019 at 12:42 PM Jitao Shi jitao.shi@mediatek.com wrote:
Config the different CMDQ reg address in driver data.
Signed-off-by: Jitao Shi jitao.shi@mediatek.com
drivers/gpu/drm/mediatek/mtk_dsi.c | 39 ++++++++++++++++++++++++------ 1 file changed, 31 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index 93fa255b4aad..80db02a25cb0 100644 --- a/drivers/gpu/drm/mediatek/mtk_dsi.c +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c @@ -156,6 +156,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 +186,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 +939,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 +959,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));
I found the writeb call _much_ clearer ... Either switch back to that, or create a new mtk_disk_mask_byte function maybe?
I'll fix it next version.
mtk_dsi_mask(dsi, DSI_CMDQ0, cmdq_mask, reg_val);
mtk_dsi_mask(dsi, reg_cmdq_off, cmdq_mask, reg_val);
You're removing DSI_CMDQ0 usage in this patch, so remove the #define in this patch too (instead of doing that in 3/3).
I'll fix it next version.
mtk_dsi_mask(dsi, DSI_CMDQ_SIZE, CMDQ_SIZE, cmdq_size);
}
@@ -1074,10 +1082,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 +1126,10 @@ 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 = (struct mtk_dsi_driver_data *)
of_id->data;
This fits in 80 chars. Also, of_id->data is a void*, so no cast needed.
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);
@@ -1194,12 +1223,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" },
{ },
-};
Any reason you moved this up?
of_match_device is called by mtk_dsi_probe.
Best regards Jitao
struct platform_driver mtk_dsi_driver = { .probe = mtk_dsi_probe, .remove = mtk_dsi_remove, -- 2.20.1
MT8183 dsi has two changes with mt8173. 1. Add the register double buffer control, but we no need it, So make it default off. 2. Add picture size control.
Signed-off-by: Jitao Shi jitao.shi@mediatek.com --- drivers/gpu/drm/mediatek/mtk_dsi.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index 80db02a25cb0..20cb53f05d42 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 @@ -131,7 +132,10 @@ #define VM_CMD_EN BIT(0) #define TS_VFP_EN BIT(5)
-#define DSI_CMDQ0 0x180 +#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 @@ -158,6 +162,7 @@ struct phy;
struct mtk_dsi_driver_data { const u32 reg_cmdq_off; + bool has_size_ctl; };
struct mtk_dsi { @@ -426,6 +431,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) @@ -595,6 +603,9 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi) }
mtk_dsi_enable(dsi); + + /* DSI no need this double buffer, disable it when writing register */ + writel(FORCE_COMMIT | BYPASS_SHADOW, dsi->regs + DSI_SHADOW_DEBUG); mtk_dsi_reset_engine(dsi); mtk_dsi_phy_timconfig(dsi);
@@ -1090,11 +1101,18 @@ 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_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 Thu, Feb 14, 2019 at 12:43 PM Jitao Shi jitao.shi@mediatek.com wrote:
MT8183 dsi has two changes with mt8173.
- Add the register double buffer control, but we no need it, So make it default off.
Can you describe a little bit more what this is about? That's shadow registers, right?
- Add picture size control.
Signed-off-by: Jitao Shi jitao.shi@mediatek.com
drivers/gpu/drm/mediatek/mtk_dsi.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index 80db02a25cb0..20cb53f05d42 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 @@ -131,7 +132,10 @@ #define VM_CMD_EN BIT(0) #define TS_VFP_EN BIT(5)
-#define DSI_CMDQ0 0x180
As I said earlier, move this to 2/3.
+#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 @@ -158,6 +162,7 @@ struct phy;
struct mtk_dsi_driver_data { const u32 reg_cmdq_off;
bool has_size_ctl;
};
struct mtk_dsi { @@ -426,6 +431,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)
@@ -595,6 +603,9 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi) }
mtk_dsi_enable(dsi);
/* DSI no need this double buffer, disable it when writing register */
"DSI does not need double buffering, disable it when writing register"
writel(FORCE_COMMIT | BYPASS_SHADOW, dsi->regs + DSI_SHADOW_DEBUG);
So you do this on all MT* variants, is that ok?
mtk_dsi_reset_engine(dsi); mtk_dsi_phy_timconfig(dsi);
@@ -1090,11 +1101,18 @@ 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_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.20.1
On Thu, 2019-02-14 at 13:54 +0800, Nicolas Boichat wrote:
On Thu, Feb 14, 2019 at 12:43 PM Jitao Shi jitao.shi@mediatek.com wrote:
MT8183 dsi has two changes with mt8173.
- Add the register double buffer control, but we no need it, So make it default off.
Can you describe a little bit more what this is about? That's shadow registers, right?
Yes, it is shadow registers.
Jitao
- Add picture size control.
Signed-off-by: Jitao Shi jitao.shi@mediatek.com
drivers/gpu/drm/mediatek/mtk_dsi.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index 80db02a25cb0..20cb53f05d42 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 @@ -131,7 +132,10 @@ #define VM_CMD_EN BIT(0) #define TS_VFP_EN BIT(5)
-#define DSI_CMDQ0 0x180
As I said earlier, move this to 2/3.
Thank for you review. I'll move it to 2/3 next version.
Best Regards Jitao
+#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 @@ -158,6 +162,7 @@ struct phy;
struct mtk_dsi_driver_data { const u32 reg_cmdq_off;
bool has_size_ctl;
};
struct mtk_dsi { @@ -426,6 +431,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)
@@ -595,6 +603,9 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi) }
mtk_dsi_enable(dsi);
/* DSI no need this double buffer, disable it when writing register */
"DSI does not need double buffering, disable it when writing register"
I'll fix it next version.
writel(FORCE_COMMIT | BYPASS_SHADOW, dsi->regs + DSI_SHADOW_DEBUG);
So you do this on all MT* variants, is that ok?
mtk_dsi_reset_engine(dsi); mtk_dsi_phy_timconfig(dsi);
@@ -1090,11 +1101,18 @@ 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_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.20.1
On Sun, Feb 17, 2019 at 10:48 PM Jitao Shi jitao.shi@mediatek.com wrote:
On Thu, 2019-02-14 at 13:54 +0800, Nicolas Boichat wrote:
On Thu, Feb 14, 2019 at 12:43 PM Jitao Shi jitao.shi@mediatek.com wrote:
MT8183 dsi has two changes with mt8173.
- Add the register double buffer control, but we no need it, So make it default off.
Can you describe a little bit more what this is about? That's shadow registers, right?
Yes, it is shadow registers.
Jitao
- Add picture size control.
Signed-off-by: Jitao Shi jitao.shi@mediatek.com
drivers/gpu/drm/mediatek/mtk_dsi.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index 80db02a25cb0..20cb53f05d42 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 @@ -131,7 +132,10 @@ #define VM_CMD_EN BIT(0) #define TS_VFP_EN BIT(5)
-#define DSI_CMDQ0 0x180
As I said earlier, move this to 2/3.
Thank for you review. I'll move it to 2/3 next version.
Best Regards Jitao
+#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 @@ -158,6 +162,7 @@ struct phy;
struct mtk_dsi_driver_data { const u32 reg_cmdq_off;
bool has_size_ctl;
};
struct mtk_dsi { @@ -426,6 +431,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)
@@ -595,6 +603,9 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi) }
mtk_dsi_enable(dsi);
/* DSI no need this double buffer, disable it when writing register */
"DSI does not need double buffering, disable it when writing register"
I'll fix it next version.
In that case, please say something about "shadow registers". (maybe it's just me, but usually double-buffering is associated with framebuffer data, not register settings)
writel(FORCE_COMMIT | BYPASS_SHADOW, dsi->regs + DSI_SHADOW_DEBUG);
So you do this on all MT* variants, is that ok?
mtk_dsi_reset_engine(dsi); mtk_dsi_phy_timconfig(dsi);
@@ -1090,11 +1101,18 @@ 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_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.20.1
On 14/02/2019 05:42, Jitao Shi wrote:
MT8183 dsi has two changes with mt8173.
- Add the register double buffer control, but we no need it, So make it default off.
- Add picture size control.
Signed-off-by: Jitao Shi jitao.shi@mediatek.com
drivers/gpu/drm/mediatek/mtk_dsi.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index 80db02a25cb0..20cb53f05d42 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 @@ -131,7 +132,10 @@ #define VM_CMD_EN BIT(0) #define TS_VFP_EN BIT(5)
-#define DSI_CMDQ0 0x180 +#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 @@ -158,6 +162,7 @@ struct phy;
struct mtk_dsi_driver_data { const u32 reg_cmdq_off;
- bool has_size_ctl;
};
struct mtk_dsi { @@ -426,6 +431,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)
@@ -595,6 +603,9 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi) }
mtk_dsi_enable(dsi);
- /* DSI no need this double buffer, disable it when writing register */
- writel(FORCE_COMMIT | BYPASS_SHADOW, dsi->regs + DSI_SHADOW_DEBUG);
Is this a mt8183 thing? Did you assure that this does not introduce regressions on other SoCs, or does it fix any?
I think this should be a independent patch. If it fixes an actual issue, then please provide a fixes tag in that patch.
Thanks, Matthias
mtk_dsi_reset_engine(dsi); mtk_dsi_phy_timconfig(dsi);
@@ -1090,11 +1101,18 @@ 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_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 Thu, 2019-02-14 at 10:54 +0100, Matthias Brugger wrote:
On 14/02/2019 05:42, Jitao Shi wrote:
MT8183 dsi has two changes with mt8173.
- Add the register double buffer control, but we no need it, So make it default off.
- Add picture size control.
Signed-off-by: Jitao Shi jitao.shi@mediatek.com
drivers/gpu/drm/mediatek/mtk_dsi.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index 80db02a25cb0..20cb53f05d42 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 @@ -131,7 +132,10 @@ #define VM_CMD_EN BIT(0) #define TS_VFP_EN BIT(5)
-#define DSI_CMDQ0 0x180 +#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 @@ -158,6 +162,7 @@ struct phy;
struct mtk_dsi_driver_data { const u32 reg_cmdq_off;
- bool has_size_ctl;
};
struct mtk_dsi { @@ -426,6 +431,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)
@@ -595,6 +603,9 @@ static int mtk_dsi_poweron(struct mtk_dsi *dsi) }
mtk_dsi_enable(dsi);
- /* DSI no need this double buffer, disable it when writing register */
- writel(FORCE_COMMIT | BYPASS_SHADOW, dsi->regs + DSI_SHADOW_DEBUG);
Is this a mt8183 thing? Did you assure that this does not introduce regressions on other SoCs, or does it fix any?
I think this should be a independent patch. If it fixes an actual issue, then please provide a fixes tag in that patch.
Thanks, Matthias
Yes, this is for mt8183. But this reg is reverse on other mtk soc. It is unsuitable. And i'll put it in mt8183 driver data next version.
Best Regards Jitao
mtk_dsi_reset_engine(dsi); mtk_dsi_phy_timconfig(dsi);
@@ -1090,11 +1101,18 @@ 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_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 },
};
Just some comments on the error path, I'm not sure about the change itself.
On Thu, Feb 14, 2019 at 12:42 PM Jitao Shi jitao.shi@mediatek.com 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.
So move the mipi_dsi_host_register to probe from bind.
Signed-off-by: Jitao Shi jitao.shi@mediatek.com
drivers/gpu/drm/mediatek/mtk_dsi.c | 49 ++++++++++++++++++------------ 1 file changed, 30 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index 27b507eb4a99..93fa255b4aad 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 -EPROBE_DEFER;
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;
ret = -EPROBE_DEFER;
I think you need to do ret = irq_num here, too (and yes, it was probably wrong before?)
goto err_unregister_host; } irq_set_status_flags(irq_num, IRQ_TYPE_LEVEL_LOW);
@@ -1163,14 +1163,25 @@ 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;
ret = -EPROBE_DEFER;
Ditto, I'd not reassign ret.
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) {
ret = -EPROBE_DEFER;
Ditto (this one _was_ right before).
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.20.1
On Thu, 2019-02-14 at 14:02 +0800, Nicolas Boichat wrote:
Just some comments on the error path, I'm not sure about the change itself.
On Thu, Feb 14, 2019 at 12:42 PM Jitao Shi jitao.shi@mediatek.com 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.
So move the mipi_dsi_host_register to probe from bind.
Signed-off-by: Jitao Shi jitao.shi@mediatek.com
drivers/gpu/drm/mediatek/mtk_dsi.c | 49 ++++++++++++++++++------------ 1 file changed, 30 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index 27b507eb4a99..93fa255b4aad 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 -EPROBE_DEFER;
return ret
Ok, I'll fix it next version.
} 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;
ret = -EPROBE_DEFER;
I think you need to do ret = irq_num here, too (and yes, it was probably wrong before?)
Yes, the "ret = irq_num" is right. We regard all the return error case as "-EPROBE_DEFER".
Actually the error case (ex. EXIO) should return error case.
I'll fix it next version.
goto err_unregister_host; } irq_set_status_flags(irq_num, IRQ_TYPE_LEVEL_LOW);
@@ -1163,14 +1163,25 @@ 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;
ret = -EPROBE_DEFER;
Ditto, I'd not reassign ret.
I'll fix it next version.
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) {
ret = -EPROBE_DEFER;
Ditto (this one _was_ right before).
I'll fix it next version.
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.20.1
On Thu, Feb 14, 2019 at 12:42:41PM +0800, Jitao Shi wrote:
DSI panel driver need attach function which is inculde in mipi_dsi_host_ops.
Which function is required from dsi_host?
Sean
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 jitao.shi@mediatek.com
drivers/gpu/drm/mediatek/mtk_dsi.c | 49 ++++++++++++++++++------------ 1 file changed, 30 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index 27b507eb4a99..93fa255b4aad 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 -EPROBE_DEFER;
}
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;
ret = -EPROBE_DEFER;
goto err_unregister_host;
}
irq_set_status_flags(irq_num, IRQ_TYPE_LEVEL_LOW);
@@ -1163,14 +1163,25 @@ 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;
ret = -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) {
ret = -EPROBE_DEFER;
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.20.1
On Thu, 2019-02-14 at 15:48 -0500, Sean Paul wrote:
On Thu, Feb 14, 2019 at 12:42:41PM +0800, Jitao Shi wrote:
DSI panel driver need attach function which is inculde in mipi_dsi_host_ops.
Which function is required from dsi_host?
Sean
mipi_dsi_attach request the mipi_dsi_host_register ready.
for example.
mipi_dsi_attach is called by panel_simple_dsi_probe.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/driv...
jitao
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 jitao.shi@mediatek.com
drivers/gpu/drm/mediatek/mtk_dsi.c | 49 ++++++++++++++++++------------ 1 file changed, 30 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index 27b507eb4a99..93fa255b4aad 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 -EPROBE_DEFER;
}
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;
ret = -EPROBE_DEFER;
goto err_unregister_host;
}
irq_set_status_flags(irq_num, IRQ_TYPE_LEVEL_LOW);
@@ -1163,14 +1163,25 @@ 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;
ret = -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) {
ret = -EPROBE_DEFER;
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.20.1
dri-devel@lists.freedesktop.org