Re: [PATCH RFC] adding support for setting bus parameters in sub device

2009-06-17 Thread Magnus Damm
On Tue, Jun 16, 2009 at 11:33 PM, Karicheri,
Muralidharanm-kariche...@ti.com wrote:

 snip

 [MK]In that case can't the driver just ignore the field polarity? I
assume that drivers implement the parameter that has support in hardware.
So it is not an issue.

No, because the same driver runs on hardware that also has the field
signal. So we need to be able to give information about which signals
that the board actually implement. We already do this with the
soc_camera framework and it is working just fine.


 Hardware with field signal used (driver use polarity from platform data and 
 set it in the hardware)
 Hardware with field signal not used (In this case, even though the driver 
 sets it in the hardware, it is not really used in the hardware design and 
 hence is a don't care. right?

 So I don't see why it matters.

Maybe I'm misunderstanding what you are trying to do. But how can the
camera sensor driver check if the field signal is present or not? The
camera sensor driver may need information if a pin is present or not
for some decision? Perhaps for hardware configuration?

A good example IMO is the tw9910 driver and the mpout signal. Right
now the mpout configuration is part of the platform data, but maybe it
would make more sense to allow the driver to check if field is used on
the platform and if so configure the pin accordingly.

/ magnus
--
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 RFC] adding support for setting bus parameters in sub device

2009-06-15 Thread Karicheri, Muralidharan
 +
 +struct v4l2_subdev_bus {
 +       enum v4l2_subdev_bus_type type;
 +       u8 width;
 +       /* 0 - active low, 1 - active high */
 +       unsigned pol_vsync:1;
 +       /* 0 - active low, 1 - active high */
 +       unsigned pol_hsync:1;
 +       /* 0 - low to high , 1 - high to low */
 +       unsigned pol_field:1;
 +       /* 0 - sample at falling edge , 1 - sample at rising edge */
 +       unsigned pol_pclock:1;
 +       /* 0 - active low , 1 - active high */
 +       unsigned pol_data:1;
 +};

As for the pins/signals, I wonder if per-signal polarity/edge is
enough. If this is going to be used by/replace the soc_camera
interface then we also need to know if the signal is present or not.
For instance, I have a SuperH board using my CEU driver together with
one OV7725 camera or one TW9910 video decoder. Some revisions of the
board do not route the field signal between the SuperH on-chip CEU and
the TW9910. Both the CEU and the TW9910 support this signal, it just
happen to be missing. 

[MK]In that case can't the driver just ignore the field polarity? I assume that 
drivers implement the parameter that has support in hardware. So it is not an 
issue.

I think we need a way to include this board
specific property somehow.


/ magnus

--
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 RFC] adding support for setting bus parameters in sub device

