[v2,7/7] arm64: dts: allwinner: a64: enable ANX6345 bridge on Teres-I
diff mbox series

Message ID 20190604122308.98D4868B20@newverein.lst.de
State New
Headers show
Series
  • Add anx6345 DP/eDP bridge for Olimex Teres-I
Related show

Commit Message

Torsten Duwe June 4, 2019, 12:23 p.m. UTC
Teres-I has an anx6345 bridge connected to the RGB666 LCD output, and
the I2C controlling signals are connected to I2C0 bus. eDP output goes
to an Innolux N116BGE panel.

Enable it in the device tree.

Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
Signed-off-by: Torsten Duwe <duwe@suse.de>
---
 .../boot/dts/allwinner/sun50i-a64-teres-i.dts      | 65 ++++++++++++++++++++--
 1 file changed, 61 insertions(+), 4 deletions(-)

Comments

Vasily Khoruzhick June 4, 2019, 3:08 p.m. UTC | #1
On Tue, Jun 4, 2019 at 5:23 AM Torsten Duwe <duwe@lst.de> wrote:
>
> Teres-I has an anx6345 bridge connected to the RGB666 LCD output, and
> the I2C controlling signals are connected to I2C0 bus. eDP output goes
> to an Innolux N116BGE panel.
>
> Enable it in the device tree.
>
> Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> Signed-off-by: Torsten Duwe <duwe@suse.de>
> ---
>  .../boot/dts/allwinner/sun50i-a64-teres-i.dts      | 65 ++++++++++++++++++++--
>  1 file changed, 61 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> index 0ec46b969a75..a0ad438b037f 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> @@ -65,6 +65,21 @@
>                 };
>         };
>
> +       panel: panel {
> +               compatible ="innolux,n116bge", "simple-panel";

It's still "simple-panel". I believe I already mentioned that Rob
asked it to be edp-connector.

> +               status = "okay";
> +               power-supply = <&reg_dcdc1>;
> +               backlight = <&backlight>;
> +
> +               ports {
> +                       panel_in: port {
> +                               panel_in_edp: endpoint {
> +                                       remote-endpoint = <&anx6345_out>;
> +                               };
> +                       };
> +               };
> +       };
> +
>         reg_usb1_vbus: usb1-vbus {
>                 compatible = "regulator-fixed";
>                 regulator-name = "usb1-vbus";
> @@ -81,20 +96,48 @@
>         };
>  };
>
> +&de {
> +       status = "okay";
> +};
> +
>  &ehci1 {
>         status = "okay";
>  };
>
>
> -/* The ANX6345 eDP-bridge is on i2c0. There is no linux (mainline)
> - * driver for this chip at the moment, the bootloader initializes it.
> - * However it can be accessed with the i2c-dev driver from user space.
> - */
>  &i2c0 {
>         clock-frequency = <100000>;
>         pinctrl-names = "default";
>         pinctrl-0 = <&i2c0_pins>;
>         status = "okay";
> +
> +       anx6345: anx6345@38 {
> +               compatible = "analogix,anx6345";
> +               reg = <0x38>;
> +               reset-gpios = <&pio 3 24 GPIO_ACTIVE_LOW>; /* PD24 */
> +               dvdd25-supply = <&reg_dldo2>;
> +               dvdd12-supply = <&reg_dldo3>;
> +
> +               ports {
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +
> +                       port@0 {
> +                               anx6345_in: endpoint {
> +                                       remote-endpoint = <&tcon0_out_anx6345>;
> +                               };
> +                       };
> +                       port@1 {
> +                               anx6345_out: endpoint {
> +                                       remote-endpoint = <&panel_in_edp>;
> +                               };
> +                       };
> +               };
> +       };
> +};
> +
> +&mixer0 {
> +       status = "okay";
>  };
>
>  &mmc0 {
> @@ -279,6 +322,20 @@
>         vcc-hdmi-supply = <&reg_dldo1>;
>  };
>
> +&tcon0 {
> +       pinctrl-names = "default";
> +       pinctrl-0 = <&lcd_rgb666_pins>;
> +
> +       status = "okay";
> +};
> +
> +&tcon0_out {
> +       tcon0_out_anx6345: endpoint@0 {
> +               reg = <0>;
> +               remote-endpoint = <&anx6345_in>;
> +       };
> +};
> +
>  &uart0 {
>         pinctrl-names = "default";
>         pinctrl-0 = <&uart0_pb_pins>;
> --
> 2.16.4
>
Torsten Duwe June 5, 2019, 10:13 a.m. UTC | #2
On Tue, Jun 04, 2019 at 08:08:40AM -0700, Vasily Khoruzhick wrote:
> On Tue, Jun 4, 2019 at 5:23 AM Torsten Duwe <duwe@lst.de> wrote:
> >
> > Teres-I has an anx6345 bridge connected to the RGB666 LCD output, and
> > the I2C controlling signals are connected to I2C0 bus. eDP output goes
> > to an Innolux N116BGE panel.
> >
> > Enable it in the device tree.
> >
> > Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> > Signed-off-by: Torsten Duwe <duwe@suse.de>
> > ---
> >  .../boot/dts/allwinner/sun50i-a64-teres-i.dts      | 65 ++++++++++++++++++++--
> >  1 file changed, 61 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> > index 0ec46b969a75..a0ad438b037f 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> > @@ -65,6 +65,21 @@
> >                 };
> >         };
> >
> > +       panel: panel {
> > +               compatible ="innolux,n116bge", "simple-panel";
> 
> It's still "simple-panel". I believe I already mentioned that Rob
> asked it to be edp-connector.
> 
For which there are neither bindings nor drivers.

Is anybody seriously proposing to hold back support for existing
(open source!) hardware in favour of an *imaginable* *possibly* better
solution? Especially when this exact line is already used in some other places?
(there's a space missing btw...)

I'm more than glad to follow any constructive improvements towards better
modularity. However there were none so far, and on top of that, it's a laptop.
I see little advantage in mentioning an internal connector when the panel
connected is always the same.

FWIW, Rob should also have received these patches.

	Torsten
Maxime Ripard June 5, 2019, 12:02 p.m. UTC | #3
On Wed, Jun 05, 2019 at 12:13:17PM +0200, Torsten Duwe wrote:
> On Tue, Jun 04, 2019 at 08:08:40AM -0700, Vasily Khoruzhick wrote:
> > On Tue, Jun 4, 2019 at 5:23 AM Torsten Duwe <duwe@lst.de> wrote:
> > >
> > > Teres-I has an anx6345 bridge connected to the RGB666 LCD output, and
> > > the I2C controlling signals are connected to I2C0 bus. eDP output goes
> > > to an Innolux N116BGE panel.
> > >
> > > Enable it in the device tree.
> > >
> > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> > > Signed-off-by: Torsten Duwe <duwe@suse.de>
> > > ---
> > >  .../boot/dts/allwinner/sun50i-a64-teres-i.dts      | 65 ++++++++++++++++++++--
> > >  1 file changed, 61 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> > > index 0ec46b969a75..a0ad438b037f 100644
> > > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> > > @@ -65,6 +65,21 @@
> > >                 };
> > >         };
> > >
> > > +       panel: panel {
> > > +               compatible ="innolux,n116bge", "simple-panel";
> >
> > It's still "simple-panel". I believe I already mentioned that Rob
> > asked it to be edp-connector.
>
> For which there are neither bindings nor drivers.
>
> Is anybody seriously proposing to hold back support for existing
> (open source!) hardware in favour of an *imaginable* *possibly*
> better solution? Especially when this exact line is already used in
> some other places?  (there's a space missing btw...)

It's non-existent and imaginable only because you've been ignoring
everyone that said that you should do it. So it's self-inflicted,
really.

And the DT is considered an ABI, so yeah, we will witheld everything
that doesn't fit what we would like. Your point of view is that it's
more work and for no particular benefit, ours is that it's a
short-term pain for a long-term gain, and the benefits will be in the
maintainance cost.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Harald Geyer June 6, 2019, 1:59 p.m. UTC | #4
Guys, this discussion is getting heated for no reason. Let's put
personal frustrations aside and discuss the issue on its merits:

Maxime Ripard writes:
> On Wed, Jun 05, 2019 at 12:13:17PM +0200, Torsten Duwe wrote:
> > On Tue, Jun 04, 2019 at 08:08:40AM -0700, Vasily Khoruzhick wrote:
> > > On Tue, Jun 4, 2019 at 5:23 AM Torsten Duwe <duwe@lst.de> wrote:
> > > >
> > > > Teres-I has an anx6345 bridge connected to the RGB666 LCD output, and
> > > > the I2C controlling signals are connected to I2C0 bus. eDP output goes
> > > > to an Innolux N116BGE panel.
> > > >
> > > > Enable it in the device tree.
> > > >
> > > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> > > > Signed-off-by: Torsten Duwe <duwe@suse.de>
> > > > ---
> > > >  .../boot/dts/allwinner/sun50i-a64-teres-i.dts      | 65 ++++++++++++++++++++--
> > > >  1 file changed, 61 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> > > > index 0ec46b969a75..a0ad438b037f 100644
> > > > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> > > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> > > > @@ -65,6 +65,21 @@
> > > >                 };
> > > >         };
> > > >
> > > > +       panel: panel {
> > > > +               compatible ="innolux,n116bge", "simple-panel";
> > >
> > > It's still "simple-panel". I believe I already mentioned that Rob
> > > asked it to be edp-connector.

Actually just dropping the "simple-panel" compatible would be a poor
choice. Even if "edp-connector" is specified as binding and implemented in a
driver, there are older kernel versions and other operating systems to
keep in mind. If the HW works with "simple-panel" driver satisfactorily,
we should definitely keep the compatible as a fall back for cases where
the edp-connector driver is unavailable.

If think valid compatible properties would be:
compatible = "innolux,n116bge", "simple-panel";
compatible = "edp-connector", "simple-panel";
compatible = "innolux,n116bge", "edp-connector", "simple-panel";
compatible = "edp-connector", "innolux,n116bge", "simple-panel";

I can't make up my mind which one I prefere. However neither of these
variants requires actually implmenting an edp-connector driver. And each
of these variants is clearly preferable to shipping DTs without
description of the panel at all and complies with bindings after adding
a stub for "edp-connector".

> And the DT is considered an ABI, so yeah, we will witheld everything
> that doesn't fit what we would like.

I fail to see how the patch in discussion adds new ABI. While I understand
the need to pester contributors for more work, outright blocking DTs, that
properly describe the HW and comply with existing bindings, seems a
bit unreasonable. (Assuming there are no other issues of course.)

Also note that the innolux,n116bge binding suggestes using simple-panel
as fallback.

HTH,
Harald
Maxime Ripard June 7, 2019, 6:28 a.m. UTC | #5
On Thu, Jun 06, 2019 at 03:59:27PM +0200, Harald Geyer wrote:
> Guys, this discussion is getting heated for no reason. Let's put
> personal frustrations aside and discuss the issue on its merits:
>
> Maxime Ripard writes:
> > On Wed, Jun 05, 2019 at 12:13:17PM +0200, Torsten Duwe wrote:
> > > On Tue, Jun 04, 2019 at 08:08:40AM -0700, Vasily Khoruzhick wrote:
> > > > On Tue, Jun 4, 2019 at 5:23 AM Torsten Duwe <duwe@lst.de> wrote:
> > > > >
> > > > > Teres-I has an anx6345 bridge connected to the RGB666 LCD output, and
> > > > > the I2C controlling signals are connected to I2C0 bus. eDP output goes
> > > > > to an Innolux N116BGE panel.
> > > > >
> > > > > Enable it in the device tree.
> > > > >
> > > > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io>
> > > > > Signed-off-by: Torsten Duwe <duwe@suse.de>
> > > > > ---
> > > > >  .../boot/dts/allwinner/sun50i-a64-teres-i.dts      | 65 ++++++++++++++++++++--
> > > > >  1 file changed, 61 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> > > > > index 0ec46b969a75..a0ad438b037f 100644
> > > > > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> > > > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
> > > > > @@ -65,6 +65,21 @@
> > > > >                 };
> > > > >         };
> > > > >
> > > > > +       panel: panel {
> > > > > +               compatible ="innolux,n116bge", "simple-panel";
> > > >
> > > > It's still "simple-panel". I believe I already mentioned that Rob
> > > > asked it to be edp-connector.
>
> Actually just dropping the "simple-panel" compatible would be a poor
> choice. Even if "edp-connector" is specified as binding and implemented in a
> driver, there are older kernel versions and other operating systems to
> keep in mind.

Which older kernels? This is a new binding, adding a new driver, so if
an older kernel uses a separate driver with its own binding, good for
them, but we don't have to support it.

> If the HW works with "simple-panel" driver satisfactorily,
> we should definitely keep the compatible as a fall back for cases where
> the edp-connector driver is unavailable.
>
> If think valid compatible properties would be:
> compatible = "innolux,n116bge", "simple-panel";
> compatible = "edp-connector", "simple-panel";

A connector isn't a panel.

> compatible = "innolux,n116bge", "edp-connector", "simple-panel";

And the innolux,n116bge is certainly not a connector either.

> compatible = "edp-connector", "innolux,n116bge", "simple-panel";
>
> I can't make up my mind which one I prefere. However neither of these
> variants requires actually implmenting an edp-connector driver.

No-one asked to do an edp-connector driver. You should use it in your
DT, but if you want to have some code in your driver that parses the
DT directly, I'm totally fine with that.

> And each of these variants is clearly preferable to shipping DTs
> without description of the panel at all and complies with bindings
> after adding a stub for "edp-connector".

I guess you should describe why do you think it's "clear", because I'm
not sure this is obvious for everyone here. eDP allows to discover
which device is on the other side and its supported timings, just like
HDMI for example (or regular DP, for that matter). Would you think
it's clearly preferable to ship a DT with the DP/HDMI monitor
connected on the other side exposed as a panel as well?

> > And the DT is considered an ABI, so yeah, we will witheld everything
> > that doesn't fit what we would like.
>
> I fail to see how the patch in discussion adds new ABI.

The binding itself is the ABI, and we will have to support that
binding for pretty much forever.

> While I understand the need to pester contributors for more work,
> outright blocking DTs, that properly describe the HW

Properly is arguable.

> and comply with existing bindings

And that's bindings meant for another use-case.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Torsten Duwe June 7, 2019, 9:40 a.m. UTC | #6
On Fri, Jun 07, 2019 at 08:28:02AM +0200, Maxime Ripard wrote:
> On Thu, Jun 06, 2019 at 03:59:27PM +0200, Harald Geyer wrote:
> >
> > If think valid compatible properties would be:
> > compatible = "innolux,n116bge", "simple-panel";
> > compatible = "edp-connector", "simple-panel";
> 
> A connector isn't a panel.
> 
> > compatible = "innolux,n116bge", "edp-connector", "simple-panel";
> 
> And the innolux,n116bge is certainly not a connector either.
> 
> > compatible = "edp-connector", "innolux,n116bge", "simple-panel";
> >
> > I can't make up my mind which one I prefere. However neither of these
> > variants requires actually implmenting an edp-connector driver.
> 
> No-one asked to do an edp-connector driver. You should use it in your
> DT, but if you want to have some code in your driver that parses the
> DT directly, I'm totally fine with that.

I must admit I fail to understand what that extra node would be good for.
Logically, the eDP far side is connected to the well-known n116bge.
Inside the laptop case it might as well be a flat ribbon cable or
soldered directly.
In good intention, that's all I wanted to express in the DT. I don't
know whether the relevant mechanical dimensions of the panel and the
connector are standardised, so whether one could in theory assemble it
with a different panel than the one it came with.

OTOH, as I checked during the discussion with anarsoul, the panel's
supply voltage is permanently connected to the main 3.3V rail.
We already agreed that the eDP output port must not neccessarily be
specified, this setup is a good example why: because the panel is
always powered, the anx6345 can always pull valid EDID data from it
so at this stage there's no need for any OS driver to reach beyond
the bridge. IIRC even the backlight got switched off for the blank
screen without.

All I wanted to say is that "there's usually an n116bge behind it";
but this is mostly redundant.

So, shall we just drop the output port specification (along with the
panel node) in order to get one step further?

> I guess you should describe why do you think it's "clear", because I'm
> not sure this is obvious for everyone here. eDP allows to discover
> which device is on the other side and its supported timings, just like
> HDMI for example (or regular DP, for that matter). Would you think
> it's clearly preferable to ship a DT with the DP/HDMI monitor
> connected on the other side exposed as a panel as well?

Well, as I wrote: "in good intention". That's the panel that comes with
the kit but it is very well detected automatically, just like you describe.

So, just leave it out?

	Torsten
Andrzej Hajda June 12, 2019, 10 a.m. UTC | #7
On 07.06.2019 11:40, Torsten Duwe wrote:
> On Fri, Jun 07, 2019 at 08:28:02AM +0200, Maxime Ripard wrote:
>> On Thu, Jun 06, 2019 at 03:59:27PM +0200, Harald Geyer wrote:
>>> If think valid compatible properties would be:
>>> compatible = "innolux,n116bge", "simple-panel";
>>> compatible = "edp-connector", "simple-panel";
>> A connector isn't a panel.
>>
>>> compatible = "innolux,n116bge", "edp-connector", "simple-panel";
>> And the innolux,n116bge is certainly not a connector either.
>>
>>> compatible = "edp-connector", "innolux,n116bge", "simple-panel";
>>>
>>> I can't make up my mind which one I prefere. However neither of these
>>> variants requires actually implmenting an edp-connector driver.
>> No-one asked to do an edp-connector driver. You should use it in your
>> DT, but if you want to have some code in your driver that parses the
>> DT directly, I'm totally fine with that.
> I must admit I fail to understand what that extra node would be good for.
> Logically, the eDP far side is connected to the well-known n116bge.
> Inside the laptop case it might as well be a flat ribbon cable or
> soldered directly.
> In good intention, that's all I wanted to express in the DT. I don't
> know whether the relevant mechanical dimensions of the panel and the
> connector are standardised, so whether one could in theory assemble it
> with a different panel than the one it came with.
>
> OTOH, as I checked during the discussion with anarsoul, the panel's
> supply voltage is permanently connected to the main 3.3V rail.
> We already agreed that the eDP output port must not neccessarily be
> specified, this setup is a good example why: because the panel is
> always powered, the anx6345 can always pull valid EDID data from it
> so at this stage there's no need for any OS driver to reach beyond
> the bridge. IIRC even the backlight got switched off for the blank
> screen without.
>
> All I wanted to say is that "there's usually an n116bge behind it";
> but this is mostly redundant.
>
> So, shall we just drop the output port specification (along with the
> panel node) in order to get one step further?


I am not sure if I understand whole discussion here, but I also do not
understand whole edp-connector thing.

According to VESA[1] eDP is "Internal display interface" - there is no
external connector for eDP, the way it is connected is integrator's
decision, but it is fixed - ie end user do not plug/unplug it.

If I remember correctly in some boards eDP is connected to some DP
connector (odroid xu3 if I remember correctly), but this is non-standard
hack, and for this case in bindings there should be rather dp-connector
not edp-connector.

[1]:
https://www.vesa.org/wp-content/uploads/2010/12/DisplayPort-DevCon-Presentation-eDP-Dec-2010-v3.pdf


Regards

Andrzej


>
>> I guess you should describe why do you think it's "clear", because I'm
>> not sure this is obvious for everyone here. eDP allows to discover
>> which device is on the other side and its supported timings, just like
>> HDMI for example (or regular DP, for that matter). Would you think
>> it's clearly preferable to ship a DT with the DP/HDMI monitor
>> connected on the other side exposed as a panel as well?
> Well, as I wrote: "in good intention". That's the panel that comes with
> the kit but it is very well detected automatically, just like you describe.
>
> So, just leave it out?
>
> 	Torsten
>
>
>
Maxime Ripard June 12, 2019, 3:20 p.m. UTC | #8
Hi,

On Wed, Jun 12, 2019 at 12:00:21PM +0200, Andrzej Hajda wrote:
> On 07.06.2019 11:40, Torsten Duwe wrote:
> > On Fri, Jun 07, 2019 at 08:28:02AM +0200, Maxime Ripard wrote:
> >> On Thu, Jun 06, 2019 at 03:59:27PM +0200, Harald Geyer wrote:
> >>> If think valid compatible properties would be:
> >>> compatible = "innolux,n116bge", "simple-panel";
> >>> compatible = "edp-connector", "simple-panel";
> >> A connector isn't a panel.
> >>
> >>> compatible = "innolux,n116bge", "edp-connector", "simple-panel";
> >> And the innolux,n116bge is certainly not a connector either.
> >>
> >>> compatible = "edp-connector", "innolux,n116bge", "simple-panel";
> >>>
> >>> I can't make up my mind which one I prefere. However neither of these
> >>> variants requires actually implmenting an edp-connector driver.
> >> No-one asked to do an edp-connector driver. You should use it in your
> >> DT, but if you want to have some code in your driver that parses the
> >> DT directly, I'm totally fine with that.
> > I must admit I fail to understand what that extra node would be good for.
> > Logically, the eDP far side is connected to the well-known n116bge.
> > Inside the laptop case it might as well be a flat ribbon cable or
> > soldered directly.
> > In good intention, that's all I wanted to express in the DT. I don't
> > know whether the relevant mechanical dimensions of the panel and the
> > connector are standardised, so whether one could in theory assemble it
> > with a different panel than the one it came with.
> >
> > OTOH, as I checked during the discussion with anarsoul, the panel's
> > supply voltage is permanently connected to the main 3.3V rail.
> > We already agreed that the eDP output port must not neccessarily be
> > specified, this setup is a good example why: because the panel is
> > always powered, the anx6345 can always pull valid EDID data from it
> > so at this stage there's no need for any OS driver to reach beyond
> > the bridge. IIRC even the backlight got switched off for the blank
> > screen without.
> >
> > All I wanted to say is that "there's usually an n116bge behind it";
> > but this is mostly redundant.
> >
> > So, shall we just drop the output port specification (along with the
> > panel node) in order to get one step further?
>
> I am not sure if I understand whole discussion here, but I also do not
> understand whole edp-connector thing.

The context is this one:
https://patchwork.freedesktop.org/patch/257352/?series=51182&rev=1
https://patchwork.freedesktop.org/patch/283012/?series=56163&rev=1
https://patchwork.freedesktop.org/patch/286468/?series=56776&rev=2

TL;DR: This bridge is being used on ARM laptops that can come with
different eDP panels. Some of these panels require a regulator to be
enabled for the panel to work, and this is obviously something that
should be in the DT.

However, we can't really describe the panel itself, since the vendor
uses several of them and just relies on the eDP bus to do its job at
retrieving the EDIDs. A generic panel isn't really working either
since that would mean having a generic behaviour for all the panels
connected to that bus, which isn't there either.

The connector allows to expose this nicely.

> According to VESA[1] eDP is "Internal display interface" - there is no
> external connector for eDP, the way it is connected is integrator's
> decision, but it is fixed - ie end user do not plug/unplug it.

I'm not sure if you mean DRM or DT connector here though. In DRM,
we're doing this all the time for panels. I'm literaly typing this
from a laptop that has a screen with an eDP connector.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Maxime Ripard June 12, 2019, 3:34 p.m. UTC | #9
On Fri, Jun 07, 2019 at 11:40:30AM +0200, Torsten Duwe wrote:
> On Fri, Jun 07, 2019 at 08:28:02AM +0200, Maxime Ripard wrote:
> > On Thu, Jun 06, 2019 at 03:59:27PM +0200, Harald Geyer wrote:
> > >
> > > If think valid compatible properties would be:
> > > compatible = "innolux,n116bge", "simple-panel";
> > > compatible = "edp-connector", "simple-panel";
> >
> > A connector isn't a panel.
> >
> > > compatible = "innolux,n116bge", "edp-connector", "simple-panel";
> >
> > And the innolux,n116bge is certainly not a connector either.
> >
> > > compatible = "edp-connector", "innolux,n116bge", "simple-panel";
> > >
> > > I can't make up my mind which one I prefere. However neither of these
> > > variants requires actually implmenting an edp-connector driver.
> >
> > No-one asked to do an edp-connector driver. You should use it in your
> > DT, but if you want to have some code in your driver that parses the
> > DT directly, I'm totally fine with that.
>
> I must admit I fail to understand what that extra node would be good for.
> Logically, the eDP far side is connected to the well-known n116bge.
> Inside the laptop case it might as well be a flat ribbon cable or
> soldered directly.
> In good intention, that's all I wanted to express in the DT. I don't
> know whether the relevant mechanical dimensions of the panel and the
> connector are standardised, so whether one could in theory assemble it
> with a different panel than the one it came with.

Because the panel that comes with the Teres-I is always the
same. However, that's not true for all the devices out there using the
bridge, starting with the pinebook.

> OTOH, as I checked during the discussion with anarsoul, the panel's
> supply voltage is permanently connected to the main 3.3V rail.

Again, that may be the case on the Teres-I, but not necessarily on
other boards.

> We already agreed that the eDP output port must not neccessarily be
> specified, this setup is a good example why: because the panel is
> always powered, the anx6345 can always pull valid EDID data from it
> so at this stage there's no need for any OS driver to reach beyond
> the bridge. IIRC even the backlight got switched off for the blank
> screen without.

That's not really the outcome of the discussion we had here though:
https://patchwork.freedesktop.org/patch/305035/

> All I wanted to say is that "there's usually an n116bge behind it";
> but this is mostly redundant.
>
> So, shall we just drop the output port specification (along with the
> panel node) in order to get one step further?

Depending on the outcome of the discussion above, yes or no :)

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Andrzej Hajda June 28, 2019, 10:39 a.m. UTC | #10
Hi Maxime,

