| Message ID | 1398083321-8668-10-git-send-email-yj44.cho@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 04/21/2014 02:28 PM, YoungJun Cho wrote: > This patch adds DT bindings for s6e3fa0 panel. > The bindings describes panel resources, display timings and cpu timings. > > Changelog v2: > - Adds unit address (commented by Sachin Kamat) > Changelog v3: > - Removes optional delay, size properties (commented by Laurent Pinchart) > - Adds OLED detection, TE gpio properties > Changelog v4: > - Moves CPU timings relevant properties from FIMD DT > (commeted by Laurent Pinchart, Andrzej Hajda) > > Signed-off-by: YoungJun Cho <yj44.cho@samsung.com> > Acked-by: Inki Dae <inki.dae@samsung.com> > Acked-by: Kyungmin Park <kyungmin.park@samsung.com> > --- > .../devicetree/bindings/panel/samsung,s6e3fa0.txt | 63 ++++++++++++++++++++ > 1 file changed, 63 insertions(+) > create mode 100644 Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt > > diff --git a/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt > new file mode 100644 > index 0000000..9eeb38b > --- /dev/null > +++ b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt > @@ -0,0 +1,63 @@ > +Samsung S6E3FA0 AMOLED LCD 5.7 inch panel > + > +Required properties: > + - compatible: "samsung,s6e3fa0" > + - reg: the virtual channel number of a DSI peripheral > + - vdd3-supply: core voltage supply > + - vci-supply: voltage supply for analog circuits > + - reset-gpio: a GPIO spec for the reset pin > + - det-gpio: a GPIO spec for the OLED detection pin > + - te-gpio: a GPIO spec for the TE pin Just FYI, according to DT documentation [1] gpio spec should be in form [name]-gpios, however there is plenty bindings with -gpio suffix, so I am not sure if it is really enforced. On the other side it is enforced by descriptor based gpio framework[2]. Integer-based gpio framework used in your driver is obsolete according to [2]. [1]: Documentation/devicetree/bindings/gpio/gpio.txt [2]: Documentation/gpio/gpio.txt Regards Andrzej > + - display-timings: timings for the connected panel as described by [1] > + - cpu-timings: CPU interface timings for the connected panel, and it contains > + following optional properties. > + - cs-setup: clock cycles for the active period of address signal > + enable until chip select is enable in CPU interface > + - wr-setup: clock cycles for the active period of CS signal enable > + until write signal is enable in CPU interface > + - wr-act: clock cycles for the active period of CS enable in CPU > + interface > + - wr-hold: clock cycles for the active period of CS disable until > + write signal is disable in CPU interface > + > +Optional properties: > + > +The device node can contain one 'port' child node with one child > +'endpoint' node, according to the bindings defined in [2]. This > +node should describe panel's video bus. > + > +[1]: Documentation/devicetree/bindings/video/display-timing.txt > +[2]: Documentation/devicetree/bindings/media/video-interfaces.txt > + > +Example: > + > + panel@0 { > + compatible = "samsung,s6e3fa0"; > + reg = <0>; > + vdd3-supply = <&vcclcd_reg>; > + vci-supply = <&vlcd_reg>; > + reset-gpio = <&gpy7 4 0>; > + det-gpio = <&gpg0 6 0>; > + te-gpio = <&gpd1 7 0>; > + > + display-timings { > + timing0: timing-0 { > + clock-frequency = <0>; > + hactive = <1080>; > + vactive = <1920>; > + hfront-porch = <2>; > + hback-porch = <2>; > + hsync-len = <1>; > + vfront-porch = <1>; > + vback-porch = <4>; > + vsync-len = <1>; > + }; > + }; > + > + cpu-timings { > + cs-setup = <0>; > + wr-setup = <0>; > + wr-act = <1>; > + wr-hold = <0>; > + }; > + }; > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Andrzej Thank you for comment. On 04/22/2014 11:02 PM, Andrzej Hajda wrote: > On 04/21/2014 02:28 PM, YoungJun Cho wrote: >> This patch adds DT bindings for s6e3fa0 panel. >> The bindings describes panel resources, display timings and cpu timings. >> >> Changelog v2: >> - Adds unit address (commented by Sachin Kamat) >> Changelog v3: >> - Removes optional delay, size properties (commented by Laurent Pinchart) >> - Adds OLED detection, TE gpio properties >> Changelog v4: >> - Moves CPU timings relevant properties from FIMD DT >> (commeted by Laurent Pinchart, Andrzej Hajda) >> >> Signed-off-by: YoungJun Cho <yj44.cho@samsung.com> >> Acked-by: Inki Dae <inki.dae@samsung.com> >> Acked-by: Kyungmin Park <kyungmin.park@samsung.com> >> --- >> .../devicetree/bindings/panel/samsung,s6e3fa0.txt | 63 ++++++++++++++++++++ >> 1 file changed, 63 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt >> >> diff --git a/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt >> new file mode 100644 >> index 0000000..9eeb38b >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt >> @@ -0,0 +1,63 @@ >> +Samsung S6E3FA0 AMOLED LCD 5.7 inch panel >> + >> +Required properties: >> + - compatible: "samsung,s6e3fa0" >> + - reg: the virtual channel number of a DSI peripheral >> + - vdd3-supply: core voltage supply >> + - vci-supply: voltage supply for analog circuits >> + - reset-gpio: a GPIO spec for the reset pin >> + - det-gpio: a GPIO spec for the OLED detection pin >> + - te-gpio: a GPIO spec for the TE pin > > Just FYI, according to DT documentation [1] gpio spec should be in form > [name]-gpios, however there is plenty bindings with -gpio suffix, so I > am not sure if it is really enforced. On the other side it is enforced > by descriptor based gpio framework[2]. Integer-based gpio framework > used in your driver is obsolete according to [2]. Yes, you're right. That is my mistake. They should be attached 's'. At first I used integer-based gpio framework and replaced to descriptor based one, but did not updated DT bindings. I'll update again. Thank you. Best regards YJ > > [1]: Documentation/devicetree/bindings/gpio/gpio.txt > [2]: Documentation/gpio/gpio.txt > > Regards > Andrzej > >> + - display-timings: timings for the connected panel as described by [1] >> + - cpu-timings: CPU interface timings for the connected panel, and it contains >> + following optional properties. >> + - cs-setup: clock cycles for the active period of address signal >> + enable until chip select is enable in CPU interface >> + - wr-setup: clock cycles for the active period of CS signal enable >> + until write signal is enable in CPU interface >> + - wr-act: clock cycles for the active period of CS enable in CPU >> + interface >> + - wr-hold: clock cycles for the active period of CS disable until >> + write signal is disable in CPU interface >> + >> +Optional properties: >> + >> +The device node can contain one 'port' child node with one child >> +'endpoint' node, according to the bindings defined in [2]. This >> +node should describe panel's video bus. >> + >> +[1]: Documentation/devicetree/bindings/video/display-timing.txt >> +[2]: Documentation/devicetree/bindings/media/video-interfaces.txt >> + >> +Example: >> + >> + panel@0 { >> + compatible = "samsung,s6e3fa0"; >> + reg = <0>; >> + vdd3-supply = <&vcclcd_reg>; >> + vci-supply = <&vlcd_reg>; >> + reset-gpio = <&gpy7 4 0>; >> + det-gpio = <&gpg0 6 0>; >> + te-gpio = <&gpd1 7 0>; >> + >> + display-timings { >> + timing0: timing-0 { >> + clock-frequency = <0>; >> + hactive = <1080>; >> + vactive = <1920>; >> + hfront-porch = <2>; >> + hback-porch = <2>; >> + hsync-len = <1>; >> + vfront-porch = <1>; >> + vback-porch = <4>; >> + vsync-len = <1>; >> + }; >> + }; >> + >> + cpu-timings { >> + cs-setup = <0>; >> + wr-setup = <0>; >> + wr-act = <1>; >> + wr-hold = <0>; >> + }; >> + }; >> > > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Apr 23, 2014 at 10:26:20AM +0900, YoungJun Cho wrote: > Hi Andrzej > > Thank you for comment. > > On 04/22/2014 11:02 PM, Andrzej Hajda wrote: > >On 04/21/2014 02:28 PM, YoungJun Cho wrote: > >>This patch adds DT bindings for s6e3fa0 panel. > >>The bindings describes panel resources, display timings and cpu timings. > >> > >>Changelog v2: > >>- Adds unit address (commented by Sachin Kamat) > >>Changelog v3: > >>- Removes optional delay, size properties (commented by Laurent Pinchart) > >>- Adds OLED detection, TE gpio properties > >>Changelog v4: > >>- Moves CPU timings relevant properties from FIMD DT > >> (commeted by Laurent Pinchart, Andrzej Hajda) > >> > >>Signed-off-by: YoungJun Cho <yj44.cho@samsung.com> > >>Acked-by: Inki Dae <inki.dae@samsung.com> > >>Acked-by: Kyungmin Park <kyungmin.park@samsung.com> > >>--- > >> .../devicetree/bindings/panel/samsung,s6e3fa0.txt | 63 ++++++++++++++++++++ > >> 1 file changed, 63 insertions(+) > >> create mode 100644 Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt > >> > >>diff --git a/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt > >>new file mode 100644 > >>index 0000000..9eeb38b > >>--- /dev/null > >>+++ b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt > >>@@ -0,0 +1,63 @@ > >>+Samsung S6E3FA0 AMOLED LCD 5.7 inch panel > >>+ > >>+Required properties: > >>+ - compatible: "samsung,s6e3fa0" > >>+ - reg: the virtual channel number of a DSI peripheral > >>+ - vdd3-supply: core voltage supply > >>+ - vci-supply: voltage supply for analog circuits > >>+ - reset-gpio: a GPIO spec for the reset pin > >>+ - det-gpio: a GPIO spec for the OLED detection pin > >>+ - te-gpio: a GPIO spec for the TE pin > > > >Just FYI, according to DT documentation [1] gpio spec should be in form > >[name]-gpios, however there is plenty bindings with -gpio suffix, so I > >am not sure if it is really enforced. On the other side it is enforced > >by descriptor based gpio framework[2]. Integer-based gpio framework > >used in your driver is obsolete according to [2]. > > Yes, you're right. That is my mistake. > They should be attached 's'. > At first I used integer-based gpio framework and replaced to descriptor > based one, but did not updated DT bindings. I've been working on a patch to support both *-gpios and *-gpio variants with the GPIO descriptor framework. *-gpios makes sense if there can indeed be several, but for something like hotplug detection I don't think there's a reason to require the plural. Furthermore some bindings already use the singular *-gpio anyway, so if we ever want to convert drivers using those bindings to the GPIO descriptor API we have to support that form too. Thierry
On 04/21/2014 02:28 PM, YoungJun Cho wrote: > This patch adds DT bindings for s6e3fa0 panel. > The bindings describes panel resources, display timings and cpu timings. > > Changelog v2: > - Adds unit address (commented by Sachin Kamat) > Changelog v3: > - Removes optional delay, size properties (commented by Laurent Pinchart) > - Adds OLED detection, TE gpio properties > Changelog v4: > - Moves CPU timings relevant properties from FIMD DT > (commeted by Laurent Pinchart, Andrzej Hajda) > > Signed-off-by: YoungJun Cho <yj44.cho@samsung.com> > Acked-by: Inki Dae <inki.dae@samsung.com> > Acked-by: Kyungmin Park <kyungmin.park@samsung.com> > --- > .../devicetree/bindings/panel/samsung,s6e3fa0.txt | 63 ++++++++++++++++++++ > 1 file changed, 63 insertions(+) > create mode 100644 Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt > > diff --git a/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt > new file mode 100644 > index 0000000..9eeb38b > --- /dev/null > +++ b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt > @@ -0,0 +1,63 @@ > +Samsung S6E3FA0 AMOLED LCD 5.7 inch panel > + > +Required properties: > + - compatible: "samsung,s6e3fa0" > + - reg: the virtual channel number of a DSI peripheral > + - vdd3-supply: core voltage supply > + - vci-supply: voltage supply for analog circuits > + - reset-gpio: a GPIO spec for the reset pin > + - det-gpio: a GPIO spec for the OLED detection pin > + - te-gpio: a GPIO spec for the TE pin > + - display-timings: timings for the connected panel as described by [1] Which properties of display-timings are relevant for CPU mode? I guess width and height. Anything more? > + - cpu-timings: CPU interface timings for the connected panel, and it contains > + following optional properties. > + - cs-setup: clock cycles for the active period of address signal > + enable until chip select is enable in CPU interface > + - wr-setup: clock cycles for the active period of CS signal enable > + until write signal is enable in CPU interface > + - wr-act: clock cycles for the active period of CS enable in CPU > + interface > + - wr-hold: clock cycles for the active period of CS disable until > + write signal is disable in CPU interface cpu-timings name does not sound to be related to display. I wonder if it would not be better to merge cpu-timings into display-timings but this will require more discussion I guess. If you want to stay with separate node please consider to make it optional as whole node or make some properties required. Making node required with all sub-properties optional is weird. By the way I hope all timings properties are generic for CPU mode, if not they should be prefixed by vendor or model. Regards Andrzej > + > +Optional properties: > + > +The device node can contain one 'port' child node with one child > +'endpoint' node, according to the bindings defined in [2]. This > +node should describe panel's video bus. > + > +[1]: Documentation/devicetree/bindings/video/display-timing.txt > +[2]: Documentation/devicetree/bindings/media/video-interfaces.txt > + > +Example: > + > + panel@0 { > + compatible = "samsung,s6e3fa0"; > + reg = <0>; > + vdd3-supply = <&vcclcd_reg>; > + vci-supply = <&vlcd_reg>; > + reset-gpio = <&gpy7 4 0>; > + det-gpio = <&gpg0 6 0>; > + te-gpio = <&gpd1 7 0>; > + > + display-timings { > + timing0: timing-0 { > + clock-frequency = <0>; > + hactive = <1080>; > + vactive = <1920>; > + hfront-porch = <2>; > + hback-porch = <2>; > + hsync-len = <1>; > + vfront-porch = <1>; > + vback-porch = <4>; > + vsync-len = <1>; > + }; > + }; > + > + cpu-timings { > + cs-setup = <0>; > + wr-setup = <0>; > + wr-act = <1>; > + wr-hold = <0>; > + }; > + }; > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Andrzej, On Wednesday 23 April 2014 11:02:21 Andrzej Hajda wrote: > On 04/21/2014 02:28 PM, YoungJun Cho wrote: > > This patch adds DT bindings for s6e3fa0 panel. > > The bindings describes panel resources, display timings and cpu timings. > > > > Changelog v2: > > - Adds unit address (commented by Sachin Kamat) > > Changelog v3: > > - Removes optional delay, size properties (commented by Laurent Pinchart) > > - Adds OLED detection, TE gpio properties > > Changelog v4: > > - Moves CPU timings relevant properties from FIMD DT > > > > (commeted by Laurent Pinchart, Andrzej Hajda) > > > > Signed-off-by: YoungJun Cho <yj44.cho@samsung.com> > > Acked-by: Inki Dae <inki.dae@samsung.com> > > Acked-by: Kyungmin Park <kyungmin.park@samsung.com> > > --- > > > > .../devicetree/bindings/panel/samsung,s6e3fa0.txt | 63 +++++++++++++++ > > 1 file changed, 63 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt> > > diff --git a/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt > > b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt new file > > mode 100644 > > index 0000000..9eeb38b > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt > > @@ -0,0 +1,63 @@ > > +Samsung S6E3FA0 AMOLED LCD 5.7 inch panel > > + > > +Required properties: > > + - compatible: "samsung,s6e3fa0" > > + - reg: the virtual channel number of a DSI peripheral > > + - vdd3-supply: core voltage supply > > + - vci-supply: voltage supply for analog circuits > > + - reset-gpio: a GPIO spec for the reset pin > > + - det-gpio: a GPIO spec for the OLED detection pin > > + - te-gpio: a GPIO spec for the TE pin > > + - display-timings: timings for the connected panel as described by [1] > > Which properties of display-timings are relevant for CPU mode? > I guess width and height. Anything more? > > > + - cpu-timings: CPU interface timings for the connected panel, and it > > contains > > + following optional properties. > > + - cs-setup: clock cycles for the active period of address > > signal > > + enable until chip select is enable in CPU interface > > + - wr-setup: clock cycles for the active period of CS signal > > enable > > + until write signal is enable in CPU interface > > + - wr-act: clock cycles for the active period of CS enable in > > CPU > > + interface What about calling this property wr-active ? I had called this wr-cycle in a previous implementation, that could be an option as well. > > + - wr-hold: clock cycles for the active period of CS disable > > until > > + write signal is disable in CPU interface > > cpu-timings name does not sound to be related to display. > I wonder if it would not be better to merge cpu-timings into > display-timings but this will require more discussion I guess. The panel has a memory-like interface with chip select, read/write and access strobe, address (1 bit) and data signals. The interface is defined in the MIPI DBI specification (a quick search turned up http://www.scribd.com/doc/206899854/MIPI-DPI-Specification-v2, there might be direct PDF downloads available), even if some panels implement a similar interface that predates MIPI DBI (with names such as i80 or SYS-80 probably due to the similarities with the 8051 external bus). The cpu-timings properties describe the read and write access timings for those signals, unrelated to the display video timings. I thus believe that they should be separate from the display timings. We will probably need to add properties for the read signal as well, but that can be done later. > If you want to stay with separate node please consider to make it > optional as whole node or make some properties required. Making node > required with all sub-properties optional is weird. > By the way I hope all timings properties are generic for CPU mode, > if not they should be prefixed by vendor or model. The timings description should be pretty generic I believe, especially given that the interface is standardized by the MIPI alliance. Could you have a quick look at the spec and share your opinion ? > > + > > +Optional properties: > > + > > +The device node can contain one 'port' child node with one child > > +'endpoint' node, according to the bindings defined in [2]. This > > +node should describe panel's video bus. > > + > > +[1]: Documentation/devicetree/bindings/video/display-timing.txt > > +[2]: Documentation/devicetree/bindings/media/video-interfaces.txt > > + > > +Example: > > + > > + panel@0 { > > + compatible = "samsung,s6e3fa0"; > > + reg = <0>; > > + vdd3-supply = <&vcclcd_reg>; > > + vci-supply = <&vlcd_reg>; > > + reset-gpio = <&gpy7 4 0>; > > + det-gpio = <&gpg0 6 0>; > > + te-gpio = <&gpd1 7 0>; > > + > > + display-timings { > > + timing0: timing-0 { > > + clock-frequency = <0>; > > + hactive = <1080>; > > + vactive = <1920>; > > + hfront-porch = <2>; > > + hback-porch = <2>; > > + hsync-len = <1>; > > + vfront-porch = <1>; > > + vback-porch = <4>; > > + vsync-len = <1>; > > + }; > > + }; > > + > > + cpu-timings { > > + cs-setup = <0>; > > + wr-setup = <0>; > > + wr-act = <1>; > > + wr-hold = <0>; > > + }; > > + };
On 04/23/2014 01:34 PM, Laurent Pinchart wrote: > Hi Andrzej, > > On Wednesday 23 April 2014 11:02:21 Andrzej Hajda wrote: >> On 04/21/2014 02:28 PM, YoungJun Cho wrote: >>> This patch adds DT bindings for s6e3fa0 panel. >>> The bindings describes panel resources, display timings and cpu timings. >>> >>> Changelog v2: >>> - Adds unit address (commented by Sachin Kamat) >>> Changelog v3: >>> - Removes optional delay, size properties (commented by Laurent Pinchart) >>> - Adds OLED detection, TE gpio properties >>> Changelog v4: >>> - Moves CPU timings relevant properties from FIMD DT >>> >>> (commeted by Laurent Pinchart, Andrzej Hajda) >>> >>> Signed-off-by: YoungJun Cho <yj44.cho@samsung.com> >>> Acked-by: Inki Dae <inki.dae@samsung.com> >>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com> >>> --- >>> >>> .../devicetree/bindings/panel/samsung,s6e3fa0.txt | 63 +++++++++++++++ >>> 1 file changed, 63 insertions(+) >>> create mode 100644 >>> Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt> >>> diff --git a/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt >>> b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt new file >>> mode 100644 >>> index 0000000..9eeb38b >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt >>> @@ -0,0 +1,63 @@ >>> +Samsung S6E3FA0 AMOLED LCD 5.7 inch panel >>> + >>> +Required properties: >>> + - compatible: "samsung,s6e3fa0" >>> + - reg: the virtual channel number of a DSI peripheral >>> + - vdd3-supply: core voltage supply >>> + - vci-supply: voltage supply for analog circuits >>> + - reset-gpio: a GPIO spec for the reset pin >>> + - det-gpio: a GPIO spec for the OLED detection pin >>> + - te-gpio: a GPIO spec for the TE pin >>> + - display-timings: timings for the connected panel as described by [1] >> Which properties of display-timings are relevant for CPU mode? >> I guess width and height. Anything more? >> >>> + - cpu-timings: CPU interface timings for the connected panel, and it >>> contains >>> + following optional properties. >>> + - cs-setup: clock cycles for the active period of address >>> signal >>> + enable until chip select is enable in CPU interface >>> + - wr-setup: clock cycles for the active period of CS signal >>> enable >>> + until write signal is enable in CPU interface >>> + - wr-act: clock cycles for the active period of CS enable in >>> CPU >>> + interface > What about calling this property wr-active ? I had called this wr-cycle in a > previous implementation, that could be an option as well. > >>> + - wr-hold: clock cycles for the active period of CS disable >>> until >>> + write signal is disable in CPU interface >> cpu-timings name does not sound to be related to display. >> I wonder if it would not be better to merge cpu-timings into >> display-timings but this will require more discussion I guess. > The panel has a memory-like interface with chip select, read/write and access > strobe, address (1 bit) and data signals. The interface is defined in the MIPI > DBI specification (a quick search turned up > http://www.scribd.com/doc/206899854/MIPI-DPI-Specification-v2, there might be > direct PDF downloads available), even if some panels implement a similar > interface that predates MIPI DBI (with names such as i80 or SYS-80 probably > due to the similarities with the 8051 external bus). > > The cpu-timings properties describe the read and write access timings for > those signals, unrelated to the display video timings. I thus believe that > they should be separate from the display timings. We will probably need to add > properties for the read signal as well, but that can be done later. cpu-timings suggests these timings are for CPU, but they are for display panel in CPU mode. I though rather about something like display-cpu-timings or i80-timings, just to avoid confusion. Regards Andrzej >> If you want to stay with separate node please consider to make it >> optional as whole node or make some properties required. Making node >> required with all sub-properties optional is weird. >> By the way I hope all timings properties are generic for CPU mode, >> if not they should be prefixed by vendor or model. > The timings description should be pretty generic I believe, especially given > that the interface is standardized by the MIPI alliance. Could you have a > quick look at the spec and share your opinion ? > >>> + >>> +Optional properties: >>> + >>> +The device node can contain one 'port' child node with one child >>> +'endpoint' node, according to the bindings defined in [2]. This >>> +node should describe panel's video bus. >>> + >>> +[1]: Documentation/devicetree/bindings/video/display-timing.txt >>> +[2]: Documentation/devicetree/bindings/media/video-interfaces.txt >>> + >>> +Example: >>> + >>> + panel@0 { >>> + compatible = "samsung,s6e3fa0"; >>> + reg = <0>; >>> + vdd3-supply = <&vcclcd_reg>; >>> + vci-supply = <&vlcd_reg>; >>> + reset-gpio = <&gpy7 4 0>; >>> + det-gpio = <&gpg0 6 0>; >>> + te-gpio = <&gpd1 7 0>; >>> + >>> + display-timings { >>> + timing0: timing-0 { >>> + clock-frequency = <0>; >>> + hactive = <1080>; >>> + vactive = <1920>; >>> + hfront-porch = <2>; >>> + hback-porch = <2>; >>> + hsync-len = <1>; >>> + vfront-porch = <1>; >>> + vback-porch = <4>; >>> + vsync-len = <1>; >>> + }; >>> + }; >>> + >>> + cpu-timings { >>> + cs-setup = <0>; >>> + wr-setup = <0>; >>> + wr-act = <1>; >>> + wr-hold = <0>; >>> + }; >>> + }; -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Andrzej, On Wednesday 23 April 2014 14:48:31 Andrzej Hajda wrote: > On 04/23/2014 01:34 PM, Laurent Pinchart wrote: > > On Wednesday 23 April 2014 11:02:21 Andrzej Hajda wrote: > >> On 04/21/2014 02:28 PM, YoungJun Cho wrote: > >>> This patch adds DT bindings for s6e3fa0 panel. > >>> The bindings describes panel resources, display timings and cpu timings. > >>> > >>> Changelog v2: > >>> - Adds unit address (commented by Sachin Kamat) > >>> Changelog v3: > >>> - Removes optional delay, size properties (commented by Laurent > >>> Pinchart) > >>> - Adds OLED detection, TE gpio properties > >>> Changelog v4: > >>> - Moves CPU timings relevant properties from FIMD DT > >>> > >>> (commeted by Laurent Pinchart, Andrzej Hajda) > >>> > >>> Signed-off-by: YoungJun Cho <yj44.cho@samsung.com> > >>> Acked-by: Inki Dae <inki.dae@samsung.com> > >>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com> > >>> --- > >>> > >>> .../devicetree/bindings/panel/samsung,s6e3fa0.txt | 63 > >>> +++++++++++++++ > >>> > >>> 1 file changed, 63 insertions(+) > >>> > >>> create mode 100644 > >>> Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt> > >>> > >>> diff --git a/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt > >>> b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt new file > >>> mode 100644 > >>> index 0000000..9eeb38b > >>> --- /dev/null > >>> +++ b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt > >>> @@ -0,0 +1,63 @@ > >>> +Samsung S6E3FA0 AMOLED LCD 5.7 inch panel > >>> + > >>> +Required properties: > >>> + - compatible: "samsung,s6e3fa0" > >>> + - reg: the virtual channel number of a DSI peripheral > >>> + - vdd3-supply: core voltage supply > >>> + - vci-supply: voltage supply for analog circuits > >>> + - reset-gpio: a GPIO spec for the reset pin > >>> + - det-gpio: a GPIO spec for the OLED detection pin > >>> + - te-gpio: a GPIO spec for the TE pin > >>> + - display-timings: timings for the connected panel as described by > >>> [1] > >> > >> Which properties of display-timings are relevant for CPU mode? > >> I guess width and height. Anything more? > >> > >>> + - cpu-timings: CPU interface timings for the connected panel, and it > >>> contains > >>> + following optional properties. > >>> + - cs-setup: clock cycles for the active period of address > >>> signal > >>> + enable until chip select is enable in CPU interface > >>> + - wr-setup: clock cycles for the active period of CS signal > >>> enable > >>> + until write signal is enable in CPU interface > >>> + - wr-act: clock cycles for the active period of CS enable in > >>> CPU > >>> + interface > > > > What about calling this property wr-active ? I had called this wr-cycle in > > a previous implementation, that could be an option as well. > > > >>> + - wr-hold: clock cycles for the active period of CS disable > >>> until > >>> + write signal is disable in CPU interface > >> > >> cpu-timings name does not sound to be related to display. > >> I wonder if it would not be better to merge cpu-timings into > >> display-timings but this will require more discussion I guess. > > > > The panel has a memory-like interface with chip select, read/write and > > access strobe, address (1 bit) and data signals. The interface is defined > > in the MIPI DBI specification (a quick search turned up > > http://www.scribd.com/doc/206899854/MIPI-DPI-Specification-v2, there might > > be direct PDF downloads available), even if some panels implement a > > similar interface that predates MIPI DBI (with names such as i80 or > > SYS-80 probably due to the similarities with the 8051 external bus). > > > > The cpu-timings properties describe the read and write access timings for > > those signals, unrelated to the display video timings. I thus believe that > > they should be separate from the display timings. We will probably need to > > add properties for the read signal as well, but that can be done later. > > cpu-timings suggests these timings are for CPU, but they are for display > panel in CPU mode. > I though rather about something like display-cpu-timings or i80-timings, > just to avoid confusion. mipi-dbi-timings maybe ? > >> If you want to stay with separate node please consider to make it > >> optional as whole node or make some properties required. Making node > >> required with all sub-properties optional is weird. > >> By the way I hope all timings properties are generic for CPU mode, > >> if not they should be prefixed by vendor or model. > > > > The timings description should be pretty generic I believe, especially > > given that the interface is standardized by the MIPI alliance. Could you > > have a quick look at the spec and share your opinion ? > > > >>> + > >>> +Optional properties: > >>> + > >>> +The device node can contain one 'port' child node with one child > >>> +'endpoint' node, according to the bindings defined in [2]. This > >>> +node should describe panel's video bus. > >>> + > >>> +[1]: Documentation/devicetree/bindings/video/display-timing.txt > >>> +[2]: Documentation/devicetree/bindings/media/video-interfaces.txt > >>> + > >>> +Example: > >>> + > >>> + panel@0 { > >>> + compatible = "samsung,s6e3fa0"; > >>> + reg = <0>; > >>> + vdd3-supply = <&vcclcd_reg>; > >>> + vci-supply = <&vlcd_reg>; > >>> + reset-gpio = <&gpy7 4 0>; > >>> + det-gpio = <&gpg0 6 0>; > >>> + te-gpio = <&gpd1 7 0>; > >>> + > >>> + display-timings { > >>> + timing0: timing-0 { > >>> + clock-frequency = <0>; > >>> + hactive = <1080>; > >>> + vactive = <1920>; > >>> + hfront-porch = <2>; > >>> + hback-porch = <2>; > >>> + hsync-len = <1>; > >>> + vfront-porch = <1>; > >>> + vback-porch = <4>; > >>> + vsync-len = <1>; > >>> + }; > >>> + }; > >>> + > >>> + cpu-timings { > >>> + cs-setup = <0>; > >>> + wr-setup = <0>; > >>> + wr-act = <1>; > >>> + wr-hold = <0>; > >>> + }; > >>> + };
On 04/23/2014 02:55 PM, Laurent Pinchart wrote: > Hi Andrzej, > > On Wednesday 23 April 2014 14:48:31 Andrzej Hajda wrote: >> On 04/23/2014 01:34 PM, Laurent Pinchart wrote: >>> On Wednesday 23 April 2014 11:02:21 Andrzej Hajda wrote: >>>> On 04/21/2014 02:28 PM, YoungJun Cho wrote: >>>>> This patch adds DT bindings for s6e3fa0 panel. >>>>> The bindings describes panel resources, display timings and cpu timings. >>>>> >>>>> Changelog v2: >>>>> - Adds unit address (commented by Sachin Kamat) >>>>> Changelog v3: >>>>> - Removes optional delay, size properties (commented by Laurent >>>>> Pinchart) >>>>> - Adds OLED detection, TE gpio properties >>>>> Changelog v4: >>>>> - Moves CPU timings relevant properties from FIMD DT >>>>> >>>>> (commeted by Laurent Pinchart, Andrzej Hajda) >>>>> >>>>> Signed-off-by: YoungJun Cho <yj44.cho@samsung.com> >>>>> Acked-by: Inki Dae <inki.dae@samsung.com> >>>>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com> >>>>> --- >>>>> >>>>> .../devicetree/bindings/panel/samsung,s6e3fa0.txt | 63 >>>>> +++++++++++++++ >>>>> >>>>> 1 file changed, 63 insertions(+) >>>>> >>>>> create mode 100644 >>>>> Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt> >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt >>>>> b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt new file >>>>> mode 100644 >>>>> index 0000000..9eeb38b >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt >>>>> @@ -0,0 +1,63 @@ >>>>> +Samsung S6E3FA0 AMOLED LCD 5.7 inch panel >>>>> + >>>>> +Required properties: >>>>> + - compatible: "samsung,s6e3fa0" >>>>> + - reg: the virtual channel number of a DSI peripheral >>>>> + - vdd3-supply: core voltage supply >>>>> + - vci-supply: voltage supply for analog circuits >>>>> + - reset-gpio: a GPIO spec for the reset pin >>>>> + - det-gpio: a GPIO spec for the OLED detection pin >>>>> + - te-gpio: a GPIO spec for the TE pin >>>>> + - display-timings: timings for the connected panel as described by >>>>> [1] >>>> Which properties of display-timings are relevant for CPU mode? >>>> I guess width and height. Anything more? >>>> >>>>> + - cpu-timings: CPU interface timings for the connected panel, and it >>>>> contains >>>>> + following optional properties. >>>>> + - cs-setup: clock cycles for the active period of address >>>>> signal >>>>> + enable until chip select is enable in CPU interface >>>>> + - wr-setup: clock cycles for the active period of CS signal >>>>> enable >>>>> + until write signal is enable in CPU interface >>>>> + - wr-act: clock cycles for the active period of CS enable in >>>>> CPU >>>>> + interface >>> What about calling this property wr-active ? I had called this wr-cycle in >>> a previous implementation, that could be an option as well. >>> >>>>> + - wr-hold: clock cycles for the active period of CS disable >>>>> until >>>>> + write signal is disable in CPU interface >>>> cpu-timings name does not sound to be related to display. >>>> I wonder if it would not be better to merge cpu-timings into >>>> display-timings but this will require more discussion I guess. >>> The panel has a memory-like interface with chip select, read/write and >>> access strobe, address (1 bit) and data signals. The interface is defined >>> in the MIPI DBI specification (a quick search turned up >>> http://www.scribd.com/doc/206899854/MIPI-DPI-Specification-v2, there might >>> be direct PDF downloads available), even if some panels implement a >>> similar interface that predates MIPI DBI (with names such as i80 or >>> SYS-80 probably due to the similarities with the 8051 external bus). >>> >>> The cpu-timings properties describe the read and write access timings for >>> those signals, unrelated to the display video timings. I thus believe that >>> they should be separate from the display timings. We will probably need to >>> add properties for the read signal as well, but that can be done later. >> cpu-timings suggests these timings are for CPU, but they are for display >> panel in CPU mode. >> I though rather about something like display-cpu-timings or i80-timings, >> just to avoid confusion. > mipi-dbi-timings maybe ? It looks OK. I guess only hactive, and vactive props of display-timings are used, maybe some flags? I wonder if it would not be better to drop display-timings node and place all props into mipi-dbi-timings or mipi-dbi-settings node. Or rather all timings props should be put into port/endpoint node with or without any container node. Regards Andrzej > >>>> If you want to stay with separate node please consider to make it >>>> optional as whole node or make some properties required. Making node >>>> required with all sub-properties optional is weird. >>>> By the way I hope all timings properties are generic for CPU mode, >>>> if not they should be prefixed by vendor or model. >>> The timings description should be pretty generic I believe, especially >>> given that the interface is standardized by the MIPI alliance. Could you >>> have a quick look at the spec and share your opinion ? >>> >>>>> + >>>>> +Optional properties: >>>>> + >>>>> +The device node can contain one 'port' child node with one child >>>>> +'endpoint' node, according to the bindings defined in [2]. This >>>>> +node should describe panel's video bus. >>>>> + >>>>> +[1]: Documentation/devicetree/bindings/video/display-timing.txt >>>>> +[2]: Documentation/devicetree/bindings/media/video-interfaces.txt >>>>> + >>>>> +Example: >>>>> + >>>>> + panel@0 { >>>>> + compatible = "samsung,s6e3fa0"; >>>>> + reg = <0>; >>>>> + vdd3-supply = <&vcclcd_reg>; >>>>> + vci-supply = <&vlcd_reg>; >>>>> + reset-gpio = <&gpy7 4 0>; >>>>> + det-gpio = <&gpg0 6 0>; >>>>> + te-gpio = <&gpd1 7 0>; >>>>> + >>>>> + display-timings { >>>>> + timing0: timing-0 { >>>>> + clock-frequency = <0>; >>>>> + hactive = <1080>; >>>>> + vactive = <1920>; >>>>> + hfront-porch = <2>; >>>>> + hback-porch = <2>; >>>>> + hsync-len = <1>; >>>>> + vfront-porch = <1>; >>>>> + vback-porch = <4>; >>>>> + vsync-len = <1>; >>>>> + }; >>>>> + }; >>>>> + >>>>> + cpu-timings { >>>>> + cs-setup = <0>; >>>>> + wr-setup = <0>; >>>>> + wr-act = <1>; >>>>> + wr-hold = <0>; >>>>> + }; >>>>> + }; -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Andrzej, Thank you for comments. On 04/23/2014 06:02 PM, Andrzej Hajda wrote: > On 04/21/2014 02:28 PM, YoungJun Cho wrote: >> This patch adds DT bindings for s6e3fa0 panel. >> The bindings describes panel resources, display timings and cpu timings. >> >> Changelog v2: >> - Adds unit address (commented by Sachin Kamat) >> Changelog v3: >> - Removes optional delay, size properties (commented by Laurent Pinchart) >> - Adds OLED detection, TE gpio properties >> Changelog v4: >> - Moves CPU timings relevant properties from FIMD DT >> (commeted by Laurent Pinchart, Andrzej Hajda) >> >> Signed-off-by: YoungJun Cho <yj44.cho@samsung.com> >> Acked-by: Inki Dae <inki.dae@samsung.com> >> Acked-by: Kyungmin Park <kyungmin.park@samsung.com> >> --- >> .../devicetree/bindings/panel/samsung,s6e3fa0.txt | 63 ++++++++++++++++++++ >> 1 file changed, 63 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt >> >> diff --git a/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt >> new file mode 100644 >> index 0000000..9eeb38b >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt >> @@ -0,0 +1,63 @@ >> +Samsung S6E3FA0 AMOLED LCD 5.7 inch panel >> + >> +Required properties: >> + - compatible: "samsung,s6e3fa0" >> + - reg: the virtual channel number of a DSI peripheral >> + - vdd3-supply: core voltage supply >> + - vci-supply: voltage supply for analog circuits >> + - reset-gpio: a GPIO spec for the reset pin >> + - det-gpio: a GPIO spec for the OLED detection pin >> + - te-gpio: a GPIO spec for the TE pin >> + - display-timings: timings for the connected panel as described by [1] > > Which properties of display-timings are relevant for CPU mode? > I guess width and height. Anything more? The CPU interface panel also requires hfront-porch, hback-porch, hsync-len, vfront-porch, vback-porch and vsync-len. > >> + - cpu-timings: CPU interface timings for the connected panel, and it contains >> + following optional properties. >> + - cs-setup: clock cycles for the active period of address signal >> + enable until chip select is enable in CPU interface >> + - wr-setup: clock cycles for the active period of CS signal enable >> + until write signal is enable in CPU interface >> + - wr-act: clock cycles for the active period of CS enable in CPU >> + interface >> + - wr-hold: clock cycles for the active period of CS disable until >> + write signal is disable in CPU interface > > cpu-timings name does not sound to be related to display. > I wonder if it would not be better to merge cpu-timings into > display-timings but this will require more discussion I guess. > > If you want to stay with separate node please consider to make it > optional as whole node or make some properties required. Making node > required with all sub-properties optional is weird. Yes, I'll fix. Thank you. Best regards YJ > By the way I hope all timings properties are generic for CPU mode, > if not they should be prefixed by vendor or model. > > Regards > Andrzej > >> + >> +Optional properties: >> + >> +The device node can contain one 'port' child node with one child >> +'endpoint' node, according to the bindings defined in [2]. This >> +node should describe panel's video bus. >> + >> +[1]: Documentation/devicetree/bindings/video/display-timing.txt >> +[2]: Documentation/devicetree/bindings/media/video-interfaces.txt >> + >> +Example: >> + >> + panel@0 { >> + compatible = "samsung,s6e3fa0"; >> + reg = <0>; >> + vdd3-supply = <&vcclcd_reg>; >> + vci-supply = <&vlcd_reg>; >> + reset-gpio = <&gpy7 4 0>; >> + det-gpio = <&gpg0 6 0>; >> + te-gpio = <&gpd1 7 0>; >> + >> + display-timings { >> + timing0: timing-0 { >> + clock-frequency = <0>; >> + hactive = <1080>; >> + vactive = <1920>; >> + hfront-porch = <2>; >> + hback-porch = <2>; >> + hsync-len = <1>; >> + vfront-porch = <1>; >> + vback-porch = <4>; >> + vsync-len = <1>; >> + }; >> + }; >> + >> + cpu-timings { >> + cs-setup = <0>; >> + wr-setup = <0>; >> + wr-act = <1>; >> + wr-hold = <0>; >> + }; >> + }; >> > > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Laurent, Thank you for comments. On 04/23/2014 08:34 PM, Laurent Pinchart wrote: > Hi Andrzej, > > On Wednesday 23 April 2014 11:02:21 Andrzej Hajda wrote: >> On 04/21/2014 02:28 PM, YoungJun Cho wrote: >>> This patch adds DT bindings for s6e3fa0 panel. >>> The bindings describes panel resources, display timings and cpu timings. >>> >>> Changelog v2: >>> - Adds unit address (commented by Sachin Kamat) >>> Changelog v3: >>> - Removes optional delay, size properties (commented by Laurent Pinchart) >>> - Adds OLED detection, TE gpio properties >>> Changelog v4: >>> - Moves CPU timings relevant properties from FIMD DT >>> >>> (commeted by Laurent Pinchart, Andrzej Hajda) >>> >>> Signed-off-by: YoungJun Cho <yj44.cho@samsung.com> >>> Acked-by: Inki Dae <inki.dae@samsung.com> >>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com> >>> --- >>> >>> .../devicetree/bindings/panel/samsung,s6e3fa0.txt | 63 +++++++++++++++ >>> 1 file changed, 63 insertions(+) >>> create mode 100644 >>> Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt> >>> diff --git a/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt >>> b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt new file >>> mode 100644 >>> index 0000000..9eeb38b >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt >>> @@ -0,0 +1,63 @@ >>> +Samsung S6E3FA0 AMOLED LCD 5.7 inch panel >>> + >>> +Required properties: >>> + - compatible: "samsung,s6e3fa0" >>> + - reg: the virtual channel number of a DSI peripheral >>> + - vdd3-supply: core voltage supply >>> + - vci-supply: voltage supply for analog circuits >>> + - reset-gpio: a GPIO spec for the reset pin >>> + - det-gpio: a GPIO spec for the OLED detection pin >>> + - te-gpio: a GPIO spec for the TE pin >>> + - display-timings: timings for the connected panel as described by [1] >> >> Which properties of display-timings are relevant for CPU mode? >> I guess width and height. Anything more? >> >>> + - cpu-timings: CPU interface timings for the connected panel, and it >>> contains >>> + following optional properties. >>> + - cs-setup: clock cycles for the active period of address >>> signal >>> + enable until chip select is enable in CPU interface >>> + - wr-setup: clock cycles for the active period of CS signal >>> enable >>> + until write signal is enable in CPU interface >>> + - wr-act: clock cycles for the active period of CS enable in >>> CPU >>> + interface > > What about calling this property wr-active ? I had called this wr-cycle in a > previous implementation, that could be an option as well. Okay, I'll use wr-active. It's better. > >>> + - wr-hold: clock cycles for the active period of CS disable >>> until >>> + write signal is disable in CPU interface >> >> cpu-timings name does not sound to be related to display. >> I wonder if it would not be better to merge cpu-timings into >> display-timings but this will require more discussion I guess. > > The panel has a memory-like interface with chip select, read/write and access > strobe, address (1 bit) and data signals. The interface is defined in the MIPI > DBI specification (a quick search turned up > http://www.scribd.com/doc/206899854/MIPI-DPI-Specification-v2, there might be > direct PDF downloads available), even if some panels implement a similar > interface that predates MIPI DBI (with names such as i80 or SYS-80 probably > due to the similarities with the 8051 external bus). > > The cpu-timings properties describe the read and write access timings for > those signals, unrelated to the display video timings. I thus believe that > they should be separate from the display timings. We will probably need to add > properties for the read signal as well, but that can be done later. Yes, as I wrote another thread before, this cpu interface timings is additional one. The video timings is required also. Thank you. Best regards YJ > >> If you want to stay with separate node please consider to make it >> optional as whole node or make some properties required. Making node >> required with all sub-properties optional is weird. >> By the way I hope all timings properties are generic for CPU mode, >> if not they should be prefixed by vendor or model. > > The timings description should be pretty generic I believe, especially given > that the interface is standardized by the MIPI alliance. Could you have a > quick look at the spec and share your opinion ? > >>> + >>> +Optional properties: >>> + >>> +The device node can contain one 'port' child node with one child >>> +'endpoint' node, according to the bindings defined in [2]. This >>> +node should describe panel's video bus. >>> + >>> +[1]: Documentation/devicetree/bindings/video/display-timing.txt >>> +[2]: Documentation/devicetree/bindings/media/video-interfaces.txt >>> + >>> +Example: >>> + >>> + panel@0 { >>> + compatible = "samsung,s6e3fa0"; >>> + reg = <0>; >>> + vdd3-supply = <&vcclcd_reg>; >>> + vci-supply = <&vlcd_reg>; >>> + reset-gpio = <&gpy7 4 0>; >>> + det-gpio = <&gpg0 6 0>; >>> + te-gpio = <&gpd1 7 0>; >>> + >>> + display-timings { >>> + timing0: timing-0 { >>> + clock-frequency = <0>; >>> + hactive = <1080>; >>> + vactive = <1920>; >>> + hfront-porch = <2>; >>> + hback-porch = <2>; >>> + hsync-len = <1>; >>> + vfront-porch = <1>; >>> + vback-porch = <4>; >>> + vsync-len = <1>; >>> + }; >>> + }; >>> + >>> + cpu-timings { >>> + cs-setup = <0>; >>> + wr-setup = <0>; >>> + wr-act = <1>; >>> + wr-hold = <0>; >>> + }; >>> + }; > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Andrzej, On 04/23/2014 10:33 PM, Andrzej Hajda wrote: > On 04/23/2014 02:55 PM, Laurent Pinchart wrote: >> Hi Andrzej, >> >> On Wednesday 23 April 2014 14:48:31 Andrzej Hajda wrote: >>> On 04/23/2014 01:34 PM, Laurent Pinchart wrote: >>>> On Wednesday 23 April 2014 11:02:21 Andrzej Hajda wrote: >>>>> On 04/21/2014 02:28 PM, YoungJun Cho wrote: >>>>>> This patch adds DT bindings for s6e3fa0 panel. >>>>>> The bindings describes panel resources, display timings and cpu timings. >>>>>> >>>>>> Changelog v2: >>>>>> - Adds unit address (commented by Sachin Kamat) >>>>>> Changelog v3: >>>>>> - Removes optional delay, size properties (commented by Laurent >>>>>> Pinchart) >>>>>> - Adds OLED detection, TE gpio properties >>>>>> Changelog v4: >>>>>> - Moves CPU timings relevant properties from FIMD DT >>>>>> >>>>>> (commeted by Laurent Pinchart, Andrzej Hajda) >>>>>> >>>>>> Signed-off-by: YoungJun Cho <yj44.cho@samsung.com> >>>>>> Acked-by: Inki Dae <inki.dae@samsung.com> >>>>>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com> >>>>>> --- >>>>>> >>>>>> .../devicetree/bindings/panel/samsung,s6e3fa0.txt | 63 >>>>>> +++++++++++++++ >>>>>> >>>>>> 1 file changed, 63 insertions(+) >>>>>> >>>>>> create mode 100644 >>>>>> Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt> >>>>>> >>>>>> diff --git a/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt >>>>>> b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt new file >>>>>> mode 100644 >>>>>> index 0000000..9eeb38b >>>>>> --- /dev/null >>>>>> +++ b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt >>>>>> @@ -0,0 +1,63 @@ >>>>>> +Samsung S6E3FA0 AMOLED LCD 5.7 inch panel >>>>>> + >>>>>> +Required properties: >>>>>> + - compatible: "samsung,s6e3fa0" >>>>>> + - reg: the virtual channel number of a DSI peripheral >>>>>> + - vdd3-supply: core voltage supply >>>>>> + - vci-supply: voltage supply for analog circuits >>>>>> + - reset-gpio: a GPIO spec for the reset pin >>>>>> + - det-gpio: a GPIO spec for the OLED detection pin >>>>>> + - te-gpio: a GPIO spec for the TE pin >>>>>> + - display-timings: timings for the connected panel as described by >>>>>> [1] >>>>> Which properties of display-timings are relevant for CPU mode? >>>>> I guess width and height. Anything more? >>>>> >>>>>> + - cpu-timings: CPU interface timings for the connected panel, and it >>>>>> contains >>>>>> + following optional properties. >>>>>> + - cs-setup: clock cycles for the active period of address >>>>>> signal >>>>>> + enable until chip select is enable in CPU interface >>>>>> + - wr-setup: clock cycles for the active period of CS signal >>>>>> enable >>>>>> + until write signal is enable in CPU interface >>>>>> + - wr-act: clock cycles for the active period of CS enable in >>>>>> CPU >>>>>> + interface >>>> What about calling this property wr-active ? I had called this wr-cycle in >>>> a previous implementation, that could be an option as well. >>>> >>>>>> + - wr-hold: clock cycles for the active period of CS disable >>>>>> until >>>>>> + write signal is disable in CPU interface >>>>> cpu-timings name does not sound to be related to display. >>>>> I wonder if it would not be better to merge cpu-timings into >>>>> display-timings but this will require more discussion I guess. >>>> The panel has a memory-like interface with chip select, read/write and >>>> access strobe, address (1 bit) and data signals. The interface is defined >>>> in the MIPI DBI specification (a quick search turned up >>>> http://www.scribd.com/doc/206899854/MIPI-DPI-Specification-v2, there might >>>> be direct PDF downloads available), even if some panels implement a >>>> similar interface that predates MIPI DBI (with names such as i80 or >>>> SYS-80 probably due to the similarities with the 8051 external bus). >>>> >>>> The cpu-timings properties describe the read and write access timings for >>>> those signals, unrelated to the display video timings. I thus believe that >>>> they should be separate from the display timings. We will probably need to >>>> add properties for the read signal as well, but that can be done later. >>> cpu-timings suggests these timings are for CPU, but they are for display >>> panel in CPU mode. >>> I though rather about something like display-cpu-timings or i80-timings, >>> just to avoid confusion. >> mipi-dbi-timings maybe ? > > It looks OK. Isn't it too generic? I prefer cpu-mode-timings. > > I guess only hactive, and vactive props of display-timings are used, > maybe some flags? > I wonder if it would not be better to drop display-timings node > and place all props into mipi-dbi-timings or mipi-dbi-settings node. > Or rather all timings props should be put into port/endpoint node with > or without > any container node. The current 'display-timings' is for video mode interface, but cpu mode interface requires it. If we use mipi-dbi-timings, we should change previous video mode relevant ones at all. Thank you. Best regards YJ > > Regards > Andrzej > >> >>>>> If you want to stay with separate node please consider to make it >>>>> optional as whole node or make some properties required. Making node >>>>> required with all sub-properties optional is weird. >>>>> By the way I hope all timings properties are generic for CPU mode, >>>>> if not they should be prefixed by vendor or model. >>>> The timings description should be pretty generic I believe, especially >>>> given that the interface is standardized by the MIPI alliance. Could you >>>> have a quick look at the spec and share your opinion ? >>>> >>>>>> + >>>>>> +Optional properties: >>>>>> + >>>>>> +The device node can contain one 'port' child node with one child >>>>>> +'endpoint' node, according to the bindings defined in [2]. This >>>>>> +node should describe panel's video bus. >>>>>> + >>>>>> +[1]: Documentation/devicetree/bindings/video/display-timing.txt >>>>>> +[2]: Documentation/devicetree/bindings/media/video-interfaces.txt >>>>>> + >>>>>> +Example: >>>>>> + >>>>>> + panel@0 { >>>>>> + compatible = "samsung,s6e3fa0"; >>>>>> + reg = <0>; >>>>>> + vdd3-supply = <&vcclcd_reg>; >>>>>> + vci-supply = <&vlcd_reg>; >>>>>> + reset-gpio = <&gpy7 4 0>; >>>>>> + det-gpio = <&gpg0 6 0>; >>>>>> + te-gpio = <&gpd1 7 0>; >>>>>> + >>>>>> + display-timings { >>>>>> + timing0: timing-0 { >>>>>> + clock-frequency = <0>; >>>>>> + hactive = <1080>; >>>>>> + vactive = <1920>; >>>>>> + hfront-porch = <2>; >>>>>> + hback-porch = <2>; >>>>>> + hsync-len = <1>; >>>>>> + vfront-porch = <1>; >>>>>> + vback-porch = <4>; >>>>>> + vsync-len = <1>; >>>>>> + }; >>>>>> + }; >>>>>> + >>>>>> + cpu-timings { >>>>>> + cs-setup = <0>; >>>>>> + wr-setup = <0>; >>>>>> + wr-act = <1>; >>>>>> + wr-hold = <0>; >>>>>> + }; >>>>>> + }; > > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt new file mode 100644 index 0000000..9eeb38b --- /dev/null +++ b/Documentation/devicetree/bindings/panel/samsung,s6e3fa0.txt @@ -0,0 +1,63 @@ +Samsung S6E3FA0 AMOLED LCD 5.7 inch panel + +Required properties: + - compatible: "samsung,s6e3fa0" + - reg: the virtual channel number of a DSI peripheral + - vdd3-supply: core voltage supply + - vci-supply: voltage supply for analog circuits + - reset-gpio: a GPIO spec for the reset pin + - det-gpio: a GPIO spec for the OLED detection pin + - te-gpio: a GPIO spec for the TE pin + - display-timings: timings for the connected panel as described by [1] + - cpu-timings: CPU interface timings for the connected panel, and it contains + following optional properties. + - cs-setup: clock cycles for the active period of address signal + enable until chip select is enable in CPU interface + - wr-setup: clock cycles for the active period of CS signal enable + until write signal is enable in CPU interface + - wr-act: clock cycles for the active period of CS enable in CPU + interface + - wr-hold: clock cycles for the active period of CS disable until + write signal is disable in CPU interface + +Optional properties: + +The device node can contain one 'port' child node with one child +'endpoint' node, according to the bindings defined in [2]. This +node should describe panel's video bus. + +[1]: Documentation/devicetree/bindings/video/display-timing.txt +[2]: Documentation/devicetree/bindings/media/video-interfaces.txt + +Example: + + panel@0 { + compatible = "samsung,s6e3fa0"; + reg = <0>; + vdd3-supply = <&vcclcd_reg>; + vci-supply = <&vlcd_reg>; + reset-gpio = <&gpy7 4 0>; + det-gpio = <&gpg0 6 0>; + te-gpio = <&gpd1 7 0>; + + display-timings { + timing0: timing-0 { + clock-frequency = <0>; + hactive = <1080>; + vactive = <1920>; + hfront-porch = <2>; + hback-porch = <2>; + hsync-len = <1>; + vfront-porch = <1>; + vback-porch = <4>; + vsync-len = <1>; + }; + }; + + cpu-timings { + cs-setup = <0>; + wr-setup = <0>; + wr-act = <1>; + wr-hold = <0>; + }; + };