[PATCH v10 0/6] drm/sun4i: Allwinner A64 MIPI-DSI support

[Jagan Teki]

This is v10 version for Allwinner A64 MIPI-DSI support
and here is the previous version set[1].

This series on top of drm-misc/for-linux-next along with video start
delay fix [2]

Changes for v10:
- updated dt-bindings as per .yaml format
- rebased on drm-misc/for-linux-next
Changes for v9:
- moved dsi fixes in separate series on top of A33 [2]
- rebase on linux-next and on top of [2]
Changes for v8:
- rebased on drm-misc change along with linux-next
- reworked video start delay patch
- tested on 4 different dsi panels
- reworked commit messages
Changes for v7:
- moved vcc-dsi binding to required filed.
- drop quotes on fallback dphy bindings.
- drop min_rate clock pll-mipi patches.
- introduce dclk divider computation as like A64 BSP.
- add A64 DSI quark patches.
- fixed A64 DSI pipeline.
- add proper commit messages.
- collect Merlijn Wajer Tested-by credits.
Changes for v6:
- dropped unneeded changes, patches
- fixed all burst mode patches as per previous version comments
- rebase on master
- update proper commit message
- dropped unneeded comments
- order the patches that make review easy
Changes for v5:
- collect Rob, Acked-by
- droped "Fix VBP size calculation" patch
- updated vblk timing calculation.
- droped techstar, bananapi dsi panel drivers which may require
bridge or other setup. it's under discussion.
Changes for v4:
- droppoed untested CCU_FEATURE_FIXED_POSTDIV check code in
nkm min, max rate patches
- create two patches for "Add Allwinner A64 MIPI DSI support"
one for has_mod_clk quirk and other one for A64 support
- use existing driver code construct for hblk computation
- dropped "Increase hfp packet overhead" patch [2], though BSP added
this but we have no issues as of now.
(no issues on panel side w/o this change)
- create separate function for vblk computation
- enable vcc-dsi regulator in dsi_runtime_resume
- collect Rob, Acked-by
- update MAINTAINERS file for panel drivers
- cleanup commit messages
- fixed checkpatch warnings/errors

[1] https://patchwork.freedesktop.org/series/61310/
[2] https://patchwork.freedesktop.org/patch/334086/

Any inputs?
Jagan.

Jagan Teki (6):
dt-bindings: sun6i-dsi: Add A64 MIPI-DSI compatible
dt-bindings: sun6i-dsi: Add A64 DPHY compatible (w/ A31 fallback)
drm/sun4i: dsi: Add has_mod_clk quirk
drm/sun4i: dsi: Add Allwinner A64 MIPI DSI support
arm64: dts: allwinner: a64: Add MIPI DSI pipeline
[DO NOT MERGE] arm64: dts: allwinner: bananapi-m64: Enable Bananapi S070WV20-CT16 DSI
panel

.../display/allwinner,sun6i-a31-mipi-dsi.yaml | 4 +-
.../phy/allwinner,sun6i-a31-mipi-dphy.yaml | 6 ++-
.../dts/allwinner/sun50i-a64-bananapi-m64.dts | 31 ++++++++++++++
arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 38 +++++++++++++++++
drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 42 ++++++++++++++-----
drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 5 +++
6 files changed, 114 insertions(+), 12 deletions(-)

--
2.18.0.321.gffc6fa0e3

[Jagan Teki]

The MIPI DSI controller in Allwinner A64 is similar to A33.

But unlike A33, A64 doesn't have DSI_SCLK gating so it is valid
to with separate compatible for A64 on the same driver.

Signed-off-by: Jagan Teki <ja...@amarulasolutions.com>
---
.../bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
index dafc0980c4fa..cfcc84d38084 100644
--- a/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
+++ b/Documentation/devicetree/bindings/display/allwinner,sun6i-a31-mipi-dsi.yaml
@@ -15,7 +15,9 @@ properties:
"#size-cells": true

compatible:
- const: allwinner,sun6i-a31-mipi-dsi
+ enum:
+ - const: allwinner,sun6i-a31-mipi-dsi
+ - const: allwinner,sun50i-a64-mipi-dsi