It seems I have missed your response.

On 12.06.2019 17:20, Maxime Ripard wrote:
>> I am not sure if I understand whole discussion here, but I also do not
>> understand whole edp-connector thing.
> The context is this one:
> https://patchwork.freedesktop.org/patch/257352/?series=51182&rev=1
> https://patchwork.freedesktop.org/patch/283012/?series=56163&rev=1
> https://patchwork.freedesktop.org/patch/286468/?series=56776&rev=2
>
> TL;DR: This bridge is being used on ARM laptops that can come with
> different eDP panels. Some of these panels require a regulator to be
> enabled for the panel to work, and this is obviously something that
> should be in the DT.
>
> However, we can't really describe the panel itself, since the vendor
> uses several of them and just relies on the eDP bus to do its job at
> retrieving the EDIDs. A generic panel isn't really working either
> since that would mean having a generic behaviour for all the panels
> connected to that bus, which isn't there either.
>
> The connector allows to expose this nicely.


As VESA presentation says[1] eDP is based on DP but is much more
flexible, it is up to integrator (!!!) how the connection, power
up/down, initialization sequence should be performed. Trying to cover
every such case in edp-connector seems to me similar to panel-simple
attempt failure. Moreover there is no such thing as physical standard
eDP connector. Till now I though DT connector should describe physical
connector on the device, now I am lost, are there some DT bindings
guidelines about definition of a connector?

Maybe instead of edp-connector one would introduce integrator's specific
connector, for example with compatible "olimex,teres-edp-connector"
which should follow edp abstract connector rules? This will be at least
consistent with below presentation[1] - eDP requirements depends on
integrator. Then if olimex has standard way of dealing with panels
present in olimex/teres platforms the driver would then create
drm_panel/drm_connector/drm_bridge(?) according to these rules, I guess.
Anyway it still looks fishy for me :), maybe because I am not
familiarized with details of these platforms.

[1]: https://www.vesa.org/wp-content/uploads/2010/12/DisplayPort-DevCon-Presentation-eDP-Dec-2010-v3.pdf


>
>> According to VESA[1] eDP is "Internal display interface" - there is no
>> external connector for eDP, the way it is connected is integrator's
>> decision, but it is fixed - ie end user do not plug/unplug it.
> I'm not sure if you mean DRM or DT connector here though. In DRM,
> we're doing this all the time for panels. I'm literaly typing this
> from a laptop that has a screen with an eDP connector.


VESA describes only hardware, but since DT also describes hardware I
guess it should be similar.


Regards

Andrzej




>
> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Maxime Ripard July 1, 2019, 9:58 a.m. UTC | #11
Hi!

On Fri, Jun 28, 2019 at 12:39:32PM +0200, Andrzej Hajda wrote:
> On 12.06.2019 17:20, Maxime Ripard wrote:
> >> I am not sure if I understand whole discussion here, but I also do not
> >> understand whole edp-connector thing.
> > The context is this one:
> > https://patchwork.freedesktop.org/patch/257352/?series=51182&rev=1
> > https://patchwork.freedesktop.org/patch/283012/?series=56163&rev=1
> > https://patchwork.freedesktop.org/patch/286468/?series=56776&rev=2
> >
> > TL;DR: This bridge is being used on ARM laptops that can come with
> > different eDP panels. Some of these panels require a regulator to be
> > enabled for the panel to work, and this is obviously something that
> > should be in the DT.
> >
> > However, we can't really describe the panel itself, since the vendor
> > uses several of them and just relies on the eDP bus to do its job at
> > retrieving the EDIDs. A generic panel isn't really working either
> > since that would mean having a generic behaviour for all the panels
> > connected to that bus, which isn't there either.
> >
> > The connector allows to expose this nicely.
>
> As VESA presentation says[1] eDP is based on DP but is much more
> flexible, it is up to integrator (!!!) how the connection, power
> up/down, initialization sequence should be performed. Trying to cover
> every such case in edp-connector seems to me similar to panel-simple
> attempt failure. Moreover there is no such thing as physical standard
> eDP connector. Till now I though DT connector should describe physical
> connector on the device, now I am lost, are there some DT bindings
> guidelines about definition of a connector?

