V4L: soc-camera: provide support for S_INPUT.

  Some v4l-subdevs such as tvp5150 have multiple
inputs. This patch allows the user of a soc-camera
device to select between them.

Signed-off-by: Javier Martin <javier.martin@vista-silicon.com>
---
 drivers/media/video/soc_camera.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
  

2011/12/16 Javier Martin <javier.martin@vista-silicon.com>:
> Some v4l-subdevs such as tvp5150 have multiple
> inputs. This patch allows the user of a soc-camera
> device to select between them.
>
> Signed-off-by: Javier Martin <javier.martin@vista-silicon.com>
> ---
>  drivers/media/video/soc_camera.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c
> index b72580c..1cea1a9 100644
> --- a/drivers/media/video/soc_camera.c
> +++ b/drivers/media/video/soc_camera.c
> @@ -235,10 +235,10 @@ static int soc_camera_g_input(struct file *file, void *priv, unsigned int *i)
>
>  static int soc_camera_s_input(struct file *file, void *priv, unsigned int i)
>  {
> -       if (i > 0)
> -               return -EINVAL;
should it check max input?

> +       struct soc_camera_device *icd = file->private_data;
> +       struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
>
> -       return 0;
> +       return v4l2_subdev_call(sd, video, s_routing, i, 0, 0);
>  }
>
why must output be zero?

Regards,
Scott
--
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
  

Hi Javier

On Fri, 16 Dec 2011, Javier Martin wrote:

> Some v4l-subdevs such as tvp5150 have multiple
> inputs. This patch allows the user of a soc-camera
> device to select between them.

Sure, we can support it. But I've got a couple of remarks:

First question: you probably also want to patch soc_camera_g_input() and 
soc_camera_enum_input(). But no, I do not know how. The video subdevice 
operations do not seem to provide a way to query subdevice routing 
capabilities, so, I've got no idea how we're supposed to support 
enum_input(). There is a g_input_status() method, but I'm not sure about 
its semantics. Would it return an error like -EINVAL on an unsupported 
index?

Secondly, I would prefer to keep the current behaviour per default. I.e., 
if the subdevice doesn't implement routing- / input-related operations, we 
should act as before - assume input 0 and return success.

Thanks
Guennadi

> 
> Signed-off-by: Javier Martin <javier.martin@vista-silicon.com>
> ---
>  drivers/media/video/soc_camera.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c
> index b72580c..1cea1a9 100644
> --- a/drivers/media/video/soc_camera.c
> +++ b/drivers/media/video/soc_camera.c
> @@ -235,10 +235,10 @@ static int soc_camera_g_input(struct file *file, void *priv, unsigned int *i)
>  
>  static int soc_camera_s_input(struct file *file, void *priv, unsigned int i)
>  {
> -	if (i > 0)
> -		return -EINVAL;
> +	struct soc_camera_device *icd = file->private_data;
> +	struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
>  
> -	return 0;
> +	return v4l2_subdev_call(sd, video, s_routing, i, 0, 0);
>  }
>  
>  static int soc_camera_s_std(struct file *file, void *priv, v4l2_std_id *a)
> -- 
> 1.7.0.4
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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
  

Hi Guennadi,

> First question: you probably also want to patch soc_camera_g_input() and
> soc_camera_enum_input(). But no, I do not know how. The video subdevice
> operations do not seem to provide a way to query subdevice routing
> capabilities, so, I've got no idea how we're supposed to support
> enum_input(). There is a g_input_status() method, but I'm not sure about
> its semantics. Would it return an error like -EINVAL on an unsupported
> index?
>
static int bcap_enum_input(struct file *file, void *priv,
                                struct v4l2_input *input)
{
        struct bcap_device *bcap_dev = video_drvdata(file);
        struct bfin_capture_config *config = bcap_dev->cfg;
        int ret;
        u32 status;

        if (input->index >= config->num_inputs)
                return -EINVAL;

        *input = config->inputs[input->index];
        /* get input status */
        ret = v4l2_subdev_call(bcap_dev->sd, video, g_input_status, &status);
        if (!ret)
                input->status = status;
        return 0;
}

How about this implementation? I know it's not for soc, but I post it
to give my idea.
Bridge knows the layout, so it doesn't need to query the subdevice.
--
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
  

On Fri, 16 Dec 2011, Scott Jiang wrote:

> Hi Guennadi,
> 
> > First question: you probably also want to patch soc_camera_g_input() and
> > soc_camera_enum_input(). But no, I do not know how. The video subdevice
> > operations do not seem to provide a way to query subdevice routing
> > capabilities, so, I've got no idea how we're supposed to support
> > enum_input(). There is a g_input_status() method, but I'm not sure about
> > its semantics. Would it return an error like -EINVAL on an unsupported
> > index?
> >
> static int bcap_enum_input(struct file *file, void *priv,
>                                 struct v4l2_input *input)
> {
>         struct bcap_device *bcap_dev = video_drvdata(file);
>         struct bfin_capture_config *config = bcap_dev->cfg;
>         int ret;
>         u32 status;
> 
>         if (input->index >= config->num_inputs)
>                 return -EINVAL;
> 
>         *input = config->inputs[input->index];
>         /* get input status */
>         ret = v4l2_subdev_call(bcap_dev->sd, video, g_input_status, &status);
>         if (!ret)
>                 input->status = status;
>         return 0;
> }
> 
> How about this implementation? I know it's not for soc, but I post it
> to give my idea.
> Bridge knows the layout, so it doesn't need to query the subdevice.

Where from? AFAIU, we are talking here about subdevice inputs, right? In 
this case about various inputs of the TV decoder. How shall the bridge 
driver know about that?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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
  

On 16 December 2011 09:34, Scott Jiang <scott.jiang.linux@gmail.com> wrote:
> 2011/12/16 Javier Martin <javier.martin@vista-silicon.com>:
>> Some v4l-subdevs such as tvp5150 have multiple
>> inputs. This patch allows the user of a soc-camera
>> device to select between them.
>>
>> Signed-off-by: Javier Martin <javier.martin@vista-silicon.com>
>> ---
>>  drivers/media/video/soc_camera.c |    6 +++---
>>  1 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c
>> index b72580c..1cea1a9 100644
>> --- a/drivers/media/video/soc_camera.c
>> +++ b/drivers/media/video/soc_camera.c
>> @@ -235,10 +235,10 @@ static int soc_camera_g_input(struct file *file, void *priv, unsigned int *i)
>>
>>  static int soc_camera_s_input(struct file *file, void *priv, unsigned int i)
>>  {
>> -       if (i > 0)
>> -               return -EINVAL;
> should it check max input?
>
>> +       struct soc_camera_device *icd = file->private_data;
>> +       struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
>>
>> -       return 0;
>> +       return v4l2_subdev_call(sd, video, s_routing, i, 0, 0);
>>  }
>>
> why must output be zero?

Current behavior of soc-camera is not touching output at all, but
s_routing() callback enforces to select both input and output values.
The more neutral value I could think of for output is 0. Of course any
other suggestions are welcome.
  

On 16 December 2011 09:47, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
> Hi Javier
>
> On Fri, 16 Dec 2011, Javier Martin wrote:
>
>> Some v4l-subdevs such as tvp5150 have multiple
>> inputs. This patch allows the user of a soc-camera
>> device to select between them.
>
> Sure, we can support it. But I've got a couple of remarks:
>
> First question: you probably also want to patch soc_camera_g_input() and
> soc_camera_enum_input(). But no, I do not know how. The video subdevice
> operations do not seem to provide a way to query subdevice routing
> capabilities, so, I've got no idea how we're supposed to support
> enum_input(). There is a g_input_status() method, but I'm not sure about
> its semantics. Would it return an error like -EINVAL on an unsupported
> index?

I would gladly implement also eum_input() and get_input() but I can't
test it since my device does not support "g_input_status" callback but
only "s_routing".

> Secondly, I would prefer to keep the current behaviour per default. I.e.,
> if the subdevice doesn't implement routing- / input-related operations, we
> should act as before - assume input 0 and return success.

Sure, it seems reasonable.
  

>> How about this implementation? I know it's not for soc, but I post it
>> to give my idea.
>> Bridge knows the layout, so it doesn't need to query the subdevice.
>
> Where from? AFAIU, we are talking here about subdevice inputs, right? In
> this case about various inputs of the TV decoder. How shall the bridge
> driver know about that?

I have asked this question before. Laurent reply me:

> >> ENUMINPUT as defined by V4L2 enumerates input connectors available on
> >> the board. Which inputs the board designer hooked up is something that
> >> only the top-level V4L driver will know. Subdevices do not have that
> >> information, so enuminputs is not applicable there.
> >>
> >> Of course, subdevices do have input pins and output pins, but these are
> >> assumed to be fixed. With the s_routing ops the top level driver selects
> >> which input and output pins are active. Enumeration of those inputs and
> >> outputs wouldn't gain you anything as far as I can tell since the
> >> subdevice simply does not know which inputs/outputs are actually hooked
> >> up. It's the top level driver that has that information (usually passed
> >> in through board/card info structures).

Regards,
Scott
--
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
  

On 16 December 2011 10:36, Scott Jiang <scott.jiang.linux@gmail.com> wrote:
>>> How about this implementation? I know it's not for soc, but I post it
>>> to give my idea.
>>> Bridge knows the layout, so it doesn't need to query the subdevice.
>>
>> Where from? AFAIU, we are talking here about subdevice inputs, right? In
>> this case about various inputs of the TV decoder. How shall the bridge
>> driver know about that?
>
> I have asked this question before. Laurent reply me:
>
>> >> ENUMINPUT as defined by V4L2 enumerates input connectors available on
>> >> the board. Which inputs the board designer hooked up is something that
>> >> only the top-level V4L driver will know. Subdevices do not have that
>> >> information, so enuminputs is not applicable there.
>> >>
>> >> Of course, subdevices do have input pins and output pins, but these are
>> >> assumed to be fixed. With the s_routing ops the top level driver selects
>> >> which input and output pins are active. Enumeration of those inputs and
>> >> outputs wouldn't gain you anything as far as I can tell since the
>> >> subdevice simply does not know which inputs/outputs are actually hooked
>> >> up. It's the top level driver that has that information (usually passed
>> >> in through board/card info structures).

But AFAIK, soc-camera does not support passing such kind of information.
Neither any of the host camera drivers currently available expect that
kind of data.
  

On Fri, 16 Dec 2011, Scott Jiang wrote:

> >> How about this implementation? I know it's not for soc, but I post it
> >> to give my idea.
> >> Bridge knows the layout, so it doesn't need to query the subdevice.
> >
> > Where from? AFAIU, we are talking here about subdevice inputs, right? In
> > this case about various inputs of the TV decoder. How shall the bridge
> > driver know about that?
> 
> I have asked this question before. Laurent reply me:
> 
> > >> ENUMINPUT as defined by V4L2 enumerates input connectors available on
> > >> the board. Which inputs the board designer hooked up is something that
> > >> only the top-level V4L driver will know. Subdevices do not have that
> > >> information, so enuminputs is not applicable there.
> > >>
> > >> Of course, subdevices do have input pins and output pins, but these are
> > >> assumed to be fixed. With the s_routing ops the top level driver selects
> > >> which input and output pins are active. Enumeration of those inputs and
> > >> outputs wouldn't gain you anything as far as I can tell since the
> > >> subdevice simply does not know which inputs/outputs are actually hooked
> > >> up. It's the top level driver that has that information (usually passed
> > >> in through board/card info structures).

