[PATCH 1/2] drm/panel: Augment the TPO TPG110 bindings

[PATCH 1/2] drm/panel: Augment the TPO TPG110 bindings

[Linus Walleij]

The TPO TPG110 bindings were using the DPI bindings (popular
in the fbdev subsystem) but this misses the finer points
learned in the DRM subsystem.

We need to augment the bindings for proper DRM integration:
the timings are expressed by the hardware, not put into the
device tree.

Old device trees with the DPI info will continue to work,
but no known deployments exist.

Cc: devicetree@vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 .../bindings/display/panel/tpo,tpg110.txt     | 34 ++++++++-----------
 1 file changed, 14 insertions(+), 20 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/panel/tpo,tpg110.txt b/Documentation/devicetree/bindings/display/panel/tpo,tpg110.txt
index f5e3c6f2095a..0e918076d55e 100644
--- a/Documentation/devicetree/bindings/display/panel/tpo,tpg110.txt
+++ b/Documentation/devicetree/bindings/display/panel/tpo,tpg110.txt
@@ -1,26 +1,32 @@
 TPO TPG110 Panel
 ================
 
-This binding builds on the DPI bindings, adding a few properties
-as a superset of a DPI. See panel-dpi.txt for the required DPI
-bindings.
+This panel driver can driver a variety of panels. It requires
+a few GPIO lines for control of its reset line and custom serial
+protocol.
 
 Required properties:
-- compatible : "tpo,tpg110"
+- compatible : one of:
+  "ste,nomadik-nhk15-display", "tpo,tpg110"
+  "tpo,tpg110"
 - grestb-gpios : panel reset GPIO
 - scen-gpios : serial control enable GPIO
 - scl-gpios : serial control clock line GPIO
 - sda-gpios : serial control data line GPIO
+- width-mm : see display/panel/panel-common.txt
+- height-mm : see display/panel/panel-common.txt
 
-Required nodes:
-- Video port for DPI input, see panel-dpi.txt
-- Panel timing for DPI setup, see panel-dpi.txt
+The device node can contain one 'port' child node with one child
+'endpoint' node, according to the bindings defined in
+media/video-interfaces.txt. This node should describe panel's video bus.
 
 Example
 -------
 
 panel {
-	compatible = "tpo,tpg110", "panel-dpi";
+	compatible = "tpo,tpg110";
+	width-mm = <116>;
+	height-mm = <87>;
 	grestb-gpios = <&stmpe_gpio44 5 GPIO_ACTIVE_LOW>;
 	scen-gpios = <&gpio0 6 GPIO_ACTIVE_LOW>;
 	scl-gpios = <&gpio0 5 GPIO_ACTIVE_HIGH>;
@@ -32,16 +38,4 @@ panel {
 			remote-endpoint = <&nomadik_clcd_pads>;
 		};
 	};
-
-	panel-timing {
-		clock-frequency = <33200000>;
-		hactive = <800>;
-		hback-porch = <216>;
-		hfront-porch = <40>;
-		hsync-len = <1>;
-		vactive = <480>;
-		vback-porch = <35>;
-		vfront-porch = <10>;
-		vsync-len = <1>;
-	};
 };
-- 
2.17.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm/panel: Augment the TPO TPG110 bindings

[Rob Herring]

On Thu, Jun 21, 2018 at 08:49:41PM +0200, Linus Walleij wrote:
> The TPO TPG110 bindings were using the DPI bindings (popular
> in the fbdev subsystem) but this misses the finer points
> learned in the DRM subsystem.
> 
> We need to augment the bindings for proper DRM integration:
> the timings are expressed by the hardware, not put into the
> device tree.
> 
> Old device trees with the DPI info will continue to work,
> but no known deployments exist.
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  .../bindings/display/panel/tpo,tpg110.txt     | 34 ++++++++-----------
>  1 file changed, 14 insertions(+), 20 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/panel/tpo,tpg110.txt b/Documentation/devicetree/bindings/display/panel/tpo,tpg110.txt
> index f5e3c6f2095a..0e918076d55e 100644
> --- a/Documentation/devicetree/bindings/display/panel/tpo,tpg110.txt
> +++ b/Documentation/devicetree/bindings/display/panel/tpo,tpg110.txt
> @@ -1,26 +1,32 @@
>  TPO TPG110 Panel
>  ================
>  
> -This binding builds on the DPI bindings, adding a few properties
> -as a superset of a DPI. See panel-dpi.txt for the required DPI
> -bindings.
> +This panel driver can driver a variety of panels. It requires

s/can driver/can drive/

Though what a driver supports is irrelevant to the binding...

If you remove timings, how does it drive a variety of panels? Just by 
compatible? That would mean "tpo,tpg110" alone is not valid nor useful 
as a fallback.

