diff mbox series

[RFCv1,11/42] ARM: dts: omap: add channel to DSI panels

Message ID 20191117024028.2233-12-sebastian.reichel@collabora.com (mailing list archive)
State New, archived
Headers show
Series drm/omap: Convert DSI code to use drm_mipi_dsi and drm_panel | expand

Commit Message

Sebastian Reichel Nov. 17, 2019, 2:39 a.m. UTC
The standard binding for DSI requires, that the channel number
of the panel is encoded in DT. This adds the channel number in
all OMAP3-5 boards, in preparation for using common infrastructure.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 .../devicetree/bindings/display/panel/panel-dsi-cm.txt      | 4 +++-
 arch/arm/boot/dts/omap3-n950.dts                            | 3 ++-
 arch/arm/boot/dts/omap3.dtsi                                | 3 +++
 arch/arm/boot/dts/omap4-droid4-xt894.dts                    | 3 ++-
 arch/arm/boot/dts/omap4-sdp.dts                             | 6 ++++--
 arch/arm/boot/dts/omap4.dtsi                                | 6 ++++++
 arch/arm/boot/dts/omap5.dtsi                                | 6 ++++++
 7 files changed, 26 insertions(+), 5 deletions(-)

Comments

Tomi Valkeinen Nov. 18, 2019, 1:05 p.m. UTC | #1
On 17/11/2019 04:39, Sebastian Reichel wrote:
> The standard binding for DSI requires, that the channel number
> of the panel is encoded in DT. This adds the channel number in
> all OMAP3-5 boards, in preparation for using common infrastructure.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>   .../devicetree/bindings/display/panel/panel-dsi-cm.txt      | 4 +++-
>   arch/arm/boot/dts/omap3-n950.dts                            | 3 ++-
>   arch/arm/boot/dts/omap3.dtsi                                | 3 +++
>   arch/arm/boot/dts/omap4-droid4-xt894.dts                    | 3 ++-
>   arch/arm/boot/dts/omap4-sdp.dts                             | 6 ++++--
>   arch/arm/boot/dts/omap4.dtsi                                | 6 ++++++
>   arch/arm/boot/dts/omap5.dtsi                                | 6 ++++++
>   7 files changed, 26 insertions(+), 5 deletions(-)

Is this required only in the .txt, or also by the driver? This does 
break backward compatibility with the dtbs, and there's always someone 
who won't like it.

  Tomi
Sebastian Reichel Nov. 18, 2019, 2:33 p.m. UTC | #2
Hi,

On Mon, Nov 18, 2019 at 03:05:07PM +0200, Tomi Valkeinen wrote:
> On 17/11/2019 04:39, Sebastian Reichel wrote:
> > The standard binding for DSI requires, that the channel number
> > of the panel is encoded in DT. This adds the channel number in
> > all OMAP3-5 boards, in preparation for using common infrastructure.
> > 
> > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> > ---
> >   .../devicetree/bindings/display/panel/panel-dsi-cm.txt      | 4 +++-
> >   arch/arm/boot/dts/omap3-n950.dts                            | 3 ++-
> >   arch/arm/boot/dts/omap3.dtsi                                | 3 +++
> >   arch/arm/boot/dts/omap4-droid4-xt894.dts                    | 3 ++-
> >   arch/arm/boot/dts/omap4-sdp.dts                             | 6 ++++--
> >   arch/arm/boot/dts/omap4.dtsi                                | 6 ++++++
> >   arch/arm/boot/dts/omap5.dtsi                                | 6 ++++++
> >   7 files changed, 26 insertions(+), 5 deletions(-)
> 
> Is this required only in the .txt, or also by the driver? This does break
> backward compatibility with the dtbs, and there's always someone who won't
> like it.

I add a compatible string for the Droid 4 panel in addition to the
generic one, which is not really required and just a precaution in
case we need some quirks in the future.

But I had to add the DSI channel to DT, which is required to follow
the standard DSI bindings. We cannot use the generic infrastructure
without this change. Technically it should have been there all the
time, it is only working because it is currently hardcoded to 0 in
the panel driver.

TLDR: Yes, it is required by the driver and it does break backward
compatibility for N950 (panel does not yet work on mainline, since
the OMAP3 quirks are missing in the omapdrm DSI code), omap4-sdp (
untested, I do not know if its working) and Droid 4 (known to be
working with current mainline code, most likely people update their
DT anyways).

