[v4,6/9] media: uapi: Add a control for DW100 driver

  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 | 12 ++++++++++++
 include/uapi/linux/dw100.h                          | 11 +++++++++++
 2 files changed, 23 insertions(+)
 create mode 100644 include/uapi/linux/dw100.h
  

On 28/03/2022 16:13, Xavier Roumegue wrote:
> 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 | 12 ++++++++++++
>  include/uapi/linux/dw100.h                          | 11 +++++++++++
>  2 files changed, 23 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 4cd55c75628e..f6d684cadf26 100644
> --- a/Documentation/userspace-api/media/drivers/dw100.rst
> +++ b/Documentation/userspace-api/media/drivers/dw100.rst
> @@ -20,4 +20,16 @@ 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_DEWARPING_16x16_VERTEX_MAP (integer)``

(integer) -> (__u32 array)

But should this be a __u32 array at all? Wouldn't a __u16 array make more sense?

> +    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. The image is divided into many small 16x16 blocks. If the
> +    width of the image is not divisible by 16, the size of the rightmost block
> +    is the remainder. 

Isn't the same true for the height?

The dewarping map only saves the vertex coordinates of the
> +    block. The dewarping grid map is comprised of vertex coordinates for x and y.
> +    Each x, y coordinate register uses 16 bits (UQ12.4) to record the coordinate

As mentioned before, UQ12.4 is not necessarily a standard notation. 'unsigned 12.4
fixed point' is better, but you also need to specify exactly where the bits are
stored inside the __u16. I.e.: 'the integer part is stored in the 12 most significant
bits, and the fractional part is stored in the 4 least significant bits of the __u16.'

> +    address, with the Y coordinate in the upper bits and X in the lower bits.

And with a __u16 array this becomes: 'The array contains pairs of X, Y coordinates.'
Or something along those lines.

> +
>  .. _IMX8MP: https://www.nxp.com/webapp/Download?colCode=IMX8MPRM
> diff --git a/include/uapi/linux/dw100.h b/include/uapi/linux/dw100.h
> new file mode 100644
> index 000000000000..7fdcf2bf42e5
> --- /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>
> +

Add a comment referring to the Documentation/userspace-api/media/drivers/dw100.rst
documentation so users of this control know where to find the associated
documentation.

> +#define V4L2_CID_DW100_DEWARPING_16x16_VERTEX_MAP (V4L2_CID_USER_DW100_BASE + 1)
> +
> +#endif

Regards,

	Hans
  

On Mon, Apr 25, 2022 at 08:57:07AM +0200, Hans Verkuil wrote:
> On 28/03/2022 16:13, Xavier Roumegue wrote:
> > 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 | 12 ++++++++++++
> >  include/uapi/linux/dw100.h                          | 11 +++++++++++
> >  2 files changed, 23 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 4cd55c75628e..f6d684cadf26 100644
> > --- a/Documentation/userspace-api/media/drivers/dw100.rst
> > +++ b/Documentation/userspace-api/media/drivers/dw100.rst
> > @@ -20,4 +20,16 @@ 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_DEWARPING_16x16_VERTEX_MAP (integer)``
> 
> (integer) -> (__u32 array)
> 
> But should this be a __u32 array at all? Wouldn't a __u16 array make more sense?
> 
> > +    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. The image is divided into many small 16x16 blocks. If the
> > +    width of the image is not divisible by 16, the size of the rightmost block
> > +    is the remainder. 
> 
> Isn't the same true for the height?
> 
> The dewarping map only saves the vertex coordinates of the
> > +    block. The dewarping grid map is comprised of vertex coordinates for x and y.
> > +    Each x, y coordinate register uses 16 bits (UQ12.4) to record the coordinate
> 
> As mentioned before, UQ12.4 is not necessarily a standard notation. 'unsigned 12.4
> fixed point' is better, but you also need to specify exactly where the bits are
> stored inside the __u16. I.e.: 'the integer part is stored in the 12 most significant
> bits, and the fractional part is stored in the 4 least significant bits of the __u16.'

Isn't that implied ? I've never seen fixed-point numbers stored the
other way around.

Regarding the Q notation, while it was coined by TI, I think it's
widespread enough to be used here. I don't mind much though.

