Re: [PATCH v2] media: v4l2-ctrls: add control for test pattern

2012-09-13 Thread Laurent Pinchart
Hi Sakari,

On Sunday 09 September 2012 10:40:17 Sakari Ailus wrote:
 On Sat, Sep 08, 2012 at 01:11:04PM +0200, Hans Verkuil wrote:
  On Fri September 7 2012 20:20:51 Sakari Ailus wrote:
   Prabhakar Lad wrote:
From: Lad, Prabhakar prabhakar@ti.com

add V4L2_CID_TEST_PATTERN of type menu, which determines
the internal test pattern selected by the device.

Signed-off-by: Lad, Prabhakar prabhakar@ti.com
Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com
Cc: Sakari Ailus sakari.ai...@iki.fi
Cc: Hans Verkuil hans.verk...@cisco.com
Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com
Cc: Mauro Carvalho Chehab mche...@infradead.org
Cc: Sylwester Nawrocki s.nawro...@samsung.com
Cc: Hans de Goede hdego...@redhat.com
Cc: Kyungmin Park kyungmin.p...@samsung.com
Cc: Rob Landley r...@landley.net
---
This patches has one checkpatch warning for line over
80 characters altough it can be avoided I have kept it
for consistency.

Changes for v2:
1: Included display devices in the description for test pattern
as pointed by Hans.

2: In the menu replaced 'Test Pattern Disabled' by 'Disabled' as
pointed by Sylwester.

3: Removed the test patterns from menu as the are hardware specific
as pointed by Sakari.
  
  Documentation/DocBook/media/v4l/controls.xml |   20 
  drivers/media/v4l2-core/v4l2-ctrls.c |8 
  include/linux/videodev2.h|4 
  3 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/controls.xml
b/Documentation/DocBook/media/v4l/controls.xml index ad873ea..173934e
100644
--- a/Documentation/DocBook/media/v4l/controls.xml
+++ b/Documentation/DocBook/media/v4l/controls.xml
@@ -4311,6 +4311,26 @@ interface and may change in the future./para
  /tbody
/entrytbl
  /row
+ row
+   entry
spanname=idconstantV4L2_CID_TEST_PATTERN/constant/entry +  
  
 entrymenu/entry
+ /row
+ row id=v4l2-test-pattern
+   entry spanname=descr The Capture/Display/Sensors have 
the
capability
+   to generate internal test patterns and this are hardware
specific. This
+   test patterns are used to test a device is properly working 
and
can generate
+   the desired waveforms that
it supports./entry
+ /row
+ row
+   entrytbl spanname=descr cols=2
+ tbody valign=top
+   row
+   
entryconstantV4L2_TEST_PATTERN_DISABLED/constant/entry
+ entryTest pattern generation is disabled/entry
+   /row
+ /tbody
+   /entrytbl
+ /row
rowentry/entry/row
/tbody
/tgroup
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
b/drivers/media/v4l2-core/v4l2-ctrls.c index 8f2f40b..d731422 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -430,6 +430,10 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
Advanced,
NULL,
};
+   static const char * const test_pattern[] = {
+   Disabled,
+   NULL,
+   };

switch (id) {
case V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ:
@@ -509,6 +513,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
return jpeg_chroma_subsampling;
case V4L2_CID_DPCM_PREDICTOR:
return dpcm_predictor;
+   case V4L2_CID_TEST_PATTERN:
+   return test_pattern;
   
   I think it's not necessary to define test_pattern (nor be prepared to
   return it) since the menu is going to be device specific. So the driver
   is responsible for all of the menu items. Such menus are created using
   v4l2_ctrl_new_custom() instead of v4l2_ctrl_new_std_menu().
   
   Looks good to me otherwise.
  
  I would suggest that we *do* make this a standard control, but the menu
  consists of just one item: Disabled. After creating the control you can
  just set the ctrl-qmenu pointer to the device-specific menu. I like
  using standard controls because they guarantee standard naming and type
  conventions. They are also easier to use in an application.
 
 That's not quite enough. Also menu_skip_mask and max also need to be
 replaced. In a general case min as well. It's easy to do mistakes in that
 --- how about a separate function for doing that? It'd be also nice to be
 able to use the existing standardised menu item strings, but that's just an
 extra plus.

Agreed. A way to create a standard menu control with driver-supplied menu 
items would be a good addition to the 

