[v3,3/5] media: uapi: Add ST VGXY61 header file

Message ID 20220512074037.3829926-4-benjamin.mugnier@foss.st.com (mailing list archive)
State Superseded
Headers
Series media: Add ST VGXY61 camera sensor driver |

Commit Message

Benjamin Mugnier May 12, 2022, 7:40 a.m. UTC
  Define an HDR control in it, and adds its documentation in st-vgxy61.rst
file.

Signed-off-by: Benjamin Mugnier <benjamin.mugnier@foss.st.com>
---
 .../userspace-api/media/drivers/st-vgxy61.rst | 23 +++++++++++++++++++
 include/uapi/linux/st-vgxy61.h                | 15 ++++++++++++
 2 files changed, 38 insertions(+)
 create mode 100644 Documentation/userspace-api/media/drivers/st-vgxy61.rst
 create mode 100644 include/uapi/linux/st-vgxy61.h
  

Comments

kernel test robot May 12, 2022, 10:32 a.m. UTC | #1
Hi Benjamin,

I love your patch! Yet something to improve:

[auto build test ERROR on media-tree/master]
[also build test ERROR on linus/master v5.18-rc6 next-20220511]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Benjamin-Mugnier/media-Add-ST-VGXY61-camera-sensor-driver/20220512-154318
base:   git://linuxtv.org/media_tree.git master
config: mips-allyesconfig (https://download.01.org/0day-ci/archive/20220512/202205121843.Y5ufdQpc-lkp@intel.com/config)
compiler: mips-linux-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/47c49a5b0ade9511ba79fbe14a0d2231975757e5
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Benjamin-Mugnier/media-Add-ST-VGXY61-camera-sensor-driver/20220512-154318
        git checkout 47c49a5b0ade9511ba79fbe14a0d2231975757e5
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=mips prepare

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   scripts/genksyms/parse.y: warning: 9 shift/reduce conflicts [-Wconflicts-sr]
   scripts/genksyms/parse.y: warning: 5 reduce/reduce conflicts [-Wconflicts-rr]
   scripts/genksyms/parse.y: note: rerun with option '-Wcounterexamples' to generate conflict counterexamples
>> error: include/uapi/linux/st-vgxy61.h: missing "WITH Linux-syscall-note" for SPDX-License-Identifier
   make[2]: *** [scripts/Makefile.headersinst:63: usr/include/linux/st-vgxy61.h] Error 1
   make[2]: Target '__headers' not remade because of errors.
   make[1]: *** [Makefile:1280: headers] Error 2
   arch/mips/kernel/asm-offsets.c:26:6: warning: no previous prototype for 'output_ptreg_defines' [-Wmissing-prototypes]
      26 | void output_ptreg_defines(void)
         |      ^~~~~~~~~~~~~~~~~~~~
   arch/mips/kernel/asm-offsets.c:78:6: warning: no previous prototype for 'output_task_defines' [-Wmissing-prototypes]
      78 | void output_task_defines(void)
         |      ^~~~~~~~~~~~~~~~~~~
   arch/mips/kernel/asm-offsets.c:92:6: warning: no previous prototype for 'output_thread_info_defines' [-Wmissing-prototypes]
      92 | void output_thread_info_defines(void)
         |      ^~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/mips/kernel/asm-offsets.c:108:6: warning: no previous prototype for 'output_thread_defines' [-Wmissing-prototypes]
     108 | void output_thread_defines(void)
         |      ^~~~~~~~~~~~~~~~~~~~~
   arch/mips/kernel/asm-offsets.c:136:6: warning: no previous prototype for 'output_thread_fpu_defines' [-Wmissing-prototypes]
     136 | void output_thread_fpu_defines(void)
         |      ^~~~~~~~~~~~~~~~~~~~~~~~~
   arch/mips/kernel/asm-offsets.c:179:6: warning: no previous prototype for 'output_mm_defines' [-Wmissing-prototypes]
     179 | void output_mm_defines(void)
         |      ^~~~~~~~~~~~~~~~~
   arch/mips/kernel/asm-offsets.c:218:6: warning: no previous prototype for 'output_sc_defines' [-Wmissing-prototypes]
     218 | void output_sc_defines(void)
         |      ^~~~~~~~~~~~~~~~~
   arch/mips/kernel/asm-offsets.c:253:6: warning: no previous prototype for 'output_signal_defined' [-Wmissing-prototypes]
     253 | void output_signal_defined(void)
         |      ^~~~~~~~~~~~~~~~~~~~~
   arch/mips/kernel/asm-offsets.c:320:6: warning: no previous prototype for 'output_pbe_defines' [-Wmissing-prototypes]
     320 | void output_pbe_defines(void)
         |      ^~~~~~~~~~~~~~~~~~
   arch/mips/kernel/asm-offsets.c:332:6: warning: no previous prototype for 'output_pm_defines' [-Wmissing-prototypes]
     332 | void output_pm_defines(void)
         |      ^~~~~~~~~~~~~~~~~
   arch/mips/kernel/asm-offsets.c:346:6: warning: no previous prototype for 'output_kvm_defines' [-Wmissing-prototypes]
     346 | void output_kvm_defines(void)
         |      ^~~~~~~~~~~~~~~~~~
   arch/mips/kernel/asm-offsets.c:390:6: warning: no previous prototype for 'output_cps_defines' [-Wmissing-prototypes]
     390 | void output_cps_defines(void)
         |      ^~~~~~~~~~~~~~~~~~
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [Makefile:219: __sub-make] Error 2
   make: Target 'prepare' not remade because of errors.
  
Benjamin Mugnier May 12, 2022, 12:43 p.m. UTC | #2
Fixed for v4.

Benjamin

On 12/05/2022 12:32, kernel test robot wrote:
> Hi Benjamin,
> 
> I love your patch! Yet something to improve:
> 
> [auto build test ERROR on media-tree/master]
> [also build test ERROR on linus/master v5.18-rc6 next-20220511]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Benjamin-Mugnier/media-Add-ST-VGXY61-camera-sensor-driver/20220512-154318
> base:   git://linuxtv.org/media_tree.git master
> config: mips-allyesconfig (https://download.01.org/0day-ci/archive/20220512/202205121843.Y5ufdQpc-lkp@intel.com/config)
> compiler: mips-linux-gcc (GCC) 11.3.0
> reproduce (this is a W=1 build):
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # https://github.com/intel-lab-lkp/linux/commit/47c49a5b0ade9511ba79fbe14a0d2231975757e5
>         git remote add linux-review https://github.com/intel-lab-lkp/linux
>         git fetch --no-tags linux-review Benjamin-Mugnier/media-Add-ST-VGXY61-camera-sensor-driver/20220512-154318
>         git checkout 47c49a5b0ade9511ba79fbe14a0d2231975757e5
>         # save the config file
>         mkdir build_dir && cp config build_dir/.config
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=mips prepare
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All errors (new ones prefixed by >>):
> 
>    scripts/genksyms/parse.y: warning: 9 shift/reduce conflicts [-Wconflicts-sr]
>    scripts/genksyms/parse.y: warning: 5 reduce/reduce conflicts [-Wconflicts-rr]
>    scripts/genksyms/parse.y: note: rerun with option '-Wcounterexamples' to generate conflict counterexamples
>>> error: include/uapi/linux/st-vgxy61.h: missing "WITH Linux-syscall-note" for SPDX-License-Identifier
>    make[2]: *** [scripts/Makefile.headersinst:63: usr/include/linux/st-vgxy61.h] Error 1
>    make[2]: Target '__headers' not remade because of errors.
>    make[1]: *** [Makefile:1280: headers] Error 2
>    arch/mips/kernel/asm-offsets.c:26:6: warning: no previous prototype for 'output_ptreg_defines' [-Wmissing-prototypes]
>       26 | void output_ptreg_defines(void)
>          |      ^~~~~~~~~~~~~~~~~~~~
>    arch/mips/kernel/asm-offsets.c:78:6: warning: no previous prototype for 'output_task_defines' [-Wmissing-prototypes]
>       78 | void output_task_defines(void)
>          |      ^~~~~~~~~~~~~~~~~~~
>    arch/mips/kernel/asm-offsets.c:92:6: warning: no previous prototype for 'output_thread_info_defines' [-Wmissing-prototypes]
>       92 | void output_thread_info_defines(void)
>          |      ^~~~~~~~~~~~~~~~~~~~~~~~~~
>    arch/mips/kernel/asm-offsets.c:108:6: warning: no previous prototype for 'output_thread_defines' [-Wmissing-prototypes]
>      108 | void output_thread_defines(void)
>          |      ^~~~~~~~~~~~~~~~~~~~~
>    arch/mips/kernel/asm-offsets.c:136:6: warning: no previous prototype for 'output_thread_fpu_defines' [-Wmissing-prototypes]
>      136 | void output_thread_fpu_defines(void)
>          |      ^~~~~~~~~~~~~~~~~~~~~~~~~
>    arch/mips/kernel/asm-offsets.c:179:6: warning: no previous prototype for 'output_mm_defines' [-Wmissing-prototypes]
>      179 | void output_mm_defines(void)
>          |      ^~~~~~~~~~~~~~~~~
>    arch/mips/kernel/asm-offsets.c:218:6: warning: no previous prototype for 'output_sc_defines' [-Wmissing-prototypes]
>      218 | void output_sc_defines(void)
>          |      ^~~~~~~~~~~~~~~~~
>    arch/mips/kernel/asm-offsets.c:253:6: warning: no previous prototype for 'output_signal_defined' [-Wmissing-prototypes]
>      253 | void output_signal_defined(void)
>          |      ^~~~~~~~~~~~~~~~~~~~~
>    arch/mips/kernel/asm-offsets.c:320:6: warning: no previous prototype for 'output_pbe_defines' [-Wmissing-prototypes]
>      320 | void output_pbe_defines(void)
>          |      ^~~~~~~~~~~~~~~~~~
>    arch/mips/kernel/asm-offsets.c:332:6: warning: no previous prototype for 'output_pm_defines' [-Wmissing-prototypes]
>      332 | void output_pm_defines(void)
>          |      ^~~~~~~~~~~~~~~~~
>    arch/mips/kernel/asm-offsets.c:346:6: warning: no previous prototype for 'output_kvm_defines' [-Wmissing-prototypes]
>      346 | void output_kvm_defines(void)
>          |      ^~~~~~~~~~~~~~~~~~
>    arch/mips/kernel/asm-offsets.c:390:6: warning: no previous prototype for 'output_cps_defines' [-Wmissing-prototypes]
>      390 | void output_cps_defines(void)
>          |      ^~~~~~~~~~~~~~~~~~
>    make[1]: Target 'prepare' not remade because of errors.
>    make: *** [Makefile:219: __sub-make] Error 2
>    make: Target 'prepare' not remade because of errors.
>
  
Sakari Ailus Aug. 22, 2022, 10:37 a.m. UTC | #3
Hi Benjamin,

On Thu, May 12, 2022 at 09:40:35AM +0200, Benjamin Mugnier wrote:
> Define an HDR control in it, and adds its documentation in st-vgxy61.rst
> file.
> 
> Signed-off-by: Benjamin Mugnier <benjamin.mugnier@foss.st.com>
> ---
>  .../userspace-api/media/drivers/st-vgxy61.rst | 23 +++++++++++++++++++
>  include/uapi/linux/st-vgxy61.h                | 15 ++++++++++++
>  2 files changed, 38 insertions(+)
>  create mode 100644 Documentation/userspace-api/media/drivers/st-vgxy61.rst
>  create mode 100644 include/uapi/linux/st-vgxy61.h
> 
> diff --git a/Documentation/userspace-api/media/drivers/st-vgxy61.rst b/Documentation/userspace-api/media/drivers/st-vgxy61.rst
> new file mode 100644
> index 000000000000..7a11adbb558f
> --- /dev/null
> +++ b/Documentation/userspace-api/media/drivers/st-vgxy61.rst
> @@ -0,0 +1,23 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +ST VGXY61 camera sensor driver
> +==============================
> +
> +The ST VGXY61 driver implements the following driver-specific controls:
> +
> +``V4L2_CID_STVGXY61_HDR``
> +-------------------------------
> +    Change the sensor HDR mode. A HDR picture is obtained by merging two captures of the same scene
> +    using two different exposure periods.
> +
> +.. flat-table::
> +    :header-rows:  0
> +    :stub-columns: 0
> +    :widths:       1 4
> +
> +    * - HDR linearize
> +      - The merger outputs a long exposure capture as long as it is not saturated.
> +    * - HDR substraction
> +      - This involves subtracting the short exposure frame from the long exposure frame.
> +    * - "no HDR"
> +      - This mode is used for standard dynamic range (SDR) exposures.

I wonder how many different kinds of HDR implementations there are that
could be meaningfully controlled using a single control. We have controls
such as scene mode that are much more generic than this.

Could this be standardised, even if the menu items wouldn't be? Say, call
it e.g. V4L2_CID_HDR_MODE? Raw sensors have different configuration with
more parameters though.

I wonder what others think.

> diff --git a/include/uapi/linux/st-vgxy61.h b/include/uapi/linux/st-vgxy61.h
> new file mode 100644
> index 000000000000..fbabe2cb64ac
> --- /dev/null
> +++ b/include/uapi/linux/st-vgxy61.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2022 STMicroelectronics SA
> + *
> + */
> +
> +#ifndef __UAPI_STVGXY61_H_
> +#define __UAPI_STVGXY61_H_
> +
> +#include <linux/v4l2-controls.h>
> +
> +/* Control HDR mode */
> +#define V4L2_CID_STVGXY61_HDR	(V4L2_CID_USER_STVGXY61_BASE + 0)
> +
> +#endif /* __UAPI_STVGXY61_H_ */
  
Benjamin Mugnier Aug. 26, 2022, 2:17 p.m. UTC | #4
On 8/22/22 12:37, Sakari Ailus wrote:
> Hi Benjamin,
> 
> On Thu, May 12, 2022 at 09:40:35AM +0200, Benjamin Mugnier wrote:
>> Define an HDR control in it, and adds its documentation in st-vgxy61.rst
>> file.
>>
>> Signed-off-by: Benjamin Mugnier <benjamin.mugnier@foss.st.com>
>> ---
>>   .../userspace-api/media/drivers/st-vgxy61.rst | 23 +++++++++++++++++++
>>   include/uapi/linux/st-vgxy61.h                | 15 ++++++++++++
>>   2 files changed, 38 insertions(+)
>>   create mode 100644 Documentation/userspace-api/media/drivers/st-vgxy61.rst
>>   create mode 100644 include/uapi/linux/st-vgxy61.h
>>
>> diff --git a/Documentation/userspace-api/media/drivers/st-vgxy61.rst b/Documentation/userspace-api/media/drivers/st-vgxy61.rst
>> new file mode 100644
>> index 000000000000..7a11adbb558f
>> --- /dev/null
>> +++ b/Documentation/userspace-api/media/drivers/st-vgxy61.rst
>> @@ -0,0 +1,23 @@
>> +.. SPDX-License-Identifier: GPL-2.0
>> +
>> +ST VGXY61 camera sensor driver
>> +==============================
>> +
>> +The ST VGXY61 driver implements the following driver-specific controls:
>> +
>> +``V4L2_CID_STVGXY61_HDR``
>> +-------------------------------
>> +    Change the sensor HDR mode. A HDR picture is obtained by merging two captures of the same scene
>> +    using two different exposure periods.
>> +
>> +.. flat-table::
>> +    :header-rows:  0
>> +    :stub-columns: 0
>> +    :widths:       1 4
>> +
>> +    * - HDR linearize
>> +      - The merger outputs a long exposure capture as long as it is not saturated.
>> +    * - HDR substraction
>> +      - This involves subtracting the short exposure frame from the long exposure frame.
>> +    * - "no HDR"
>> +      - This mode is used for standard dynamic range (SDR) exposures.
> 
> I wonder how many different kinds of HDR implementations there are that
> could be meaningfully controlled using a single control. We have controls
> such as scene mode that are much more generic than this.
> 
> Could this be standardised, even if the menu items wouldn't be? Say, call
> it e.g. V4L2_CID_HDR_MODE? Raw sensors have different configuration with
> more parameters though.
> 
> I wonder what others think.
> 

I agree.
I would like to standardize the control and not the menu items just like 
you said, as some sensors are missing some modes and/or have some 
others. This is the way to go.

I will submit something for v4 if nobody is against this idea.

>> diff --git a/include/uapi/linux/st-vgxy61.h b/include/uapi/linux/st-vgxy61.h
>> new file mode 100644
>> index 000000000000..fbabe2cb64ac
>> --- /dev/null
>> +++ b/include/uapi/linux/st-vgxy61.h
>> @@ -0,0 +1,15 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Copyright (C) 2022 STMicroelectronics SA
>> + *
>> + */
>> +
>> +#ifndef __UAPI_STVGXY61_H_
>> +#define __UAPI_STVGXY61_H_
>> +
>> +#include <linux/v4l2-controls.h>
>> +
>> +/* Control HDR mode */
>> +#define V4L2_CID_STVGXY61_HDR	(V4L2_CID_USER_STVGXY61_BASE + 0)
>> +
>> +#endif /* __UAPI_STVGXY61_H_ */
>
  

Patch

diff --git a/Documentation/userspace-api/media/drivers/st-vgxy61.rst b/Documentation/userspace-api/media/drivers/st-vgxy61.rst
new file mode 100644
index 000000000000..7a11adbb558f
--- /dev/null
+++ b/Documentation/userspace-api/media/drivers/st-vgxy61.rst
@@ -0,0 +1,23 @@ 
+.. SPDX-License-Identifier: GPL-2.0
+
+ST VGXY61 camera sensor driver
+==============================
+
+The ST VGXY61 driver implements the following driver-specific controls:
+
+``V4L2_CID_STVGXY61_HDR``
+-------------------------------
+    Change the sensor HDR mode. A HDR picture is obtained by merging two captures of the same scene
+    using two different exposure periods.
+
+.. flat-table::
+    :header-rows:  0
+    :stub-columns: 0
+    :widths:       1 4
+
+    * - HDR linearize
+      - The merger outputs a long exposure capture as long as it is not saturated.
+    * - HDR substraction
+      - This involves subtracting the short exposure frame from the long exposure frame.
+    * - "no HDR"
+      - This mode is used for standard dynamic range (SDR) exposures.
diff --git a/include/uapi/linux/st-vgxy61.h b/include/uapi/linux/st-vgxy61.h
new file mode 100644
index 000000000000..fbabe2cb64ac
--- /dev/null
+++ b/include/uapi/linux/st-vgxy61.h
@@ -0,0 +1,15 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2022 STMicroelectronics SA
+ *
+ */
+
+#ifndef __UAPI_STVGXY61_H_
+#define __UAPI_STVGXY61_H_
+
+#include <linux/v4l2-controls.h>
+
+/* Control HDR mode */
+#define V4L2_CID_STVGXY61_HDR	(V4L2_CID_USER_STVGXY61_BASE + 0)
+
+#endif /* __UAPI_STVGXY61_H_ */