This might be semantics but I guess we're in some kind of grey area?

Like, for eDP, if it's soldered I guess we could say that there's no
connector. But what happens if for some other board, that signal is
routed through a ribbon?

You could argue that there's no physical connector in both cases, or
that there's one in both, or one for the ribbon and no connector for
the one soldered in.

> Maybe instead of edp-connector one would introduce integrator's specific
> connector, for example with compatible "olimex,teres-edp-connector"
> which should follow edp abstract connector rules? This will be at least
> consistent with below presentation[1] - eDP requirements depends on
> integrator. Then if olimex has standard way of dealing with panels
> present in olimex/teres platforms the driver would then create
> drm_panel/drm_connector/drm_bridge(?) according to these rules, I guess.
> Anyway it still looks fishy for me :), maybe because I am not
> familiarized with details of these platforms.

That makes sense yes

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Andrzej Hajda July 1, 2019, 12:27 p.m. UTC | #12
On 01.07.2019 11:58, Maxime Ripard wrote:
> Hi!
>
> On Fri, Jun 28, 2019 at 12:39:32PM +0200, Andrzej Hajda wrote:
>> On 12.06.2019 17:20, Maxime Ripard wrote:
>>>> I am not sure if I understand whole discussion here, but I also do not
>>>> understand whole edp-connector thing.
>>> The context is this one:
>>> https://patchwork.freedesktop.org/patch/257352/?series=51182&rev=1
>>> https://patchwork.freedesktop.org/patch/283012/?series=56163&rev=1
>>> https://patchwork.freedesktop.org/patch/286468/?series=56776&rev=2
>>>
>>> TL;DR: This bridge is being used on ARM laptops that can come with
>>> different eDP panels. Some of these panels require a regulator to be
>>> enabled for the panel to work, and this is obviously something that
>>> should be in the DT.
>>>
>>> However, we can't really describe the panel itself, since the vendor
>>> uses several of them and just relies on the eDP bus to do its job at
>>> retrieving the EDIDs. A generic panel isn't really working either
>>> since that would mean having a generic behaviour for all the panels
>>> connected to that bus, which isn't there either.
>>>
>>> The connector allows to expose this nicely.
>> As VESA presentation says[1] eDP is based on DP but is much more
>> flexible, it is up to integrator (!!!) how the connection, power
>> up/down, initialization sequence should be performed. Trying to cover
>> every such case in edp-connector seems to me similar to panel-simple
>> attempt failure. Moreover there is no such thing as physical standard
>> eDP connector. Till now I though DT connector should describe physical
>> connector on the device, now I am lost, are there some DT bindings
>> guidelines about definition of a connector?
> This might be semantics but I guess we're in some kind of grey area?
>
> Like, for eDP, if it's soldered I guess we could say that there's no
> connector. But what happens if for some other board, that signal is
> routed through a ribbon?
>
> You could argue that there's no physical connector in both cases, or
> that there's one in both, or one for the ribbon and no connector for
> the one soldered in.


This is not about ribbon vs soldering. It is about usage: this
connection is static across the whole life of the device (except
exceptional things: repair, non-standard usage, etc).

And "the real connector" is (at least for me) something where end-user
can connect/disconnect different things: USB, HDMI, ethernet, etc. And
obviously to be functional it should be somehow standardized. So even if
there could be some grey area, I do not see it here.


>
>> Maybe instead of edp-connector one would introduce integrator's specific
>> connector, for example with compatible "olimex,teres-edp-connector"
>> which should follow edp abstract connector rules? This will be at least
>> consistent with below presentation[1] - eDP requirements depends on
>> integrator. Then if olimex has standard way of dealing with panels
>> present in olimex/teres platforms the driver would then create
>> drm_panel/drm_connector/drm_bridge(?) according to these rules, I guess.
>> Anyway it still looks fishy for me :), maybe because I am not
>> familiarized with details of these platforms.
> That makes sense yes


And what if some panel can be used with this pseudo-connecter and in
some different hw directly? Code duplication? DT overlays?


Regards

Andrzej


>
> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Maxime Ripard July 2, 2019, 8:13 a.m. UTC | #13
On Mon, Jul 01, 2019 at 02:27:51PM +0200, Andrzej Hajda wrote:
> On 01.07.2019 11:58, Maxime Ripard wrote:
> > On Fri, Jun 28, 2019 at 12:39:32PM +0200, Andrzej Hajda wrote:
> >> On 12.06.2019 17:20, Maxime Ripard wrote:
> >>>> I am not sure if I understand whole discussion here, but I also do not
> >>>> understand whole edp-connector thing.
> >>> The context is this one:
> >>> https://patchwork.freedesktop.org/patch/257352/?series=51182&rev=1
> >>> https://patchwork.freedesktop.org/patch/283012/?series=56163&rev=1
> >>> https://patchwork.freedesktop.org/patch/286468/?series=56776&rev=2
> >>>
> >>> TL;DR: This bridge is being used on ARM laptops that can come with
> >>> different eDP panels. Some of these panels require a regulator to be
> >>> enabled for the panel to work, and this is obviously something that
> >>> should be in the DT.
> >>>
> >>> However, we can't really describe the panel itself, since the vendor
> >>> uses several of them and just relies on the eDP bus to do its job at
> >>> retrieving the EDIDs. A generic panel isn't really working either
> >>> since that would mean having a generic behaviour for all the panels
> >>> connected to that bus, which isn't there either.
> >>>
> >>> The connector allows to expose this nicely.
> >> As VESA presentation says[1] eDP is based on DP but is much more
> >> flexible, it is up to integrator (!!!) how the connection, power
> >> up/down, initialization sequence should be performed. Trying to cover
> >> every such case in edp-connector seems to me similar to panel-simple
> >> attempt failure. Moreover there is no such thing as physical standard
> >> eDP connector. Till now I though DT connector should describe physical
> >> connector on the device, now I am lost, are there some DT bindings
> >> guidelines about definition of a connector?
> > This might be semantics but I guess we're in some kind of grey area?
> >
> > Like, for eDP, if it's soldered I guess we could say that there's no
> > connector. But what happens if for some other board, that signal is
> > routed through a ribbon?
> >
> > You could argue that there's no physical connector in both cases, or
> > that there's one in both, or one for the ribbon and no connector for
> > the one soldered in.
>
> This is not about ribbon vs soldering. It is about usage: this
> connection is static across the whole life of the device (except
> exceptional things: repair, non-standard usage, etc).