-- Sebastian
H. Nikolaus Schaller Nov. 18, 2019, 2:37 p.m. UTC | #3
> Am 18.11.2019 um 15:33 schrieb Sebastian Reichel <sebastian.reichel@collabora.com>:
> 
> Hi,
> 
> On Mon, Nov 18, 2019 at 03:05:07PM +0200, Tomi Valkeinen wrote:
>> On 17/11/2019 04:39, Sebastian Reichel wrote:
>>> The standard binding for DSI requires, that the channel number
>>> of the panel is encoded in DT. This adds the channel number in
>>> all OMAP3-5 boards, in preparation for using common infrastructure.
>>> 
>>> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
>>> ---
>>>  .../devicetree/bindings/display/panel/panel-dsi-cm.txt      | 4 +++-
>>>  arch/arm/boot/dts/omap3-n950.dts                            | 3 ++-
>>>  arch/arm/boot/dts/omap3.dtsi                                | 3 +++
>>>  arch/arm/boot/dts/omap4-droid4-xt894.dts                    | 3 ++-
>>>  arch/arm/boot/dts/omap4-sdp.dts                             | 6 ++++--
>>>  arch/arm/boot/dts/omap4.dtsi                                | 6 ++++++
>>>  arch/arm/boot/dts/omap5.dtsi                                | 6 ++++++
>>>  7 files changed, 26 insertions(+), 5 deletions(-)
>> 
>> Is this required only in the .txt, or also by the driver? This does break
>> backward compatibility with the dtbs, and there's always someone who won't
>> like it.
> 
> I add a compatible string for the Droid 4 panel in addition to the
> generic one, which is not really required and just a precaution in
> case we need some quirks in the future.
> 
> But I had to add the DSI channel to DT, which is required to follow
> the standard DSI bindings. We cannot use the generic infrastructure
> without this change. Technically it should have been there all the
> time, it is only working because it is currently hardcoded to 0 in
> the panel driver.

Is it possible to change it to default to channel <0> if reg is not
specified?

> 
> TLDR: Yes, it is required by the driver and it does break backward
> compatibility for N950 (panel does not yet work on mainline, since
> the OMAP3 quirks are missing in the omapdrm DSI code), omap4-sdp (
> untested, I do not know if its working) and Droid 4 (known to be
> working with current mainline code, most likely people update their
> DT anyways).
> 
> -- Sebastian

BR,
Nikolaus
Sebastian Reichel Nov. 18, 2019, 3:03 p.m. UTC | #4
Hi,

On Mon, Nov 18, 2019 at 03:37:12PM +0100, H. Nikolaus Schaller wrote:
> > Am 18.11.2019 um 15:33 schrieb Sebastian Reichel <sebastian.reichel@collabora.com>:
> > On Mon, Nov 18, 2019 at 03:05:07PM +0200, Tomi Valkeinen wrote:
> >> On 17/11/2019 04:39, Sebastian Reichel wrote:
> >>> The standard binding for DSI requires, that the channel number
> >>> of the panel is encoded in DT. This adds the channel number in
> >>> all OMAP3-5 boards, in preparation for using common infrastructure.
> >>> 
> >>> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> >>> ---
> >>>  .../devicetree/bindings/display/panel/panel-dsi-cm.txt      | 4 +++-
> >>>  arch/arm/boot/dts/omap3-n950.dts                            | 3 ++-
> >>>  arch/arm/boot/dts/omap3.dtsi                                | 3 +++
> >>>  arch/arm/boot/dts/omap4-droid4-xt894.dts                    | 3 ++-
> >>>  arch/arm/boot/dts/omap4-sdp.dts                             | 6 ++++--
> >>>  arch/arm/boot/dts/omap4.dtsi                                | 6 ++++++
> >>>  arch/arm/boot/dts/omap5.dtsi                                | 6 ++++++
> >>>  7 files changed, 26 insertions(+), 5 deletions(-)
> >> 
> >> Is this required only in the .txt, or also by the driver? This does break
> >> backward compatibility with the dtbs, and there's always someone who won't
> >> like it.
> > 
> > I add a compatible string for the Droid 4 panel in addition to the
> > generic one, which is not really required and just a precaution in
> > case we need some quirks in the future.
> > 
> > But I had to add the DSI channel to DT, which is required to follow
> > the standard DSI bindings. We cannot use the generic infrastructure
> > without this change. Technically it should have been there all the
> > time, it is only working because it is currently hardcoded to 0 in
> > the panel driver.
> 
> Is it possible to change it to default to channel <0> if reg is not
> specified?

