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]

 



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

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