Re: [PATCH v2 6/9] media: uapi: Add a control for DW100 driver

Le mardi 08 mars 2022 à 20:42 +0100, Xavier Roumegue (OSS) a écrit :

Hello Nicolas,

On 3/8/22 20:15, Nicolas Dufresne wrote:

quoted

Le mardi 08 mars 2022 à 19:48 +0100, Xavier Roumegue a écrit :

quoted

The DW100 driver gets the dewarping mapping as a binary blob from the
userspace application through a custom control.
The blob format is hardware specific so create a dedicated control for
this purpose.

Signed-off-by: Xavier Roumegue <xavier.roumegue@oss.nxp.com>
---
  Documentation/userspace-api/media/drivers/dw100.rst |  7 +++++++
  include/uapi/linux/dw100.h                          | 11 +++++++++++
  2 files changed, 18 insertions(+)
  create mode 100644 include/uapi/linux/dw100.h

diff --git a/Documentation/userspace-api/media/drivers/dw100.rst b/Documentation/userspace-api/media/drivers/dw100.rst
index 20aeae63a94f..3abad05849ad 100644
--- a/Documentation/userspace-api/media/drivers/dw100.rst
+++ b/Documentation/userspace-api/media/drivers/dw100.rst

@@ -20,4 +20,11 @@ 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.
  
+The Vivante DW100 m2m driver implements the following driver-specific control:
+
+``V4L2_CID_DW100_MAPPING (integer)``
+    Specifies to DW100 driver its dewarping map (aka LUT) blob as described in
+    *chapter 13.15.2.3 Dewarping Remap* of IMX8MP_ reference manual as an U32
+    dynamic array.
+
  .. _IMX8MP: https://www.nxp.com/webapp/Download?colCode=IMX8MPIEC

This point to a document names "i.MX 8M Plus Applications Processor Datasheet
for Industrial Products" which does not contain that reference.

My bad.. Wrong link. :)
Will repost with correct link.

Thanks. What I wanted to check is if it actually made sense to expose the
synthetized HW LUT. But for this, one need to share the parameters / algo needed
to generate them. This way we can compare against other popular dewarp
algorithms / API and see if they have something in common.

The issue I see with this control is relate to the message it gives. When adding
controls for the prosperity, we want these control to actually be usable. This
is possible if the documentation makes its usage obvious, or if there is Open
Source userland to support that.

None of this is met, so as a side effect, this looks like NXP sneaking in
private blob control into a publicly maintained Open Source project. This isn't
truly aligned with how V4L2 controls are meant to be. Doing trivial lut
synthesis in the kernel could be fine though.

quoted

quoted

diff --git a/include/uapi/linux/dw100.h b/include/uapi/linux/dw100.h
new file mode 100644
index 000000000000..0ef926c61cf0
--- /dev/null
+++ b/include/uapi/linux/dw100.h

@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0-only WITH Linux-syscall-note */
+/* Copyright 2022 NXP */
+
+#ifndef __UAPI_DW100_H__
+#define __UAPI_DW100_H__
+
+#include <linux/v4l2-controls.h>
+
+#define V4L2_CID_DW100_MAPPING		(V4L2_CID_USER_DW100_BASE + 1)
+
+#endif