| Message ID | 20200212222355.17141-2-andrey.lebedev@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/1] Support LVDS output on Allwinner A20 | expand |
On Thu, Feb 13, 2020 at 12:23:57AM +0200, andrey.lebedev@gmail.com wrote: > From: Andrey Lebedev <andrey@lebedev.lt> > > Define pins for LVDS channels 0 and 1, configure reset line for tcon0 and > provide sample LVDS panel, connected to tcon0. > > Signed-off-by: Andrey Lebedev <andrey@lebedev.lt> And this prefix should be ARM: dts: sun7i ;) > --- > arch/arm/boot/dts/sun7i-a20.dtsi | 45 +++++++++++++++++++++++++++++--- > 1 file changed, 42 insertions(+), 3 deletions(-) > > diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi > index 92b5be97085d..b05fdf8df32e 100644 > --- a/arch/arm/boot/dts/sun7i-a20.dtsi > +++ b/arch/arm/boot/dts/sun7i-a20.dtsi > @@ -47,6 +47,7 @@ > #include <dt-bindings/dma/sun4i-a10.h> > #include <dt-bindings/clock/sun7i-a20-ccu.h> > #include <dt-bindings/reset/sun4i-a10-ccu.h> > +#include <dt-bindings/pinctrl/sun4i-a10.h> > > / { > interrupt-parent = <&gic>; > @@ -407,8 +408,8 @@ > compatible = "allwinner,sun7i-a20-tcon"; > reg = <0x01c0c000 0x1000>; > interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>; > - resets = <&ccu RST_TCON0>; > - reset-names = "lcd"; > + resets = <&ccu RST_TCON0>, <&ccu RST_LVDS>; > + reset-names = "lcd", "lvds"; > clocks = <&ccu CLK_AHB_LCD0>, > <&ccu CLK_TCON0_CH0>, > <&ccu CLK_TCON0_CH1>; > @@ -444,6 +445,11 @@ > #size-cells = <0>; > reg = <1>; > > + tcon0_out_lvds: endpoint@0 { > + reg = <0>; > + remote-endpoint = <&lvds_in_tcon0>; > + allwinner,tcon-channel = <0>; > + }; A new line here would be nice > tcon0_out_hdmi: endpoint@1 { > reg = <1>; > remote-endpoint = <&hdmi_in_tcon0>; > @@ -686,6 +692,19 @@ > }; > }; > > + lvds_panel: panel@1c16500 { > + compatible = "panel-lvds"; > + #address-cells = <1>; > + #size-cells = <0>; > + status = "disabled"; > + > + port { > + lvds_in_tcon0: endpoint { > + remote-endpoint = <&tcon0_out_lvds>; > + }; > + }; > + }; > + There's no point in creating that panel. > spi2: spi@1c17000 { > compatible = "allwinner,sun4i-a10-spi"; > reg = <0x01c17000 0x1000>; > @@ -872,7 +891,7 @@ > gmac_rgmii_pins: gmac-rgmii-pins { > pins = "PA0", "PA1", "PA2", > "PA3", "PA4", "PA5", "PA6", > - "PA7", "PA8", "PA10", > + "PA7", "PA8", "PA10", > "PA11", "PA12", "PA13", > "PA15", "PA16"; > function = "gmac"; > @@ -1162,6 +1181,26 @@ > pins = "PI20", "PI21"; > function = "uart7"; > }; > + > + /omit-if-no-ref/ > + lcd_lvds0_pins: lcd_lvds0_pins { underscores in the node names will create a dtc warning at compilation, you should use lcd-lvds0-pins instead. > + allwinner,pins = > + "PD0", "PD1", "PD2", "PD3", "PD4", > + "PD5", "PD6", "PD7", "PD8", "PD9"; > + allwinner,function = "lvds0"; > + allwinner,drive = <SUN4I_PINCTRL_10_MA>; > + allwinner,pull = <SUN4I_PINCTRL_NO_PULL>; Those properties are deprecated and should be replaced by pins and functions. allwinner,drive and allwinner,pull are at their default values and can be dropped. This will create a spurious warning message for TCON1, since we adjusted the driver to tell it supports LVDS, but there's no LVDS reset line, so we need to make it finer grained. Maybe adding a tcon0 / tcon1 compatible? Chen-Yu, any thought? Maxime
On Thu, Feb 13, 2020 at 10:43:04AM +0100, Maxime Ripard wrote: > On Thu, Feb 13, 2020 at 12:23:57AM +0200, andrey.lebedev@gmail.com wrote: > > From: Andrey Lebedev <andrey@lebedev.lt> > > > > Define pins for LVDS channels 0 and 1, configure reset line for tcon0 and > > provide sample LVDS panel, connected to tcon0. > > > > Signed-off-by: Andrey Lebedev <andrey@lebedev.lt> > > And this prefix should be ARM: dts: sun7i ;) Ah, thanks, I think I've got the pattern now! > > + /omit-if-no-ref/ > > + lcd_lvds0_pins: lcd_lvds0_pins { > > underscores in the node names will create a dtc warning at > compilation, you should use lcd-lvds0-pins instead. You're right, I wasn't following the naming convention here. dtc doesn't produce any warning on this though. Fixed that anyway. > This will create a spurious warning message for TCON1, since we > adjusted the driver to tell it supports LVDS, but there's no LVDS > reset line, so we need to make it finer grained. Yes, I can attribute two of the messages in my dmesg log [1] to this ("Missing LVDS properties" and "LVDS output disabled". "sun4i-tcon 1c0d000.lcd-controller" is indeed tcon1). And yes, I can see how they can be confusing to someone. I'd need some pointers on how to deal with that though (if we want to do it in this scope). [1] excerpt from kernel boot log: [ 4.890975] sun4i-drm display-engine: bound 1e00000.display-frontend (ops sun4i_frontend_driver_exit [sun4i_frontend]) [ 4.902032] sun4i-drm display-engine: bound 1e20000.display-frontend (ops sun4i_frontend_driver_exit [sun4i_frontend]) [ 4.913467] sun4i-drm display-engine: bound 1e60000.display-backend (ops sun4i_backend_ops [sun4i_backend]) [ 4.923806] sun4i-drm display-engine: bound 1e40000.display-backend (ops sun4i_backend_ops [sun4i_backend]) [ 4.934451] sun4i-drm display-engine: bound 1c0c000.lcd-controller (ops sun4i_tcon_platform_driver_exit [sun4i_tcon]) [ 4.945254] sun4i-tcon 1c0d000.lcd-controller: Missing LVDS properties, Please upgrade your DT [ 4.953935] sun4i-tcon 1c0d000.lcd-controller: LVDS output disabled [ 4.960857] sun4i-drm display-engine: No panel or bridge found... RGB output disabled [ 4.968845] sun4i-drm display-engine: bound 1c0d000.lcd-controller (ops sun4i_tcon_platform_driver_exit [sun4i_tcon]) [ 5.080874] sun4i-drm display-engine: bound 1c16000.hdmi (ops sun4i_hdmi_driver_exit [sun4i_drm_hdmi]) [ 5.110087] [drm] Initialized sun4i-drm 1.0.0 20150629 for display-engine on minor 0 [ 5.763064] sun4i-drm display-engine: fb0: sun4i-drmdrmfb frame buffer device
Hi, On Thu, Feb 13, 2020 at 10:08:23PM +0200, Andrey Lebedev wrote: > On Thu, Feb 13, 2020 at 10:43:04AM +0100, Maxime Ripard wrote: > > On Thu, Feb 13, 2020 at 12:23:57AM +0200, andrey.lebedev@gmail.com wrote: > > > From: Andrey Lebedev <andrey@lebedev.lt> > > > > > > Define pins for LVDS channels 0 and 1, configure reset line for tcon0 and > > > provide sample LVDS panel, connected to tcon0. > > > > > > Signed-off-by: Andrey Lebedev <andrey@lebedev.lt> > > > > And this prefix should be ARM: dts: sun7i ;) > > Ah, thanks, I think I've got the pattern now! > > > > + /omit-if-no-ref/ > > > + lcd_lvds0_pins: lcd_lvds0_pins { > > > > underscores in the node names will create a dtc warning at > > compilation, you should use lcd-lvds0-pins instead. It's silenced by default, but you'll get them with W=1 > You're right, I wasn't following the naming convention here. dtc doesn't > produce any warning on this though. Fixed that anyway. > > > This will create a spurious warning message for TCON1, since we > > adjusted the driver to tell it supports LVDS, but there's no LVDS > > reset line, so we need to make it finer grained. > > Yes, I can attribute two of the messages in my dmesg log [1] to this > ("Missing LVDS properties" and "LVDS output disabled". "sun4i-tcon > 1c0d000.lcd-controller" is indeed tcon1). And yes, I can see how they > can be confusing to someone. > > I'd need some pointers on how to deal with that though (if we want to do > it in this scope). Like I was mentionning, you could introduce a new compatible for each TCON (tcon0 and tcon1) and only set the support_lvds flag for tcon0 Maxime
On Fri, Feb 14, 2020 at 08:52:18AM +0100, Maxime Ripard wrote: > > > This will create a spurious warning message for TCON1, since we > > > adjusted the driver to tell it supports LVDS, but there's no LVDS > > > reset line, so we need to make it finer grained. > > > > Yes, I can attribute two of the messages in my dmesg log [1] to this > > ("Missing LVDS properties" and "LVDS output disabled". "sun4i-tcon > > 1c0d000.lcd-controller" is indeed tcon1). And yes, I can see how they > > can be confusing to someone. > > > > I'd need some pointers on how to deal with that though (if we want to do > > it in this scope). > > Like I was mentionning, you could introduce a new compatible for each > TCON (tcon0 and tcon1) and only set the support_lvds flag for tcon0 Can you give me an idea how that compatible might look like? tcon0: lcd-controller@1c0c000 { compatible = "allwinner,sun7i-a20-tcon", "allwinner,lvds"; or tcon0: lcd-controller@1c0c000 { compatible = "allwinner,sun7i-a20-tcon", "allwinner,tcon0"; ? Or something completely different?
On Fri, Feb 14, 2020 at 10:43:58AM +0200, Andrey Lebedev wrote: > On Fri, Feb 14, 2020 at 08:52:18AM +0100, Maxime Ripard wrote: > > > > This will create a spurious warning message for TCON1, since we > > > > adjusted the driver to tell it supports LVDS, but there's no LVDS > > > > reset line, so we need to make it finer grained. > > > > > > Yes, I can attribute two of the messages in my dmesg log [1] to this > > > ("Missing LVDS properties" and "LVDS output disabled". "sun4i-tcon > > > 1c0d000.lcd-controller" is indeed tcon1). And yes, I can see how they > > > can be confusing to someone. > > > > > > I'd need some pointers on how to deal with that though (if we want to do > > > it in this scope). > > > > Like I was mentionning, you could introduce a new compatible for each > > TCON (tcon0 and tcon1) and only set the support_lvds flag for tcon0 > > Can you give me an idea how that compatible might look like? > > tcon0: lcd-controller@1c0c000 { > compatible = "allwinner,sun7i-a20-tcon", "allwinner,lvds"; > > or > > tcon0: lcd-controller@1c0c000 { > compatible = "allwinner,sun7i-a20-tcon", "allwinner,tcon0"; > > ? Or something completely different? Something like &tcon0 { compatible = "allwinner,sun7i-a20-tcon0", "allwinner,sun7i-a20-tcon"; }; &tcon1 { compatible = "allwinner,sun7i-a20-tcon1", "allwinner,sun7i-a20-tcon"; }; Maxime
On Fri, Feb 14, 2020 at 09:53:51AM +0100, Maxime Ripard wrote: > On Fri, Feb 14, 2020 at 10:43:58AM +0200, Andrey Lebedev wrote: > > On Fri, Feb 14, 2020 at 08:52:18AM +0100, Maxime Ripard wrote: > > > > > This will create a spurious warning message for TCON1, since we > > > > > adjusted the driver to tell it supports LVDS, but there's no LVDS > > > > > reset line, so we need to make it finer grained. > > > > > > > > Yes, I can attribute two of the messages in my dmesg log [1] to this > > > > ("Missing LVDS properties" and "LVDS output disabled". "sun4i-tcon > > > > 1c0d000.lcd-controller" is indeed tcon1). And yes, I can see how they > > > > can be confusing to someone. > > > > > > > > I'd need some pointers on how to deal with that though (if we want to do > > > > it in this scope). > > > > > > Like I was mentionning, you could introduce a new compatible for each > > > TCON (tcon0 and tcon1) and only set the support_lvds flag for tcon0 > > > > Can you give me an idea how that compatible might look like? > > > > tcon0: lcd-controller@1c0c000 { > > compatible = "allwinner,sun7i-a20-tcon", "allwinner,lvds"; > > > > or > > > > tcon0: lcd-controller@1c0c000 { > > compatible = "allwinner,sun7i-a20-tcon", "allwinner,tcon0"; > > > > ? Or something completely different? > > Something like > > &tcon0 { > compatible = "allwinner,sun7i-a20-tcon0", "allwinner,sun7i-a20-tcon"; > }; > > &tcon1 { > compatible = "allwinner,sun7i-a20-tcon1", "allwinner,sun7i-a20-tcon"; > }; > Hi Maxime, here is what I came up with, please take a look. If the approach is right, I'll split it up and include into the patch set. From f3e45c958a9551a52ac26435785bdb572e54d8db Mon Sep 17 00:00:00 2001 From: Andrey Lebedev <andrey@lebedev.lt> Date: Fri, 14 Feb 2020 23:21:59 +0200 Subject: [PATCH] Mark tcon0 to be the only tcon capable of LVDS on sun7i-a20 This allows to avoid warnings about reset line not provided for tcon1. Signed-off-by: Andrey Lebedev <andrey@lebedev.lt> --- arch/arm/boot/dts/sun7i-a20.dtsi | 2 +- drivers/gpu/drm/sun4i/sun4i_tcon.c | 22 +++++++++++++++++++++- drivers/gpu/drm/sun4i/sun4i_tcon.h | 2 ++ 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi index 3b3c366a2bee..bab59fc4d9b1 100644 --- a/arch/arm/boot/dts/sun7i-a20.dtsi +++ b/arch/arm/boot/dts/sun7i-a20.dtsi @@ -405,7 +405,7 @@ }; tcon0: lcd-controller@1c0c000 { - compatible = "allwinner,sun7i-a20-tcon"; + compatible = "allwinner,sun7i-a20-tcon0", "allwinner,sun7i-a20-tcon"; reg = <0x01c0c000 0x1000>; interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>; resets = <&ccu RST_TCON0>, <&ccu RST_LVDS>; diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index 800a9bd86112..cb2040aec436 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -1107,6 +1107,25 @@ static struct sunxi_engine *sun4i_tcon_find_engine(struct sun4i_drv *drv, return sun4i_tcon_find_engine_traverse(drv, node, 0); } +/* + * Check if given tcon supports LVDS + * + * Some of the sunxi SoC variants contain several timing controllers, but only + * one of them can be used to drive LVDS screen. In this case such tcon is + * identified in respective quirks struct: lvds_compatible_tcon property will + * hold "compatible" string of the tcon, that supports LVDS. + * + * If lvds_compatible_tcon is not set, all tcons are considered capable of + * driving LVDS. + */ +static bool sun4i_tcon_lvds_compat(struct device *dev, struct sun4i_tcon *tcon) +{ + if (tcon->quirks->lvds_compatible_tcon == NULL) + return true; + return of_device_is_compatible(dev->of_node, + tcon->quirks->lvds_compatible_tcon); +} + static int sun4i_tcon_bind(struct device *dev, struct device *master, void *data) { @@ -1161,7 +1180,7 @@ static int sun4i_tcon_bind(struct device *dev, struct device *master, return ret; } - if (tcon->quirks->supports_lvds) { + if (tcon->quirks->supports_lvds && sun4i_tcon_lvds_compat(dev, tcon)) { /* * This can only be made optional since we've had DT * nodes without the LVDS reset properties. @@ -1481,6 +1500,7 @@ static const struct sun4i_tcon_quirks sun6i_a31s_quirks = { static const struct sun4i_tcon_quirks sun7i_a20_quirks = { .supports_lvds = true, + .lvds_compatible_tcon = "allwinner,sun7i-a20-tcon0", .has_channel_0 = true, .has_channel_1 = true, .dclk_min_div = 4, diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h index cfbf4e6c1679..bc87d28ee341 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.h +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h @@ -235,6 +235,8 @@ struct sun4i_tcon_quirks { bool needs_de_be_mux; /* sun6i needs mux to select backend */ bool needs_edp_reset; /* a80 edp reset needed for tcon0 access */ bool supports_lvds; /* Does the TCON support an LVDS output? */ + /* "compatible" string of TCON that exclusively supports LVDS */ + const char *lvds_compatible_tcon; u8 dclk_min_div; /* minimum divider for TCON0 DCLK */ /* callback to handle tcon muxing options */
Hi Andrey, On Fri, Feb 14, 2020 at 11:32:31PM +0200, Andrey Lebedev wrote: > On Fri, Feb 14, 2020 at 09:53:51AM +0100, Maxime Ripard wrote: > > On Fri, Feb 14, 2020 at 10:43:58AM +0200, Andrey Lebedev wrote: > > > On Fri, Feb 14, 2020 at 08:52:18AM +0100, Maxime Ripard wrote: > > > > > > This will create a spurious warning message for TCON1, since we > > > > > > adjusted the driver to tell it supports LVDS, but there's no LVDS > > > > > > reset line, so we need to make it finer grained. > > > > > > > > > > Yes, I can attribute two of the messages in my dmesg log [1] to this > > > > > ("Missing LVDS properties" and "LVDS output disabled". "sun4i-tcon > > > > > 1c0d000.lcd-controller" is indeed tcon1). And yes, I can see how they > > > > > can be confusing to someone. > > > > > > > > > > I'd need some pointers on how to deal with that though (if we want to do > > > > > it in this scope). > > > > > > > > Like I was mentionning, you could introduce a new compatible for each > > > > TCON (tcon0 and tcon1) and only set the support_lvds flag for tcon0 > > > > > > Can you give me an idea how that compatible might look like? > > > > > > tcon0: lcd-controller@1c0c000 { > > > compatible = "allwinner,sun7i-a20-tcon", "allwinner,lvds"; > > > > > > or > > > > > > tcon0: lcd-controller@1c0c000 { > > > compatible = "allwinner,sun7i-a20-tcon", "allwinner,tcon0"; > > > > > > ? Or something completely different? > > > > Something like > > > > &tcon0 { > > compatible = "allwinner,sun7i-a20-tcon0", "allwinner,sun7i-a20-tcon"; > > }; > > > > &tcon1 { > > compatible = "allwinner,sun7i-a20-tcon1", "allwinner,sun7i-a20-tcon"; > > }; > > > > Hi Maxime, here is what I came up with, please take a look. If the > approach is right, I'll split it up and include into the patch set. > > From f3e45c958a9551a52ac26435785bdb572e54d8db Mon Sep 17 00:00:00 2001 > From: Andrey Lebedev <andrey@lebedev.lt> > Date: Fri, 14 Feb 2020 23:21:59 +0200 > Subject: [PATCH] Mark tcon0 to be the only tcon capable of LVDS on sun7i-a20 > > This allows to avoid warnings about reset line not provided for tcon1. > > Signed-off-by: Andrey Lebedev <andrey@lebedev.lt> > --- > arch/arm/boot/dts/sun7i-a20.dtsi | 2 +- > drivers/gpu/drm/sun4i/sun4i_tcon.c | 22 +++++++++++++++++++++- > drivers/gpu/drm/sun4i/sun4i_tcon.h | 2 ++ > 3 files changed, 24 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi > index 3b3c366a2bee..bab59fc4d9b1 100644 > --- a/arch/arm/boot/dts/sun7i-a20.dtsi > +++ b/arch/arm/boot/dts/sun7i-a20.dtsi > @@ -405,7 +405,7 @@ > }; > > tcon0: lcd-controller@1c0c000 { > - compatible = "allwinner,sun7i-a20-tcon"; > + compatible = "allwinner,sun7i-a20-tcon0", "allwinner,sun7i-a20-tcon"; That's correct > reg = <0x01c0c000 0x1000>; > interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>; > resets = <&ccu RST_TCON0>, <&ccu RST_LVDS>; > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c > index 800a9bd86112..cb2040aec436 100644 > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c > @@ -1107,6 +1107,25 @@ static struct sunxi_engine *sun4i_tcon_find_engine(struct sun4i_drv *drv, > return sun4i_tcon_find_engine_traverse(drv, node, 0); > } > > +/* > + * Check if given tcon supports LVDS > + * > + * Some of the sunxi SoC variants contain several timing controllers, but only > + * one of them can be used to drive LVDS screen. In this case such tcon is > + * identified in respective quirks struct: lvds_compatible_tcon property will > + * hold "compatible" string of the tcon, that supports LVDS. > + * > + * If lvds_compatible_tcon is not set, all tcons are considered capable of > + * driving LVDS. > + */ > +static bool sun4i_tcon_lvds_compat(struct device *dev, struct sun4i_tcon *tcon) > +{ > + if (tcon->quirks->lvds_compatible_tcon == NULL) > + return true; > + return of_device_is_compatible(dev->of_node, > + tcon->quirks->lvds_compatible_tcon); > +} > + > static int sun4i_tcon_bind(struct device *dev, struct device *master, > void *data) > { > @@ -1161,7 +1180,7 @@ static int sun4i_tcon_bind(struct device *dev, struct device *master, > return ret; > } > > - if (tcon->quirks->supports_lvds) { > + if (tcon->quirks->supports_lvds && sun4i_tcon_lvds_compat(dev, tcon)) { > /* > * This can only be made optional since we've had DT > * nodes without the LVDS reset properties. > @@ -1481,6 +1500,7 @@ static const struct sun4i_tcon_quirks sun6i_a31s_quirks = { > > static const struct sun4i_tcon_quirks sun7i_a20_quirks = { > .supports_lvds = true, > + .lvds_compatible_tcon = "allwinner,sun7i-a20-tcon0", > .has_channel_0 = true, > .has_channel_1 = true, > .dclk_min_div = 4, > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h > index cfbf4e6c1679..bc87d28ee341 100644 > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.h > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h > @@ -235,6 +235,8 @@ struct sun4i_tcon_quirks { > bool needs_de_be_mux; /* sun6i needs mux to select backend */ > bool needs_edp_reset; /* a80 edp reset needed for tcon0 access */ > bool supports_lvds; /* Does the TCON support an LVDS output? */ > + /* "compatible" string of TCON that exclusively supports LVDS */ > + const char *lvds_compatible_tcon; However this is far more complicated than needed, you can simply add a new quirks structure associated to the tcon0 compatible in sun4i_tcon_of_table, and that will do Maxime
On Mon, Feb 17, 2020 at 06:51:35PM +0100, Maxime Ripard wrote: > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h > > index cfbf4e6c1679..bc87d28ee341 100644 > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.h > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h > > @@ -235,6 +235,8 @@ struct sun4i_tcon_quirks { > > bool needs_de_be_mux; /* sun6i needs mux to select backend */ > > bool needs_edp_reset; /* a80 edp reset needed for tcon0 access */ > > bool supports_lvds; /* Does the TCON support an LVDS output? */ > > + /* "compatible" string of TCON that exclusively supports LVDS */ > > + const char *lvds_compatible_tcon; > > However this is far more complicated than needed, you can simply add a > new quirks structure associated to the tcon0 compatible in > sun4i_tcon_of_table, and that will do Aha! Does this look good to you? From 4ad2978e404c63d4cca1b7890bc5bdd4d1e8965d Mon Sep 17 00:00:00 2001 From: Andrey Lebedev <andrey@lebedev.lt> Date: Tue, 18 Feb 2020 19:47:33 +0200 Subject: [PATCH] Mark tcon0 to be the only tcon capable of LVDS on sun7i-a20 This allows to avoid warnings about reset line not provided for tcon1. Signed-off-by: Andrey Lebedev <andrey@lebedev.lt> --- arch/arm/boot/dts/sun7i-a20.dtsi | 6 ++++-- drivers/gpu/drm/sun4i/sun4i_tcon.c | 14 ++++++++++++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi index dc4f3f249aee..d50263c1ca9a 100644 --- a/arch/arm/boot/dts/sun7i-a20.dtsi +++ b/arch/arm/boot/dts/sun7i-a20.dtsi @@ -405,7 +405,8 @@ }; tcon0: lcd-controller@1c0c000 { - compatible = "allwinner,sun7i-a20-tcon"; + compatible = "allwinner,sun7i-a20-tcon0", + "allwinner,sun7i-a20-tcon"; reg = <0x01c0c000 0x1000>; interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>; resets = <&ccu RST_TCON0>, <&ccu RST_LVDS>; @@ -460,7 +461,8 @@ }; tcon1: lcd-controller@1c0d000 { - compatible = "allwinner,sun7i-a20-tcon"; + compatible = "allwinner,sun7i-a20-tcon1", + "allwinner,sun7i-a20-tcon"; reg = <0x01c0d000 0x1000>; interrupts = <GIC_SPI 45 IRQ_TYPE_LEVEL_HIGH>; resets = <&ccu RST_TCON1>; diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index 800a9bd86112..d9605d331010 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -1479,7 +1479,7 @@ static const struct sun4i_tcon_quirks sun6i_a31s_quirks = { .dclk_min_div = 1, }; -static const struct sun4i_tcon_quirks sun7i_a20_quirks = { +static const struct sun4i_tcon_quirks sun7i_a20_tcon0_quirks = { .supports_lvds = true, .has_channel_0 = true, .has_channel_1 = true, @@ -1489,6 +1489,15 @@ static const struct sun4i_tcon_quirks sun7i_a20_quirks = { .setup_lvds_phy = sun4i_tcon_setup_lvds_phy, }; +static const struct sun4i_tcon_quirks sun7i_a20_tcon1_quirks = { + .supports_lvds = false, + .has_channel_0 = true, + .has_channel_1 = true, + .dclk_min_div = 4, + /* Same display pipeline structure as A10 */ + .set_mux = sun4i_a10_tcon_set_mux, +}; + static const struct sun4i_tcon_quirks sun8i_a33_quirks = { .has_channel_0 = true, .has_lvds_alt = true, @@ -1534,7 +1543,8 @@ const struct of_device_id sun4i_tcon_of_table[] = { { .compatible = "allwinner,sun5i-a13-tcon", .data = &sun5i_a13_quirks }, { .compatible = "allwinner,sun6i-a31-tcon", .data = &sun6i_a31_quirks }, { .compatible = "allwinner,sun6i-a31s-tcon", .data = &sun6i_a31s_quirks }, - { .compatible = "allwinner,sun7i-a20-tcon", .data = &sun7i_a20_quirks }, + { .compatible = "allwinner,sun7i-a20-tcon0", .data = &sun7i_a20_tcon0_quirks }, + { .compatible = "allwinner,sun7i-a20-tcon1", .data = &sun7i_a20_tcon1_quirks }, { .compatible = "allwinner,sun8i-a23-tcon", .data = &sun8i_a33_quirks }, { .compatible = "allwinner,sun8i-a33-tcon", .data = &sun8i_a33_quirks }, { .compatible = "allwinner,sun8i-a83t-tcon-lcd", .data = &sun8i_a83t_lcd_quirks },
On Tue, Feb 18, 2020 at 07:50:33PM +0200, Andrey Lebedev wrote: > On Mon, Feb 17, 2020 at 06:51:35PM +0100, Maxime Ripard wrote: > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h > > > index cfbf4e6c1679..bc87d28ee341 100644 > > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.h > > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h > > > @@ -235,6 +235,8 @@ struct sun4i_tcon_quirks { > > > bool needs_de_be_mux; /* sun6i needs mux to select backend */ > > > bool needs_edp_reset; /* a80 edp reset needed for tcon0 access */ > > > bool supports_lvds; /* Does the TCON support an LVDS output? */ > > > + /* "compatible" string of TCON that exclusively supports LVDS */ > > > + const char *lvds_compatible_tcon; > > > > However this is far more complicated than needed, you can simply add a > > new quirks structure associated to the tcon0 compatible in > > sun4i_tcon_of_table, and that will do > > Aha! Does this look good to you? > > From 4ad2978e404c63d4cca1b7890bc5bdd4d1e8965d Mon Sep 17 00:00:00 2001 > From: Andrey Lebedev <andrey@lebedev.lt> > Date: Tue, 18 Feb 2020 19:47:33 +0200 > Subject: [PATCH] Mark tcon0 to be the only tcon capable of LVDS on sun7i-a20 > > This allows to avoid warnings about reset line not provided for tcon1. > > Signed-off-by: Andrey Lebedev <andrey@lebedev.lt> > --- > arch/arm/boot/dts/sun7i-a20.dtsi | 6 ++++-- > drivers/gpu/drm/sun4i/sun4i_tcon.c | 14 ++++++++++++-- > 2 files changed, 16 insertions(+), 4 deletions(-) > > diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi > index dc4f3f249aee..d50263c1ca9a 100644 > --- a/arch/arm/boot/dts/sun7i-a20.dtsi > +++ b/arch/arm/boot/dts/sun7i-a20.dtsi > @@ -405,7 +405,8 @@ > }; > > tcon0: lcd-controller@1c0c000 { > - compatible = "allwinner,sun7i-a20-tcon"; > + compatible = "allwinner,sun7i-a20-tcon0", > + "allwinner,sun7i-a20-tcon"; > reg = <0x01c0c000 0x1000>; > interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>; > resets = <&ccu RST_TCON0>, <&ccu RST_LVDS>; > @@ -460,7 +461,8 @@ > }; > > tcon1: lcd-controller@1c0d000 { > - compatible = "allwinner,sun7i-a20-tcon"; > + compatible = "allwinner,sun7i-a20-tcon1", > + "allwinner,sun7i-a20-tcon"; > reg = <0x01c0d000 0x1000>; > interrupts = <GIC_SPI 45 IRQ_TYPE_LEVEL_HIGH>; > resets = <&ccu RST_TCON1>; This needs to be in a separate patch > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c > index 800a9bd86112..d9605d331010 100644 > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c > @@ -1479,7 +1479,7 @@ static const struct sun4i_tcon_quirks sun6i_a31s_quirks = { > .dclk_min_div = 1, > }; > > -static const struct sun4i_tcon_quirks sun7i_a20_quirks = { > +static const struct sun4i_tcon_quirks sun7i_a20_tcon0_quirks = { > .supports_lvds = true, > .has_channel_0 = true, > .has_channel_1 = true, > @@ -1489,6 +1489,15 @@ static const struct sun4i_tcon_quirks sun7i_a20_quirks = { > .setup_lvds_phy = sun4i_tcon_setup_lvds_phy, > }; > > +static const struct sun4i_tcon_quirks sun7i_a20_tcon1_quirks = { > + .supports_lvds = false, > + .has_channel_0 = true, > + .has_channel_1 = true, > + .dclk_min_div = 4, > + /* Same display pipeline structure as A10 */ > + .set_mux = sun4i_a10_tcon_set_mux, > +}; > + > static const struct sun4i_tcon_quirks sun8i_a33_quirks = { > .has_channel_0 = true, > .has_lvds_alt = true, > @@ -1534,7 +1543,8 @@ const struct of_device_id sun4i_tcon_of_table[] = { > { .compatible = "allwinner,sun5i-a13-tcon", .data = &sun5i_a13_quirks }, > { .compatible = "allwinner,sun6i-a31-tcon", .data = &sun6i_a31_quirks }, > { .compatible = "allwinner,sun6i-a31s-tcon", .data = &sun6i_a31s_quirks }, > - { .compatible = "allwinner,sun7i-a20-tcon", .data = &sun7i_a20_quirks }, > + { .compatible = "allwinner,sun7i-a20-tcon0", .data = &sun7i_a20_tcon0_quirks }, > + { .compatible = "allwinner,sun7i-a20-tcon1", .data = &sun7i_a20_tcon1_quirks }, > { .compatible = "allwinner,sun8i-a23-tcon", .data = &sun8i_a33_quirks }, > { .compatible = "allwinner,sun8i-a33-tcon", .data = &sun8i_a33_quirks }, > { .compatible = "allwinner,sun8i-a83t-tcon-lcd", .data = &sun8i_a83t_lcd_quirks }, And we need to support older DT as well, so you shouldn't remove the older TCON compatible and its structure. Make sure to document the new compatibles too. Maxime
diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi index 92b5be97085d..b05fdf8df32e 100644 --- a/arch/arm/boot/dts/sun7i-a20.dtsi +++ b/arch/arm/boot/dts/sun7i-a20.dtsi @@ -47,6 +47,7 @@ #include <dt-bindings/dma/sun4i-a10.h> #include <dt-bindings/clock/sun7i-a20-ccu.h> #include <dt-bindings/reset/sun4i-a10-ccu.h> +#include <dt-bindings/pinctrl/sun4i-a10.h> / { interrupt-parent = <&gic>; @@ -407,8 +408,8 @@ compatible = "allwinner,sun7i-a20-tcon"; reg = <0x01c0c000 0x1000>; interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>; - resets = <&ccu RST_TCON0>; - reset-names = "lcd"; + resets = <&ccu RST_TCON0>, <&ccu RST_LVDS>; + reset-names = "lcd", "lvds"; clocks = <&ccu CLK_AHB_LCD0>, <&ccu CLK_TCON0_CH0>, <&ccu CLK_TCON0_CH1>; @@ -444,6 +445,11 @@ #size-cells = <0>; reg = <1>; + tcon0_out_lvds: endpoint@0 { + reg = <0>; + remote-endpoint = <&lvds_in_tcon0>; + allwinner,tcon-channel = <0>; + }; tcon0_out_hdmi: endpoint@1 { reg = <1>; remote-endpoint = <&hdmi_in_tcon0>; @@ -686,6 +692,19 @@ }; }; + lvds_panel: panel@1c16500 { + compatible = "panel-lvds"; + #address-cells = <1>; + #size-cells = <0>; + status = "disabled"; + + port { + lvds_in_tcon0: endpoint { + remote-endpoint = <&tcon0_out_lvds>; + }; + }; + }; + spi2: spi@1c17000 { compatible = "allwinner,sun4i-a10-spi"; reg = <0x01c17000 0x1000>; @@ -872,7 +891,7 @@ gmac_rgmii_pins: gmac-rgmii-pins { pins = "PA0", "PA1", "PA2", "PA3", "PA4", "PA5", "PA6", - "PA7", "PA8", "PA10", + "PA7", "PA8", "PA10", "PA11", "PA12", "PA13", "PA15", "PA16"; function = "gmac"; @@ -1162,6 +1181,26 @@ pins = "PI20", "PI21"; function = "uart7"; }; + + /omit-if-no-ref/ + lcd_lvds0_pins: lcd_lvds0_pins { + allwinner,pins = + "PD0", "PD1", "PD2", "PD3", "PD4", + "PD5", "PD6", "PD7", "PD8", "PD9"; + allwinner,function = "lvds0"; + allwinner,drive = <SUN4I_PINCTRL_10_MA>; + allwinner,pull = <SUN4I_PINCTRL_NO_PULL>; + }; + + /omit-if-no-ref/ + lcd_lvds1_pins: lcd_lvds1_pins { + allwinner,pins = + "PD10", "PD11", "PD12", "PD13", "PD14", + "PD15", "PD16", "PD17", "PD18", "PD19"; + allwinner,function = "lvds1"; + allwinner,drive = <SUN4I_PINCTRL_10_MA>; + allwinner,pull = <SUN4I_PINCTRL_NO_PULL>; + }; }; timer@1c20c00 {