diff mbox series

[v4,4/9] media: Documentation: dw100: Add user documentation for the DW100 driver

Message ID 20220328141309.177611-5-xavier.roumegue@oss.nxp.com
State New
Headers show
Series [v4,1/9] videodev2.h: add V4L2_CTRL_FLAG_DYNAMIC_ARRAY | expand

Commit Message

Xavier Roumegue (OSS) March 28, 2022, 2:13 p.m. UTC 
Add user documentation for the DW100 driver.

Signed-off-by: Xavier Roumegue <xavier.roumegue@oss.nxp.com>
---
 .../userspace-api/media/drivers/dw100.rst     | 23 +++++++++++++++++++
 .../userspace-api/media/drivers/index.rst     |  1 +
 2 files changed, 24 insertions(+)
 create mode 100644 Documentation/userspace-api/media/drivers/dw100.rst

Comments

Hans Verkuil April 25, 2022, 6:18 a.m. UTC | #1 
On 28/03/2022 16:13, Xavier Roumegue wrote:
> Add user documentation for the DW100 driver.
> 
> Signed-off-by: Xavier Roumegue <xavier.roumegue@oss.nxp.com>
> ---
>  .../userspace-api/media/drivers/dw100.rst     | 23 +++++++++++++++++++
>  .../userspace-api/media/drivers/index.rst     |  1 +
>  2 files changed, 24 insertions(+)
>  create mode 100644 Documentation/userspace-api/media/drivers/dw100.rst
> 
> diff --git a/Documentation/userspace-api/media/drivers/dw100.rst b/Documentation/userspace-api/media/drivers/dw100.rst
> new file mode 100644
> index 000000000000..4cd55c75628e
> --- /dev/null
> +++ b/Documentation/userspace-api/media/drivers/dw100.rst
> @@ -0,0 +1,23 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +DW100 dewarp driver
> +===========================
> +
> +The Vivante DW100 Dewarp Processor IP core found on i.MX8MP SoC applies a
> +programmable geometrical transformation on input image to correct distorsion

distorsion -> distortion

> +introduced by lenses.
> +
> +The transformation function is exposed by the hardware as a grid map with 16x16
> +pixel macroblocks indexed using X, Y vertex coordinates. Each x, y coordinate
> +register uses 16 bits to record the coordinate address in UQ12.4 fixed point
> +format.
> +
> +The dewarping map can be set from application through a dedicated v4l2 control.
> +If not set or invalid, the driver computes an identity map prior to start the
> +processing engine. The map is evaluated as invalid if the array size does not
> +match the expected size inherited from the destination image resolution.
> +
> +More details on the DW100 hardware operations can be found in
> +*chapter 13.15 DeWarp* of IMX8MP_ reference manuel.

manuel -> manual

> +
> +.. _IMX8MP: https://www.nxp.com/webapp/Download?colCode=IMX8MPRM
> diff --git a/Documentation/userspace-api/media/drivers/index.rst b/Documentation/userspace-api/media/drivers/index.rst
> index 12e3c512d718..8826777321b0 100644
> --- a/Documentation/userspace-api/media/drivers/index.rst
> +++ b/Documentation/userspace-api/media/drivers/index.rst
> @@ -33,6 +33,7 @@ For more details see the file COPYING in the source distribution of Linux.
>  
>  	ccs
>  	cx2341x-uapi
> +	dw100
>          hantro
>  	imx-uapi
>  	max2175

Regards,

	Hans