Laurent, right, I now remember reading this discussion before. But I'm not 
sure I completely agree:-) Yes, you're right - the board decides which 
pins are routed to which connectors. And it has to provide this 
information to the driver in its platform data. But - I think, this 
information should be provided not to the bridge driver, but to respective 
subdevice drivers, because only they know what exactly those interfaces 
are good for and how to report them to the bridge or the user, if we 
decide to also export this information over the subdevice user-space API.

So, I would say, the board has to tell the subdevice driver: yes, your 
inputs 0 and 1 are routed to external connectors. On input 1 I've put a 
pullup, it is connected to connector of type X over a circuit Y, clocked 
from your output Z, if the driver needs to know all that. And the subdev 
driver will just tell the bridge only what that one needs to know - number 
of inputs and their capabilities.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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
  

Hi Guennadi,

On Friday 16 December 2011 10:50:21 Guennadi Liakhovetski wrote:
> On Fri, 16 Dec 2011, Scott Jiang wrote:
> > >> How about this implementation? I know it's not for soc, but I post it
> > >> to give my idea.
> > >> Bridge knows the layout, so it doesn't need to query the subdevice.
> > > 
> > > Where from? AFAIU, we are talking here about subdevice inputs, right?
> > > In this case about various inputs of the TV decoder. How shall the
> > > bridge driver know about that?
> > 
> > I have asked this question before. Laurent reply me:
> > > >> ENUMINPUT as defined by V4L2 enumerates input connectors available
> > > >> on the board. Which inputs the board designer hooked up is
> > > >> something that only the top-level V4L driver will know. Subdevices
> > > >> do not have that information, so enuminputs is not applicable
> > > >> there.
> > > >> 
> > > >> Of course, subdevices do have input pins and output pins, but these
> > > >> are assumed to be fixed. With the s_routing ops the top level
> > > >> driver selects which input and output pins are active. Enumeration
> > > >> of those inputs and outputs wouldn't gain you anything as far as I
> > > >> can tell since the subdevice simply does not know which
> > > >> inputs/outputs are actually hooked up. It's the top level driver
> > > >> that has that information (usually passed in through board/card
> > > >> info structures).
> 
> Laurent, right, I now remember reading this discussion before. But I'm not
> sure I completely agree:-) Yes, you're right - the board decides which
> pins are routed to which connectors. And it has to provide this
> information to the driver in its platform data. But - I think, this
> information should be provided not to the bridge driver, but to respective
> subdevice drivers, because only they know what exactly those interfaces
> are good for and how to report them to the bridge or the user, if we
> decide to also export this information over the subdevice user-space API.
> 
> So, I would say, the board has to tell the subdevice driver: yes, your
> inputs 0 and 1 are routed to external connectors. On input 1 I've put a
> pullup, it is connected to connector of type X over a circuit Y, clocked
> from your output Z, if the driver needs to know all that. And the subdev
> driver will just tell the bridge only what that one needs to know - number
> of inputs and their capabilities.

That sounds reasonable.
  

Hi,
thank you for your comments.

Let me try to summarize the conclusions we've agreed here:
1.- soc-camera can support S_INPUT as long as I provide backwards
compatibility in case subdev does not support s_routing (i.e. I must
resend my patch returning input 0 in case s_routing is not supported).
2.- Board specific code must tell the subdevice which inputs are
really connected and how through platform data.

Is that OK?
  

On Mon, 19 Dec 2011, Laurent Pinchart wrote:

> Hi Guennadi,
> 
> On Friday 16 December 2011 10:50:21 Guennadi Liakhovetski wrote:
> > On Fri, 16 Dec 2011, Scott Jiang wrote:
> > > >> How about this implementation? I know it's not for soc, but I post it
> > > >> to give my idea.
> > > >> Bridge knows the layout, so it doesn't need to query the subdevice.
> > > > 
> > > > Where from? AFAIU, we are talking here about subdevice inputs, right?
> > > > In this case about various inputs of the TV decoder. How shall the
> > > > bridge driver know about that?
> > > 
> > > I have asked this question before. Laurent reply me:
> > > > >> ENUMINPUT as defined by V4L2 enumerates input connectors available
> > > > >> on the board. Which inputs the board designer hooked up is
> > > > >> something that only the top-level V4L driver will know. Subdevices
> > > > >> do not have that information, so enuminputs is not applicable
> > > > >> there.
> > > > >> 
> > > > >> Of course, subdevices do have input pins and output pins, but these
> > > > >> are assumed to be fixed. With the s_routing ops the top level
> > > > >> driver selects which input and output pins are active. Enumeration
> > > > >> of those inputs and outputs wouldn't gain you anything as far as I
> > > > >> can tell since the subdevice simply does not know which
> > > > >> inputs/outputs are actually hooked up. It's the top level driver
> > > > >> that has that information (usually passed in through board/card
> > > > >> info structures).
> > 
> > Laurent, right, I now remember reading this discussion before. But I'm not
> > sure I completely agree:-) Yes, you're right - the board decides which
> > pins are routed to which connectors. And it has to provide this
> > information to the driver in its platform data. But - I think, this
> > information should be provided not to the bridge driver, but to respective
> > subdevice drivers, because only they know what exactly those interfaces
> > are good for and how to report them to the bridge or the user, if we
> > decide to also export this information over the subdevice user-space API.
> > 
> > So, I would say, the board has to tell the subdevice driver: yes, your
> > inputs 0 and 1 are routed to external connectors. On input 1 I've put a
> > pullup, it is connected to connector of type X over a circuit Y, clocked
> > from your output Z, if the driver needs to know all that. And the subdev
> > driver will just tell the bridge only what that one needs to know - number
> > of inputs and their capabilities.
> 
> That sounds reasonable.

Good, this would mean, we need additional subdevice operations along the 
lines of enum_input and enum_output, and maybe also g_input and g_output?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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
  

Hi Guennadi,

On Monday 19 December 2011 09:09:34 Guennadi Liakhovetski wrote:
> On Mon, 19 Dec 2011, Laurent Pinchart wrote:
> > On Friday 16 December 2011 10:50:21 Guennadi Liakhovetski wrote:
> > > On Fri, 16 Dec 2011, Scott Jiang wrote:
> > > > >> How about this implementation? I know it's not for soc, but I post
> > > > >> it to give my idea.
> > > > >> Bridge knows the layout, so it doesn't need to query the
> > > > >> subdevice.
> > > > > 
> > > > > Where from? AFAIU, we are talking here about subdevice inputs,
> > > > > right? In this case about various inputs of the TV decoder. How
> > > > > shall the bridge driver know about that?
> > > > 
> > > > I have asked this question before. Laurent reply me:
> > > > > >> ENUMINPUT as defined by V4L2 enumerates input connectors
> > > > > >> available on the board. Which inputs the board designer hooked
> > > > > >> up is something that only the top-level V4L driver will know.
> > > > > >> Subdevices do not have that information, so enuminputs is not
> > > > > >> applicable there.
> > > > > >> 
> > > > > >> Of course, subdevices do have input pins and output pins, but
> > > > > >> these are assumed to be fixed. With the s_routing ops the top
> > > > > >> level driver selects which input and output pins are active.
> > > > > >> Enumeration of those inputs and outputs wouldn't gain you
> > > > > >> anything as far as I can tell since the subdevice simply does
> > > > > >> not know which inputs/outputs are actually hooked up. It's the
> > > > > >> top level driver that has that information (usually passed in
> > > > > >> through board/card info structures).
> > > 
> > > Laurent, right, I now remember reading this discussion before. But I'm
> > > not sure I completely agree:-) Yes, you're right - the board decides
> > > which pins are routed to which connectors. And it has to provide this
> > > information to the driver in its platform data. But - I think, this
> > > information should be provided not to the bridge driver, but to
> > > respective subdevice drivers, because only they know what exactly
> > > those interfaces are good for and how to report them to the bridge or
> > > the user, if we decide to also export this information over the
> > > subdevice user-space API.
> > > 
> > > So, I would say, the board has to tell the subdevice driver: yes, your
> > > inputs 0 and 1 are routed to external connectors. On input 1 I've put a
> > > pullup, it is connected to connector of type X over a circuit Y,
> > > clocked from your output Z, if the driver needs to know all that. And
> > > the subdev driver will just tell the bridge only what that one needs
> > > to know - number of inputs and their capabilities.
> > 
> > That sounds reasonable.
> 
> Good, this would mean, we need additional subdevice operations along the
> lines of enum_input and enum_output, and maybe also g_input and g_output?

What about implementing pad support in the subdevice ? Input enumeration could 
then be performed without a subdev operation.
  

Hi Javier,

On Monday 19 December 2011 08:44:58 javier Martin wrote:
> Hi,
> thank you for your comments.
> 
> Let me try to summarize the conclusions we've agreed here:
> 1.- soc-camera can support S_INPUT as long as I provide backwards
> compatibility in case subdev does not support s_routing (i.e. I must
> resend my patch returning input 0 in case s_routing is not supported).

Yes. As Guennadi pointed out, we also need to support input enumeration. One 
possible solution for that is pad support in the subdev.

> 2.- Board specific code must tell the subdevice which inputs are
> really connected and how through platform data.

That's right. Please make sure that platform data only use static data and no 
callback function if possible, to make the transition to the device tree 
easier.

> Is that OK?
  

On Mon, 19 Dec 2011, Laurent Pinchart wrote:

> Hi Guennadi,
> 
> On Monday 19 December 2011 09:09:34 Guennadi Liakhovetski wrote:
> > On Mon, 19 Dec 2011, Laurent Pinchart wrote:
> > > On Friday 16 December 2011 10:50:21 Guennadi Liakhovetski wrote:
> > > > On Fri, 16 Dec 2011, Scott Jiang wrote:
> > > > > >> How about this implementation? I know it's not for soc, but I post
> > > > > >> it to give my idea.
> > > > > >> Bridge knows the layout, so it doesn't need to query the
> > > > > >> subdevice.
> > > > > > 
> > > > > > Where from? AFAIU, we are talking here about subdevice inputs,
> > > > > > right? In this case about various inputs of the TV decoder. How
> > > > > > shall the bridge driver know about that?
> > > > > 
> > > > > I have asked this question before. Laurent reply me:
> > > > > > >> ENUMINPUT as defined by V4L2 enumerates input connectors
> > > > > > >> available on the board. Which inputs the board designer hooked
> > > > > > >> up is something that only the top-level V4L driver will know.
> > > > > > >> Subdevices do not have that information, so enuminputs is not
> > > > > > >> applicable there.
> > > > > > >> 
> > > > > > >> Of course, subdevices do have input pins and output pins, but
> > > > > > >> these are assumed to be fixed. With the s_routing ops the top
> > > > > > >> level driver selects which input and output pins are active.
> > > > > > >> Enumeration of those inputs and outputs wouldn't gain you
> > > > > > >> anything as far as I can tell since the subdevice simply does
> > > > > > >> not know which inputs/outputs are actually hooked up. It's the
> > > > > > >> top level driver that has that information (usually passed in
> > > > > > >> through board/card info structures).
> > > > 
> > > > Laurent, right, I now remember reading this discussion before. But I'm
> > > > not sure I completely agree:-) Yes, you're right - the board decides
> > > > which pins are routed to which connectors. And it has to provide this
> > > > information to the driver in its platform data. But - I think, this
> > > > information should be provided not to the bridge driver, but to
> > > > respective subdevice drivers, because only they know what exactly
> > > > those interfaces are good for and how to report them to the bridge or
> > > > the user, if we decide to also export this information over the
> > > > subdevice user-space API.
> > > > 
> > > > So, I would say, the board has to tell the subdevice driver: yes, your
> > > > inputs 0 and 1 are routed to external connectors. On input 1 I've put a
> > > > pullup, it is connected to connector of type X over a circuit Y,
> > > > clocked from your output Z, if the driver needs to know all that. And
> > > > the subdev driver will just tell the bridge only what that one needs
> > > > to know - number of inputs and their capabilities.
> > > 
> > > That sounds reasonable.
> > 
> > Good, this would mean, we need additional subdevice operations along the
> > lines of enum_input and enum_output, and maybe also g_input and g_output?
> 
> What about implementing pad support in the subdevice ? Input enumeration could 
> then be performed without a subdev operation.

