devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] drm/imx/lcdc: drm driver for imx21/25/27
@ 2022-01-28 10:58 Uwe Kleine-König
  2022-01-28 10:58 ` [PATCH 1/2] dt-bindings: display: imx: Add fsl,imx21-lcdc docs Uwe Kleine-König
  0 siblings, 1 reply; 9+ messages in thread
From: Uwe Kleine-König @ 2022-01-28 10:58 UTC (permalink / raw)
  To: Philipp Zabel, Rob Herring
  Cc: David Airlie, Daniel Vetter, Shawn Guo, Pengutronix Kernel Team,
	Fabio Estevam, NXP Linux Team, dri-devel, linux-arm-kernel,
	devicetree

Hello,

this patchset was created mostly by Marian Cichy, who in the meantime
left Pengutronix. I still kept his name and email address as author, but
note that the email address doesn't reach Marian any more.

There is already a maintainer entry for imx DRM drivers that matches
good enough.

This was tested on an i.MX25 based customer machine.

Best regards
Uwe

Marian Cichy (2):
  dt-bindings: display: imx: Add fsl,imx21-lcdc docs
  drm/imx/lcdc: Implement DRM driver for imx21

 .../bindings/display/imx/fsl,imx21-lcdc.yaml  |  79 +++
 drivers/gpu/drm/imx/Kconfig                   |   9 +
 drivers/gpu/drm/imx/Makefile                  |   2 +
 drivers/gpu/drm/imx/imx21-lcdc/imx21-lcdc.c   | 631 ++++++++++++++++++
 4 files changed, 721 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,imx21-lcdc.yaml
 create mode 100644 drivers/gpu/drm/imx/imx21-lcdc/imx21-lcdc.c

-- 
2.34.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/2] dt-bindings: display: imx: Add fsl,imx21-lcdc docs
  2022-01-28 10:58 [PATCH 0/2] drm/imx/lcdc: drm driver for imx21/25/27 Uwe Kleine-König
@ 2022-01-28 10:58 ` Uwe Kleine-König
  2022-01-28 13:04   ` Rob Herring
  0 siblings, 1 reply; 9+ messages in thread
From: Uwe Kleine-König @ 2022-01-28 10:58 UTC (permalink / raw)
  To: Philipp Zabel, Rob Herring
  Cc: David Airlie, Daniel Vetter, Shawn Guo, Pengutronix Kernel Team,
	Fabio Estevam, NXP Linux Team, dri-devel, linux-arm-kernel,
	devicetree

From: Marian Cichy <m.cichy@pengutronix.de>

This files documents the device tree for the new imx21-lcdc DRM driver.

Signed-off-by: Marian Cichy <m.cichy@pengutronix.de>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 .../bindings/display/imx/fsl,imx21-lcdc.yaml  | 79 +++++++++++++++++++
 1 file changed, 79 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,imx21-lcdc.yaml

diff --git a/Documentation/devicetree/bindings/display/imx/fsl,imx21-lcdc.yaml b/Documentation/devicetree/bindings/display/imx/fsl,imx21-lcdc.yaml
new file mode 100644
index 000000000000..edf71cfac81c
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/imx/fsl,imx21-lcdc.yaml
@@ -0,0 +1,79 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/imx/fsl,imx21-lcdc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: i.MX21 LCD Controller
+
+maintainers:
+  - Philipp Zabel <p.zabel@pengutronix.de>
+
+properties:
+  compatible:
+    oneOf:
+      - const: fsl,imx21-lcdc
+      - items:
+          - enum:
+              - fsl,imx25-lcdc
+              - fsl,imx27-lcdc
+          - const: fsl,imx21-lcdc
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 3
+
+  clock-names:
+    items:
+      - const: ipg
+      - const: per
+      - const: ahb
+
+  resets:
+    maxItems: 1
+
+  port:
+    type: object
+    description:
+      "Video port for DPI RGB output."
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+  - port
+
+additionalProperties: false
+
+examples:
+  - |
+    lcdc: lcdc@53fbc000 {
+        compatible = "fsl,imx25-lcdc", "fsl,imx21-lcdc";
+        reg = <0x53fbc000 0x4000>;
+        interrupts = <39>;
+        clocks = <&clks 103>, <&clks 66>, <&clks 49>;
+        clock-names = "ipg", "ahb", "per";
+
+        port {
+             parallel_out: endpoint {
+                 remote-endpoint = <&panel_in>;
+             };
+        };
+
+    };
+
+    panel: panel {
+        compatible = "edt,etm0700g0dh6";
+        power-supply = <&lcd_supply>;
+        backlight = <&bl>;
+
+        port {
+                panel_in: endpoint {
+                        remote-endpoint = <&parallel_out>;
+                };
+        };
+    };
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] dt-bindings: display: imx: Add fsl,imx21-lcdc docs
  2022-01-28 10:58 ` [PATCH 1/2] dt-bindings: display: imx: Add fsl,imx21-lcdc docs Uwe Kleine-König