> > +    address, with the Y coordinate in the upper bits and X in the lower bits.
> 
> And with a __u16 array this becomes: 'The array contains pairs of X, Y coordinates.'
> Or something along those lines.
> 
> > +
> >  .. _IMX8MP: https://www.nxp.com/webapp/Download?colCode=IMX8MPRM
> > diff --git a/include/uapi/linux/dw100.h b/include/uapi/linux/dw100.h
> > new file mode 100644
> > index 000000000000..7fdcf2bf42e5
> > --- /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>
> > +
> 
> Add a comment referring to the Documentation/userspace-api/media/drivers/dw100.rst
> documentation so users of this control know where to find the associated
> documentation.
> 
> > +#define V4L2_CID_DW100_DEWARPING_16x16_VERTEX_MAP (V4L2_CID_USER_DW100_BASE + 1)
> > +
> > +#endif
  

On 25/04/2022 09:11, Laurent Pinchart wrote:
> On Mon, Apr 25, 2022 at 08:57:07AM +0200, Hans Verkuil wrote:
>> On 28/03/2022 16:13, Xavier Roumegue wrote:
>>> 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 | 12 ++++++++++++
>>>  include/uapi/linux/dw100.h                          | 11 +++++++++++
>>>  2 files changed, 23 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 4cd55c75628e..f6d684cadf26 100644
>>> --- a/Documentation/userspace-api/media/drivers/dw100.rst
>>> +++ b/Documentation/userspace-api/media/drivers/dw100.rst
>>> @@ -20,4 +20,16 @@ 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_DEWARPING_16x16_VERTEX_MAP (integer)``
>>
>> (integer) -> (__u32 array)
>>
>> But should this be a __u32 array at all? Wouldn't a __u16 array make more sense?
>>
>>> +    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. The image is divided into many small 16x16 blocks. If the
>>> +    width of the image is not divisible by 16, the size of the rightmost block
>>> +    is the remainder. 
>>
>> Isn't the same true for the height?
>>
>> The dewarping map only saves the vertex coordinates of the
>>> +    block. The dewarping grid map is comprised of vertex coordinates for x and y.
>>> +    Each x, y coordinate register uses 16 bits (UQ12.4) to record the coordinate
>>
>> As mentioned before, UQ12.4 is not necessarily a standard notation. 'unsigned 12.4
>> fixed point' is better, but you also need to specify exactly where the bits are
>> stored inside the __u16. I.e.: 'the integer part is stored in the 12 most significant
>> bits, and the fractional part is stored in the 4 least significant bits of the __u16.'
> 
> Isn't that implied ? I've never seen fixed-point numbers stored the
> other way around.

True, perhaps that's overkill.

> 
> Regarding the Q notation, while it was coined by TI, I think it's
> widespread enough to be used here. I don't mind much though.

I had to look it up :-)

That might say more about me, though...

I think the key phrase that is missing here is "fixed point".

Regards,

	Hans

> 
>>> +    address, with the Y coordinate in the upper bits and X in the lower bits.
>>
>> And with a __u16 array this becomes: 'The array contains pairs of X, Y coordinates.'
>> Or something along those lines.
>>
>>> +
>>>  .. _IMX8MP: https://www.nxp.com/webapp/Download?colCode=IMX8MPRM
>>> diff --git a/include/uapi/linux/dw100.h b/include/uapi/linux/dw100.h
>>> new file mode 100644
>>> index 000000000000..7fdcf2bf42e5
>>> --- /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>
>>> +
>>
>> Add a comment referring to the Documentation/userspace-api/media/drivers/dw100.rst
>> documentation so users of this control know where to find the associated
>> documentation.
>>
>>> +#define V4L2_CID_DW100_DEWARPING_16x16_VERTEX_MAP (V4L2_CID_USER_DW100_BASE + 1)
>>> +
>>> +#endif
>
  

Hello Hans,