Currently nodes without reg property are skipped by of_mipi_dsi_device_add()
and of_mipi_dsi_device_add() fails if reg node is missing. Technically
it should be possible to default to channel 0 there. That affects all
platforms, though. Considering the small amount of boards affected, I think
its better to just fix the DT. Also the fixed DT does not make problems
with older kernels and can be backported.

> > TLDR: Yes, it is required by the driver and it does break backward
> > compatibility for N950 (panel does not yet work on mainline, since
> > the OMAP3 quirks are missing in the omapdrm DSI code), omap4-sdp (
> > untested, I do not know if its working) and Droid 4 (known to be
> > working with current mainline code, most likely people update their
> > DT anyways).

-- Sebastian
Tony Lindgren Nov. 18, 2019, 10:52 p.m. UTC | #5
* Sebastian Reichel <sebastian.reichel@collabora.com> [191118 15:03]:
> On Mon, Nov 18, 2019 at 03:37:12PM +0100, H. Nikolaus Schaller wrote:
> > > Am 18.11.2019 um 15:33 schrieb Sebastian Reichel <sebastian.reichel@collabora.com>:
> > > On Mon, Nov 18, 2019 at 03:05:07PM +0200, Tomi Valkeinen wrote:
> > >> On 17/11/2019 04:39, Sebastian Reichel wrote:
> > >>> The standard binding for DSI requires, that the channel number
> > >>> of the panel is encoded in DT. This adds the channel number in
> > >>> all OMAP3-5 boards, in preparation for using common infrastructure.
> > >>> 
> > >>> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> > >>> ---
> > >>>  .../devicetree/bindings/display/panel/panel-dsi-cm.txt      | 4 +++-
> > >>>  arch/arm/boot/dts/omap3-n950.dts                            | 3 ++-
> > >>>  arch/arm/boot/dts/omap3.dtsi                                | 3 +++
> > >>>  arch/arm/boot/dts/omap4-droid4-xt894.dts                    | 3 ++-
> > >>>  arch/arm/boot/dts/omap4-sdp.dts                             | 6 ++++--
> > >>>  arch/arm/boot/dts/omap4.dtsi                                | 6 ++++++
> > >>>  arch/arm/boot/dts/omap5.dtsi                                | 6 ++++++
> > >>>  7 files changed, 26 insertions(+), 5 deletions(-)
> > >> 
> > >> Is this required only in the .txt, or also by the driver? This does break
> > >> backward compatibility with the dtbs, and there's always someone who won't
> > >> like it.
> > > 
> > > I add a compatible string for the Droid 4 panel in addition to the
> > > generic one, which is not really required and just a precaution in
> > > case we need some quirks in the future.
> > > 
> > > But I had to add the DSI channel to DT, which is required to follow
> > > the standard DSI bindings. We cannot use the generic infrastructure
> > > without this change. Technically it should have been there all the
> > > time, it is only working because it is currently hardcoded to 0 in
> > > the panel driver.
> > 
> > Is it possible to change it to default to channel <0> if reg is not
> > specified?
> 
> Currently nodes without reg property are skipped by of_mipi_dsi_device_add()
> and of_mipi_dsi_device_add() fails if reg node is missing. Technically
> it should be possible to default to channel 0 there. That affects all
> platforms, though. Considering the small amount of boards affected, I think
> its better to just fix the DT. Also the fixed DT does not make problems
> with older kernels and can be backported.

You might be able to do a local fixup at driver probe time using
of_add_property(). See for example pcs_quirk_missing_pinctrl_cells()
I added earlier because of similar issues.

Regards,

Tony
Sebastian Reichel Nov. 19, 2019, 9:23 p.m. UTC | #6
Hi Tony,