@ 2022-01-28 13:04   ` Rob Herring
  2022-01-28 17:58     ` Uwe Kleine-König
  0 siblings, 1 reply; 9+ messages in thread
From: Rob Herring @ 2022-01-28 13:04 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Philipp Zabel, David Airlie, Daniel Vetter, Shawn Guo,
	Pengutronix Kernel Team, Fabio Estevam, NXP Linux Team,
	dri-devel, linux-arm-kernel, devicetree

On Fri, Jan 28, 2022 at 4:59 AM Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
>
> From: Marian Cichy <m.cichy@pengutronix.de>
>
> This files documents the device tree for the new imx21-lcdc DRM driver.

No, bindings document h/w and the h/w has not changed. We already have
a binding for the LCDC.

>
> Signed-off-by: Marian Cichy <m.cichy@pengutronix.de>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  .../bindings/display/imx/fsl,imx21-lcdc.yaml  | 79 +++++++++++++++++++
>  1 file changed, 79 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,imx21-lcdc.yaml
>
> diff --git a/Documentation/devicetree/bindings/display/imx/fsl,imx21-lcdc.yaml b/Documentation/devicetree/bindings/display/imx/fsl,imx21-lcdc.yaml
> new file mode 100644
> index 000000000000..edf71cfac81c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/imx/fsl,imx21-lcdc.yaml
> @@ -0,0 +1,79 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/imx/fsl,imx21-lcdc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: i.MX21 LCD Controller
> +
> +maintainers:
> +  - Philipp Zabel <p.zabel@pengutronix.de>
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - const: fsl,imx21-lcdc
> +      - items:
> +          - enum:
> +              - fsl,imx25-lcdc
> +              - fsl,imx27-lcdc
> +          - const: fsl,imx21-lcdc
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 3
> +
> +  clock-names:
> +    items:
> +      - const: ipg
> +      - const: per
> +      - const: ahb
> +
> +  resets:
> +    maxItems: 1
> +
> +  port:
> +    type: object
> +    description:
> +      "Video port for DPI RGB output."
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +  - port
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    lcdc: lcdc@53fbc000 {
> +        compatible = "fsl,imx25-lcdc", "fsl,imx21-lcdc";
> +        reg = <0x53fbc000 0x4000>;
> +        interrupts = <39>;
> +        clocks = <&clks 103>, <&clks 66>, <&clks 49>;
> +        clock-names = "ipg", "ahb", "per";
> +
> +        port {
> +             parallel_out: endpoint {
> +                 remote-endpoint = <&panel_in>;
> +             };
> +        };
> +
> +    };
> +
> +    panel: panel {
> +        compatible = "edt,etm0700g0dh6";
> +        power-supply = <&lcd_supply>;
> +        backlight = <&bl>;
> +
> +        port {
> +                panel_in: endpoint {
> +                        remote-endpoint = <&parallel_out>;
> +                };
> +        };
> +    };
> --
> 2.34.1
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] dt-bindings: display: imx: Add fsl,imx21-lcdc docs
  2022-01-28 13:04   ` Rob Herring
@ 2022-01-28 17:58     ` Uwe Kleine-König
  2022-02-01 17:35       ` Rob Herring
  0 siblings, 1 reply; 9+ messages in thread
From: Uwe Kleine-König @ 2022-01-28 17:58 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree, Daniel Vetter, David Airlie, Shawn Guo, dri-devel,
	NXP Linux Team, Pengutronix Kernel Team, Philipp Zabel,
	Fabio Estevam, linux-arm-kernel

[-- Attachment #1: Type: text/plain, Size: 989 bytes --]

Hello Rob,

On Fri, Jan 28, 2022 at 07:04:10AM -0600, Rob Herring wrote:
> On Fri, Jan 28, 2022 at 4:59 AM Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> wrote:
> >
> > From: Marian Cichy <m.cichy@pengutronix.de>
> >
> > This files documents the device tree for the new imx21-lcdc DRM driver.
> 
> No, bindings document h/w and the h/w has not changed. We already have
> a binding for the LCDC.

Just to be sure we're talking about the same thing: You're refering to
Documentation/devicetree/bindings/display/imx/fsl,imx-fb.txt, right?

I'm unsure what to do now. Should the two different bindings just be
described in the same file? Should I stick to fsl,imx21-fb even for the
new binding? (The hardware unit is named LCDC, so the name chosen here
is the better one.) Please advise.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] dt-bindings: display: imx: Add fsl,imx21-lcdc docs
  2022-01-28 17:58     ` Uwe Kleine-König
