| Message ID | 20180621184942.26087-1-linus.walleij@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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
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
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>; - }; };
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(-)