On Mon, Nov 18, 2019 at 02:52:09PM -0800, Tony Lindgren wrote:
> * Sebastian Reichel <sebastian.reichel@collabora.com> [191118 15:03]:
> > On Mon, Nov 18, 2019 at 03:37:12PM +0100, H. Nikolaus Schaller wrote:
> > > > Am 18.11.2019 um 15:33 schrieb Sebastian Reichel <sebastian.reichel@collabora.com>:
> > > > On Mon, Nov 18, 2019 at 03:05:07PM +0200, Tomi Valkeinen wrote:
> > > >> On 17/11/2019 04:39, Sebastian Reichel wrote:
> > > >>> The standard binding for DSI requires, that the channel number
> > > >>> of the panel is encoded in DT. This adds the channel number in
> > > >>> all OMAP3-5 boards, in preparation for using common infrastructure.
> > > >>> 
> > > >>> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> > > >>> ---
> > > >>>  .../devicetree/bindings/display/panel/panel-dsi-cm.txt      | 4 +++-
> > > >>>  arch/arm/boot/dts/omap3-n950.dts                            | 3 ++-
> > > >>>  arch/arm/boot/dts/omap3.dtsi                                | 3 +++
> > > >>>  arch/arm/boot/dts/omap4-droid4-xt894.dts                    | 3 ++-
> > > >>>  arch/arm/boot/dts/omap4-sdp.dts                             | 6 ++++--
> > > >>>  arch/arm/boot/dts/omap4.dtsi                                | 6 ++++++
> > > >>>  arch/arm/boot/dts/omap5.dtsi                                | 6 ++++++
> > > >>>  7 files changed, 26 insertions(+), 5 deletions(-)
> > > >> 
> > > >> Is this required only in the .txt, or also by the driver? This does break
> > > >> backward compatibility with the dtbs, and there's always someone who won't
> > > >> like it.
> > > > 
> > > > I add a compatible string for the Droid 4 panel in addition to the
> > > > generic one, which is not really required and just a precaution in
> > > > case we need some quirks in the future.
> > > > 
> > > > But I had to add the DSI channel to DT, which is required to follow
> > > > the standard DSI bindings. We cannot use the generic infrastructure
> > > > without this change. Technically it should have been there all the
> > > > time, it is only working because it is currently hardcoded to 0 in
> > > > the panel driver.
> > > 
> > > Is it possible to change it to default to channel <0> if reg is not
> > > specified?
> > 
> > Currently nodes without reg property are skipped by of_mipi_dsi_device_add()
> > and of_mipi_dsi_device_add() fails if reg node is missing. Technically
> > it should be possible to default to channel 0 there. That affects all
> > platforms, though. Considering the small amount of boards affected, I think
> > its better to just fix the DT. Also the fixed DT does not make problems
> > with older kernels and can be backported.
> 
> You might be able to do a local fixup at driver probe time using
> of_add_property(). See for example pcs_quirk_missing_pinctrl_cells()
> I added earlier because of similar issues.

That sounds like a good plan. I suppose it could be added for some
kernel releases with a WARN() asking the user to update their DT.

-- Sebastian
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/display/panel/panel-dsi-cm.txt b/Documentation/devicetree/bindings/display/panel/panel-dsi-cm.txt
index dce48eb9db57..f92d5c9adfc5 100644
--- a/Documentation/devicetree/bindings/display/panel/panel-dsi-cm.txt
+++ b/Documentation/devicetree/bindings/display/panel/panel-dsi-cm.txt
@@ -3,6 +3,7 @@  Generic MIPI DSI Command Mode Panel
 
 Required properties:
 - compatible: "panel-dsi-cm"
+- reg: DSI channel number
 
 Optional properties:
 - label: a symbolic name for the panel
@@ -15,9 +16,10 @@  Required nodes:
 Example
 -------
 
-lcd0: display {
+lcd0: panel@0 {
 	compatible = "tpo,taal", "panel-dsi-cm";
 	label = "lcd0";
+	reg = <0>;
 
 	reset-gpios = <&gpio4 6 GPIO_ACTIVE_HIGH>;
 
diff --git a/arch/arm/boot/dts/omap3-n950.dts b/arch/arm/boot/dts/omap3-n950.dts
index 9886bf8b90ab..d1dfafde351e 100644
--- a/arch/arm/boot/dts/omap3-n950.dts
+++ b/arch/arm/boot/dts/omap3-n950.dts
@@ -225,8 +225,9 @@ 
 		};
 	};
 