Re: [PATCH v2] media: v4l2-ctrls: add control for test pattern

2012-09-09 Thread Sakari Ailus
Hi Hans,

On Sat, Sep 08, 2012 at 01:11:04PM +0200, Hans Verkuil wrote:
 On Fri September 7 2012 20:20:51 Sakari Ailus wrote:
  Hi Prabhakar,
  
  Thanks for the patch!
  
  Prabhakar Lad wrote:
   From: Lad, Prabhakar prabhakar@ti.com
  
   add V4L2_CID_TEST_PATTERN of type menu, which determines
   the internal test pattern selected by the device.
  
   Signed-off-by: Lad, Prabhakar prabhakar@ti.com
   Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com
   Cc: Sakari Ailus sakari.ai...@iki.fi
   Cc: Hans Verkuil hans.verk...@cisco.com
   Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com
   Cc: Mauro Carvalho Chehab mche...@infradead.org
   Cc: Sylwester Nawrocki s.nawro...@samsung.com
   Cc: Hans de Goede hdego...@redhat.com
   Cc: Kyungmin Park kyungmin.p...@samsung.com
   Cc: Rob Landley r...@landley.net
   ---
   This patches has one checkpatch warning for line over
   80 characters altough it can be avoided I have kept it
   for consistency.
  
   Changes for v2:
   1: Included display devices in the description for test pattern
   as pointed by Hans.
   2: In the menu replaced 'Test Pattern Disabled' by 'Disabled' as
   pointed by Sylwester.
   3: Removed the test patterns from menu as the are hardware specific
   as pointed by Sakari.
  
 Documentation/DocBook/media/v4l/controls.xml |   20 
 drivers/media/v4l2-core/v4l2-ctrls.c |8 
 include/linux/videodev2.h|4 
 3 files changed, 32 insertions(+), 0 deletions(-)
  
   diff --git a/Documentation/DocBook/media/v4l/controls.xml 
   b/Documentation/DocBook/media/v4l/controls.xml
   index ad873ea..173934e 100644
   --- a/Documentation/DocBook/media/v4l/controls.xml
   +++ b/Documentation/DocBook/media/v4l/controls.xml
   @@ -4311,6 +4311,26 @@ interface and may change in the future./para
   /tbody
 /entrytbl
   /row
   +   row
   + entry 
   spanname=idconstantV4L2_CID_TEST_PATTERN/constant/entry
   + entrymenu/entry
   +   /row
   +   row id=v4l2-test-pattern
   + entry spanname=descr The Capture/Display/Sensors have the 
   capability
   + to generate internal test patterns and this are hardware specific. 
   This
   + test patterns are used to test a device is properly working and can 
   generate
   + the desired waveforms that it supports./entry
   +   /row
   +   row
   + entrytbl spanname=descr cols=2
   +   tbody valign=top
   + row
   +  entryconstantV4L2_TEST_PATTERN_DISABLED/constant/entry
   +   entryTest pattern generation is disabled/entry
   + /row
   +   /tbody
   + /entrytbl
   +   /row
 rowentry/entry/row
 /tbody
   /tgroup
   diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
   b/drivers/media/v4l2-core/v4l2-ctrls.c
   index 8f2f40b..d731422 100644
   --- a/drivers/media/v4l2-core/v4l2-ctrls.c
   +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
   @@ -430,6 +430,10 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
 Advanced,
 NULL,
 };
   + static const char * const test_pattern[] = {
   + Disabled,
   + NULL,
   + };
  
 switch (id) {
 case V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ:
   @@ -509,6 +513,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
 return jpeg_chroma_subsampling;
 case V4L2_CID_DPCM_PREDICTOR:
 return dpcm_predictor;
   + case V4L2_CID_TEST_PATTERN:
   + return test_pattern;
  
  I think it's not necessary to define test_pattern (nor be prepared to 
  return it) since the menu is going to be device specific. So the driver 
  is responsible for all of the menu items. Such menus are created using 
  v4l2_ctrl_new_custom() instead of v4l2_ctrl_new_std_menu().
  
  Looks good to me otherwise.
 
 I would suggest that we *do* make this a standard control, but the menu 
 consists
 of just one item: Disabled. After creating the control you can just set the
 ctrl-qmenu pointer to the device-specific menu. I like using standard 
 controls
 because they guarantee standard naming and type conventions. They are also
 easier to use in an application.