Hans Verkuil April 25, 2022, 6:41 a.m. UTC | #2 
On 28/03/2022 16:13, Xavier Roumegue wrote:
> Add user documentation for the DW100 driver.
> 
> Signed-off-by: Xavier Roumegue <xavier.roumegue@oss.nxp.com>
> ---
>  .../userspace-api/media/drivers/dw100.rst     | 23 +++++++++++++++++++
>  .../userspace-api/media/drivers/index.rst     |  1 +
>  2 files changed, 24 insertions(+)
>  create mode 100644 Documentation/userspace-api/media/drivers/dw100.rst
> 
> diff --git a/Documentation/userspace-api/media/drivers/dw100.rst b/Documentation/userspace-api/media/drivers/dw100.rst
> new file mode 100644
> index 000000000000..4cd55c75628e
> --- /dev/null
> +++ b/Documentation/userspace-api/media/drivers/dw100.rst
> @@ -0,0 +1,23 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +DW100 dewarp driver
> +===========================
> +
> +The Vivante DW100 Dewarp Processor IP core found on i.MX8MP SoC applies a
> +programmable geometrical transformation on input image to correct distorsion
> +introduced by lenses.
> +
> +The transformation function is exposed by the hardware as a grid map with 16x16
> +pixel macroblocks indexed using X, Y vertex coordinates. Each x, y coordinate
> +register uses 16 bits to record the coordinate address in UQ12.4 fixed point

'UQ12.4' is a mostly TI notation it seems: https://en.wikipedia.org/wiki/Q_(number_format)

I would replace this with: ...in an unsigned 12.4 fixed point format

And in the control documentation this can be documented more precisely (I'll comment on
that patch).

Regards,

	Hans

> +format.
> +
> +The dewarping map can be set from application through a dedicated v4l2 control.
> +If not set or invalid, the driver computes an identity map prior to start the
> +processing engine. The map is evaluated as invalid if the array size does not
> +match the expected size inherited from the destination image resolution.
> +
> +More details on the DW100 hardware operations can be found in
> +*chapter 13.15 DeWarp* of IMX8MP_ reference manuel.
> +
> +.. _IMX8MP: https://www.nxp.com/webapp/Download?colCode=IMX8MPRM
> diff --git a/Documentation/userspace-api/media/drivers/index.rst b/Documentation/userspace-api/media/drivers/index.rst
> index 12e3c512d718..8826777321b0 100644
> --- a/Documentation/userspace-api/media/drivers/index.rst
> +++ b/Documentation/userspace-api/media/drivers/index.rst
> @@ -33,6 +33,7 @@ For more details see the file COPYING in the source distribution of Linux.
>  
>  	ccs
>  	cx2341x-uapi
> +	dw100
>          hantro
>  	imx-uapi
>  	max2175
Laurent Pinchart April 26, 2022, 9:26 p.m. UTC | #3 
On Mon, Apr 25, 2022 at 08:18:21AM +0200, Hans Verkuil wrote:
> On 28/03/2022 16:13, Xavier Roumegue wrote:
> > Add user documentation for the DW100 driver.
> > 
> > Signed-off-by: Xavier Roumegue <xavier.roumegue@oss.nxp.com>
> > ---
> >  .../userspace-api/media/drivers/dw100.rst     | 23 +++++++++++++++++++
> >  .../userspace-api/media/drivers/index.rst     |  1 +
> >  2 files changed, 24 insertions(+)
> >  create mode 100644 Documentation/userspace-api/media/drivers/dw100.rst
> > 
> > diff --git a/Documentation/userspace-api/media/drivers/dw100.rst b/Documentation/userspace-api/media/drivers/dw100.rst
> > new file mode 100644
> > index 000000000000..4cd55c75628e
> > --- /dev/null
> > +++ b/Documentation/userspace-api/media/drivers/dw100.rst
> > @@ -0,0 +1,23 @@
> > +.. SPDX-License-Identifier: GPL-2.0
> > +
> > +DW100 dewarp driver
> > +===========================

Looks like the underline can be shorten.

> > +
> > +The Vivante DW100 Dewarp Processor IP core found on i.MX8MP SoC applies a
> > +programmable geometrical transformation on input image to correct distorsion
> 
> distorsion -> distortion
> 
> > +introduced by lenses.
> > +
> > +The transformation function is exposed by the hardware as a grid map with 16x16
> > +pixel macroblocks indexed using X, Y vertex coordinates. Each x, y coordinate
> > +register uses 16 bits to record the coordinate address in UQ12.4 fixed point
> > +format.
> > +
> > +The dewarping map can be set from application through a dedicated v4l2 control.