@ 2022-02-01 17:35       ` Rob Herring
  2022-02-10 17:54         ` Lucas Stach
  0 siblings, 1 reply; 9+ messages in thread
From: Rob Herring @ 2022-02-01 17:35 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: devicetree, Daniel Vetter, David Airlie, Shawn Guo, dri-devel,
	NXP Linux Team, Pengutronix Kernel Team, Philipp Zabel,
	Fabio Estevam, linux-arm-kernel

On Fri, Jan 28, 2022 at 06:58:29PM +0100, Uwe Kleine-König wrote:
> Hello Rob,
> 
> On Fri, Jan 28, 2022 at 07:04:10AM -0600, Rob Herring wrote:
> > On Fri, Jan 28, 2022 at 4:59 AM Uwe Kleine-König
> > <u.kleine-koenig@pengutronix.de> wrote:
> > >
> > > From: Marian Cichy <m.cichy@pengutronix.de>
> > >
> > > This files documents the device tree for the new imx21-lcdc DRM driver.
> > 
> > No, bindings document h/w and the h/w has not changed. We already have
> > a binding for the LCDC.
> 
> Just to be sure we're talking about the same thing: You're refering to
> Documentation/devicetree/bindings/display/imx/fsl,imx-fb.txt, right?

Looks right...

> I'm unsure what to do now. Should the two different bindings just be
> described in the same file? Should I stick to fsl,imx21-fb even for the
> new binding? (The hardware unit is named LCDC, so the name chosen here
> is the better one.) Please advise.

Yes, the name is unfortunate, but it should be 1 binding, 1 file, and 
unchanged (unless you want to add new optional properties). 

Rob

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] dt-bindings: display: imx: Add fsl,imx21-lcdc docs
  2022-02-01 17:35       ` Rob Herring
@ 2022-02-10 17:54         ` Lucas Stach
  2022-02-21 13:55           ` Uwe Kleine-König
  2022-03-22 18:56           ` Rob Herring
  0 siblings, 2 replies; 9+ messages in thread
From: Lucas Stach @ 2022-02-10 17:54 UTC (permalink / raw)
  To: Rob Herring, Uwe Kleine-König
  Cc: devicetree, Pengutronix Kernel Team, David Airlie, Fabio Estevam,
	dri-devel, NXP Linux Team, Daniel Vetter, Philipp Zabel,
	Shawn Guo, linux-arm-kernel

Hi Rob,

Am Dienstag, dem 01.02.2022 um 11:35 -0600 schrieb Rob Herring:
> On Fri, Jan 28, 2022 at 06:58:29PM +0100, Uwe Kleine-König wrote:
> > Hello Rob,
> > 
> > On Fri, Jan 28, 2022 at 07:04:10AM -0600, Rob Herring wrote:
> > > On Fri, Jan 28, 2022 at 4:59 AM Uwe Kleine-König
> > > <u.kleine-koenig@pengutronix.de> wrote:
> > > > 
> > > > From: Marian Cichy <m.cichy@pengutronix.de>
> > > > 
> > > > This files documents the device tree for the new imx21-lcdc DRM driver.
> > > 
> > > No, bindings document h/w and the h/w has not changed. We already have
> > > a binding for the LCDC.
> > 
> > Just to be sure we're talking about the same thing: You're refering to
> > Documentation/devicetree/bindings/display/imx/fsl,imx-fb.txt, right?
> 
> Looks right...
> 
> > I'm unsure what to do now. Should the two different bindings just be
> > described in the same file? Should I stick to fsl,imx21-fb even for the
> > new binding? (The hardware unit is named LCDC, so the name chosen here
> > is the better one.) Please advise.
> 
> Yes, the name is unfortunate, but it should be 1 binding, 1 file, and 
> unchanged (unless you want to add new optional properties). 
> 
The old FB driver binding is pretty insane. Except the reg and
interrupt properties it is very confused about things. It exposes
internal implementation details (like specifying verbatim register
settings in the DT) and other properties are just misplaced, like the
lcd-supply property that controls the panel power supply.

I really don't think that trying to stay backwards compatible here is a
win for anyone. Anyone willing to switch their systems running on a 15
year old SoC to the new DRM driver probably doesn't mind if they have
to modify the DTS to make it work. Can we please let the FB driver die
in peace and have a clean slate driver/binding for the DRM driver?

Regards,
Lucas


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] dt-bindings: display: imx: Add fsl,imx21-lcdc docs
  2022-02-10 17:54         ` Lucas Stach
@ 2022-02-21 13:55           ` Uwe Kleine-König
  2022-03-22 17:43             ` Uwe Kleine-König
  2022-03-22 18:56           ` Rob Herring
  1 sibling, 1 reply; 9+ messages in thread
From: Uwe Kleine-König @ 2022-02-21 13:55 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree, Lucas Stach, Daniel Vetter, David Airlie, Shawn Guo,
	dri-devel, NXP Linux Team, Pengutronix Kernel Team,
	Philipp Zabel, Fabio Estevam, linux-arm-kernel

[-- Attachment #1: Type: text/plain, Size: 2610 bytes --]

Hi Rob,

On Thu, Feb 10, 2022 at 06:54:13PM +0100, Lucas Stach wrote:
> Am Dienstag, dem 01.02.2022 um 11:35 -0600 schrieb Rob Herring:
> > On Fri, Jan 28, 2022 at 06:58:29PM +0100, Uwe Kleine-König wrote:
> > > On Fri, Jan 28, 2022 at 07:04:10AM -0600, Rob Herring wrote:
> > > > On Fri, Jan 28, 2022 at 4:59 AM Uwe Kleine-König
> > > > <u.kleine-koenig@pengutronix.de> wrote:
> > > > > 
> > > > > From: Marian Cichy <m.cichy@pengutronix.de>
> > > > > 
> > > > > This files documents the device tree for the new imx21-lcdc DRM driver.
> > > > 
> > > > No, bindings document h/w and the h/w has not changed. We already have
> > > > a binding for the LCDC.
> > > 
> > > Just to be sure we're talking about the same thing: You're refering to
> > > Documentation/devicetree/bindings/display/imx/fsl,imx-fb.txt, right?
> > 
> > Looks right...
> > 
> > > I'm unsure what to do now. Should the two different bindings just be
> > > described in the same file? Should I stick to fsl,imx21-fb even for the
> > > new binding? (The hardware unit is named LCDC, so the name chosen here
> > > is the better one.) Please advise.
> > 
> > Yes, the name is unfortunate, but it should be 1 binding, 1 file, and 
> > unchanged (unless you want to add new optional properties). 
>
> The old FB driver binding is pretty insane. Except the reg and
> interrupt properties it is very confused about things. It exposes
> internal implementation details (like specifying verbatim register
> settings in the DT) and other properties are just misplaced, like the
> lcd-supply property that controls the panel power supply.
> 
> I really don't think that trying to stay backwards compatible here is a
> win for anyone. Anyone willing to switch their systems running on a 15
> year old SoC to the new DRM driver probably doesn't mind if they have
> to modify the DTS to make it work. Can we please let the FB driver die
> in peace and have a clean slate driver/binding for the DRM driver?

Does this feedback change anything on your side? My expectation is that
the graphics people will be happy about every fb driver being replaced
by a DRM driver and there will be hardly any incentive to add a layer
over the DRM driver to make it honor the old fb binding.

Assume I convert the old binding to yaml and then add the newly
supported binding to that using a big oneOf, would that be an acceptable
compromise?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] dt-bindings: display: imx: Add fsl,imx21-lcdc docs
  2022-02-21 13:55           ` Uwe Kleine-König
@ 2022-03-22 17:43             ` Uwe Kleine-König
  0 siblings, 0 replies; 9+ messages in thread
