* [PATCH v3 0/3] smiapp test pattern support
@ 2014-05-29 14:40 Sakari Ailus
2014-05-29 14:40 ` [PATCH v3 1/3] v4l: Add test pattern colour component controls Sakari Ailus
` (2 more replies)
0 siblings, 3 replies; 24+ messages in thread
From: Sakari Ailus @ 2014-05-29 14:40 UTC (permalink / raw)
To: linux-media; +Cc: laurent.pinchart, hverkuil
Hi,
These patches add four standard controls for test pattern raw bayer colour
component values and test pattern support for the smiapp driver.
Additionally, definitions are provided to control the smiapp driver's test
pattern menu, the items of which are driver specific.
Also the location of the header file has been fixed in the 2nd patch.
The new controls are located in the image source class (vs. the image
processing class where the test pattern control is). I do prefer consistency
but when it's in conflict with correctness, the latter often wins. :-)
Kind regards,
Sakari
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH v3 1/3] v4l: Add test pattern colour component controls 2014-05-29 14:40 [PATCH v3 0/3] smiapp test pattern support Sakari Ailus @ 2014-05-29 14:40 ` Sakari Ailus 2014-05-29 14:47 ` Laurent Pinchart 2014-06-10 12:15 ` Hans Verkuil 2014-05-29 14:40 ` [PATCH v3 2/3] smiapp: Add driver-specific test pattern menu item definitions Sakari Ailus 2014-05-29 14:40 ` [PATCH v3 3/3] smiapp: Implement the test pattern control Sakari Ailus 2 siblings, 2 replies; 24+ messages in thread From: Sakari Ailus @ 2014-05-29 14:40 UTC (permalink / raw) To: linux-media; +Cc: laurent.pinchart, hverkuil In many cases the test pattern has selectable values for each colour component. Implement controls for raw bayer components. Additional controls should be defined for colour components that are not covered by these controls. Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> --- Documentation/DocBook/media/v4l/controls.xml | 34 ++++++++++++++++++++++++++++ drivers/media/v4l2-core/v4l2-ctrls.c | 4 ++++ include/uapi/linux/v4l2-controls.h | 4 ++++ 3 files changed, 42 insertions(+) diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml index 47198ee..bf23994 100644 --- a/Documentation/DocBook/media/v4l/controls.xml +++ b/Documentation/DocBook/media/v4l/controls.xml @@ -4677,6 +4677,40 @@ interface and may change in the future.</para> conversion. </entry> </row> + <row> + <entry spanname="id"><constant>V4L2_CID_TEST_PATTERN_RED</constant></entry> + <entry>integer</entry> + </row> + <row> + <entry spanname="descr">Test pattern red colour component. + </entry> + </row> + <row> + <entry spanname="id"><constant>V4L2_CID_TEST_PATTERN_GREENR</constant></entry> + <entry>integer</entry> + </row> + <row> + <entry spanname="descr">Test pattern green (next to red) + colour component. + </entry> + </row> + <row> + <entry spanname="id"><constant>V4L2_CID_TEST_PATTERN_BLUE</constant></entry> + <entry>integer</entry> + </row> + <row> + <entry spanname="descr">Test pattern blue colour component. + </entry> + </row> + <row> + <entry spanname="id"><constant>V4L2_CID_TEST_PATTERN_GREENB</constant></entry> + <entry>integer</entry> + </row> + <row> + <entry spanname="descr">Test pattern green (next to blue) + colour component. + </entry> + </row> <row><entry></entry></row> </tbody> </tgroup> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c index 55c6832..a4104a7 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls.c +++ b/drivers/media/v4l2-core/v4l2-ctrls.c @@ -839,6 +839,10 @@ const char *v4l2_ctrl_get_name(u32 id) case V4L2_CID_VBLANK: return "Vertical Blanking"; case V4L2_CID_HBLANK: return "Horizontal Blanking"; case V4L2_CID_ANALOGUE_GAIN: return "Analogue Gain"; + case V4L2_CID_TEST_PATTERN_RED: return "Red Pixel Value"; + case V4L2_CID_TEST_PATTERN_GREENR: return "Green (Red) Pixel Value"; + case V4L2_CID_TEST_PATTERN_BLUE: return "Blue Pixel Value"; + case V4L2_CID_TEST_PATTERN_GREENB: return "Green (Blue) Pixel Value"; /* Image processing controls */ case V4L2_CID_IMAGE_PROC_CLASS: return "Image Processing Controls"; diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h index 2ac5597..5c55a19 100644 --- a/include/uapi/linux/v4l2-controls.h +++ b/include/uapi/linux/v4l2-controls.h @@ -855,6 +855,10 @@ enum v4l2_jpeg_chroma_subsampling { #define V4L2_CID_VBLANK (V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 1) #define V4L2_CID_HBLANK (V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 2) #define V4L2_CID_ANALOGUE_GAIN (V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 3) +#define V4L2_CID_TEST_PATTERN_RED (V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 4) +#define V4L2_CID_TEST_PATTERN_GREENR (V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 5) +#define V4L2_CID_TEST_PATTERN_BLUE (V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 6) +#define V4L2_CID_TEST_PATTERN_GREENB (V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 7) /* Image processing controls */ -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v3 1/3] v4l: Add test pattern colour component controls 2014-05-29 14:40 ` [PATCH v3 1/3] v4l: Add test pattern colour component controls Sakari Ailus @ 2014-05-29 14:47 ` Laurent Pinchart 2014-05-29 14:58 ` Sakari Ailus 2014-06-10 12:15 ` Hans Verkuil 1 sibling, 1 reply; 24+ messages in thread From: Laurent Pinchart @ 2014-05-29 14:47 UTC (permalink / raw) To: Sakari Ailus; +Cc: linux-media, hverkuil Hi Sakari, Thank you for the patch. On Thursday 29 May 2014 17:40:46 Sakari Ailus wrote: > In many cases the test pattern has selectable values for each colour > component. Implement controls for raw bayer components. Additional controls > should be defined for colour components that are not covered by these > controls. > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > --- > Documentation/DocBook/media/v4l/controls.xml | 34 +++++++++++++++++++++++++ > drivers/media/v4l2-core/v4l2-ctrls.c | 4 ++++ > include/uapi/linux/v4l2-controls.h | 4 ++++ > 3 files changed, 42 insertions(+) > > diff --git a/Documentation/DocBook/media/v4l/controls.xml > b/Documentation/DocBook/media/v4l/controls.xml index 47198ee..bf23994 > 100644 > --- a/Documentation/DocBook/media/v4l/controls.xml > +++ b/Documentation/DocBook/media/v4l/controls.xml > @@ -4677,6 +4677,40 @@ interface and may change in the future.</para> > conversion. > </entry> > </row> > + <row> > + <entry > spanname="id"><constant>V4L2_CID_TEST_PATTERN_RED</constant></entry> + > <entry>integer</entry> > + </row> > + <row> > + <entry spanname="descr">Test pattern red colour component. > + </entry> > + </row> > + <row> > + <entry > spanname="id"><constant>V4L2_CID_TEST_PATTERN_GREENR</constant></entry> > + <entry>integer</entry> > + </row> > + <row> > + <entry spanname="descr">Test pattern green (next to red) > + colour component. What about non-Bayer RGB sensors ? Should they use the GREENR or the GREENB control for the green component ? Or a different control ? I'm wondering whether we shouldn't have a single test pattern color control and create a color type using Hans' complex controls API. > + </entry> > + </row> > + <row> > + <entry > spanname="id"><constant>V4L2_CID_TEST_PATTERN_BLUE</constant></entry> + > <entry>integer</entry> > + </row> > + <row> > + <entry spanname="descr">Test pattern blue colour component. > + </entry> > + </row> > + <row> > + <entry > spanname="id"><constant>V4L2_CID_TEST_PATTERN_GREENB</constant></entry> + > <entry>integer</entry> > + </row> > + <row> > + <entry spanname="descr">Test pattern green (next to blue) > + colour component. > + </entry> > + </row> > <row><entry></entry></row> > </tbody> > </tgroup> > diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c > b/drivers/media/v4l2-core/v4l2-ctrls.c index 55c6832..a4104a7 100644 > --- a/drivers/media/v4l2-core/v4l2-ctrls.c > +++ b/drivers/media/v4l2-core/v4l2-ctrls.c > @@ -839,6 +839,10 @@ const char *v4l2_ctrl_get_name(u32 id) > case V4L2_CID_VBLANK: return "Vertical Blanking"; > case V4L2_CID_HBLANK: return "Horizontal Blanking"; > case V4L2_CID_ANALOGUE_GAIN: return "Analogue Gain"; > + case V4L2_CID_TEST_PATTERN_RED: return "Red Pixel Value"; > + case V4L2_CID_TEST_PATTERN_GREENR: return "Green (Red) Pixel Value"; > + case V4L2_CID_TEST_PATTERN_BLUE: return "Blue Pixel Value"; > + case V4L2_CID_TEST_PATTERN_GREENB: return "Green (Blue) Pixel Value"; > > /* Image processing controls */ > case V4L2_CID_IMAGE_PROC_CLASS: return "Image Processing Controls"; > diff --git a/include/uapi/linux/v4l2-controls.h > b/include/uapi/linux/v4l2-controls.h index 2ac5597..5c55a19 100644 > --- a/include/uapi/linux/v4l2-controls.h > +++ b/include/uapi/linux/v4l2-controls.h > @@ -855,6 +855,10 @@ enum v4l2_jpeg_chroma_subsampling { > #define V4L2_CID_VBLANK (V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 1) > #define V4L2_CID_HBLANK (V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 2) > #define V4L2_CID_ANALOGUE_GAIN (V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 3) > +#define V4L2_CID_TEST_PATTERN_RED (V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 4) > +#define V4L2_CID_TEST_PATTERN_GREENR (V4L2_CID_IMAGE_SOURCE_CLASS_BASE + > 5) +#define V4L2_CID_TEST_PATTERN_BLUE (V4L2_CID_IMAGE_SOURCE_CLASS_BASE + > 6) +#define V4L2_CID_TEST_PATTERN_GREENB (V4L2_CID_IMAGE_SOURCE_CLASS_BASE > + 7) > > > /* Image processing controls */ -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 1/3] v4l: Add test pattern colour component controls 2014-05-29 14:47 ` Laurent Pinchart @ 2014-05-29 14:58 ` Sakari Ailus 2014-05-29 15:01 ` Laurent Pinchart 0 siblings, 1 reply; 24+ messages in thread From: Sakari Ailus @ 2014-05-29 14:58 UTC (permalink / raw) To: Laurent Pinchart; +Cc: linux-media, hverkuil Hi Laurent, Thanks for your comments. Laurent Pinchart wrote: > Hi Sakari, > > Thank you for the patch. > > On Thursday 29 May 2014 17:40:46 Sakari Ailus wrote: >> In many cases the test pattern has selectable values for each colour >> component. Implement controls for raw bayer components. Additional controls >> should be defined for colour components that are not covered by these >> controls. >> >> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> >> --- >> Documentation/DocBook/media/v4l/controls.xml | 34 +++++++++++++++++++++++++ >> drivers/media/v4l2-core/v4l2-ctrls.c | 4 ++++ >> include/uapi/linux/v4l2-controls.h | 4 ++++ >> 3 files changed, 42 insertions(+) >> >> diff --git a/Documentation/DocBook/media/v4l/controls.xml >> b/Documentation/DocBook/media/v4l/controls.xml index 47198ee..bf23994 >> 100644 >> --- a/Documentation/DocBook/media/v4l/controls.xml >> +++ b/Documentation/DocBook/media/v4l/controls.xml >> @@ -4677,6 +4677,40 @@ interface and may change in the future.</para> >> conversion. >> </entry> >> </row> >> + <row> >> + <entry >> spanname="id"><constant>V4L2_CID_TEST_PATTERN_RED</constant></entry> + >> <entry>integer</entry> >> + </row> >> + <row> >> + <entry spanname="descr">Test pattern red colour component. >> + </entry> >> + </row> >> + <row> >> + <entry >> spanname="id"><constant>V4L2_CID_TEST_PATTERN_GREENR</constant></entry> >> + <entry>integer</entry> >> + </row> >> + <row> >> + <entry spanname="descr">Test pattern green (next to red) >> + colour component. > > What about non-Bayer RGB sensors ? Should they use the GREENR or the GREENB > control for the green component ? Or a different control ? A different one. It should be simply green. I could add it to the same patch if you wish. > I'm wondering whether we shouldn't have a single test pattern color control > and create a color type using Hans' complex controls API. A raw bayer four-pixel value, you mean? -- Regards, Sakari Ailus sakari.ailus@linux.intel.com ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 1/3] v4l: Add test pattern colour component controls 2014-05-29 14:58 ` Sakari Ailus @ 2014-05-29 15:01 ` Laurent Pinchart 2014-05-30 12:28 ` Hans Verkuil 0 siblings, 1 reply; 24+ messages in thread From: Laurent Pinchart @ 2014-05-29 15:01 UTC (permalink / raw) To: Sakari Ailus; +Cc: linux-media, hverkuil On Thursday 29 May 2014 17:58:59 Sakari Ailus wrote: > Laurent Pinchart wrote: > > On Thursday 29 May 2014 17:40:46 Sakari Ailus wrote: > >> In many cases the test pattern has selectable values for each colour > >> component. Implement controls for raw bayer components. Additional > >> controls > >> should be defined for colour components that are not covered by these > >> controls. > >> > >> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > >> --- > >> > >> Documentation/DocBook/media/v4l/controls.xml | 34 +++++++++++++++++++++ > >> drivers/media/v4l2-core/v4l2-ctrls.c | 4 ++++ > >> include/uapi/linux/v4l2-controls.h | 4 ++++ > >> 3 files changed, 42 insertions(+) > >> > >> diff --git a/Documentation/DocBook/media/v4l/controls.xml > >> b/Documentation/DocBook/media/v4l/controls.xml index 47198ee..bf23994 > >> 100644 > >> --- a/Documentation/DocBook/media/v4l/controls.xml > >> +++ b/Documentation/DocBook/media/v4l/controls.xml > >> @@ -4677,6 +4677,40 @@ interface and may change in the future.</para> > >> conversion. > >> </entry> > >> </row> > >> + <row> > >> + <entry > >> spanname="id"><constant>V4L2_CID_TEST_PATTERN_RED</constant></entry> > >> + <entry>integer</entry> > >> + </row> > >> + <row> > >> + <entry spanname="descr">Test pattern red colour component. > >> + </entry> > >> + </row> > >> + <row> > >> + <entry > >> spanname="id"><constant>V4L2_CID_TEST_PATTERN_GREENR</constant></entry> > >> + <entry>integer</entry> > >> + </row> > >> + <row> > >> + <entry spanname="descr">Test pattern green (next to red) > >> + colour component. > > > > What about non-Bayer RGB sensors ? Should they use the GREENR or the > > GREENB control for the green component ? Or a different control ? > > A different one. It should be simply green. I could add it to the same > patch if you wish. > > > I'm wondering whether we shouldn't have a single test pattern color > > control and create a color type using Hans' complex controls API. > > A raw bayer four-pixel value, you mean? Yes. I'll let Hans comment on that. -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 1/3] v4l: Add test pattern colour component controls 2014-05-29 15:01 ` Laurent Pinchart @ 2014-05-30 12:28 ` Hans Verkuil 2014-06-03 12:21 ` Laurent Pinchart 0 siblings, 1 reply; 24+ messages in thread From: Hans Verkuil @ 2014-05-30 12:28 UTC (permalink / raw) To: Laurent Pinchart, Sakari Ailus; +Cc: linux-media On 05/29/2014 05:01 PM, Laurent Pinchart wrote: > On Thursday 29 May 2014 17:58:59 Sakari Ailus wrote: >> Laurent Pinchart wrote: >>> On Thursday 29 May 2014 17:40:46 Sakari Ailus wrote: >>>> In many cases the test pattern has selectable values for each colour >>>> component. Implement controls for raw bayer components. Additional >>>> controls >>>> should be defined for colour components that are not covered by these >>>> controls. >>>> >>>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> >>>> --- >>>> >>>> Documentation/DocBook/media/v4l/controls.xml | 34 +++++++++++++++++++++ >>>> drivers/media/v4l2-core/v4l2-ctrls.c | 4 ++++ >>>> include/uapi/linux/v4l2-controls.h | 4 ++++ >>>> 3 files changed, 42 insertions(+) >>>> >>>> diff --git a/Documentation/DocBook/media/v4l/controls.xml >>>> b/Documentation/DocBook/media/v4l/controls.xml index 47198ee..bf23994 >>>> 100644 >>>> --- a/Documentation/DocBook/media/v4l/controls.xml >>>> +++ b/Documentation/DocBook/media/v4l/controls.xml >>>> @@ -4677,6 +4677,40 @@ interface and may change in the future.</para> >>>> conversion. >>>> </entry> >>>> </row> >>>> + <row> >>>> + <entry >>>> spanname="id"><constant>V4L2_CID_TEST_PATTERN_RED</constant></entry> >>>> + <entry>integer</entry> >>>> + </row> >>>> + <row> >>>> + <entry spanname="descr">Test pattern red colour component. >>>> + </entry> >>>> + </row> >>>> + <row> >>>> + <entry >>>> spanname="id"><constant>V4L2_CID_TEST_PATTERN_GREENR</constant></entry> >>>> + <entry>integer</entry> >>>> + </row> >>>> + <row> >>>> + <entry spanname="descr">Test pattern green (next to red) >>>> + colour component. >>> >>> What about non-Bayer RGB sensors ? Should they use the GREENR or the >>> GREENB control for the green component ? Or a different control ? >> >> A different one. It should be simply green. I could add it to the same >> patch if you wish. >> >>> I'm wondering whether we shouldn't have a single test pattern color >>> control and create a color type using Hans' complex controls API. >> >> A raw bayer four-pixel value, you mean? > > Yes. I'll let Hans comment on that. > Why would you need the complex control API for that? It would fit in a s32, and certainly in a s64. We have done something similar to this in the past (V4L2_CID_BG_COLOR). The main problem is that the interpretation of the s32 value has to be clearly defined. And if different sensors might have different min/max values for each component, then it becomes messy to use a single control. My feeling is that it is better to go with separate controls, one for each component. Regards, Hans ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 1/3] v4l: Add test pattern colour component controls 2014-05-30 12:28 ` Hans Verkuil @ 2014-06-03 12:21 ` Laurent Pinchart 2014-06-03 12:27 ` Sakari Ailus 2014-06-03 12:42 ` Hans Verkuil 0 siblings, 2 replies; 24+ messages in thread From: Laurent Pinchart @ 2014-06-03 12:21 UTC (permalink / raw) To: Hans Verkuil; +Cc: Sakari Ailus, linux-media Hi Hans, On Friday 30 May 2014 14:28:16 Hans Verkuil wrote: > On 05/29/2014 05:01 PM, Laurent Pinchart wrote: > > On Thursday 29 May 2014 17:58:59 Sakari Ailus wrote: > >> Laurent Pinchart wrote: > >>> On Thursday 29 May 2014 17:40:46 Sakari Ailus wrote: > >>>> In many cases the test pattern has selectable values for each colour > >>>> component. Implement controls for raw bayer components. Additional > >>>> controls should be defined for colour components that are not covered > >>>> by these controls. > >>>> > >>>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > >>>> --- > >>>> > >>>> Documentation/DocBook/media/v4l/controls.xml | 34 ++++++++++++++++++++ > >>>> drivers/media/v4l2-core/v4l2-ctrls.c | 4 ++++ > >>>> include/uapi/linux/v4l2-controls.h | 4 ++++ > >>>> 3 files changed, 42 insertions(+) > >>>> > >>>> diff --git a/Documentation/DocBook/media/v4l/controls.xml > >>>> b/Documentation/DocBook/media/v4l/controls.xml index 47198ee..bf23994 > >>>> 100644 > >>>> --- a/Documentation/DocBook/media/v4l/controls.xml > >>>> +++ b/Documentation/DocBook/media/v4l/controls.xml > >>>> @@ -4677,6 +4677,40 @@ interface and may change in the future.</para> > >>>> conversion. > >>>> </entry> > >>>> </row> > >>>> + <row> > >>>> + <entry > >>>> spanname="id"><constant>V4L2_CID_TEST_PATTERN_RED</constant></entry> > >>>> + <entry>integer</entry> > >>>> + </row> > >>>> + <row> > >>>> + <entry spanname="descr">Test pattern red colour component. > >>>> + </entry> > >>>> + </row> > >>>> + <row> > >>>> + <entry > >>>> spanname="id"><constant>V4L2_CID_TEST_PATTERN_GREENR</constant></entry> > >>>> + <entry>integer</entry> > >>>> + </row> > >>>> + <row> > >>>> + <entry spanname="descr">Test pattern green (next to red) > >>>> + colour component. > >>> > >>> What about non-Bayer RGB sensors ? Should they use the GREENR or the > >>> GREENB control for the green component ? Or a different control ? > >> > >> A different one. It should be simply green. I could add it to the same > >> patch if you wish. > >> > >>> I'm wondering whether we shouldn't have a single test pattern color > >>> control and create a color type using Hans' complex controls API. > >> > >> A raw bayer four-pixel value, you mean? > > > > Yes. I'll let Hans comment on that. > > Why would you need the complex control API for that? It would fit in a s32, > and certainly in a s64. It wouldn't fit in a s32 when using more than 8 bits per component. s64 would be an option, until we reach 16 bits per component (or more than 4 components). > We have done something similar to this in the past (V4L2_CID_BG_COLOR). > > The main problem is that the interpretation of the s32 value has to be > clearly defined. And if different sensors might have different min/max > values for each component, then it becomes messy to use a single control. The interpretation would depend on both the sensor and the color format. > My feeling is that it is better to go with separate controls, one for each > component. What bothers me is that we'll need to add lots of controls, for each component. There's 4 controls for Bayer, one additional green control for RGB, 3 controls for YUV, ... That's already 8 controls to support the common Bayer/RGB/YUV formats. As colors can be used for different purposes (test pattern with possibly more than one color, background color, ...) that would increase the complexity beyond my comfort zone. -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 1/3] v4l: Add test pattern colour component controls 2014-06-03 12:21 ` Laurent Pinchart @ 2014-06-03 12:27 ` Sakari Ailus 2014-06-03 12:42 ` Hans Verkuil 1 sibling, 0 replies; 24+ messages in thread From: Sakari Ailus @ 2014-06-03 12:27 UTC (permalink / raw) To: Laurent Pinchart, Hans Verkuil; +Cc: linux-media Hi Laurent, Laurent Pinchart wrote: > Hi Hans, > > On Friday 30 May 2014 14:28:16 Hans Verkuil wrote: >> On 05/29/2014 05:01 PM, Laurent Pinchart wrote: >>> On Thursday 29 May 2014 17:58:59 Sakari Ailus wrote: >>>> Laurent Pinchart wrote: >>>>> On Thursday 29 May 2014 17:40:46 Sakari Ailus wrote: >>>>>> In many cases the test pattern has selectable values for each colour >>>>>> component. Implement controls for raw bayer components. Additional >>>>>> controls should be defined for colour components that are not covered >>>>>> by these controls. >>>>>> >>>>>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> >>>>>> --- >>>>>> >>>>>> Documentation/DocBook/media/v4l/controls.xml | 34 ++++++++++++++++++++ >>>>>> drivers/media/v4l2-core/v4l2-ctrls.c | 4 ++++ >>>>>> include/uapi/linux/v4l2-controls.h | 4 ++++ >>>>>> 3 files changed, 42 insertions(+) >>>>>> >>>>>> diff --git a/Documentation/DocBook/media/v4l/controls.xml >>>>>> b/Documentation/DocBook/media/v4l/controls.xml index 47198ee..bf23994 >>>>>> 100644 >>>>>> --- a/Documentation/DocBook/media/v4l/controls.xml >>>>>> +++ b/Documentation/DocBook/media/v4l/controls.xml >>>>>> @@ -4677,6 +4677,40 @@ interface and may change in the future.</para> >>>>>> conversion. >>>>>> </entry> >>>>>> </row> >>>>>> + <row> >>>>>> + <entry >>>>>> spanname="id"><constant>V4L2_CID_TEST_PATTERN_RED</constant></entry> >>>>>> + <entry>integer</entry> >>>>>> + </row> >>>>>> + <row> >>>>>> + <entry spanname="descr">Test pattern red colour component. >>>>>> + </entry> >>>>>> + </row> >>>>>> + <row> >>>>>> + <entry >>>>>> spanname="id"><constant>V4L2_CID_TEST_PATTERN_GREENR</constant></entry> >>>>>> + <entry>integer</entry> >>>>>> + </row> >>>>>> + <row> >>>>>> + <entry spanname="descr">Test pattern green (next to red) >>>>>> + colour component. >>>>> >>>>> What about non-Bayer RGB sensors ? Should they use the GREENR or the >>>>> GREENB control for the green component ? Or a different control ? >>>> >>>> A different one. It should be simply green. I could add it to the same >>>> patch if you wish. >>>> >>>>> I'm wondering whether we shouldn't have a single test pattern color >>>>> control and create a color type using Hans' complex controls API. >>>> >>>> A raw bayer four-pixel value, you mean? >>> >>> Yes. I'll let Hans comment on that. >> >> Why would you need the complex control API for that? It would fit in a s32, >> and certainly in a s64. > > It wouldn't fit in a s32 when using more than 8 bits per component. s64 would > be an option, until we reach 16 bits per component (or more than 4 > components). > >> We have done something similar to this in the past (V4L2_CID_BG_COLOR). >> >> The main problem is that the interpretation of the s32 value has to be >> clearly defined. And if different sensors might have different min/max >> values for each component, then it becomes messy to use a single control. > > The interpretation would depend on both the sensor and the color format. > >> My feeling is that it is better to go with separate controls, one for each >> component. > > What bothers me is that we'll need to add lots of controls, for each > component. There's 4 controls for Bayer, one additional green control for RGB, > 3 controls for YUV, ... That's already 8 controls to support the common > Bayer/RGB/YUV formats. As colors can be used for different purposes (test Three if you make colour controls, and three more control types. Something to consider as well is that these controls are commonly used by test programs and you'd need to implement support for each new colour specific control type in the test programs such as yavta. I'm not sure if implementing three new control types for that purpose only makes sense. Do you expect to see much use for such control types outside the test patterns? > pattern with possibly more than one color, background color, ...) that would > increase the complexity beyond my comfort zone. I think that where they exist, such, likely device specific, controls wouldn't make it to the list of standard controls anyway. The test patterns typically are quite simple after all. -- Kind regards, Sakari Ailus sakari.ailus@linux.intel.com ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 1/3] v4l: Add test pattern colour component controls 2014-06-03 12:21 ` Laurent Pinchart 2014-06-03 12:27 ` Sakari Ailus @ 2014-06-03 12:42 ` Hans Verkuil 1 sibling, 0 replies; 24+ messages in thread From: Hans Verkuil @ 2014-06-03 12:42 UTC (permalink / raw) To: Laurent Pinchart; +Cc: Sakari Ailus, linux-media On 06/03/14 14:21, Laurent Pinchart wrote: > Hi Hans, > > On Friday 30 May 2014 14:28:16 Hans Verkuil wrote: >> On 05/29/2014 05:01 PM, Laurent Pinchart wrote: >>> On Thursday 29 May 2014 17:58:59 Sakari Ailus wrote: >>>> Laurent Pinchart wrote: >>>>> On Thursday 29 May 2014 17:40:46 Sakari Ailus wrote: >>>>>> In many cases the test pattern has selectable values for each colour >>>>>> component. Implement controls for raw bayer components. Additional >>>>>> controls should be defined for colour components that are not covered >>>>>> by these controls. >>>>>> >>>>>> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> >>>>>> --- >>>>>> >>>>>> Documentation/DocBook/media/v4l/controls.xml | 34 ++++++++++++++++++++ >>>>>> drivers/media/v4l2-core/v4l2-ctrls.c | 4 ++++ >>>>>> include/uapi/linux/v4l2-controls.h | 4 ++++ >>>>>> 3 files changed, 42 insertions(+) >>>>>> >>>>>> diff --git a/Documentation/DocBook/media/v4l/controls.xml >>>>>> b/Documentation/DocBook/media/v4l/controls.xml index 47198ee..bf23994 >>>>>> 100644 >>>>>> --- a/Documentation/DocBook/media/v4l/controls.xml >>>>>> +++ b/Documentation/DocBook/media/v4l/controls.xml >>>>>> @@ -4677,6 +4677,40 @@ interface and may change in the future.</para> >>>>>> conversion. >>>>>> </entry> >>>>>> </row> >>>>>> + <row> >>>>>> + <entry >>>>>> spanname="id"><constant>V4L2_CID_TEST_PATTERN_RED</constant></entry> >>>>>> + <entry>integer</entry> >>>>>> + </row> >>>>>> + <row> >>>>>> + <entry spanname="descr">Test pattern red colour component. >>>>>> + </entry> >>>>>> + </row> >>>>>> + <row> >>>>>> + <entry >>>>>> spanname="id"><constant>V4L2_CID_TEST_PATTERN_GREENR</constant></entry> >>>>>> + <entry>integer</entry> >>>>>> + </row> >>>>>> + <row> >>>>>> + <entry spanname="descr">Test pattern green (next to red) >>>>>> + colour component. >>>>> >>>>> What about non-Bayer RGB sensors ? Should they use the GREENR or the >>>>> GREENB control for the green component ? Or a different control ? >>>> >>>> A different one. It should be simply green. I could add it to the same >>>> patch if you wish. >>>> >>>>> I'm wondering whether we shouldn't have a single test pattern color >>>>> control and create a color type using Hans' complex controls API. >>>> >>>> A raw bayer four-pixel value, you mean? >>> >>> Yes. I'll let Hans comment on that. >> >> Why would you need the complex control API for that? It would fit in a s32, >> and certainly in a s64. > > It wouldn't fit in a s32 when using more than 8 bits per component. s64 would > be an option, until we reach 16 bits per component (or more than 4 > components). > >> We have done something similar to this in the past (V4L2_CID_BG_COLOR). >> >> The main problem is that the interpretation of the s32 value has to be >> clearly defined. And if different sensors might have different min/max >> values for each component, then it becomes messy to use a single control. > > The interpretation would depend on both the sensor and the color format. > >> My feeling is that it is better to go with separate controls, one for each >> component. > > What bothers me is that we'll need to add lots of controls, for each > component. There's 4 controls for Bayer, one additional green control for RGB, > 3 controls for YUV, ... That's already 8 controls to support the common > Bayer/RGB/YUV formats. As colors can be used for different purposes (test > pattern with possibly more than one color, background color, ...) that would > increase the complexity beyond my comfort zone. Why would this be any better with compound type controls? In fact, I think it is worse: simple controls will show up in qv4l2 and v4l2-ctl and are easy to set from the GUI/command line. To set compound controls you will need to write custom code for them. You also only see the controls that the driver actually needs. So while there may be X component controls, a driver for a Bayer sensor will only use four of them. If you want to support a driver that has highly specialized test pattern support, then I'm leaning towards a custom ioctl for that driver. Regards, Hans ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 1/3] v4l: Add test pattern colour component controls 2014-05-29 14:40 ` [PATCH v3 1/3] v4l: Add test pattern colour component controls Sakari Ailus 2014-05-29 14:47 ` Laurent Pinchart @ 2014-06-10 12:15 ` Hans Verkuil 1 sibling, 0 replies; 24+ messages in thread From: Hans Verkuil @ 2014-06-10 12:15 UTC (permalink / raw) To: Sakari Ailus, linux-media; +Cc: laurent.pinchart On 05/29/14 16:40, Sakari Ailus wrote: > In many cases the test pattern has selectable values for each colour > component. Implement controls for raw bayer components. Additional controls > should be defined for colour components that are not covered by these > controls. > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> Acked-by: Hans Verkuil <hans.verkuil@cisco.com> Thanks! Hans > --- > Documentation/DocBook/media/v4l/controls.xml | 34 ++++++++++++++++++++++++++++ > drivers/media/v4l2-core/v4l2-ctrls.c | 4 ++++ > include/uapi/linux/v4l2-controls.h | 4 ++++ > 3 files changed, 42 insertions(+) > > diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml > index 47198ee..bf23994 100644 > --- a/Documentation/DocBook/media/v4l/controls.xml > +++ b/Documentation/DocBook/media/v4l/controls.xml > @@ -4677,6 +4677,40 @@ interface and may change in the future.</para> > conversion. > </entry> > </row> > + <row> > + <entry spanname="id"><constant>V4L2_CID_TEST_PATTERN_RED</constant></entry> > + <entry>integer</entry> > + </row> > + <row> > + <entry spanname="descr">Test pattern red colour component. > + </entry> > + </row> > + <row> > + <entry spanname="id"><constant>V4L2_CID_TEST_PATTERN_GREENR</constant></entry> > + <entry>integer</entry> > + </row> > + <row> > + <entry spanname="descr">Test pattern green (next to red) > + colour component. > + </entry> > + </row> > + <row> > + <entry spanname="id"><constant>V4L2_CID_TEST_PATTERN_BLUE</constant></entry> > + <entry>integer</entry> > + </row> > + <row> > + <entry spanname="descr">Test pattern blue colour component. > + </entry> > + </row> > + <row> > + <entry spanname="id"><constant>V4L2_CID_TEST_PATTERN_GREENB</constant></entry> > + <entry>integer</entry> > + </row> > + <row> > + <entry spanname="descr">Test pattern green (next to blue) > + colour component. > + </entry> > + </row> > <row><entry></entry></row> > </tbody> > </tgroup> > diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c > index 55c6832..a4104a7 100644 > --- a/drivers/media/v4l2-core/v4l2-ctrls.c > +++ b/drivers/media/v4l2-core/v4l2-ctrls.c > @@ -839,6 +839,10 @@ const char *v4l2_ctrl_get_name(u32 id) > case V4L2_CID_VBLANK: return "Vertical Blanking"; > case V4L2_CID_HBLANK: return "Horizontal Blanking"; > case V4L2_CID_ANALOGUE_GAIN: return "Analogue Gain"; > + case V4L2_CID_TEST_PATTERN_RED: return "Red Pixel Value"; > + case V4L2_CID_TEST_PATTERN_GREENR: return "Green (Red) Pixel Value"; > + case V4L2_CID_TEST_PATTERN_BLUE: return "Blue Pixel Value"; > + case V4L2_CID_TEST_PATTERN_GREENB: return "Green (Blue) Pixel Value"; > > /* Image processing controls */ > case V4L2_CID_IMAGE_PROC_CLASS: return "Image Processing Controls"; > diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h > index 2ac5597..5c55a19 100644 > --- a/include/uapi/linux/v4l2-controls.h > +++ b/include/uapi/linux/v4l2-controls.h > @@ -855,6 +855,10 @@ enum v4l2_jpeg_chroma_subsampling { > #define V4L2_CID_VBLANK (V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 1) > #define V4L2_CID_HBLANK (V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 2) > #define V4L2_CID_ANALOGUE_GAIN (V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 3) > +#define V4L2_CID_TEST_PATTERN_RED (V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 4) > +#define V4L2_CID_TEST_PATTERN_GREENR (V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 5) > +#define V4L2_CID_TEST_PATTERN_BLUE (V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 6) > +#define V4L2_CID_TEST_PATTERN_GREENB (V4L2_CID_IMAGE_SOURCE_CLASS_BASE + 7) > > > /* Image processing controls */ > ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3 2/3] smiapp: Add driver-specific test pattern menu item definitions 2014-05-29 14:40 [PATCH v3 0/3] smiapp test pattern support Sakari Ailus 2014-05-29 14:40 ` [PATCH v3 1/3] v4l: Add test pattern colour component controls Sakari Ailus @ 2014-05-29 14:40 ` Sakari Ailus 2014-05-29 14:48 ` Laurent Pinchart ` (3 more replies) 2014-05-29 14:40 ` [PATCH v3 3/3] smiapp: Implement the test pattern control Sakari Ailus 2 siblings, 4 replies; 24+ messages in thread From: Sakari Ailus @ 2014-05-29 14:40 UTC (permalink / raw) To: linux-media; +Cc: laurent.pinchart, hverkuil Add numeric definitions for menu items used in the smiapp driver's test pattern menu. Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> --- include/uapi/linux/smiapp.h | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 include/uapi/linux/smiapp.h diff --git a/include/uapi/linux/smiapp.h b/include/uapi/linux/smiapp.h new file mode 100644 index 0000000..53938f4 --- /dev/null +++ b/include/uapi/linux/smiapp.h @@ -0,0 +1,29 @@ +/* + * include/uapi/linux/smiapp.h + * + * Generic driver for SMIA/SMIA++ compliant camera modules + * + * Copyright (C) 2014 Intel Corporation + * Contact: Sakari Ailus <sakari.ailus@iki.fi> + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + */ + +#ifndef __UAPI_LINUX_SMIAPP_H_ +#define __UAPI_LINUX_SMIAPP_H_ + +#define V4L2_SMIAPP_TEST_PATTERN_MODE_DISABLED 0 +#define V4L2_SMIAPP_TEST_PATTERN_MODE_SOLID_COLOUR 1 +#define V4L2_SMIAPP_TEST_PATTERN_MODE_COLOUR_BARS 2 +#define V4L2_SMIAPP_TEST_PATTERN_MODE_COLOUR_BARS_GREY 3 +#define V4L2_SMIAPP_TEST_PATTERN_MODE_PN9 4 + +#endif /* __UAPI_LINUX_SMIAPP_H_ */ -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v3 2/3] smiapp: Add driver-specific test pattern menu item definitions 2014-05-29 14:40 ` [PATCH v3 2/3] smiapp: Add driver-specific test pattern menu item definitions Sakari Ailus @ 2014-05-29 14:48 ` Laurent Pinchart 2014-05-29 15:08 ` Sakari Ailus 2014-06-10 12:16 ` Hans Verkuil ` (2 subsequent siblings) 3 siblings, 1 reply; 24+ messages in thread From: Laurent Pinchart @ 2014-05-29 14:48 UTC (permalink / raw) To: Sakari Ailus; +Cc: linux-media, hverkuil On Thursday 29 May 2014 17:40:47 Sakari Ailus wrote: > Add numeric definitions for menu items used in the smiapp driver's test > pattern menu. > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > include/uapi/linux/smiapp.h | 29 +++++++++++++++++++++++++++++ > 1 file changed, 29 insertions(+) > create mode 100644 include/uapi/linux/smiapp.h > > diff --git a/include/uapi/linux/smiapp.h b/include/uapi/linux/smiapp.h > new file mode 100644 > index 0000000..53938f4 > --- /dev/null > +++ b/include/uapi/linux/smiapp.h > @@ -0,0 +1,29 @@ > +/* > + * include/uapi/linux/smiapp.h > + * > + * Generic driver for SMIA/SMIA++ compliant camera modules > + * > + * Copyright (C) 2014 Intel Corporation > + * Contact: Sakari Ailus <sakari.ailus@iki.fi> > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License > + * version 2 as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, but > + * WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * General Public License for more details. > + * > + */ > + > +#ifndef __UAPI_LINUX_SMIAPP_H_ > +#define __UAPI_LINUX_SMIAPP_H_ > + > +#define V4L2_SMIAPP_TEST_PATTERN_MODE_DISABLED 0 > +#define V4L2_SMIAPP_TEST_PATTERN_MODE_SOLID_COLOUR 1 > +#define V4L2_SMIAPP_TEST_PATTERN_MODE_COLOUR_BARS 2 > +#define V4L2_SMIAPP_TEST_PATTERN_MODE_COLOUR_BARS_GREY 3 > +#define V4L2_SMIAPP_TEST_PATTERN_MODE_PN9 4 Out of curiosity, what's PN9 ? > + > +#endif /* __UAPI_LINUX_SMIAPP_H_ */ -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 2/3] smiapp: Add driver-specific test pattern menu item definitions 2014-05-29 14:48 ` Laurent Pinchart @ 2014-05-29 15:08 ` Sakari Ailus 0 siblings, 0 replies; 24+ messages in thread From: Sakari Ailus @ 2014-05-29 15:08 UTC (permalink / raw) To: Laurent Pinchart; +Cc: linux-media, hverkuil Laurent Pinchart wrote: >> diff --git a/include/uapi/linux/smiapp.h b/include/uapi/linux/smiapp.h >> new file mode 100644 >> index 0000000..53938f4 >> --- /dev/null >> +++ b/include/uapi/linux/smiapp.h >> @@ -0,0 +1,29 @@ >> +/* >> + * include/uapi/linux/smiapp.h >> + * >> + * Generic driver for SMIA/SMIA++ compliant camera modules >> + * >> + * Copyright (C) 2014 Intel Corporation >> + * Contact: Sakari Ailus <sakari.ailus@iki.fi> >> + * >> + * This program is free software; you can redistribute it and/or >> + * modify it under the terms of the GNU General Public License >> + * version 2 as published by the Free Software Foundation. >> + * >> + * This program is distributed in the hope that it will be useful, but >> + * WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU >> + * General Public License for more details. >> + * >> + */ >> + >> +#ifndef __UAPI_LINUX_SMIAPP_H_ >> +#define __UAPI_LINUX_SMIAPP_H_ >> + >> +#define V4L2_SMIAPP_TEST_PATTERN_MODE_DISABLED 0 >> +#define V4L2_SMIAPP_TEST_PATTERN_MODE_SOLID_COLOUR 1 >> +#define V4L2_SMIAPP_TEST_PATTERN_MODE_COLOUR_BARS 2 >> +#define V4L2_SMIAPP_TEST_PATTERN_MODE_COLOUR_BARS_GREY 3 >> +#define V4L2_SMIAPP_TEST_PATTERN_MODE_PN9 4 > > Out of curiosity, what's PN9 ? It's a sequence of pseudo-random binary numbers, e.g.: <URL:http://en.wikipedia.org/wiki/Pseudorandom_binary_sequence> 9 is the order of the polynomial. -- Sakari Ailus sakari.ailus@linux.intel.com ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 2/3] smiapp: Add driver-specific test pattern menu item definitions 2014-05-29 14:40 ` [PATCH v3 2/3] smiapp: Add driver-specific test pattern menu item definitions Sakari Ailus 2014-05-29 14:48 ` Laurent Pinchart @ 2014-06-10 12:16 ` Hans Verkuil 2014-06-10 15:57 ` Prabhakar Lad 2014-06-11 6:36 ` [PATCH v3.1 2/4] " Sakari Ailus 3 siblings, 0 replies; 24+ messages in thread From: Hans Verkuil @ 2014-06-10 12:16 UTC (permalink / raw) To: Sakari Ailus, linux-media; +Cc: laurent.pinchart On 05/29/14 16:40, Sakari Ailus wrote: > Add numeric definitions for menu items used in the smiapp driver's test > pattern menu. > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> Acked-by: Hans Verkuil <hans.verkuil@cisco.com> > --- > include/uapi/linux/smiapp.h | 29 +++++++++++++++++++++++++++++ > 1 file changed, 29 insertions(+) > create mode 100644 include/uapi/linux/smiapp.h > > diff --git a/include/uapi/linux/smiapp.h b/include/uapi/linux/smiapp.h > new file mode 100644 > index 0000000..53938f4 > --- /dev/null > +++ b/include/uapi/linux/smiapp.h > @@ -0,0 +1,29 @@ > +/* > + * include/uapi/linux/smiapp.h > + * > + * Generic driver for SMIA/SMIA++ compliant camera modules > + * > + * Copyright (C) 2014 Intel Corporation > + * Contact: Sakari Ailus <sakari.ailus@iki.fi> > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License > + * version 2 as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, but > + * WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * General Public License for more details. > + * > + */ > + > +#ifndef __UAPI_LINUX_SMIAPP_H_ > +#define __UAPI_LINUX_SMIAPP_H_ > + > +#define V4L2_SMIAPP_TEST_PATTERN_MODE_DISABLED 0 > +#define V4L2_SMIAPP_TEST_PATTERN_MODE_SOLID_COLOUR 1 > +#define V4L2_SMIAPP_TEST_PATTERN_MODE_COLOUR_BARS 2 > +#define V4L2_SMIAPP_TEST_PATTERN_MODE_COLOUR_BARS_GREY 3 > +#define V4L2_SMIAPP_TEST_PATTERN_MODE_PN9 4 > + > +#endif /* __UAPI_LINUX_SMIAPP_H_ */ > ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 2/3] smiapp: Add driver-specific test pattern menu item definitions 2014-05-29 14:40 ` [PATCH v3 2/3] smiapp: Add driver-specific test pattern menu item definitions Sakari Ailus 2014-05-29 14:48 ` Laurent Pinchart 2014-06-10 12:16 ` Hans Verkuil @ 2014-06-10 15:57 ` Prabhakar Lad 2014-06-11 6:34 ` Sakari Ailus 2014-06-11 6:36 ` [PATCH v3.1 2/4] " Sakari Ailus 3 siblings, 1 reply; 24+ messages in thread From: Prabhakar Lad @ 2014-06-10 15:57 UTC (permalink / raw) To: Sakari Ailus; +Cc: linux-media, Laurent Pinchart, Hans Verkuil Hi Sakari, On Thu, May 29, 2014 at 3:40 PM, Sakari Ailus <sakari.ailus@linux.intel.com> wrote: > Add numeric definitions for menu items used in the smiapp driver's test > pattern menu. > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > --- > include/uapi/linux/smiapp.h | 29 +++++++++++++++++++++++++++++ Don't you need to add an entry in Kbuild file for this ? Regards, --Prabhakar Lad > 1 file changed, 29 insertions(+) > create mode 100644 include/uapi/linux/smiapp.h > > diff --git a/include/uapi/linux/smiapp.h b/include/uapi/linux/smiapp.h > new file mode 100644 > index 0000000..53938f4 > --- /dev/null > +++ b/include/uapi/linux/smiapp.h > @@ -0,0 +1,29 @@ > +/* > + * include/uapi/linux/smiapp.h > + * > + * Generic driver for SMIA/SMIA++ compliant camera modules > + * > + * Copyright (C) 2014 Intel Corporation > + * Contact: Sakari Ailus <sakari.ailus@iki.fi> > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License > + * version 2 as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, but > + * WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * General Public License for more details. > + * > + */ > + > +#ifndef __UAPI_LINUX_SMIAPP_H_ > +#define __UAPI_LINUX_SMIAPP_H_ > + > +#define V4L2_SMIAPP_TEST_PATTERN_MODE_DISABLED 0 > +#define V4L2_SMIAPP_TEST_PATTERN_MODE_SOLID_COLOUR 1 > +#define V4L2_SMIAPP_TEST_PATTERN_MODE_COLOUR_BARS 2 > +#define V4L2_SMIAPP_TEST_PATTERN_MODE_COLOUR_BARS_GREY 3 > +#define V4L2_SMIAPP_TEST_PATTERN_MODE_PN9 4 > + > +#endif /* __UAPI_LINUX_SMIAPP_H_ */ > -- > 1.8.3.2 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-media" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 2/3] smiapp: Add driver-specific test pattern menu item definitions 2014-06-10 15:57 ` Prabhakar Lad @ 2014-06-11 6:34 ` Sakari Ailus 0 siblings, 0 replies; 24+ messages in thread From: Sakari Ailus @ 2014-06-11 6:34 UTC (permalink / raw) To: Prabhakar Lad; +Cc: linux-media, Laurent Pinchart, Hans Verkuil Hi Prabhakar, Thanks for the review! :-) Prabhakar Lad wrote: > Hi Sakari, > > On Thu, May 29, 2014 at 3:40 PM, Sakari Ailus > <sakari.ailus@linux.intel.com> wrote: >> Add numeric definitions for menu items used in the smiapp driver's test >> pattern menu. >> >> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> >> --- >> include/uapi/linux/smiapp.h | 29 +++++++++++++++++++++++++++++ > > Don't you need to add an entry in Kbuild file for this ? Good poing. I'll send v3.1 for this one as well. -- Sakari Ailus sakari.ailus@linux.intel.com ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3.1 2/4] smiapp: Add driver-specific test pattern menu item definitions 2014-05-29 14:40 ` [PATCH v3 2/3] smiapp: Add driver-specific test pattern menu item definitions Sakari Ailus ` (2 preceding siblings ...) 2014-06-10 15:57 ` Prabhakar Lad @ 2014-06-11 6:36 ` Sakari Ailus 2014-06-11 7:58 ` Prabhakar Lad 3 siblings, 1 reply; 24+ messages in thread From: Sakari Ailus @ 2014-06-11 6:36 UTC (permalink / raw) To: linux-media; +Cc: prabhakar.csengg, hverkuil, laurent.pinchart Add numeric definitions for menu items used in the smiapp driver's test pattern menu. Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- since v3: - Add Kbuild entry for the header include/uapi/linux/Kbuild | 1 + include/uapi/linux/smiapp.h | 29 +++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+) create mode 100644 include/uapi/linux/smiapp.h diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild index 6929571..a3ee163 100644 --- a/include/uapi/linux/Kbuild +++ b/include/uapi/linux/Kbuild @@ -352,6 +352,7 @@ header-y += serio.h header-y += shm.h header-y += signal.h header-y += signalfd.h +header-y += smiapp.h header-y += snmp.h header-y += sock_diag.h header-y += socket.h diff --git a/include/uapi/linux/smiapp.h b/include/uapi/linux/smiapp.h new file mode 100644 index 0000000..53938f4 --- /dev/null +++ b/include/uapi/linux/smiapp.h @@ -0,0 +1,29 @@ +/* + * include/uapi/linux/smiapp.h + * + * Generic driver for SMIA/SMIA++ compliant camera modules + * + * Copyright (C) 2014 Intel Corporation + * Contact: Sakari Ailus <sakari.ailus@iki.fi> + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + */ + +#ifndef __UAPI_LINUX_SMIAPP_H_ +#define __UAPI_LINUX_SMIAPP_H_ + +#define V4L2_SMIAPP_TEST_PATTERN_MODE_DISABLED 0 +#define V4L2_SMIAPP_TEST_PATTERN_MODE_SOLID_COLOUR 1 +#define V4L2_SMIAPP_TEST_PATTERN_MODE_COLOUR_BARS 2 +#define V4L2_SMIAPP_TEST_PATTERN_MODE_COLOUR_BARS_GREY 3 +#define V4L2_SMIAPP_TEST_PATTERN_MODE_PN9 4 + +#endif /* __UAPI_LINUX_SMIAPP_H_ */ -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v3.1 2/4] smiapp: Add driver-specific test pattern menu item definitions 2014-06-11 6:36 ` [PATCH v3.1 2/4] " Sakari Ailus @ 2014-06-11 7:58 ` Prabhakar Lad 0 siblings, 0 replies; 24+ messages in thread From: Prabhakar Lad @ 2014-06-11 7:58 UTC (permalink / raw) To: Sakari Ailus; +Cc: linux-media, Hans Verkuil, Laurent Pinchart On Wed, Jun 11, 2014 at 7:36 AM, Sakari Ailus <sakari.ailus@linux.intel.com> wrote: > Add numeric definitions for menu items used in the smiapp driver's test > pattern menu. > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Acked-by: Lad, Prabhakar <prabhakar.csengg@gmail.com> Regards, --Prabhakar Lad > --- > since v3: > - Add Kbuild entry for the header > > include/uapi/linux/Kbuild | 1 + > include/uapi/linux/smiapp.h | 29 +++++++++++++++++++++++++++++ > 2 files changed, 30 insertions(+) > create mode 100644 include/uapi/linux/smiapp.h > > diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild > index 6929571..a3ee163 100644 > --- a/include/uapi/linux/Kbuild > +++ b/include/uapi/linux/Kbuild > @@ -352,6 +352,7 @@ header-y += serio.h > header-y += shm.h > header-y += signal.h > header-y += signalfd.h > +header-y += smiapp.h > header-y += snmp.h > header-y += sock_diag.h > header-y += socket.h > diff --git a/include/uapi/linux/smiapp.h b/include/uapi/linux/smiapp.h > new file mode 100644 > index 0000000..53938f4 > --- /dev/null > +++ b/include/uapi/linux/smiapp.h > @@ -0,0 +1,29 @@ > +/* > + * include/uapi/linux/smiapp.h > + * > + * Generic driver for SMIA/SMIA++ compliant camera modules > + * > + * Copyright (C) 2014 Intel Corporation > + * Contact: Sakari Ailus <sakari.ailus@iki.fi> > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License > + * version 2 as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, but > + * WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * General Public License for more details. > + * > + */ > + > +#ifndef __UAPI_LINUX_SMIAPP_H_ > +#define __UAPI_LINUX_SMIAPP_H_ > + > +#define V4L2_SMIAPP_TEST_PATTERN_MODE_DISABLED 0 > +#define V4L2_SMIAPP_TEST_PATTERN_MODE_SOLID_COLOUR 1 > +#define V4L2_SMIAPP_TEST_PATTERN_MODE_COLOUR_BARS 2 > +#define V4L2_SMIAPP_TEST_PATTERN_MODE_COLOUR_BARS_GREY 3 > +#define V4L2_SMIAPP_TEST_PATTERN_MODE_PN9 4 > + > +#endif /* __UAPI_LINUX_SMIAPP_H_ */ > -- > 1.8.3.2 > ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3 3/3] smiapp: Implement the test pattern control 2014-05-29 14:40 [PATCH v3 0/3] smiapp test pattern support Sakari Ailus 2014-05-29 14:40 ` [PATCH v3 1/3] v4l: Add test pattern colour component controls Sakari Ailus 2014-05-29 14:40 ` [PATCH v3 2/3] smiapp: Add driver-specific test pattern menu item definitions Sakari Ailus @ 2014-05-29 14:40 ` Sakari Ailus 2014-05-29 14:54 ` Laurent Pinchart 2014-05-29 15:16 ` [PATCH v3.1 " Sakari Ailus 2 siblings, 2 replies; 24+ messages in thread From: Sakari Ailus @ 2014-05-29 14:40 UTC (permalink / raw) To: linux-media; +Cc: laurent.pinchart, hverkuil Add support for the V4L2_CID_TEST_PATTERN control. When the solid colour mode is selected, additional controls become available for setting the solid four solid colour components. Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> Acked-by: Hans Verkuil <hans.verkuil@cisco.com> --- drivers/media/i2c/smiapp/smiapp-core.c | 84 ++++++++++++++++++++++++++++++++-- drivers/media/i2c/smiapp/smiapp.h | 4 ++ 2 files changed, 84 insertions(+), 4 deletions(-) diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c index 446c82c..dc82adb 100644 --- a/drivers/media/i2c/smiapp/smiapp-core.c +++ b/drivers/media/i2c/smiapp/smiapp-core.c @@ -404,6 +404,16 @@ static void smiapp_update_mbus_formats(struct smiapp_sensor *sensor) pixel_order_str[pixel_order]); } +static const char * const smiapp_test_patterns[] = { + "Disabled", + "Solid Colour", + "Eight Vertical Colour Bars", + "Colour Bars With Fade to Grey", + "Pseudorandom Sequence (PN9)", +}; + +static const struct v4l2_ctrl_ops smiapp_ctrl_ops; + static int smiapp_set_ctrl(struct v4l2_ctrl *ctrl) { struct smiapp_sensor *sensor = @@ -477,6 +487,35 @@ static int smiapp_set_ctrl(struct v4l2_ctrl *ctrl) return smiapp_pll_update(sensor); + case V4L2_CID_TEST_PATTERN: { + unsigned int i; + + for (i = 0; i < ARRAY_SIZE(sensor->test_data); i++) + v4l2_ctrl_activate( + sensor->test_data[i], + ctrl->val == + V4L2_SMIAPP_TEST_PATTERN_MODE_SOLID_COLOUR); + + return smiapp_write( + sensor, SMIAPP_REG_U16_TEST_PATTERN_MODE, ctrl->val); + } + + case V4L2_CID_TEST_PATTERN_RED: + return smiapp_write( + sensor, SMIAPP_REG_U16_TEST_DATA_RED, ctrl->val); + + case V4L2_CID_TEST_PATTERN_GREENR: + return smiapp_write( + sensor, SMIAPP_REG_U16_TEST_DATA_GREENR, ctrl->val); + + case V4L2_CID_TEST_PATTERN_BLUE: + return smiapp_write( + sensor, SMIAPP_REG_U16_TEST_DATA_BLUE, ctrl->val); + + case V4L2_CID_TEST_PATTERN_GREENB: + return smiapp_write( + sensor, SMIAPP_REG_U16_TEST_DATA_GREENB, ctrl->val); + default: return -EINVAL; } @@ -489,10 +528,10 @@ static const struct v4l2_ctrl_ops smiapp_ctrl_ops = { static int smiapp_init_controls(struct smiapp_sensor *sensor) { struct i2c_client *client = v4l2_get_subdevdata(&sensor->src->sd); - unsigned int max; + unsigned int max, i; int rval; - rval = v4l2_ctrl_handler_init(&sensor->pixel_array->ctrl_handler, 7); + rval = v4l2_ctrl_handler_init(&sensor->pixel_array->ctrl_handler, 12); if (rval) return rval; sensor->pixel_array->ctrl_handler.lock = &sensor->mutex; @@ -535,6 +574,18 @@ static int smiapp_init_controls(struct smiapp_sensor *sensor) &sensor->pixel_array->ctrl_handler, &smiapp_ctrl_ops, V4L2_CID_PIXEL_RATE, 0, 0, 1, 0); + v4l2_ctrl_new_std_menu_items(&sensor->pixel_array->ctrl_handler, + &smiapp_ctrl_ops, V4L2_CID_TEST_PATTERN, + ARRAY_SIZE(smiapp_test_patterns) - 1, + 0, 0, smiapp_test_patterns); + + for (i = 0; i < ARRAY_SIZE(sensor->test_data); i++) + sensor->test_data[i] = + v4l2_ctrl_new_std(&sensor->pixel_array->ctrl_handler, + &smiapp_ctrl_ops, + V4L2_CID_TEST_PATTERN_RED + i, + 0, 0, 1, 0); + if (sensor->pixel_array->ctrl_handler.error) { dev_err(&client->dev, "pixel array controls initialization failed (%d)\n", @@ -543,6 +594,14 @@ static int smiapp_init_controls(struct smiapp_sensor *sensor) goto error; } + for (i = 0; i < ARRAY_SIZE(sensor->test_data); i++) { + struct v4l2_ctrl *ctrl = sensor->test_data[i]; + + ctrl->maximum = + ctrl->default_value = + ctrl->cur.val = (1 << sensor->csi_format->width) - 1; + } + sensor->pixel_array->sd.ctrl_handler = &sensor->pixel_array->ctrl_handler; @@ -1670,17 +1729,34 @@ static int smiapp_set_format(struct v4l2_subdev *subdev, if (fmt->pad == ssd->source_pad) { u32 code = fmt->format.code; int rval = __smiapp_get_format(subdev, fh, fmt); + bool range_changed = false; + unsigned int i; if (!rval && subdev == &sensor->src->sd) { const struct smiapp_csi_data_format *csi_format = smiapp_validate_csi_data_format(sensor, code); - if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) + + if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) { + if (csi_format->width != + sensor->csi_format->width) + range_changed = true; + sensor->csi_format = csi_format; + } + fmt->format.code = csi_format->code; } mutex_unlock(&sensor->mutex); - return rval; + if (rval || !range_changed) + return rval; + + for (i = 0; i < ARRAY_SIZE(sensor->test_data); i++) + v4l2_ctrl_modify_range( + sensor->test_data[i], + 0, (1 << sensor->csi_format->width) - 1, 1, 0); + + return 0; } /* Sink pad. Width and height are changeable here. */ diff --git a/drivers/media/i2c/smiapp/smiapp.h b/drivers/media/i2c/smiapp/smiapp.h index 7cc5aae..874b49f 100644 --- a/drivers/media/i2c/smiapp/smiapp.h +++ b/drivers/media/i2c/smiapp/smiapp.h @@ -54,6 +54,8 @@ (1000 + (SMIAPP_RESET_DELAY_CLOCKS * 1000 \ + (clk) / 1000 - 1) / ((clk) / 1000)) +#define SMIAPP_COLOUR_COMPONENTS 4 + #include "smiapp-limits.h" struct smiapp_quirk; @@ -241,6 +243,8 @@ struct smiapp_sensor { /* src controls */ struct v4l2_ctrl *link_freq; struct v4l2_ctrl *pixel_rate_csi; + /* test pattern colour components */ + struct v4l2_ctrl *test_data[SMIAPP_COLOUR_COMPONENTS]; }; #define to_smiapp_subdev(_sd) \ -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v3 3/3] smiapp: Implement the test pattern control 2014-05-29 14:40 ` [PATCH v3 3/3] smiapp: Implement the test pattern control Sakari Ailus @ 2014-05-29 14:54 ` Laurent Pinchart 2014-05-29 15:01 ` Sakari Ailus 2014-05-29 15:14 ` Sakari Ailus 2014-05-29 15:16 ` [PATCH v3.1 " Sakari Ailus 1 sibling, 2 replies; 24+ messages in thread From: Laurent Pinchart @ 2014-05-29 14:54 UTC (permalink / raw) To: Sakari Ailus; +Cc: linux-media, hverkuil Hi Sakari, Thank you for the patch. On Thursday 29 May 2014 17:40:48 Sakari Ailus wrote: > Add support for the V4L2_CID_TEST_PATTERN control. When the solid colour > mode is selected, additional controls become available for setting the > solid four solid colour components. > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > Acked-by: Hans Verkuil <hans.verkuil@cisco.com> > --- > drivers/media/i2c/smiapp/smiapp-core.c | 84 +++++++++++++++++++++++++++++-- > drivers/media/i2c/smiapp/smiapp.h | 4 ++ > 2 files changed, 84 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/i2c/smiapp/smiapp-core.c > b/drivers/media/i2c/smiapp/smiapp-core.c index 446c82c..dc82adb 100644 > --- a/drivers/media/i2c/smiapp/smiapp-core.c > +++ b/drivers/media/i2c/smiapp/smiapp-core.c > @@ -404,6 +404,16 @@ static void smiapp_update_mbus_formats(struct > smiapp_sensor *sensor) pixel_order_str[pixel_order]); > } > > +static const char * const smiapp_test_patterns[] = { > + "Disabled", > + "Solid Colour", > + "Eight Vertical Colour Bars", > + "Colour Bars With Fade to Grey", > + "Pseudorandom Sequence (PN9)", > +}; > + > +static const struct v4l2_ctrl_ops smiapp_ctrl_ops; Is this needed ? > + > static int smiapp_set_ctrl(struct v4l2_ctrl *ctrl) > { > struct smiapp_sensor *sensor = > @@ -477,6 +487,35 @@ static int smiapp_set_ctrl(struct v4l2_ctrl *ctrl) > > return smiapp_pll_update(sensor); > > + case V4L2_CID_TEST_PATTERN: { > + unsigned int i; > + > + for (i = 0; i < ARRAY_SIZE(sensor->test_data); i++) > + v4l2_ctrl_activate( > + sensor->test_data[i], > + ctrl->val == > + V4L2_SMIAPP_TEST_PATTERN_MODE_SOLID_COLOUR); > + > + return smiapp_write( > + sensor, SMIAPP_REG_U16_TEST_PATTERN_MODE, ctrl->val); > + } > + > + case V4L2_CID_TEST_PATTERN_RED: > + return smiapp_write( > + sensor, SMIAPP_REG_U16_TEST_DATA_RED, ctrl->val); > + > + case V4L2_CID_TEST_PATTERN_GREENR: > + return smiapp_write( > + sensor, SMIAPP_REG_U16_TEST_DATA_GREENR, ctrl->val); > + > + case V4L2_CID_TEST_PATTERN_BLUE: > + return smiapp_write( > + sensor, SMIAPP_REG_U16_TEST_DATA_BLUE, ctrl->val); > + > + case V4L2_CID_TEST_PATTERN_GREENB: > + return smiapp_write( > + sensor, SMIAPP_REG_U16_TEST_DATA_GREENB, ctrl->val); > + > default: > return -EINVAL; > } > @@ -489,10 +528,10 @@ static const struct v4l2_ctrl_ops smiapp_ctrl_ops = { > static int smiapp_init_controls(struct smiapp_sensor *sensor) > { > struct i2c_client *client = v4l2_get_subdevdata(&sensor->src->sd); > - unsigned int max; > + unsigned int max, i; > int rval; > > - rval = v4l2_ctrl_handler_init(&sensor->pixel_array->ctrl_handler, 7); > + rval = v4l2_ctrl_handler_init(&sensor->pixel_array->ctrl_handler, 12); > if (rval) > return rval; > sensor->pixel_array->ctrl_handler.lock = &sensor->mutex; > @@ -535,6 +574,18 @@ static int smiapp_init_controls(struct smiapp_sensor > *sensor) &sensor->pixel_array->ctrl_handler, &smiapp_ctrl_ops, > V4L2_CID_PIXEL_RATE, 0, 0, 1, 0); > > + v4l2_ctrl_new_std_menu_items(&sensor->pixel_array->ctrl_handler, > + &smiapp_ctrl_ops, V4L2_CID_TEST_PATTERN, > + ARRAY_SIZE(smiapp_test_patterns) - 1, > + 0, 0, smiapp_test_patterns); > + > + for (i = 0; i < ARRAY_SIZE(sensor->test_data); i++) > + sensor->test_data[i] = > + v4l2_ctrl_new_std(&sensor->pixel_array->ctrl_handler, > + &smiapp_ctrl_ops, > + V4L2_CID_TEST_PATTERN_RED + i, > + 0, 0, 1, 0); > + > if (sensor->pixel_array->ctrl_handler.error) { > dev_err(&client->dev, > "pixel array controls initialization failed (%d)\n", > @@ -543,6 +594,14 @@ static int smiapp_init_controls(struct smiapp_sensor > *sensor) goto error; > } > > + for (i = 0; i < ARRAY_SIZE(sensor->test_data); i++) { > + struct v4l2_ctrl *ctrl = sensor->test_data[i]; > + > + ctrl->maximum = > + ctrl->default_value = > + ctrl->cur.val = (1 << sensor->csi_format->width) - 1; I think multiple assignments on the same line are discouraged. Furthermore, couldn't you move this above and use the right values directly when creating the controls ? > + } > + > sensor->pixel_array->sd.ctrl_handler = > &sensor->pixel_array->ctrl_handler; > > @@ -1670,17 +1729,34 @@ static int smiapp_set_format(struct v4l2_subdev > *subdev, if (fmt->pad == ssd->source_pad) { > u32 code = fmt->format.code; > int rval = __smiapp_get_format(subdev, fh, fmt); > + bool range_changed = false; > + unsigned int i; > > if (!rval && subdev == &sensor->src->sd) { > const struct smiapp_csi_data_format *csi_format = > smiapp_validate_csi_data_format(sensor, code); > - if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) > + > + if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) { > + if (csi_format->width != > + sensor->csi_format->width) > + range_changed = true; > + > sensor->csi_format = csi_format; > + } > + > fmt->format.code = csi_format->code; > } > > mutex_unlock(&sensor->mutex); > - return rval; > + if (rval || !range_changed) > + return rval; > + > + for (i = 0; i < ARRAY_SIZE(sensor->test_data); i++) > + v4l2_ctrl_modify_range( > + sensor->test_data[i], > + 0, (1 << sensor->csi_format->width) - 1, 1, 0); > + > + return 0; > } > > /* Sink pad. Width and height are changeable here. */ > diff --git a/drivers/media/i2c/smiapp/smiapp.h > b/drivers/media/i2c/smiapp/smiapp.h index 7cc5aae..874b49f 100644 > --- a/drivers/media/i2c/smiapp/smiapp.h > +++ b/drivers/media/i2c/smiapp/smiapp.h > @@ -54,6 +54,8 @@ > (1000 + (SMIAPP_RESET_DELAY_CLOCKS * 1000 \ > + (clk) / 1000 - 1) / ((clk) / 1000)) > > +#define SMIAPP_COLOUR_COMPONENTS 4 > + > #include "smiapp-limits.h" > > struct smiapp_quirk; > @@ -241,6 +243,8 @@ struct smiapp_sensor { > /* src controls */ > struct v4l2_ctrl *link_freq; > struct v4l2_ctrl *pixel_rate_csi; > + /* test pattern colour components */ > + struct v4l2_ctrl *test_data[SMIAPP_COLOUR_COMPONENTS]; > }; > > #define to_smiapp_subdev(_sd) \ -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 3/3] smiapp: Implement the test pattern control 2014-05-29 14:54 ` Laurent Pinchart @ 2014-05-29 15:01 ` Sakari Ailus 2014-05-29 15:14 ` Sakari Ailus 1 sibling, 0 replies; 24+ messages in thread From: Sakari Ailus @ 2014-05-29 15:01 UTC (permalink / raw) To: Laurent Pinchart; +Cc: linux-media, hverkuil Laurent Pinchart wrote: >> @@ -404,6 +404,16 @@ static void smiapp_update_mbus_formats(struct >> smiapp_sensor *sensor) pixel_order_str[pixel_order]); >> } >> >> +static const char * const smiapp_test_patterns[] = { >> + "Disabled", >> + "Solid Colour", >> + "Eight Vertical Colour Bars", >> + "Colour Bars With Fade to Grey", >> + "Pseudorandom Sequence (PN9)", >> +}; >> + >> +static const struct v4l2_ctrl_ops smiapp_ctrl_ops; > > Is this needed ? Not anymore. It was necessary when there was configuration information for custom controls. I'll remove it. -- Sakari Ailus sakari.ailus@linux.intel.com ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 3/3] smiapp: Implement the test pattern control 2014-05-29 14:54 ` Laurent Pinchart 2014-05-29 15:01 ` Sakari Ailus @ 2014-05-29 15:14 ` Sakari Ailus 1 sibling, 0 replies; 24+ messages in thread From: Sakari Ailus @ 2014-05-29 15:14 UTC (permalink / raw) To: Laurent Pinchart; +Cc: linux-media, hverkuil Laurent Pinchart wrote: >> @@ -543,6 +594,14 @@ static int smiapp_init_controls(struct smiapp_sensor >> *sensor) goto error; >> } >> >> + for (i = 0; i < ARRAY_SIZE(sensor->test_data); i++) { >> + struct v4l2_ctrl *ctrl = sensor->test_data[i]; >> + >> + ctrl->maximum = >> + ctrl->default_value = >> + ctrl->cur.val = (1 << sensor->csi_format->width) - 1; > > I think multiple assignments on the same line are discouraged. > > Furthermore, couldn't you move this above and use the right values directly > when creating the controls ? Good point. There might have been a reason to do this in a past version of the patch but it no longer exists. -- Sakari Ailus sakari.ailus@linux.intel.com ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3.1 3/3] smiapp: Implement the test pattern control 2014-05-29 14:40 ` [PATCH v3 3/3] smiapp: Implement the test pattern control Sakari Ailus 2014-05-29 14:54 ` Laurent Pinchart @ 2014-05-29 15:16 ` Sakari Ailus 2014-07-10 14:16 ` Laurent Pinchart 1 sibling, 1 reply; 24+ messages in thread From: Sakari Ailus @ 2014-05-29 15:16 UTC (permalink / raw) To: linux-media; +Cc: laurent.pinchart, hverkuil Add support for the V4L2_CID_TEST_PATTERN control. When the solid colour mode is selected, additional controls become available for setting the solid four solid colour components. Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> Acked-by: Hans Verkuil <hans.verkuil@cisco.com> --- since v3: - Remove redundant definition of smiapp_ctrl_ops. - Initialise min, max and default in control creation time. drivers/media/i2c/smiapp/smiapp-core.c | 75 ++++++++++++++++++++++++++++++++-- drivers/media/i2c/smiapp/smiapp.h | 4 ++ 2 files changed, 75 insertions(+), 4 deletions(-) diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c index 446c82c..4ac7780 100644 --- a/drivers/media/i2c/smiapp/smiapp-core.c +++ b/drivers/media/i2c/smiapp/smiapp-core.c @@ -404,6 +404,14 @@ static void smiapp_update_mbus_formats(struct smiapp_sensor *sensor) pixel_order_str[pixel_order]); } +static const char * const smiapp_test_patterns[] = { + "Disabled", + "Solid Colour", + "Eight Vertical Colour Bars", + "Colour Bars With Fade to Grey", + "Pseudorandom Sequence (PN9)", +}; + static int smiapp_set_ctrl(struct v4l2_ctrl *ctrl) { struct smiapp_sensor *sensor = @@ -477,6 +485,35 @@ static int smiapp_set_ctrl(struct v4l2_ctrl *ctrl) return smiapp_pll_update(sensor); + case V4L2_CID_TEST_PATTERN: { + unsigned int i; + + for (i = 0; i < ARRAY_SIZE(sensor->test_data); i++) + v4l2_ctrl_activate( + sensor->test_data[i], + ctrl->val == + V4L2_SMIAPP_TEST_PATTERN_MODE_SOLID_COLOUR); + + return smiapp_write( + sensor, SMIAPP_REG_U16_TEST_PATTERN_MODE, ctrl->val); + } + + case V4L2_CID_TEST_PATTERN_RED: + return smiapp_write( + sensor, SMIAPP_REG_U16_TEST_DATA_RED, ctrl->val); + + case V4L2_CID_TEST_PATTERN_GREENR: + return smiapp_write( + sensor, SMIAPP_REG_U16_TEST_DATA_GREENR, ctrl->val); + + case V4L2_CID_TEST_PATTERN_BLUE: + return smiapp_write( + sensor, SMIAPP_REG_U16_TEST_DATA_BLUE, ctrl->val); + + case V4L2_CID_TEST_PATTERN_GREENB: + return smiapp_write( + sensor, SMIAPP_REG_U16_TEST_DATA_GREENB, ctrl->val); + default: return -EINVAL; } @@ -489,10 +526,10 @@ static const struct v4l2_ctrl_ops smiapp_ctrl_ops = { static int smiapp_init_controls(struct smiapp_sensor *sensor) { struct i2c_client *client = v4l2_get_subdevdata(&sensor->src->sd); - unsigned int max; + unsigned int max, i; int rval; - rval = v4l2_ctrl_handler_init(&sensor->pixel_array->ctrl_handler, 7); + rval = v4l2_ctrl_handler_init(&sensor->pixel_array->ctrl_handler, 12); if (rval) return rval; sensor->pixel_array->ctrl_handler.lock = &sensor->mutex; @@ -535,6 +572,19 @@ static int smiapp_init_controls(struct smiapp_sensor *sensor) &sensor->pixel_array->ctrl_handler, &smiapp_ctrl_ops, V4L2_CID_PIXEL_RATE, 0, 0, 1, 0); + v4l2_ctrl_new_std_menu_items(&sensor->pixel_array->ctrl_handler, + &smiapp_ctrl_ops, V4L2_CID_TEST_PATTERN, + ARRAY_SIZE(smiapp_test_patterns) - 1, + 0, 0, smiapp_test_patterns); + + for (i = 0; i < ARRAY_SIZE(sensor->test_data); i++) + sensor->test_data[i] = + v4l2_ctrl_new_std( + &sensor->pixel_array->ctrl_handler, + &smiapp_ctrl_ops, V4L2_CID_TEST_PATTERN_RED + i, + 0, (1 << sensor->csi_format->width) - 1, 1, + (1 << sensor->csi_format->width) - 1); + if (sensor->pixel_array->ctrl_handler.error) { dev_err(&client->dev, "pixel array controls initialization failed (%d)\n", @@ -1670,17 +1720,34 @@ static int smiapp_set_format(struct v4l2_subdev *subdev, if (fmt->pad == ssd->source_pad) { u32 code = fmt->format.code; int rval = __smiapp_get_format(subdev, fh, fmt); + bool range_changed = false; + unsigned int i; if (!rval && subdev == &sensor->src->sd) { const struct smiapp_csi_data_format *csi_format = smiapp_validate_csi_data_format(sensor, code); - if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) + + if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) { + if (csi_format->width != + sensor->csi_format->width) + range_changed = true; + sensor->csi_format = csi_format; + } + fmt->format.code = csi_format->code; } mutex_unlock(&sensor->mutex); - return rval; + if (rval || !range_changed) + return rval; + + for (i = 0; i < ARRAY_SIZE(sensor->test_data); i++) + v4l2_ctrl_modify_range( + sensor->test_data[i], + 0, (1 << sensor->csi_format->width) - 1, 1, 0); + + return 0; } /* Sink pad. Width and height are changeable here. */ diff --git a/drivers/media/i2c/smiapp/smiapp.h b/drivers/media/i2c/smiapp/smiapp.h index 7cc5aae..874b49f 100644 --- a/drivers/media/i2c/smiapp/smiapp.h +++ b/drivers/media/i2c/smiapp/smiapp.h @@ -54,6 +54,8 @@ (1000 + (SMIAPP_RESET_DELAY_CLOCKS * 1000 \ + (clk) / 1000 - 1) / ((clk) / 1000)) +#define SMIAPP_COLOUR_COMPONENTS 4 + #include "smiapp-limits.h" struct smiapp_quirk; @@ -241,6 +243,8 @@ struct smiapp_sensor { /* src controls */ struct v4l2_ctrl *link_freq; struct v4l2_ctrl *pixel_rate_csi; + /* test pattern colour components */ + struct v4l2_ctrl *test_data[SMIAPP_COLOUR_COMPONENTS]; }; #define to_smiapp_subdev(_sd) \ -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v3.1 3/3] smiapp: Implement the test pattern control 2014-05-29 15:16 ` [PATCH v3.1 " Sakari Ailus @ 2014-07-10 14:16 ` Laurent Pinchart 0 siblings, 0 replies; 24+ messages in thread From: Laurent Pinchart @ 2014-07-10 14:16 UTC (permalink / raw) To: Sakari Ailus; +Cc: linux-media, hverkuil Hi Sakari, Thank you for the patch. On Thursday 29 May 2014 18:16:54 Sakari Ailus wrote: > Add support for the V4L2_CID_TEST_PATTERN control. When the solid colour > mode is selected, additional controls become available for setting the > solid four solid colour components. > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> > Acked-by: Hans Verkuil <hans.verkuil@cisco.com> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Please see below for a minor comment. > --- > since v3: > - Remove redundant definition of smiapp_ctrl_ops. > > - Initialise min, max and default in control creation time. > > drivers/media/i2c/smiapp/smiapp-core.c | 75 +++++++++++++++++++++++++++++-- > drivers/media/i2c/smiapp/smiapp.h | 4 ++ > 2 files changed, 75 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/i2c/smiapp/smiapp-core.c > b/drivers/media/i2c/smiapp/smiapp-core.c index 446c82c..4ac7780 100644 > --- a/drivers/media/i2c/smiapp/smiapp-core.c > +++ b/drivers/media/i2c/smiapp/smiapp-core.c [snip] > @@ -535,6 +572,19 @@ static int smiapp_init_controls(struct smiapp_sensor > *sensor) &sensor->pixel_array->ctrl_handler, &smiapp_ctrl_ops, > V4L2_CID_PIXEL_RATE, 0, 0, 1, 0); > > + v4l2_ctrl_new_std_menu_items(&sensor->pixel_array->ctrl_handler, > + &smiapp_ctrl_ops, V4L2_CID_TEST_PATTERN, > + ARRAY_SIZE(smiapp_test_patterns) - 1, > + 0, 0, smiapp_test_patterns); > + > + for (i = 0; i < ARRAY_SIZE(sensor->test_data); i++) > + sensor->test_data[i] = > + v4l2_ctrl_new_std( > + &sensor->pixel_array->ctrl_handler, > + &smiapp_ctrl_ops, V4L2_CID_TEST_PATTERN_RED + i, > + 0, (1 << sensor->csi_format->width) - 1, 1, > + (1 << sensor->csi_format->width) - 1); I would have used a local variable assigned to (1 << sensor->csi_format- >width) - 1 outside of the loop. > + > if (sensor->pixel_array->ctrl_handler.error) { > dev_err(&client->dev, > "pixel array controls initialization failed (%d)\n", -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2014-07-10 14:16 UTC | newest] Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-05-29 14:40 [PATCH v3 0/3] smiapp test pattern support Sakari Ailus 2014-05-29 14:40 ` [PATCH v3 1/3] v4l: Add test pattern colour component controls Sakari Ailus 2014-05-29 14:47 ` Laurent Pinchart 2014-05-29 14:58 ` Sakari Ailus 2014-05-29 15:01 ` Laurent Pinchart 2014-05-30 12:28 ` Hans Verkuil 2014-06-03 12:21 ` Laurent Pinchart 2014-06-03 12:27 ` Sakari Ailus 2014-06-03 12:42 ` Hans Verkuil 2014-06-10 12:15 ` Hans Verkuil 2014-05-29 14:40 ` [PATCH v3 2/3] smiapp: Add driver-specific test pattern menu item definitions Sakari Ailus 2014-05-29 14:48 ` Laurent Pinchart 2014-05-29 15:08 ` Sakari Ailus 2014-06-10 12:16 ` Hans Verkuil 2014-06-10 15:57 ` Prabhakar Lad 2014-06-11 6:34 ` Sakari Ailus 2014-06-11 6:36 ` [PATCH v3.1 2/4] " Sakari Ailus 2014-06-11 7:58 ` Prabhakar Lad 2014-05-29 14:40 ` [PATCH v3 3/3] smiapp: Implement the test pattern control Sakari Ailus 2014-05-29 14:54 ` Laurent Pinchart 2014-05-29 15:01 ` Sakari Ailus 2014-05-29 15:14 ` Sakari Ailus 2014-05-29 15:16 ` [PATCH v3.1 " Sakari Ailus 2014-07-10 14:16 ` Laurent Pinchart
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.