| Message ID | 1479526093-7014-10-git-send-email-laurent.pinchart+renesas@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Laurent, On Sat, Nov 19, 2016 at 4:28 AM, Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> wrote: > The panel backlight is controlled through a GPIO and a PWM channel. > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts > +++ b/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts > @@ -178,6 +178,16 @@ > }; > }; > }; > + > + backlight: backlight { > + compatible = "pwm-backlight"; > + pwms = <&pwm1 0 50000>; > + > + brightness-levels = <256 128 64 16 8 4 0>; Would it make sense to define more and/or linear levels? > + default-brightness-level = <6>; Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert, On Monday 21 Nov 2016 09:36:22 Geert Uytterhoeven wrote: > On Sat, Nov 19, 2016 at 4:28 AM, Laurent Pinchart wrote: > > The panel backlight is controlled through a GPIO and a PWM channel. > > > > Signed-off-by: Laurent Pinchart > > <laurent.pinchart+renesas@ideasonboard.com> > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > --- a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts > > +++ b/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts > > @@ -178,6 +178,16 @@ > > }; > > }; > > }; > > + > > + backlight: backlight { > > + compatible = "pwm-backlight"; > > + pwms = <&pwm1 0 50000>; > > + > > + brightness-levels = <256 128 64 16 8 4 0>; > > Would it make sense to define more and/or linear levels? Possibly, this is pretty arbitrary. Linear levels might not be the best option given that the human eye doesn't have a linear response to light power, but we could certainly have more levels. In that case I'd prefer modifying the pwm- backlight DT bindings though, and specifying the PWM resolution instead of discrete levels. Note that the LVDS panel backlight PWM control signal is multiplexed with the external memory A21 signal on the Salvator-X board, with SW5 selecting which how to route the signal. When using backlight control we can't access the whole NOR flash anymore, so I'm not sure this patch should be merged. > > + default-brightness-level = <6>;
Hi Laurent, On Mon, Nov 21, 2016 at 10:19 AM, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > On Monday 21 Nov 2016 09:36:22 Geert Uytterhoeven wrote: >> On Sat, Nov 19, 2016 at 4:28 AM, Laurent Pinchart wrote: >> > The panel backlight is controlled through a GPIO and a PWM channel. >> > --- a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts >> > +++ b/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts >> > @@ -178,6 +178,16 @@ >> > }; >> > }; >> > }; >> > + >> > + backlight: backlight { >> > + compatible = "pwm-backlight"; >> > + pwms = <&pwm1 0 50000>; >> > + >> > + brightness-levels = <256 128 64 16 8 4 0>; >> >> Would it make sense to define more and/or linear levels? > > Possibly, this is pretty arbitrary. Linear levels might not be the best option > given that the human eye doesn't have a linear response to light power, but we It not only depends on the human eye, but also on the backlight hardware (is the conversion from voltage (L_VBRT) to light linear?). > could certainly have more levels. In that case I'd prefer modifying the pwm- > backlight DT bindings though, and specifying the PWM resolution instead of > discrete levels. > > Note that the LVDS panel backlight PWM control signal is multiplexed with the > external memory A21 signal on the Salvator-X board, with SW5 selecting which > how to route the signal. When using backlight control we can't access the > whole NOR flash anymore, so I'm not sure this patch should be merged. That NOR flash is also optional, right? My Ex Memory Connector is not populated. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert, On Monday 21 Nov 2016 10:23:46 Geert Uytterhoeven wrote: > On Mon, Nov 21, 2016 at 10:19 AM, Laurent Pinchart wrote: > > On Monday 21 Nov 2016 09:36:22 Geert Uytterhoeven wrote: > >> On Sat, Nov 19, 2016 at 4:28 AM, Laurent Pinchart wrote: > >>> The panel backlight is controlled through a GPIO and a PWM channel. > >>> > >>> --- a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts > >>> +++ b/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts > >>> @@ -178,6 +178,16 @@ > >>> }; > >>> }; > >>> }; > >>> + > >>> + backlight: backlight { > >>> + compatible = "pwm-backlight"; > >>> + pwms = <&pwm1 0 50000>; > >>> + > >>> + brightness-levels = <256 128 64 16 8 4 0>; > >> > >> Would it make sense to define more and/or linear levels? > > > > Possibly, this is pretty arbitrary. Linear levels might not be the best > > option given that the human eye doesn't have a linear response to light > > power, but we > > It not only depends on the human eye, but also on the backlight hardware > (is the conversion from voltage (L_VBRT) to light linear?). So we need to specify transfer functions in DT ;-) > > could certainly have more levels. In that case I'd prefer modifying the > > pwm- backlight DT bindings though, and specifying the PWM resolution > > instead of discrete levels. > > > > Note that the LVDS panel backlight PWM control signal is multiplexed with > > the external memory A21 signal on the Salvator-X board, with SW5 > > selecting which how to route the signal. When using backlight control we > > can't access the whole NOR flash anymore, so I'm not sure this patch > > should be merged. > > That NOR flash is also optional, right? > My Ex Memory Connector is not populated. That's correct. The Salvator-X DT file in mainline is just an example anyway, and we should pick the most useful peripherals for that purpose.
Hi Geert, On Monday 21 Nov 2016 11:59:47 Laurent Pinchart wrote: > On Monday 21 Nov 2016 10:23:46 Geert Uytterhoeven wrote: > > On Mon, Nov 21, 2016 at 10:19 AM, Laurent Pinchart wrote: > >> On Monday 21 Nov 2016 09:36:22 Geert Uytterhoeven wrote: > >>> On Sat, Nov 19, 2016 at 4:28 AM, Laurent Pinchart wrote: > >>>> The panel backlight is controlled through a GPIO and a PWM channel. > >>>> > >>>> --- a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts > >>>> +++ b/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts > >>>> @@ -178,6 +178,16 @@ > >>>> }; > >>>> }; > >>>> }; > >>>> + > >>>> + backlight: backlight { > >>>> + compatible = "pwm-backlight"; > >>>> + pwms = <&pwm1 0 50000>; > >>>> + > >>>> + brightness-levels = <256 128 64 16 8 4 0>; > >>> > >>> Would it make sense to define more and/or linear levels? > >> > >> Possibly, this is pretty arbitrary. Linear levels might not be the best > >> option given that the human eye doesn't have a linear response to light > >> power, but we > > > > It not only depends on the human eye, but also on the backlight hardware > > (is the conversion from voltage (L_VBRT) to light linear?). > > So we need to specify transfer functions in DT ;-) > > >> could certainly have more levels. In that case I'd prefer modifying the > >> pwm- backlight DT bindings though, and specifying the PWM resolution > >> instead of discrete levels. > >> > >> Note that the LVDS panel backlight PWM control signal is multiplexed > >> with the external memory A21 signal on the Salvator-X board, with SW5 > >> selecting which how to route the signal. When using backlight control we > >> can't access the whole NOR flash anymore, so I'm not sure this patch > >> should be merged. > > > > That NOR flash is also optional, right? > > My Ex Memory Connector is not populated. > > That's correct. The Salvator-X DT file in mainline is just an example > anyway, and we should pick the most useful peripherals for that purpose. Would you like me to include this in my next Salvator-X DT patch series for upstream merge ?
Hi Laurent, On Wed, Apr 5, 2017 at 10:45 AM, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > On Monday 21 Nov 2016 11:59:47 Laurent Pinchart wrote: >> On Monday 21 Nov 2016 10:23:46 Geert Uytterhoeven wrote: >> > On Mon, Nov 21, 2016 at 10:19 AM, Laurent Pinchart wrote: >> >> On Monday 21 Nov 2016 09:36:22 Geert Uytterhoeven wrote: >> >>> On Sat, Nov 19, 2016 at 4:28 AM, Laurent Pinchart wrote: >> >>>> The panel backlight is controlled through a GPIO and a PWM channel. >> >>>> >> >>>> --- a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts >> >>>> +++ b/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts >> >>>> @@ -178,6 +178,16 @@ >> >>>> }; >> >>>> }; >> >>>> }; >> >>>> + >> >>>> + backlight: backlight { >> >>>> + compatible = "pwm-backlight"; >> >>>> + pwms = <&pwm1 0 50000>; >> >>>> + >> >>>> + brightness-levels = <256 128 64 16 8 4 0>; >> >>> >> >>> Would it make sense to define more and/or linear levels? >> >> >> >> Possibly, this is pretty arbitrary. Linear levels might not be the best >> >> option given that the human eye doesn't have a linear response to light >> >> power, but we >> > >> > It not only depends on the human eye, but also on the backlight hardware >> > (is the conversion from voltage (L_VBRT) to light linear?). >> >> So we need to specify transfer functions in DT ;-) >> >> >> could certainly have more levels. In that case I'd prefer modifying the >> >> pwm- backlight DT bindings though, and specifying the PWM resolution >> >> instead of discrete levels. >> >> >> >> Note that the LVDS panel backlight PWM control signal is multiplexed >> >> with the external memory A21 signal on the Salvator-X board, with SW5 >> >> selecting which how to route the signal. When using backlight control we >> >> can't access the whole NOR flash anymore, so I'm not sure this patch >> >> should be merged. >> > >> > That NOR flash is also optional, right? >> > My Ex Memory Connector is not populated. >> >> That's correct. The Salvator-X DT file in mainline is just an example >> anyway, and we should pick the most useful peripherals for that purpose. > > Would you like me to include this in my next Salvator-X DT patch series for > upstream merge ? That's fine for me (CC Simon). Thx! Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Wed, Apr 05, 2017 at 10:55:32AM +0200, Geert Uytterhoeven wrote: > Hi Laurent, > > On Wed, Apr 5, 2017 at 10:45 AM, Laurent Pinchart > <laurent.pinchart@ideasonboard.com> wrote: > > On Monday 21 Nov 2016 11:59:47 Laurent Pinchart wrote: > >> On Monday 21 Nov 2016 10:23:46 Geert Uytterhoeven wrote: > >> > On Mon, Nov 21, 2016 at 10:19 AM, Laurent Pinchart wrote: > >> >> On Monday 21 Nov 2016 09:36:22 Geert Uytterhoeven wrote: > >> >>> On Sat, Nov 19, 2016 at 4:28 AM, Laurent Pinchart wrote: > >> >>>> The panel backlight is controlled through a GPIO and a PWM channel. > >> >>>> > >> >>>> --- a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts > >> >>>> +++ b/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts > >> >>>> @@ -178,6 +178,16 @@ > >> >>>> }; > >> >>>> }; > >> >>>> }; > >> >>>> + > >> >>>> + backlight: backlight { > >> >>>> + compatible = "pwm-backlight"; > >> >>>> + pwms = <&pwm1 0 50000>; > >> >>>> + > >> >>>> + brightness-levels = <256 128 64 16 8 4 0>; > >> >>> > >> >>> Would it make sense to define more and/or linear levels? > >> >> > >> >> Possibly, this is pretty arbitrary. Linear levels might not be the best > >> >> option given that the human eye doesn't have a linear response to light > >> >> power, but we > >> > > >> > It not only depends on the human eye, but also on the backlight hardware > >> > (is the conversion from voltage (L_VBRT) to light linear?). > >> > >> So we need to specify transfer functions in DT ;-) > >> > >> >> could certainly have more levels. In that case I'd prefer modifying the > >> >> pwm- backlight DT bindings though, and specifying the PWM resolution > >> >> instead of discrete levels. > >> >> > >> >> Note that the LVDS panel backlight PWM control signal is multiplexed > >> >> with the external memory A21 signal on the Salvator-X board, with SW5 > >> >> selecting which how to route the signal. When using backlight control we > >> >> can't access the whole NOR flash anymore, so I'm not sure this patch > >> >> should be merged. > >> > > >> > That NOR flash is also optional, right? > >> > My Ex Memory Connector is not populated. > >> > >> That's correct. The Salvator-X DT file in mainline is just an example > >> anyway, and we should pick the most useful peripherals for that purpose. > > > > Would you like me to include this in my next Salvator-X DT patch series for > > upstream merge ? > > That's fine for me (CC Simon). Thx! Sounds good to me.
diff --git a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts b/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts index 2c30813d0f86..a905318a347c 100644 --- a/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts +++ b/arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts @@ -178,6 +178,16 @@ }; }; }; + + backlight: backlight { + compatible = "pwm-backlight"; + pwms = <&pwm1 0 50000>; + + brightness-levels = <256 128 64 16 8 4 0>; + default-brightness-level = <6>; + + enable-gpios = <&gpio6 7 GPIO_ACTIVE_HIGH>; + }; }; &du { @@ -238,6 +248,11 @@ function = "du"; }; + pwm1_pins: pwm { + groups = "pwm1_a"; + function = "pwm1"; + }; + sdhi0_pins: sd0 { groups = "sdhi0_data4", "sdhi0_ctrl"; function = "sdhi0"; @@ -275,6 +290,13 @@ }; }; +&pwm1 { + pinctrl-0 = <&pwm1_pins>; + pinctrl-names = "default"; + + status = "okay"; +}; + &scif1 { pinctrl-0 = <&scif1_pins>; pinctrl-names = "default";
The panel backlight is controlled through a GPIO and a PWM channel. Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> --- arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dts | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)