It doesn't have to be.

> And "the real connector" is (at least for me) something where
> end-user can connect/disconnect different things: USB, HDMI,
> ethernet, etc. And obviously to be functional it should be somehow
> standardized. So even if there could be some grey area, I do not see
> it here.

Well, if there's a ribbon connector, then you have a physical
connector, with the end user being able to connect / disconnect
various displays. It might not be the case with actual products, but
it's pretty common with SBCs to have that signal routed through a
connector, and the user has several options to connect a display to
it.

The line really is blurred.

> >> Maybe instead of edp-connector one would introduce integrator's specific
> >> connector, for example with compatible "olimex,teres-edp-connector"
> >> which should follow edp abstract connector rules? This will be at least
> >> consistent with below presentation[1] - eDP requirements depends on
> >> integrator. Then if olimex has standard way of dealing with panels
> >> present in olimex/teres platforms the driver would then create
> >> drm_panel/drm_connector/drm_bridge(?) according to these rules, I guess.
> >> Anyway it still looks fishy for me :), maybe because I am not
> >> familiarized with details of these platforms.
>
> > That makes sense yes
>
> And what if some panel can be used with this pseudo-connecter and in
> some different hw directly? Code duplication? DT overlays?

Overlays are a solution, but I would advocate to always have the
connector.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Vasily Khoruzhick July 9, 2019, 12:49 a.m. UTC | #14
On Mon, Jul 1, 2019 at 2:58 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> Hi!
>
> On Fri, Jun 28, 2019 at 12:39:32PM +0200, Andrzej Hajda wrote:
> > On 12.06.2019 17:20, Maxime Ripard wrote:
> > >> I am not sure if I understand whole discussion here, but I also do not
> > >> understand whole edp-connector thing.
> > > The context is this one:
> > > https://patchwork.freedesktop.org/patch/257352/?series=51182&rev=1
> > > https://patchwork.freedesktop.org/patch/283012/?series=56163&rev=1
> > > https://patchwork.freedesktop.org/patch/286468/?series=56776&rev=2
> > >
> > > TL;DR: This bridge is being used on ARM laptops that can come with
> > > different eDP panels. Some of these panels require a regulator to be
> > > enabled for the panel to work, and this is obviously something that
> > > should be in the DT.
> > >
> > > However, we can't really describe the panel itself, since the vendor
> > > uses several of them and just relies on the eDP bus to do its job at
> > > retrieving the EDIDs. A generic panel isn't really working either
> > > since that would mean having a generic behaviour for all the panels
> > > connected to that bus, which isn't there either.
> > >
> > > The connector allows to expose this nicely.
> >
> > As VESA presentation says[1] eDP is based on DP but is much more
> > flexible, it is up to integrator (!!!) how the connection, power
> > up/down, initialization sequence should be performed. Trying to cover
> > every such case in edp-connector seems to me similar to panel-simple
> > attempt failure. Moreover there is no such thing as physical standard
> > eDP connector. Till now I though DT connector should describe physical
> > connector on the device, now I am lost, are there some DT bindings
> > guidelines about definition of a connector?
>
> This might be semantics but I guess we're in some kind of grey area?
>
> Like, for eDP, if it's soldered I guess we could say that there's no
> connector. But what happens if for some other board, that signal is
> routed through a ribbon?
>
> You could argue that there's no physical connector in both cases, or
> that there's one in both, or one for the ribbon and no connector for
> the one soldered in.
>
> > Maybe instead of edp-connector one would introduce integrator's specific
> > connector, for example with compatible "olimex,teres-edp-connector"
> > which should follow edp abstract connector rules? This will be at least
> > consistent with below presentation[1] - eDP requirements depends on
> > integrator. Then if olimex has standard way of dealing with panels
> > present in olimex/teres platforms the driver would then create
> > drm_panel/drm_connector/drm_bridge(?) according to these rules, I guess.
> > Anyway it still looks fishy for me :), maybe because I am not
> > familiarized with details of these platforms.
>
> That makes sense yes

Actually, it makes no sense at all. Current implementation for anx6345
driver works fine as is with any panel specified assuming panel delays
are long enough for connected panel. It just doesn't use panel timings
from the driver. Creating a platform driver for connector itself looks
redundant since it can't be reused, it doesn't describe actual
hardware and it's just defeats purpose of DT by introducing
board-specific code.

There's another issue: if we introduce edp-connector we'll have to
specify power up delays somewhere (in dts? or in platform driver?), so
edp-connector doesn't really solve the issue of multiple panels with
same motherboard.

I'd say DT overlays should be preferred solution here, not another
connector binding.

> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Maxime Ripard July 9, 2019, 8:55 a.m. UTC | #15
On Mon, Jul 08, 2019 at 05:49:21PM -0700, Vasily Khoruzhick wrote:
> > > Maybe instead of edp-connector one would introduce integrator's specific
> > > connector, for example with compatible "olimex,teres-edp-connector"
> > > which should follow edp abstract connector rules? This will be at least
> > > consistent with below presentation[1] - eDP requirements depends on
> > > integrator. Then if olimex has standard way of dealing with panels
> > > present in olimex/teres platforms the driver would then create
> > > drm_panel/drm_connector/drm_bridge(?) according to these rules, I guess.
> > > Anyway it still looks fishy for me :), maybe because I am not
> > > familiarized with details of these platforms.
> >
> > That makes sense yes
>
> Actually, it makes no sense at all. Current implementation for anx6345
> driver works fine as is with any panel specified assuming panel delays
> are long enough for connected panel. It just doesn't use panel timings
> from the driver. Creating a platform driver for connector itself looks
> redundant since it can't be reused, it doesn't describe actual
> hardware and it's just defeats purpose of DT by introducing
> board-specific code.

I'm not sure where you got the idea that the purpose of DT is to not
have any board-specific code.

It's perfectly fine to have some, that's even why there's a compatible
assigned to each and every board.

What the DT is about is allowing us to have a generic behaviour that
we can detect: we can have a given behaviour for a given board, and a
separate one for another one, and this will be evaluated at runtime.

This is *exactly* what this is about: we can have a compatible that
sets a given, more specific, behaviour (olimex,teres-edp-connector)
while saying that this is compatible with the generic behaviour
(edp-connector). That way, any OS will know what quirk to apply if
needed, and if not that it can use the generic behaviour.

And we could create a generic driver, for the generic behaviour if
needed.

> There's another issue: if we introduce edp-connector we'll have to
> specify power up delays somewhere (in dts? or in platform driver?), so
> edp-connector doesn't really solve the issue of multiple panels with
> same motherboard.

And that's what that compatible is about :)

> I'd say DT overlays should be preferred solution here, not another
> connector binding.

Overlays are a way to apply a device tree dynamically. It's orthogonal
to the binding.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Icenowy Zheng July 9, 2019, 8:58 a.m. UTC | #16
于 2019年7月9日 GMT+08:00 下午4:55:32, Maxime Ripard <maxime.ripard@bootlin.com> 写到:
>On Mon, Jul 08, 2019 at 05:49:21PM -0700, Vasily Khoruzhick wrote:
>> > > Maybe instead of edp-connector one would introduce integrator's
>specific
>> > > connector, for example with compatible
>"olimex,teres-edp-connector"
>> > > which should follow edp abstract connector rules? This will be at
>least
>> > > consistent with below presentation[1] - eDP requirements depends
>on
>> > > integrator. Then if olimex has standard way of dealing with
>panels
>> > > present in olimex/teres platforms the driver would then create
>> > > drm_panel/drm_connector/drm_bridge(?) according to these rules, I
>guess.
>> > > Anyway it still looks fishy for me :), maybe because I am not
>> > > familiarized with details of these platforms.
>> >
>> > That makes sense yes
>>
>> Actually, it makes no sense at all. Current implementation for
>anx6345
>> driver works fine as is with any panel specified assuming panel
>delays
>> are long enough for connected panel. It just doesn't use panel
>timings
>> from the driver. Creating a platform driver for connector itself
>looks
>> redundant since it can't be reused, it doesn't describe actual
>> hardware and it's just defeats purpose of DT by introducing
>> board-specific code.
>
>I'm not sure where you got the idea that the purpose of DT is to not
>have any board-specific code.
>
>It's perfectly fine to have some, that's even why there's a compatible
>assigned to each and every board.
>
>What the DT is about is allowing us to have a generic behaviour that
>we can detect: we can have a given behaviour for a given board, and a
>separate one for another one, and this will be evaluated at runtime.
>
>This is *exactly* what this is about: we can have a compatible that
>sets a given, more specific, behaviour (olimex,teres-edp-connector)
>while saying that this is compatible with the generic behaviour
>(edp-connector). That way, any OS will know what quirk to apply if
>needed, and if not that it can use the generic behaviour.
>
>And we could create a generic driver, for the generic behaviour if
>needed.
>
>> There's another issue: if we introduce edp-connector we'll have to
>> specify power up delays somewhere (in dts? or in platform driver?),
>so
>> edp-connector doesn't really solve the issue of multiple panels with
>> same motherboard.
>
>And that's what that compatible is about :)