reg:
maxItems: 1
--
2.18.0.321.gffc6fa0e3

[Jagan Teki]

The MIPI DSI PHY controller on Allwinner A64 is similar
on the one on A31.

Add A64 compatible and append A31 compatible as fallback.

Signed-off-by: Jagan Teki <ja...@amarulasolutions.com>
---

.../bindings/phy/allwinner,sun6i-a31-mipi-dphy.yaml | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/phy/allwinner,sun6i-a31-mipi-dphy.yaml b/Documentation/devicetree/bindings/phy/allwinner,sun6i-a31-mipi-dphy.yaml
index fa46670de299..8841938050b2 100644
--- a/Documentation/devicetree/bindings/phy/allwinner,sun6i-a31-mipi-dphy.yaml
+++ b/Documentation/devicetree/bindings/phy/allwinner,sun6i-a31-mipi-dphy.yaml
@@ -15,7 +15,11 @@ properties:
const: 0

compatible:
- const: allwinner,sun6i-a31-mipi-dphy
+ oneOf:
+ - const: allwinner,sun6i-a31-mipi-dphy
+ - items:
+ - const: allwinner,sun50i-a64-mipi-dphy
+ - const: allwinner,sun6i-a31-mipi-dphy

[Jagan Teki]

As per the user manual, look like mod clock is not mandatory
for all Allwinner MIPI DSI controllers, it is connected to
CLK_DSI_SCLK for A31 and not available in A64.

So add has_mod_clk quirk and process the clk accordingly.

Tested-by: Merlijn Wajer <mer...@wizzup.org>

Signed-off-by: Jagan Teki <ja...@amarulasolutions.com>
---

drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 35 ++++++++++++++++++--------
drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 5 ++++
2 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index b8a0d0501ca7..6724a14c816d 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -11,6 +11,7 @@
#include <linux/crc-ccitt.h>
#include <linux/module.h>
#include <linux/of_address.h>
+#include <linux/of_device.h>
#include <linux/phy/phy-mipi-dphy.h>
#include <linux/phy/phy.h>
#include <linux/platform_device.h>
@@ -1092,6 +1093,7 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
dsi->dev = dev;
dsi->host.ops = &sun6i_dsi_host_ops;
dsi->host.dev = dev;
+ dsi->variant = of_device_get_match_data(dev);

res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
base = devm_ioremap_resource(dev, res);
@@ -1119,17 +1121,20 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
return PTR_ERR(dsi->reset);
}

- dsi->mod_clk = devm_clk_get(dev, "mod");
- if (IS_ERR(dsi->mod_clk)) {
- dev_err(dev, "Couldn't get the DSI mod clock\n");
- return PTR_ERR(dsi->mod_clk);
+ if (dsi->variant->has_mod_clk) {
+ dsi->mod_clk = devm_clk_get(dev, "mod");
+ if (IS_ERR(dsi->mod_clk)) {
+ dev_err(dev, "Couldn't get the DSI mod clock\n");
+ return PTR_ERR(dsi->mod_clk);
+ }
}

/*
* In order to operate properly, that clock seems to be always
* set to 297MHz.
*/
- clk_set_rate_exclusive(dsi->mod_clk, 297000000);
+ if (dsi->variant->has_mod_clk)
+ clk_set_rate_exclusive(dsi->mod_clk, 297000000);

dsi->dphy = devm_phy_get(dev, "dphy");
if (IS_ERR(dsi->dphy)) {
@@ -1159,7 +1164,8 @@ static int sun6i_dsi_probe(struct platform_device *pdev)
err_pm_disable:
pm_runtime_disable(dev);
err_unprotect_clk:
- clk_rate_exclusive_put(dsi->mod_clk);
+ if (dsi->variant->has_mod_clk)
+ clk_rate_exclusive_put(dsi->mod_clk);
return ret;
}

@@ -1171,7 +1177,8 @@ static int sun6i_dsi_remove(struct platform_device *pdev)
component_del(&pdev->dev, &sun6i_dsi_ops);
mipi_dsi_host_unregister(&dsi->host);
pm_runtime_disable(dev);
- clk_rate_exclusive_put(dsi->mod_clk);
+ if (dsi->variant->has_mod_clk)
+ clk_rate_exclusive_put(dsi->mod_clk);

return 0;
}
@@ -1188,7 +1195,8 @@ static int __maybe_unused sun6i_dsi_runtime_resume(struct device *dev)
}

