[RFC PATCH 0/3] media: imx: imx7-media-csi: i.MX8MM support

[RFC PATCH 0/3] media: imx: imx7-media-csi: i.MX8MM support

[Laurent Pinchart]

Hello,

This small patch series updates the imx7-media-csi driver to work on the
i.MX8MM with an OV5640 sensor.

Patch 1/3 extends the nxp,imx7-csi DT bindings with a compatible string
for the i.MX8MM. While the CSI bridge in that SoC doesn't seem to differ
from the one in the i.MX7 according to the reference manual, experience
shows that NXP reference manuals are not always reliable. To be on the
safe side, a new fsl,imx8mm-csi compatible string, with a fallback on
fsl,imx7-csi, will avoid future backward-compatibility problems.

Patches 2/3 and 3/3 fix issues with RAW8 and RAW10 capture from an
OV5640 sensor. The fixes are the result of experimentation and study of
NXP BSP drivers, as the reference manual doesn't provide much
information in this area. I'm not very happy with this, as understanding
the exact effect of the register fields modified by those two patches
would be better. Still, without support from NXP (which I would really,
really appreciate - anyone from NXP reading this ?), I can't do better.

Given those concerns, I would also appreciate if this series could be
tested widely for possible regressions. There should be no change for
YUV formats, so only raw formats (RAW8, RAW10, RAW12 and RAW14) need to
be tested.

Laurent Pinchart (3):
  dt-bindings: media: nxp,imx7-csi: Add i.MX8MM support
  media: imx: imx7-media-csi: Set TWO_8BIT_SENSOR for >= 10-bit formats
  media: imx: imx7-media-csi: Don't set PIXEL_BIT in CSICR1

 .../bindings/media/nxp,imx7-csi.yaml          | 12 +++++++----
 drivers/staging/media/imx/imx7-media-csi.c    | 21 +++++--------------
 2 files changed, 13 insertions(+), 20 deletions(-)

-- 
Regards,

Laurent Pinchart

[RFC PATCH 1/3] dt-bindings: media: nxp,imx7-csi: Add i.MX8MM support

[Laurent Pinchart]

The i.MX8MM integrates a CSI bridge IP core, as the i.MX7. There seems
to be no difference between the two SoCs according to the reference
manual, but as documentation may not be accurate, add a compatible
string for the i.MX8MM, with a fallback on the compatible i.MX7.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 .../devicetree/bindings/media/nxp,imx7-csi.yaml      | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml b/Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
index d91575b8ebb9..5922a2795167 100644
--- a/Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
+++ b/Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
@@ -4,7 +4,7 @@
 $id: http://devicetree.org/schemas/media/nxp,imx7-csi.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: i.MX7 CMOS Sensor Interface
+title: i.MX7 and i.MX8 CSI bridge (CMOS Sensor Interface)
 
 maintainers:
   - Rui Miguel Silva <rmfrfs@gmail.com>
@@ -15,9 +15,13 @@ description: |
 
 properties:
   compatible:
-    enum:
-      - fsl,imx7-csi
-      - fsl,imx6ul-csi
+    oneOf:
+      - enum:
+          - fsl,imx7-csi
+          - fsl,imx6ul-csi
+      - items:
+          - const: fsl,imx8mm-csi
+          - const: fsl,imx7-csi
 
   reg:
     maxItems: 1
-- 
Regards,

Laurent Pinchart

Re: [RFC PATCH 1/3] dt-bindings: media: nxp,imx7-csi: Add i.MX8MM support

[Martin Kepplinger]

Am Sonntag, dem 16.05.2021 um 05:42 +0300 schrieb Laurent Pinchart:
> The i.MX8MM integrates a CSI bridge IP core, as the i.MX7. There
> seems
> to be no difference between the two SoCs according to the reference
> manual, but as documentation may not be accurate, add a compatible
> string for the i.MX8MM, with a fallback on the compatible i.MX7.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  .../devicetree/bindings/media/nxp,imx7-csi.yaml      | 12 ++++++++--
> --
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/nxp,imx7-
> csi.yaml b/Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
> index d91575b8ebb9..5922a2795167 100644
> --- a/Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
> +++ b/Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
> @@ -4,7 +4,7 @@
>  $id: http://devicetree.org/schemas/media/nxp,imx7-csi.yaml#
>  $schema: http://devicetree.org/meta-schemas/core.yaml#
>  
> -title: i.MX7 CMOS Sensor Interface
> +title: i.MX7 and i.MX8 CSI bridge (CMOS Sensor Interface)
>  
>  maintainers:
>    - Rui Miguel Silva <rmfrfs@gmail.com>
> @@ -15,9 +15,13 @@ description: |
>  
>  properties:
>    compatible:
> -    enum:
> -      - fsl,imx7-csi
> -      - fsl,imx6ul-csi
> +    oneOf:
> +      - enum:
> +          - fsl,imx7-csi
> +          - fsl,imx6ul-csi
> +      - items:
> +          - const: fsl,imx8mm-csi
> +          - const: fsl,imx7-csi
>  
>    reg:
>      maxItems: 1

isn't the fsl,imx8mm-csi compatible missing in the driver then?

thanks!
                     martin