-	lcd0: display {
+	lcd0: panel@0 {
 		compatible = "nokia,himalaya", "panel-dsi-cm";
+		reg = <0>;
 		label = "lcd0";
 
 		pinctrl-names = "default";
diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
index 4043ecb38016..b872b933f2cb 100644
--- a/arch/arm/boot/dts/omap3.dtsi
+++ b/arch/arm/boot/dts/omap3.dtsi
@@ -771,6 +771,9 @@ 
 				ti,hwmods = "dss_dsi1";
 				clocks = <&dss1_alwon_fck>, <&dss2_alwon_fck>;
 				clock-names = "fck", "sys_clk";
+
+				#address-cells = <1>;
+				#size-cells = <0>;
 			};
 
 			rfbi: encoder@48050800 {
diff --git a/arch/arm/boot/dts/omap4-droid4-xt894.dts b/arch/arm/boot/dts/omap4-droid4-xt894.dts
index a40fe8d49da6..6af0a9288940 100644
--- a/arch/arm/boot/dts/omap4-droid4-xt894.dts
+++ b/arch/arm/boot/dts/omap4-droid4-xt894.dts
@@ -202,8 +202,9 @@ 
 		};
 	};
 
-	lcd0: display {
+	lcd0: panel@0 {
 		compatible = "panel-dsi-cm";
+		reg = <0>;
 		label = "lcd0";
 		vddi-supply = <&lcd_regulator>;
 		reset-gpios = <&gpio4 5 GPIO_ACTIVE_HIGH>;	/* gpio101 */
diff --git a/arch/arm/boot/dts/omap4-sdp.dts b/arch/arm/boot/dts/omap4-sdp.dts
index 91480ac1f328..8a8307517dab 100644
--- a/arch/arm/boot/dts/omap4-sdp.dts
+++ b/arch/arm/boot/dts/omap4-sdp.dts
@@ -662,8 +662,9 @@ 
 		};
 	};
 
-	lcd0: display {
+	lcd0: panel@0 {
 		compatible = "tpo,taal", "panel-dsi-cm";
+		reg = <0>;
 		label = "lcd0";
 
 		reset-gpios = <&gpio4 6 GPIO_ACTIVE_HIGH>;	/* 102 */
@@ -687,8 +688,9 @@ 
 		};
 	};
 
-	lcd1: display {
+	lcd1: panel@0 {
 		compatible = "tpo,taal", "panel-dsi-cm";
+		reg = <0>;
 		label = "lcd1";
 
 		reset-gpios = <&gpio4 8 GPIO_ACTIVE_HIGH>;	/* 104 */
diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
index 7cc95bc1598b..a0807cd90eff 100644
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -403,6 +403,9 @@ 
 				clocks = <&l3_dss_clkctrl OMAP4_DSS_CORE_CLKCTRL 8>,
 					 <&l3_dss_clkctrl OMAP4_DSS_CORE_CLKCTRL 10>;
 				clock-names = "fck", "sys_clk";
+
+				#address-cells = <1>;
+				#size-cells = <0>;
 			};
 
 			dsi2: encoder@58005000 {
@@ -417,6 +420,9 @@ 
 				clocks = <&l3_dss_clkctrl OMAP4_DSS_CORE_CLKCTRL 8>,
 					 <&l3_dss_clkctrl OMAP4_DSS_CORE_CLKCTRL 10>;
 				clock-names = "fck", "sys_clk";
+
+				#address-cells = <1>;
+				#size-cells = <0>;
 			};
 
 			hdmi: encoder@58006000 {
diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
index 1fb7937638f0..05fd27a1f42e 100644
--- a/arch/arm/boot/dts/omap5.dtsi
+++ b/arch/arm/boot/dts/omap5.dtsi
@@ -321,6 +321,9 @@ 
 				clocks = <&dss_clkctrl OMAP5_DSS_CORE_CLKCTRL 8>,
 					 <&dss_clkctrl OMAP5_DSS_CORE_CLKCTRL 10>;
 				clock-names = "fck", "sys_clk";
+
+				#address-cells = <1>;
+				#size-cells = <0>;
 			};
 
 			dsi2: encoder@58005000 {
@@ -335,6 +338,9 @@ 
 				clocks = <&dss_clkctrl OMAP5_DSS_CORE_CLKCTRL 8>,
 					 <&dss_clkctrl OMAP5_DSS_CORE_CLKCTRL 10>;
 				clock-names = "fck", "sys_clk";
+
+				#address-cells = <1>;
+				#size-cells = <0>;
 			};
 
 			hdmi: encoder@58060000 {