reset_control_deassert(dsi->reset);
- clk_prepare_enable(dsi->mod_clk);
+ if (dsi->variant->has_mod_clk)
+ clk_prepare_enable(dsi->mod_clk);

/*
* Enable the DSI block.
@@ -1229,9 +1237,16 @@ static const struct dev_pm_ops sun6i_dsi_pm_ops = {
NULL)
};

+static const struct sun6i_dsi_variant sun6i_a31_mipi_dsi = {
+ .has_mod_clk = true,
+};
+
static const struct of_device_id sun6i_dsi_of_table[] = {
- { .compatible = "allwinner,sun6i-a31-mipi-dsi" },
- { }
+ {
+ .compatible = "allwinner,sun6i-a31-mipi-dsi",
+ .data = &sun6i_a31_mipi_dsi,
+ },
+ { /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, sun6i_dsi_of_table);

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
index 3f4846f581ef..d791c9f6fccf 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
@@ -15,6 +15,10 @@

#define SUN6I_DSI_TCON_DIV 4

+struct sun6i_dsi_variant {
+ bool has_mod_clk;
+};
+
struct sun6i_dsi {
struct drm_connector connector;
struct drm_encoder encoder;
@@ -31,6 +35,7 @@ struct sun6i_dsi {
struct sun4i_drv *drv;
struct mipi_dsi_device *device;
struct drm_panel *panel;
+ const struct sun6i_dsi_variant *variant;
};

static inline struct sun6i_dsi *host_to_sun6i_dsi(struct mipi_dsi_host *host)
--
2.18.0.321.gffc6fa0e3

[Jagan Teki]

The MIPI DSI controller in Allwinner A64 is similar to A33.

But unlike A33, A64 doesn't have DSI_SCLK gating so add compatible
for Allwinner A64 with uninitialized has_mod_clk driver.

Signed-off-by: Jagan Teki <ja...@amarulasolutions.com>
Tested-by: Merlijn Wajer <mer...@wizzup.org>
---
drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index 6724a14c816d..e3aab815fa73 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -1241,11 +1241,18 @@ static const struct sun6i_dsi_variant sun6i_a31_mipi_dsi = {
.has_mod_clk = true,
};

+static const struct sun6i_dsi_variant sun50i_a64_mipi_dsi = {

+};
+
static const struct of_device_id sun6i_dsi_of_table[] = {

{
.compatible = "allwinner,sun6i-a31-mipi-dsi",
.data = &sun6i_a31_mipi_dsi,
},
+ {
+ .compatible = "allwinner,sun50i-a64-mipi-dsi",
+ .data = &sun50i_a64_mipi_dsi,
+ },

{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, sun6i_dsi_of_table);

--
2.18.0.321.gffc6fa0e3

[Jagan Teki]

Add MIPI DSI pipeline for Allwinner A64.

- dsi node, with A64 compatible since it doesn't support
DSI_SCLK gating unlike A33
- dphy node, with A64 compatible with A33 fallback since
DPHY on A64 and A33 is similar
- finally, attach the dsi_in to tcon0 for complete MIPI DSI

Signed-off-by: Jagan Teki <ja...@amarulasolutions.com>
Tested-by: Merlijn Wajer <mer...@wizzup.org>
---

arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 38 +++++++++++++++++++
1 file changed, 38 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 69128a6dfc46..ad4170b8aee0 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -382,6 +382,12 @@
#address-cells = <1>;
#size-cells = <0>;
reg = <1>;
+
+ tcon0_out_dsi: endpoint@1 {
+ reg = <1>;
+ remote-endpoint = <&dsi_in_tcon0>;
+ allwinner,tcon-channel = <1>;
+ };
};
};
};
@@ -1003,6 +1009,38 @@
status = "disabled";
};

+ dsi: dsi@1ca0000 {
+ compatible = "allwinner,sun50i-a64-mipi-dsi";
+ reg = <0x01ca0000 0x1000>;
+ interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&ccu CLK_BUS_MIPI_DSI>;
+ clock-names = "bus";
+ resets = <&ccu RST_BUS_MIPI_DSI>;
+ phys = <&dphy>;
+ phy-names = "dphy";
+ status = "disabled";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port {
+ dsi_in_tcon0: endpoint {
+ remote-endpoint = <&tcon0_out_dsi>;
+ };
+ };
+ };
+
+ dphy: d-phy@1ca1000 {
+ compatible = "allwinner,sun50i-a64-mipi-dphy",
+ "allwinner,sun6i-a31-mipi-dphy";
+ reg = <0x01ca1000 0x1000>;
+ clocks = <&ccu CLK_BUS_MIPI_DSI>,
+ <&ccu CLK_DSI_DPHY>;
+ clock-names = "bus", "mod";
+ resets = <&ccu RST_BUS_MIPI_DSI>;
+ status = "disabled";
+ #phy-cells = <0>;
+ };
+
hdmi: hdmi@1ee0000 {
compatible = "allwinner,sun50i-a64-dw-hdmi",
"allwinner,sun8i-a83t-dw-hdmi";
--
2.18.0.321.gffc6fa0e3

[Jagan Teki]

This patch add support for Bananapi S070WV20-CT16 DSI panel to
BPI-M64 board.

DSI panel connected via board DSI port with,
- DLDO1 as VCC-DSI supply
- DCDC1 as VDD supply
- PD7 gpio for lcd enable pin
- PD6 gpio for lcd reset pin
- PD5 gpio for backlight enable pin

Signed-off-by: Jagan Teki <ja...@amarulasolutions.com>
---

.../dts/allwinner/sun50i-a64-bananapi-m64.dts | 31 +++++++++++++++++++
1 file changed, 31 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
index 208373efee49..6beaecdd802a 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
@@ -45,6 +45,7 @@
#include "sun50i-a64.dtsi"

#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/pwm/pwm.h>

/ {
model = "BananaPi-M64";
@@ -56,6 +57,14 @@
serial1 = &uart1;
};

+ backlight: backlight {
+ compatible = "pwm-backlight";
+ pwms = <&r_pwm 0 50000 PWM_POLARITY_INVERTED>;
+ brightness-levels = <1 2 4 8 16 32 64 128 255>;
+ default-brightness-level = <2>;
+ enable-gpios = <&pio 3 5 GPIO_ACTIVE_HIGH>; /* LCD-BL-EN: PD5 */
+ };
+
chosen {
stdout-path = "serial0:115200n8";
};
@@ -116,6 +125,24 @@
status = "okay";
};