2009-06-14 Thread Magnus Damm
On Wed, Jun 10, 2009 at 5:55 AM, m-kariche...@ti.com wrote:
 From: Muralidharan Karicheri a0868...@gt516km11.gt.design.ti.com

 re-sending with RFC in the header

 This patch adds support for setting bus parameters such as bus type
 (BT.656, BT.1120 etc), width (example 10 bit raw image data bus)
 and polarities (vsync, hsync, field etc) in sub device. This allows
 bridge driver to configure the sub device for a specific set of bus
 parameters through s_bus() function call.

 Reviewed By Hans Verkuil.
 Signed-off-by: Muralidharan Karicheri m-kariche...@ti.com
 ---
 Applies to v4l-dvb repository

  include/media/v4l2-subdev.h |   36 
  1 files changed, 36 insertions(+), 0 deletions(-)

 diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
 index 1785608..c1cfb3b 100644
 --- a/include/media/v4l2-subdev.h
 +++ b/include/media/v4l2-subdev.h
 @@ -37,6 +37,41 @@ struct v4l2_decode_vbi_line {
        u32 type;               /* VBI service type (V4L2_SLICED_*). 0 if no 
 service found */
  };

 +/*
 + * Some sub-devices are connected to the bridge device through a bus that
 + * carries * the clock, vsync, hsync and data. Some interfaces such as BT.656
 + * carries the sync embedded in the data where as others have separate line
 + * carrying the sync signals. The structure below is used by bridge driver to
 + * set the desired bus parameters in the sub device to work with it.
 + */
 +enum v4l2_subdev_bus_type {
 +       /* BT.656 interface. Embedded sync */
 +       V4L2_SUBDEV_BUS_BT_656,
 +       /* BT.1120 interface. Embedded sync */
 +       V4L2_SUBDEV_BUS_BT_1120,
 +       /* 8 bit muxed YCbCr bus, separate sync and field signals */
 +       V4L2_SUBDEV_BUS_YCBCR_8,
 +       /* 16 bit YCbCr bus, separate sync and field signals */
 +       V4L2_SUBDEV_BUS_YCBCR_16,
 +       /* Raw Bayer image data bus , 8 - 16 bit wide, sync signals */
 +       V4L2_SUBDEV_BUS_RAW_BAYER
 +};
 +
 +struct v4l2_subdev_bus {
 +       enum v4l2_subdev_bus_type type;
 +       u8 width;
 +       /* 0 - active low, 1 - active high */
 +       unsigned pol_vsync:1;
 +       /* 0 - active low, 1 - active high */
 +       unsigned pol_hsync:1;
 +       /* 0 - low to high , 1 - high to low */
 +       unsigned pol_field:1;
 +       /* 0 - sample at falling edge , 1 - sample at rising edge */
 +       unsigned pol_pclock:1;
 +       /* 0 - active low , 1 - active high */
 +       unsigned pol_data:1;
 +};

As for the pins/signals, I wonder if per-signal polarity/edge is
enough. If this is going to be used by/replace the soc_camera
interface then we also need to know if the signal is present or not.
For instance, I have a SuperH board using my CEU driver together with
one OV7725 camera or one TW9910 video decoder. Some revisions of the
board do not route the field signal between the SuperH on-chip CEU and
the TW9910. Both the CEU and the TW9910 support this signal, it just
happen to be missing. I think we need a way to include this board
specific property somehow.

/ magnus
--
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 RFC] adding support for setting bus parameters in sub device

