| Message ID | 1412175188-28278-11-git-send-email-boris.brezillon@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Oct 01, 2014 at 04:53:07PM +0200, Boris Brezillon wrote: [...] > diff --git a/arch/arm/boot/dts/sama5d3xdm.dtsi b/arch/arm/boot/dts/sama5d3xdm.dtsi [...] > + bl_reg: backlight_regulator { > + compatible = "regulator-fixed"; > + regulator-name = "backlight-power-supply"; > + regulator-min-microvolt = <5000000>; > + regulator-max-microvolt = <5000000>; > + status = "disabled"; > + }; > + > + panel_reg: panel_regulator { > + compatible = "regulator-fixed"; > + regulator-name = "panel-power-supply"; > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + status = "disabled"; > + }; > + > + backlight: backlight { > + compatible = "pwm-backlight"; > + pwms = <&hlcdc_pwm 0 50000 0>; > + brightness-levels = <0 4 8 16 32 64 128 255>; > + default-brightness-level = <6>; > + power-supply = <&bl_reg>; > + status = "disabled"; > + }; Why are these all disabled? Patch 11/11 now needs to enable all of them explicitly for each board that uses this display module. > + panel: panel { > + compatible = "foxlink,fl500wvr00-a0t", "simple-panel"; "simple-panel" shouldn't be in this list. There's nothing useful a driver can do by matching on it. > + backlight = <&backlight>; > + power-supply = <&panel_reg>; > + #address-cells = <1>; > + #size-cells = <0>; > + status = "disabled"; > + > + port@0 { > + #address-cells = <1>; > + #size-cells = <0>; > + > + panel_input: endpoint@0 { > + reg = <0>; > + remote-endpoint = <&hlcdc_panel_output>; > + }; > }; There's no support for OF graphs in simple-panel, so this is unused, isn't it? Thierry
On Mon, 6 Oct 2014 13:01:16 +0200 Thierry Reding <thierry.reding@gmail.com> wrote: > On Wed, Oct 01, 2014 at 04:53:07PM +0200, Boris Brezillon wrote: > [...] > > diff --git a/arch/arm/boot/dts/sama5d3xdm.dtsi b/arch/arm/boot/dts/sama5d3xdm.dtsi > [...] > > + bl_reg: backlight_regulator { > > + compatible = "regulator-fixed"; > > + regulator-name = "backlight-power-supply"; > > + regulator-min-microvolt = <5000000>; > > + regulator-max-microvolt = <5000000>; > > + status = "disabled"; > > + }; > > + > > + panel_reg: panel_regulator { > > + compatible = "regulator-fixed"; > > + regulator-name = "panel-power-supply"; > > + regulator-min-microvolt = <3300000>; > > + regulator-max-microvolt = <3300000>; > > + status = "disabled"; > > + }; > > + > > + backlight: backlight { > > + compatible = "pwm-backlight"; > > + pwms = <&hlcdc_pwm 0 50000 0>; > > + brightness-levels = <0 4 8 16 32 64 128 255>; > > + default-brightness-level = <6>; > > + power-supply = <&bl_reg>; > > + status = "disabled"; > > + }; > > Why are these all disabled? Patch 11/11 now needs to enable all of them > explicitly for each board that uses this display module. Yes, these nodes should be enabled. I just thought we could include the xdm dtsi and enable what we really need in the board file, but it doesn't make any sense... > > > + panel: panel { > > + compatible = "foxlink,fl500wvr00-a0t", "simple-panel"; > > "simple-panel" shouldn't be in this list. There's nothing useful a > driver can do by matching on it. Sure, I'll remove the "simple-panel" compatible string. > > > + backlight = <&backlight>; > > + power-supply = <&panel_reg>; > > + #address-cells = <1>; > > + #size-cells = <0>; > > + status = "disabled"; > > + > > + port@0 { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + panel_input: endpoint@0 { > > + reg = <0>; > > + remote-endpoint = <&hlcdc_panel_output>; > > + }; > > }; > > There's no support for OF graphs in simple-panel, so this is unused, > isn't it? Actually I use it in my atmel_hlcdc_ouput implementation to figure out the link between a panel and a device connected on the RGB/DPI bus.
On Mon, Oct 06, 2014 at 02:25:38PM +0200, Boris Brezillon wrote: > On Mon, 6 Oct 2014 13:01:16 +0200 Thierry Reding <thierry.reding@gmail.com> wrote: > > On Wed, Oct 01, 2014 at 04:53:07PM +0200, Boris Brezillon wrote: > > [...] > > > diff --git a/arch/arm/boot/dts/sama5d3xdm.dtsi b/arch/arm/boot/dts/sama5d3xdm.dtsi [...] > > > + backlight = <&backlight>; > > > + power-supply = <&panel_reg>; > > > + #address-cells = <1>; > > > + #size-cells = <0>; > > > + status = "disabled"; > > > + > > > + port@0 { > > > + #address-cells = <1>; > > > + #size-cells = <0>; > > > + > > > + panel_input: endpoint@0 { > > > + reg = <0>; > > > + remote-endpoint = <&hlcdc_panel_output>; > > > + }; > > > }; > > > > There's no support for OF graphs in simple-panel, so this is unused, > > isn't it? > > Actually I use it in my atmel_hlcdc_ouput implementation to figure out > the link between a panel and a device connected on the RGB/DPI bus. That's kind of weird and one of the reasons why I can't make myself like the OF graph bindings. It requires drivers for one device to reach into the device tree node of some other device and look for content. Or put another way, a DT node for a panel that works on one platform doesn't work on another because the display controller needs additional DT content that isn't required by the original binding for the panel. Thierry
On Mon, 6 Oct 2014 14:40:15 +0200 Thierry Reding <thierry.reding@gmail.com> wrote: > On Mon, Oct 06, 2014 at 02:25:38PM +0200, Boris Brezillon wrote: > > On Mon, 6 Oct 2014 13:01:16 +0200 Thierry Reding <thierry.reding@gmail.com> wrote: > > > On Wed, Oct 01, 2014 at 04:53:07PM +0200, Boris Brezillon wrote: > > > [...] > > > > diff --git a/arch/arm/boot/dts/sama5d3xdm.dtsi b/arch/arm/boot/dts/sama5d3xdm.dtsi > [...] > > > > + backlight = <&backlight>; > > > > + power-supply = <&panel_reg>; > > > > + #address-cells = <1>; > > > > + #size-cells = <0>; > > > > + status = "disabled"; > > > > + > > > > + port@0 { > > > > + #address-cells = <1>; > > > > + #size-cells = <0>; > > > > + > > > > + panel_input: endpoint@0 { > > > > + reg = <0>; > > > > + remote-endpoint = <&hlcdc_panel_output>; > > > > + }; > > > > }; > > > > > > There's no support for OF graphs in simple-panel, so this is unused, > > > isn't it? > > > > Actually I use it in my atmel_hlcdc_ouput implementation to figure out > > the link between a panel and a device connected on the RGB/DPI bus. > > That's kind of weird and one of the reasons why I can't make myself like > the OF graph bindings. It requires drivers for one device to reach into > the device tree node of some other device and look for content. Or put > another way, a DT node for a panel that works on one platform doesn't > work on another because the display controller needs additional DT > content that isn't required by the original binding for the panel. I also have a working POC of a DPI bus implementation (with DPI support in panel-simple driver). This is a solution I developed to provide a generic DPI implementation in my HLCDC driver and rely on generic external implementations for slave devices (panels, encoders, ...). But, IIRC, Laurent was not in favor of a bus approach because the DPI bus is just a data bus and not a control bus. Anyway, I'll clean it up and post an RFC.
On Mon, Oct 06, 2014 at 03:11:11PM +0200, Boris Brezillon wrote: > On Mon, 6 Oct 2014 14:40:15 +0200 > Thierry Reding <thierry.reding@gmail.com> wrote: > > > On Mon, Oct 06, 2014 at 02:25:38PM +0200, Boris Brezillon wrote: > > > On Mon, 6 Oct 2014 13:01:16 +0200 Thierry Reding <thierry.reding@gmail.com> wrote: > > > > On Wed, Oct 01, 2014 at 04:53:07PM +0200, Boris Brezillon wrote: > > > > [...] > > > > > diff --git a/arch/arm/boot/dts/sama5d3xdm.dtsi b/arch/arm/boot/dts/sama5d3xdm.dtsi > > [...] > > > > > + backlight = <&backlight>; > > > > > + power-supply = <&panel_reg>; > > > > > + #address-cells = <1>; > > > > > + #size-cells = <0>; > > > > > + status = "disabled"; > > > > > + > > > > > + port@0 { > > > > > + #address-cells = <1>; > > > > > + #size-cells = <0>; > > > > > + > > > > > + panel_input: endpoint@0 { > > > > > + reg = <0>; > > > > > + remote-endpoint = <&hlcdc_panel_output>; > > > > > + }; > > > > > }; > > > > > > > > There's no support for OF graphs in simple-panel, so this is unused, > > > > isn't it? > > > > > > Actually I use it in my atmel_hlcdc_ouput implementation to figure out > > > the link between a panel and a device connected on the RGB/DPI bus. > > > > That's kind of weird and one of the reasons why I can't make myself like > > the OF graph bindings. It requires drivers for one device to reach into > > the device tree node of some other device and look for content. Or put > > another way, a DT node for a panel that works on one platform doesn't > > work on another because the display controller needs additional DT > > content that isn't required by the original binding for the panel. > > I also have a working POC of a DPI bus implementation (with DPI support > in panel-simple driver). > > This is a solution I developed to provide a generic DPI implementation > in my HLCDC driver and rely on generic external implementations for > slave devices (panels, encoders, ...). > > But, IIRC, Laurent was not in favor of a bus approach because the DPI > bus is just a data bus and not a control bus. > > Anyway, I'll clean it up and post an RFC. According to the MIPI website there are also control signals and a command set to control display behaviour. Does your implementation handle any of that? Thierry
On Mon, 6 Oct 2014 15:30:59 +0200 Thierry Reding <thierry.reding@gmail.com> wrote: > On Mon, Oct 06, 2014 at 03:11:11PM +0200, Boris Brezillon wrote: > > On Mon, 6 Oct 2014 14:40:15 +0200 > > Thierry Reding <thierry.reding@gmail.com> wrote: > > > > > On Mon, Oct 06, 2014 at 02:25:38PM +0200, Boris Brezillon wrote: > > > > On Mon, 6 Oct 2014 13:01:16 +0200 Thierry Reding <thierry.reding@gmail.com> wrote: > > > > > On Wed, Oct 01, 2014 at 04:53:07PM +0200, Boris Brezillon wrote: > > > > > [...] > > > > > > diff --git a/arch/arm/boot/dts/sama5d3xdm.dtsi b/arch/arm/boot/dts/sama5d3xdm.dtsi > > > [...] > > > > > > + backlight = <&backlight>; > > > > > > + power-supply = <&panel_reg>; > > > > > > + #address-cells = <1>; > > > > > > + #size-cells = <0>; > > > > > > + status = "disabled"; > > > > > > + > > > > > > + port@0 { > > > > > > + #address-cells = <1>; > > > > > > + #size-cells = <0>; > > > > > > + > > > > > > + panel_input: endpoint@0 { > > > > > > + reg = <0>; > > > > > > + remote-endpoint = <&hlcdc_panel_output>; > > > > > > + }; > > > > > > }; > > > > > > > > > > There's no support for OF graphs in simple-panel, so this is unused, > > > > > isn't it? > > > > > > > > Actually I use it in my atmel_hlcdc_ouput implementation to figure out > > > > the link between a panel and a device connected on the RGB/DPI bus. > > > > > > That's kind of weird and one of the reasons why I can't make myself like > > > the OF graph bindings. It requires drivers for one device to reach into > > > the device tree node of some other device and look for content. Or put > > > another way, a DT node for a panel that works on one platform doesn't > > > work on another because the display controller needs additional DT > > > content that isn't required by the original binding for the panel. > > > > I also have a working POC of a DPI bus implementation (with DPI support > > in panel-simple driver). > > > > This is a solution I developed to provide a generic DPI implementation > > in my HLCDC driver and rely on generic external implementations for > > slave devices (panels, encoders, ...). > > > > But, IIRC, Laurent was not in favor of a bus approach because the DPI > > bus is just a data bus and not a control bus. > > > > Anyway, I'll clean it up and post an RFC. > > According to the MIPI website there are also control signals and a > command set to control display behaviour. Does your implementation > handle any of that? No it doesn't, and I'm pretty sure what I call DPI does not exactly respect the MIPI DPI standard (and as such should not be called DPI, but this is another problem). This infrastructure provides a way for devices connected on a DPI bus to agree on a video_bus_format (in my case RGB888, RGB666, ...).
diff --git a/arch/arm/boot/dts/sama5d3xdm.dtsi b/arch/arm/boot/dts/sama5d3xdm.dtsi index 035ab72..91975eb 100644 --- a/arch/arm/boot/dts/sama5d3xdm.dtsi +++ b/arch/arm/boot/dts/sama5d3xdm.dtsi @@ -36,6 +36,64 @@ }; }; }; + + hlcdc: hlcdc@f0030000 { + hlcdc-display-controller { + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_lcd_base &pinctrl_lcd_rgb888_alt>; + + port@0 { + hlcdc_panel_output: endpoint@0 { + reg = <0>; + remote-endpoint = <&panel_input>; + }; + }; + }; + }; + }; + }; + + bl_reg: backlight_regulator { + compatible = "regulator-fixed"; + regulator-name = "backlight-power-supply"; + regulator-min-microvolt = <5000000>; + regulator-max-microvolt = <5000000>; + status = "disabled"; + }; + + panel_reg: panel_regulator { + compatible = "regulator-fixed"; + regulator-name = "panel-power-supply"; + regulator-min-microvolt = <3300000>; + regulator-max-microvolt = <3300000>; + status = "disabled"; + }; + + backlight: backlight { + compatible = "pwm-backlight"; + pwms = <&hlcdc_pwm 0 50000 0>; + brightness-levels = <0 4 8 16 32 64 128 255>; + default-brightness-level = <6>; + power-supply = <&bl_reg>; + status = "disabled"; + }; + + panel: panel { + compatible = "foxlink,fl500wvr00-a0t", "simple-panel"; + backlight = <&backlight>; + power-supply = <&panel_reg>; + #address-cells = <1>; + #size-cells = <0>; + status = "disabled"; + + port@0 { + #address-cells = <1>; + #size-cells = <0>; + + panel_input: endpoint@0 { + reg = <0>; + remote-endpoint = <&hlcdc_panel_output>; + }; }; }; };