soc-camera doesn't support pad operations yet.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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
  

Hi Guennadi,

On Monday 19 December 2011 11:13:58 Guennadi Liakhovetski wrote:
> On Mon, 19 Dec 2011, Laurent Pinchart wrote:
> > On Monday 19 December 2011 09:09:34 Guennadi Liakhovetski wrote:
> > > On Mon, 19 Dec 2011, Laurent Pinchart wrote:
> > > > On Friday 16 December 2011 10:50:21 Guennadi Liakhovetski wrote:
> > > > > On Fri, 16 Dec 2011, Scott Jiang wrote:
> > > > > > >> How about this implementation? I know it's not for soc, but I
> > > > > > >> post it to give my idea.
> > > > > > >> Bridge knows the layout, so it doesn't need to query the
> > > > > > >> subdevice.
> > > > > > > 
> > > > > > > Where from? AFAIU, we are talking here about subdevice inputs,
> > > > > > > right? In this case about various inputs of the TV decoder. How
> > > > > > > shall the bridge driver know about that?
> > > > > > 
> > > > > > I have asked this question before. Laurent reply me:
> > > > > > > >> ENUMINPUT as defined by V4L2 enumerates input connectors
> > > > > > > >> available on the board. Which inputs the board designer
> > > > > > > >> hooked up is something that only the top-level V4L driver
> > > > > > > >> will know. Subdevices do not have that information, so
> > > > > > > >> enuminputs is not applicable there.
> > > > > > > >> 
> > > > > > > >> Of course, subdevices do have input pins and output pins,
> > > > > > > >> but these are assumed to be fixed. With the s_routing ops
> > > > > > > >> the top level driver selects which input and output pins
> > > > > > > >> are active. Enumeration of those inputs and outputs
> > > > > > > >> wouldn't gain you anything as far as I can tell since the
> > > > > > > >> subdevice simply does not know which inputs/outputs are
> > > > > > > >> actually hooked up. It's the top level driver that has that
> > > > > > > >> information (usually passed in through board/card info
> > > > > > > >> structures).
> > > > > 
> > > > > Laurent, right, I now remember reading this discussion before. But
> > > > > I'm not sure I completely agree:-) Yes, you're right - the board
> > > > > decides which pins are routed to which connectors. And it has to
> > > > > provide this information to the driver in its platform data. But -
> > > > > I think, this information should be provided not to the bridge
> > > > > driver, but to respective subdevice drivers, because only they
> > > > > know what exactly those interfaces are good for and how to report
> > > > > them to the bridge or the user, if we decide to also export this
> > > > > information over the subdevice user-space API.
> > > > > 
> > > > > So, I would say, the board has to tell the subdevice driver: yes,
> > > > > your inputs 0 and 1 are routed to external connectors. On input 1
> > > > > I've put a pullup, it is connected to connector of type X over a
> > > > > circuit Y, clocked from your output Z, if the driver needs to know
> > > > > all that. And the subdev driver will just tell the bridge only
> > > > > what that one needs to know - number of inputs and their
> > > > > capabilities.
> > > > 
> > > > That sounds reasonable.
> > > 
> > > Good, this would mean, we need additional subdevice operations along
> > > the lines of enum_input and enum_output, and maybe also g_input and
> > > g_output?
> > 
> > What about implementing pad support in the subdevice ? Input enumeration
> > could then be performed without a subdev operation.
> 
> soc-camera doesn't support pad operations yet.

soc-camera doesn't support enum_input yet either, so you need to implement 
something anyway ;-)

You wouldn't need to call a pad operation here, you would just need to iterate 
through the pads provided by the subdev.
  

On Mon, 19 Dec 2011, Laurent Pinchart wrote:

> Hi Guennadi,
> 
> On Monday 19 December 2011 11:13:58 Guennadi Liakhovetski wrote:
> > On Mon, 19 Dec 2011, Laurent Pinchart wrote:
> > > On Monday 19 December 2011 09:09:34 Guennadi Liakhovetski wrote:
> > > > On Mon, 19 Dec 2011, Laurent Pinchart wrote:
> > > > > On Friday 16 December 2011 10:50:21 Guennadi Liakhovetski wrote:
> > > > > > On Fri, 16 Dec 2011, Scott Jiang wrote:
> > > > > > > >> How about this implementation? I know it's not for soc, but I
> > > > > > > >> post it to give my idea.
> > > > > > > >> Bridge knows the layout, so it doesn't need to query the
> > > > > > > >> subdevice.
> > > > > > > > 
> > > > > > > > Where from? AFAIU, we are talking here about subdevice inputs,
> > > > > > > > right? In this case about various inputs of the TV decoder. How
> > > > > > > > shall the bridge driver know about that?
> > > > > > > 
> > > > > > > I have asked this question before. Laurent reply me:
> > > > > > > > >> ENUMINPUT as defined by V4L2 enumerates input connectors
> > > > > > > > >> available on the board. Which inputs the board designer
> > > > > > > > >> hooked up is something that only the top-level V4L driver
> > > > > > > > >> will know. Subdevices do not have that information, so
> > > > > > > > >> enuminputs is not applicable there.
> > > > > > > > >> 
> > > > > > > > >> Of course, subdevices do have input pins and output pins,
> > > > > > > > >> but these are assumed to be fixed. With the s_routing ops
> > > > > > > > >> the top level driver selects which input and output pins
> > > > > > > > >> are active. Enumeration of those inputs and outputs
> > > > > > > > >> wouldn't gain you anything as far as I can tell since the
> > > > > > > > >> subdevice simply does not know which inputs/outputs are
> > > > > > > > >> actually hooked up. It's the top level driver that has that
> > > > > > > > >> information (usually passed in through board/card info
> > > > > > > > >> structures).
> > > > > > 
> > > > > > Laurent, right, I now remember reading this discussion before. But
> > > > > > I'm not sure I completely agree:-) Yes, you're right - the board
> > > > > > decides which pins are routed to which connectors. And it has to
> > > > > > provide this information to the driver in its platform data. But -
> > > > > > I think, this information should be provided not to the bridge
> > > > > > driver, but to respective subdevice drivers, because only they
> > > > > > know what exactly those interfaces are good for and how to report
> > > > > > them to the bridge or the user, if we decide to also export this
> > > > > > information over the subdevice user-space API.
> > > > > > 
> > > > > > So, I would say, the board has to tell the subdevice driver: yes,
> > > > > > your inputs 0 and 1 are routed to external connectors. On input 1
> > > > > > I've put a pullup, it is connected to connector of type X over a
> > > > > > circuit Y, clocked from your output Z, if the driver needs to know
> > > > > > all that. And the subdev driver will just tell the bridge only
> > > > > > what that one needs to know - number of inputs and their
> > > > > > capabilities.
> > > > > 
> > > > > That sounds reasonable.
> > > > 
> > > > Good, this would mean, we need additional subdevice operations along
> > > > the lines of enum_input and enum_output, and maybe also g_input and
> > > > g_output?
> > > 
> > > What about implementing pad support in the subdevice ? Input enumeration
> > > could then be performed without a subdev operation.
> > 
> > soc-camera doesn't support pad operations yet.
> 
> soc-camera doesn't support enum_input yet either, so you need to implement 
> something anyway ;-)
> 
> You wouldn't need to call a pad operation here, you would just need to iterate 
> through the pads provided by the subdev.

tvp5150 doesn't implement it either yet. So, I would say, it is a better 
solution ATM to fix this functionality independent of the pad-level API.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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
  

On 19 December 2011 11:41, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
> On Mon, 19 Dec 2011, Laurent Pinchart wrote:
>
>> Hi Guennadi,
>>
>> On Monday 19 December 2011 11:13:58 Guennadi Liakhovetski wrote:
>> > On Mon, 19 Dec 2011, Laurent Pinchart wrote:
>> > > On Monday 19 December 2011 09:09:34 Guennadi Liakhovetski wrote:
>> > > > On Mon, 19 Dec 2011, Laurent Pinchart wrote:
>> > > > > On Friday 16 December 2011 10:50:21 Guennadi Liakhovetski wrote:
>> > > > > > On Fri, 16 Dec 2011, Scott Jiang wrote:
>> > > > > > > >> How about this implementation? I know it's not for soc, but I
>> > > > > > > >> post it to give my idea.
>> > > > > > > >> Bridge knows the layout, so it doesn't need to query the
>> > > > > > > >> subdevice.
>> > > > > > > >
>> > > > > > > > Where from? AFAIU, we are talking here about subdevice inputs,
>> > > > > > > > right? In this case about various inputs of the TV decoder. How
>> > > > > > > > shall the bridge driver know about that?
>> > > > > > >
>> > > > > > > I have asked this question before. Laurent reply me:
>> > > > > > > > >> ENUMINPUT as defined by V4L2 enumerates input connectors
>> > > > > > > > >> available on the board. Which inputs the board designer
>> > > > > > > > >> hooked up is something that only the top-level V4L driver
>> > > > > > > > >> will know. Subdevices do not have that information, so
>> > > > > > > > >> enuminputs is not applicable there.
>> > > > > > > > >>
>> > > > > > > > >> Of course, subdevices do have input pins and output pins,
>> > > > > > > > >> but these are assumed to be fixed. With the s_routing ops
>> > > > > > > > >> the top level driver selects which input and output pins
>> > > > > > > > >> are active. Enumeration of those inputs and outputs
>> > > > > > > > >> wouldn't gain you anything as far as I can tell since the
>> > > > > > > > >> subdevice simply does not know which inputs/outputs are
>> > > > > > > > >> actually hooked up. It's the top level driver that has that
>> > > > > > > > >> information (usually passed in through board/card info
>> > > > > > > > >> structures).
>> > > > > >
>> > > > > > Laurent, right, I now remember reading this discussion before. But
>> > > > > > I'm not sure I completely agree:-) Yes, you're right - the board
>> > > > > > decides which pins are routed to which connectors. And it has to
>> > > > > > provide this information to the driver in its platform data. But -
>> > > > > > I think, this information should be provided not to the bridge
>> > > > > > driver, but to respective subdevice drivers, because only they
>> > > > > > know what exactly those interfaces are good for and how to report
>> > > > > > them to the bridge or the user, if we decide to also export this
>> > > > > > information over the subdevice user-space API.
>> > > > > >
>> > > > > > So, I would say, the board has to tell the subdevice driver: yes,
>> > > > > > your inputs 0 and 1 are routed to external connectors. On input 1
>> > > > > > I've put a pullup, it is connected to connector of type X over a
>> > > > > > circuit Y, clocked from your output Z, if the driver needs to know
>> > > > > > all that. And the subdev driver will just tell the bridge only
>> > > > > > what that one needs to know - number of inputs and their
>> > > > > > capabilities.
>> > > > >
>> > > > > That sounds reasonable.
>> > > >
>> > > > Good, this would mean, we need additional subdevice operations along
>> > > > the lines of enum_input and enum_output, and maybe also g_input and
>> > > > g_output?
>> > >
>> > > What about implementing pad support in the subdevice ? Input enumeration
>> > > could then be performed without a subdev operation.
>> >
>> > soc-camera doesn't support pad operations yet.
>>
>> soc-camera doesn't support enum_input yet either, so you need to implement
>> something anyway ;-)
>>
>> You wouldn't need to call a pad operation here, you would just need to iterate
>> through the pads provided by the subdev.
>
> tvp5150 doesn't implement it either yet. So, I would say, it is a better
> solution ATM to fix this functionality independent of the pad-level API.