+&dphy {
+ status = "okay";
+};
+
+&dsi {
+ vcc-dsi-supply = <&reg_dldo1>; /* VCC3V3-DSI */
+ status = "okay";
+
+ panel@0 {
+ compatible = "bananapi,s070wv20-ct16-icn6211";
+ reg = <0>;
+ enable-gpios = <&pio 3 7 GPIO_ACTIVE_HIGH>; /* LCD-PWR-EN: PD7 */
+ reset-gpios = <&pio 3 6 GPIO_ACTIVE_HIGH>; /* LCD-RST: PD6 */
+ vdd-supply = <&reg_dcdc1>;
+ backlight = <&backlight>;
+ };
+};
+
&ehci0 {
status = "okay";
};
@@ -206,6 +233,10 @@
status = "okay";
};

+&r_pwm {
+ status = "okay";
+};
+
&r_rsb {
status = "okay";

--
2.18.0.321.gffc6fa0e3

[Jagan Teki]

On Mon, Oct 7, 2019 at 3:01 PM Maxime Ripard <mri...@kernel.org> wrote:

> How did you test this? It will report an error when running the
> validation

I did follow the v9 comments [1] and forgot to do dt-doc-validate.
will send the v11 for this patch alone, will that be okay?

[1] https://patchwork.freedesktop.org/patch/307499/

[Jagan Teki]

On Mon, Oct 7, 2019 at 4:27 PM Maxime Ripard <mri...@kernel.org> wrote:

> This won't validate with the bindings you have either here, since it
> still expects bus and mod.
>
> I guess in that cas, we can just drop clock-names, which will require
> a bit of work on the driver side as well.

Okay.
mod clock is not required for a64, ie reason we have has_mod_clk quirk
patch. Adjust the clock-names: on dt-bindings would make sense here,
what do you think?

[Jagan Teki]

On Wed, Oct 16, 2019 at 1:33 PM Maxime Ripard <mri...@kernel.org> wrote:

> I'm confused, what are you suggesting?

Sorry for the confusion.

The mod clock is not required for A64 and we have a patch for handling
mod clock using has_mod_clk quirk(on the series), indeed the mod clock
is available in A31 and not needed for A64. So, to satisfy this
requirement the clock-names on dt-bindings can update to make mod
clock-name is optional and bus clock is required.

I'm not exactly sure, this is correct but trying to understand if it
is possible or not? something like

clocks:
minItems: 1
maxItems: 2
items:
- description: Bus Clock
- description: Module Clock

clock-names:
minItems: 1
maxItems: 2
items:
- const: bus
- const: mod

[Jagan Teki]

On Thu, Oct 17, 2019 at 3:22 PM Maxime Ripard <mri...@kernel.org> wrote:

> No, the bus clock name is not needed if there's only one clock.

Okay, is it because the same clock handle it on PHY side?

>
> > I'm not exactly sure, this is correct but trying to understand if it
> > is possible or not? something like
> >
> > clocks:
> > minItems: 1
> > maxItems: 2
> > items:
> > - description: Bus Clock
> > - description: Module Clock
>

> That's correct.

>
> > clock-names:
> > minItems: 1
> > maxItems: 2
> > items:
> > - const: bus
> > - const: mod
>

> Here, just keep the current clock-names definition, and make it
> required only for SoCs that are not the A64

Okay, please have a look here I have pasted the diff for comments.

clocks:
+ minItems: 2

items:
- description: Bus Clock
- description: Module Clock

@@ -64,14 +65,26 @@ required:
- compatible
- reg
- interrupts
- - clocks
- - clock-names
- phys
- phy-names
- resets
- vcc-dsi-supply
- port

+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: allwinner,sun6i-a31-mipi-dsi
+ then:
+ properties:
+ clocks:
+ minItems: 2
+ required:
+ - clocks
+ - clock-names
+
additionalProperties: false

I have marked minItems: 2 on clocks since we need to use minimum of 2
clocks like both bus and mod not mod clock alone.

Please let me know your comments.

Jagan.

[Jagan Teki]

On Thu, Oct 17, 2019 at 3:22 PM Maxime Ripard <mri...@kernel.org> wrote:

> No, the bus clock name is not needed if there's only one clock.

Looks like we need "bus" clock required since the
devm_regmap_init_mmio_clk is created only if bus clock-names added in
dt.

[Jagan Teki]

On Fri, Oct 25, 2019 at 12:33 AM Maxime Ripard <mri...@kernel.org> wrote:

> No, because there's only one clock and thus you don't need to
> differentiate them.

>
> > >
> > > > I'm not exactly sure, this is correct but trying to understand if it
> > > > is possible or not? something like
> > > >
> > > > clocks:
> > > > minItems: 1
> > > > maxItems: 2
> > > > items:
> > > > - description: Bus Clock
> > > > - description: Module Clock
> > >
> > > That's correct.
> > >
> > > > clock-names:
> > > > minItems: 1
> > > > maxItems: 2
> > > > items:
> > > > - const: bus
> > > > - const: mod
> > >
> > > Here, just keep the current clock-names definition, and make it
> > > required only for SoCs that are not the A64
> >
> > Okay, please have a look here I have pasted the diff for comments.
> >
> > clocks:
> > + minItems: 2
> > items:
> > - description: Bus Clock
> > - description: Module Clock
>

> Didn't you tell me that you didn't need the module clock?
>
> How do you handle the case were you just have the bus clock then?

Make sense, it is my mistake then. we don't require to specify here I
think since it implies globally. I think it should be sufficient to
mention on allOf: section based on the SoC like I mentioned in above
snippet.