2009-06-10 Thread Karicheri, Muralidharan

 
  +/*
  + * Some sub-devices are connected to the bridge device through a bus
  that + * carries * the clock, vsync, hsync and data. Some interfaces
  such as BT.656 + * carries the sync embedded in the data where as
  others have separate line + * carrying the sync signals. The structure
  below is used by bridge driver to + * set the desired bus parameters in
  the sub device to work with it. + */
  +enum v4l2_subdev_bus_type {
  +  /* BT.656 interface. Embedded sync */
  +  V4L2_SUBDEV_BUS_BT_656,
  +  /* BT.1120 interface. Embedded sync */
  +  V4L2_SUBDEV_BUS_BT_1120,
  +  /* 8 bit muxed YCbCr bus, separate sync and field signals */
  +  V4L2_SUBDEV_BUS_YCBCR_8,
  +  /* 16 bit YCbCr bus, separate sync and field signals */
  +  V4L2_SUBDEV_BUS_YCBCR_16,

 Hmm, what do you mean with 8 bit muxed YCbCr bus? It's not clear to me
 what the format of these YCBCR bus types is exactly.

[MK]I spent sometime yesterday looking at different interfaces that we support 
in our soc. Here is the list...
BT.656, which is 8 bit or 10 bit multiplexed YCbCr bus
BT.1120, which is 16 bit or 20 bit YCbCr bus
YUV bus with separate sync signals (hsync, vsync and field)which could be 8 
bit, 10 bit, 16 bit and 20 bit
Since BT_656  BT_1120 also carries YUV data, we could call them  as YUV bus. 
So we can classify bus type based on the type of data it carries as below..

enum v4l2_subdev_bus_type {
/* Raw YUV image data bus, such as BT.656, BT.1120, or with
 * separate sync
 */
V4L2_SUBDEV_BUS_RAW_YUV,
  /* Raw Bayer image data bus , 8 - 16 bit wide, sync signals */
V4L2_SUBDEV_BUS_RAW_BAYER
};

Since we have width to describe the bus size, we could define all of the above 
bus types using above bus types and width. In addition we could add another 
Boolean to describe if sync is sent embedded or separate as below.
Just want to do it right. If anyone has any comments about the classification, 
please reply. I will sent an updated patch soon.

  +
  +struct v4l2_subdev_bus{
  +  enum v4l2_subdev_bus_type type;
  +  u8 width;
unsigned embedded_sync:1;
  +  /* 0 - active low, 1 - active high */
  +  unsigned pol_vsync:1;
  +  /* 0 - active low, 1 - active high */
  +  unsigned pol_hsync:1;
  +  /* 0 - low to high , 1 - high to low */
  +  unsigned pol_field:1;
  +  /* 0 - sample at falling edge , 1 - sample at rising edge */
  +  unsigned pol_pclock:1;
  +  /* 0 - active low , 1 - active high */
  +  unsigned pol_data:1;
  +};
  +
   /* Sub-devices are devices that are connected somehow to the main
  bridge device. These devices are usually audio/video
  muxers/encoders/decoders or sensors and webcam controllers.
  @@ -109,6 +144,7 @@ struct v4l2_subdev_core_ops {
 int (*querymenu)(struct v4l2_subdev *sd, struct v4l2_querymenu *qm);
 int (*s_std)(struct v4l2_subdev *sd, v4l2_std_id norm);
 long (*ioctl)(struct v4l2_subdev *sd, unsigned int cmd, void *arg);
  +  int (*s_bus)(struct v4l2_subdev *sd, struct v4l2_subdev_bus *bus);

 Make this 'const struct v4l2_subdev_bus *bus'.

And move it to the video ops. This op is only relevant for video, after all.
Yes, I know I said to add it to core initially; so sue me :-)

Regards,

   Hans


   #ifdef CONFIG_VIDEO_ADV_DEBUG
 int (*g_register)(struct v4l2_subdev *sd, struct v4l2_dbg_register
  *reg); int (*s_register)(struct v4l2_subdev *sd, struct
  v4l2_dbg_register *reg);

 Regards,

  Hans



--
Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom

--
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 RFC] adding support for setting bus parameters in sub device

2009-06-09 Thread Hans Verkuil
On Tuesday 09 June 2009 22:55:53 m-kariche...@ti.com wrote:
 From: Muralidharan Karicheri a0868...@gt516km11.gt.design.ti.com

 re-sending with RFC in the header

 This patch adds support for setting bus parameters such as bus type
 (BT.656, BT.1120 etc), width (example 10 bit raw image data bus)
 and polarities (vsync, hsync, field etc) in sub device. This allows
 bridge driver to configure the sub device for a specific set of bus
 parameters through s_bus() function call.

 Reviewed By Hans Verkuil.
 Signed-off-by: Muralidharan Karicheri m-kariche...@ti.com
 ---
 Applies to v4l-dvb repository

  include/media/v4l2-subdev.h |   36 
  1 files changed, 36 insertions(+), 0 deletions(-)

 diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
 index 1785608..c1cfb3b 100644
 --- a/include/media/v4l2-subdev.h
 +++ b/include/media/v4l2-subdev.h
 @@ -37,6 +37,41 @@ struct v4l2_decode_vbi_line {
   u32 type;   /* VBI service type (V4L2_SLICED_*). 0 if no 
 service found
 */ };

 +/*
 + * Some sub-devices are connected to the bridge device through a bus
 that + * carries * the clock, vsync, hsync and data. Some interfaces such
 as BT.656 + * carries the sync embedded in the data where as others have
 separate line + * carrying the sync signals. The structure below is used
 by bridge driver to + * set the desired bus parameters in the sub device
 to work with it. + */
 +enum v4l2_subdev_bus_type {
 + /* BT.656 interface. Embedded sync */
 + V4L2_SUBDEV_BUS_BT_656,
 + /* BT.1120 interface. Embedded sync */
 + V4L2_SUBDEV_BUS_BT_1120,
 + /* 8 bit muxed YCbCr bus, separate sync and field signals */
 + V4L2_SUBDEV_BUS_YCBCR_8,
 + /* 16 bit YCbCr bus, separate sync and field signals */
 + V4L2_SUBDEV_BUS_YCBCR_16,

Hmm, what do you mean with 8 bit muxed YCbCr bus? It's not clear to me 
what the format of these YCBCR bus types is exactly.

 + /* Raw Bayer image data bus , 8 - 16 bit wide, sync signals */
 + V4L2_SUBDEV_BUS_RAW_BAYER
 +};
 +
 +struct v4l2_subdev_bus   {
 + enum v4l2_subdev_bus_type type;
 + u8 width;
 + /* 0 - active low, 1 - active high */
 + unsigned pol_vsync:1;
 + /* 0 - active low, 1 - active high */
 + unsigned pol_hsync:1;
 + /* 0 - low to high , 1 - high to low */
 + unsigned pol_field:1;
 + /* 0 - sample at falling edge , 1 - sample at rising edge */
 + unsigned pol_pclock:1;
 + /* 0 - active low , 1 - active high */
 + unsigned pol_data:1;
 +};
 +
  /* Sub-devices are devices that are connected somehow to the main bridge
 device. These devices are usually audio/video
 muxers/encoders/decoders or sensors and webcam controllers.
 @@ -109,6 +144,7 @@ struct v4l2_subdev_core_ops {
   int (*querymenu)(struct v4l2_subdev *sd, struct v4l2_querymenu *qm);
   int (*s_std)(struct v4l2_subdev *sd, v4l2_std_id norm);
   long (*ioctl)(struct v4l2_subdev *sd, unsigned int cmd, void *arg);
 + int (*s_bus)(struct v4l2_subdev *sd, struct v4l2_subdev_bus *bus);

Make this 'const struct v4l2_subdev_bus *bus'.

  #ifdef CONFIG_VIDEO_ADV_DEBUG
   int (*g_register)(struct v4l2_subdev *sd, struct v4l2_dbg_register
 *reg); int (*s_register)(struct v4l2_subdev *sd, struct v4l2_dbg_register
 *reg);

Regards,

Hans

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom
--
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 RFC] adding support for setting bus parameters in sub device

2009-06-09 Thread Hans Verkuil
On Tuesday 09 June 2009 23:03:01 Hans Verkuil wrote:
 On Tuesday 09 June 2009 22:55:53 m-kariche...@ti.com wrote:
  From: Muralidharan Karicheri a0868...@gt516km11.gt.design.ti.com
 
  re-sending with RFC in the header
 
  This patch adds support for setting bus parameters such as bus type
  (BT.656, BT.1120 etc), width (example 10 bit raw image data bus)
  and polarities (vsync, hsync, field etc) in sub device. This allows
  bridge driver to configure the sub device for a specific set of bus
  parameters through s_bus() function call.
 
  Reviewed By Hans Verkuil.
  Signed-off-by: Muralidharan Karicheri m-kariche...@ti.com
  ---
  Applies to v4l-dvb repository
 
   include/media/v4l2-subdev.h |   36
   1 files changed, 36 insertions(+),
  0 deletions(-)
 
  diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
  index 1785608..c1cfb3b 100644
  --- a/include/media/v4l2-subdev.h
  +++ b/include/media/v4l2-subdev.h
  @@ -37,6 +37,41 @@ struct v4l2_decode_vbi_line {
  u32 type;   /* VBI service type (V4L2_SLICED_*). 0 if no 
  service found
  */ };
 
  +/*
  + * Some sub-devices are connected to the bridge device through a bus
  that + * carries * the clock, vsync, hsync and data. Some interfaces
  such as BT.656 + * carries the sync embedded in the data where as
  others have separate line + * carrying the sync signals. The structure
  below is used by bridge driver to + * set the desired bus parameters in
  the sub device to work with it. + */
  +enum v4l2_subdev_bus_type {
  +   /* BT.656 interface. Embedded sync */
  +   V4L2_SUBDEV_BUS_BT_656,
  +   /* BT.1120 interface. Embedded sync */
  +   V4L2_SUBDEV_BUS_BT_1120,
  +   /* 8 bit muxed YCbCr bus, separate sync and field signals */
  +   V4L2_SUBDEV_BUS_YCBCR_8,
  +   /* 16 bit YCbCr bus, separate sync and field signals */
  +   V4L2_SUBDEV_BUS_YCBCR_16,

 Hmm, what do you mean with 8 bit muxed YCbCr bus? It's not clear to me
 what the format of these YCBCR bus types is exactly.

  +   /* Raw Bayer image data bus , 8 - 16 bit wide, sync signals */
  +   V4L2_SUBDEV_BUS_RAW_BAYER
  +};
  +
  +struct v4l2_subdev_bus {
  +   enum v4l2_subdev_bus_type type;
  +   u8 width;
  +   /* 0 - active low, 1 - active high */
  +   unsigned pol_vsync:1;
  +   /* 0 - active low, 1 - active high */
  +   unsigned pol_hsync:1;
  +   /* 0 - low to high , 1 - high to low */
  +   unsigned pol_field:1;
  +   /* 0 - sample at falling edge , 1 - sample at rising edge */
  +   unsigned pol_pclock:1;
  +   /* 0 - active low , 1 - active high */
  +   unsigned pol_data:1;
  +};
  +
   /* Sub-devices are devices that are connected somehow to the main
  bridge device. These devices are usually audio/video
  muxers/encoders/decoders or sensors and webcam controllers.
  @@ -109,6 +144,7 @@ struct v4l2_subdev_core_ops {
  int (*querymenu)(struct v4l2_subdev *sd, struct v4l2_querymenu *qm);
  int (*s_std)(struct v4l2_subdev *sd, v4l2_std_id norm);
  long (*ioctl)(struct v4l2_subdev *sd, unsigned int cmd, void *arg);
  +   int (*s_bus)(struct v4l2_subdev *sd, struct v4l2_subdev_bus *bus);

 Make this 'const struct v4l2_subdev_bus *bus'.

And move it to the video ops. This op is only relevant for video, after all. 
Yes, I know I said to add it to core initially; so sue me :-)

Regards,

Hans


   #ifdef CONFIG_VIDEO_ADV_DEBUG
  int (*g_register)(struct v4l2_subdev *sd, struct v4l2_dbg_register
  *reg); int (*s_register)(struct v4l2_subdev *sd, struct
  v4l2_dbg_register *reg);

 Regards,

   Hans



-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom
--
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 RFC] adding support for setting bus parameters in sub device

2009-06-09 Thread Karicheri, Muralidharan

email: m-kariche...@ti.com

-Original Message-
From: Hans Verkuil [mailto:hverk...@xs4all.nl]
Sent: Tuesday, June 09, 2009 5:03 PM
To: Karicheri, Muralidharan
Cc: linux-media@vger.kernel.org; davinci-linux-open-
sou...@linux.davincidsp.com; Muralidharan Karicheri
Subject: Re: [PATCH RFC] adding support for setting bus parameters in sub
device

On Tuesday 09 June 2009 22:55:53 m-kariche...@ti.com wrote:
 From: Muralidharan Karicheri a0868...@gt516km11.gt.design.ti.com

 re-sending with RFC in the header

 This patch adds support for setting bus parameters such as bus type
 (BT.656, BT.1120 etc), width (example 10 bit raw image data bus)
 and polarities (vsync, hsync, field etc) in sub device. This allows
 bridge driver to configure the sub device for a specific set of bus
 parameters through s_bus() function call.

 Reviewed By Hans Verkuil.
 Signed-off-by: Muralidharan Karicheri m-kariche...@ti.com
 ---
 Applies to v4l-dvb repository

  include/media/v4l2-subdev.h |   36 
  1 files changed, 36 insertions(+), 0 deletions(-)

 diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
 index 1785608..c1cfb3b 100644
 --- a/include/media/v4l2-subdev.h
 +++ b/include/media/v4l2-subdev.h
 @@ -37,6 +37,41 @@ struct v4l2_decode_vbi_line {
  u32 type;   /* VBI service type (V4L2_SLICED_*). 0 if no
service found
 */ };

 +/*
 + * Some sub-devices are connected to the bridge device through a bus
 that + * carries * the clock, vsync, hsync and data. Some interfaces such
 as BT.656 + * carries the sync embedded in the data where as others have
 separate line + * carrying the sync signals. The structure below is used
 by bridge driver to + * set the desired bus parameters in the sub device
 to work with it. + */
 +enum v4l2_subdev_bus_type {
 +/* BT.656 interface. Embedded sync */
 +V4L2_SUBDEV_BUS_BT_656,
 +/* BT.1120 interface. Embedded sync */
 +V4L2_SUBDEV_BUS_BT_1120,
 +/* 8 bit muxed YCbCr bus, separate sync and field signals */
 +V4L2_SUBDEV_BUS_YCBCR_8,
 +/* 16 bit YCbCr bus, separate sync and field signals */
 +V4L2_SUBDEV_BUS_YCBCR_16,

Hmm, what do you mean with 8 bit muxed YCbCr bus? It's not clear to me
what the format of these YCBCR bus types is exactly.

[MK] For YCbCr16, there is separate bus to carry Y and CbCr data, where as on 
YCbCr8, both gets multiplexed over same 8 bit bus (Y, Cb, Y, Cr, Y, Cb The 
difference between V4L2_SUBDEV_BUS_BT_656 and V4L2_SUBDEV_BUS_YCBCR_8 is that 
sync is embedded with data in the former, where as there is dedicated sync 
lines for the latter.
 +/* Raw Bayer image data bus , 8 - 16 bit wide, sync signals */
 +V4L2_SUBDEV_BUS_RAW_BAYER
 +};
 +
 +struct v4l2_subdev_bus  {
 +enum v4l2_subdev_bus_type type;
 +u8 width;
 +/* 0 - active low, 1 - active high */
 +unsigned pol_vsync:1;
 +/* 0 - active low, 1 - active high */
 +unsigned pol_hsync:1;
 +/* 0 - low to high , 1 - high to low */
 +unsigned pol_field:1;
 +/* 0 - sample at falling edge , 1 - sample at rising edge */
 +unsigned pol_pclock:1;
 +/* 0 - active low , 1 - active high */
 +unsigned pol_data:1;
 +};
 +
  /* Sub-devices are devices that are connected somehow to the main bridge
 device. These devices are usually audio/video
 muxers/encoders/decoders or sensors and webcam controllers.
 @@ -109,6 +144,7 @@ struct v4l2_subdev_core_ops {
  int (*querymenu)(struct v4l2_subdev *sd, struct v4l2_querymenu *qm);
  int (*s_std)(struct v4l2_subdev *sd, v4l2_std_id norm);
  long (*ioctl)(struct v4l2_subdev *sd, unsigned int cmd, void *arg);
 +int (*s_bus)(struct v4l2_subdev *sd, struct v4l2_subdev_bus *bus);

Make this 'const struct v4l2_subdev_bus *bus'.

Ok.
  #ifdef CONFIG_VIDEO_ADV_DEBUG
  int (*g_register)(struct v4l2_subdev *sd, struct v4l2_dbg_register
 *reg); int (*s_register)(struct v4l2_subdev *sd, struct v4l2_dbg_register
 *reg);

Regards,

   Hans

--
Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom

--
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