s/v4l2/V4L2/

I'd mention the control ID.

> > +If not set or invalid, the driver computes an identity map prior to start the
> > +processing engine. The map is evaluated as invalid if the array size does not
> > +match the expected size inherited from the destination image resolution.

I'd swap the two sentences and clarify this a bit:

----
The dewarping map is set from applications using the
V4L2_CID_DW100_DEWARPING_16x16_VERTEX_MAP control. The control contains
an array of u32 values storing (x, y) destination coordinates for each
vertex of the grid. The x coordinate is stored in the 16 LSBs and the y
coordinate in the 16 MSBs. The number of elements in the array must
match the image size:

    elems = (DIV_ROUND_UP(width, 16) + 1)
          * (DIV_ROUND_UP(height, 16) + 1);

If the control doesn't contain the correct number of elements, the
driver uses an identity map.
----

This assumes I got the size calculation right :-)

Bonus points if you can format the code block correctly for sphinx.

I'm also wondering if the driver shouldn't return an error when the map
is incorrect, defaulting silently to an identity map can be confusing.

> > +
> > +More details on the DW100 hardware operations can be found in
> > +*chapter 13.15 DeWarp* of IMX8MP_ reference manuel.
> 
> manuel -> manual
> 
> > +
> > +.. _IMX8MP: https://www.nxp.com/webapp/Download?colCode=IMX8MPRM
> > diff --git a/Documentation/userspace-api/media/drivers/index.rst b/Documentation/userspace-api/media/drivers/index.rst
> > index 12e3c512d718..8826777321b0 100644
> > --- a/Documentation/userspace-api/media/drivers/index.rst
> > +++ b/Documentation/userspace-api/media/drivers/index.rst
> > @@ -33,6 +33,7 @@ For more details see the file COPYING in the source distribution of Linux.
> >  
> >  	ccs
> >  	cx2341x-uapi
> > +	dw100
> >          hantro

While at it you could replace the spaces with a tab here (and mention
that in the commit message, with a "While at it, ..." line).

> >  	imx-uapi
> >  	max2175
diff mbox series

Patch

diff --git a/Documentation/userspace-api/media/drivers/dw100.rst b/Documentation/userspace-api/media/drivers/dw100.rst
new file mode 100644
index 000000000000..4cd55c75628e
--- /dev/null
+++ b/Documentation/userspace-api/media/drivers/dw100.rst
@@ -0,0 +1,23 @@ 
+.. SPDX-License-Identifier: GPL-2.0
+
+DW100 dewarp driver
+===========================
+
+The Vivante DW100 Dewarp Processor IP core found on i.MX8MP SoC applies a
+programmable geometrical transformation on input image to correct distorsion
+introduced by lenses.
+
+The transformation function is exposed by the hardware as a grid map with 16x16
+pixel macroblocks indexed using X, Y vertex coordinates. Each x, y coordinate
+register uses 16 bits to record the coordinate address in UQ12.4 fixed point
+format.
+
+The dewarping map can be set from application through a dedicated v4l2 control.
+If not set or invalid, the driver computes an identity map prior to start the
+processing engine. The map is evaluated as invalid if the array size does not
+match the expected size inherited from the destination image resolution.
+
+More details on the DW100 hardware operations can be found in
+*chapter 13.15 DeWarp* of IMX8MP_ reference manuel.
+
+.. _IMX8MP: https://www.nxp.com/webapp/Download?colCode=IMX8MPRM
diff --git a/Documentation/userspace-api/media/drivers/index.rst b/Documentation/userspace-api/media/drivers/index.rst
index 12e3c512d718..8826777321b0 100644
--- a/Documentation/userspace-api/media/drivers/index.rst
+++ b/Documentation/userspace-api/media/drivers/index.rst
@@ -33,6 +33,7 @@  For more details see the file COPYING in the source distribution of Linux.
 
 	ccs
 	cx2341x-uapi
+	dw100
         hantro
 	imx-uapi
 	max2175