mbox series

[0/2] drm/imx/lcdc: drm driver for imx21/25/27

Message ID 20220128105849.368438-1-u.kleine-koenig@pengutronix.de
Headers show
Series drm/imx/lcdc: drm driver for imx21/25/27 | expand

Message

Uwe Kleine-König Jan. 28, 2022, 10:58 a.m. UTC
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

Comments

Rob Herring Jan. 28, 2022, 1:04 p.m. UTC | #1
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
>
Uwe Kleine-König Jan. 28, 2022, 5:58 p.m. UTC | #2
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
Lucas Stach Feb. 10, 2022, 5:54 p.m. UTC | #3
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