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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 3/8/22 21:28, Nicolas Dufresne wrote:
Le mardi 08 mars 2022 à 20:42 +0100, Xavier Roumegue (OSS) a écrit :
Hello Nicolas,

On 3/8/22 20:15, Nicolas Dufresne wrote:
Le mardi 08 mars 2022 à 19:48 +0100, Xavier Roumegue a écrit :
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@xxxxxxxxxxx>
---
   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.
There is no special dewarping algorithm which strictly depends on the dw100 IP, or optimized for the IP capabilities.

 This way we can compare against other popular dewarp
algorithms / API and see if they have something in common.
The dw100 hw lut description is rather close to a how you implement dewarping with openGL taking benefit of the shader pipeline stage.
The main differences with OpenGL implementation are:
- Fixed vertices coordinates (16x16) vs any
- Limited resolution on input (texture) coordinates (UQ12.4) vs float

Standard routines from OpenCV such as initUndistortRectifyMap()
https://docs.opencv.org/4.5.5/d9/d0c/group__calib3d.html#ga7dfb72c9cf9780a347fbe3d1c47e5d5a
can be used to generate the binary blob, with an additional decimation processing stage to satisfy the 16x16 macro block vertices grid and the fixed point format.


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.
So yes, most famous vision opensource project such OpenCV can be used to generate the blob.

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.
I then disagree with this statement considering my previous comments.

I plan to release publicly some programming examples on how to generate the dewarping map only using openCV library routines and aligned with lenses calibration state of the art method. A dedicated openCV module taking benefit of the DW100 will be published as well.

A long term target is to add its support in libcamera, combined with all media components (CSI, ISP, ISI) pulled from upstream kernel tree.

 This isn't
truly aligned with how V4L2 controls are meant to be. Doing trivial lut
synthesis in the kernel could be fine though.
I am not sure what you meant with this comment.

As part of this patch series, an identity map is generated in the driver which should be enough for anyone familiar with dewarping process. If you meant to generate the remapping table from the lens calibration data, I don't think this is a reasonable option considering the NP-completeness of the problem.

If this is the idea of binary blob (despite its public format description) which hurts you, the map can be exposed to the kernel in a more human readable format such Image_in(xin, yin) -> Image_out(xout, yout) in UQ1.31 format but will add extra processing at runtime for something which has to be done anyway offline, and memory overhead. But I don't think we can end with a generic v4l2 control considering the hardware restrictions (vertices position, limited fixed point resolution, etc..).

Adding a generic dewarping API to V4L2 is possible but this was not the scope of this patchset, and anyway missing data on any existing public dewarp hardware implementation supported by the kernel is somehow a blocker for this.



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





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux