Message ID | 20220512074037.3829926-4-benjamin.mugnier@foss.st.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | media: Add ST VGXY61 camera sensor driver | expand |
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.
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. >
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_ */
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_ */ >
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_ */
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