On 4/25/22 09:38, Hans Verkuil wrote:
> On 25/04/2022 09:11, Laurent Pinchart wrote:
>> On Mon, Apr 25, 2022 at 08:57:07AM +0200, Hans Verkuil wrote:
>>> On 28/03/2022 16:13, Xavier Roumegue wrote:
>>>> 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 | 12 ++++++++++++
>>>>   include/uapi/linux/dw100.h                          | 11 +++++++++++
>>>>   2 files changed, 23 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 4cd55c75628e..f6d684cadf26 100644
>>>> --- a/Documentation/userspace-api/media/drivers/dw100.rst
>>>> +++ b/Documentation/userspace-api/media/drivers/dw100.rst
>>>> @@ -20,4 +20,16 @@ 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_DEWARPING_16x16_VERTEX_MAP (integer)``
>>>
>>> (integer) -> (__u32 array)
>>>
>>> But should this be a __u32 array at all? Wouldn't a __u16 array make more sense?
>>>
>>>> +    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. The image is divided into many small 16x16 blocks. If the
>>>> +    width of the image is not divisible by 16, the size of the rightmost block
>>>> +    is the remainder.
>>>
>>> Isn't the same true for the height?
>>>
>>> The dewarping map only saves the vertex coordinates of the
>>>> +    block. The dewarping grid map is comprised of vertex coordinates for x and y.
>>>> +    Each x, y coordinate register uses 16 bits (UQ12.4) to record the coordinate
>>>
>>> As mentioned before, UQ12.4 is not necessarily a standard notation. 'unsigned 12.4
>>> fixed point' is better, but you also need to specify exactly where the bits are
>>> stored inside the __u16. I.e.: 'the integer part is stored in the 12 most significant
>>> bits, and the fractional part is stored in the 4 least significant bits of the __u16.'
>>
>> Isn't that implied ? I've never seen fixed-point numbers stored the
>> other way around.
> 
> True, perhaps that's overkill.
> 
>>
>> Regarding the Q notation, while it was coined by TI, I think it's
>> widespread enough to be used here. I don't mind much though.
> 
> I had to look it up :-)
> 
> That might say more about me, though...
> 
> I think the key phrase that is missing here is "fixed point".
I will then replace in the documentation and comments "UQ12.4" by "an 
unsigned 12.4 fixed point format (UQ12.4)". Considering that I am using 
"UQ12_4" in some symbol names, this might ease the overall code 
understanding.

Regards,
  Xavier
> Regards,
> 
> 	Hans
> 
>>
>>>> +    address, with the Y coordinate in the upper bits and X in the lower bits.
>>>
>>> And with a __u16 array this becomes: 'The array contains pairs of X, Y coordinates.'
>>> Or something along those lines.
>>>
>>>> +
>>>>   .. _IMX8MP: https://www.nxp.com/webapp/Download?colCode=IMX8MPRM
>>>> diff --git a/include/uapi/linux/dw100.h b/include/uapi/linux/dw100.h
>>>> new file mode 100644
>>>> index 000000000000..7fdcf2bf42e5
>>>> --- /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>
>>>> +
>>>
>>> Add a comment referring to the Documentation/userspace-api/media/drivers/dw100.rst
>>> documentation so users of this control know where to find the associated
>>> documentation.
>>>
>>>> +#define V4L2_CID_DW100_DEWARPING_16x16_VERTEX_MAP (V4L2_CID_USER_DW100_BASE + 1)
>>>> +
>>>> +#endif
>>
>
  

Hello Hans,

On 4/25/22 08:57, Hans Verkuil wrote:
> On 28/03/2022 16:13, Xavier Roumegue wrote:
>> 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 | 12 ++++++++++++
>>   include/uapi/linux/dw100.h                          | 11 +++++++++++
>>   2 files changed, 23 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 4cd55c75628e..f6d684cadf26 100644
>> --- a/Documentation/userspace-api/media/drivers/dw100.rst
>> +++ b/Documentation/userspace-api/media/drivers/dw100.rst
>> @@ -20,4 +20,16 @@ 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_DEWARPING_16x16_VERTEX_MAP (integer)``
> 
> (integer) -> (__u32 array)
> 
> But should this be a __u32 array at all? Wouldn't a __u16 array make more sense?
This is indeed debatable. But the array is describing vertices positions 
on a 2D dimension space, and thus its size is always even.
More importantly, the array must follow the format imposed by the 
hardware which expects __u16 pairs packed in a __u32.
Lastly, the lut (map) register size unit is __u32.

Hence, IMHO, using __u32 might make more sense to highlight its hardware 
dependency.