I agree,
I cannot contribute to implement pad-level API stuff since I can't
test it with tvp5150.

Would you accept a patch implementing only S_INPUT?
  

On Mon, 19 Dec 2011, javier Martin wrote:

> On 19 December 2011 11:41, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
> > On Mon, 19 Dec 2011, Laurent Pinchart wrote:
> >
> >> Hi Guennadi,
> >>
> >> On Monday 19 December 2011 11:13:58 Guennadi Liakhovetski wrote:
> >> > On Mon, 19 Dec 2011, Laurent Pinchart wrote:
> >> > > On Monday 19 December 2011 09:09:34 Guennadi Liakhovetski wrote:
> >> > > > On Mon, 19 Dec 2011, Laurent Pinchart wrote:
> >> > > > > On Friday 16 December 2011 10:50:21 Guennadi Liakhovetski wrote:
> >> > > > > > On Fri, 16 Dec 2011, Scott Jiang wrote:
> >> > > > > > > >> How about this implementation? I know it's not for soc, but I
> >> > > > > > > >> post it to give my idea.
> >> > > > > > > >> Bridge knows the layout, so it doesn't need to query the
> >> > > > > > > >> subdevice.
> >> > > > > > > >
> >> > > > > > > > Where from? AFAIU, we are talking here about subdevice inputs,
> >> > > > > > > > right? In this case about various inputs of the TV decoder. How
> >> > > > > > > > shall the bridge driver know about that?
> >> > > > > > >
> >> > > > > > > I have asked this question before. Laurent reply me:
> >> > > > > > > > >> ENUMINPUT as defined by V4L2 enumerates input connectors
> >> > > > > > > > >> available on the board. Which inputs the board designer
> >> > > > > > > > >> hooked up is something that only the top-level V4L driver
> >> > > > > > > > >> will know. Subdevices do not have that information, so
> >> > > > > > > > >> enuminputs is not applicable there.
> >> > > > > > > > >>
> >> > > > > > > > >> Of course, subdevices do have input pins and output pins,
> >> > > > > > > > >> but these are assumed to be fixed. With the s_routing ops
> >> > > > > > > > >> the top level driver selects which input and output pins
> >> > > > > > > > >> are active. Enumeration of those inputs and outputs
> >> > > > > > > > >> wouldn't gain you anything as far as I can tell since the
> >> > > > > > > > >> subdevice simply does not know which inputs/outputs are
> >> > > > > > > > >> actually hooked up. It's the top level driver that has that
> >> > > > > > > > >> information (usually passed in through board/card info
> >> > > > > > > > >> structures).
> >> > > > > >
> >> > > > > > Laurent, right, I now remember reading this discussion before. But
> >> > > > > > I'm not sure I completely agree:-) Yes, you're right - the board
> >> > > > > > decides which pins are routed to which connectors. And it has to
> >> > > > > > provide this information to the driver in its platform data. But -
> >> > > > > > I think, this information should be provided not to the bridge
> >> > > > > > driver, but to respective subdevice drivers, because only they
> >> > > > > > know what exactly those interfaces are good for and how to report
> >> > > > > > them to the bridge or the user, if we decide to also export this
> >> > > > > > information over the subdevice user-space API.
> >> > > > > >
> >> > > > > > So, I would say, the board has to tell the subdevice driver: yes,
> >> > > > > > your inputs 0 and 1 are routed to external connectors. On input 1
> >> > > > > > I've put a pullup, it is connected to connector of type X over a
> >> > > > > > circuit Y, clocked from your output Z, if the driver needs to know
> >> > > > > > all that. And the subdev driver will just tell the bridge only
> >> > > > > > what that one needs to know - number of inputs and their
> >> > > > > > capabilities.
> >> > > > >
> >> > > > > That sounds reasonable.
> >> > > >
> >> > > > Good, this would mean, we need additional subdevice operations along
> >> > > > the lines of enum_input and enum_output, and maybe also g_input and
> >> > > > g_output?
> >> > >
> >> > > What about implementing pad support in the subdevice ? Input enumeration
> >> > > could then be performed without a subdev operation.
> >> >
> >> > soc-camera doesn't support pad operations yet.
> >>
> >> soc-camera doesn't support enum_input yet either, so you need to implement
> >> something anyway ;-)
> >>
> >> You wouldn't need to call a pad operation here, you would just need to iterate
> >> through the pads provided by the subdev.
> >
> > tvp5150 doesn't implement it either yet. So, I would say, it is a better
> > solution ATM to fix this functionality independent of the pad-level API.
> 
> I agree,
> I cannot contribute to implement pad-level API stuff since I can't
> test it with tvp5150.
> 
> Would you accept a patch implementing only S_INPUT?

Sorry, maybe I'm missing something, but how would it work? I mean, how can 
we accept from the user any S_INPUT request with index != 0, if we always 
return only 0 in reply to ENUM_INPUT? Ok, G_INPUT we could implement 
internally in soc-camera: return 0 by default, then remember last set 
input number per soc-camera device / subdev. But ENUM_INPUT?...

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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
  

On 19 December 2011 11:58, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
> On Mon, 19 Dec 2011, javier Martin wrote:
>
>> On 19 December 2011 11:41, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
>> > On Mon, 19 Dec 2011, Laurent Pinchart wrote:
>> >
>> >> Hi Guennadi,
>> >>
>> >> On Monday 19 December 2011 11:13:58 Guennadi Liakhovetski wrote:
>> >> > On Mon, 19 Dec 2011, Laurent Pinchart wrote:
>> >> > > On Monday 19 December 2011 09:09:34 Guennadi Liakhovetski wrote:
>> >> > > > On Mon, 19 Dec 2011, Laurent Pinchart wrote:
>> >> > > > > On Friday 16 December 2011 10:50:21 Guennadi Liakhovetski wrote:
>> >> > > > > > On Fri, 16 Dec 2011, Scott Jiang wrote:
>> >> > > > > > > >> How about this implementation? I know it's not for soc, but I
>> >> > > > > > > >> post it to give my idea.
>> >> > > > > > > >> Bridge knows the layout, so it doesn't need to query the
>> >> > > > > > > >> subdevice.
>> >> > > > > > > >
>> >> > > > > > > > Where from? AFAIU, we are talking here about subdevice inputs,
>> >> > > > > > > > right? In this case about various inputs of the TV decoder. How
>> >> > > > > > > > shall the bridge driver know about that?
>> >> > > > > > >
>> >> > > > > > > I have asked this question before. Laurent reply me:
>> >> > > > > > > > >> ENUMINPUT as defined by V4L2 enumerates input connectors
>> >> > > > > > > > >> available on the board. Which inputs the board designer
>> >> > > > > > > > >> hooked up is something that only the top-level V4L driver
>> >> > > > > > > > >> will know. Subdevices do not have that information, so
>> >> > > > > > > > >> enuminputs is not applicable there.
>> >> > > > > > > > >>
>> >> > > > > > > > >> Of course, subdevices do have input pins and output pins,
>> >> > > > > > > > >> but these are assumed to be fixed. With the s_routing ops
>> >> > > > > > > > >> the top level driver selects which input and output pins
>> >> > > > > > > > >> are active. Enumeration of those inputs and outputs
>> >> > > > > > > > >> wouldn't gain you anything as far as I can tell since the
>> >> > > > > > > > >> subdevice simply does not know which inputs/outputs are
>> >> > > > > > > > >> actually hooked up. It's the top level driver that has that
>> >> > > > > > > > >> information (usually passed in through board/card info
>> >> > > > > > > > >> structures).
>> >> > > > > >
>> >> > > > > > Laurent, right, I now remember reading this discussion before. But
>> >> > > > > > I'm not sure I completely agree:-) Yes, you're right - the board
>> >> > > > > > decides which pins are routed to which connectors. And it has to
>> >> > > > > > provide this information to the driver in its platform data. But -
>> >> > > > > > I think, this information should be provided not to the bridge
>> >> > > > > > driver, but to respective subdevice drivers, because only they
>> >> > > > > > know what exactly those interfaces are good for and how to report
>> >> > > > > > them to the bridge or the user, if we decide to also export this
>> >> > > > > > information over the subdevice user-space API.
>> >> > > > > >
>> >> > > > > > So, I would say, the board has to tell the subdevice driver: yes,
>> >> > > > > > your inputs 0 and 1 are routed to external connectors. On input 1
>> >> > > > > > I've put a pullup, it is connected to connector of type X over a
>> >> > > > > > circuit Y, clocked from your output Z, if the driver needs to know
>> >> > > > > > all that. And the subdev driver will just tell the bridge only
>> >> > > > > > what that one needs to know - number of inputs and their
>> >> > > > > > capabilities.
>> >> > > > >
>> >> > > > > That sounds reasonable.
>> >> > > >
>> >> > > > Good, this would mean, we need additional subdevice operations along
>> >> > > > the lines of enum_input and enum_output, and maybe also g_input and
>> >> > > > g_output?
>> >> > >
>> >> > > What about implementing pad support in the subdevice ? Input enumeration
>> >> > > could then be performed without a subdev operation.
>> >> >
>> >> > soc-camera doesn't support pad operations yet.
>> >>
>> >> soc-camera doesn't support enum_input yet either, so you need to implement
>> >> something anyway ;-)
>> >>
>> >> You wouldn't need to call a pad operation here, you would just need to iterate
>> >> through the pads provided by the subdev.
>> >
>> > tvp5150 doesn't implement it either yet. So, I would say, it is a better
>> > solution ATM to fix this functionality independent of the pad-level API.
>>
>> I agree,
>> I cannot contribute to implement pad-level API stuff since I can't
>> test it with tvp5150.
>>
>> Would you accept a patch implementing only S_INPUT?
>
> Sorry, maybe I'm missing something, but how would it work? I mean, how can
> we accept from the user any S_INPUT request with index != 0, if we always
> return only 0 in reply to ENUM_INPUT? Ok, G_INPUT we could implement
> internally in soc-camera: return 0 by default, then remember last set
> input number per soc-camera device / subdev. But ENUM_INPUT?...

It clearly is not a complete solution but at least it allows setting
input 0 in broken drivers such as tvp5150 which have input 1 enabled
by default, while soc-camera assumes input 0 is enabled.
  

On Mon, 19 Dec 2011, javier Martin wrote:

> On 19 December 2011 11:58, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
> > On Mon, 19 Dec 2011, javier Martin wrote:
> >
> >> On 19 December 2011 11:41, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
> >> > On Mon, 19 Dec 2011, Laurent Pinchart wrote:
> >> >
> >> >> Hi Guennadi,
> >> >>
> >> >> On Monday 19 December 2011 11:13:58 Guennadi Liakhovetski wrote:
> >> >> > On Mon, 19 Dec 2011, Laurent Pinchart wrote:
> >> >> > > On Monday 19 December 2011 09:09:34 Guennadi Liakhovetski wrote:
> >> >> > > > On Mon, 19 Dec 2011, Laurent Pinchart wrote:
> >> >> > > > > On Friday 16 December 2011 10:50:21 Guennadi Liakhovetski wrote:
> >> >> > > > > > On Fri, 16 Dec 2011, Scott Jiang wrote:
> >> >> > > > > > > >> How about this implementation? I know it's not for soc, but I
> >> >> > > > > > > >> post it to give my idea.
> >> >> > > > > > > >> Bridge knows the layout, so it doesn't need to query the
> >> >> > > > > > > >> subdevice.
> >> >> > > > > > > >
> >> >> > > > > > > > Where from? AFAIU, we are talking here about subdevice inputs,
> >> >> > > > > > > > right? In this case about various inputs of the TV decoder. How
> >> >> > > > > > > > shall the bridge driver know about that?
> >> >> > > > > > >
> >> >> > > > > > > I have asked this question before. Laurent reply me:
> >> >> > > > > > > > >> ENUMINPUT as defined by V4L2 enumerates input connectors
> >> >> > > > > > > > >> available on the board. Which inputs the board designer
> >> >> > > > > > > > >> hooked up is something that only the top-level V4L driver
> >> >> > > > > > > > >> will know. Subdevices do not have that information, so
> >> >> > > > > > > > >> enuminputs is not applicable there.
> >> >> > > > > > > > >>
> >> >> > > > > > > > >> Of course, subdevices do have input pins and output pins,
> >> >> > > > > > > > >> but these are assumed to be fixed. With the s_routing ops
> >> >> > > > > > > > >> the top level driver selects which input and output pins
> >> >> > > > > > > > >> are active. Enumeration of those inputs and outputs
> >> >> > > > > > > > >> wouldn't gain you anything as far as I can tell since the
> >> >> > > > > > > > >> subdevice simply does not know which inputs/outputs are
> >> >> > > > > > > > >> actually hooked up. It's the top level driver that has that
> >> >> > > > > > > > >> information (usually passed in through board/card info
> >> >> > > > > > > > >> structures).
> >> >> > > > > >
> >> >> > > > > > Laurent, right, I now remember reading this discussion before. But
> >> >> > > > > > I'm not sure I completely agree:-) Yes, you're right - the board
> >> >> > > > > > decides which pins are routed to which connectors. And it has to
> >> >> > > > > > provide this information to the driver in its platform data. But -
> >> >> > > > > > I think, this information should be provided not to the bridge
> >> >> > > > > > driver, but to respective subdevice drivers, because only they
> >> >> > > > > > know what exactly those interfaces are good for and how to report
> >> >> > > > > > them to the bridge or the user, if we decide to also export this
> >> >> > > > > > information over the subdevice user-space API.
> >> >> > > > > >
> >> >> > > > > > So, I would say, the board has to tell the subdevice driver: yes,
> >> >> > > > > > your inputs 0 and 1 are routed to external connectors. On input 1
> >> >> > > > > > I've put a pullup, it is connected to connector of type X over a
> >> >> > > > > > circuit Y, clocked from your output Z, if the driver needs to know
> >> >> > > > > > all that. And the subdev driver will just tell the bridge only
> >> >> > > > > > what that one needs to know - number of inputs and their
> >> >> > > > > > capabilities.
> >> >> > > > >
> >> >> > > > > That sounds reasonable.
> >> >> > > >
> >> >> > > > Good, this would mean, we need additional subdevice operations along
> >> >> > > > the lines of enum_input and enum_output, and maybe also g_input and
> >> >> > > > g_output?
> >> >> > >
> >> >> > > What about implementing pad support in the subdevice ? Input enumeration
> >> >> > > could then be performed without a subdev operation.
> >> >> >
> >> >> > soc-camera doesn't support pad operations yet.
> >> >>
> >> >> soc-camera doesn't support enum_input yet either, so you need to implement
> >> >> something anyway ;-)
> >> >>
> >> >> You wouldn't need to call a pad operation here, you would just need to iterate
> >> >> through the pads provided by the subdev.
> >> >
> >> > tvp5150 doesn't implement it either yet. So, I would say, it is a better
> >> > solution ATM to fix this functionality independent of the pad-level API.
> >>
> >> I agree,
> >> I cannot contribute to implement pad-level API stuff since I can't
> >> test it with tvp5150.
> >>
> >> Would you accept a patch implementing only S_INPUT?
> >
> > Sorry, maybe I'm missing something, but how would it work? I mean, how can
> > we accept from the user any S_INPUT request with index != 0, if we always
> > return only 0 in reply to ENUM_INPUT? Ok, G_INPUT we could implement
> > internally in soc-camera: return 0 by default, then remember last set
> > input number per soc-camera device / subdev. But ENUM_INPUT?...
> 
> It clearly is not a complete solution but at least it allows setting
> input 0 in broken drivers such as tvp5150 which have input 1 enabled
> by default, while soc-camera assumes input 0 is enabled.

I would really prefer an addition of an .enum_input() video subdev 
operation. Please, try to propose such a patch. If it absolutely gets 
rejected, maybe we can add a hack to soc-camera, returning -ENOSYS or 
-ENOIOCTLCMD in reply to ENUM_INPUT with index > 0, like saying "we have 
no idea, whether this device has any more inputs, than #0, try at your own 
risk." But this would be a user-visible change in behaviour, so, actually 
a bad thing (TM).

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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
  

Hi,

On Monday 19 December 2011 12:43:28 Guennadi Liakhovetski wrote:
> On Mon, 19 Dec 2011, javier Martin wrote:
> > On 19 December 2011 11:58, Guennadi Liakhovetski wrote:
> > > On Mon, 19 Dec 2011, javier Martin wrote:
> > >> On 19 December 2011 11:41, Guennadi Liakhovetski wrote:
> > >> > On Mon, 19 Dec 2011, Laurent Pinchart wrote:
> > >> >> On Monday 19 December 2011 11:13:58 Guennadi Liakhovetski wrote:
> > >> >> > On Mon, 19 Dec 2011, Laurent Pinchart wrote:
> > >> >> > > On Monday 19 December 2011 09:09:34 Guennadi Liakhovetski wrote:

[snip]

> > >> >> > > > Good, this would mean, we need additional subdevice
> > >> >> > > > operations along the lines of enum_input and enum_output,
> > >> >> > > > and maybe also g_input and g_output?
> > >> >> > > 
> > >> >> > > What about implementing pad support in the subdevice ? Input
> > >> >> > > enumeration could then be performed without a subdev
> > >> >> > > operation.
> > >> >> > 
> > >> >> > soc-camera doesn't support pad operations yet.
> > >> >> 
> > >> >> soc-camera doesn't support enum_input yet either, so you need to
> > >> >> implement something anyway ;-)
> > >> >> 
> > >> >> You wouldn't need to call a pad operation here, you would just need
> > >> >> to iterate through the pads provided by the subdev.
> > >> > 
> > >> > tvp5150 doesn't implement it either yet. So, I would say, it is a
> > >> > better solution ATM to fix this functionality independent of the
> > >> > pad-level API.
> > >> 
> > >> I agree,
> > >> I cannot contribute to implement pad-level API stuff since I can't
> > >> test it with tvp5150.
> > >> 
> > >> Would you accept a patch implementing only S_INPUT?
> > > 
> > > Sorry, maybe I'm missing something, but how would it work? I mean, how
> > > can we accept from the user any S_INPUT request with index != 0, if we
> > > always return only 0 in reply to ENUM_INPUT? Ok, G_INPUT we could
> > > implement internally in soc-camera: return 0 by default, then remember
> > > last set input number per soc-camera device / subdev. But
> > > ENUM_INPUT?...
> > 
> > It clearly is not a complete solution but at least it allows setting
> > input 0 in broken drivers such as tvp5150 which have input 1 enabled
> > by default, while soc-camera assumes input 0 is enabled.
> 
> I would really prefer an addition of an .enum_input() video subdev
> operation.

I agree that input enumeration is needed, but I really think this should be 
handled through pads, no with a new subdev operation. I don't like the idea of 
introducing a new operation that will already be deprecated from the very 
beginning.

Implementing this through pads isn't difficult. You don't need to implement 
any pad operation in the tvp5150 driver. All you need to do is setup an array 
of pads at probe time with information provided through platform data. soc-
camera should then just access the pads array and implement enum_input 
internally.

> Please, try to propose such a patch. If it absolutely gets
> rejected, maybe we can add a hack to soc-camera, returning -ENOSYS or
> -ENOIOCTLCMD in reply to ENUM_INPUT with index > 0, like saying "we have
> no idea, whether this device has any more inputs, than #0, try at your own
> risk." But this would be a user-visible change in behaviour, so, actually
> a bad thing (TM).
  

On Mon, 19 Dec 2011, Laurent Pinchart wrote:

> Hi,
> 
> On Monday 19 December 2011 12:43:28 Guennadi Liakhovetski wrote:
> > On Mon, 19 Dec 2011, javier Martin wrote:
> > > On 19 December 2011 11:58, Guennadi Liakhovetski wrote:
> > > > On Mon, 19 Dec 2011, javier Martin wrote:
> > > >> On 19 December 2011 11:41, Guennadi Liakhovetski wrote:
> > > >> > On Mon, 19 Dec 2011, Laurent Pinchart wrote:
> > > >> >> On Monday 19 December 2011 11:13:58 Guennadi Liakhovetski wrote:
> > > >> >> > On Mon, 19 Dec 2011, Laurent Pinchart wrote:
> > > >> >> > > On Monday 19 December 2011 09:09:34 Guennadi Liakhovetski wrote:
> 
> [snip]
> 
> > > >> >> > > > Good, this would mean, we need additional subdevice
> > > >> >> > > > operations along the lines of enum_input and enum_output,
> > > >> >> > > > and maybe also g_input and g_output?
> > > >> >> > > 
> > > >> >> > > What about implementing pad support in the subdevice ? Input
> > > >> >> > > enumeration could then be performed without a subdev
> > > >> >> > > operation.
> > > >> >> > 
> > > >> >> > soc-camera doesn't support pad operations yet.
> > > >> >> 
> > > >> >> soc-camera doesn't support enum_input yet either, so you need to
> > > >> >> implement something anyway ;-)
> > > >> >> 
> > > >> >> You wouldn't need to call a pad operation here, you would just need
> > > >> >> to iterate through the pads provided by the subdev.
> > > >> > 
> > > >> > tvp5150 doesn't implement it either yet. So, I would say, it is a
> > > >> > better solution ATM to fix this functionality independent of the
> > > >> > pad-level API.
> > > >> 
> > > >> I agree,
> > > >> I cannot contribute to implement pad-level API stuff since I can't
> > > >> test it with tvp5150.
> > > >> 
> > > >> Would you accept a patch implementing only S_INPUT?
> > > > 
> > > > Sorry, maybe I'm missing something, but how would it work? I mean, how
> > > > can we accept from the user any S_INPUT request with index != 0, if we
> > > > always return only 0 in reply to ENUM_INPUT? Ok, G_INPUT we could
> > > > implement internally in soc-camera: return 0 by default, then remember
> > > > last set input number per soc-camera device / subdev. But
> > > > ENUM_INPUT?...
> > > 
> > > It clearly is not a complete solution but at least it allows setting
> > > input 0 in broken drivers such as tvp5150 which have input 1 enabled
> > > by default, while soc-camera assumes input 0 is enabled.
> > 
> > I would really prefer an addition of an .enum_input() video subdev
> > operation.
> 
> I agree that input enumeration is needed, but I really think this should be 
> handled through pads, no with a new subdev operation. I don't like the idea of 
> introducing a new operation that will already be deprecated from the very 
> beginning.
> 
> Implementing this through pads isn't difficult. You don't need to implement 
> any pad operation in the tvp5150 driver. All you need to do is setup an array 
> of pads at probe time with information provided through platform data. soc-
> camera should then just access the pads array and implement enum_input 
> internally.