Re: [RFC PATCH 1/3] dt-bindings: media: nxp,imx7-csi: Add i.MX8MM support

[Laurent Pinchart]

Hi Martin,

On Tue, May 18, 2021 at 01:26:21PM +0200, Martin Kepplinger wrote:
> Am Sonntag, dem 16.05.2021 um 05:42 +0300 schrieb Laurent Pinchart:
> > The i.MX8MM integrates a CSI bridge IP core, as the i.MX7. There seems
> > to be no difference between the two SoCs according to the reference
> > manual, but as documentation may not be accurate, add a compatible
> > string for the i.MX8MM, with a fallback on the compatible i.MX7.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  .../devicetree/bindings/media/nxp,imx7-csi.yaml      | 12 ++++++++--
> > --
> >  1 file changed, 8 insertions(+), 4 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/media/nxp,imx7-
> > csi.yaml b/Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
> > index d91575b8ebb9..5922a2795167 100644
> > --- a/Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
> > +++ b/Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
> > @@ -4,7 +4,7 @@
> >  $id: http://devicetree.org/schemas/media/nxp,imx7-csi.yaml#
> >  $schema: http://devicetree.org/meta-schemas/core.yaml#
> >  
> > -title: i.MX7 CMOS Sensor Interface
> > +title: i.MX7 and i.MX8 CSI bridge (CMOS Sensor Interface)
> >  
> >  maintainers:
> >    - Rui Miguel Silva <rmfrfs@gmail.com>
> > @@ -15,9 +15,13 @@ description: |
> >  
> >  properties:
> >    compatible:
> > -    enum:
> > -      - fsl,imx7-csi
> > -      - fsl,imx6ul-csi
> > +    oneOf:
> > +      - enum:
> > +          - fsl,imx7-csi
> > +          - fsl,imx6ul-csi
> > +      - items:
> > +          - const: fsl,imx8mm-csi
> > +          - const: fsl,imx7-csi
> >  
> >    reg:
> >      maxItems: 1
> 
> isn't the fsl,imx8mm-csi compatible missing in the driver then?

The driver will match on the "fsl,imx7-csi" compatible string, which the
bindings makes mandatory as a fallback for i.MX8MM. If we later find
differences between the CSI bridge in the i.MX7 and i.MX8MM, we can add
the compatible string to the driver to enable device-specific code
without any backward-compatibility issue.

-- 
Regards,

Laurent Pinchart

Re: [RFC PATCH 1/3] dt-bindings: media: nxp,imx7-csi: Add i.MX8MM support

[Rob Herring]

On Sun, 16 May 2021 05:42:14 +0300, Laurent Pinchart wrote:
> The i.MX8MM integrates a CSI bridge IP core, as the i.MX7. There seems
> to be no difference between the two SoCs according to the reference
> manual, but as documentation may not be accurate, add a compatible
> string for the i.MX8MM, with a fallback on the compatible i.MX7.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  .../devicetree/bindings/media/nxp,imx7-csi.yaml      | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [RFC PATCH 0/3] media: imx: imx7-media-csi: i.MX8MM support

[Rui Miguel Silva]

Hi Laurent,
Thanks for extending support for imx8.

On Sun May 16, 2021 at 3:42 AM WEST, Laurent Pinchart wrote:
> Hello,
>
> This small patch series updates the imx7-media-csi driver to work on the
> i.MX8MM with an OV5640 sensor.
>
> Patch 1/3 extends the nxp,imx7-csi DT bindings with a compatible string
> for the i.MX8MM. While the CSI bridge in that SoC doesn't seem to differ
> from the one in the i.MX7 according to the reference manual, experience
> shows that NXP reference manuals are not always reliable. To be on the
> safe side, a new fsl,imx8mm-csi compatible string, with a fallback on
> fsl,imx7-csi, will avoid future backward-compatibility problems.
>
> Patches 2/3 and 3/3 fix issues with RAW8 and RAW10 capture from an
> OV5640 sensor. The fixes are the result of experimentation and study of
> NXP BSP drivers, as the reference manual doesn't provide much
> information in this area. I'm not very happy with this, as understanding
> the exact effect of the register fields modified by those two patches
> would be better. Still, without support from NXP (which I would really,
> really appreciate - anyone from NXP reading this ?), I can't do better.
>
> Given those concerns, I would also appreciate if this series could be
> tested widely for possible regressions. There should be no change for
> YUV formats, so only raw formats (RAW8, RAW10, RAW12 and RAW14) need to
> be tested.

I've tested with my setup in imx7 which is RAW10 only, and everything
looks fine.

I only have a small suggestion in 2/3.

------
Cheers,
     Rui

>
> Laurent Pinchart (3):
>   dt-bindings: media: nxp,imx7-csi: Add i.MX8MM support
>   media: imx: imx7-media-csi: Set TWO_8BIT_SENSOR for >= 10-bit formats
>   media: imx: imx7-media-csi: Don't set PIXEL_BIT in CSICR1
>
>  .../bindings/media/nxp,imx7-csi.yaml          | 12 +++++++----
>  drivers/staging/media/imx/imx7-media-csi.c    | 21 +++++--------------
>  2 files changed, 13 insertions(+), 20 deletions(-)
>
> -- 
> Regards,
>
> Laurent Pinchart