Maybe we can introduce a connector w/o any driver just like hdmi-connector?

>
>> I'd say DT overlays should be preferred solution here, not another
>> connector binding.
>
>Overlays are a way to apply a device tree dynamically. It's orthogonal
>to the binding.
>
>Maxime
>
>--
>Maxime Ripard, Bootlin
>Embedded Linux and Kernel engineering
>https://bootlin.com
Maxime Ripard July 9, 2019, 2:21 p.m. UTC | #17
On Tue, Jul 09, 2019 at 04:58:35PM +0800, Icenowy Zheng wrote:
>
>
> 于 2019年7月9日 GMT+08:00 下午4:55:32, Maxime Ripard <maxime.ripard@bootlin.com> 写到:
> >On Mon, Jul 08, 2019 at 05:49:21PM -0700, Vasily Khoruzhick wrote:
> >> > > Maybe instead of edp-connector one would introduce integrator's
> >specific
> >> > > connector, for example with compatible
> >"olimex,teres-edp-connector"
> >> > > which should follow edp abstract connector rules? This will be at
> >least
> >> > > consistent with below presentation[1] - eDP requirements depends
> >on
> >> > > integrator. Then if olimex has standard way of dealing with
> >panels
> >> > > present in olimex/teres platforms the driver would then create
> >> > > drm_panel/drm_connector/drm_bridge(?) according to these rules, I
> >guess.
> >> > > Anyway it still looks fishy for me :), maybe because I am not
> >> > > familiarized with details of these platforms.
> >> >
> >> > That makes sense yes
> >>
> >> Actually, it makes no sense at all. Current implementation for
> >anx6345
> >> driver works fine as is with any panel specified assuming panel
> >delays
> >> are long enough for connected panel. It just doesn't use panel
> >timings
> >> from the driver. Creating a platform driver for connector itself
> >looks
> >> redundant since it can't be reused, it doesn't describe actual
> >> hardware and it's just defeats purpose of DT by introducing
> >> board-specific code.
> >
> >I'm not sure where you got the idea that the purpose of DT is to not
> >have any board-specific code.
> >
> >It's perfectly fine to have some, that's even why there's a compatible
> >assigned to each and every board.
> >
> >What the DT is about is allowing us to have a generic behaviour that
> >we can detect: we can have a given behaviour for a given board, and a
> >separate one for another one, and this will be evaluated at runtime.
> >
> >This is *exactly* what this is about: we can have a compatible that
> >sets a given, more specific, behaviour (olimex,teres-edp-connector)
> >while saying that this is compatible with the generic behaviour
> >(edp-connector). That way, any OS will know what quirk to apply if
> >needed, and if not that it can use the generic behaviour.
> >
> >And we could create a generic driver, for the generic behaviour if
> >needed.
> >
> >> There's another issue: if we introduce edp-connector we'll have to
> >> specify power up delays somewhere (in dts? or in platform driver?),
> >so
> >> edp-connector doesn't really solve the issue of multiple panels with
> >> same motherboard.
> >
> >And that's what that compatible is about :)
>
> Maybe we can introduce a connector w/o any driver just like hdmi-connector?

Ironically, a driver for it has been sent yesterday :)

But yeah, we can definitely do that too.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Vasily Khoruzhick July 9, 2019, 8:30 p.m. UTC | #18
On Tue, Jul 9, 2019 at 1:55 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Mon, Jul 08, 2019 at 05:49:21PM -0700, Vasily Khoruzhick wrote:
> > > > Maybe instead of edp-connector one would introduce integrator's specific
> > > > connector, for example with compatible "olimex,teres-edp-connector"
> > > > which should follow edp abstract connector rules? This will be at least
> > > > consistent with below presentation[1] - eDP requirements depends on
> > > > integrator. Then if olimex has standard way of dealing with panels
> > > > present in olimex/teres platforms the driver would then create
> > > > drm_panel/drm_connector/drm_bridge(?) according to these rules, I guess.
> > > > Anyway it still looks fishy for me :), maybe because I am not
> > > > familiarized with details of these platforms.
> > >
> > > That makes sense yes
> >
> > Actually, it makes no sense at all. Current implementation for anx6345
> > driver works fine as is with any panel specified assuming panel delays
> > are long enough for connected panel. It just doesn't use panel timings
> > from the driver. Creating a platform driver for connector itself looks
> > redundant since it can't be reused, it doesn't describe actual
> > hardware and it's just defeats purpose of DT by introducing
> > board-specific code.
>
> I'm not sure where you got the idea that the purpose of DT is to not
> have any board-specific code.

I believe DT was an attempt to move to declarative approach for
describing hardware. Yes, we have different compatibles for different
devices but they're specific to particular device rather than
particular board. Device interconnection is described in DT along with
some properties rather than in board-specific C-file. Introducing
board-specific compatible for a connector isn't looking right to me.

> It's perfectly fine to have some, that's even why there's a compatible
> assigned to each and every board.
>
> What the DT is about is allowing us to have a generic behaviour that
> we can detect: we can have a given behaviour for a given board, and a
> separate one for another one, and this will be evaluated at runtime.
>
> This is *exactly* what this is about: we can have a compatible that
> sets a given, more specific, behaviour (olimex,teres-edp-connector)
> while saying that this is compatible with the generic behaviour
> (edp-connector). That way, any OS will know what quirk to apply if
> needed, and if not that it can use the generic behaviour.
>
> And we could create a generic driver, for the generic behaviour if
> needed.
>
> > There's another issue: if we introduce edp-connector we'll have to
> > specify power up delays somewhere (in dts? or in platform driver?), so
> > edp-connector doesn't really solve the issue of multiple panels with
> > same motherboard.
>
> And that's what that compatible is about :)

Sorry, I fail to see how it would be different from using existing
panels infrastructure and different panels compatibles. I think Rob's
idea was to introduce generic edp-connector. If we can't make it
generic then let's use panel infrastructure.

> > I'd say DT overlays should be preferred solution here, not another
> > connector binding.
>
> Overlays are a way to apply a device tree dynamically. It's orthogonal
> to the binding.

It isn't orthogonal to original problem though.

> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Maxime Ripard July 10, 2019, 11:40 a.m. UTC | #19
On Tue, Jul 09, 2019 at 01:30:18PM -0700, Vasily Khoruzhick wrote:
> On Tue, Jul 9, 2019 at 1:55 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> >
> > On Mon, Jul 08, 2019 at 05:49:21PM -0700, Vasily Khoruzhick wrote:
> > > > > Maybe instead of edp-connector one would introduce integrator's specific
> > > > > connector, for example with compatible "olimex,teres-edp-connector"
> > > > > which should follow edp abstract connector rules? This will be at least
> > > > > consistent with below presentation[1] - eDP requirements depends on
> > > > > integrator. Then if olimex has standard way of dealing with panels
> > > > > present in olimex/teres platforms the driver would then create
> > > > > drm_panel/drm_connector/drm_bridge(?) according to these rules, I guess.
> > > > > Anyway it still looks fishy for me :), maybe because I am not
> > > > > familiarized with details of these platforms.
> > > >
> > > > That makes sense yes
> > >
> > > Actually, it makes no sense at all. Current implementation for anx6345
> > > driver works fine as is with any panel specified assuming panel delays
> > > are long enough for connected panel. It just doesn't use panel timings
> > > from the driver. Creating a platform driver for connector itself looks
> > > redundant since it can't be reused, it doesn't describe actual
> > > hardware and it's just defeats purpose of DT by introducing
> > > board-specific code.
> >
> > I'm not sure where you got the idea that the purpose of DT is to not
> > have any board-specific code.
>
> I believe DT was an attempt to move to declarative approach for
> describing hardware. Yes, we have different compatibles for different
> devices but they're specific to particular device rather than
> particular board. Device interconnection is described in DT along with
> some properties rather than in board-specific C-file.

You're right, but it's not incompatible with having some code to deal
with some board quirk.