That's not quite enough. Also menu_skip_mask and max also need to be
replaced. In a general case min as well. It's easy to do mistakes in that
--- how about a separate function for doing that? It'd be also nice to be
able to use the existing standardised menu item strings, but that's just an
extra plus.

However I think it's beyond this patch, which I think then is be fine w/o
modifications. So on my behalf,

Acked-by: Sakari Ailus sakari.ai...@iki.fi

Cheers,

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  

Re: [PATCH v2] media: v4l2-ctrls: add control for test pattern

2012-09-08 Thread Hans Verkuil
On Fri September 7 2012 20:20:51 Sakari Ailus wrote:
 Hi Prabhakar,
 
 Thanks for the patch!
 
 Prabhakar Lad wrote:
  From: Lad, Prabhakar prabhakar@ti.com
 
  add V4L2_CID_TEST_PATTERN of type menu, which determines
  the internal test pattern selected by the device.
 
  Signed-off-by: Lad, Prabhakar prabhakar@ti.com
  Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com
  Cc: Sakari Ailus sakari.ai...@iki.fi
  Cc: Hans Verkuil hans.verk...@cisco.com
  Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com
  Cc: Mauro Carvalho Chehab mche...@infradead.org
  Cc: Sylwester Nawrocki s.nawro...@samsung.com
  Cc: Hans de Goede hdego...@redhat.com
  Cc: Kyungmin Park kyungmin.p...@samsung.com
  Cc: Rob Landley r...@landley.net
  ---
  This patches has one checkpatch warning for line over
  80 characters altough it can be avoided I have kept it
  for consistency.
 
  Changes for v2:
  1: Included display devices in the description for test pattern
  as pointed by Hans.
  2: In the menu replaced 'Test Pattern Disabled' by 'Disabled' as
  pointed by Sylwester.
  3: Removed the test patterns from menu as the are hardware specific
  as pointed by Sakari.
 
Documentation/DocBook/media/v4l/controls.xml |   20 
drivers/media/v4l2-core/v4l2-ctrls.c |8 
include/linux/videodev2.h|4 
3 files changed, 32 insertions(+), 0 deletions(-)
 
  diff --git a/Documentation/DocBook/media/v4l/controls.xml 
  b/Documentation/DocBook/media/v4l/controls.xml
  index ad873ea..173934e 100644
  --- a/Documentation/DocBook/media/v4l/controls.xml
  +++ b/Documentation/DocBook/media/v4l/controls.xml
  @@ -4311,6 +4311,26 @@ interface and may change in the future./para
/tbody
  /entrytbl
/row
  + row
  +   entry 
  spanname=idconstantV4L2_CID_TEST_PATTERN/constant/entry
  +   entrymenu/entry
  + /row
  + row id=v4l2-test-pattern
  +   entry spanname=descr The Capture/Display/Sensors have the 
  capability
  +   to generate internal test patterns and this are hardware specific. 
  This
  +   test patterns are used to test a device is properly working and can 
  generate
  +   the desired waveforms that it supports./entry
  + /row
  + row
  +   entrytbl spanname=descr cols=2
  + tbody valign=top
  +   row
  +entryconstantV4L2_TEST_PATTERN_DISABLED/constant/entry
  + entryTest pattern generation is disabled/entry
  +   /row
  + /tbody
  +   /entrytbl
  + /row
  rowentry/entry/row
  /tbody
  /tgroup
  diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
  b/drivers/media/v4l2-core/v4l2-ctrls.c
  index 8f2f40b..d731422 100644
  --- a/drivers/media/v4l2-core/v4l2-ctrls.c
  +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
  @@ -430,6 +430,10 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
  Advanced,
  NULL,
  };
  +   static const char * const test_pattern[] = {
  +   Disabled,
  +   NULL,
  +   };
 
  switch (id) {
  case V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ:
  @@ -509,6 +513,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
  return jpeg_chroma_subsampling;
  case V4L2_CID_DPCM_PREDICTOR:
  return dpcm_predictor;
  +   case V4L2_CID_TEST_PATTERN:
  +   return test_pattern;
 
 I think it's not necessary to define test_pattern (nor be prepared to 
 return it) since the menu is going to be device specific. So the driver 
 is responsible for all of the menu items. Such menus are created using 
 v4l2_ctrl_new_custom() instead of v4l2_ctrl_new_std_menu().
 
 Looks good to me otherwise.

