[PATCH v3 0/2] drm/imx/lcdc: Implement DRM driver for imx21

[PATCH v3 0/2] drm/imx/lcdc: Implement DRM driver for imx21

[Uwe Kleine-König]

Hello,

Changes since v2:

 - added allOf as Krzysztof requested
 - reworked driver based on Philipp's comments
   (improved error handling, different selects, moved driver to a subdirectory,
   header sorting, drm_err instead of DRM_ERROR, inlined
   imx_lcdc_check_mode_change, make use of dev_err_probe())
 
Krzysztof also pointed out that we're now having two compatibles for a
single hardware. Admittedly this is unusual, but this is the chance that
the (bad) compatible identifier imx21-fb gets deprecated. The hardware
is called LCDC and only the linux (framebuffer) driver is called imxfb.

The two prerequisite commits on top of v6.1 are:

 - 93266da2409b ("dt-bindings: display: Convert fsl,imx-fb.txt to
   dt-schema") which is currently in next via branch 'for-next' of
   git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git .

 - "drm/imx: move IPUv3 driver into separate subdirectory"
   from https://lore.kernel.org/r/20221125112519.3849636-1-l.stach@pengutronix.de

Best regards
Uwe

Marian Cichy (1):
  drm/imx/lcdc: Implement DRM driver for imx21

Uwe Kleine-König (1):
  dt-bindings: display: imx: Describe drm binding for fsl,imx-lcdc

 .../bindings/display/imx/fsl,imx-lcdc.yaml    |  46 +-
 drivers/gpu/drm/imx/Kconfig                   |   1 +
 drivers/gpu/drm/imx/Makefile                  |   1 +
 drivers/gpu/drm/imx/lcdc/imx-lcdc.c           | 587 ++++++++++++++++++
 4 files changed, 634 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/imx/lcdc/imx-lcdc.c


base-commit: 830b3c68c1fb1e9176028d02ef86f3cf76aa2476
prerequisite-patch-id: 386dd075d3384181945f8333e887bd00be3b23aa
prerequisite-patch-id: c3ef3de02516b5c159e76b40d2b4348a5ce0fe51
-- 
2.38.1

[PATCH v3 1/2] dt-bindings: display: imx: Describe drm binding for fsl,imx-lcdc

[Uwe Kleine-König]

Modify the existing (fb-like) binding to support the drm-like binding in
parallel.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 .../bindings/display/imx/fsl,imx-lcdc.yaml    | 46 ++++++++++++++++++-
 1 file changed, 45 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/display/imx/fsl,imx-lcdc.yaml b/Documentation/devicetree/bindings/display/imx/fsl,imx-lcdc.yaml
index 35a8fff036ca..c2a063bd5fb3 100644
--- a/Documentation/devicetree/bindings/display/imx/fsl,imx-lcdc.yaml
+++ b/Documentation/devicetree/bindings/display/imx/fsl,imx-lcdc.yaml
@@ -21,6 +21,9 @@ properties:
               - fsl,imx25-fb
               - fsl,imx27-fb
           - const: fsl,imx21-fb
+      - items:
+          - const: fsl,imx25-lcdc
+          - const: fsl,imx21-lcdc
 
   clocks:
     maxItems: 3
@@ -31,6 +34,9 @@ properties:
       - const: ahb
       - const: per
 
+  port:
+    $ref: /schemas/graph.yaml#/properties/port
+
   display:
     $ref: /schemas/types.yaml#/definitions/phandle
 
@@ -59,17 +65,55 @@ properties:
     description:
       LCDC Sharp Configuration Register value.
 
+allOf:
+  if:
+    properties:
+      compatible:
+        contains:
+          enum:
+            - fsl,imx1-lcdc
+            - fsl,imx21-lcdc
+  then:
+    properties:
+      display: false
+      fsl,dmacr: false
+      fsl,lpccr: false
+      fsl,lscr1: false
+
+    required:
+      - port
+
+  else:
+    properties:
+      port: false
+
+    required:
+      - display
+
 required:
   - compatible
   - clocks
   - clock-names
-  - display
   - interrupts
   - reg
 
 additionalProperties: false
 
 examples:
+  - |
+    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>;
+            };
+        };
+    };
   - |
     imxfb: fb@10021000 {
         compatible = "fsl,imx21-fb";
-- 
2.38.1

Re: [PATCH v3 0/2] drm/imx/lcdc: Implement DRM driver for imx21

[Philipp Zabel]

On Fr, 2022-12-16 at 18:50 +0100, Uwe Kleine-König wrote:
> Hello,
> 
> Changes since v2:
> 
>  - added allOf as Krzysztof requested
>  - reworked driver based on Philipp's comments
>    (improved error handling, different selects, moved driver to a subdirectory,
>    header sorting, drm_err instead of DRM_ERROR, inlined
>    imx_lcdc_check_mode_change, make use of dev_err_probe())
>  
> 
> 
> 
> Krzysztof also pointed out that we're now having two compatibles for a
> single hardware. Admittedly this is unusual, but this is the chance that
> the (bad) compatible identifier imx21-fb gets deprecated. The hardware
> is called LCDC and only the linux (framebuffer) driver is called imxfb.
> 
> The two prerequisite commits on top of v6.1 are:
> 
>  - 93266da2409b ("dt-bindings: display: Convert fsl,imx-fb.txt to
>    dt-schema") which is currently in next via branch 'for-next' of
>    git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git .
> 
>  - "drm/imx: move IPUv3 driver into separate subdirectory"
>    from https://lore.kernel.org/r/20221125112519.3849636-1-l.stach@pengutronix.de

This is on drm-misc-next now, so patch 2 applies there.

regards
Philipp

Re: [PATCH v3 0/2] drm/imx/lcdc: Implement DRM driver for imx21

[Rob Herring]

On Fri, Dec 16, 2022 at 06:50:04PM +0100, Uwe Kleine-König wrote:
> Hello,
> 
> Changes since v2:
> 
>  - added allOf as Krzysztof requested
>  - reworked driver based on Philipp's comments
>    (improved error handling, different selects, moved driver to a subdirectory,
>    header sorting, drm_err instead of DRM_ERROR, inlined
>    imx_lcdc_check_mode_change, make use of dev_err_probe())
>  
> Krzysztof also pointed out that we're now having two compatibles for a
> single hardware. Admittedly this is unusual, but this is the chance that
> the (bad) compatible identifier imx21-fb gets deprecated. The hardware
> is called LCDC and only the linux (framebuffer) driver is called imxfb.

The problem is you can't have firmware (with the DTB) that supports 
both. Well, you can if you want to have some firmware setting that 
selects which one. Otherwise, it's really an OS problem to decide what 
to use. 

Rob

Re: [PATCH v3 0/2] drm/imx/lcdc: Implement DRM driver for imx21

[Uwe Kleine-König]

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

On Fri, Dec 16, 2022 at 05:57:58PM -0600, Rob Herring wrote:
> On Fri, Dec 16, 2022 at 06:50:04PM +0100, Uwe Kleine-König wrote:
> > Hello,
> > 
> > Changes since v2:
> > 
> >  - added allOf as Krzysztof requested
> >  - reworked driver based on Philipp's comments
> >    (improved error handling, different selects, moved driver to a subdirectory,
> >    header sorting, drm_err instead of DRM_ERROR, inlined
> >    imx_lcdc_check_mode_change, make use of dev_err_probe())
> >  
> > Krzysztof also pointed out that we're now having two compatibles for a
> > single hardware. Admittedly this is unusual, but this is the chance that
> > the (bad) compatible identifier imx21-fb gets deprecated. The hardware
> > is called LCDC and only the linux (framebuffer) driver is called imxfb.
> 
> The problem is you can't have firmware (with the DTB) that supports 
> both. Well, you can if you want to have some firmware setting that 
> selects which one. Otherwise, it's really an OS problem to decide what 
> to use. 

I don't understand what you intend to say here. The same applies if the
compatible is the same for both binding alternatives, isn't it? Do you
consider a firmware problem better or an OS problem?

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 --]

Re: [PATCH v3 0/2] drm/imx/lcdc: Implement DRM driver for imx21

[Rob Herring]

On Sat, Dec 17, 2022 at 07:38:06PM +0100, Uwe Kleine-König wrote:
> On Fri, Dec 16, 2022 at 05:57:58PM -0600, Rob Herring wrote:
> > On Fri, Dec 16, 2022 at 06:50:04PM +0100, Uwe Kleine-König wrote:
> > > Hello,
> > > 
> > > Changes since v2:
> > > 
> > >  - added allOf as Krzysztof requested
> > >  - reworked driver based on Philipp's comments
> > >    (improved error handling, different selects, moved driver to a subdirectory,
> > >    header sorting, drm_err instead of DRM_ERROR, inlined
> > >    imx_lcdc_check_mode_change, make use of dev_err_probe())
> > >  
> > > Krzysztof also pointed out that we're now having two compatibles for a
> > > single hardware. Admittedly this is unusual, but this is the chance that
> > > the (bad) compatible identifier imx21-fb gets deprecated. The hardware
> > > is called LCDC and only the linux (framebuffer) driver is called imxfb.
> > 
> > The problem is you can't have firmware (with the DTB) that supports 
> > both. Well, you can if you want to have some firmware setting that 
> > selects which one. Otherwise, it's really an OS problem to decide what 
> > to use. 
> 
> I don't understand what you intend to say here. The same applies if the
> compatible is the same for both binding alternatives, isn't it? 

Only if you have both nodes in the DT and both enabled. But 2 enabled 
nodes at the same address is also a dtc warning, so I was assuming you 
didn't do that.

> Do you consider a firmware problem better or an OS problem?

The OS created the problem, so they get to keep it. But a PC BIOS is 
full of OS compatibility switches, so...

In the end, it's the platforms' decision really. I just want what the 
implications of having 2 compatibles are to be understood.

Rob

Re: [PATCH v3 0/2] drm/imx/lcdc: Implement DRM driver for imx21

[Uwe Kleine-König]

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

Hello Rob,

On Tue, Dec 20, 2022 at 12:19:48PM -0600, Rob Herring wrote:
> On Sat, Dec 17, 2022 at 07:38:06PM +0100, Uwe Kleine-König wrote:
> > On Fri, Dec 16, 2022 at 05:57:58PM -0600, Rob Herring wrote:
> > > On Fri, Dec 16, 2022 at 06:50:04PM +0100, Uwe Kleine-König wrote:
> > > > Hello,
> > > > 
> > > > Changes since v2:
> > > > 
> > > >  - added allOf as Krzysztof requested
> > > >  - reworked driver based on Philipp's comments
> > > >    (improved error handling, different selects, moved driver to a subdirectory,
> > > >    header sorting, drm_err instead of DRM_ERROR, inlined
> > > >    imx_lcdc_check_mode_change, make use of dev_err_probe())
> > > >  
> > > > Krzysztof also pointed out that we're now having two compatibles for a
> > > > single hardware. Admittedly this is unusual, but this is the chance that
> > > > the (bad) compatible identifier imx21-fb gets deprecated. The hardware
> > > > is called LCDC and only the linux (framebuffer) driver is called imxfb.
> > > 
> > > The problem is you can't have firmware (with the DTB) that supports 
> > > both. Well, you can if you want to have some firmware setting that 
> > > selects which one. Otherwise, it's really an OS problem to decide what 
> > > to use. 
> > 
> > I don't understand what you intend to say here. The same applies if the
> > compatible is the same for both binding alternatives, isn't it? 
> 
> Only if you have both nodes in the DT and both enabled. But 2 enabled 
> nodes at the same address is also a dtc warning, so I was assuming you 
> didn't do that.

My idea was to use the new compatible in the soc.dtsi, and the old one
in the soc-machine.dts until they are converted. And then eventually
drop the old driver.

> > Do you consider a firmware problem better or an OS problem?
> 
> The OS created the problem, so they get to keep it. But a PC BIOS is 
> full of OS compatibility switches, so...
> 
> In the end, it's the platforms' decision really. I just want what the 
> implications of having 2 compatibles are to be understood.

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 --]