Ok, this might indeed be simple enough. Javier, could you give it a try?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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
  

>> I agree that input enumeration is needed, but I really think this should be
>> handled through pads, no with a new subdev operation. I don't like the idea of
>> introducing a new operation that will already be deprecated from the very
>> beginning.
>>
>> Implementing this through pads isn't difficult. You don't need to implement
>> any pad operation in the tvp5150 driver. All you need to do is setup an array
>> of pads at probe time with information provided through platform data. soc-
>> camera should then just access the pads array and implement enum_input
>> internally.
>
> Ok, this might indeed be simple enough. Javier, could you give it a try?

All right,
as soon as I have some time I'll go for that approach.

Regards.
  

On 19-12-2011 09:52, Laurent Pinchart wrote:
> Hi,
> 
> On Monday 19 December 2011 12:43:28 Guennadi Liakhovetski wrote:
>> On Mon, 19 Dec 2011, javier Martin wrote:
>>> On 19 December 2011 11:58, Guennadi Liakhovetski wrote:
>>>> On Mon, 19 Dec 2011, javier Martin wrote:
>>>>> On 19 December 2011 11:41, Guennadi Liakhovetski wrote:
>>>>>> On Mon, 19 Dec 2011, Laurent Pinchart wrote:
>>>>>>> On Monday 19 December 2011 11:13:58 Guennadi Liakhovetski wrote:
>>>>>>>> On Mon, 19 Dec 2011, Laurent Pinchart wrote:
>>>>>>>>> On Monday 19 December 2011 09:09:34 Guennadi Liakhovetski wrote:
> 
> [snip]
> 
>>>>>>>>>> Good, this would mean, we need additional subdevice
>>>>>>>>>> operations along the lines of enum_input and enum_output,
>>>>>>>>>> and maybe also g_input and g_output?
>>>>>>>>>
>>>>>>>>> What about implementing pad support in the subdevice ? Input
>>>>>>>>> enumeration could then be performed without a subdev
>>>>>>>>> operation.
>>>>>>>>
>>>>>>>> soc-camera doesn't support pad operations yet.
>>>>>>>
>>>>>>> soc-camera doesn't support enum_input yet either, so you need to
>>>>>>> implement something anyway ;-)
>>>>>>>
>>>>>>> You wouldn't need to call a pad operation here, you would just need
>>>>>>> to iterate through the pads provided by the subdev.
>>>>>>
>>>>>> tvp5150 doesn't implement it either yet. So, I would say, it is a
>>>>>> better solution ATM to fix this functionality independent of the
>>>>>> pad-level API.
>>>>>
>>>>> I agree,
>>>>> I cannot contribute to implement pad-level API stuff since I can't
>>>>> test it with tvp5150.
>>>>>
>>>>> Would you accept a patch implementing only S_INPUT?
>>>>
>>>> Sorry, maybe I'm missing something, but how would it work? I mean, how
>>>> can we accept from the user any S_INPUT request with index != 0, if we
>>>> always return only 0 in reply to ENUM_INPUT? Ok, G_INPUT we could
>>>> implement internally in soc-camera: return 0 by default, then remember
>>>> last set input number per soc-camera device / subdev. But
>>>> ENUM_INPUT?...
>>>
>>> It clearly is not a complete solution but at least it allows setting
>>> input 0 in broken drivers such as tvp5150 which have input 1 enabled
>>> by default, while soc-camera assumes input 0 is enabled.
>>
>> I would really prefer an addition of an .enum_input() video subdev
>> operation.
> 
> I agree that input enumeration is needed, but I really think this should be 
> handled through pads, no with a new subdev operation. I don't like the idea of 
> introducing a new operation that will already be deprecated from the very 
> beginning.

The enum_input/g_input/s_input operations/callbacks are not deprecated at all.
They're widely used on all analog TV devices, and there's absolutely no reason
at all to deprecate them.

Regards,
Mauro
--
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
  

Hi Mauro,

On Monday 19 December 2011 17:28:18 Mauro Carvalho Chehab wrote:
> On 19-12-2011 09:52, Laurent Pinchart wrote:
> > On Monday 19 December 2011 12:43:28 Guennadi Liakhovetski wrote:
> >> On Mon, 19 Dec 2011, javier Martin wrote:
> >>> On 19 December 2011 11:58, Guennadi Liakhovetski wrote:
> >>>> On Mon, 19 Dec 2011, javier Martin wrote:
> >>>>> On 19 December 2011 11:41, Guennadi Liakhovetski wrote:
> >>>>>> On Mon, 19 Dec 2011, Laurent Pinchart wrote:
> >>>>>>> On Monday 19 December 2011 11:13:58 Guennadi Liakhovetski wrote:
> >>>>>>>> On Mon, 19 Dec 2011, Laurent Pinchart wrote:
> >>>>>>>>> On Monday 19 December 2011 09:09:34 Guennadi Liakhovetski wrote:
> > [snip]
> > 
> >>>>>>>>>> Good, this would mean, we need additional subdevice
> >>>>>>>>>> operations along the lines of enum_input and enum_output,
> >>>>>>>>>> and maybe also g_input and g_output?
> >>>>>>>>> 
> >>>>>>>>> What about implementing pad support in the subdevice ? Input
> >>>>>>>>> enumeration could then be performed without a subdev
> >>>>>>>>> operation.
> >>>>>>>> 
> >>>>>>>> soc-camera doesn't support pad operations yet.
> >>>>>>> 
> >>>>>>> soc-camera doesn't support enum_input yet either, so you need to
> >>>>>>> implement something anyway ;-)
> >>>>>>> 
> >>>>>>> You wouldn't need to call a pad operation here, you would just need
> >>>>>>> to iterate through the pads provided by the subdev.
> >>>>>> 
> >>>>>> tvp5150 doesn't implement it either yet. So, I would say, it is a
> >>>>>> better solution ATM to fix this functionality independent of the
> >>>>>> pad-level API.
> >>>>> 
> >>>>> I agree,
> >>>>> I cannot contribute to implement pad-level API stuff since I can't
> >>>>> test it with tvp5150.
> >>>>> 
> >>>>> Would you accept a patch implementing only S_INPUT?
> >>>> 
> >>>> Sorry, maybe I'm missing something, but how would it work? I mean, how
> >>>> can we accept from the user any S_INPUT request with index != 0, if we
> >>>> always return only 0 in reply to ENUM_INPUT? Ok, G_INPUT we could
> >>>> implement internally in soc-camera: return 0 by default, then remember
> >>>> last set input number per soc-camera device / subdev. But
> >>>> ENUM_INPUT?...
> >>> 
> >>> It clearly is not a complete solution but at least it allows setting
> >>> input 0 in broken drivers such as tvp5150 which have input 1 enabled
> >>> by default, while soc-camera assumes input 0 is enabled.
> >> 
> >> I would really prefer an addition of an .enum_input() video subdev
> >> operation.
> > 
> > I agree that input enumeration is needed, but I really think this should
> > be handled through pads, no with a new subdev operation. I don't like
> > the idea of introducing a new operation that will already be deprecated
> > from the very beginning.
> 
> The enum_input/g_input/s_input operations/callbacks are not deprecated at
> all. They're widely used on all analog TV devices, and there's absolutely
> no reason at all to deprecate them.

I was talking about the subdev operations, not the V4L2 ioctls. Those are of 
course not deprecated, and will probably not be until at least V4L3 ;-)
  

On 19-12-2011 09:25, javier Martin wrote:
> On 19 December 2011 11:58, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
>> On Mon, 19 Dec 2011, javier Martin wrote:
>>
>>> On 19 December 2011 11:41, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
>>>> On Mon, 19 Dec 2011, Laurent Pinchart wrote:
>>>>
>>>>> Hi Guennadi,
>>>>>
>>>>> On Monday 19 December 2011 11:13:58 Guennadi Liakhovetski wrote:
>>>>>> On Mon, 19 Dec 2011, Laurent Pinchart wrote:
>>>>>>> On Monday 19 December 2011 09:09:34 Guennadi Liakhovetski wrote:
>>>>>>>> On Mon, 19 Dec 2011, Laurent Pinchart wrote:
>>>>>>>>> On Friday 16 December 2011 10:50:21 Guennadi Liakhovetski wrote:
>>>>>>>>>> On Fri, 16 Dec 2011, Scott Jiang wrote:
>>>>>>>>>>>>> How about this implementation? I know it's not for soc, but I
>>>>>>>>>>>>> post it to give my idea.
>>>>>>>>>>>>> Bridge knows the layout, so it doesn't need to query the
>>>>>>>>>>>>> subdevice.
>>>>>>>>>>>>
>>>>>>>>>>>> Where from? AFAIU, we are talking here about subdevice inputs,
>>>>>>>>>>>> right? In this case about various inputs of the TV decoder. How
>>>>>>>>>>>> shall the bridge driver know about that?
>>>>>>>>>>>
>>>>>>>>>>> I have asked this question before. Laurent reply me:
>>>>>>>>>>>>>> ENUMINPUT as defined by V4L2 enumerates input connectors
>>>>>>>>>>>>>> available on the board. Which inputs the board designer
>>>>>>>>>>>>>> hooked up is something that only the top-level V4L driver
>>>>>>>>>>>>>> will know. Subdevices do not have that information, so
>>>>>>>>>>>>>> enuminputs is not applicable there.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Of course, subdevices do have input pins and output pins,
>>>>>>>>>>>>>> but these are assumed to be fixed. With the s_routing ops
>>>>>>>>>>>>>> the top level driver selects which input and output pins
>>>>>>>>>>>>>> are active. Enumeration of those inputs and outputs
>>>>>>>>>>>>>> wouldn't gain you anything as far as I can tell since the
>>>>>>>>>>>>>> subdevice simply does not know which inputs/outputs are
>>>>>>>>>>>>>> actually hooked up. It's the top level driver that has that
>>>>>>>>>>>>>> information (usually passed in through board/card info
>>>>>>>>>>>>>> structures).
>>>>>>>>>>
>>>>>>>>>> Laurent, right, I now remember reading this discussion before. But
>>>>>>>>>> I'm not sure I completely agree:-) Yes, you're right - the board
>>>>>>>>>> decides which pins are routed to which connectors. And it has to
>>>>>>>>>> provide this information to the driver in its platform data. But -
>>>>>>>>>> I think, this information should be provided not to the bridge
>>>>>>>>>> driver, but to respective subdevice drivers, because only they
>>>>>>>>>> know what exactly those interfaces are good for and how to report
>>>>>>>>>> them to the bridge or the user, if we decide to also export this
>>>>>>>>>> information over the subdevice user-space API.
>>>>>>>>>>
>>>>>>>>>> So, I would say, the board has to tell the subdevice driver: yes,
>>>>>>>>>> your inputs 0 and 1 are routed to external connectors. On input 1
>>>>>>>>>> I've put a pullup, it is connected to connector of type X over a
>>>>>>>>>> circuit Y, clocked from your output Z, if the driver needs to know
>>>>>>>>>> all that. And the subdev driver will just tell the bridge only
>>>>>>>>>> what that one needs to know - number of inputs and their
>>>>>>>>>> capabilities.
>>>>>>>>>
>>>>>>>>> That sounds reasonable.
>>>>>>>>
>>>>>>>> Good, this would mean, we need additional subdevice operations along
>>>>>>>> the lines of enum_input and enum_output, and maybe also g_input and
>>>>>>>> g_output?
>>>>>>>
>>>>>>> What about implementing pad support in the subdevice ? Input enumeration
>>>>>>> could then be performed without a subdev operation.
>>>>>>
>>>>>> soc-camera doesn't support pad operations yet.
>>>>>
>>>>> soc-camera doesn't support enum_input yet either, so you need to implement
>>>>> something anyway ;-)
>>>>>
>>>>> You wouldn't need to call a pad operation here, you would just need to iterate
>>>>> through the pads provided by the subdev.
>>>>
>>>> tvp5150 doesn't implement it either yet. So, I would say, it is a better
>>>> solution ATM to fix this functionality independent of the pad-level API.
>>>
>>> I agree,
>>> I cannot contribute to implement pad-level API stuff since I can't
>>> test it with tvp5150.
>>>
>>> Would you accept a patch implementing only S_INPUT?
>>
>> Sorry, maybe I'm missing something, but how would it work? I mean, how can
>> we accept from the user any S_INPUT request with index != 0, if we always
>> return only 0 in reply to ENUM_INPUT? Ok, G_INPUT we could implement
>> internally in soc-camera: return 0 by default, then remember last set
>> input number per soc-camera device / subdev. But ENUM_INPUT?...
> 
> It clearly is not a complete solution but at least it allows setting
> input 0 in broken drivers such as tvp5150 which have input 1 enabled
> by default, while soc-camera assumes input 0 is enabled.

