* [PATCH] rcar_vin: propagate querystd() error upstream
@ 2015-08-19 21:02 ` Sergei Shtylyov
0 siblings, 0 replies; 12+ messages in thread
From: Sergei Shtylyov @ 2015-08-19 21:02 UTC (permalink / raw)
To: g.liakhovetski, mchehab, linux-media; +Cc: linux-sh
rcar_vin_set_fmt() defaults to PAL when the subdevice's querystd() method call
fails (e.g. due to I2C error). This doesn't work very well when a camera being
used outputs NTSC which has different order of fields and resolution. Let us
stop pretending and return the actual error (which would prevent video capture
on at least Renesas Henninger/Porter board where I2C seems particularly buggy).
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
---
The patch is against the 'media_tree.git' repo's 'fixes' branch.
drivers/media/platform/soc_camera/rcar_vin.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: media_tree/drivers/media/platform/soc_camera/rcar_vin.c
=================================--- media_tree.orig/drivers/media/platform/soc_camera/rcar_vin.c
+++ media_tree/drivers/media/platform/soc_camera/rcar_vin.c
@@ -1592,7 +1592,7 @@ static int rcar_vin_set_fmt(struct soc_c
/* Query for standard if not explicitly mentioned _TB/_BT */
ret = v4l2_subdev_call(sd, video, querystd, &std);
if (ret < 0)
- std = V4L2_STD_625_50;
+ return ret;
field = std & V4L2_STD_625_50 ? V4L2_FIELD_INTERLACED_TB :
V4L2_FIELD_INTERLACED_BT;
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] rcar_vin: propagate querystd() error upstream
@ 2015-08-19 21:02 ` Sergei Shtylyov
0 siblings, 0 replies; 12+ messages in thread
From: Sergei Shtylyov @ 2015-08-19 21:02 UTC (permalink / raw)
To: g.liakhovetski, mchehab, linux-media; +Cc: linux-sh
rcar_vin_set_fmt() defaults to PAL when the subdevice's querystd() method call
fails (e.g. due to I2C error). This doesn't work very well when a camera being
used outputs NTSC which has different order of fields and resolution. Let us
stop pretending and return the actual error (which would prevent video capture
on at least Renesas Henninger/Porter board where I2C seems particularly buggy).
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
---
The patch is against the 'media_tree.git' repo's 'fixes' branch.
drivers/media/platform/soc_camera/rcar_vin.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: media_tree/drivers/media/platform/soc_camera/rcar_vin.c
===================================================================
--- media_tree.orig/drivers/media/platform/soc_camera/rcar_vin.c
+++ media_tree/drivers/media/platform/soc_camera/rcar_vin.c
@@ -1592,7 +1592,7 @@ static int rcar_vin_set_fmt(struct soc_c
/* Query for standard if not explicitly mentioned _TB/_BT */
ret = v4l2_subdev_call(sd, video, querystd, &std);
if (ret < 0)
- std = V4L2_STD_625_50;
+ return ret;
field = std & V4L2_STD_625_50 ? V4L2_FIELD_INTERLACED_TB :
V4L2_FIELD_INTERLACED_BT;
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] rcar_vin: propagate querystd() error upstream
2015-08-19 21:02 ` Sergei Shtylyov
@ 2015-08-20 21:51 ` Laurent Pinchart
-1 siblings, 0 replies; 12+ messages in thread
From: Laurent Pinchart @ 2015-08-20 21:51 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: g.liakhovetski, mchehab, linux-media, linux-sh
Hi Sergei,
Thank you for the patch.
On Thursday 20 August 2015 00:02:17 Sergei Shtylyov wrote:
> rcar_vin_set_fmt() defaults to PAL when the subdevice's querystd() method
> call fails (e.g. due to I2C error). This doesn't work very well when a
> camera being used outputs NTSC which has different order of fields and
> resolution. Let us stop pretending and return the actual error (which
> would prevent video capture on at least Renesas Henninger/Porter board
> where I2C seems particularly buggy).
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
> ---
> The patch is against the 'media_tree.git' repo's 'fixes' branch.
>
> drivers/media/platform/soc_camera/rcar_vin.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> Index: media_tree/drivers/media/platform/soc_camera/rcar_vin.c
> =================================> --- media_tree.orig/drivers/media/platform/soc_camera/rcar_vin.c
> +++ media_tree/drivers/media/platform/soc_camera/rcar_vin.c
> @@ -1592,7 +1592,7 @@ static int rcar_vin_set_fmt(struct soc_c
> /* Query for standard if not explicitly mentioned _TB/_BT */
> ret = v4l2_subdev_call(sd, video, querystd, &std);
> if (ret < 0)
> - std = V4L2_STD_625_50;
> + return ret;
What if the subdev doesn't implement querystd ? That's the case of camera
sensors for instance. In that case we should default to V4L2_FIELD_NONE.
> field = std & V4L2_STD_625_50 ? V4L2_FIELD_INTERLACED_TB :
> V4L2_FIELD_INTERLACED_BT;
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] rcar_vin: propagate querystd() error upstream
@ 2015-08-20 21:51 ` Laurent Pinchart
0 siblings, 0 replies; 12+ messages in thread
From: Laurent Pinchart @ 2015-08-20 21:51 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: g.liakhovetski, mchehab, linux-media, linux-sh
Hi Sergei,
Thank you for the patch.
On Thursday 20 August 2015 00:02:17 Sergei Shtylyov wrote:
> rcar_vin_set_fmt() defaults to PAL when the subdevice's querystd() method
> call fails (e.g. due to I2C error). This doesn't work very well when a
> camera being used outputs NTSC which has different order of fields and
> resolution. Let us stop pretending and return the actual error (which
> would prevent video capture on at least Renesas Henninger/Porter board
> where I2C seems particularly buggy).
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
> ---
> The patch is against the 'media_tree.git' repo's 'fixes' branch.
>
> drivers/media/platform/soc_camera/rcar_vin.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> Index: media_tree/drivers/media/platform/soc_camera/rcar_vin.c
> ===================================================================
> --- media_tree.orig/drivers/media/platform/soc_camera/rcar_vin.c
> +++ media_tree/drivers/media/platform/soc_camera/rcar_vin.c
> @@ -1592,7 +1592,7 @@ static int rcar_vin_set_fmt(struct soc_c
> /* Query for standard if not explicitly mentioned _TB/_BT */
> ret = v4l2_subdev_call(sd, video, querystd, &std);
> if (ret < 0)
> - std = V4L2_STD_625_50;
> + return ret;
What if the subdev doesn't implement querystd ? That's the case of camera
sensors for instance. In that case we should default to V4L2_FIELD_NONE.
> field = std & V4L2_STD_625_50 ? V4L2_FIELD_INTERLACED_TB :
> V4L2_FIELD_INTERLACED_BT;
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] rcar_vin: propagate querystd() error upstream
2015-08-20 21:51 ` Laurent Pinchart
@ 2015-08-24 21:58 ` Sergei Shtylyov
-1 siblings, 0 replies; 12+ messages in thread
From: Sergei Shtylyov @ 2015-08-24 21:58 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: g.liakhovetski, mchehab, linux-media, linux-sh
hello.
On 08/21/2015 12:51 AM, Laurent Pinchart wrote:
>> rcar_vin_set_fmt() defaults to PAL when the subdevice's querystd() method
>> call fails (e.g. due to I2C error). This doesn't work very well when a
>> camera being used outputs NTSC which has different order of fields and
>> resolution. Let us stop pretending and return the actual error (which
>> would prevent video capture on at least Renesas Henninger/Porter board
>> where I2C seems particularly buggy).
>>
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>
>> ---
>> The patch is against the 'media_tree.git' repo's 'fixes' branch.
>>
>> drivers/media/platform/soc_camera/rcar_vin.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> Index: media_tree/drivers/media/platform/soc_camera/rcar_vin.c
>> =================================>> --- media_tree.orig/drivers/media/platform/soc_camera/rcar_vin.c
>> +++ media_tree/drivers/media/platform/soc_camera/rcar_vin.c
>> @@ -1592,7 +1592,7 @@ static int rcar_vin_set_fmt(struct soc_c
>> /* Query for standard if not explicitly mentioned _TB/_BT */
>> ret = v4l2_subdev_call(sd, video, querystd, &std);
>> if (ret < 0)
>> - std = V4L2_STD_625_50;
>> + return ret;
>
> What if the subdev doesn't implement querystd ? That's the case of camera
> sensors for instance.
Indeed.
> In that case we should default to V4L2_FIELD_NONE.
Hmm, even if the set_fmt() method is called with V4L2_FIELD_INTERLACED
already, like in this case?
>> field = std & V4L2_STD_625_50 ? V4L2_FIELD_INTERLACED_TB :
>> V4L2_FIELD_INTERLACED_BT;
MBR, Sergei
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] rcar_vin: propagate querystd() error upstream
@ 2015-08-24 21:58 ` Sergei Shtylyov
0 siblings, 0 replies; 12+ messages in thread
From: Sergei Shtylyov @ 2015-08-24 21:58 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: g.liakhovetski, mchehab, linux-media, linux-sh
hello.
On 08/21/2015 12:51 AM, Laurent Pinchart wrote:
>> rcar_vin_set_fmt() defaults to PAL when the subdevice's querystd() method
>> call fails (e.g. due to I2C error). This doesn't work very well when a
>> camera being used outputs NTSC which has different order of fields and
>> resolution. Let us stop pretending and return the actual error (which
>> would prevent video capture on at least Renesas Henninger/Porter board
>> where I2C seems particularly buggy).
>>
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>
>> ---
>> The patch is against the 'media_tree.git' repo's 'fixes' branch.
>>
>> drivers/media/platform/soc_camera/rcar_vin.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> Index: media_tree/drivers/media/platform/soc_camera/rcar_vin.c
>> ===================================================================
>> --- media_tree.orig/drivers/media/platform/soc_camera/rcar_vin.c
>> +++ media_tree/drivers/media/platform/soc_camera/rcar_vin.c
>> @@ -1592,7 +1592,7 @@ static int rcar_vin_set_fmt(struct soc_c
>> /* Query for standard if not explicitly mentioned _TB/_BT */
>> ret = v4l2_subdev_call(sd, video, querystd, &std);
>> if (ret < 0)
>> - std = V4L2_STD_625_50;
>> + return ret;
>
> What if the subdev doesn't implement querystd ? That's the case of camera
> sensors for instance.
Indeed.
> In that case we should default to V4L2_FIELD_NONE.
Hmm, even if the set_fmt() method is called with V4L2_FIELD_INTERLACED
already, like in this case?
>> field = std & V4L2_STD_625_50 ? V4L2_FIELD_INTERLACED_TB :
>> V4L2_FIELD_INTERLACED_BT;
MBR, Sergei
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] rcar_vin: propagate querystd() error upstream
2015-08-24 21:58 ` Sergei Shtylyov
@ 2015-08-25 19:42 ` Laurent Pinchart
-1 siblings, 0 replies; 12+ messages in thread
From: Laurent Pinchart @ 2015-08-25 19:42 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: g.liakhovetski, mchehab, linux-media, linux-sh
Hi Sergei,
On Tuesday 25 August 2015 00:58:07 Sergei Shtylyov wrote:
> On 08/21/2015 12:51 AM, Laurent Pinchart wrote:
> >> rcar_vin_set_fmt() defaults to PAL when the subdevice's querystd()
> >> method call fails (e.g. due to I2C error). This doesn't work very well
> >> when a camera being used outputs NTSC which has different order of
> >> fields and resolution. Let us stop pretending and return the actual
> >> error (which would prevent video capture on at least Renesas
> >> Henninger/Porter board where I2C seems particularly buggy).
> >>
> >> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> >>
> >> ---
> >> The patch is against the 'media_tree.git' repo's 'fixes' branch.
> >>
> >> drivers/media/platform/soc_camera/rcar_vin.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> Index: media_tree/drivers/media/platform/soc_camera/rcar_vin.c
> >> =================================> >> --- media_tree.orig/drivers/media/platform/soc_camera/rcar_vin.c
> >> +++ media_tree/drivers/media/platform/soc_camera/rcar_vin.c
> >> @@ -1592,7 +1592,7 @@ static int rcar_vin_set_fmt(struct soc_c
> >>
> >> /* Query for standard if not explicitly mentioned _TB/_BT */
> >> ret = v4l2_subdev_call(sd, video, querystd, &std);
> >> if (ret < 0)
> >> - std = V4L2_STD_625_50;
> >> + return ret;
> >
> > What if the subdev doesn't implement querystd ? That's the case of camera
> > sensors for instance.
>
> Indeed.
>
> > In that case we should default to V4L2_FIELD_NONE.
>
> Hmm, even if the set_fmt() method is called with V4L2_FIELD_INTERLACED
> already, like in this case?
Yes. If the device doesn't support interlacing then the field passed to
set_fmt() should be set to V4L2_FIELD_NONE. The V4L2 API requires drivers to
fix unsupported parameters passed to the VIDIOC_S_FMT ioctl instead of
returning an error like commonly done in such situation.
> >> field = std & V4L2_STD_625_50 ? V4L2_FIELD_INTERLACED_TB :
> >> V4L2_FIELD_INTERLACED_BT;
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] rcar_vin: propagate querystd() error upstream
@ 2015-08-25 19:42 ` Laurent Pinchart
0 siblings, 0 replies; 12+ messages in thread
From: Laurent Pinchart @ 2015-08-25 19:42 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: g.liakhovetski, mchehab, linux-media, linux-sh
Hi Sergei,
On Tuesday 25 August 2015 00:58:07 Sergei Shtylyov wrote:
> On 08/21/2015 12:51 AM, Laurent Pinchart wrote:
> >> rcar_vin_set_fmt() defaults to PAL when the subdevice's querystd()
> >> method call fails (e.g. due to I2C error). This doesn't work very well
> >> when a camera being used outputs NTSC which has different order of
> >> fields and resolution. Let us stop pretending and return the actual
> >> error (which would prevent video capture on at least Renesas
> >> Henninger/Porter board where I2C seems particularly buggy).
> >>
> >> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> >>
> >> ---
> >> The patch is against the 'media_tree.git' repo's 'fixes' branch.
> >>
> >> drivers/media/platform/soc_camera/rcar_vin.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> Index: media_tree/drivers/media/platform/soc_camera/rcar_vin.c
> >> ===================================================================
> >> --- media_tree.orig/drivers/media/platform/soc_camera/rcar_vin.c
> >> +++ media_tree/drivers/media/platform/soc_camera/rcar_vin.c
> >> @@ -1592,7 +1592,7 @@ static int rcar_vin_set_fmt(struct soc_c
> >>
> >> /* Query for standard if not explicitly mentioned _TB/_BT */
> >> ret = v4l2_subdev_call(sd, video, querystd, &std);
> >> if (ret < 0)
> >> - std = V4L2_STD_625_50;
> >> + return ret;
> >
> > What if the subdev doesn't implement querystd ? That's the case of camera
> > sensors for instance.
>
> Indeed.
>
> > In that case we should default to V4L2_FIELD_NONE.
>
> Hmm, even if the set_fmt() method is called with V4L2_FIELD_INTERLACED
> already, like in this case?
Yes. If the device doesn't support interlacing then the field passed to
set_fmt() should be set to V4L2_FIELD_NONE. The V4L2 API requires drivers to
fix unsupported parameters passed to the VIDIOC_S_FMT ioctl instead of
returning an error like commonly done in such situation.
> >> field = std & V4L2_STD_625_50 ? V4L2_FIELD_INTERLACED_TB :
> >> V4L2_FIELD_INTERLACED_BT;
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] rcar_vin: propagate querystd() error upstream
2015-08-19 21:02 ` Sergei Shtylyov
@ 2015-08-25 14:25 ` Hans Verkuil
-1 siblings, 0 replies; 12+ messages in thread
From: Hans Verkuil @ 2015-08-25 14:25 UTC (permalink / raw)
To: Sergei Shtylyov, g.liakhovetski, mchehab, linux-media; +Cc: linux-sh
On 08/19/15 23:02, Sergei Shtylyov wrote:
> rcar_vin_set_fmt() defaults to PAL when the subdevice's querystd() method call
> fails (e.g. due to I2C error). This doesn't work very well when a camera being
> used outputs NTSC which has different order of fields and resolution. Let us
> stop pretending and return the actual error (which would prevent video capture
> on at least Renesas Henninger/Porter board where I2C seems particularly buggy).
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
> ---
> The patch is against the 'media_tree.git' repo's 'fixes' branch.
>
> drivers/media/platform/soc_camera/rcar_vin.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> Index: media_tree/drivers/media/platform/soc_camera/rcar_vin.c
> =================================> --- media_tree.orig/drivers/media/platform/soc_camera/rcar_vin.c
> +++ media_tree/drivers/media/platform/soc_camera/rcar_vin.c
> @@ -1592,7 +1592,7 @@ static int rcar_vin_set_fmt(struct soc_c
> /* Query for standard if not explicitly mentioned _TB/_BT */
> ret = v4l2_subdev_call(sd, video, querystd, &std);
Ouch, this should never be done like this.
Instead the decision should be made using the last set std, never by querying.
So querystd should be replaced by g_std in the v4l2_subdev_call above.
The only place querystd can be called is in the QUERYSTD ioctl, all other
ioctls should use the last set standard.
Regards,
Hans
> if (ret < 0)
> - std = V4L2_STD_625_50;
> + return ret;
>
> field = std & V4L2_STD_625_50 ? V4L2_FIELD_INTERLACED_TB :
> V4L2_FIELD_INTERLACED_BT;
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] rcar_vin: propagate querystd() error upstream
@ 2015-08-25 14:25 ` Hans Verkuil
0 siblings, 0 replies; 12+ messages in thread
From: Hans Verkuil @ 2015-08-25 14:25 UTC (permalink / raw)
To: Sergei Shtylyov, g.liakhovetski, mchehab, linux-media; +Cc: linux-sh
On 08/19/15 23:02, Sergei Shtylyov wrote:
> rcar_vin_set_fmt() defaults to PAL when the subdevice's querystd() method call
> fails (e.g. due to I2C error). This doesn't work very well when a camera being
> used outputs NTSC which has different order of fields and resolution. Let us
> stop pretending and return the actual error (which would prevent video capture
> on at least Renesas Henninger/Porter board where I2C seems particularly buggy).
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
> ---
> The patch is against the 'media_tree.git' repo's 'fixes' branch.
>
> drivers/media/platform/soc_camera/rcar_vin.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> Index: media_tree/drivers/media/platform/soc_camera/rcar_vin.c
> ===================================================================
> --- media_tree.orig/drivers/media/platform/soc_camera/rcar_vin.c
> +++ media_tree/drivers/media/platform/soc_camera/rcar_vin.c
> @@ -1592,7 +1592,7 @@ static int rcar_vin_set_fmt(struct soc_c
> /* Query for standard if not explicitly mentioned _TB/_BT */
> ret = v4l2_subdev_call(sd, video, querystd, &std);
Ouch, this should never be done like this.
Instead the decision should be made using the last set std, never by querying.
So querystd should be replaced by g_std in the v4l2_subdev_call above.
The only place querystd can be called is in the QUERYSTD ioctl, all other
ioctls should use the last set standard.
Regards,
Hans
> if (ret < 0)
> - std = V4L2_STD_625_50;
> + return ret;
>
> field = std & V4L2_STD_625_50 ? V4L2_FIELD_INTERLACED_TB :
> V4L2_FIELD_INTERLACED_BT;
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] rcar_vin: propagate querystd() error upstream
2015-08-25 14:25 ` Hans Verkuil
@ 2015-08-31 22:13 ` Sergei Shtylyov
-1 siblings, 0 replies; 12+ messages in thread
From: Sergei Shtylyov @ 2015-08-31 22:13 UTC (permalink / raw)
To: Hans Verkuil, g.liakhovetski, mchehab, linux-media; +Cc: linux-sh
Hello.
On 08/25/2015 05:25 PM, Hans Verkuil wrote:
>> rcar_vin_set_fmt() defaults to PAL when the subdevice's querystd() method call
>> fails (e.g. due to I2C error). This doesn't work very well when a camera being
>> used outputs NTSC which has different order of fields and resolution. Let us
>> stop pretending and return the actual error (which would prevent video capture
>> on at least Renesas Henninger/Porter board where I2C seems particularly buggy).
>>
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>
>> ---
>> The patch is against the 'media_tree.git' repo's 'fixes' branch.
>>
>> drivers/media/platform/soc_camera/rcar_vin.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> Index: media_tree/drivers/media/platform/soc_camera/rcar_vin.c
>> =================================>> --- media_tree.orig/drivers/media/platform/soc_camera/rcar_vin.c
>> +++ media_tree/drivers/media/platform/soc_camera/rcar_vin.c
>> @@ -1592,7 +1592,7 @@ static int rcar_vin_set_fmt(struct soc_c
>> /* Query for standard if not explicitly mentioned _TB/_BT */
>> ret = v4l2_subdev_call(sd, video, querystd, &std);
> Ouch, this should never be done like this.
Too late. :-)
> Instead the decision should be made using the last set std, never by querying.
> So querystd should be replaced by g_std in the v4l2_subdev_call above.
Hm, then this code will stop working, as adv7180.c and ml86v7667.c we use
don't support the g_std() method yet...
> The only place querystd can be called is in the QUERYSTD ioctl, all other
> ioctls should use the last set standard.
OK, I'll have to fix all the drivers involved...
> Regards,
> Hans
MBR, Sergei
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH] rcar_vin: propagate querystd() error upstream
@ 2015-08-31 22:13 ` Sergei Shtylyov
0 siblings, 0 replies; 12+ messages in thread
From: Sergei Shtylyov @ 2015-08-31 22:13 UTC (permalink / raw)
To: Hans Verkuil, g.liakhovetski, mchehab, linux-media; +Cc: linux-sh
Hello.
On 08/25/2015 05:25 PM, Hans Verkuil wrote:
>> rcar_vin_set_fmt() defaults to PAL when the subdevice's querystd() method call
>> fails (e.g. due to I2C error). This doesn't work very well when a camera being
>> used outputs NTSC which has different order of fields and resolution. Let us
>> stop pretending and return the actual error (which would prevent video capture
>> on at least Renesas Henninger/Porter board where I2C seems particularly buggy).
>>
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>
>> ---
>> The patch is against the 'media_tree.git' repo's 'fixes' branch.
>>
>> drivers/media/platform/soc_camera/rcar_vin.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> Index: media_tree/drivers/media/platform/soc_camera/rcar_vin.c
>> ===================================================================
>> --- media_tree.orig/drivers/media/platform/soc_camera/rcar_vin.c
>> +++ media_tree/drivers/media/platform/soc_camera/rcar_vin.c
>> @@ -1592,7 +1592,7 @@ static int rcar_vin_set_fmt(struct soc_c
>> /* Query for standard if not explicitly mentioned _TB/_BT */
>> ret = v4l2_subdev_call(sd, video, querystd, &std);
> Ouch, this should never be done like this.
Too late. :-)
> Instead the decision should be made using the last set std, never by querying.
> So querystd should be replaced by g_std in the v4l2_subdev_call above.
Hm, then this code will stop working, as adv7180.c and ml86v7667.c we use
don't support the g_std() method yet...
> The only place querystd can be called is in the QUERYSTD ioctl, all other
> ioctls should use the last set standard.
OK, I'll have to fix all the drivers involved...
> Regards,
> Hans
MBR, Sergei
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2015-08-31 22:13 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-19 21:02 [PATCH] rcar_vin: propagate querystd() error upstream Sergei Shtylyov
2015-08-19 21:02 ` Sergei Shtylyov
2015-08-20 21:51 ` Laurent Pinchart
2015-08-20 21:51 ` Laurent Pinchart
2015-08-24 21:58 ` Sergei Shtylyov
2015-08-24 21:58 ` Sergei Shtylyov
2015-08-25 19:42 ` Laurent Pinchart
2015-08-25 19:42 ` Laurent Pinchart
2015-08-25 14:25 ` Hans Verkuil
2015-08-25 14:25 ` Hans Verkuil
2015-08-31 22:13 ` Sergei Shtylyov
2015-08-31 22:13 ` Sergei Shtylyov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.