> +a few GPIO lines for control of its reset line and custom serial
> +protocol.
>  
>  Required properties:
> -- compatible : "tpo,tpg110"
> +- compatible : one of:
> +  "ste,nomadik-nhk15-display", "tpo,tpg110"
> +  "tpo,tpg110"
>  - grestb-gpios : panel reset GPIO
>  - scen-gpios : serial control enable GPIO
>  - scl-gpios : serial control clock line GPIO
>  - sda-gpios : serial control data line GPIO

I2C? That should be done differently...

> +- width-mm : see display/panel/panel-common.txt
> +- height-mm : see display/panel/panel-common.txt
>  
> -Required nodes:
> -- Video port for DPI input, see panel-dpi.txt
> -- Panel timing for DPI setup, see panel-dpi.txt
> +The device node can contain one 'port' child node with one child
> +'endpoint' node, according to the bindings defined in
> +media/video-interfaces.txt. This node should describe panel's video bus.
>  
>  Example
>  -------
>  
>  panel {
> -	compatible = "tpo,tpg110", "panel-dpi";
> +	compatible = "tpo,tpg110";
> +	width-mm = <116>;
> +	height-mm = <87>;
>  	grestb-gpios = <&stmpe_gpio44 5 GPIO_ACTIVE_LOW>;
>  	scen-gpios = <&gpio0 6 GPIO_ACTIVE_LOW>;
>  	scl-gpios = <&gpio0 5 GPIO_ACTIVE_HIGH>;
> @@ -32,16 +38,4 @@ panel {
>  			remote-endpoint = <&nomadik_clcd_pads>;
>  		};
>  	};
> -
> -	panel-timing {
> -		clock-frequency = <33200000>;
> -		hactive = <800>;
> -		hback-porch = <216>;
> -		hfront-porch = <40>;
> -		hsync-len = <1>;
> -		vactive = <480>;
> -		vback-porch = <35>;
> -		vfront-porch = <10>;
> -		vsync-len = <1>;
> -	};
>  };
> -- 
> 2.17.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm/panel: Augment the TPO TPG110 bindings

[Linus Walleij]

On Wed, Jun 27, 2018 at 7:21 PM Rob Herring <robh@kernel.org> wrote:
> On Thu, Jun 21, 2018 at 08:49:41PM +0200, Linus Walleij wrote:

> > +This panel driver can driver a variety of panels. It requires
>
> s/can driver/can drive/
>
> Though what a driver supports is irrelevant to the binding...

It it not a software driver the text is referring to. It is a
electrical interface to a panel. Like how a TTL circuit connected
to a LED is referred to as a "LED driver", it's simply what the
industry calls these things.

So there are two things: the panel driver and the panel, the
same panel driver is used with several panels. What the
electronics engineer will do is put a panel driver like this
into his design and then connect some panel s/he finds
in the right quantity in the streets of Shenzhen.

> If you remove timings, how does it drive a variety of panels? Just by
> compatible?

Yes.

Like we did for
Documentation/devicetree/bindings/display/panel/ilitek,ili9322.txt
which is similar to this.

In fact I think many panel drivers are just sloppily slipping in
under the radar as "panels" in our bindings.

>That would mean "tpo,tpg110" alone is not valid nor useful
> as a fallback.

Actually it is. The hardware is wired up to the desired
resolution with hardware straps, which appear in
the registers the (software) driver can read out so
this is ideally self-describing hardware.

But for the event that something needs tweaking in the
future, like we overspecify say SoCs, I include the
exact system on which it is deployd as a separate
compatible string.

> > +a few GPIO lines for control of its reset line and custom serial
> > +protocol.
> >
> >  Required properties:
> > -- compatible : "tpo,tpg110"
> > +- compatible : one of:
> > +  "ste,nomadik-nhk15-display", "tpo,tpg110"
> > +  "tpo,tpg110"
> >  - grestb-gpios : panel reset GPIO
> >  - scen-gpios : serial control enable GPIO
> >  - scl-gpios : serial control clock line GPIO
> >  - sda-gpios : serial control data line GPIO
>
> I2C? That should be done differently...

It is not I2C, the lines are just named confusingly
similar. None of the I2C (-like) protocols apply.
I was similarly confused when I first implemented it.

(Maybe I should add a comment to explain this.)

Yours,
Linus Walleij
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm/panel: Augment the TPO TPG110 bindings

[Andrzej Hajda]