If you're willing to implement it via s_input, you should really implement
g_input and enum_input.

If you'll otherwise implement it via PAD, I'll need to analyze it better,
by the time you'll be submitting the libv4l plugin that will convert G_INPUT,
S_INPUT, ENUM_INPUT ioctl's into pad calls.

Regards,
Mauro

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

Guennadi,
probably you could answer me some question:

as we agreed I'm trying to implement ENUM_INPUT support to soc-camera
through pads. This means I must be able to pass the tvp5150 decoder
some platform_data in order to configure what inputs are really routed
in my board.

For that purpose I do the following in my board specific code:

 static struct tvp5150_platform_data visstrim_tvp5150_data = {
	.inputs = 55,
};

static struct i2c_board_info visstrim_i2c_camera =  {
	.type = "tvp5150",
	.addr = 0x5d,
	.platform_data = &visstrim_tvp5150_data,
};

static struct soc_camera_link iclink_tvp5150 = {
	.bus_id         = 0,            /* Must match with the camera ID */
	.board_info     = &visstrim_i2c_camera,
	.i2c_adapter_id = 0,
	.power = visstrim_camera_power,
	.reset = visstrim_camera_reset,
};

static struct platform_device visstrim_tvp5150_soc = {
	.name   = "soc-camera-pdrv",
	.id     = 0,
	.dev    = {
		.platform_data = &iclink_tvp5150,
	},
};


However, it seems soc-camera ignores "board_info.platform_data" field
and assigns a value of its own:

http://lxr.linux.no/#linux+v3.1.6/drivers/media/video/soc_camera.c#L1006


How am I suppose to pass that information to the tvp5150 then?

Thank you.
  

Hi Javier

On Tue, 3 Jan 2012, javier Martin wrote:

> Guennadi,
> probably you could answer me some question:
> 
> as we agreed I'm trying to implement ENUM_INPUT support to soc-camera
> through pads.

No, you probably mean in the subdev driver, not in soc-camera core.

> This means I must be able to pass the tvp5150 decoder
> some platform_data in order to configure what inputs are really routed
> in my board.
> 
> For that purpose I do the following in my board specific code:
> 
>  static struct tvp5150_platform_data visstrim_tvp5150_data = {
> 	.inputs = 55,
> };
> 
> static struct i2c_board_info visstrim_i2c_camera =  {
> 	.type = "tvp5150",
> 	.addr = 0x5d,
> 	.platform_data = &visstrim_tvp5150_data,
> };
> 
> static struct soc_camera_link iclink_tvp5150 = {
> 	.bus_id         = 0,            /* Must match with the camera ID */
> 	.board_info     = &visstrim_i2c_camera,
> 	.i2c_adapter_id = 0,
> 	.power = visstrim_camera_power,
> 	.reset = visstrim_camera_reset,
> };
> 
> static struct platform_device visstrim_tvp5150_soc = {
> 	.name   = "soc-camera-pdrv",
> 	.id     = 0,
> 	.dev    = {
> 		.platform_data = &iclink_tvp5150,
> 	},
> };
> 
> 
> However, it seems soc-camera ignores "board_info.platform_data" field
> and assigns a value of its own:
> 
> http://lxr.linux.no/#linux+v3.1.6/drivers/media/video/soc_camera.c#L1006
> 
> 
> How am I suppose to pass that information to the tvp5150 then?

Have a look at some examples, e.g., arch/sh/boards/mach-migor/setup.c:

static struct soc_camera_link ov7725_link = {
	.power		= ov7725_power,
	.board_info	= &migor_i2c_camera[0],
	.i2c_adapter_id	= 0,
	.priv		= &ov7725_info,
};

I.e., soc-camera expects you to use the struct soc_camera_link::priv field 
for subdevice private platform data.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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
  

Hi Guennadi.
Thanks for your help.

On 3 January 2012 16:05, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
> Hi Javier
>
> On Tue, 3 Jan 2012, javier Martin wrote:
>
>> Guennadi,
>> probably you could answer me some question:
>>
>> as we agreed I'm trying to implement ENUM_INPUT support to soc-camera
>> through pads.
>
> No, you probably mean in the subdev driver, not in soc-camera core.
>
>> This means I must be able to pass the tvp5150 decoder
>> some platform_data in order to configure what inputs are really routed
>> in my board.
>>
>> For that purpose I do the following in my board specific code:
>>
>>  static struct tvp5150_platform_data visstrim_tvp5150_data = {
>>       .inputs = 55,
>> };
>>
>> static struct i2c_board_info visstrim_i2c_camera =  {
>>       .type = "tvp5150",
>>       .addr = 0x5d,
>>       .platform_data = &visstrim_tvp5150_data,
>> };
>>
>> static struct soc_camera_link iclink_tvp5150 = {
>>       .bus_id         = 0,            /* Must match with the camera ID */
>>       .board_info     = &visstrim_i2c_camera,
>>       .i2c_adapter_id = 0,
>>       .power = visstrim_camera_power,
>>       .reset = visstrim_camera_reset,
>> };
>>
>> static struct platform_device visstrim_tvp5150_soc = {
>>       .name   = "soc-camera-pdrv",
>>       .id     = 0,
>>       .dev    = {
>>               .platform_data = &iclink_tvp5150,
>>       },
>> };
>>
>>
>> However, it seems soc-camera ignores "board_info.platform_data" field
>> and assigns a value of its own:
>>
>> http://lxr.linux.no/#linux+v3.1.6/drivers/media/video/soc_camera.c#L1006
>>
>>
>> How am I suppose to pass that information to the tvp5150 then?
>
> Have a look at some examples, e.g., arch/sh/boards/mach-migor/setup.c:
>
> static struct soc_camera_link ov7725_link = {
>        .power          = ov7725_power,
>        .board_info     = &migor_i2c_camera[0],
>        .i2c_adapter_id = 0,
>        .priv           = &ov7725_info,
> };
>
> I.e., soc-camera expects you to use the struct soc_camera_link::priv field
> for subdevice private platform data.

For ov7725 it is a natural thing to do since it was originally
developed for soc-camera and it can easily do the following to access
platform data:

struct soc_camera_link	*icl = soc_camera_i2c_to_link(client);
pdata = icl->priv;

However, tvp5150 is not aware about soc_camera. I should be able to
tell whether it's being used with soc-camera or not. If soc camera was
used I would do the previous method to retrieve platform data.
But if soc-camera was not used I would do the classic method:

struct tvp5150_platform_data *pdata = client->dev.platform_data;

The problem is how to distinguish in tvp5150 whether I am using
soc_camera or not.
  

On Wed, 4 Jan 2012, javier Martin wrote:

[snip]

> For ov7725 it is a natural thing to do since it was originally
> developed for soc-camera and it can easily do the following to access
> platform data:
> 
> struct soc_camera_link	*icl = soc_camera_i2c_to_link(client);
> pdata = icl->priv;
> 
> However, tvp5150 is not aware about soc_camera. I should be able to
> tell whether it's being used with soc-camera or not. If soc camera was
> used I would do the previous method to retrieve platform data.
> But if soc-camera was not used I would do the classic method:
> 
> struct tvp5150_platform_data *pdata = client->dev.platform_data;
> 
> The problem is how to distinguish in tvp5150 whether I am using
> soc_camera or not.

Right, that _is_ the problem now. And we've known about it since the very 
first time we started to think about re-using the subdev drivers. The only 
solution I see so far is to introduce a standard platform data struct for 
all subdevices, similar to soc_camera_link. We could use it as a basis, of 
course, use a generic name, maybe reconsider fields - rename / remove / 
add, but the principle would be the same: a standard platform data struct 
with an optional private field.

Alternatively - would it be possible to find all tvp5150 users and port 
them to use struct soc_camera_link too?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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
  

Hi Guennadi,

On Wednesday 04 January 2012 17:35:27 Guennadi Liakhovetski wrote:
> On Wed, 4 Jan 2012, javier Martin wrote:
> 
> [snip]
> 
> > For ov7725 it is a natural thing to do since it was originally
> > developed for soc-camera and it can easily do the following to access
> > platform data:
> > 
> > struct soc_camera_link	*icl = soc_camera_i2c_to_link(client);
> > pdata = icl->priv;
> > 
> > However, tvp5150 is not aware about soc_camera. I should be able to
> > tell whether it's being used with soc-camera or not. If soc camera was
> > used I would do the previous method to retrieve platform data.
> > But if soc-camera was not used I would do the classic method:
> > 
> > struct tvp5150_platform_data *pdata = client->dev.platform_data;
> > 
> > The problem is how to distinguish in tvp5150 whether I am using
> > soc_camera or not.
> 
> Right, that _is_ the problem now. And we've known about it since the very
> first time we started to think about re-using the subdev drivers. The only
> solution I see so far is to introduce a standard platform data struct for
> all subdevices, similar to soc_camera_link. We could use it as a basis, of
> course, use a generic name, maybe reconsider fields - rename / remove /
> add, but the principle would be the same: a standard platform data struct
> with an optional private field.