> Introducing board-specific compatible for a connector isn't looking
> right to me.

If that board has a board-specific behaviour for it's connector, then
what's the issue?

You can't describe all the quirks in the all boards using purely
properties.

> > It's perfectly fine to have some, that's even why there's a compatible
> > assigned to each and every board.
> >
> > What the DT is about is allowing us to have a generic behaviour that
> > we can detect: we can have a given behaviour for a given board, and a
> > separate one for another one, and this will be evaluated at runtime.
> >
> > This is *exactly* what this is about: we can have a compatible that
> > sets a given, more specific, behaviour (olimex,teres-edp-connector)
> > while saying that this is compatible with the generic behaviour
> > (edp-connector). That way, any OS will know what quirk to apply if
> > needed, and if not that it can use the generic behaviour.
> >
> > And we could create a generic driver, for the generic behaviour if
> > needed.
> >
> > > There's another issue: if we introduce edp-connector we'll have to
> > > specify power up delays somewhere (in dts? or in platform driver?), so
> > > edp-connector doesn't really solve the issue of multiple panels with
> > > same motherboard.
> >
> > And that's what that compatible is about :)
>
> Sorry, I fail to see how it would be different from using existing
> panels infrastructure and different panels compatibles. I think Rob's
> idea was to introduce generic edp-connector.

Again, there's no such thing as a generic edp-connector. The spec
doesn't define anything related to the power sequence for example.

> If we can't make it generic then let's use panel infrastructure.

Which uses a device specific compatible. Really, I'm not sure what
your objection and / or argument is here.

In addition, when that was brought up in the discussion, you rejected
it because it was inconvenient:
https://patchwork.freedesktop.org/patch/283012/?series=56163&rev=1#comment_535206

And I agree with you on that one.

> > > I'd say DT overlays should be preferred solution here, not another
> > > connector binding.
> >
> > Overlays are a way to apply a device tree dynamically. It's orthogonal
> > to the binding.
>
> It isn't orthogonal to original problem though.

It is. The original problem is that you want to power up whatever is
on the other side of a eDP link using an arbitrary regulator.

This is a "how do I describe that in my DT" problem, and it really has
nothing to do with how the DT is being passed to the kernel.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Vasily Khoruzhick July 10, 2019, 10:11 p.m. UTC | #20
On Wed, Jul 10, 2019 at 4:40 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Tue, Jul 09, 2019 at 01:30:18PM -0700, Vasily Khoruzhick wrote:
> > On Tue, Jul 9, 2019 at 1:55 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > >
> > > On Mon, Jul 08, 2019 at 05:49:21PM -0700, Vasily Khoruzhick wrote:
> > > > > > Maybe instead of edp-connector one would introduce integrator's specific
> > > > > > connector, for example with compatible "olimex,teres-edp-connector"
> > > > > > which should follow edp abstract connector rules? This will be at least
> > > > > > consistent with below presentation[1] - eDP requirements depends on
> > > > > > integrator. Then if olimex has standard way of dealing with panels
> > > > > > present in olimex/teres platforms the driver would then create
> > > > > > drm_panel/drm_connector/drm_bridge(?) according to these rules, I guess.
> > > > > > Anyway it still looks fishy for me :), maybe because I am not
> > > > > > familiarized with details of these platforms.
> > > > >
> > > > > That makes sense yes
> > > >
> > > > Actually, it makes no sense at all. Current implementation for anx6345
> > > > driver works fine as is with any panel specified assuming panel delays
> > > > are long enough for connected panel. It just doesn't use panel timings
> > > > from the driver. Creating a platform driver for connector itself looks
> > > > redundant since it can't be reused, it doesn't describe actual
> > > > hardware and it's just defeats purpose of DT by introducing
> > > > board-specific code.
> > >
> > > I'm not sure where you got the idea that the purpose of DT is to not
> > > have any board-specific code.
> >
> > I believe DT was an attempt to move to declarative approach for
> > describing hardware. Yes, we have different compatibles for different
> > devices but they're specific to particular device rather than
> > particular board. Device interconnection is described in DT along with
> > some properties rather than in board-specific C-file.
>
> You're right, but it's not incompatible with having some code to deal
> with some board quirk.
>
> > Introducing board-specific compatible for a connector isn't looking
> > right to me.
>
> If that board has a board-specific behaviour for it's connector, then
> what's the issue?
>
> You can't describe all the quirks in the all boards using purely
> properties.
>
> > > It's perfectly fine to have some, that's even why there's a compatible
> > > assigned to each and every board.
> > >
> > > What the DT is about is allowing us to have a generic behaviour that
> > > we can detect: we can have a given behaviour for a given board, and a
> > > separate one for another one, and this will be evaluated at runtime.
> > >
> > > This is *exactly* what this is about: we can have a compatible that
> > > sets a given, more specific, behaviour (olimex,teres-edp-connector)
> > > while saying that this is compatible with the generic behaviour
> > > (edp-connector). That way, any OS will know what quirk to apply if
> > > needed, and if not that it can use the generic behaviour.
> > >
> > > And we could create a generic driver, for the generic behaviour if
> > > needed.
> > >
> > > > There's another issue: if we introduce edp-connector we'll have to
> > > > specify power up delays somewhere (in dts? or in platform driver?), so
> > > > edp-connector doesn't really solve the issue of multiple panels with
> > > > same motherboard.
> > >
> > > And that's what that compatible is about :)
> >
> > Sorry, I fail to see how it would be different from using existing
> > panels infrastructure and different panels compatibles. I think Rob's
> > idea was to introduce generic edp-connector.
>
> Again, there's no such thing as a generic edp-connector. The spec
> doesn't define anything related to the power sequence for example.
>
> > If we can't make it generic then let's use panel infrastructure.
>
> Which uses a device specific compatible. Really, I'm not sure what
> your objection and / or argument is here.
>
> In addition, when that was brought up in the discussion, you rejected
> it because it was inconvenient:
> https://patchwork.freedesktop.org/patch/283012/?series=56163&rev=1#comment_535206

It is inconvenient, but I don't understand how having board-specific
connectors fixes it.



> And I agree with you on that one.
>
> > > > I'd say DT overlays should be preferred solution here, not another
> > > > connector binding.
> > >
> > > Overlays are a way to apply a device tree dynamically. It's orthogonal
> > > to the binding.
> >
> > It isn't orthogonal to original problem though.
>
> It is. The original problem is that you want to power up whatever is
> on the other side of a eDP link using an arbitrary regulator.
>
> This is a "how do I describe that in my DT" problem, and it really has
> nothing to do with how the DT is being passed to the kernel.
>
> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Maxime Ripard July 12, 2019, 8:15 p.m. UTC | #21
On Wed, Jul 10, 2019 at 03:11:04PM -0700, Vasily Khoruzhick wrote:
> On Wed, Jul 10, 2019 at 4:40 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > There's another issue: if we introduce edp-connector we'll have to
> > > > > specify power up delays somewhere (in dts? or in platform driver?), so
> > > > > edp-connector doesn't really solve the issue of multiple panels with
> > > > > same motherboard.
> > > >
> > > > And that's what that compatible is about :)
> > >
> > > Sorry, I fail to see how it would be different from using existing
> > > panels infrastructure and different panels compatibles. I think Rob's
> > > idea was to introduce generic edp-connector.
> >
> > Again, there's no such thing as a generic edp-connector. The spec
> > doesn't define anything related to the power sequence for example.
> >
> > > If we can't make it generic then let's use panel infrastructure.
> >
> > Which uses a device specific compatible. Really, I'm not sure what
> > your objection and / or argument is here.
> >
> > In addition, when that was brought up in the discussion, you rejected
> > it because it was inconvenient:
> > https://patchwork.freedesktop.org/patch/283012/?series=56163&rev=1#comment_535206
>
> It is inconvenient, but I don't understand how having board-specific
> connectors fixes it.

How it would not fix it?

You'll have one connector, without the need to describe each and every
panel in the device tree and rely on the EDID instead, and you'll have
the option to power up the regulator you need.

I really don't understand what's the issue here, so let's take a step
back. What are is the issue , what are your requirements, and how
would you like that to be described ?

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Vasily Khoruzhick July 16, 2019, 12:28 a.m. UTC | #22
On Fri, Jul 12, 2019 at 1:15 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Wed, Jul 10, 2019 at 03:11:04PM -0700, Vasily Khoruzhick wrote:
> > On Wed, Jul 10, 2019 at 4:40 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > > There's another issue: if we introduce edp-connector we'll have to
> > > > > > specify power up delays somewhere (in dts? or in platform driver?), so
> > > > > > edp-connector doesn't really solve the issue of multiple panels with
> > > > > > same motherboard.
> > > > >
> > > > > And that's what that compatible is about :)
> > > >
> > > > Sorry, I fail to see how it would be different from using existing
> > > > panels infrastructure and different panels compatibles. I think Rob's
> > > > idea was to introduce generic edp-connector.
> > >
> > > Again, there's no such thing as a generic edp-connector. The spec
> > > doesn't define anything related to the power sequence for example.
> > >
> > > > If we can't make it generic then let's use panel infrastructure.
> > >
> > > Which uses a device specific compatible. Really, I'm not sure what
> > > your objection and / or argument is here.
> > >
> > > In addition, when that was brought up in the discussion, you rejected
> > > it because it was inconvenient:
> > > https://patchwork.freedesktop.org/patch/283012/?series=56163&rev=1#comment_535206
> >
> > It is inconvenient, but I don't understand how having board-specific
> > connectors fixes it.
>
> How it would not fix it?

