[PATCH 0/3] adv7604: .g_crop and .cropcap support

[PATCH 0/3] adv7604: .g_crop and .cropcap support

[Ulrich Hecht]

Hi!

The rcar_vin driver relies on these methods.  The third patch makes sure
that they return up-to-date data if the input signal has changed since
initialization.

CU
Uli


Ulrich Hecht (3):
  media: adv7604: implement g_crop
  media: adv7604: implement cropcap
  media: adv7604: update timings on change of input signal

 drivers/media/i2c/adv7604.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

-- 
2.6.3

[PATCH 0/3] adv7604: .g_crop and .cropcap support

[Ulrich Hecht]

Hi!

The rcar_vin driver relies on these methods.  The third patch makes sure
that they return up-to-date data if the input signal has changed since
initialization.

CU
Uli


Ulrich Hecht (3):
  media: adv7604: implement g_crop
  media: adv7604: implement cropcap
  media: adv7604: update timings on change of input signal

 drivers/media/i2c/adv7604.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

-- 
2.6.3

[PATCH 1/3] media: adv7604: implement g_crop

[Ulrich Hecht]

The rcar_vin driver relies on this.

Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
---
 drivers/media/i2c/adv7604.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
index 129009f..d30e7cc 100644
--- a/drivers/media/i2c/adv7604.c
+++ b/drivers/media/i2c/adv7604.c
@@ -1885,6 +1885,17 @@ static int adv76xx_get_format(struct v4l2_subdev *sd,
 	return 0;
 }
 
+static int adv76xx_g_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
+{
+	struct adv76xx_state *state = to_state(sd);
+
+	a->c.height = state->timings.bt.height;
+	a->c.width  = state->timings.bt.width;
+	a->type	    = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+
+	return 0;
+}
+
 static int adv76xx_set_format(struct v4l2_subdev *sd,
 			      struct v4l2_subdev_pad_config *cfg,
 			      struct v4l2_subdev_format *format)
