* [PATCH V3] ARM: dts: da850-evm: Enable LCD and Backlight
@ 2018-05-13 23:20 ` Adam Ford
0 siblings, 0 replies; 14+ messages in thread
From: Adam Ford @ 2018-05-13 23:20 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: Adam Ford, devicetree, robh+dt, nsekhar, khilman
When using the board files the LCD works, but not with the DT.
This adds enables the original da850-evm to work with the same
LCD in device tree mode.
The EVM has a gpio for the regulator and a gpio enable. The LCD and
the vpif display pins are mutually exclusive, so if using the LCD,
do not load the vpif driver.
Signed-off-by: Adam Ford <aford173@gmail.com>
---
V3: Fix errant GPIO, label GPIO pins, and rename the regulator to be more explict to
backlight which better matches the schematic. Updated the description to explain
that it cannot be used at the same time as the vpif driver.
V2: Add regulator and GPIO enable pins. Remove PWM backlight and replace with GPIO
diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
index 2e817da37fdb..3f1c8be07efe 100644
--- a/arch/arm/boot/dts/da850-evm.dts
+++ b/arch/arm/boot/dts/da850-evm.dts
@@ -27,6 +27,50 @@
spi0 = &spi1;
};
+ backlight {
+ compatible = "gpio-backlight";
+ enable-gpios = <&gpio 7 GPIO_ACTIVE_HIGH>; /* GP0[7] */
+ };
+
+ panel {
+ compatible = "ti,tilcdc,panel";
+ pinctrl-names = "default";
+ pinctrl-0 = <&lcd_pins>;
+ status = "okay";
+ enable-gpios = <&gpio 40 GPIO_ACTIVE_HIGH>; /* GP2[8] */
+
+ panel-info {
+ ac-bias = <255>;
+ ac-bias-intrpt = <0>;
+ dma-burst-sz = <16>;
+ bpp = <16>;
+ fdd = <0x80>;
+ sync-edge = <0>;
+ sync-ctrl = <1>;
+ raster-order = <0>;
+ fifo-th = <0>;
+ };
+
+ display-timings {
+ native-mode = <&timing0>;
+ timing0: 480x272 {
+ clock-frequency = <9000000>;
+ hactive = <480>;
+ vactive = <272>;
+ hfront-porch = <3>;
+ hback-porch = <2>;
+ hsync-len = <42>;
+ vback-porch = <3>;
+ vfront-porch = <4>;
+ vsync-len = <11>;
+ hsync-active = <0>;
+ vsync-active = <0>;
+ de-active = <1>;
+ pixelclk-active = <1>;
+ };
+ };
+ };
+
vbat: fixedregulator0 {
compatible = "regulator-fixed";
regulator-name = "vbat";
@@ -35,6 +79,16 @@
regulator-boot-on;
};
+ backlight_reg: backlight-regulator {
+ compatible = "regulator-fixed";
+ regulator-name = "lcd_backlight_pwr";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ gpio = <&gpio 47 GPIO_ACTIVE_HIGH>; /* GP2[15] */
+ regulator-always-on;
+ enable-active-high;
+ };
+
sound {
compatible = "simple-audio-card";
simple-audio-card,name = "DA850/OMAP-L138 EVM";
@@ -109,6 +163,10 @@
status = "okay";
};
+&lcdc {
+ status = "okay";
+};
+
&i2c0 {
status = "okay";
clock-frequency = <100000>;
--
2.17.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH V3] ARM: dts: da850-evm: Enable LCD and Backlight @ 2018-05-13 23:20 ` Adam Ford 0 siblings, 0 replies; 14+ messages in thread From: Adam Ford @ 2018-05-13 23:20 UTC (permalink / raw) To: linux-arm-kernel When using the board files the LCD works, but not with the DT. This adds enables the original da850-evm to work with the same LCD in device tree mode. The EVM has a gpio for the regulator and a gpio enable. The LCD and the vpif display pins are mutually exclusive, so if using the LCD, do not load the vpif driver. Signed-off-by: Adam Ford <aford173@gmail.com> --- V3: Fix errant GPIO, label GPIO pins, and rename the regulator to be more explict to backlight which better matches the schematic. Updated the description to explain that it cannot be used at the same time as the vpif driver. V2: Add regulator and GPIO enable pins. Remove PWM backlight and replace with GPIO diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts index 2e817da37fdb..3f1c8be07efe 100644 --- a/arch/arm/boot/dts/da850-evm.dts +++ b/arch/arm/boot/dts/da850-evm.dts @@ -27,6 +27,50 @@ spi0 = &spi1; }; + backlight { + compatible = "gpio-backlight"; + enable-gpios = <&gpio 7 GPIO_ACTIVE_HIGH>; /* GP0[7] */ + }; + + panel { + compatible = "ti,tilcdc,panel"; + pinctrl-names = "default"; + pinctrl-0 = <&lcd_pins>; + status = "okay"; + enable-gpios = <&gpio 40 GPIO_ACTIVE_HIGH>; /* GP2[8] */ + + panel-info { + ac-bias = <255>; + ac-bias-intrpt = <0>; + dma-burst-sz = <16>; + bpp = <16>; + fdd = <0x80>; + sync-edge = <0>; + sync-ctrl = <1>; + raster-order = <0>; + fifo-th = <0>; + }; + + display-timings { + native-mode = <&timing0>; + timing0: 480x272 { + clock-frequency = <9000000>; + hactive = <480>; + vactive = <272>; + hfront-porch = <3>; + hback-porch = <2>; + hsync-len = <42>; + vback-porch = <3>; + vfront-porch = <4>; + vsync-len = <11>; + hsync-active = <0>; + vsync-active = <0>; + de-active = <1>; + pixelclk-active = <1>; + }; + }; + }; + vbat: fixedregulator0 { compatible = "regulator-fixed"; regulator-name = "vbat"; @@ -35,6 +79,16 @@ regulator-boot-on; }; + backlight_reg: backlight-regulator { + compatible = "regulator-fixed"; + regulator-name = "lcd_backlight_pwr"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + gpio = <&gpio 47 GPIO_ACTIVE_HIGH>; /* GP2[15] */ + regulator-always-on; + enable-active-high; + }; + sound { compatible = "simple-audio-card"; simple-audio-card,name = "DA850/OMAP-L138 EVM"; @@ -109,6 +163,10 @@ status = "okay"; }; +&lcdc { + status = "okay"; +}; + &i2c0 { status = "okay"; clock-frequency = <100000>; -- 2.17.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH V3] ARM: dts: da850-evm: Enable LCD and Backlight 2018-05-13 23:20 ` Adam Ford @ 2018-05-14 5:29 ` Sekhar Nori -1 siblings, 0 replies; 14+ messages in thread From: Sekhar Nori @ 2018-05-14 5:29 UTC (permalink / raw) To: Adam Ford, linux-arm-kernel; +Cc: devicetree, robh+dt, khilman Hi Adam, On Monday 14 May 2018 04:50 AM, Adam Ford wrote: > When using the board files the LCD works, but not with the DT. > This adds enables the original da850-evm to work with the same > LCD in device tree mode. > > The EVM has a gpio for the regulator and a gpio enable. The LCD and > the vpif display pins are mutually exclusive, so if using the LCD, > do not load the vpif driver. Its not sufficient just note this in patch description. a) Disable (status = "disabled") the VPIF node which clashes for pins with LCD. b) Add a comment on top of the status = "disabled" giving information on how user can enable it (disable lcdc node and then change to status = "okay"). > > Signed-off-by: Adam Ford <aford173@gmail.com> > --- > V3: Fix errant GPIO, label GPIO pins, and rename the regulator to be more explict to > backlight which better matches the schematic. Updated the description to explain > that it cannot be used at the same time as the vpif driver. > > V2: Add regulator and GPIO enable pins. Remove PWM backlight and replace with GPIO > > diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts > index 2e817da37fdb..3f1c8be07efe 100644 > --- a/arch/arm/boot/dts/da850-evm.dts > +++ b/arch/arm/boot/dts/da850-evm.dts > @@ -27,6 +27,50 @@ > spi0 = &spi1; > }; > > + backlight { > + compatible = "gpio-backlight"; > + enable-gpios = <&gpio 7 GPIO_ACTIVE_HIGH>; /* GP0[7] */ The gpio-backlight binding does not describe a property called enable-gpios. It should just be gpios. a) Are you using gpio-backlight because you are not able to get the PWM to work? b) What is GP0[7] connected to in the schematic you have? In the schematic I have I see LCD_PWM0 is connected to SPI1_SCS[0]/EPWM1B/GP2[14]/TM64P3_IN12. c) The /* GP0[7] */ comment is not really useful on its own as it can be computed. What I wanted to see is the schematic symbol like "LCD_PWM0". Same for other places like this below. > @@ -35,6 +79,16 @@ > regulator-boot-on; > }; > > + backlight_reg: backlight-regulator { > + compatible = "regulator-fixed"; > + regulator-name = "lcd_backlight_pwr"; > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + gpio = <&gpio 47 GPIO_ACTIVE_HIGH>; /* GP2[15] */ > + regulator-always-on; Why should this regulator never be disabled? Thanks, Sekhar ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH V3] ARM: dts: da850-evm: Enable LCD and Backlight @ 2018-05-14 5:29 ` Sekhar Nori 0 siblings, 0 replies; 14+ messages in thread From: Sekhar Nori @ 2018-05-14 5:29 UTC (permalink / raw) To: linux-arm-kernel Hi Adam, On Monday 14 May 2018 04:50 AM, Adam Ford wrote: > When using the board files the LCD works, but not with the DT. > This adds enables the original da850-evm to work with the same > LCD in device tree mode. > > The EVM has a gpio for the regulator and a gpio enable. The LCD and > the vpif display pins are mutually exclusive, so if using the LCD, > do not load the vpif driver. Its not sufficient just note this in patch description. a) Disable (status = "disabled") the VPIF node which clashes for pins with LCD. b) Add a comment on top of the status = "disabled" giving information on how user can enable it (disable lcdc node and then change to status = "okay"). > > Signed-off-by: Adam Ford <aford173@gmail.com> > --- > V3: Fix errant GPIO, label GPIO pins, and rename the regulator to be more explict to > backlight which better matches the schematic. Updated the description to explain > that it cannot be used at the same time as the vpif driver. > > V2: Add regulator and GPIO enable pins. Remove PWM backlight and replace with GPIO > > diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts > index 2e817da37fdb..3f1c8be07efe 100644 > --- a/arch/arm/boot/dts/da850-evm.dts > +++ b/arch/arm/boot/dts/da850-evm.dts > @@ -27,6 +27,50 @@ > spi0 = &spi1; > }; > > + backlight { > + compatible = "gpio-backlight"; > + enable-gpios = <&gpio 7 GPIO_ACTIVE_HIGH>; /* GP0[7] */ The gpio-backlight binding does not describe a property called enable-gpios. It should just be gpios. a) Are you using gpio-backlight because you are not able to get the PWM to work? b) What is GP0[7] connected to in the schematic you have? In the schematic I have I see LCD_PWM0 is connected to SPI1_SCS[0]/EPWM1B/GP2[14]/TM64P3_IN12. c) The /* GP0[7] */ comment is not really useful on its own as it can be computed. What I wanted to see is the schematic symbol like "LCD_PWM0". Same for other places like this below. > @@ -35,6 +79,16 @@ > regulator-boot-on; > }; > > + backlight_reg: backlight-regulator { > + compatible = "regulator-fixed"; > + regulator-name = "lcd_backlight_pwr"; > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + gpio = <&gpio 47 GPIO_ACTIVE_HIGH>; /* GP2[15] */ > + regulator-always-on; Why should this regulator never be disabled? Thanks, Sekhar ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V3] ARM: dts: da850-evm: Enable LCD and Backlight 2018-05-14 5:29 ` Sekhar Nori @ 2018-05-14 10:52 ` Adam Ford -1 siblings, 0 replies; 14+ messages in thread From: Adam Ford @ 2018-05-14 10:52 UTC (permalink / raw) To: Sekhar Nori; +Cc: devicetree, Rob Herring, arm-soc, Kevin Hilman On Mon, May 14, 2018 at 12:29 AM, Sekhar Nori <nsekhar@ti.com> wrote: > Hi Adam, > > On Monday 14 May 2018 04:50 AM, Adam Ford wrote: >> When using the board files the LCD works, but not with the DT. >> This adds enables the original da850-evm to work with the same >> LCD in device tree mode. >> >> The EVM has a gpio for the regulator and a gpio enable. The LCD and >> the vpif display pins are mutually exclusive, so if using the LCD, >> do not load the vpif driver. > > Its not sufficient just note this in patch description. > > a) Disable (status = "disabled") the VPIF node which clashes for pins > with LCD. > b) Add a comment on top of the status = "disabled" giving information on > how user can enable it (disable lcdc node and then change to status = > "okay"). > >> >> Signed-off-by: Adam Ford <aford173@gmail.com> >> --- >> V3: Fix errant GPIO, label GPIO pins, and rename the regulator to be more explict to >> backlight which better matches the schematic. Updated the description to explain >> that it cannot be used at the same time as the vpif driver. >> >> V2: Add regulator and GPIO enable pins. Remove PWM backlight and replace with GPIO >> >> diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts >> index 2e817da37fdb..3f1c8be07efe 100644 >> --- a/arch/arm/boot/dts/da850-evm.dts >> +++ b/arch/arm/boot/dts/da850-evm.dts >> @@ -27,6 +27,50 @@ >> spi0 = &spi1; >> }; >> >> + backlight { >> + compatible = "gpio-backlight"; >> + enable-gpios = <&gpio 7 GPIO_ACTIVE_HIGH>; /* GP0[7] */ > > The gpio-backlight binding does not describe a property called > enable-gpios. It should just be gpios. I will fix that. > > a) Are you using gpio-backlight because you are not able to get the PWM > to work? > Yes, You told me not to worry about doing a PWM backlight because the legacy board does not PWM either. > b) What is GP0[7] connected to in the schematic you have? In the > schematic I have I see LCD_PWM0 is connected to > SPI1_SCS[0]/EPWM1B/GP2[14]/TM64P3_IN12. I have schematic 1016572 dated Wednesday, August 18, 2010. According to it, AXR15 / EPWMN0_TZ[0] / ECAP2_APWM2 / GPIO0[7] connects to U25, Pin 46 to generate M_LCD_PWM0. You might have one of the early, pre-release versions. > > c) The /* GP0[7] */ comment is not really useful on its own as it can be > computed. What I wanted to see is the schematic symbol like "LCD_PWM0". > Same for other places like this below. I can do that. > >> @@ -35,6 +79,16 @@ >> regulator-boot-on; >> }; >> >> + backlight_reg: backlight-regulator { >> + compatible = "regulator-fixed"; >> + regulator-name = "lcd_backlight_pwr"; >> + regulator-min-microvolt = <3300000>; >> + regulator-max-microvolt = <3300000>; >> + gpio = <&gpio 47 GPIO_ACTIVE_HIGH>; /* GP2[15] */ >> + regulator-always-on; > > Why should this regulator never be disabled? The gpio-backlight does not have a way that I can see to associate the regulator to it. I read through the bindings, but I didn't see an option to associate a regulator it. I use this regulator to drive lcd_backlight_pwr and the backlight driver to write lcd_pwm0. Without this option, the system disables lcd_backlight_pwr and the screen is blank adam > > Thanks, > Sekhar ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH V3] ARM: dts: da850-evm: Enable LCD and Backlight @ 2018-05-14 10:52 ` Adam Ford 0 siblings, 0 replies; 14+ messages in thread From: Adam Ford @ 2018-05-14 10:52 UTC (permalink / raw) To: linux-arm-kernel On Mon, May 14, 2018 at 12:29 AM, Sekhar Nori <nsekhar@ti.com> wrote: > Hi Adam, > > On Monday 14 May 2018 04:50 AM, Adam Ford wrote: >> When using the board files the LCD works, but not with the DT. >> This adds enables the original da850-evm to work with the same >> LCD in device tree mode. >> >> The EVM has a gpio for the regulator and a gpio enable. The LCD and >> the vpif display pins are mutually exclusive, so if using the LCD, >> do not load the vpif driver. > > Its not sufficient just note this in patch description. > > a) Disable (status = "disabled") the VPIF node which clashes for pins > with LCD. > b) Add a comment on top of the status = "disabled" giving information on > how user can enable it (disable lcdc node and then change to status = > "okay"). > >> >> Signed-off-by: Adam Ford <aford173@gmail.com> >> --- >> V3: Fix errant GPIO, label GPIO pins, and rename the regulator to be more explict to >> backlight which better matches the schematic. Updated the description to explain >> that it cannot be used at the same time as the vpif driver. >> >> V2: Add regulator and GPIO enable pins. Remove PWM backlight and replace with GPIO >> >> diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts >> index 2e817da37fdb..3f1c8be07efe 100644 >> --- a/arch/arm/boot/dts/da850-evm.dts >> +++ b/arch/arm/boot/dts/da850-evm.dts >> @@ -27,6 +27,50 @@ >> spi0 = &spi1; >> }; >> >> + backlight { >> + compatible = "gpio-backlight"; >> + enable-gpios = <&gpio 7 GPIO_ACTIVE_HIGH>; /* GP0[7] */ > > The gpio-backlight binding does not describe a property called > enable-gpios. It should just be gpios. I will fix that. > > a) Are you using gpio-backlight because you are not able to get the PWM > to work? > Yes, You told me not to worry about doing a PWM backlight because the legacy board does not PWM either. > b) What is GP0[7] connected to in the schematic you have? In the > schematic I have I see LCD_PWM0 is connected to > SPI1_SCS[0]/EPWM1B/GP2[14]/TM64P3_IN12. I have schematic 1016572 dated Wednesday, August 18, 2010. According to it, AXR15 / EPWMN0_TZ[0] / ECAP2_APWM2 / GPIO0[7] connects to U25, Pin 46 to generate M_LCD_PWM0. You might have one of the early, pre-release versions. > > c) The /* GP0[7] */ comment is not really useful on its own as it can be > computed. What I wanted to see is the schematic symbol like "LCD_PWM0". > Same for other places like this below. I can do that. > >> @@ -35,6 +79,16 @@ >> regulator-boot-on; >> }; >> >> + backlight_reg: backlight-regulator { >> + compatible = "regulator-fixed"; >> + regulator-name = "lcd_backlight_pwr"; >> + regulator-min-microvolt = <3300000>; >> + regulator-max-microvolt = <3300000>; >> + gpio = <&gpio 47 GPIO_ACTIVE_HIGH>; /* GP2[15] */ >> + regulator-always-on; > > Why should this regulator never be disabled? The gpio-backlight does not have a way that I can see to associate the regulator to it. I read through the bindings, but I didn't see an option to associate a regulator it. I use this regulator to drive lcd_backlight_pwr and the backlight driver to write lcd_pwm0. Without this option, the system disables lcd_backlight_pwr and the screen is blank adam > > Thanks, > Sekhar ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V3] ARM: dts: da850-evm: Enable LCD and Backlight 2018-05-14 10:52 ` Adam Ford @ 2018-05-14 12:35 ` Sekhar Nori -1 siblings, 0 replies; 14+ messages in thread From: Sekhar Nori @ 2018-05-14 12:35 UTC (permalink / raw) To: Adam Ford; +Cc: devicetree, Rob Herring, arm-soc, Kevin Hilman On Monday 14 May 2018 04:22 PM, Adam Ford wrote: > On Mon, May 14, 2018 at 12:29 AM, Sekhar Nori <nsekhar@ti.com> wrote: >> Hi Adam, >> >> On Monday 14 May 2018 04:50 AM, Adam Ford wrote: >>> When using the board files the LCD works, but not with the DT. >>> This adds enables the original da850-evm to work with the same >>> LCD in device tree mode. >>> >>> The EVM has a gpio for the regulator and a gpio enable. The LCD and >>> the vpif display pins are mutually exclusive, so if using the LCD, >>> do not load the vpif driver. >> >> Its not sufficient just note this in patch description. >> >> a) Disable (status = "disabled") the VPIF node which clashes for pins >> with LCD. >> b) Add a comment on top of the status = "disabled" giving information on >> how user can enable it (disable lcdc node and then change to status = >> "okay"). >> >>> >>> Signed-off-by: Adam Ford <aford173@gmail.com> >>> --- >>> V3: Fix errant GPIO, label GPIO pins, and rename the regulator to be more explict to >>> backlight which better matches the schematic. Updated the description to explain >>> that it cannot be used at the same time as the vpif driver. >>> >>> V2: Add regulator and GPIO enable pins. Remove PWM backlight and replace with GPIO >>> >>> diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts >>> index 2e817da37fdb..3f1c8be07efe 100644 >>> --- a/arch/arm/boot/dts/da850-evm.dts >>> +++ b/arch/arm/boot/dts/da850-evm.dts >>> @@ -27,6 +27,50 @@ >>> spi0 = &spi1; >>> }; >>> >>> + backlight { >>> + compatible = "gpio-backlight"; >>> + enable-gpios = <&gpio 7 GPIO_ACTIVE_HIGH>; /* GP0[7] */ >> >> The gpio-backlight binding does not describe a property called >> enable-gpios. It should just be gpios. > > I will fix that. > >> >> a) Are you using gpio-backlight because you are not able to get the PWM >> to work? >> > Yes, You told me not to worry about doing a PWM backlight because the > legacy board does not PWM either. Yeah, I meant not to add backlight control till the time we are able to get it working using PWM. Is this needed for the basic LCD functionality to work? I would like to avoid the churn of adding it using GPIO now and changing to PWM later, if possible. > >> b) What is GP0[7] connected to in the schematic you have? In the >> schematic I have I see LCD_PWM0 is connected to >> SPI1_SCS[0]/EPWM1B/GP2[14]/TM64P3_IN12. > > I have schematic 1016572 dated Wednesday, August 18, 2010. According > to it, AXR15 / EPWMN0_TZ[0] / ECAP2_APWM2 / GPIO0[7] connects to U25, > Pin 46 to generate M_LCD_PWM0. You might have one of the early, > pre-release versions. Ah, okay. In your schematic, is GP2[14] connected to anything? > >> >> c) The /* GP0[7] */ comment is not really useful on its own as it can be >> computed. What I wanted to see is the schematic symbol like "LCD_PWM0". >> Same for other places like this below. > > I can do that. >> >>> @@ -35,6 +79,16 @@ >>> regulator-boot-on; >>> }; >>> >>> + backlight_reg: backlight-regulator { >>> + compatible = "regulator-fixed"; >>> + regulator-name = "lcd_backlight_pwr"; >>> + regulator-min-microvolt = <3300000>; >>> + regulator-max-microvolt = <3300000>; >>> + gpio = <&gpio 47 GPIO_ACTIVE_HIGH>; /* GP2[15] */ >>> + regulator-always-on; >> >> Why should this regulator never be disabled? > > The gpio-backlight does not have a way that I can see to associate the > regulator to it. I read through the bindings, but I didn't see an > option to associate a regulator it. I use this regulator to drive > lcd_backlight_pwr and the backlight driver to write lcd_pwm0. Without > this option, the system disables lcd_backlight_pwr and the screen is > blank It sounds like this is a hack to enable backlight on this board. I think either the backlight driver needs to gain functionality to enable the GPIO. Or backlight could be treated as part of the panel and enabled using enable-gpios property in the panel. TBH, I will be okay either way. Can you check with Jyri, Tomi and rest of the DRM folks on what should be right way of dealing with this? Thanks, Sekhar ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH V3] ARM: dts: da850-evm: Enable LCD and Backlight @ 2018-05-14 12:35 ` Sekhar Nori 0 siblings, 0 replies; 14+ messages in thread From: Sekhar Nori @ 2018-05-14 12:35 UTC (permalink / raw) To: linux-arm-kernel On Monday 14 May 2018 04:22 PM, Adam Ford wrote: > On Mon, May 14, 2018 at 12:29 AM, Sekhar Nori <nsekhar@ti.com> wrote: >> Hi Adam, >> >> On Monday 14 May 2018 04:50 AM, Adam Ford wrote: >>> When using the board files the LCD works, but not with the DT. >>> This adds enables the original da850-evm to work with the same >>> LCD in device tree mode. >>> >>> The EVM has a gpio for the regulator and a gpio enable. The LCD and >>> the vpif display pins are mutually exclusive, so if using the LCD, >>> do not load the vpif driver. >> >> Its not sufficient just note this in patch description. >> >> a) Disable (status = "disabled") the VPIF node which clashes for pins >> with LCD. >> b) Add a comment on top of the status = "disabled" giving information on >> how user can enable it (disable lcdc node and then change to status = >> "okay"). >> >>> >>> Signed-off-by: Adam Ford <aford173@gmail.com> >>> --- >>> V3: Fix errant GPIO, label GPIO pins, and rename the regulator to be more explict to >>> backlight which better matches the schematic. Updated the description to explain >>> that it cannot be used at the same time as the vpif driver. >>> >>> V2: Add regulator and GPIO enable pins. Remove PWM backlight and replace with GPIO >>> >>> diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts >>> index 2e817da37fdb..3f1c8be07efe 100644 >>> --- a/arch/arm/boot/dts/da850-evm.dts >>> +++ b/arch/arm/boot/dts/da850-evm.dts >>> @@ -27,6 +27,50 @@ >>> spi0 = &spi1; >>> }; >>> >>> + backlight { >>> + compatible = "gpio-backlight"; >>> + enable-gpios = <&gpio 7 GPIO_ACTIVE_HIGH>; /* GP0[7] */ >> >> The gpio-backlight binding does not describe a property called >> enable-gpios. It should just be gpios. > > I will fix that. > >> >> a) Are you using gpio-backlight because you are not able to get the PWM >> to work? >> > Yes, You told me not to worry about doing a PWM backlight because the > legacy board does not PWM either. Yeah, I meant not to add backlight control till the time we are able to get it working using PWM. Is this needed for the basic LCD functionality to work? I would like to avoid the churn of adding it using GPIO now and changing to PWM later, if possible. > >> b) What is GP0[7] connected to in the schematic you have? In the >> schematic I have I see LCD_PWM0 is connected to >> SPI1_SCS[0]/EPWM1B/GP2[14]/TM64P3_IN12. > > I have schematic 1016572 dated Wednesday, August 18, 2010. According > to it, AXR15 / EPWMN0_TZ[0] / ECAP2_APWM2 / GPIO0[7] connects to U25, > Pin 46 to generate M_LCD_PWM0. You might have one of the early, > pre-release versions. Ah, okay. In your schematic, is GP2[14] connected to anything? > >> >> c) The /* GP0[7] */ comment is not really useful on its own as it can be >> computed. What I wanted to see is the schematic symbol like "LCD_PWM0". >> Same for other places like this below. > > I can do that. >> >>> @@ -35,6 +79,16 @@ >>> regulator-boot-on; >>> }; >>> >>> + backlight_reg: backlight-regulator { >>> + compatible = "regulator-fixed"; >>> + regulator-name = "lcd_backlight_pwr"; >>> + regulator-min-microvolt = <3300000>; >>> + regulator-max-microvolt = <3300000>; >>> + gpio = <&gpio 47 GPIO_ACTIVE_HIGH>; /* GP2[15] */ >>> + regulator-always-on; >> >> Why should this regulator never be disabled? > > The gpio-backlight does not have a way that I can see to associate the > regulator to it. I read through the bindings, but I didn't see an > option to associate a regulator it. I use this regulator to drive > lcd_backlight_pwr and the backlight driver to write lcd_pwm0. Without > this option, the system disables lcd_backlight_pwr and the screen is > blank It sounds like this is a hack to enable backlight on this board. I think either the backlight driver needs to gain functionality to enable the GPIO. Or backlight could be treated as part of the panel and enabled using enable-gpios property in the panel. TBH, I will be okay either way. Can you check with Jyri, Tomi and rest of the DRM folks on what should be right way of dealing with this? Thanks, Sekhar ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V3] ARM: dts: da850-evm: Enable LCD and Backlight 2018-05-14 12:35 ` Sekhar Nori @ 2018-05-14 18:04 ` Adam Ford -1 siblings, 0 replies; 14+ messages in thread From: Adam Ford @ 2018-05-14 18:04 UTC (permalink / raw) To: Sekhar Nori Cc: devicetree, Kevin Hilman, jsarha, Rob Herring, Tomi Valkeinen, Laurent Pinchart, arm-soc On Mon, May 14, 2018 at 7:35 AM, Sekhar Nori <nsekhar@ti.com> wrote: > On Monday 14 May 2018 04:22 PM, Adam Ford wrote: >> On Mon, May 14, 2018 at 12:29 AM, Sekhar Nori <nsekhar@ti.com> wrote: >>> Hi Adam, Added Tomi, Laurent, and Jyri for feedback. >>> >>> On Monday 14 May 2018 04:50 AM, Adam Ford wrote: >>>> When using the board files the LCD works, but not with the DT. >>>> This adds enables the original da850-evm to work with the same >>>> LCD in device tree mode. >>>> >>>> The EVM has a gpio for the regulator and a gpio enable. The LCD and >>>> the vpif display pins are mutually exclusive, so if using the LCD, >>>> do not load the vpif driver. >>> >>> Its not sufficient just note this in patch description. >>> >>> a) Disable (status = "disabled") the VPIF node which clashes for pins >>> with LCD. >>> b) Add a comment on top of the status = "disabled" giving information on >>> how user can enable it (disable lcdc node and then change to status = >>> "okay"). >>> >>>> >>>> Signed-off-by: Adam Ford <aford173@gmail.com> >>>> --- >>>> V3: Fix errant GPIO, label GPIO pins, and rename the regulator to be more explict to >>>> backlight which better matches the schematic. Updated the description to explain >>>> that it cannot be used at the same time as the vpif driver. >>>> >>>> V2: Add regulator and GPIO enable pins. Remove PWM backlight and replace with GPIO >>>> >>>> diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts >>>> index 2e817da37fdb..3f1c8be07efe 100644 >>>> --- a/arch/arm/boot/dts/da850-evm.dts >>>> +++ b/arch/arm/boot/dts/da850-evm.dts >>>> @@ -27,6 +27,50 @@ >>>> spi0 = &spi1; >>>> }; >>>> >>>> + backlight { >>>> + compatible = "gpio-backlight"; >>>> + enable-gpios = <&gpio 7 GPIO_ACTIVE_HIGH>; /* GP0[7] */ >>> >>> The gpio-backlight binding does not describe a property called >>> enable-gpios. It should just be gpios. >> >> I will fix that. >> >>> >>> a) Are you using gpio-backlight because you are not able to get the PWM >>> to work? >>> >> Yes, You told me not to worry about doing a PWM backlight because the >> legacy board does not PWM either. > > Yeah, I meant not to add backlight control till the time we are able to > get it working using PWM. Is this needed for the basic LCD functionality > to work? I would like to avoid the churn of adding it using GPIO now and > changing to PWM later, if possible. > >> >>> b) What is GP0[7] connected to in the schematic you have? In the >>> schematic I have I see LCD_PWM0 is connected to >>> SPI1_SCS[0]/EPWM1B/GP2[14]/TM64P3_IN12. >> >> I have schematic 1016572 dated Wednesday, August 18, 2010. According >> to it, AXR15 / EPWMN0_TZ[0] / ECAP2_APWM2 / GPIO0[7] connects to U25, >> Pin 46 to generate M_LCD_PWM0. You might have one of the early, >> pre-release versions. > > Ah, okay. In your schematic, is GP2[14] connected to anything? > >> >>> >>> c) The /* GP0[7] */ comment is not really useful on its own as it can be >>> computed. What I wanted to see is the schematic symbol like "LCD_PWM0". >>> Same for other places like this below. >> >> I can do that. >>> >>>> @@ -35,6 +79,16 @@ >>>> regulator-boot-on; >>>> }; >>>> >>>> + backlight_reg: backlight-regulator { >>>> + compatible = "regulator-fixed"; >>>> + regulator-name = "lcd_backlight_pwr"; >>>> + regulator-min-microvolt = <3300000>; >>>> + regulator-max-microvolt = <3300000>; >>>> + gpio = <&gpio 47 GPIO_ACTIVE_HIGH>; /* GP2[15] */ >>>> + regulator-always-on; >>> >>> Why should this regulator never be disabled? >> >> The gpio-backlight does not have a way that I can see to associate the >> regulator to it. I read through the bindings, but I didn't see an >> option to associate a regulator it. I use this regulator to drive >> lcd_backlight_pwr and the backlight driver to write lcd_pwm0. Without >> this option, the system disables lcd_backlight_pwr and the screen is >> blank > > It sounds like this is a hack to enable backlight on this board. I think > either the backlight driver needs to gain functionality to enable the > GPIO. Or backlight could be treated as part of the panel and enabled > using enable-gpios property in the panel. TBH, I will be okay either > way. Can you check with Jyri, Tomi and rest of the DRM folks on what > should be right way of dealing with this? Per your request I added them into this thread. I added Tomi, Jyri, and Laurent to this as Laurent's name is associated with the gpio backlight driver. I am not sure why you think it's a hack. I pulled up the schematic for the LCD to see what it's doing, and the lcd_backlight_pwr pin controls the power-on sequence of the back-light controller. Without this, there is no power, so it seems to me that the 'regulator-fixed' device is the correct way to do it. The separate pin associated to the gpio is used to tell the backlight IC to actually turn on/off the back-light. Ideally it seems like it would nice to have the gpio-backlight driver be able to specify the regulator, so when the backlight is in use, it would power the regulator, but until that's available, the it seems like 'regulator-always-on' is the way to make it stay on. Laurent, Jyri, Tomi, do you have any thoughts on this matter? Thanks, adam > > Thanks, > Sekhar ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH V3] ARM: dts: da850-evm: Enable LCD and Backlight @ 2018-05-14 18:04 ` Adam Ford 0 siblings, 0 replies; 14+ messages in thread From: Adam Ford @ 2018-05-14 18:04 UTC (permalink / raw) To: linux-arm-kernel On Mon, May 14, 2018 at 7:35 AM, Sekhar Nori <nsekhar@ti.com> wrote: > On Monday 14 May 2018 04:22 PM, Adam Ford wrote: >> On Mon, May 14, 2018 at 12:29 AM, Sekhar Nori <nsekhar@ti.com> wrote: >>> Hi Adam, Added Tomi, Laurent, and Jyri for feedback. >>> >>> On Monday 14 May 2018 04:50 AM, Adam Ford wrote: >>>> When using the board files the LCD works, but not with the DT. >>>> This adds enables the original da850-evm to work with the same >>>> LCD in device tree mode. >>>> >>>> The EVM has a gpio for the regulator and a gpio enable. The LCD and >>>> the vpif display pins are mutually exclusive, so if using the LCD, >>>> do not load the vpif driver. >>> >>> Its not sufficient just note this in patch description. >>> >>> a) Disable (status = "disabled") the VPIF node which clashes for pins >>> with LCD. >>> b) Add a comment on top of the status = "disabled" giving information on >>> how user can enable it (disable lcdc node and then change to status = >>> "okay"). >>> >>>> >>>> Signed-off-by: Adam Ford <aford173@gmail.com> >>>> --- >>>> V3: Fix errant GPIO, label GPIO pins, and rename the regulator to be more explict to >>>> backlight which better matches the schematic. Updated the description to explain >>>> that it cannot be used at the same time as the vpif driver. >>>> >>>> V2: Add regulator and GPIO enable pins. Remove PWM backlight and replace with GPIO >>>> >>>> diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts >>>> index 2e817da37fdb..3f1c8be07efe 100644 >>>> --- a/arch/arm/boot/dts/da850-evm.dts >>>> +++ b/arch/arm/boot/dts/da850-evm.dts >>>> @@ -27,6 +27,50 @@ >>>> spi0 = &spi1; >>>> }; >>>> >>>> + backlight { >>>> + compatible = "gpio-backlight"; >>>> + enable-gpios = <&gpio 7 GPIO_ACTIVE_HIGH>; /* GP0[7] */ >>> >>> The gpio-backlight binding does not describe a property called >>> enable-gpios. It should just be gpios. >> >> I will fix that. >> >>> >>> a) Are you using gpio-backlight because you are not able to get the PWM >>> to work? >>> >> Yes, You told me not to worry about doing a PWM backlight because the >> legacy board does not PWM either. > > Yeah, I meant not to add backlight control till the time we are able to > get it working using PWM. Is this needed for the basic LCD functionality > to work? I would like to avoid the churn of adding it using GPIO now and > changing to PWM later, if possible. > >> >>> b) What is GP0[7] connected to in the schematic you have? In the >>> schematic I have I see LCD_PWM0 is connected to >>> SPI1_SCS[0]/EPWM1B/GP2[14]/TM64P3_IN12. >> >> I have schematic 1016572 dated Wednesday, August 18, 2010. According >> to it, AXR15 / EPWMN0_TZ[0] / ECAP2_APWM2 / GPIO0[7] connects to U25, >> Pin 46 to generate M_LCD_PWM0. You might have one of the early, >> pre-release versions. > > Ah, okay. In your schematic, is GP2[14] connected to anything? > >> >>> >>> c) The /* GP0[7] */ comment is not really useful on its own as it can be >>> computed. What I wanted to see is the schematic symbol like "LCD_PWM0". >>> Same for other places like this below. >> >> I can do that. >>> >>>> @@ -35,6 +79,16 @@ >>>> regulator-boot-on; >>>> }; >>>> >>>> + backlight_reg: backlight-regulator { >>>> + compatible = "regulator-fixed"; >>>> + regulator-name = "lcd_backlight_pwr"; >>>> + regulator-min-microvolt = <3300000>; >>>> + regulator-max-microvolt = <3300000>; >>>> + gpio = <&gpio 47 GPIO_ACTIVE_HIGH>; /* GP2[15] */ >>>> + regulator-always-on; >>> >>> Why should this regulator never be disabled? >> >> The gpio-backlight does not have a way that I can see to associate the >> regulator to it. I read through the bindings, but I didn't see an >> option to associate a regulator it. I use this regulator to drive >> lcd_backlight_pwr and the backlight driver to write lcd_pwm0. Without >> this option, the system disables lcd_backlight_pwr and the screen is >> blank > > It sounds like this is a hack to enable backlight on this board. I think > either the backlight driver needs to gain functionality to enable the > GPIO. Or backlight could be treated as part of the panel and enabled > using enable-gpios property in the panel. TBH, I will be okay either > way. Can you check with Jyri, Tomi and rest of the DRM folks on what > should be right way of dealing with this? Per your request I added them into this thread. I added Tomi, Jyri, and Laurent to this as Laurent's name is associated with the gpio backlight driver. I am not sure why you think it's a hack. I pulled up the schematic for the LCD to see what it's doing, and the lcd_backlight_pwr pin controls the power-on sequence of the back-light controller. Without this, there is no power, so it seems to me that the 'regulator-fixed' device is the correct way to do it. The separate pin associated to the gpio is used to tell the backlight IC to actually turn on/off the back-light. Ideally it seems like it would nice to have the gpio-backlight driver be able to specify the regulator, so when the backlight is in use, it would power the regulator, but until that's available, the it seems like 'regulator-always-on' is the way to make it stay on. Laurent, Jyri, Tomi, do you have any thoughts on this matter? Thanks, adam > > Thanks, > Sekhar ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V3] ARM: dts: da850-evm: Enable LCD and Backlight 2018-05-14 18:04 ` Adam Ford @ 2018-05-15 5:19 ` Sekhar Nori -1 siblings, 0 replies; 14+ messages in thread From: Sekhar Nori @ 2018-05-15 5:19 UTC (permalink / raw) To: Adam Ford Cc: devicetree, Kevin Hilman, jsarha, Rob Herring, Tomi Valkeinen, Laurent Pinchart, arm-soc On Monday 14 May 2018 11:34 PM, Adam Ford wrote: > On Mon, May 14, 2018 at 7:35 AM, Sekhar Nori <nsekhar@ti.com> wrote: >> On Monday 14 May 2018 04:22 PM, Adam Ford wrote: >>> On Mon, May 14, 2018 at 12:29 AM, Sekhar Nori <nsekhar@ti.com> wrote: >>>> Hi Adam, > > Added Tomi, Laurent, and Jyri for feedback. > >>>> >>>> On Monday 14 May 2018 04:50 AM, Adam Ford wrote: >>>>> When using the board files the LCD works, but not with the DT. >>>>> This adds enables the original da850-evm to work with the same >>>>> LCD in device tree mode. >>>>> >>>>> The EVM has a gpio for the regulator and a gpio enable. The LCD and >>>>> the vpif display pins are mutually exclusive, so if using the LCD, >>>>> do not load the vpif driver. >>>> >>>> Its not sufficient just note this in patch description. >>>> >>>> a) Disable (status = "disabled") the VPIF node which clashes for pins >>>> with LCD. >>>> b) Add a comment on top of the status = "disabled" giving information on >>>> how user can enable it (disable lcdc node and then change to status = >>>> "okay"). >>>> >>>>> >>>>> Signed-off-by: Adam Ford <aford173@gmail.com> >>>>> --- >>>>> V3: Fix errant GPIO, label GPIO pins, and rename the regulator to be more explict to >>>>> backlight which better matches the schematic. Updated the description to explain >>>>> that it cannot be used at the same time as the vpif driver. >>>>> >>>>> V2: Add regulator and GPIO enable pins. Remove PWM backlight and replace with GPIO >>>>> >>>>> diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts >>>>> index 2e817da37fdb..3f1c8be07efe 100644 >>>>> --- a/arch/arm/boot/dts/da850-evm.dts >>>>> +++ b/arch/arm/boot/dts/da850-evm.dts >>>>> @@ -27,6 +27,50 @@ >>>>> spi0 = &spi1; >>>>> }; >>>>> >>>>> + backlight { >>>>> + compatible = "gpio-backlight"; >>>>> + enable-gpios = <&gpio 7 GPIO_ACTIVE_HIGH>; /* GP0[7] */ >>>> >>>> The gpio-backlight binding does not describe a property called >>>> enable-gpios. It should just be gpios. >>> >>> I will fix that. >>> >>>> >>>> a) Are you using gpio-backlight because you are not able to get the PWM >>>> to work? >>>> >>> Yes, You told me not to worry about doing a PWM backlight because the >>> legacy board does not PWM either. >> >> Yeah, I meant not to add backlight control till the time we are able to >> get it working using PWM. Is this needed for the basic LCD functionality >> to work? I would like to avoid the churn of adding it using GPIO now and >> changing to PWM later, if possible. >> >>> >>>> b) What is GP0[7] connected to in the schematic you have? In the >>>> schematic I have I see LCD_PWM0 is connected to >>>> SPI1_SCS[0]/EPWM1B/GP2[14]/TM64P3_IN12. >>> >>> I have schematic 1016572 dated Wednesday, August 18, 2010. According >>> to it, AXR15 / EPWMN0_TZ[0] / ECAP2_APWM2 / GPIO0[7] connects to U25, >>> Pin 46 to generate M_LCD_PWM0. You might have one of the early, >>> pre-release versions. >> >> Ah, okay. In your schematic, is GP2[14] connected to anything? >> >>> >>>> >>>> c) The /* GP0[7] */ comment is not really useful on its own as it can be >>>> computed. What I wanted to see is the schematic symbol like "LCD_PWM0". >>>> Same for other places like this below. >>> >>> I can do that. >>>> >>>>> @@ -35,6 +79,16 @@ >>>>> regulator-boot-on; >>>>> }; >>>>> >>>>> + backlight_reg: backlight-regulator { >>>>> + compatible = "regulator-fixed"; >>>>> + regulator-name = "lcd_backlight_pwr"; >>>>> + regulator-min-microvolt = <3300000>; >>>>> + regulator-max-microvolt = <3300000>; >>>>> + gpio = <&gpio 47 GPIO_ACTIVE_HIGH>; /* GP2[15] */ >>>>> + regulator-always-on; >>>> >>>> Why should this regulator never be disabled? >>> >>> The gpio-backlight does not have a way that I can see to associate the >>> regulator to it. I read through the bindings, but I didn't see an >>> option to associate a regulator it. I use this regulator to drive >>> lcd_backlight_pwr and the backlight driver to write lcd_pwm0. Without >>> this option, the system disables lcd_backlight_pwr and the screen is >>> blank >> >> It sounds like this is a hack to enable backlight on this board. I think >> either the backlight driver needs to gain functionality to enable the >> GPIO. Or backlight could be treated as part of the panel and enabled >> using enable-gpios property in the panel. TBH, I will be okay either >> way. Can you check with Jyri, Tomi and rest of the DRM folks on what >> should be right way of dealing with this? > > Per your request I added them into this thread. I added Tomi, Jyri, > and Laurent to this as Laurent's name is associated with the gpio > backlight driver. Okay. The reason I did not loop them in myself is because I thought a fresh thread with background will be better. But okay. > I am not sure why you think it's a hack. I pulled up the schematic > for the LCD to see what it's doing, and the lcd_backlight_pwr pin > controls the power-on sequence of the back-light controller. Without > this, there is no power, so it seems to me that the 'regulator-fixed' > device is the correct way to do it. Not questioning modeling the GPIO as a regulator. > > The separate pin associated to the gpio is used to tell the backlight > IC to actually turn on/off the back-light. Ideally it seems like it > would nice to have the gpio-backlight driver be able to specify the > regulator, so when the backlight is in use, it would power the > regulator, but until that's available, the it seems like > 'regulator-always-on' is the way to make it stay on. We need to add support for this in backlight driver. Using regulator-always-on to paper over this lack of support in backlight driver is what I am calling a hack. 'regulator-always-on' means the regulator cannot be turned off. Thats certainly not the case as you have pointed out. Thanks, Sekhar ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH V3] ARM: dts: da850-evm: Enable LCD and Backlight @ 2018-05-15 5:19 ` Sekhar Nori 0 siblings, 0 replies; 14+ messages in thread From: Sekhar Nori @ 2018-05-15 5:19 UTC (permalink / raw) To: linux-arm-kernel On Monday 14 May 2018 11:34 PM, Adam Ford wrote: > On Mon, May 14, 2018 at 7:35 AM, Sekhar Nori <nsekhar@ti.com> wrote: >> On Monday 14 May 2018 04:22 PM, Adam Ford wrote: >>> On Mon, May 14, 2018 at 12:29 AM, Sekhar Nori <nsekhar@ti.com> wrote: >>>> Hi Adam, > > Added Tomi, Laurent, and Jyri for feedback. > >>>> >>>> On Monday 14 May 2018 04:50 AM, Adam Ford wrote: >>>>> When using the board files the LCD works, but not with the DT. >>>>> This adds enables the original da850-evm to work with the same >>>>> LCD in device tree mode. >>>>> >>>>> The EVM has a gpio for the regulator and a gpio enable. The LCD and >>>>> the vpif display pins are mutually exclusive, so if using the LCD, >>>>> do not load the vpif driver. >>>> >>>> Its not sufficient just note this in patch description. >>>> >>>> a) Disable (status = "disabled") the VPIF node which clashes for pins >>>> with LCD. >>>> b) Add a comment on top of the status = "disabled" giving information on >>>> how user can enable it (disable lcdc node and then change to status = >>>> "okay"). >>>> >>>>> >>>>> Signed-off-by: Adam Ford <aford173@gmail.com> >>>>> --- >>>>> V3: Fix errant GPIO, label GPIO pins, and rename the regulator to be more explict to >>>>> backlight which better matches the schematic. Updated the description to explain >>>>> that it cannot be used at the same time as the vpif driver. >>>>> >>>>> V2: Add regulator and GPIO enable pins. Remove PWM backlight and replace with GPIO >>>>> >>>>> diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts >>>>> index 2e817da37fdb..3f1c8be07efe 100644 >>>>> --- a/arch/arm/boot/dts/da850-evm.dts >>>>> +++ b/arch/arm/boot/dts/da850-evm.dts >>>>> @@ -27,6 +27,50 @@ >>>>> spi0 = &spi1; >>>>> }; >>>>> >>>>> + backlight { >>>>> + compatible = "gpio-backlight"; >>>>> + enable-gpios = <&gpio 7 GPIO_ACTIVE_HIGH>; /* GP0[7] */ >>>> >>>> The gpio-backlight binding does not describe a property called >>>> enable-gpios. It should just be gpios. >>> >>> I will fix that. >>> >>>> >>>> a) Are you using gpio-backlight because you are not able to get the PWM >>>> to work? >>>> >>> Yes, You told me not to worry about doing a PWM backlight because the >>> legacy board does not PWM either. >> >> Yeah, I meant not to add backlight control till the time we are able to >> get it working using PWM. Is this needed for the basic LCD functionality >> to work? I would like to avoid the churn of adding it using GPIO now and >> changing to PWM later, if possible. >> >>> >>>> b) What is GP0[7] connected to in the schematic you have? In the >>>> schematic I have I see LCD_PWM0 is connected to >>>> SPI1_SCS[0]/EPWM1B/GP2[14]/TM64P3_IN12. >>> >>> I have schematic 1016572 dated Wednesday, August 18, 2010. According >>> to it, AXR15 / EPWMN0_TZ[0] / ECAP2_APWM2 / GPIO0[7] connects to U25, >>> Pin 46 to generate M_LCD_PWM0. You might have one of the early, >>> pre-release versions. >> >> Ah, okay. In your schematic, is GP2[14] connected to anything? >> >>> >>>> >>>> c) The /* GP0[7] */ comment is not really useful on its own as it can be >>>> computed. What I wanted to see is the schematic symbol like "LCD_PWM0". >>>> Same for other places like this below. >>> >>> I can do that. >>>> >>>>> @@ -35,6 +79,16 @@ >>>>> regulator-boot-on; >>>>> }; >>>>> >>>>> + backlight_reg: backlight-regulator { >>>>> + compatible = "regulator-fixed"; >>>>> + regulator-name = "lcd_backlight_pwr"; >>>>> + regulator-min-microvolt = <3300000>; >>>>> + regulator-max-microvolt = <3300000>; >>>>> + gpio = <&gpio 47 GPIO_ACTIVE_HIGH>; /* GP2[15] */ >>>>> + regulator-always-on; >>>> >>>> Why should this regulator never be disabled? >>> >>> The gpio-backlight does not have a way that I can see to associate the >>> regulator to it. I read through the bindings, but I didn't see an >>> option to associate a regulator it. I use this regulator to drive >>> lcd_backlight_pwr and the backlight driver to write lcd_pwm0. Without >>> this option, the system disables lcd_backlight_pwr and the screen is >>> blank >> >> It sounds like this is a hack to enable backlight on this board. I think >> either the backlight driver needs to gain functionality to enable the >> GPIO. Or backlight could be treated as part of the panel and enabled >> using enable-gpios property in the panel. TBH, I will be okay either >> way. Can you check with Jyri, Tomi and rest of the DRM folks on what >> should be right way of dealing with this? > > Per your request I added them into this thread. I added Tomi, Jyri, > and Laurent to this as Laurent's name is associated with the gpio > backlight driver. Okay. The reason I did not loop them in myself is because I thought a fresh thread with background will be better. But okay. > I am not sure why you think it's a hack. I pulled up the schematic > for the LCD to see what it's doing, and the lcd_backlight_pwr pin > controls the power-on sequence of the back-light controller. Without > this, there is no power, so it seems to me that the 'regulator-fixed' > device is the correct way to do it. Not questioning modeling the GPIO as a regulator. > > The separate pin associated to the gpio is used to tell the backlight > IC to actually turn on/off the back-light. Ideally it seems like it > would nice to have the gpio-backlight driver be able to specify the > regulator, so when the backlight is in use, it would power the > regulator, but until that's available, the it seems like > 'regulator-always-on' is the way to make it stay on. We need to add support for this in backlight driver. Using regulator-always-on to paper over this lack of support in backlight driver is what I am calling a hack. 'regulator-always-on' means the regulator cannot be turned off. Thats certainly not the case as you have pointed out. Thanks, Sekhar ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V3] ARM: dts: da850-evm: Enable LCD and Backlight 2018-05-14 18:04 ` Adam Ford @ 2018-05-15 5:25 ` Laurent Pinchart -1 siblings, 0 replies; 14+ messages in thread From: Laurent Pinchart @ 2018-05-15 5:25 UTC (permalink / raw) To: Adam Ford Cc: devicetree, Kevin Hilman, Sekhar Nori, Rob Herring, Tomi Valkeinen, jsarha, arm-soc Hi Adam, On Monday, 14 May 2018 21:04:07 EEST Adam Ford wrote: > On Mon, May 14, 2018 at 7:35 AM, Sekhar Nori <nsekhar@ti.com> wrote: > > On Monday 14 May 2018 04:22 PM, Adam Ford wrote: > >> On Mon, May 14, 2018 at 12:29 AM, Sekhar Nori <nsekhar@ti.com> wrote: > >>> Hi Adam, > > Added Tomi, Laurent, and Jyri for feedback. > > >>> On Monday 14 May 2018 04:50 AM, Adam Ford wrote: > >>>> When using the board files the LCD works, but not with the DT. > >>>> This adds enables the original da850-evm to work with the same > >>>> LCD in device tree mode. > >>>> > >>>> The EVM has a gpio for the regulator and a gpio enable. The LCD and > >>>> the vpif display pins are mutually exclusive, so if using the LCD, > >>>> do not load the vpif driver. > >>> > >>> Its not sufficient just note this in patch description. > >>> > >>> a) Disable (status = "disabled") the VPIF node which clashes for pins > >>> with LCD. > >>> b) Add a comment on top of the status = "disabled" giving information on > >>> how user can enable it (disable lcdc node and then change to status = > >>> "okay"). > >>> > >>>> Signed-off-by: Adam Ford <aford173@gmail.com> > >>>> --- > >>>> V3: Fix errant GPIO, label GPIO pins, and rename the regulator to be > >>>> more explict to backlight which better matches the schematic. > >>>> Updated the description to explain that it cannot be used at the > >>>> same time as the vpif driver. > >>>> > >>>> V2: Add regulator and GPIO enable pins. Remove PWM backlight and > >>>> replace with GPIO > >>>> > >>>> diff --git a/arch/arm/boot/dts/da850-evm.dts > >>>> b/arch/arm/boot/dts/da850-evm.dts index 2e817da37fdb..3f1c8be07efe > >>>> 100644 > >>>> --- a/arch/arm/boot/dts/da850-evm.dts > >>>> +++ b/arch/arm/boot/dts/da850-evm.dts > >>>> @@ -27,6 +27,50 @@ > >>>> spi0 = &spi1; > >>>> }; > >>>> > >>>> + backlight { > >>>> + compatible = "gpio-backlight"; > >>>> + enable-gpios = <&gpio 7 GPIO_ACTIVE_HIGH>; /* GP0[7] */ > >>> > >>> The gpio-backlight binding does not describe a property called > >>> enable-gpios. It should just be gpios. > >> > >> I will fix that. > >> > >>> a) Are you using gpio-backlight because you are not able to get the PWM > >>> to work? > >> > >> Yes, You told me not to worry about doing a PWM backlight because the > >> legacy board does not PWM either. > > > > Yeah, I meant not to add backlight control till the time we are able to > > get it working using PWM. Is this needed for the basic LCD functionality > > to work? I would like to avoid the churn of adding it using GPIO now and > > changing to PWM later, if possible. > > > >>> b) What is GP0[7] connected to in the schematic you have? In the > >>> schematic I have I see LCD_PWM0 is connected to > >>> SPI1_SCS[0]/EPWM1B/GP2[14]/TM64P3_IN12. > >> > >> I have schematic 1016572 dated Wednesday, August 18, 2010. According > >> to it, AXR15 / EPWMN0_TZ[0] / ECAP2_APWM2 / GPIO0[7] connects to U25, > >> Pin 46 to generate M_LCD_PWM0. You might have one of the early, > >> pre-release versions. > > > > Ah, okay. In your schematic, is GP2[14] connected to anything? > > > >>> c) The /* GP0[7] */ comment is not really useful on its own as it can be > >>> computed. What I wanted to see is the schematic symbol like "LCD_PWM0". > >>> Same for other places like this below. > >> > >> I can do that. > >> > >>>> @@ -35,6 +79,16 @@ > >>>> regulator-boot-on; > >>>> }; > >>>> > >>>> + backlight_reg: backlight-regulator { > >>>> + compatible = "regulator-fixed"; > >>>> + regulator-name = "lcd_backlight_pwr"; > >>>> + regulator-min-microvolt = <3300000>; > >>>> + regulator-max-microvolt = <3300000>; > >>>> + gpio = <&gpio 47 GPIO_ACTIVE_HIGH>; /* GP2[15] */ > >>>> + regulator-always-on; > >>> > >>> Why should this regulator never be disabled? > >> > >> The gpio-backlight does not have a way that I can see to associate the > >> regulator to it. I read through the bindings, but I didn't see an > >> option to associate a regulator it. I use this regulator to drive > >> lcd_backlight_pwr and the backlight driver to write lcd_pwm0. Without > >> this option, the system disables lcd_backlight_pwr and the screen is > >> blank > > > > It sounds like this is a hack to enable backlight on this board. I think > > either the backlight driver needs to gain functionality to enable the > > GPIO. Or backlight could be treated as part of the panel and enabled > > using enable-gpios property in the panel. TBH, I will be okay either > > way. Can you check with Jyri, Tomi and rest of the DRM folks on what > > should be right way of dealing with this? > > Per your request I added them into this thread. I added Tomi, Jyri, > and Laurent to this as Laurent's name is associated with the gpio > backlight driver. > > I am not sure why you think it's a hack. I pulled up the schematic > for the LCD to see what it's doing, and the lcd_backlight_pwr pin > controls the power-on sequence of the back-light controller. Without > this, there is no power, so it seems to me that the 'regulator-fixed' > device is the correct way to do it. It's a hack because keeping the backlight power on when the display is off will consume power unnecessarily. The backlight power should be turned off in that case. > The separate pin associated to the gpio is used to tell the backlight > IC to actually turn on/off the back-light. Ideally it seems like it > would nice to have the gpio-backlight driver be able to specify the > regulator, so when the backlight is in use, it would power the > regulator, but until that's available, the it seems like > 'regulator-always-on' is the way to make it stay on. > > Laurent, Jyri, Tomi, do you have any thoughts on this matter? This has been attempted 6 years ago, and got rejected. See https://lwn.net/ Articles/525422/. The problem is that the number of power supplies, the order in which they need to be sequenced, and the related timings (as in delays required between turning power supplies on/off) is highly hardware-specific. We can't hardcode a specific sequence and specific timings in the gpio-backlight (or pwm- backlight) driver, and specifying the sequence in DT was considered to be over-engineered. You thus have two options, either creating a custom backlight driver that will handle the power supplies specific to your backlight controller, or integrating backlight power supply control in your panel driver. If the power supplies also power the panel and not just the backlight, or if their sequencing depends in any way on the panel model in which the backlight controller is integrated, I'd go for the latter, otherwise you can pick any of the two methods. -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH V3] ARM: dts: da850-evm: Enable LCD and Backlight @ 2018-05-15 5:25 ` Laurent Pinchart 0 siblings, 0 replies; 14+ messages in thread From: Laurent Pinchart @ 2018-05-15 5:25 UTC (permalink / raw) To: linux-arm-kernel Hi Adam, On Monday, 14 May 2018 21:04:07 EEST Adam Ford wrote: > On Mon, May 14, 2018 at 7:35 AM, Sekhar Nori <nsekhar@ti.com> wrote: > > On Monday 14 May 2018 04:22 PM, Adam Ford wrote: > >> On Mon, May 14, 2018 at 12:29 AM, Sekhar Nori <nsekhar@ti.com> wrote: > >>> Hi Adam, > > Added Tomi, Laurent, and Jyri for feedback. > > >>> On Monday 14 May 2018 04:50 AM, Adam Ford wrote: > >>>> When using the board files the LCD works, but not with the DT. > >>>> This adds enables the original da850-evm to work with the same > >>>> LCD in device tree mode. > >>>> > >>>> The EVM has a gpio for the regulator and a gpio enable. The LCD and > >>>> the vpif display pins are mutually exclusive, so if using the LCD, > >>>> do not load the vpif driver. > >>> > >>> Its not sufficient just note this in patch description. > >>> > >>> a) Disable (status = "disabled") the VPIF node which clashes for pins > >>> with LCD. > >>> b) Add a comment on top of the status = "disabled" giving information on > >>> how user can enable it (disable lcdc node and then change to status = > >>> "okay"). > >>> > >>>> Signed-off-by: Adam Ford <aford173@gmail.com> > >>>> --- > >>>> V3: Fix errant GPIO, label GPIO pins, and rename the regulator to be > >>>> more explict to backlight which better matches the schematic. > >>>> Updated the description to explain that it cannot be used at the > >>>> same time as the vpif driver. > >>>> > >>>> V2: Add regulator and GPIO enable pins. Remove PWM backlight and > >>>> replace with GPIO > >>>> > >>>> diff --git a/arch/arm/boot/dts/da850-evm.dts > >>>> b/arch/arm/boot/dts/da850-evm.dts index 2e817da37fdb..3f1c8be07efe > >>>> 100644 > >>>> --- a/arch/arm/boot/dts/da850-evm.dts > >>>> +++ b/arch/arm/boot/dts/da850-evm.dts > >>>> @@ -27,6 +27,50 @@ > >>>> spi0 = &spi1; > >>>> }; > >>>> > >>>> + backlight { > >>>> + compatible = "gpio-backlight"; > >>>> + enable-gpios = <&gpio 7 GPIO_ACTIVE_HIGH>; /* GP0[7] */ > >>> > >>> The gpio-backlight binding does not describe a property called > >>> enable-gpios. It should just be gpios. > >> > >> I will fix that. > >> > >>> a) Are you using gpio-backlight because you are not able to get the PWM > >>> to work? > >> > >> Yes, You told me not to worry about doing a PWM backlight because the > >> legacy board does not PWM either. > > > > Yeah, I meant not to add backlight control till the time we are able to > > get it working using PWM. Is this needed for the basic LCD functionality > > to work? I would like to avoid the churn of adding it using GPIO now and > > changing to PWM later, if possible. > > > >>> b) What is GP0[7] connected to in the schematic you have? In the > >>> schematic I have I see LCD_PWM0 is connected to > >>> SPI1_SCS[0]/EPWM1B/GP2[14]/TM64P3_IN12. > >> > >> I have schematic 1016572 dated Wednesday, August 18, 2010. According > >> to it, AXR15 / EPWMN0_TZ[0] / ECAP2_APWM2 / GPIO0[7] connects to U25, > >> Pin 46 to generate M_LCD_PWM0. You might have one of the early, > >> pre-release versions. > > > > Ah, okay. In your schematic, is GP2[14] connected to anything? > > > >>> c) The /* GP0[7] */ comment is not really useful on its own as it can be > >>> computed. What I wanted to see is the schematic symbol like "LCD_PWM0". > >>> Same for other places like this below. > >> > >> I can do that. > >> > >>>> @@ -35,6 +79,16 @@ > >>>> regulator-boot-on; > >>>> }; > >>>> > >>>> + backlight_reg: backlight-regulator { > >>>> + compatible = "regulator-fixed"; > >>>> + regulator-name = "lcd_backlight_pwr"; > >>>> + regulator-min-microvolt = <3300000>; > >>>> + regulator-max-microvolt = <3300000>; > >>>> + gpio = <&gpio 47 GPIO_ACTIVE_HIGH>; /* GP2[15] */ > >>>> + regulator-always-on; > >>> > >>> Why should this regulator never be disabled? > >> > >> The gpio-backlight does not have a way that I can see to associate the > >> regulator to it. I read through the bindings, but I didn't see an > >> option to associate a regulator it. I use this regulator to drive > >> lcd_backlight_pwr and the backlight driver to write lcd_pwm0. Without > >> this option, the system disables lcd_backlight_pwr and the screen is > >> blank > > > > It sounds like this is a hack to enable backlight on this board. I think > > either the backlight driver needs to gain functionality to enable the > > GPIO. Or backlight could be treated as part of the panel and enabled > > using enable-gpios property in the panel. TBH, I will be okay either > > way. Can you check with Jyri, Tomi and rest of the DRM folks on what > > should be right way of dealing with this? > > Per your request I added them into this thread. I added Tomi, Jyri, > and Laurent to this as Laurent's name is associated with the gpio > backlight driver. > > I am not sure why you think it's a hack. I pulled up the schematic > for the LCD to see what it's doing, and the lcd_backlight_pwr pin > controls the power-on sequence of the back-light controller. Without > this, there is no power, so it seems to me that the 'regulator-fixed' > device is the correct way to do it. It's a hack because keeping the backlight power on when the display is off will consume power unnecessarily. The backlight power should be turned off in that case. > The separate pin associated to the gpio is used to tell the backlight > IC to actually turn on/off the back-light. Ideally it seems like it > would nice to have the gpio-backlight driver be able to specify the > regulator, so when the backlight is in use, it would power the > regulator, but until that's available, the it seems like > 'regulator-always-on' is the way to make it stay on. > > Laurent, Jyri, Tomi, do you have any thoughts on this matter? This has been attempted 6 years ago, and got rejected. See https://lwn.net/ Articles/525422/. The problem is that the number of power supplies, the order in which they need to be sequenced, and the related timings (as in delays required between turning power supplies on/off) is highly hardware-specific. We can't hardcode a specific sequence and specific timings in the gpio-backlight (or pwm- backlight) driver, and specifying the sequence in DT was considered to be over-engineered. You thus have two options, either creating a custom backlight driver that will handle the power supplies specific to your backlight controller, or integrating backlight power supply control in your panel driver. If the power supplies also power the panel and not just the backlight, or if their sequencing depends in any way on the panel model in which the backlight controller is integrated, I'd go for the latter, otherwise you can pick any of the two methods. -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2018-05-15 5:25 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-05-13 23:20 [PATCH V3] ARM: dts: da850-evm: Enable LCD and Backlight Adam Ford 2018-05-13 23:20 ` Adam Ford 2018-05-14 5:29 ` Sekhar Nori 2018-05-14 5:29 ` Sekhar Nori 2018-05-14 10:52 ` Adam Ford 2018-05-14 10:52 ` Adam Ford 2018-05-14 12:35 ` Sekhar Nori 2018-05-14 12:35 ` Sekhar Nori 2018-05-14 18:04 ` Adam Ford 2018-05-14 18:04 ` Adam Ford 2018-05-15 5:19 ` Sekhar Nori 2018-05-15 5:19 ` Sekhar Nori 2018-05-15 5:25 ` Laurent Pinchart 2018-05-15 5:25 ` Laurent Pinchart
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.