Why is that needed ? Why can't a tvp5150-specific platform data structure do ?

> Alternatively - would it be possible to find all tvp5150 users and port
> them to use struct soc_camera_link too?
  

On Wed, 4 Jan 2012, Laurent Pinchart wrote:

> Hi Guennadi,
> 
> On Wednesday 04 January 2012 17:35:27 Guennadi Liakhovetski wrote:
> > On Wed, 4 Jan 2012, javier Martin wrote:
> > 
> > [snip]
> > 
> > > For ov7725 it is a natural thing to do since it was originally
> > > developed for soc-camera and it can easily do the following to access
> > > platform data:
> > > 
> > > struct soc_camera_link	*icl = soc_camera_i2c_to_link(client);
> > > pdata = icl->priv;
> > > 
> > > However, tvp5150 is not aware about soc_camera. I should be able to
> > > tell whether it's being used with soc-camera or not. If soc camera was
> > > used I would do the previous method to retrieve platform data.
> > > But if soc-camera was not used I would do the classic method:
> > > 
> > > struct tvp5150_platform_data *pdata = client->dev.platform_data;
> > > 
> > > The problem is how to distinguish in tvp5150 whether I am using
> > > soc_camera or not.
> > 
> > Right, that _is_ the problem now. And we've known about it since the very
> > first time we started to think about re-using the subdev drivers. The only
> > solution I see so far is to introduce a standard platform data struct for
> > all subdevices, similar to soc_camera_link. We could use it as a basis, of
> > course, use a generic name, maybe reconsider fields - rename / remove /
> > add, but the principle would be the same: a standard platform data struct
> > with an optional private field.
> 
> Why is that needed ? Why can't a tvp5150-specific platform data structure do ?

Javier has actually explained this already. Ok, again: he wants to use 
tvp5150 with an soc-camera host driver, i.e., with the soc-camera 
subsystem. And the soc-camera core sets board_info->platform_data itself 
to a pointer to the struct soc_camera_link instance.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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
  

On Wednesday 04 January 2012 18:13:58 Guennadi Liakhovetski wrote:
> On Wed, 4 Jan 2012, Laurent Pinchart wrote:
> > On Wednesday 04 January 2012 17:35:27 Guennadi Liakhovetski wrote:
> > > On Wed, 4 Jan 2012, javier Martin wrote:
> > > 
> > > [snip]
> > > 
> > > > For ov7725 it is a natural thing to do since it was originally
> > > > developed for soc-camera and it can easily do the following to access
> > > > platform data:
> > > > 
> > > > struct soc_camera_link	*icl = soc_camera_i2c_to_link(client);
> > > > pdata = icl->priv;
> > > > 
> > > > However, tvp5150 is not aware about soc_camera. I should be able to
> > > > tell whether it's being used with soc-camera or not. If soc camera
> > > > was used I would do the previous method to retrieve platform data.
> > > > But if soc-camera was not used I would do the classic method:
> > > > 
> > > > struct tvp5150_platform_data *pdata = client->dev.platform_data;
> > > > 
> > > > The problem is how to distinguish in tvp5150 whether I am using
> > > > soc_camera or not.
> > > 
> > > Right, that _is_ the problem now. And we've known about it since the
> > > very first time we started to think about re-using the subdev drivers.
> > > The only solution I see so far is to introduce a standard platform
> > > data struct for all subdevices, similar to soc_camera_link. We could
> > > use it as a basis, of course, use a generic name, maybe reconsider
> > > fields - rename / remove / add, but the principle would be the same: a
> > > standard platform data struct with an optional private field.
> > 
> > Why is that needed ? Why can't a tvp5150-specific platform data structure
> > do ?
> 
> Javier has actually explained this already.

Sorry for not having followed.

> Ok, again: he wants to use tvp5150 with an soc-camera host driver, i.e.,
> with the soc-camera subsystem. And the soc-camera core sets board_info->
> platform_data itself to a pointer to the struct soc_camera_link instance.

That looks to me like it's the part to be changed...
  

On Wed, 4 Jan 2012, Laurent Pinchart wrote:

> On Wednesday 04 January 2012 18:13:58 Guennadi Liakhovetski wrote:
> > On Wed, 4 Jan 2012, Laurent Pinchart wrote:
> > > On Wednesday 04 January 2012 17:35:27 Guennadi Liakhovetski wrote:
> > > > On Wed, 4 Jan 2012, javier Martin wrote:
> > > > 
> > > > [snip]
> > > > 
> > > > > For ov7725 it is a natural thing to do since it was originally
> > > > > developed for soc-camera and it can easily do the following to access
> > > > > platform data:
> > > > > 
> > > > > struct soc_camera_link	*icl = soc_camera_i2c_to_link(client);
> > > > > pdata = icl->priv;
> > > > > 
> > > > > However, tvp5150 is not aware about soc_camera. I should be able to
> > > > > tell whether it's being used with soc-camera or not. If soc camera
> > > > > was used I would do the previous method to retrieve platform data.
> > > > > But if soc-camera was not used I would do the classic method:
> > > > > 
> > > > > struct tvp5150_platform_data *pdata = client->dev.platform_data;
> > > > > 
> > > > > The problem is how to distinguish in tvp5150 whether I am using
> > > > > soc_camera or not.
> > > > 
> > > > Right, that _is_ the problem now. And we've known about it since the
> > > > very first time we started to think about re-using the subdev drivers.
> > > > The only solution I see so far is to introduce a standard platform
> > > > data struct for all subdevices, similar to soc_camera_link. We could
> > > > use it as a basis, of course, use a generic name, maybe reconsider
> > > > fields - rename / remove / add, but the principle would be the same: a
> > > > standard platform data struct with an optional private field.
> > > 
> > > Why is that needed ? Why can't a tvp5150-specific platform data structure
> > > do ?
> > 
> > Javier has actually explained this already.
> 
> Sorry for not having followed.
> 
> > Ok, again: he wants to use tvp5150 with an soc-camera host driver, i.e.,
> > with the soc-camera subsystem. And the soc-camera core sets board_info->
> > platform_data itself to a pointer to the struct soc_camera_link instance.
> 
> That looks to me like it's the part to be changed...

Well, we could do this, but this would require changing a few soc-camera 
subdev drivers and respective platforms and (slightly) the core itself.

The soc-camera core needs access to the struct soc_camera_link instance, 
supplied by the platform. It is passed in .platform_data of the soc-camera 
client platform device, there's no need to change that. struct 
soc_camera_link::board_info points to struct i2c_board_info, this is also 
ok. Basically, that's all the soc-camera core needs - access to these two 
structs. Next, subdevice drivers also need access to struct 
soc_camera_link and to their private data. If we let platforms pass 
subdevice driver private data in i2c_board_info::platform_data, then each 
driver will need to invent its own way how to also get to struct 
soc_camera_link. They would either have to point at it from their private 
data or embed it therein.

So, yes, it is doable. AFAICS currently these subdevice drivers

soc_camera_platform
rj54n1cb0c
tw9910
mt9t112
ov772x

and these platforms

sh/ecovec24
sh/kfr2r09
sh/migor
sh/ap325rxa

arm/mackerel

are affected and have to be modified. After which the core can be fixed 
too. I could do that, but not sure when I find the time. Javier, if you 
like, feel free to give it a try.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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
  

On Wed, 4 Jan 2012, Guennadi Liakhovetski wrote:

> On Wed, 4 Jan 2012, Laurent Pinchart wrote:
> 
> > On Wednesday 04 January 2012 18:13:58 Guennadi Liakhovetski wrote:
> > > On Wed, 4 Jan 2012, Laurent Pinchart wrote:
> > > > On Wednesday 04 January 2012 17:35:27 Guennadi Liakhovetski wrote:
> > > > > On Wed, 4 Jan 2012, javier Martin wrote:
> > > > > 
> > > > > [snip]
> > > > > 
> > > > > > For ov7725 it is a natural thing to do since it was originally
> > > > > > developed for soc-camera and it can easily do the following to access
> > > > > > platform data:
> > > > > > 
> > > > > > struct soc_camera_link	*icl = soc_camera_i2c_to_link(client);
> > > > > > pdata = icl->priv;
> > > > > > 
> > > > > > However, tvp5150 is not aware about soc_camera. I should be able to
> > > > > > tell whether it's being used with soc-camera or not. If soc camera
> > > > > > was used I would do the previous method to retrieve platform data.
> > > > > > But if soc-camera was not used I would do the classic method:
> > > > > > 
> > > > > > struct tvp5150_platform_data *pdata = client->dev.platform_data;
> > > > > > 
> > > > > > The problem is how to distinguish in tvp5150 whether I am using
> > > > > > soc_camera or not.
> > > > > 
> > > > > Right, that _is_ the problem now. And we've known about it since the
> > > > > very first time we started to think about re-using the subdev drivers.
> > > > > The only solution I see so far is to introduce a standard platform
> > > > > data struct for all subdevices, similar to soc_camera_link. We could
> > > > > use it as a basis, of course, use a generic name, maybe reconsider
> > > > > fields - rename / remove / add, but the principle would be the same: a
> > > > > standard platform data struct with an optional private field.
> > > > 
> > > > Why is that needed ? Why can't a tvp5150-specific platform data structure
> > > > do ?
> > > 
> > > Javier has actually explained this already.
> > 
> > Sorry for not having followed.
> > 
> > > Ok, again: he wants to use tvp5150 with an soc-camera host driver, i.e.,
> > > with the soc-camera subsystem. And the soc-camera core sets board_info->
> > > platform_data itself to a pointer to the struct soc_camera_link instance.
> > 
> > That looks to me like it's the part to be changed...
> 
> Well, we could do this, but this would require changing a few soc-camera 
> subdev drivers and respective platforms and (slightly) the core itself.
> 
> The soc-camera core needs access to the struct soc_camera_link instance, 
> supplied by the platform. It is passed in .platform_data of the soc-camera 
> client platform device, there's no need to change that. struct 
> soc_camera_link::board_info points to struct i2c_board_info, this is also 
> ok. Basically, that's all the soc-camera core needs - access to these two 
> structs. Next, subdevice drivers also need access to struct 
> soc_camera_link and to their private data. If we let platforms pass 
> subdevice driver private data in i2c_board_info::platform_data, then each 
> driver will need to invent its own way how to also get to struct 
> soc_camera_link. They would either have to point at it from their private 
> data or embed it therein.
> 
> So, yes, it is doable. AFAICS currently these subdevice drivers
> 
> soc_camera_platform
> rj54n1cb0c
> tw9910
> mt9t112
> ov772x
> 
> and these platforms
> 
> sh/ecovec24
> sh/kfr2r09
> sh/migor
> sh/ap325rxa
> 
> arm/mackerel
> 
> are affected and have to be modified. After which the core can be fixed 
> too. I could do that, but not sure when I find the time. Javier, if you 
> like, feel free to give it a try.

Thinking a bit more about this, looks like the drivers don't have to be 
modified in fact. We just would have to make the reference from 
soc_camera_link::board_info to the specific struct i2c_board_info in 
platform data on the above platforms and remove it from soc_camera.c. This 
would look ugly in platform data, because structs i2c_board_info and 
soc_camera_link would then hold pointers to each other, but it would work.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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