From: Uwe Kleine-König @ 2022-03-22 17:43 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree, Pengutronix Kernel Team, David Airlie, Fabio Estevam,
	dri-devel, NXP Linux Team, Daniel Vetter, Philipp Zabel,
	Shawn Guo, linux-arm-kernel, Lucas Stach

[-- Attachment #1: Type: text/plain, Size: 2861 bytes --]

Hi Rob,

On Mon, Feb 21, 2022 at 02:55:36PM +0100, Uwe Kleine-König wrote:
> On Thu, Feb 10, 2022 at 06:54:13PM +0100, Lucas Stach wrote:
> > Am Dienstag, dem 01.02.2022 um 11:35 -0600 schrieb Rob Herring:
> > > On Fri, Jan 28, 2022 at 06:58:29PM +0100, Uwe Kleine-König wrote:
> > > > On Fri, Jan 28, 2022 at 07:04:10AM -0600, Rob Herring wrote:
> > > > > On Fri, Jan 28, 2022 at 4:59 AM Uwe Kleine-König
> > > > > <u.kleine-koenig@pengutronix.de> wrote:
> > > > > > 
> > > > > > From: Marian Cichy <m.cichy@pengutronix.de>
> > > > > > 
> > > > > > This files documents the device tree for the new imx21-lcdc DRM driver.
> > > > > 
> > > > > No, bindings document h/w and the h/w has not changed. We already have
> > > > > a binding for the LCDC.
> > > > 
> > > > Just to be sure we're talking about the same thing: You're refering to
> > > > Documentation/devicetree/bindings/display/imx/fsl,imx-fb.txt, right?
> > > 
> > > Looks right...
> > > 
> > > > I'm unsure what to do now. Should the two different bindings just be
> > > > described in the same file? Should I stick to fsl,imx21-fb even for the
> > > > new binding? (The hardware unit is named LCDC, so the name chosen here
> > > > is the better one.) Please advise.
> > > 
> > > Yes, the name is unfortunate, but it should be 1 binding, 1 file, and 
> > > unchanged (unless you want to add new optional properties). 
> >
> > The old FB driver binding is pretty insane. Except the reg and
> > interrupt properties it is very confused about things. It exposes
> > internal implementation details (like specifying verbatim register
> > settings in the DT) and other properties are just misplaced, like the
> > lcd-supply property that controls the panel power supply.
> > 
> > I really don't think that trying to stay backwards compatible here is a
> > win for anyone. Anyone willing to switch their systems running on a 15
> > year old SoC to the new DRM driver probably doesn't mind if they have
> > to modify the DTS to make it work. Can we please let the FB driver die
> > in peace and have a clean slate driver/binding for the DRM driver?
> 
> Does this feedback change anything on your side? My expectation is that
> the graphics people will be happy about every fb driver being replaced
> by a DRM driver and there will be hardly any incentive to add a layer
> over the DRM driver to make it honor the old fb binding.
> 
> Assume I convert the old binding to yaml and then add the newly
> supported binding to that using a big oneOf, would that be an acceptable
> compromise?

I'd like to get forward with this driver. What would be a good way to
continue here? 

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] dt-bindings: display: imx: Add fsl,imx21-lcdc docs
  2022-02-10 17:54         ` Lucas Stach
  2022-02-21 13:55           ` Uwe Kleine-König