> 
>> +    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. The image is divided into many small 16x16 blocks. If the
>> +    width of the image is not divisible by 16, the size of the rightmost block
>> +    is the remainder.
> 
> Isn't the same true for the height?
Yes, will update accordingly.
> 
> The dewarping map only saves the vertex coordinates of the
>> +    block. The dewarping grid map is comprised of vertex coordinates for x and y.
>> +    Each x, y coordinate register uses 16 bits (UQ12.4) to record the coordinate
> 
> As mentioned before, UQ12.4 is not necessarily a standard notation. 'unsigned 12.4
> fixed point' is better, but you also need to specify exactly where the bits are
> stored inside the __u16. I.e.: 'the integer part is stored in the 12 most significant
> bits, and the fractional part is stored in the 4 least significant bits of the __u16.'
> 
>> +    address, with the Y coordinate in the upper bits and X in the lower bits.
> 
> And with a __u16 array this becomes: 'The array contains pairs of X, Y coordinates.'
> Or something along those lines.
> 
>> +
>>   .. _IMX8MP: https://www.nxp.com/webapp/Download?colCode=IMX8MPRM
>> diff --git a/include/uapi/linux/dw100.h b/include/uapi/linux/dw100.h
>> new file mode 100644
>> index 000000000000..7fdcf2bf42e5
>> --- /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>
>> +
> 
> Add a comment referring to the Documentation/userspace-api/media/drivers/dw100.rst
> documentation so users of this control know where to find the associated
> documentation.
> 
>> +#define V4L2_CID_DW100_DEWARPING_16x16_VERTEX_MAP (V4L2_CID_USER_DW100_BASE + 1)
>> +
>> +#endif
> 
> Regards,
> 
> 	Hans
Regards,
  Xavier
  

Hi Xavier,

On Tue, Apr 26, 2022 at 11:34:55PM +0200, Xavier Roumegue (OSS) wrote:
> On 4/25/22 08:57, Hans Verkuil wrote:
> > On 28/03/2022 16:13, Xavier Roumegue wrote:
> >> 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 | 12 ++++++++++++
> >>   include/uapi/linux/dw100.h                          | 11 +++++++++++
> >>   2 files changed, 23 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 4cd55c75628e..f6d684cadf26 100644
> >> --- a/Documentation/userspace-api/media/drivers/dw100.rst
> >> +++ b/Documentation/userspace-api/media/drivers/dw100.rst
> >> @@ -20,4 +20,16 @@ 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_DEWARPING_16x16_VERTEX_MAP (integer)``
> > 
> > (integer) -> (__u32 array)
> > 
> > But should this be a __u32 array at all? Wouldn't a __u16 array make more sense?
>
> This is indeed debatable. But the array is describing vertices positions 
> on a 2D dimension space, and thus its size is always even.
> More importantly, the array must follow the format imposed by the 
> hardware which expects __u16 pairs packed in a __u32.
> Lastly, the lut (map) register size unit is __u32.
> 
> Hence, IMHO, using __u32 might make more sense to highlight its hardware 
> dependency.

Agreed.

As mentioned in a reply to another patch, I think it would be useful to
explain this a bit more clearly. Hans mentioned in the review of the
driver itself that there was a bug as an image width of 16 bits results
in a grid width of 2. I think that's correct (a width between 1 and 16
pixels results in a single grid cell horizontally, and thus two
vertices, on the left and right side of the cell), but it would benefit
from an explanation. A small ascii art diagram could help.

> >> +    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. The image is divided into many small 16x16 blocks. If the
> >> +    width of the image is not divisible by 16, the size of the rightmost block
> >> +    is the remainder.
> > 
> > Isn't the same true for the height?
> 
> Yes, will update accordingly.
>
> > The dewarping map only saves the vertex coordinates of the
> > 
> >> +    block. The dewarping grid map is comprised of vertex coordinates for x and y.
> >> +    Each x, y coordinate register uses 16 bits (UQ12.4) to record the coordinate
> > 
> > As mentioned before, UQ12.4 is not necessarily a standard notation. 'unsigned 12.4
> > fixed point' is better, but you also need to specify exactly where the bits are
> > stored inside the __u16. I.e.: 'the integer part is stored in the 12 most significant
> > bits, and the fractional part is stored in the 4 least significant bits of the __u16.'
> > 
> >> +    address, with the Y coordinate in the upper bits and X in the lower bits.
> > 
> > And with a __u16 array this becomes: 'The array contains pairs of X, Y coordinates.'
> > Or something along those lines.
> > 
> >> +
> >>   .. _IMX8MP: https://www.nxp.com/webapp/Download?colCode=IMX8MPRM
> >> diff --git a/include/uapi/linux/dw100.h b/include/uapi/linux/dw100.h
> >> new file mode 100644
> >> index 000000000000..7fdcf2bf42e5
> >> --- /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>
> >> +
> > 
> > Add a comment referring to the Documentation/userspace-api/media/drivers/dw100.rst
> > documentation so users of this control know where to find the associated
> > documentation.
> > 
> >> +#define V4L2_CID_DW100_DEWARPING_16x16_VERTEX_MAP (V4L2_CID_USER_DW100_BASE + 1)
> >> +
> >> +#endif