I think I got your idea, but yet I think it's not the best solution.

Do I understand correctly that you're proposing to introduce
board-specific edp-connector driver that will be aware of worst case
power up delays and will control backlight and power?

Then why not to add another board-specific panel (e.g.
"pine64,pinebook-panel") to simple-panel.c that does the same?

> You'll have one connector, without the need to describe each and every
> panel in the device tree and rely on the EDID instead, and you'll have
> the option to power up the regulator you need.
>
> I really don't understand what's the issue here, so let's take a step
> back. What are is the issue , what are your requirements, and how
> would you like that to be described ?

We have a device (Pinebook) that uses the same board with multiple edp
panels. So far there're pinebooks with 3 different panels: 11" with
768p panel, 11" with 1080p panel, 14" with 768p panel.

Currently there's no way to describe all pinebooks with a single dts.
There's a simple workaround though -- we can just specify a panel with
worst power up delays in dts and it'll work since anx6345 driver
ignores panel modes anyway and uses EDID.

Originally I proposed to extend simple-panel driver to support generic
edp-panel but it was rejected. I still believe that it's the best
solution assuming we can specify delays in dts, since panels list is
specific to particular device and it probably can't be reused, i.e.
there's no good reason to move it into C code.

Rob Herring proposed to introduce edp-connector. While I still believe
that it's not accurate description of hardware since it'll have to
have backlight node (backlight is actually panel property) I was OK
with this approach assuming we can store delays in dts.

Later it evolved into board-specific edp-connector.

So far I don't understand why everyone is trying to avoid introducing
edp-panel driver that can read delays from dts. Basically, I don't
understand what's the magic behind simple-panel.c and why new panels
should be added there rather than described in dts. [1] Doesn't
explain that.

[1] http://sietch-tagr.blogspot.com/2016/04/display-panels-are-not-special.html

Regards,
Vasily


> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Maxime Ripard July 24, 2019, 1:58 p.m. UTC | #23
On Mon, Jul 15, 2019 at 05:28:53PM -0700, Vasily Khoruzhick wrote:
> On Fri, Jul 12, 2019 at 1:15 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> >
> > On Wed, Jul 10, 2019 at 03:11:04PM -0700, Vasily Khoruzhick wrote:
> > > On Wed, Jul 10, 2019 at 4:40 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > > > There's another issue: if we introduce edp-connector we'll have to
> > > > > > > specify power up delays somewhere (in dts? or in platform driver?), so
> > > > > > > edp-connector doesn't really solve the issue of multiple panels with
> > > > > > > same motherboard.
> > > > > >
> > > > > > And that's what that compatible is about :)
> > > > >
> > > > > Sorry, I fail to see how it would be different from using existing
> > > > > panels infrastructure and different panels compatibles. I think Rob's
> > > > > idea was to introduce generic edp-connector.
> > > >
> > > > Again, there's no such thing as a generic edp-connector. The spec
> > > > doesn't define anything related to the power sequence for example.
> > > >
> > > > > If we can't make it generic then let's use panel infrastructure.
> > > >
> > > > Which uses a device specific compatible. Really, I'm not sure what
> > > > your objection and / or argument is here.
> > > >
> > > > In addition, when that was brought up in the discussion, you rejected
> > > > it because it was inconvenient:
> > > > https://patchwork.freedesktop.org/patch/283012/?series=56163&rev=1#comment_535206
> > >
> > > It is inconvenient, but I don't understand how having board-specific
> > > connectors fixes it.
> >
> > How it would not fix it?
>
> I think I got your idea, but yet I think it's not the best solution.
>
> Do I understand correctly that you're proposing to introduce
> board-specific edp-connector driver that will be aware of worst case
> power up delays and will control backlight and power?
>
> Then why not to add another board-specific panel (e.g.
> "pine64,pinebook-panel") to simple-panel.c that does the same?

That would be fine for me too. Thierry was against it though IIRC, and
I don't recall why exactly.

> > You'll have one connector, without the need to describe each and every
> > panel in the device tree and rely on the EDID instead, and you'll have
> > the option to power up the regulator you need.
> >
> > I really don't understand what's the issue here, so let's take a step
> > back. What are is the issue , what are your requirements, and how
> > would you like that to be described ?
>
> We have a device (Pinebook) that uses the same board with multiple edp
> panels. So far there're pinebooks with 3 different panels: 11" with
> 768p panel, 11" with 1080p panel, 14" with 768p panel.
>
> Currently there's no way to describe all pinebooks with a single dts.
> There's a simple workaround though -- we can just specify a panel with
> worst power up delays in dts and it'll work since anx6345 driver
> ignores panel modes anyway and uses EDID.
>
> Originally I proposed to extend simple-panel driver to support generic
> edp-panel but it was rejected. I still believe that it's the best
> solution assuming we can specify delays in dts, since panels list is
> specific to particular device and it probably can't be reused, i.e.
> there's no good reason to move it into C code.
>
> Rob Herring proposed to introduce edp-connector. While I still believe
> that it's not accurate description of hardware since it'll have to
> have backlight node (backlight is actually panel property) I was OK
> with this approach assuming we can store delays in dts.
>
> Later it evolved into board-specific edp-connector.

I think you got that wrong. As far as I'm concerned, the plan was to
have two compatibles: the board-specific one, and the generic one.

Something like compatible = "pine64,pinebook-edp-connector",
"edp-connector"; or whatever.

> So far I don't understand why everyone is trying to avoid introducing
> edp-panel driver that can read delays from dts. Basically, I don't
> understand what's the magic behind simple-panel.c and why new panels
> should be added there rather than described in dts. [1] Doesn't
> explain that.

So others might have different viewpoints here as well, but the major
downside I see in putting those kind of values in the device tree is
that at some point, someone will get it wrong, and chances are that
even for the same panel, everyone will use a slightly different set of
timings.

And once it's wrong, then it's a mess to fix. You have to track down
every DT using it, make sure it's corrected, and then every user will
have to change their DT in their system. Whereas if you have just a
compatible and those timings in the kernel, then the only thing
required is a kernel update, which should be a pretty standard
operation.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Patch
diff mbox series

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
index 0ec46b969a75..a0ad438b037f 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
@@ -65,6 +65,21 @@ 
 		};
 	};
 
+	panel: panel {
+		compatible ="innolux,n116bge", "simple-panel";
+		status = "okay";
+		power-supply = <&reg_dcdc1>;
+		backlight = <&backlight>;
+
+		ports {
+			panel_in: port {
+				panel_in_edp: endpoint {
+					remote-endpoint = <&anx6345_out>;
+				};
+			};
+		};
+	};
+
 	reg_usb1_vbus: usb1-vbus {
 		compatible = "regulator-fixed";
 		regulator-name = "usb1-vbus";
@@ -81,20 +96,48 @@ 
 	};
 };
 
+&de {
+	status = "okay";
+};
+
 &ehci1 {
 	status = "okay";
 };
 
 
-/* The ANX6345 eDP-bridge is on i2c0. There is no linux (mainline)
- * driver for this chip at the moment, the bootloader initializes it.
- * However it can be accessed with the i2c-dev driver from user space.
- */
 &i2c0 {
 	clock-frequency = <100000>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&i2c0_pins>;
 	status = "okay";
+
+	anx6345: anx6345@38 {
+		compatible = "analogix,anx6345";
+		reg = <0x38>;
+		reset-gpios = <&pio 3 24 GPIO_ACTIVE_LOW>; /* PD24 */
+		dvdd25-supply = <&reg_dldo2>;
+		dvdd12-supply = <&reg_dldo3>;
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				anx6345_in: endpoint {
+					remote-endpoint = <&tcon0_out_anx6345>;
+				};
+			};
+			port@1 {
+				anx6345_out: endpoint {
+					remote-endpoint = <&panel_in_edp>;
+				};
+			};
+		};
+	};
+};
+
+&mixer0 {
+	status = "okay";
 };
 
 &mmc0 {
@@ -279,6 +322,20 @@ 
 	vcc-hdmi-supply = <&reg_dldo1>;
 };
 
+&tcon0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&lcd_rgb666_pins>;
+
+	status = "okay";
+};
+
+&tcon0_out {
+	tcon0_out_anx6345: endpoint@0 {
+		reg = <0>;
+		remote-endpoint = <&anx6345_in>;
+	};
+};
+
 &uart0 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&uart0_pb_pins>;