I would suggest that we *do* make this a standard control, but the menu consists
of just one item: Disabled. After creating the control you can just set the
ctrl-qmenu pointer to the device-specific menu. I like using standard controls
because they guarantee standard naming and type conventions. They are also
easier to use in an application.

This would obvious require good documentation, both in the code and in the
spec.

BTW, see also this patch that needs to use the test pattern control:

http://www.spinics.net/lists/linux-media/msg52747.html

Regards,

Hans
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2] media: v4l2-ctrls: add control for test pattern

2012-09-07 Thread Prabhakar Lad
From: Lad, Prabhakar prabhakar@ti.com

add V4L2_CID_TEST_PATTERN of type menu, which determines
the internal test pattern selected by the device.

Signed-off-by: Lad, Prabhakar prabhakar@ti.com
Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com
Cc: Sakari Ailus sakari.ai...@iki.fi
Cc: Hans Verkuil hans.verk...@cisco.com
Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com
Cc: Mauro Carvalho Chehab mche...@infradead.org
Cc: Sylwester Nawrocki s.nawro...@samsung.com
Cc: Hans de Goede hdego...@redhat.com
Cc: Kyungmin Park kyungmin.p...@samsung.com
Cc: Rob Landley r...@landley.net
---
This patches has one checkpatch warning for line over
80 characters altough it can be avoided I have kept it
for consistency.

Changes for v2:
1: Included display devices in the description for test pattern
   as pointed by Hans.
2: In the menu replaced 'Test Pattern Disabled' by 'Disabled' as
   pointed by Sylwester.
3: Removed the test patterns from menu as the are hardware specific
   as pointed by Sakari.

 Documentation/DocBook/media/v4l/controls.xml |   20 
 drivers/media/v4l2-core/v4l2-ctrls.c |8 
 include/linux/videodev2.h|4 
 3 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/controls.xml 
b/Documentation/DocBook/media/v4l/controls.xml
index ad873ea..173934e 100644
--- a/Documentation/DocBook/media/v4l/controls.xml
+++ b/Documentation/DocBook/media/v4l/controls.xml
@@ -4311,6 +4311,26 @@ interface and may change in the future./para
  /tbody
/entrytbl
  /row
+ row
+   entry 
spanname=idconstantV4L2_CID_TEST_PATTERN/constant/entry
+   entrymenu/entry
+ /row
+ row id=v4l2-test-pattern
+   entry spanname=descr The Capture/Display/Sensors have the 
capability
+   to generate internal test patterns and this are hardware specific. 
This
+   test patterns are used to test a device is properly working and can 
generate
+   the desired waveforms that it supports./entry
+ /row
+ row
+   entrytbl spanname=descr cols=2
+ tbody valign=top
+   row
+entryconstantV4L2_TEST_PATTERN_DISABLED/constant/entry
+ entryTest pattern generation is disabled/entry
+   /row
+ /tbody
+   /entrytbl
+ /row
rowentry/entry/row
/tbody
   /tgroup
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
b/drivers/media/v4l2-core/v4l2-ctrls.c
index 8f2f40b..d731422 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -430,6 +430,10 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
Advanced,
NULL,
};
+   static const char * const test_pattern[] = {
+   Disabled,
+   NULL,
+   };
 