@@ -2407,6 +2418,7 @@ static const struct v4l2_subdev_core_ops adv76xx_core_ops = {
 
 static const struct v4l2_subdev_video_ops adv76xx_video_ops = {
 	.s_routing = adv76xx_s_routing,
+	.g_crop = adv76xx_g_crop,
 	.g_input_status = adv76xx_g_input_status,
 	.s_dv_timings = adv76xx_s_dv_timings,
 	.g_dv_timings = adv76xx_g_dv_timings,
-- 
2.6.3

[PATCH 1/3] media: adv7604: implement g_crop

[Ulrich Hecht]

The rcar_vin driver relies on this.

Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
---
 drivers/media/i2c/adv7604.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
index 129009f..d30e7cc 100644
--- a/drivers/media/i2c/adv7604.c
+++ b/drivers/media/i2c/adv7604.c
@@ -1885,6 +1885,17 @@ static int adv76xx_get_format(struct v4l2_subdev *sd,
 	return 0;
 }
 
+static int adv76xx_g_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
+{
+	struct adv76xx_state *state = to_state(sd);
+
+	a->c.height = state->timings.bt.height;
+	a->c.width  = state->timings.bt.width;
+	a->type	    = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+
+	return 0;
+}
+
 static int adv76xx_set_format(struct v4l2_subdev *sd,
 			      struct v4l2_subdev_pad_config *cfg,
 			      struct v4l2_subdev_format *format)
@@ -2407,6 +2418,7 @@ static const struct v4l2_subdev_core_ops adv76xx_core_ops = {
 
 static const struct v4l2_subdev_video_ops adv76xx_video_ops = {
 	.s_routing = adv76xx_s_routing,
+	.g_crop = adv76xx_g_crop,
 	.g_input_status = adv76xx_g_input_status,
 	.s_dv_timings = adv76xx_s_dv_timings,
 	.g_dv_timings = adv76xx_g_dv_timings,
-- 
2.6.3

[PATCH 2/3] media: adv7604: implement cropcap

[Ulrich Hecht]

Used by the rcar_vin driver.

Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
---
 drivers/media/i2c/adv7604.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
index d30e7cc..1bfa9f3 100644
--- a/drivers/media/i2c/adv7604.c
+++ b/drivers/media/i2c/adv7604.c
@@ -1896,6 +1896,22 @@ static int adv76xx_g_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
 	return 0;
 }
 
+static int adv76xx_cropcap(struct v4l2_subdev *sd, struct v4l2_cropcap *a)
+{
+	struct adv76xx_state *state = to_state(sd);
+
+	a->bounds.left	 = 0;
+	a->bounds.top	 = 0;
+	a->bounds.width	 = state->timings.bt.width;
+	a->bounds.height = state->timings.bt.height;
+	a->defrect	 = a->bounds;
+	a->type		 = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+	a->pixelaspect.numerator   = 1;
+	a->pixelaspect.denominator = 1;
+
+	return 0;
+}
+
 static int adv76xx_set_format(struct v4l2_subdev *sd,
 			      struct v4l2_subdev_pad_config *cfg,
 			      struct v4l2_subdev_format *format)
@@ -2419,6 +2435,7 @@ static const struct v4l2_subdev_core_ops adv76xx_core_ops = {
 static const struct v4l2_subdev_video_ops adv76xx_video_ops = {
 	.s_routing = adv76xx_s_routing,
 	.g_crop = adv76xx_g_crop,
+	.cropcap = adv76xx_cropcap,
 	.g_input_status = adv76xx_g_input_status,
 	.s_dv_timings = adv76xx_s_dv_timings,
 	.g_dv_timings = adv76xx_g_dv_timings,
-- 
2.6.3

[PATCH 2/3] media: adv7604: implement cropcap

[Ulrich Hecht]

Used by the rcar_vin driver.

Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
---
 drivers/media/i2c/adv7604.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
index d30e7cc..1bfa9f3 100644
--- a/drivers/media/i2c/adv7604.c
+++ b/drivers/media/i2c/adv7604.c
@@ -1896,6 +1896,22 @@ static int adv76xx_g_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
 	return 0;
 }
 
+static int adv76xx_cropcap(struct v4l2_subdev *sd, struct v4l2_cropcap *a)
+{
+	struct adv76xx_state *state = to_state(sd);
+
+	a->bounds.left	 = 0;
+	a->bounds.top	 = 0;
+	a->bounds.width	 = state->timings.bt.width;
+	a->bounds.height = state->timings.bt.height;
+	a->defrect	 = a->bounds;
+	a->type		 = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+	a->pixelaspect.numerator   = 1;
+	a->pixelaspect.denominator = 1;
+
+	return 0;
+}
+
 static int adv76xx_set_format(struct v4l2_subdev *sd,
 			      struct v4l2_subdev_pad_config *cfg,
 			      struct v4l2_subdev_format *format)
@@ -2419,6 +2435,7 @@ static const struct v4l2_subdev_core_ops adv76xx_core_ops = {
 static const struct v4l2_subdev_video_ops adv76xx_video_ops = {
 	.s_routing = adv76xx_s_routing,
 	.g_crop = adv76xx_g_crop,
+	.cropcap = adv76xx_cropcap,
 	.g_input_status = adv76xx_g_input_status,
 	.s_dv_timings = adv76xx_s_dv_timings,
 	.g_dv_timings = adv76xx_g_dv_timings,
-- 
2.6.3

[PATCH 3/3] media: adv7604: update timings on change of input signal

[Ulrich Hecht]

Without this, g_crop will always return the boot-time state.

Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
---
 drivers/media/i2c/adv7604.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
index 1bfa9f3..d7d0bb7 100644
--- a/drivers/media/i2c/adv7604.c
+++ b/drivers/media/i2c/adv7604.c
@@ -1975,6 +1975,15 @@ static int adv76xx_isr(struct v4l2_subdev *sd, u32 status, bool *handled)
 
 		v4l2_subdev_notify_event(sd, &adv76xx_ev_fmt);
 
+		/* update timings */
+		if (adv76xx_query_dv_timings(sd, &state->timings)
+		    = -ENOLINK) {
+			/* no signal, fall back to default timings */
+			const struct v4l2_dv_timings cea640x480 +				V4L2_DV_BT_CEA_640X480P59_94;
+			state->timings = cea640x480;
+		}
+
 		if (handled)
 			*handled = true;
 	}
-- 
2.6.3

[PATCH 3/3] media: adv7604: update timings on change of input signal

[Ulrich Hecht]

Without this, g_crop will always return the boot-time state.

Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
---
 drivers/media/i2c/adv7604.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
index 1bfa9f3..d7d0bb7 100644
--- a/drivers/media/i2c/adv7604.c
+++ b/drivers/media/i2c/adv7604.c
@@ -1975,6 +1975,15 @@ static int adv76xx_isr(struct v4l2_subdev *sd, u32 status, bool *handled)
 
 		v4l2_subdev_notify_event(sd, &adv76xx_ev_fmt);
 
+		/* update timings */
+		if (adv76xx_query_dv_timings(sd, &state->timings)
+		    == -ENOLINK) {
+			/* no signal, fall back to default timings */
+			const struct v4l2_dv_timings cea640x480 =
+				V4L2_DV_BT_CEA_640X480P59_94;
+			state->timings = cea640x480;
+		}
+
 		if (handled)
 			*handled = true;
 	}
-- 
2.6.3

Re: [PATCH 0/3] adv7604: .g_crop and .cropcap support

[Hans Verkuil]

Hi Ulrich,

On 12/11/2015 05:04 PM, Ulrich Hecht wrote:
> Hi!
> 
> The rcar_vin driver relies on these methods.  The third patch makes sure
> that they return up-to-date data if the input signal has changed since
> initialization.
> 
> CU
> Uli
> 
> 
> Ulrich Hecht (3):
>   media: adv7604: implement g_crop
>   media: adv7604: implement cropcap

I'm not keen on these changes. The reason is that these ops are deprecated and
soc-camera is - almost - the last user. The g/s_selection ops should be used instead.

Now, I have a patch that changes soc-camera to g/s_selection. The reason it was never
applied is that I had a hard time finding hardware to test it with.

Since you clearly have that hardware I think I'll rebase my (by now rather old) patch
and post it again. If you can switch the adv7604 patch to g/s_selection and everything
works with my patch, then I think I should just make a pull request for it.

I hope to be able to do this on Monday.

If switching soc-camera over to g/s_selection isn't possible, then at the very least
your adv7604 changes should provide the g/s_selection implementation. I don't want
to have to convert this driver later to g/s_selection.

Regards,

	Hans

>   media: adv7604: update timings on change of input signal
> 
>  drivers/media/i2c/adv7604.c | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 

Re: [PATCH 0/3] adv7604: .g_crop and .cropcap support

[Hans Verkuil]

Hi Ulrich,

On 12/11/2015 05:04 PM, Ulrich Hecht wrote:
> Hi!
> 
> The rcar_vin driver relies on these methods.  The third patch makes sure
> that they return up-to-date data if the input signal has changed since
> initialization.
> 
> CU
> Uli
> 
> 
> Ulrich Hecht (3):
>   media: adv7604: implement g_crop
>   media: adv7604: implement cropcap

I'm not keen on these changes. The reason is that these ops are deprecated and
soc-camera is - almost - the last user. The g/s_selection ops should be used instead.

Now, I have a patch that changes soc-camera to g/s_selection. The reason it was never
applied is that I had a hard time finding hardware to test it with.

Since you clearly have that hardware I think I'll rebase my (by now rather old) patch
and post it again. If you can switch the adv7604 patch to g/s_selection and everything
works with my patch, then I think I should just make a pull request for it.

I hope to be able to do this on Monday.

If switching soc-camera over to g/s_selection isn't possible, then at the very least
your adv7604 changes should provide the g/s_selection implementation. I don't want
to have to convert this driver later to g/s_selection.

Regards,

	Hans

>   media: adv7604: update timings on change of input signal
> 
>  drivers/media/i2c/adv7604.c | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 

Re: [PATCH 3/3] media: adv7604: update timings on change of input signal

[Sergei Shtylyov]

Hello.

On 12/11/2015 07:04 PM, Ulrich Hecht wrote:

> Without this, g_crop will always return the boot-time state.
>
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> ---
>   drivers/media/i2c/adv7604.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
>
> diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
> index 1bfa9f3..d7d0bb7 100644
> --- a/drivers/media/i2c/adv7604.c
> +++ b/drivers/media/i2c/adv7604.c
> @@ -1975,6 +1975,15 @@ static int adv76xx_isr(struct v4l2_subdev *sd, u32 status, bool *handled)
>
>   		v4l2_subdev_notify_event(sd, &adv76xx_ev_fmt);
>
> +		/* update timings */
> +		if (adv76xx_query_dv_timings(sd, &state->timings)
> +		    = -ENOLINK) {

    Please don't put the binary operators on the continuation line, leave them 
at the end of he broken up line.

[...]

MBR, Sergei

Re: [PATCH 3/3] media: adv7604: update timings on change of input signal

[Sergei Shtylyov]

Hello.

On 12/11/2015 07:04 PM, Ulrich Hecht wrote:

> Without this, g_crop will always return the boot-time state.
>
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> ---
>   drivers/media/i2c/adv7604.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
>
> diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
> index 1bfa9f3..d7d0bb7 100644
> --- a/drivers/media/i2c/adv7604.c
> +++ b/drivers/media/i2c/adv7604.c
> @@ -1975,6 +1975,15 @@ static int adv76xx_isr(struct v4l2_subdev *sd, u32 status, bool *handled)
>
>   		v4l2_subdev_notify_event(sd, &adv76xx_ev_fmt);
>
> +		/* update timings */
> +		if (adv76xx_query_dv_timings(sd, &state->timings)
> +		    == -ENOLINK) {

    Please don't put the binary operators on the continuation line, leave them 
at the end of he broken up line.

[...]

MBR, Sergei

Re: [PATCH 0/3] adv7604: .g_crop and .cropcap support

[Laurent Pinchart]

Hi Hans,

On Friday 11 December 2015 17:25:40 Hans Verkuil wrote:
> On 12/11/2015 05:04 PM, Ulrich Hecht wrote:
> > Hi!
> > 
> > The rcar_vin driver relies on these methods.  The third patch makes sure
> > that they return up-to-date data if the input signal has changed since
> > initialization.
> > 
> > CU
> > Uli
> > 
> > Ulrich Hecht (3):
> >   media: adv7604: implement g_crop
> >   media: adv7604: implement cropcap
> 
> I'm not keen on these changes. The reason is that these ops are deprecated
> and soc-camera is - almost - the last user. The g/s_selection ops should be
> used instead.
> 
> Now, I have a patch that changes soc-camera to g/s_selection. The reason it
> was never applied is that I had a hard time finding hardware to test it
> with.
> 
> Since you clearly have that hardware I think I'll rebase my (by now rather
> old) patch and post it again. If you can switch the adv7604 patch to
> g/s_selection and everything works with my patch, then I think I should
> just make a pull request for it.
> 
> I hope to be able to do this on Monday.
> 
> If switching soc-camera over to g/s_selection isn't possible, then at the
> very least your adv7604 changes should provide the g/s_selection
> implementation. I don't want to have to convert this driver later to
> g/s_selection.

I understand your concern and i agree with you. Our plan is to move the rcar-
vin driver away from soc-camera. Unfortunately that will take some time, and 
being able to use the adv7604 driver with rcar-vin would be very handy for 
testing on some of our boards.

Let's see how g/s_selection support in soc-camera works out and then decide on 
what to do.

> >   media: adv7604: update timings on change of input signal
> >  
> >  drivers/media/i2c/adv7604.c | 38 ++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 38 insertions(+)

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/3] adv7604: .g_crop and .cropcap support

[Laurent Pinchart]

Hi Hans,

On Friday 11 December 2015 17:25:40 Hans Verkuil wrote:
> On 12/11/2015 05:04 PM, Ulrich Hecht wrote:
> > Hi!
> > 
> > The rcar_vin driver relies on these methods.  The third patch makes sure
> > that they return up-to-date data if the input signal has changed since
> > initialization.
> > 
> > CU
> > Uli
> > 
> > Ulrich Hecht (3):
> >   media: adv7604: implement g_crop
> >   media: adv7604: implement cropcap
> 
> I'm not keen on these changes. The reason is that these ops are deprecated
> and soc-camera is - almost - the last user. The g/s_selection ops should be
> used instead.
> 
> Now, I have a patch that changes soc-camera to g/s_selection. The reason it
> was never applied is that I had a hard time finding hardware to test it
> with.
> 
> Since you clearly have that hardware I think I'll rebase my (by now rather
> old) patch and post it again. If you can switch the adv7604 patch to
> g/s_selection and everything works with my patch, then I think I should
> just make a pull request for it.
> 
> I hope to be able to do this on Monday.
> 
> If switching soc-camera over to g/s_selection isn't possible, then at the
> very least your adv7604 changes should provide the g/s_selection
> implementation. I don't want to have to convert this driver later to
> g/s_selection.

I understand your concern and i agree with you. Our plan is to move the rcar-
vin driver away from soc-camera. Unfortunately that will take some time, and 
being able to use the adv7604 driver with rcar-vin would be very handy for 
testing on some of our boards.

Let's see how g/s_selection support in soc-camera works out and then decide on 
what to do.

> >   media: adv7604: update timings on change of input signal
> >  
> >  drivers/media/i2c/adv7604.c | 38 ++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 38 insertions(+)

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/3] media: adv7604: implement g_crop

[Laurent Pinchart]

Hi Ulrich,

Thank you for the patch.

On Friday 11 December 2015 17:04:51 Ulrich Hecht wrote:
> The rcar_vin driver relies on this.
> 
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> ---
>  drivers/media/i2c/adv7604.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
> index 129009f..d30e7cc 100644
> --- a/drivers/media/i2c/adv7604.c
> +++ b/drivers/media/i2c/adv7604.c
> @@ -1885,6 +1885,17 @@ static int adv76xx_get_format(struct v4l2_subdev *sd,
> return 0;
>  }
> 
> +static int adv76xx_g_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
> +{
> +	struct adv76xx_state *state = to_state(sd);
> +
> +	a->c.height = state->timings.bt.height;
> +	a->c.width  = state->timings.bt.width;

Shouldn't you set a->c.top and a->c.left to 0 ? There's no guarantee that the 
caller will always zero the structure before calling you.

> +	a->type	    = V4L2_BUF_TYPE_VIDEO_CAPTURE;

The type field is an input parameter, you should just return -EINVAL if the 
value is not V4L2_BUF_TYPE_VIDEO_CAPTURE.

> +
> +	return 0;
> +}
> +
>  static int adv76xx_set_format(struct v4l2_subdev *sd,
>  			      struct v4l2_subdev_pad_config *cfg,
>  			      struct v4l2_subdev_format *format)
> @@ -2407,6 +2418,7 @@ static const struct v4l2_subdev_core_ops
> adv76xx_core_ops = {
> 
>  static const struct v4l2_subdev_video_ops adv76xx_video_ops = {
>  	.s_routing = adv76xx_s_routing,
> +	.g_crop = adv76xx_g_crop,
>  	.g_input_status = adv76xx_g_input_status,
>  	.s_dv_timings = adv76xx_s_dv_timings,
>  	.g_dv_timings = adv76xx_g_dv_timings,

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/3] media: adv7604: implement g_crop

[Laurent Pinchart]

Hi Ulrich,

Thank you for the patch.

On Friday 11 December 2015 17:04:51 Ulrich Hecht wrote:
> The rcar_vin driver relies on this.
> 
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> ---
>  drivers/media/i2c/adv7604.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
> index 129009f..d30e7cc 100644
> --- a/drivers/media/i2c/adv7604.c
> +++ b/drivers/media/i2c/adv7604.c
> @@ -1885,6 +1885,17 @@ static int adv76xx_get_format(struct v4l2_subdev *sd,
> return 0;
>  }
> 
> +static int adv76xx_g_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
> +{
> +	struct adv76xx_state *state = to_state(sd);
> +
> +	a->c.height = state->timings.bt.height;
> +	a->c.width  = state->timings.bt.width;

Shouldn't you set a->c.top and a->c.left to 0 ? There's no guarantee that the 
caller will always zero the structure before calling you.

> +	a->type	    = V4L2_BUF_TYPE_VIDEO_CAPTURE;

The type field is an input parameter, you should just return -EINVAL if the 
value is not V4L2_BUF_TYPE_VIDEO_CAPTURE.

> +
> +	return 0;
> +}
> +
>  static int adv76xx_set_format(struct v4l2_subdev *sd,
>  			      struct v4l2_subdev_pad_config *cfg,
>  			      struct v4l2_subdev_format *format)
> @@ -2407,6 +2418,7 @@ static const struct v4l2_subdev_core_ops
> adv76xx_core_ops = {
> 
>  static const struct v4l2_subdev_video_ops adv76xx_video_ops = {
>  	.s_routing = adv76xx_s_routing,
> +	.g_crop = adv76xx_g_crop,
>  	.g_input_status = adv76xx_g_input_status,
>  	.s_dv_timings = adv76xx_s_dv_timings,
>  	.g_dv_timings = adv76xx_g_dv_timings,

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/3] media: adv7604: implement cropcap

[Laurent Pinchart]

Hi Ulrich,

Thank you for the patch.

On Friday 11 December 2015 17:04:52 Ulrich Hecht wrote:
> Used by the rcar_vin driver.
> 
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> ---
>  drivers/media/i2c/adv7604.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
> index d30e7cc..1bfa9f3 100644
> --- a/drivers/media/i2c/adv7604.c
> +++ b/drivers/media/i2c/adv7604.c
> @@ -1896,6 +1896,22 @@ static int adv76xx_g_crop(struct v4l2_subdev *sd,
> struct v4l2_crop *a) return 0;
>  }
> 
> +static int adv76xx_cropcap(struct v4l2_subdev *sd, struct v4l2_cropcap *a)
> +{
> +	struct adv76xx_state *state = to_state(sd);
> +
> +	a->bounds.left	 = 0;
> +	a->bounds.top	 = 0;
> +	a->bounds.width	 = state->timings.bt.width;
> +	a->bounds.height = state->timings.bt.height;
> +	a->defrect	 = a->bounds;
> +	a->type		 = V4L2_BUF_TYPE_VIDEO_CAPTURE;

As for patch 1/3 the type field is an input parameter.

> +	a->pixelaspect.numerator   = 1;
> +	a->pixelaspect.denominator = 1;

I'm curious, is this always true with HDMI ?

> +
> +	return 0;
> +}
> +
>  static int adv76xx_set_format(struct v4l2_subdev *sd,
>  			      struct v4l2_subdev_pad_config *cfg,
>  			      struct v4l2_subdev_format *format)
> @@ -2419,6 +2435,7 @@ static const struct v4l2_subdev_core_ops
> adv76xx_core_ops = { static const struct v4l2_subdev_video_ops
> adv76xx_video_ops = {
>  	.s_routing = adv76xx_s_routing,
>  	.g_crop = adv76xx_g_crop,
> +	.cropcap = adv76xx_cropcap,
>  	.g_input_status = adv76xx_g_input_status,
>  	.s_dv_timings = adv76xx_s_dv_timings,
>  	.g_dv_timings = adv76xx_g_dv_timings,

-- 
Regards,

Laurent Pinchart

Re: [PATCH 2/3] media: adv7604: implement cropcap

[Laurent Pinchart]

Hi Ulrich,

Thank you for the patch.

On Friday 11 December 2015 17:04:52 Ulrich Hecht wrote:
> Used by the rcar_vin driver.
> 
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> ---
>  drivers/media/i2c/adv7604.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
> index d30e7cc..1bfa9f3 100644
> --- a/drivers/media/i2c/adv7604.c
> +++ b/drivers/media/i2c/adv7604.c
> @@ -1896,6 +1896,22 @@ static int adv76xx_g_crop(struct v4l2_subdev *sd,
> struct v4l2_crop *a) return 0;
>  }
> 
> +static int adv76xx_cropcap(struct v4l2_subdev *sd, struct v4l2_cropcap *a)
> +{
> +	struct adv76xx_state *state = to_state(sd);
> +
> +	a->bounds.left	 = 0;
> +	a->bounds.top	 = 0;
> +	a->bounds.width	 = state->timings.bt.width;
> +	a->bounds.height = state->timings.bt.height;
> +	a->defrect	 = a->bounds;
> +	a->type		 = V4L2_BUF_TYPE_VIDEO_CAPTURE;

As for patch 1/3 the type field is an input parameter.

> +	a->pixelaspect.numerator   = 1;
> +	a->pixelaspect.denominator = 1;

I'm curious, is this always true with HDMI ?

> +
> +	return 0;
> +}
> +
>  static int adv76xx_set_format(struct v4l2_subdev *sd,
>  			      struct v4l2_subdev_pad_config *cfg,
>  			      struct v4l2_subdev_format *format)
> @@ -2419,6 +2435,7 @@ static const struct v4l2_subdev_core_ops
> adv76xx_core_ops = { static const struct v4l2_subdev_video_ops
> adv76xx_video_ops = {
>  	.s_routing = adv76xx_s_routing,
>  	.g_crop = adv76xx_g_crop,
> +	.cropcap = adv76xx_cropcap,
>  	.g_input_status = adv76xx_g_input_status,
>  	.s_dv_timings = adv76xx_s_dv_timings,
>  	.g_dv_timings = adv76xx_g_dv_timings,

-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/3] media: adv7604: update timings on change of input signal

[Laurent Pinchart]

Hi Ulrich,

Thank you for the patch.

On Friday 11 December 2015 17:04:53 Ulrich Hecht wrote:
> Without this, g_crop will always return the boot-time state.
> 
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> ---
>  drivers/media/i2c/adv7604.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
> index 1bfa9f3..d7d0bb7 100644
> --- a/drivers/media/i2c/adv7604.c
> +++ b/drivers/media/i2c/adv7604.c
> @@ -1975,6 +1975,15 @@ static int adv76xx_isr(struct v4l2_subdev *sd, u32
> status, bool *handled)
> 
>  		v4l2_subdev_notify_event(sd, &adv76xx_ev_fmt);
> 
> +		/* update timings */
> +		if (adv76xx_query_dv_timings(sd, &state->timings)
> +		    = -ENOLINK) {

Nitpicking, I would write this as

		ret = adv76xx_query_dv_timings(sd, &state->timings);
		if (ret = -ENOLINK) {

to make it more explicit that the function has side effects. Functions called 
inside an if () statement are often assumed (at least by me) to perform checks 
only and not modify their parameters.

> +			/* no signal, fall back to default timings */
> +			const struct v4l2_dv_timings cea640x480 > +				V4L2_DV_BT_CEA_640X480P59_94;
> +			state->timings = cea640x480;

You can write this as

			state->timings = (struct v4l2_dv_timings)
				V4L2_DV_BT_CEA_640X480P59_94;

without using a local variable.

(And now that I mention that I wonder whether the definition of 
V4L2_DV_BT_CEA_640X480P59_94 should be updated to include the (struct 
v4l2_dv_timings))

> +		}
> +
>  		if (handled)
>  			*handled = true;
>  	}

-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/3] media: adv7604: update timings on change of input signal

[Laurent Pinchart]

Hi Ulrich,

Thank you for the patch.

On Friday 11 December 2015 17:04:53 Ulrich Hecht wrote:
> Without this, g_crop will always return the boot-time state.
> 
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> ---
>  drivers/media/i2c/adv7604.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
> index 1bfa9f3..d7d0bb7 100644
> --- a/drivers/media/i2c/adv7604.c
> +++ b/drivers/media/i2c/adv7604.c
> @@ -1975,6 +1975,15 @@ static int adv76xx_isr(struct v4l2_subdev *sd, u32
> status, bool *handled)
> 
>  		v4l2_subdev_notify_event(sd, &adv76xx_ev_fmt);
> 
> +		/* update timings */
> +		if (adv76xx_query_dv_timings(sd, &state->timings)
> +		    == -ENOLINK) {

Nitpicking, I would write this as

		ret = adv76xx_query_dv_timings(sd, &state->timings);
		if (ret == -ENOLINK) {

to make it more explicit that the function has side effects. Functions called 
inside an if () statement are often assumed (at least by me) to perform checks 
only and not modify their parameters.

> +			/* no signal, fall back to default timings */
> +			const struct v4l2_dv_timings cea640x480 =
> +				V4L2_DV_BT_CEA_640X480P59_94;
> +			state->timings = cea640x480;

You can write this as

			state->timings = (struct v4l2_dv_timings)
				V4L2_DV_BT_CEA_640X480P59_94;

without using a local variable.

(And now that I mention that I wonder whether the definition of 
V4L2_DV_BT_CEA_640X480P59_94 should be updated to include the (struct 
v4l2_dv_timings))

> +		}
> +
>  		if (handled)
>  			*handled = true;
>  	}

-- 
Regards,

Laurent Pinchart

Re: [PATCH 0/3] adv7604: .g_crop and .cropcap support

[Hans Verkuil]

On 12/11/2015 05:25 PM, Hans Verkuil wrote:
> Hi Ulrich,
> 
> On 12/11/2015 05:04 PM, Ulrich Hecht wrote:
>> Hi!
>>
>> The rcar_vin driver relies on these methods.  The third patch makes sure
>> that they return up-to-date data if the input signal has changed since
>> initialization.
>>
>> CU
>> Uli
>>
>>
>> Ulrich Hecht (3):
>>   media: adv7604: implement g_crop
>>   media: adv7604: implement cropcap
> 
> I'm not keen on these changes. The reason is that these ops are deprecated and
> soc-camera is - almost - the last user. The g/s_selection ops should be used instead.
> 
> Now, I have a patch that changes soc-camera to g/s_selection. The reason it was never
> applied is that I had a hard time finding hardware to test it with.
> 
> Since you clearly have that hardware I think I'll rebase my (by now rather old) patch
> and post it again. If you can switch the adv7604 patch to g/s_selection and everything
> works with my patch, then I think I should just make a pull request for it.
> 
> I hope to be able to do this on Monday.

OK, my http://git.linuxtv.org/hverkuil/media_tree.git/log/?h=rmcrop branch now has a
rebased patch to remove g/s_crop. Only compile-tested. It's just the one patch that you
need.

Regards,

	Hans

> If switching soc-camera over to g/s_selection isn't possible, then at the very least
> your adv7604 changes should provide the g/s_selection implementation. I don't want
> to have to convert this driver later to g/s_selection.
> 
> Regards,
> 
> 	Hans
> 
>>   media: adv7604: update timings on change of input signal
>>
>>  drivers/media/i2c/adv7604.c | 38 ++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 38 insertions(+)
>>
> 
> --
> 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
> 

Re: [PATCH 0/3] adv7604: .g_crop and .cropcap support

[Hans Verkuil]

On 12/11/2015 05:25 PM, Hans Verkuil wrote:
> Hi Ulrich,
> 
> On 12/11/2015 05:04 PM, Ulrich Hecht wrote:
>> Hi!
>>
>> The rcar_vin driver relies on these methods.  The third patch makes sure
>> that they return up-to-date data if the input signal has changed since
>> initialization.
>>
>> CU
>> Uli
>>
>>
>> Ulrich Hecht (3):
>>   media: adv7604: implement g_crop
>>   media: adv7604: implement cropcap
> 
> I'm not keen on these changes. The reason is that these ops are deprecated and
> soc-camera is - almost - the last user. The g/s_selection ops should be used instead.
> 
> Now, I have a patch that changes soc-camera to g/s_selection. The reason it was never
> applied is that I had a hard time finding hardware to test it with.
> 
> Since you clearly have that hardware I think I'll rebase my (by now rather old) patch
> and post it again. If you can switch the adv7604 patch to g/s_selection and everything
> works with my patch, then I think I should just make a pull request for it.
> 
> I hope to be able to do this on Monday.

OK, my http://git.linuxtv.org/hverkuil/media_tree.git/log/?h=rmcrop branch now has a
rebased patch to remove g/s_crop. Only compile-tested. It's just the one patch that you
need.

Regards,

	Hans

> If switching soc-camera over to g/s_selection isn't possible, then at the very least
> your adv7604 changes should provide the g/s_selection implementation. I don't want
> to have to convert this driver later to g/s_selection.
> 
> Regards,
> 
> 	Hans
> 
>>   media: adv7604: update timings on change of input signal
>>
>>  drivers/media/i2c/adv7604.c | 38 ++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 38 insertions(+)
>>
> 
> --
> 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
> 

Re: [PATCH 0/3] adv7604: .g_crop and .cropcap support

[Hans Verkuil]

On 12/13/2015 07:10 PM, Laurent Pinchart wrote:
> Hi Hans,
> 
> On Friday 11 December 2015 17:25:40 Hans Verkuil wrote:
>> On 12/11/2015 05:04 PM, Ulrich Hecht wrote:
>>> Hi!
>>>
>>> The rcar_vin driver relies on these methods.  The third patch makes sure
>>> that they return up-to-date data if the input signal has changed since
>>> initialization.
>>>
>>> CU
>>> Uli
>>>
>>> Ulrich Hecht (3):
>>>   media: adv7604: implement g_crop
>>>   media: adv7604: implement cropcap
>>
>> I'm not keen on these changes. The reason is that these ops are deprecated
>> and soc-camera is - almost - the last user. The g/s_selection ops should be
>> used instead.
>>
>> Now, I have a patch that changes soc-camera to g/s_selection. The reason it
>> was never applied is that I had a hard time finding hardware to test it
>> with.
>>
>> Since you clearly have that hardware I think I'll rebase my (by now rather
>> old) patch and post it again. If you can switch the adv7604 patch to
>> g/s_selection and everything works with my patch, then I think I should
>> just make a pull request for it.
>>
>> I hope to be able to do this on Monday.
>>
>> If switching soc-camera over to g/s_selection isn't possible, then at the
>> very least your adv7604 changes should provide the g/s_selection
>> implementation. I don't want to have to convert this driver later to
>> g/s_selection.
> 
> I understand your concern and i agree with you. Our plan is to move the rcar-
> vin driver away from soc-camera. Unfortunately that will take some time, and 
> being able to use the adv7604 driver with rcar-vin would be very handy for 
> testing on some of our boards.

That would be so nice. Once rcar is converted, then we need to take a really
good look at soc-camera and decide what to do with it.

> Let's see how g/s_selection support in soc-camera works out and then decide on 
> what to do.

Ack.

	Hans

Re: [PATCH 0/3] adv7604: .g_crop and .cropcap support

[Hans Verkuil]

On 12/13/2015 07:10 PM, Laurent Pinchart wrote:
> Hi Hans,
> 
> On Friday 11 December 2015 17:25:40 Hans Verkuil wrote:
>> On 12/11/2015 05:04 PM, Ulrich Hecht wrote:
>>> Hi!
>>>
>>> The rcar_vin driver relies on these methods.  The third patch makes sure
>>> that they return up-to-date data if the input signal has changed since
>>> initialization.
>>>
>>> CU
>>> Uli
>>>
>>> Ulrich Hecht (3):
>>>   media: adv7604: implement g_crop
>>>   media: adv7604: implement cropcap
>>
>> I'm not keen on these changes. The reason is that these ops are deprecated
>> and soc-camera is - almost - the last user. The g/s_selection ops should be
>> used instead.
>>
>> Now, I have a patch that changes soc-camera to g/s_selection. The reason it
>> was never applied is that I had a hard time finding hardware to test it
>> with.
>>
>> Since you clearly have that hardware I think I'll rebase my (by now rather
>> old) patch and post it again. If you can switch the adv7604 patch to
>> g/s_selection and everything works with my patch, then I think I should
>> just make a pull request for it.
>>
>> I hope to be able to do this on Monday.
>>
>> If switching soc-camera over to g/s_selection isn't possible, then at the
>> very least your adv7604 changes should provide the g/s_selection
>> implementation. I don't want to have to convert this driver later to
>> g/s_selection.
> 
> I understand your concern and i agree with you. Our plan is to move the rcar-
> vin driver away from soc-camera. Unfortunately that will take some time, and 
> being able to use the adv7604 driver with rcar-vin would be very handy for 
> testing on some of our boards.

That would be so nice. Once rcar is converted, then we need to take a really
good look at soc-camera and decide what to do with it.

> Let's see how g/s_selection support in soc-camera works out and then decide on 
> what to do.

Ack.

	Hans

Re: [PATCH 0/3] adv7604: .g_crop and .cropcap support

[Ulrich Hecht]

On Mon, Dec 14, 2015 at 11:33 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> OK, my http://git.linuxtv.org/hverkuil/media_tree.git/log/?h=rmcrop branch now has a
> rebased patch to remove g/s_crop. Only compile-tested. It's just the one patch that you
> need.

Thank you, that works perfectly with rcar_vin and adv7604; I'll send a
revised series.

CU
Uli

Re: [PATCH 0/3] adv7604: .g_crop and .cropcap support

[Ulrich Hecht]

On Mon, Dec 14, 2015 at 11:33 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> OK, my http://git.linuxtv.org/hverkuil/media_tree.git/log/?h=rmcrop branch now has a
> rebased patch to remove g/s_crop. Only compile-tested. It's just the one patch that you
> need.

Thank you, that works perfectly with rcar_vin and adv7604; I'll send a
revised series.

CU
Uli

Re: [PATCH 0/3] adv7604: .g_crop and .cropcap support

[Hans Verkuil]

On 12/14/2015 01:55 PM, Ulrich Hecht wrote:
> On Mon, Dec 14, 2015 at 11:33 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>> OK, my http://git.linuxtv.org/hverkuil/media_tree.git/log/?h=rmcrop branch now has a
>> rebased patch to remove g/s_crop. Only compile-tested. It's just the one patch that you
>> need.
> 
> Thank you, that works perfectly with rcar_vin and adv7604; I'll send a
> revised series.

Just making sure: you have actually tested cropping with my patch?

It would also be interesting to see if you can run v4l2-compliance for rcar. See what it says.

Running with the -f option would be the ultimate test, but I think that's too much to ask.

Regards,

	Hans

Re: [PATCH 0/3] adv7604: .g_crop and .cropcap support

[Hans Verkuil]

On 12/14/2015 01:55 PM, Ulrich Hecht wrote:
> On Mon, Dec 14, 2015 at 11:33 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>> OK, my http://git.linuxtv.org/hverkuil/media_tree.git/log/?h=rmcrop branch now has a
>> rebased patch to remove g/s_crop. Only compile-tested. It's just the one patch that you
>> need.
> 
> Thank you, that works perfectly with rcar_vin and adv7604; I'll send a
> revised series.

Just making sure: you have actually tested cropping with my patch?

It would also be interesting to see if you can run v4l2-compliance for rcar. See what it says.

Running with the -f option would be the ultimate test, but I think that's too much to ask.

Regards,

	Hans

Re: [PATCH 0/3] adv7604: .g_crop and .cropcap support

[Ulrich Hecht]

On Mon, Dec 14, 2015 at 2:02 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> On 12/14/2015 01:55 PM, Ulrich Hecht wrote:
>> On Mon, Dec 14, 2015 at 11:33 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>>> OK, my http://git.linuxtv.org/hverkuil/media_tree.git/log/?h=rmcrop branch now has a
>>> rebased patch to remove g/s_crop. Only compile-tested. It's just the one patch that you
>>> need.
>>
>> Thank you, that works perfectly with rcar_vin and adv7604; I'll send a
>> revised series.
>
> Just making sure: you have actually tested cropping with my patch?

OK, I'll revise my statement: The "get" part works perfectly. :)

>
> It would also be interesting to see if you can run v4l2-compliance for rcar. See what it says.

Driver Info:
       Driver name   : e6ef0000.video
       Card type     : R_Car_VIN
       Bus info      : platform:rcar_vin0
       Driver version: 4.4.0
       Capabilities  : 0x84200001
               Video Capture
               Streaming
               Extended Pix Format
               Device Capabilities
       Device Caps   : 0x04200001
               Video Capture
               Streaming
               Extended Pix Format

Compliance test for device /dev/video0 (not using libv4l2):

Required ioctls:
       test VIDIOC_QUERYCAP: OK

Allow for multiple opens:
       test second video open: OK
       test VIDIOC_QUERYCAP: OK
               fail: v4l2-compliance.cpp(585): prio != match
       test VIDIOC_G/S_PRIORITY: FAIL

Debug ioctls:
       test VIDIOC_DBG_G/S_REGISTER: OK
       test VIDIOC_LOG_STATUS: OK (Not Supported)

Input ioctls:
       test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
       test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
       test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
       test VIDIOC_ENUMAUDIO: OK (Not Supported)
       test VIDIOC_G/S/ENUMINPUT: OK
       test VIDIOC_G/S_AUDIO: OK (Not Supported)
       Inputs: 1 Audio Inputs: 0 Tuners: 0

Output ioctls:
       test VIDIOC_G/S_MODULATOR: OK (Not Supported)
       test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
       test VIDIOC_ENUMAUDOUT: OK (Not Supported)
       test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
       test VIDIOC_G/S_AUDOUT: OK (Not Supported)
       Outputs: 0 Audio Outputs: 0 Modulators: 0

Input/Output configuration ioctls:
       test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
       test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
       test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
       test VIDIOC_G/S_EDID: OK (Not Supported)

Test input 0:

       Control ioctls:
               test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
               test VIDIOC_QUERYCTRL: OK
               test VIDIOC_G/S_CTRL: OK
               test VIDIOC_G/S/TRY_EXT_CTRLS: OK
               fail: v4l2-test-controls.cpp(782): subscribe event for
control 'User Co
ntrols' failed
               test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: FAIL
               test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
               Standard Controls: 5 Private Controls: 0

       Format ioctls:
               fail: v4l2-test-formats.cpp(268): duplicate format 34424752
               test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: FAIL
               test VIDIOC_G/S_PARM: OK (Not Supported)
               test VIDIOC_G_FBUF: OK (Not Supported)
               test VIDIOC_G_FMT: OK
               test VIDIOC_TRY_FMT: OK
               warn: v4l2-test-formats.cpp(827): Could not set fmt2
               test VIDIOC_S_FMT: OK
               test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
               test Cropping: OK
               test Composing: OK
               test Scaling: OK (Not Supported)

       Codec ioctls:
               test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
               test VIDIOC_G_ENC_INDEX: OK (Not Supported)
               test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)

       Buffer ioctls:
               test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
               test VIDIOC_EXPBUF: OK

Test input 0:


Total: 42, Succeeded: 39, Failed: 3, Warnings: 1

CU
Uli

Re: [PATCH 0/3] adv7604: .g_crop and .cropcap support

[Ulrich Hecht]

On Mon, Dec 14, 2015 at 2:02 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> On 12/14/2015 01:55 PM, Ulrich Hecht wrote:
>> On Mon, Dec 14, 2015 at 11:33 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>>> OK, my http://git.linuxtv.org/hverkuil/media_tree.git/log/?h=rmcrop branch now has a
>>> rebased patch to remove g/s_crop. Only compile-tested. It's just the one patch that you
>>> need.
>>
>> Thank you, that works perfectly with rcar_vin and adv7604; I'll send a
>> revised series.
>
> Just making sure: you have actually tested cropping with my patch?

OK, I'll revise my statement: The "get" part works perfectly. :)

>
> It would also be interesting to see if you can run v4l2-compliance for rcar. See what it says.

Driver Info:
       Driver name   : e6ef0000.video
       Card type     : R_Car_VIN
       Bus info      : platform:rcar_vin0
       Driver version: 4.4.0
       Capabilities  : 0x84200001
               Video Capture
               Streaming
               Extended Pix Format
               Device Capabilities
       Device Caps   : 0x04200001
               Video Capture
               Streaming
               Extended Pix Format

Compliance test for device /dev/video0 (not using libv4l2):

Required ioctls:
       test VIDIOC_QUERYCAP: OK

Allow for multiple opens:
       test second video open: OK
       test VIDIOC_QUERYCAP: OK
               fail: v4l2-compliance.cpp(585): prio != match
       test VIDIOC_G/S_PRIORITY: FAIL

Debug ioctls:
       test VIDIOC_DBG_G/S_REGISTER: OK
       test VIDIOC_LOG_STATUS: OK (Not Supported)

Input ioctls:
       test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
       test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
       test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
       test VIDIOC_ENUMAUDIO: OK (Not Supported)
       test VIDIOC_G/S/ENUMINPUT: OK
       test VIDIOC_G/S_AUDIO: OK (Not Supported)
       Inputs: 1 Audio Inputs: 0 Tuners: 0

Output ioctls:
       test VIDIOC_G/S_MODULATOR: OK (Not Supported)
       test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
       test VIDIOC_ENUMAUDOUT: OK (Not Supported)
       test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
       test VIDIOC_G/S_AUDOUT: OK (Not Supported)
       Outputs: 0 Audio Outputs: 0 Modulators: 0

Input/Output configuration ioctls:
       test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
       test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
       test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
       test VIDIOC_G/S_EDID: OK (Not Supported)

Test input 0:

       Control ioctls:
               test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
               test VIDIOC_QUERYCTRL: OK
               test VIDIOC_G/S_CTRL: OK
               test VIDIOC_G/S/TRY_EXT_CTRLS: OK
               fail: v4l2-test-controls.cpp(782): subscribe event for
control 'User Co
ntrols' failed
               test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: FAIL
               test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
               Standard Controls: 5 Private Controls: 0

       Format ioctls:
               fail: v4l2-test-formats.cpp(268): duplicate format 34424752
               test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: FAIL
               test VIDIOC_G/S_PARM: OK (Not Supported)
               test VIDIOC_G_FBUF: OK (Not Supported)
               test VIDIOC_G_FMT: OK
               test VIDIOC_TRY_FMT: OK
               warn: v4l2-test-formats.cpp(827): Could not set fmt2
               test VIDIOC_S_FMT: OK
               test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
               test Cropping: OK
               test Composing: OK
               test Scaling: OK (Not Supported)

       Codec ioctls:
               test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
               test VIDIOC_G_ENC_INDEX: OK (Not Supported)
               test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)

       Buffer ioctls:
               test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
               test VIDIOC_EXPBUF: OK

Test input 0:


Total: 42, Succeeded: 39, Failed: 3, Warnings: 1

CU
Uli