@ 2022-03-22 18:56           ` Rob Herring
  1 sibling, 0 replies; 9+ messages in thread
From: Rob Herring @ 2022-03-22 18:56 UTC (permalink / raw)
  To: Lucas Stach
  Cc: Uwe Kleine-König, devicetree, Pengutronix Kernel Team,
	David Airlie, Fabio Estevam, dri-devel, NXP Linux Team,
	Daniel Vetter, Philipp Zabel, Shawn Guo, linux-arm-kernel

On Thu, Feb 10, 2022 at 11:54 AM Lucas Stach <l.stach@pengutronix.de> wrote:
>
> Hi Rob,
>
> Am Dienstag, dem 01.02.2022 um 11:35 -0600 schrieb Rob Herring:
> > On Fri, Jan 28, 2022 at 06:58:29PM +0100, Uwe Kleine-König wrote:
> > > Hello Rob,
> > >
> > > On Fri, Jan 28, 2022 at 07:04:10AM -0600, Rob Herring wrote:
> > > > On Fri, Jan 28, 2022 at 4:59 AM Uwe Kleine-König
> > > > <u.kleine-koenig@pengutronix.de> wrote:
> > > > >
> > > > > From: Marian Cichy <m.cichy@pengutronix.de>
> > > > >
> > > > > This files documents the device tree for the new imx21-lcdc DRM driver.
> > > >
> > > > No, bindings document h/w and the h/w has not changed. We already have
> > > > a binding for the LCDC.
> > >
> > > Just to be sure we're talking about the same thing: You're refering to
> > > Documentation/devicetree/bindings/display/imx/fsl,imx-fb.txt, right?
> >
> > Looks right...
> >
> > > I'm unsure what to do now. Should the two different bindings just be
> > > described in the same file? Should I stick to fsl,imx21-fb even for the
> > > new binding? (The hardware unit is named LCDC, so the name chosen here
> > > is the better one.) Please advise.
> >
> > Yes, the name is unfortunate, but it should be 1 binding, 1 file, and
> > unchanged (unless you want to add new optional properties).
> >
> The old FB driver binding is pretty insane. Except the reg and
> interrupt properties it is very confused about things. It exposes
> internal implementation details (like specifying verbatim register
> settings in the DT) and other properties are just misplaced, like the
> lcd-supply property that controls the panel power supply.

I agree on 'lcd-supply', but that can simply be marked as deprecated
as can anything else. From what I remember working on these chips, I'm
not sure you can really avoid some of these register properties. For
example, the Sharp config is pretty much a use some value with some
specific Sharp panel. I guess we can have 'if panel A, then register
value is X' type code in the driver. Maybe the DMA settings can be
heuristics based on the pixel data rate, but I recall avoiding
underruns was challenging on some parts.

> I really don't think that trying to stay backwards compatible here is a
> win for anyone. Anyone willing to switch their systems running on a 15
> year old SoC to the new DRM driver probably doesn't mind if they have
> to modify the DTS to make it work. Can we please let the FB driver die
> in peace and have a clean slate driver/binding for the DRM driver?

The existing binding will still need a schema if it appears in dts files.

There's nothing really conflicting between the 2 bindings. Your
choices are to merge it all into 1 node and it's up to kernel
configuration (or module load) to select which driver. Or you have 2
nodes in the DT with one enabled at a time (because 2 enabled nodes at
the same address is not allowed). Then you need a DT change to switch.
Either way is fine and the schema should match which one you pick, but
I would do 1 node.

If you do a new binding, then justify it for the reasons above, not
the old one is for the FB driver and the new one is for the DRM
driver.

Rob

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-03-22 18:56 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-28 10:58 [PATCH 0/2] drm/imx/lcdc: drm driver for imx21/25/27 Uwe Kleine-König
2022-01-28 10:58 ` [PATCH 1/2] dt-bindings: display: imx: Add fsl,imx21-lcdc docs Uwe Kleine-König
2022-01-28 13:04   ` Rob Herring
2022-01-28 17:58     ` Uwe Kleine-König
2022-02-01 17:35       ` Rob Herring
2022-02-10 17:54         ` Lucas Stach
2022-02-21 13:55           ` Uwe Kleine-König
2022-03-22 17:43             ` Uwe Kleine-König
2022-03-22 18:56           ` Rob Herring

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).