On 01.07.2018 21:01, Linus Walleij wrote:
> On Wed, Jun 27, 2018 at 7:21 PM Rob Herring <robh@kernel.org> wrote:
>> On Thu, Jun 21, 2018 at 08:49:41PM +0200, Linus Walleij wrote:
>>> +This panel driver can driver a variety of panels. It requires
>> s/can driver/can drive/
>>
>> Though what a driver supports is irrelevant to the binding...
> It it not a software driver the text is referring to. It is a
> electrical interface to a panel. Like how a TTL circuit connected
> to a LED is referred to as a "LED driver", it's simply what the
> industry calls these things.
>
> So there are two things: the panel driver and the panel, the
> same panel driver is used with several panels. What the
> electronics engineer will do is put a panel driver like this
> into his design and then connect some panel s/he finds
> in the right quantity in the streets of Shenzhen.
>
>> If you remove timings, how does it drive a variety of panels? Just by
>> compatible?
> Yes.
>
> Like we did for
> Documentation/devicetree/bindings/display/panel/ilitek,ili9322.txt
> which is similar to this.
>
> In fact I think many panel drivers are just sloppily slipping in
> under the radar as "panels" in our bindings.
>
>> That would mean "tpo,tpg110" alone is not valid nor useful
>> as a fallback.
> Actually it is. The hardware is wired up to the desired
> resolution with hardware straps, which appear in
> the registers the (software) driver can read out so
> this is ideally self-describing hardware.
>
> But for the event that something needs tweaking in the
> future, like we overspecify say SoCs, I include the
> exact system on which it is deployd as a separate
> compatible string.
>
>>> +a few GPIO lines for control of its reset line and custom serial
>>> +protocol.
>>>
>>>  Required properties:
>>> -- compatible : "tpo,tpg110"
>>> +- compatible : one of:
>>> +  "ste,nomadik-nhk15-display", "tpo,tpg110"
>>> +  "tpo,tpg110"
>>>  - grestb-gpios : panel reset GPIO
>>>  - scen-gpios : serial control enable GPIO
>>>  - scl-gpios : serial control clock line GPIO
>>>  - sda-gpios : serial control data line GPIO
>> I2C? That should be done differently...
> It is not I2C, the lines are just named confusingly
> similar. None of the I2C (-like) protocols apply.
> I was similarly confused when I first implemented it.
>
> (Maybe I should add a comment to explain this.)

I have grepped Internet, out of curiosity, and it seems it is so-called
3-wire spi [1].

[1]:
http://aitendo3.sakura.ne.jp/aitendo_data/product_img/lcd/tft/T43P00/TPG110%20Customer%20Spec_0.6.pdf


Regards
Andrzej

>
> Yours,
> Linus Walleij
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm/panel: Augment the TPO TPG110 bindings

[Rob Herring]

On Sun, Jul 1, 2018 at 1:01 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Wed, Jun 27, 2018 at 7:21 PM Rob Herring <robh@kernel.org> wrote:
> > On Thu, Jun 21, 2018 at 08:49:41PM +0200, Linus Walleij wrote:
>
> > > +This panel driver can driver a variety of panels. It requires
> >
> > s/can driver/can drive/
> >
> > Though what a driver supports is irrelevant to the binding...
>
> It it not a software driver the text is referring to. It is a
> electrical interface to a panel. Like how a TTL circuit connected
> to a LED is referred to as a "LED driver", it's simply what the
> industry calls these things.

Yes, I'm aware of the term, but I see *far* more of the other use of "driver".

Can you say "panel driver IC" to clarify.

> So there are two things: the panel driver and the panel, the
> same panel driver is used with several panels. What the
> electronics engineer will do is put a panel driver like this
> into his design and then connect some panel s/he finds
> in the right quantity in the streets of Shenzhen.

Yep. I've had the fun of trying to figure out how to get panels
working and having the piecemeal documentation of the driver IC
without all the hook-up details...

> > If you remove timings, how does it drive a variety of panels? Just by
> > compatible?
>
> Yes.
>
> Like we did for
> Documentation/devicetree/bindings/display/panel/ilitek,ili9322.txt
> which is similar to this.

According to that, there's only 1 panel supported.

> In fact I think many panel drivers are just sloppily slipping in
> under the radar as "panels" in our bindings.

Indeed. I'm not sure that trying to split driver ICs and panels is
really feasible at least up front given how poor the documentation is
typically.

> >That would mean "tpo,tpg110" alone is not valid nor useful
> > as a fallback.
>
> Actually it is. The hardware is wired up to the desired
> resolution with hardware straps, which appear in
> the registers the (software) driver can read out so
> this is ideally self-describing hardware.

Nice. Can you add that detail.

> But for the event that something needs tweaking in the
> future, like we overspecify say SoCs, I include the
> exact system on which it is deployd as a separate
> compatible string.
>
> > > +a few GPIO lines for control of its reset line and custom serial
> > > +protocol.
> > >
> > >  Required properties:
> > > -- compatible : "tpo,tpg110"
> > > +- compatible : one of:
> > > +  "ste,nomadik-nhk15-display", "tpo,tpg110"
> > > +  "tpo,tpg110"
> > >  - grestb-gpios : panel reset GPIO

Also, use 'reset-gpios' as that is the standard name.

> > >  - scen-gpios : serial control enable GPIO
> > >  - scl-gpios : serial control clock line GPIO
> > >  - sda-gpios : serial control data line GPIO
> >
> > I2C? That should be done differently...
>
> It is not I2C, the lines are just named confusingly
> similar. None of the I2C (-like) protocols apply.
> I was similarly confused when I first implemented it.

If this is 3-wire SPI then as Andrzej says, then still it should be
done as spi-gpio. Which really just means these gpio properties go
away and this should be a SPI child node.

Rob
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel