Hi,
Here's a series adding support for the DSI0 controller in the BCM2835 and the DSI1 controller found in the BCM2711.
Let me know what you think, Maxime
Dave Stevenson (5): drm/vc4: dsi: Correct DSI register definition drm/vc4: dsi: Add support for DSI0 dt-bindings: Add compatible for BCM2711 DSI1 drm/vc4: dsi: Add configuration for BCM2711 DSI1 ARM: dts: bcm2711: Use compatible string for BCM2711 DSI1
Maxime Ripard (3): drm/vc4: drv: Remove the DSI pointer in vc4_drv drm/vc4: dsi: Use snprintf for the PHY clocks instead of an array drm/vc4: dsi: Introduce a variant structure
.../bindings/display/brcm,bcm2835-dsi0.yaml | 1 + arch/arm/boot/dts/bcm2711.dtsi | 1 + drivers/gpu/drm/vc4/vc4_drv.h | 1 - drivers/gpu/drm/vc4/vc4_dsi.c | 111 ++++++++++-------- 4 files changed, 67 insertions(+), 47 deletions(-)
That pointer isn't used anywhere, so there's no point in keeping it.
Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_drv.h | 1 - drivers/gpu/drm/vc4/vc4_dsi.c | 9 --------- 2 files changed, 10 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h index c5f2944d5bc6..ee95b4327796 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.h +++ b/drivers/gpu/drm/vc4/vc4_drv.h @@ -77,7 +77,6 @@ struct vc4_dev { struct vc4_hvs *hvs; struct vc4_v3d *v3d; struct vc4_dpi *dpi; - struct vc4_dsi *dsi1; struct vc4_vec *vec; struct vc4_txp *txp;
diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c index 19aab4e7e209..b1d8765795f1 100644 --- a/drivers/gpu/drm/vc4/vc4_dsi.c +++ b/drivers/gpu/drm/vc4/vc4_dsi.c @@ -1459,7 +1459,6 @@ static int vc4_dsi_bind(struct device *dev, struct device *master, void *data) { struct platform_device *pdev = to_platform_device(dev); struct drm_device *drm = dev_get_drvdata(master); - struct vc4_dev *vc4 = to_vc4_dev(drm); struct vc4_dsi *dsi = dev_get_drvdata(dev); struct vc4_dsi_encoder *vc4_dsi_encoder; struct drm_panel *panel; @@ -1612,9 +1611,6 @@ static int vc4_dsi_bind(struct device *dev, struct device *master, void *data) if (ret) return ret;
- if (dsi->port == 1) - vc4->dsi1 = dsi; - drm_simple_encoder_init(drm, dsi->encoder, DRM_MODE_ENCODER_DSI); drm_encoder_helper_add(dsi->encoder, &vc4_dsi_encoder_helper_funcs);
@@ -1643,8 +1639,6 @@ static int vc4_dsi_bind(struct device *dev, struct device *master, void *data) static void vc4_dsi_unbind(struct device *dev, struct device *master, void *data) { - struct drm_device *drm = dev_get_drvdata(master); - struct vc4_dev *vc4 = to_vc4_dev(drm); struct vc4_dsi *dsi = dev_get_drvdata(dev);
if (dsi->bridge) @@ -1656,9 +1650,6 @@ static void vc4_dsi_unbind(struct device *dev, struct device *master, */ list_splice_init(&dsi->bridge_chain, &dsi->encoder->bridge_chain); drm_encoder_cleanup(dsi->encoder); - - if (dsi->port == 1) - vc4->dsi1 = NULL; }
static const struct component_ops vc4_dsi_ops = {
On Thu, 2020-12-03 at 14:25 +0100, Maxime Ripard wrote:
That pointer isn't used anywhere, so there's no point in keeping it.
Signed-off-by: Maxime Ripard maxime@cerno.tech
Reviewed-by: Nicolas Saenz Julienne nsaenzjulienne@suse.de
drivers/gpu/drm/vc4/vc4_drv.h | 1 - drivers/gpu/drm/vc4/vc4_dsi.c | 9 --------- 2 files changed, 10 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h index c5f2944d5bc6..ee95b4327796 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.h +++ b/drivers/gpu/drm/vc4/vc4_drv.h @@ -77,7 +77,6 @@ struct vc4_dev { struct vc4_hvs *hvs; struct vc4_v3d *v3d; struct vc4_dpi *dpi;
- struct vc4_dsi *dsi1;
struct vc4_vec *vec; struct vc4_txp *txp;
diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c index 19aab4e7e209..b1d8765795f1 100644 --- a/drivers/gpu/drm/vc4/vc4_dsi.c +++ b/drivers/gpu/drm/vc4/vc4_dsi.c @@ -1459,7 +1459,6 @@ static int vc4_dsi_bind(struct device *dev, struct device *master, void *data) { struct platform_device *pdev = to_platform_device(dev); struct drm_device *drm = dev_get_drvdata(master);
- struct vc4_dev *vc4 = to_vc4_dev(drm);
struct vc4_dsi *dsi = dev_get_drvdata(dev); struct vc4_dsi_encoder *vc4_dsi_encoder; struct drm_panel *panel; @@ -1612,9 +1611,6 @@ static int vc4_dsi_bind(struct device *dev, struct device *master, void *data) if (ret) return ret;
- if (dsi->port == 1)
vc4->dsi1 = dsi;drm_simple_encoder_init(drm, dsi->encoder, DRM_MODE_ENCODER_DSI); drm_encoder_helper_add(dsi->encoder, &vc4_dsi_encoder_helper_funcs);
@@ -1643,8 +1639,6 @@ static int vc4_dsi_bind(struct device *dev, struct device *master, void *data) static void vc4_dsi_unbind(struct device *dev, struct device *master, void *data) {
- struct drm_device *drm = dev_get_drvdata(master);
- struct vc4_dev *vc4 = to_vc4_dev(drm);
struct vc4_dsi *dsi = dev_get_drvdata(dev);
if (dsi->bridge) @@ -1656,9 +1650,6 @@ static void vc4_dsi_unbind(struct device *dev, struct device *master, */ list_splice_init(&dsi->bridge_chain, &dsi->encoder->bridge_chain); drm_encoder_cleanup(dsi->encoder);
- if (dsi->port == 1)
vc4->dsi1 = NULL;}
static const struct component_ops vc4_dsi_ops = {
From: Dave Stevenson dave.stevenson@raspberrypi.com
The DSI1_PHY_AFEC0_PD_DLANE1 and DSI1_PHY_AFEC0_PD_DLANE3 register definitions were swapped, so trying to use more than a single data lane failed as lane 1 would get powered down. (In theory a 4 lane device would work as all lanes would remain powered).
Correct the definitions.
Signed-off-by: Dave Stevenson dave.stevenson@raspberrypi.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_dsi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c index b1d8765795f1..bb316e6cc12b 100644 --- a/drivers/gpu/drm/vc4/vc4_dsi.c +++ b/drivers/gpu/drm/vc4/vc4_dsi.c @@ -306,11 +306,11 @@ # define DSI0_PHY_AFEC0_RESET BIT(11) # define DSI1_PHY_AFEC0_PD_BG BIT(11) # define DSI0_PHY_AFEC0_PD BIT(10) -# define DSI1_PHY_AFEC0_PD_DLANE3 BIT(10) +# define DSI1_PHY_AFEC0_PD_DLANE1 BIT(10) # define DSI0_PHY_AFEC0_PD_BG BIT(9) # define DSI1_PHY_AFEC0_PD_DLANE2 BIT(9) # define DSI0_PHY_AFEC0_PD_DLANE1 BIT(8) -# define DSI1_PHY_AFEC0_PD_DLANE1 BIT(8) +# define DSI1_PHY_AFEC0_PD_DLANE3 BIT(8) # define DSI_PHY_AFEC0_PTATADJ_MASK VC4_MASK(7, 4) # define DSI_PHY_AFEC0_PTATADJ_SHIFT 4 # define DSI_PHY_AFEC0_CTATADJ_MASK VC4_MASK(3, 0)
Hi Maxime,
On 03.12.20 14:25, Maxime Ripard wrote:
From: Dave Stevenson dave.stevenson@raspberrypi.com
The DSI1_PHY_AFEC0_PD_DLANE1 and DSI1_PHY_AFEC0_PD_DLANE3 register definitions were swapped, so trying to use more than a single data lane failed as lane 1 would get powered down. (In theory a 4 lane device would work as all lanes would remain powered).
Correct the definitions.
Signed-off-by: Dave Stevenson dave.stevenson@raspberrypi.com Signed-off-by: Maxime Ripard maxime@cerno.tech
Wouldn't this deserve a "Fixes: ..." and "Cc: stable@vger.kernel.org" tag, as this bug is present in all stable releases since this driver was introduced? I think it would be really helpful to have it backported.
Thanks Frieder
drivers/gpu/drm/vc4/vc4_dsi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c index b1d8765795f1..bb316e6cc12b 100644 --- a/drivers/gpu/drm/vc4/vc4_dsi.c +++ b/drivers/gpu/drm/vc4/vc4_dsi.c @@ -306,11 +306,11 @@ # define DSI0_PHY_AFEC0_RESET BIT(11) # define DSI1_PHY_AFEC0_PD_BG BIT(11) # define DSI0_PHY_AFEC0_PD BIT(10) -# define DSI1_PHY_AFEC0_PD_DLANE3 BIT(10) +# define DSI1_PHY_AFEC0_PD_DLANE1 BIT(10) # define DSI0_PHY_AFEC0_PD_BG BIT(9) # define DSI1_PHY_AFEC0_PD_DLANE2 BIT(9) # define DSI0_PHY_AFEC0_PD_DLANE1 BIT(8) -# define DSI1_PHY_AFEC0_PD_DLANE1 BIT(8) +# define DSI1_PHY_AFEC0_PD_DLANE3 BIT(8) # define DSI_PHY_AFEC0_PTATADJ_MASK VC4_MASK(7, 4) # define DSI_PHY_AFEC0_PTATADJ_SHIFT 4 # define DSI_PHY_AFEC0_CTATADJ_MASK VC4_MASK(3, 0)
On Tue, 2020-12-08 at 09:34 +0100, Frieder Schrempf wrote:
Hi Maxime,
On 03.12.20 14:25, Maxime Ripard wrote:
From: Dave Stevenson dave.stevenson@raspberrypi.com
The DSI1_PHY_AFEC0_PD_DLANE1 and DSI1_PHY_AFEC0_PD_DLANE3 register definitions were swapped, so trying to use more than a single data lane failed as lane 1 would get powered down. (In theory a 4 lane device would work as all lanes would remain powered).
Correct the definitions.
Signed-off-by: Dave Stevenson dave.stevenson@raspberrypi.com Signed-off-by: Maxime Ripard maxime@cerno.tech
Wouldn't this deserve a "Fixes: ..." and "Cc: stable@vger.kernel.org" tag, as this bug is present in all stable releases since this driver was introduced? I think it would be really helpful to have it backported.
Agree, this would be nice:
Fixes: 4078f5757144 ("drm/vc4: Add DSI driver")
With that,
Reviewed-by: Nicolas Saenz Julienne nsaenzjulienne@suse.de
Regards, Nicolas
Hi,
On Tue, Dec 08, 2020 at 09:34:05AM +0100, Frieder Schrempf wrote:
On 03.12.20 14:25, Maxime Ripard wrote:
From: Dave Stevenson dave.stevenson@raspberrypi.com
The DSI1_PHY_AFEC0_PD_DLANE1 and DSI1_PHY_AFEC0_PD_DLANE3 register definitions were swapped, so trying to use more than a single data lane failed as lane 1 would get powered down. (In theory a 4 lane device would work as all lanes would remain powered).
Correct the definitions.
Signed-off-by: Dave Stevenson dave.stevenson@raspberrypi.com Signed-off-by: Maxime Ripard maxime@cerno.tech
Wouldn't this deserve a "Fixes: ..." and "Cc: stable@vger.kernel.org" tag, as this bug is present in all stable releases since this driver was introduced? I think it would be really helpful to have it backported.
Sorry I forgot it. The patch is applied however and drm-misc-next doesn't get rebased, so I can't add it now.
We can always send it for stable by hand though once it's in Linus' tree
Maxime
The DSI clocks setup function has been using an array to store the clock name of either the DSI0 or DSI1 blocks, using the port ID to choose the proper one.
Let's switch to an snprintf call to do the same thing and simplify the array a bit.
Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_dsi.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c index bb316e6cc12b..f704d959e65b 100644 --- a/drivers/gpu/drm/vc4/vc4_dsi.c +++ b/drivers/gpu/drm/vc4/vc4_dsi.c @@ -1398,12 +1398,12 @@ vc4_dsi_init_phy_clocks(struct vc4_dsi *dsi) struct device *dev = &dsi->pdev->dev; const char *parent_name = __clk_get_name(dsi->pll_phy_clock); static const struct { - const char *dsi0_name, *dsi1_name; + const char *name; int div; } phy_clocks[] = { - { "dsi0_byte", "dsi1_byte", 8 }, - { "dsi0_ddr2", "dsi1_ddr2", 4 }, - { "dsi0_ddr", "dsi1_ddr", 2 }, + { "byte", 8 }, + { "ddr2", 4 }, + { "ddr", 2 }, }; int i;
@@ -1419,8 +1419,12 @@ vc4_dsi_init_phy_clocks(struct vc4_dsi *dsi) for (i = 0; i < ARRAY_SIZE(phy_clocks); i++) { struct clk_fixed_factor *fix = &dsi->phy_clocks[i]; struct clk_init_data init; + char clk_name[16]; int ret;
+ snprintf(clk_name, sizeof(clk_name), + "dsi%u_%s", dsi->port, phy_clocks[i].name); + /* We just use core fixed factor clock ops for the PHY * clocks. The clocks are actually gated by the * PHY_AFEC0_DDRCLK_EN bits, which we should be @@ -1437,10 +1441,7 @@ vc4_dsi_init_phy_clocks(struct vc4_dsi *dsi) memset(&init, 0, sizeof(init)); init.parent_names = &parent_name; init.num_parents = 1; - if (dsi->port == 1) - init.name = phy_clocks[i].dsi1_name; - else - init.name = phy_clocks[i].dsi0_name; + init.name = clk_name; init.ops = &clk_fixed_factor_ops;
ret = devm_clk_hw_register(dev, &fix->hw);
Most of the differences between DSI0 and DSI1 are handled through the ID. However, the BCM2711 DSI is going to introduce one more variable to the mix and will break some expectations of the earlier, simpler, test.
Let's add a variant structure that will address most of the differences between those three controllers.
Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_dsi.c | 63 ++++++++++++++++++++--------------- 1 file changed, 37 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c index f704d959e65b..601020c10053 100644 --- a/drivers/gpu/drm/vc4/vc4_dsi.c +++ b/drivers/gpu/drm/vc4/vc4_dsi.c @@ -493,6 +493,18 @@ */ #define DSI1_ID 0x8c
+struct vc4_dsi_variant { + /* Whether we're on bcm2835's DSI0 or DSI1. */ + unsigned int port; + + bool broken_axi_workaround; + + const char *debugfs_name; + const struct debugfs_reg32 *regs; + size_t nregs; + +}; + /* General DSI hardware state. */ struct vc4_dsi { struct platform_device *pdev; @@ -509,8 +521,7 @@ struct vc4_dsi { u32 *reg_dma_mem; dma_addr_t reg_paddr;
- /* Whether we're on bcm2835's DSI0 or DSI1. */ - int port; + const struct vc4_dsi_variant *variant;
/* DSI channel for the panel we're connected to. */ u32 channel; @@ -586,10 +597,10 @@ dsi_dma_workaround_write(struct vc4_dsi *dsi, u32 offset, u32 val) #define DSI_READ(offset) readl(dsi->regs + (offset)) #define DSI_WRITE(offset, val) dsi_dma_workaround_write(dsi, offset, val) #define DSI_PORT_READ(offset) \ - DSI_READ(dsi->port ? DSI1_##offset : DSI0_##offset) + DSI_READ(dsi->variant->port ? DSI1_##offset : DSI0_##offset) #define DSI_PORT_WRITE(offset, val) \ - DSI_WRITE(dsi->port ? DSI1_##offset : DSI0_##offset, val) -#define DSI_PORT_BIT(bit) (dsi->port ? DSI1_##bit : DSI0_##bit) + DSI_WRITE(dsi->variant->port ? DSI1_##offset : DSI0_##offset, val) +#define DSI_PORT_BIT(bit) (dsi->variant->port ? DSI1_##bit : DSI0_##bit)
/* VC4 DSI encoder KMS struct */ struct vc4_dsi_encoder { @@ -837,7 +848,7 @@ static void vc4_dsi_encoder_enable(struct drm_encoder *encoder)
ret = pm_runtime_get_sync(dev); if (ret) { - DRM_ERROR("Failed to runtime PM enable on DSI%d\n", dsi->port); + DRM_ERROR("Failed to runtime PM enable on DSI%d\n", dsi->variant->port); return; }
@@ -871,7 +882,7 @@ static void vc4_dsi_encoder_enable(struct drm_encoder *encoder) DSI_PORT_WRITE(STAT, DSI_PORT_READ(STAT));
/* Set AFE CTR00/CTR1 to release powerdown of analog. */ - if (dsi->port == 0) { + if (dsi->variant->port == 0) { u32 afec0 = (VC4_SET_FIELD(7, DSI_PHY_AFEC0_PTATADJ) | VC4_SET_FIELD(7, DSI_PHY_AFEC0_CTATADJ));
@@ -1017,7 +1028,7 @@ static void vc4_dsi_encoder_enable(struct drm_encoder *encoder) DSI_PORT_BIT(PHYC_CLANE_ENABLE) | ((dsi->mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS) ? 0 : DSI_PORT_BIT(PHYC_HS_CLK_CONTINUOUS)) | - (dsi->port == 0 ? + (dsi->variant->port == 0 ? VC4_SET_FIELD(lpx - 1, DSI0_PHYC_ESC_CLK_LPDT) : VC4_SET_FIELD(lpx - 1, DSI1_PHYC_ESC_CLK_LPDT)));
@@ -1043,13 +1054,13 @@ static void vc4_dsi_encoder_enable(struct drm_encoder *encoder) DSI_DISP1_ENABLE);
/* Ungate the block. */ - if (dsi->port == 0) + if (dsi->variant->port == 0) DSI_PORT_WRITE(CTRL, DSI_PORT_READ(CTRL) | DSI0_CTRL_CTRL0); else DSI_PORT_WRITE(CTRL, DSI_PORT_READ(CTRL) | DSI1_CTRL_EN);
/* Bring AFE out of reset. */ - if (dsi->port == 0) { + if (dsi->variant->port == 0) { } else { DSI_PORT_WRITE(PHY_AFEC0, DSI_PORT_READ(PHY_AFEC0) & @@ -1313,8 +1324,16 @@ static const struct drm_encoder_helper_funcs vc4_dsi_encoder_helper_funcs = { .mode_fixup = vc4_dsi_encoder_mode_fixup, };
+static const struct vc4_dsi_variant bcm2835_dsi1_variant = { + .port = 1, + .broken_axi_workaround = true, + .debugfs_name = "dsi1_regs", + .regs = dsi1_regs, + .nregs = ARRAY_SIZE(dsi1_regs), +}; + static const struct of_device_id vc4_dsi_dt_match[] = { - { .compatible = "brcm,bcm2835-dsi1", (void *)(uintptr_t)1 }, + { .compatible = "brcm,bcm2835-dsi1", &bcm2835_dsi1_variant }, {} };
@@ -1325,7 +1344,7 @@ static void dsi_handle_error(struct vc4_dsi *dsi, if (!(stat & bit)) return;
- DRM_ERROR("DSI%d: %s error\n", dsi->port, type); + DRM_ERROR("DSI%d: %s error\n", dsi->variant->port, type); *ret = IRQ_HANDLED; }
@@ -1423,7 +1442,7 @@ vc4_dsi_init_phy_clocks(struct vc4_dsi *dsi) int ret;
snprintf(clk_name, sizeof(clk_name), - "dsi%u_%s", dsi->port, phy_clocks[i].name); + "dsi%u_%s", dsi->variant->port, phy_clocks[i].name);
/* We just use core fixed factor clock ops for the PHY * clocks. The clocks are actually gated by the @@ -1471,7 +1490,7 @@ static int vc4_dsi_bind(struct device *dev, struct device *master, void *data) if (!match) return -ENODEV;
- dsi->port = (uintptr_t)match->data; + dsi->variant = match->data;
vc4_dsi_encoder = devm_kzalloc(dev, sizeof(*vc4_dsi_encoder), GFP_KERNEL); @@ -1488,13 +1507,8 @@ static int vc4_dsi_bind(struct device *dev, struct device *master, void *data) return PTR_ERR(dsi->regs);
dsi->regset.base = dsi->regs; - if (dsi->port == 0) { - dsi->regset.regs = dsi0_regs; - dsi->regset.nregs = ARRAY_SIZE(dsi0_regs); - } else { - dsi->regset.regs = dsi1_regs; - dsi->regset.nregs = ARRAY_SIZE(dsi1_regs); - } + dsi->regset.regs = dsi->variant->regs; + dsi->regset.nregs = dsi->variant->nregs;
if (DSI_PORT_READ(ID) != DSI_ID_VALUE) { dev_err(dev, "Port returned 0x%08x for ID instead of 0x%08x\n", @@ -1506,7 +1520,7 @@ static int vc4_dsi_bind(struct device *dev, struct device *master, void *data) * from the ARM. It does handle writes from the DMA engine, * so set up a channel for talking to it. */ - if (dsi->port == 1) { + if (dsi->variant->broken_axi_workaround) { dsi->reg_dma_mem = dma_alloc_coherent(dev, 4, &dsi->reg_dma_paddr, GFP_KERNEL); @@ -1627,10 +1641,7 @@ static int vc4_dsi_bind(struct device *dev, struct device *master, void *data) */ list_splice_init(&dsi->encoder->bridge_chain, &dsi->bridge_chain);
- if (dsi->port == 0) - vc4_debugfs_add_regset32(drm, "dsi0_regs", &dsi->regset); - else - vc4_debugfs_add_regset32(drm, "dsi1_regs", &dsi->regset); + vc4_debugfs_add_regset32(drm, dsi->variant->debugfs_name, &dsi->regset);
pm_runtime_enable(dev);
From: Dave Stevenson dave.stevenson@raspberrypi.com
DSI0 was partially supported, but didn't register with the main driver, and the code was inconsistent as to whether it checked port == 0 or port == 1.
Add compatible string and other support to make it consistent.
Signed-off-by: Dave Stevenson dave.stevenson@raspberrypi.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_dsi.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c index 601020c10053..82162900e351 100644 --- a/drivers/gpu/drm/vc4/vc4_dsi.c +++ b/drivers/gpu/drm/vc4/vc4_dsi.c @@ -1324,6 +1324,13 @@ static const struct drm_encoder_helper_funcs vc4_dsi_encoder_helper_funcs = { .mode_fixup = vc4_dsi_encoder_mode_fixup, };
+static const struct vc4_dsi_variant bcm2835_dsi0_variant = { + .port = 0, + .debugfs_name = "dsi0_regs", + .regs = dsi0_regs, + .nregs = ARRAY_SIZE(dsi0_regs), +}; + static const struct vc4_dsi_variant bcm2835_dsi1_variant = { .port = 1, .broken_axi_workaround = true, @@ -1333,6 +1340,7 @@ static const struct vc4_dsi_variant bcm2835_dsi1_variant = { };
static const struct of_device_id vc4_dsi_dt_match[] = { + { .compatible = "brcm,bcm2835-dsi0", &bcm2835_dsi0_variant }, { .compatible = "brcm,bcm2835-dsi1", &bcm2835_dsi1_variant }, {} };
From: Dave Stevenson dave.stevenson@raspberrypi.com
DSI1 on BCM2711 doesn't require the DMA workaround that is used on BCM2835/6/7, therefore it needs a new compatible string.
Signed-off-by: Dave Stevenson dave.stevenson@raspberrypi.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- Documentation/devicetree/bindings/display/brcm,bcm2835-dsi0.yaml | 1 + 1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/display/brcm,bcm2835-dsi0.yaml b/Documentation/devicetree/bindings/display/brcm,bcm2835-dsi0.yaml index eb44e072b6e5..55c60919991f 100644 --- a/Documentation/devicetree/bindings/display/brcm,bcm2835-dsi0.yaml +++ b/Documentation/devicetree/bindings/display/brcm,bcm2835-dsi0.yaml @@ -18,6 +18,7 @@ properties:
compatible: enum: + - brcm,bcm2711-dsi1 - brcm,bcm2835-dsi0 - brcm,bcm2835-dsi1
From: Dave Stevenson dave.stevenson@raspberrypi.com
BCM2711 DSI1 doesn't have the issue with the ARM not being able to write to the registers, therefore remove the DMA workaround for that compatible string.
Signed-off-by: Dave Stevenson dave.stevenson@raspberrypi.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- drivers/gpu/drm/vc4/vc4_dsi.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_dsi.c b/drivers/gpu/drm/vc4/vc4_dsi.c index 82162900e351..a55256ed0955 100644 --- a/drivers/gpu/drm/vc4/vc4_dsi.c +++ b/drivers/gpu/drm/vc4/vc4_dsi.c @@ -1324,6 +1324,13 @@ static const struct drm_encoder_helper_funcs vc4_dsi_encoder_helper_funcs = { .mode_fixup = vc4_dsi_encoder_mode_fixup, };
+static const struct vc4_dsi_variant bcm2711_dsi1_variant = { + .port = 1, + .debugfs_name = "dsi1_regs", + .regs = dsi1_regs, + .nregs = ARRAY_SIZE(dsi1_regs), +}; + static const struct vc4_dsi_variant bcm2835_dsi0_variant = { .port = 0, .debugfs_name = "dsi0_regs", @@ -1340,6 +1347,7 @@ static const struct vc4_dsi_variant bcm2835_dsi1_variant = { };
static const struct of_device_id vc4_dsi_dt_match[] = { + { .compatible = "brcm,bcm2711-dsi1", &bcm2711_dsi1_variant }, { .compatible = "brcm,bcm2835-dsi0", &bcm2835_dsi0_variant }, { .compatible = "brcm,bcm2835-dsi1", &bcm2835_dsi1_variant }, {} @@ -1524,8 +1532,8 @@ static int vc4_dsi_bind(struct device *dev, struct device *master, void *data) return -ENODEV; }
- /* DSI1 has a broken AXI slave that doesn't respond to writes - * from the ARM. It does handle writes from the DMA engine, + /* DSI1 on BCM2835/6/7 has a broken AXI slave that doesn't respond to + * writes from the ARM. It does handle writes from the DMA engine, * so set up a channel for talking to it. */ if (dsi->variant->broken_axi_workaround) {
From: Dave Stevenson dave.stevenson@raspberrypi.com
Updates the compatible string for DSI1 on BCM2711 to differentiate it from BCM2835.
Signed-off-by: Dave Stevenson dave.stevenson@raspberrypi.com Signed-off-by: Maxime Ripard maxime@cerno.tech --- arch/arm/boot/dts/bcm2711.dtsi | 1 + 1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/bcm2711.dtsi b/arch/arm/boot/dts/bcm2711.dtsi index 4847dd305317..f53a51cc91f0 100644 --- a/arch/arm/boot/dts/bcm2711.dtsi +++ b/arch/arm/boot/dts/bcm2711.dtsi @@ -540,6 +540,7 @@ &dsi0 {
&dsi1 { interrupts = <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH>; + compatible = "brcm,bcm2711-dsi1"; };
&gpio {
On Thu, 2020-12-03 at 14:25 +0100, Maxime Ripard wrote:
From: Dave Stevenson dave.stevenson@raspberrypi.com
Updates the compatible string for DSI1 on BCM2711 to differentiate it from BCM2835.
Signed-off-by: Dave Stevenson dave.stevenson@raspberrypi.com Signed-off-by: Maxime Ripard maxime@cerno.tech
Applied for-next,
Thanks!
Hi Maxime
On Thu, 3 Dec 2020 at 13:25, Maxime Ripard maxime@cerno.tech wrote:
Hi,
Here's a series adding support for the DSI0 controller in the BCM2835 and the DSI1 controller found in the BCM2711.
Let me know what you think, Maxime
Thanks for that series - your using a variant structure is much cleaner than the hack I had.
For those that I didn't author (ie 1, 3, and 4) Reviewed-by: Dave Stevenson dave.stevenson@raspberrypi.com
Dave Stevenson (5): drm/vc4: dsi: Correct DSI register definition drm/vc4: dsi: Add support for DSI0 dt-bindings: Add compatible for BCM2711 DSI1 drm/vc4: dsi: Add configuration for BCM2711 DSI1 ARM: dts: bcm2711: Use compatible string for BCM2711 DSI1
Maxime Ripard (3): drm/vc4: drv: Remove the DSI pointer in vc4_drv drm/vc4: dsi: Use snprintf for the PHY clocks instead of an array drm/vc4: dsi: Introduce a variant structure
.../bindings/display/brcm,bcm2835-dsi0.yaml | 1 + arch/arm/boot/dts/bcm2711.dtsi | 1 + drivers/gpu/drm/vc4/vc4_drv.h | 1 - drivers/gpu/drm/vc4/vc4_dsi.c | 111 ++++++++++-------- 4 files changed, 67 insertions(+), 47 deletions(-)
-- 2.28.0
On Thu, Dec 03, 2020 at 03:19:15PM +0000, Dave Stevenson wrote:
Hi Maxime
On Thu, 3 Dec 2020 at 13:25, Maxime Ripard maxime@cerno.tech wrote:
Hi,
Here's a series adding support for the DSI0 controller in the BCM2835 and the DSI1 controller found in the BCM2711.
Let me know what you think, Maxime
Thanks for that series - your using a variant structure is much cleaner than the hack I had.
For those that I didn't author (ie 1, 3, and 4) Reviewed-by: Dave Stevenson dave.stevenson@raspberrypi.com
Applied 1-7 to drm-misc-next
Thanks! Maxime
dri-devel@lists.freedesktop.org
-
Dave Stevenson -
Frieder Schrempf -
Maxime Ripard -
Nicolas Saenz Julienne