switch (id) {
case V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ:
@@ -509,6 +513,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
return jpeg_chroma_subsampling;
case V4L2_CID_DPCM_PREDICTOR:
return dpcm_predictor;
+   case V4L2_CID_TEST_PATTERN:
+   return test_pattern;
 
default:
return NULL;
@@ -740,6 +746,7 @@ const char *v4l2_ctrl_get_name(u32 id)
case V4L2_CID_LINK_FREQ:return Link Frequency;
case V4L2_CID_PIXEL_RATE:   return Pixel Rate;
case V4L2_CID_DPCM_PREDICTOR:   return DPCM Predictor;
+   case V4L2_CID_TEST_PATTERN: return Test Pattern;
 
default:
return NULL;
@@ -841,6 +848,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum 
v4l2_ctrl_type *type,
case V4L2_CID_EXPOSURE_METERING:
case V4L2_CID_SCENE_MODE:
case V4L2_CID_DPCM_PREDICTOR:
+   case V4L2_CID_TEST_PATTERN:
*type = V4L2_CTRL_TYPE_MENU;
break;
case V4L2_CID_LINK_FREQ:
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index ca9fb78..6a2d499 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -2005,6 +2005,10 @@ enum v4l2_dpcm_predictor {
V4L2_DPCM_PREDICTOR_SIMPLE  = 0,
V4L2_DPCM_PREDICTOR_ADVANCED= 1,
 };
+#define V4L2_CID_TEST_PATTERN  (V4L2_CID_IMAGE_PROC_CLASS_BASE 
+ 4)
+enum v4l2_test_pattern {
+   V4L2_TEST_PATTERN_DISABLED  = 0,
+};
 
 /*
  * T U N I N G
-- 
1.7.0.4

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] media: v4l2-ctrls: add control for test pattern

2012-09-07 Thread Sakari Ailus

Hi Prabhakar,

Thanks for the patch!

Prabhakar Lad wrote:

From: Lad, Prabhakar prabhakar@ti.com

add V4L2_CID_TEST_PATTERN of type menu, which determines
the internal test pattern selected by the device.

Signed-off-by: Lad, Prabhakar prabhakar@ti.com
Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com
Cc: Sakari Ailus sakari.ai...@iki.fi
Cc: Hans Verkuil hans.verk...@cisco.com
Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com
Cc: Mauro Carvalho Chehab mche...@infradead.org
Cc: Sylwester Nawrocki s.nawro...@samsung.com
Cc: Hans de Goede hdego...@redhat.com
Cc: Kyungmin Park kyungmin.p...@samsung.com
Cc: Rob Landley r...@landley.net
---
This patches has one checkpatch warning for line over
80 characters altough it can be avoided I have kept it
for consistency.

Changes for v2:
1: Included display devices in the description for test pattern
as pointed by Hans.
2: In the menu replaced 'Test Pattern Disabled' by 'Disabled' as
pointed by Sylwester.
3: Removed the test patterns from menu as the are hardware specific
as pointed by Sakari.

  Documentation/DocBook/media/v4l/controls.xml |   20 
  drivers/media/v4l2-core/v4l2-ctrls.c |8 
  include/linux/videodev2.h|4 
  3 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/controls.xml 
b/Documentation/DocBook/media/v4l/controls.xml
index ad873ea..173934e 100644
--- a/Documentation/DocBook/media/v4l/controls.xml
+++ b/Documentation/DocBook/media/v4l/controls.xml
@@ -4311,6 +4311,26 @@ interface and may change in the future./para
  /tbody
/entrytbl
  /row
+ row
+   entry 
spanname=idconstantV4L2_CID_TEST_PATTERN/constant/entry
+   entrymenu/entry
+ /row
+ row id=v4l2-test-pattern
+   entry spanname=descr The Capture/Display/Sensors have the 
capability
+   to generate internal test patterns and this are hardware specific. 
This
+   test patterns are used to test a device is properly working and can 
generate
+   the desired waveforms that it supports./entry
+ /row
+ row
+   entrytbl spanname=descr cols=2
+ tbody valign=top
+   row
+entryconstantV4L2_TEST_PATTERN_DISABLED/constant/entry
+ entryTest pattern generation is disabled/entry
+   /row
+ /tbody
+   /entrytbl
+ /row
rowentry/entry/row
/tbody
/tgroup
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c 
b/drivers/media/v4l2-core/v4l2-ctrls.c
index 8f2f40b..d731422 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -430,6 +430,10 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
Advanced,
NULL,
};
+   static const char * const test_pattern[] = {
+   Disabled,
+   NULL,
+   };

switch (id) {
case V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ:
@@ -509,6 +513,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
return jpeg_chroma_subsampling;
case V4L2_CID_DPCM_PREDICTOR:
return dpcm_predictor;
+   case V4L2_CID_TEST_PATTERN:
+   return test_pattern;


I think it's not necessary to define test_pattern (nor be prepared to 
return it) since the menu is going to be device specific. So the driver 
is responsible for all of the menu items. Such menus are created using 
v4l2_ctrl_new_custom() instead of v4l2_ctrl_new_std_menu().


Looks good to me otherwise.

Kind regards,

--
Sakari Ailus
sakari.ai...@iki.fi
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] media: v4l2-ctrls: add control for test pattern

2012-09-07 Thread Prabhakar Lad
Hi Sakari,

Thanks for the review.

On Fri, Sep 7, 2012 at 11:50 PM, Sakari Ailus sakari.ai...@iki.fi wrote:
 Hi Prabhakar,

 Thanks for the patch!


 Prabhakar Lad wrote:

 From: Lad, Prabhakar prabhakar@ti.com

 add V4L2_CID_TEST_PATTERN of type menu, which determines
 the internal test pattern selected by the device.

 Signed-off-by: Lad, Prabhakar prabhakar@ti.com
 Signed-off-by: Manjunath Hadli manjunath.ha...@ti.com
 Cc: Sakari Ailus sakari.ai...@iki.fi
 Cc: Hans Verkuil hans.verk...@cisco.com
 Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com
 Cc: Mauro Carvalho Chehab mche...@infradead.org
 Cc: Sylwester Nawrocki s.nawro...@samsung.com
 Cc: Hans de Goede hdego...@redhat.com
 Cc: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Rob Landley r...@landley.net
 ---
 This patches has one checkpatch warning for line over
 80 characters altough it can be avoided I have kept it
 for consistency.

 Changes for v2:
 1: Included display devices in the description for test pattern
 as pointed by Hans.
 2: In the menu replaced 'Test Pattern Disabled' by 'Disabled' as
 pointed by Sylwester.
 3: Removed the test patterns from menu as the are hardware specific
 as pointed by Sakari.

   Documentation/DocBook/media/v4l/controls.xml |   20 
   drivers/media/v4l2-core/v4l2-ctrls.c |8 
   include/linux/videodev2.h|4 
   3 files changed, 32 insertions(+), 0 deletions(-)

 diff --git a/Documentation/DocBook/media/v4l/controls.xml
 b/Documentation/DocBook/media/v4l/controls.xml
 index ad873ea..173934e 100644
 --- a/Documentation/DocBook/media/v4l/controls.xml
 +++ b/Documentation/DocBook/media/v4l/controls.xml
 @@ -4311,6 +4311,26 @@ interface and may change in the future./para
   /tbody
 /entrytbl
   /row
 + row
 +   entry
 spanname=idconstantV4L2_CID_TEST_PATTERN/constant/entry
 +   entrymenu/entry
 + /row
 + row id=v4l2-test-pattern
 +   entry spanname=descr The Capture/Display/Sensors have the
 capability
 +   to generate internal test patterns and this are hardware
 specific. This
 +   test patterns are used to test a device is properly working
 and can generate
 +   the desired waveforms that it supports./entry
 + /row
 + row
 +   entrytbl spanname=descr cols=2
 + tbody valign=top
 +   row
 +
 entryconstantV4L2_TEST_PATTERN_DISABLED/constant/entry
 + entryTest pattern generation is disabled/entry
 +   /row
 + /tbody
 +   /entrytbl
 + /row
 rowentry/entry/row
 /tbody
 /tgroup
 diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
 b/drivers/media/v4l2-core/v4l2-ctrls.c
 index 8f2f40b..d731422 100644
 --- a/drivers/media/v4l2-core/v4l2-ctrls.c
 +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
 @@ -430,6 +430,10 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
 Advanced,
 NULL,
 };
 +   static const char * const test_pattern[] = {
 +   Disabled,
 +   NULL,
 +   };

 switch (id) {
 case V4L2_CID_MPEG_AUDIO_SAMPLING_FREQ:
 @@ -509,6 +513,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
 return jpeg_chroma_subsampling;
 case V4L2_CID_DPCM_PREDICTOR:
 return dpcm_predictor;
 +   case V4L2_CID_TEST_PATTERN:
 +   return test_pattern;


 I think it's not necessary to define test_pattern (nor be prepared to return
 it) since the menu is going to be device specific. So the driver is
 responsible for all of the menu items. Such menus are created using
 v4l2_ctrl_new_custom() instead of v4l2_ctrl_new_std_menu().

Ok.

Regrads,
--Prabhakar Lad

 Looks good to me otherwise.

 Kind regards,

 --
 Sakari Ailus
 sakari.ai...@iki.fi

 ___
 Davinci-linux-open-source mailing list
 davinci-linux-open-sou...@linux.davincidsp.com
 http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html