* [PATCH v8 0/2] rcar-vin EDID control ioctls
@ 2016-09-15 13:24 Ulrich Hecht
2016-09-15 13:24 ` [PATCH v8 1/2] media: adv7604: automatic "default-input" selection Ulrich Hecht
` (2 more replies)
0 siblings, 3 replies; 18+ messages in thread
From: Ulrich Hecht @ 2016-09-15 13:24 UTC (permalink / raw)
To: hans.verkuil, niklas.soderlund
Cc: linux-media, linux-renesas-soc, magnus.damm,
ulrich.hecht+renesas, laurent.pinchart, william.towle
Hi!
This is a spinoff of "Lager/Koelsch board HDMI input support" that excludes
the DT portions, and that works without the unmerged subdevice abstraction
layer.
This revision improves over v7 from earlier today in that it does not break
analog input devices...
CU
Uli
Changes since v7:
- do not fail if there is no sink pad
Changes since v6:
- work without subdev abstraction layer
- split off DT parts, to be handled separately
Changes since v5:
- implement vin/subdev pad translation
- move i2c devices
Changes since v4:
- drop merged patches
- adv7604: always fall back to input 0 if nothing else is specified
- rcar-vin: implement G_EDID, S_EDID in place of hard-coded EDID blob
Changes since v3:
- rvin_enum_dv_timings(): use vin->src_pad_idx
- rvin_dv_timings_cap(): likewise
- rvin_s_dv_timings(): update vin->format
- add Koelsch support
Changes since v2:
- rebased on top of rcar-vin driver v4
- removed "adv7604: fix SPA register location for ADV7612" (picked up)
- changed prefix of dts patch to "ARM: dts: lager: "
Ulrich Hecht (2):
media: adv7604: automatic "default-input" selection
rcar-vin: implement EDID control ioctls
drivers/media/i2c/adv7604.c | 5 +++-
drivers/media/platform/rcar-vin/rcar-v4l2.c | 42 +++++++++++++++++++++++++++++
drivers/media/platform/rcar-vin/rcar-vin.h | 1 +
3 files changed, 47 insertions(+), 1 deletion(-)
--
2.9.3
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v8 1/2] media: adv7604: automatic "default-input" selection
2016-09-15 13:24 [PATCH v8 0/2] rcar-vin EDID control ioctls Ulrich Hecht
@ 2016-09-15 13:24 ` Ulrich Hecht
2016-09-15 16:42 ` Laurent Pinchart
2016-09-15 13:24 ` [PATCH v8 2/2] rcar-vin: implement EDID control ioctls Ulrich Hecht
2016-09-15 13:25 ` [PATCH v8 0/2] rcar-vin " Hans Verkuil
2 siblings, 1 reply; 18+ messages in thread
From: Ulrich Hecht @ 2016-09-15 13:24 UTC (permalink / raw)
To: hans.verkuil, niklas.soderlund
Cc: linux-media, linux-renesas-soc, magnus.damm,
ulrich.hecht+renesas, laurent.pinchart, william.towle
Fall back to input 0 if "default-input" property is not present.
Additionally, documentation in commit bf9c82278c34 ("[media]
media: adv7604: ability to read default input port from DT") states
that the "default-input" property should reside directly in the node
for adv7612. Hence, also adjust the parsing to make the implementation
consistent with this.
Based on patch by William Towle <william.towle@codethink.co.uk>.
Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
---
drivers/media/i2c/adv7604.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
index 4003831..055c9df 100644
--- a/drivers/media/i2c/adv7604.c
+++ b/drivers/media/i2c/adv7604.c
@@ -3077,10 +3077,13 @@ static int adv76xx_parse_dt(struct adv76xx_state *state)
if (!of_property_read_u32(endpoint, "default-input", &v))
state->pdata.default_input = v;
else
- state->pdata.default_input = -1;
+ state->pdata.default_input = 0;
of_node_put(endpoint);
+ if (!of_property_read_u32(np, "default-input", &v))
+ state->pdata.default_input = v;
+
flags = bus_cfg.bus.parallel.flags;
if (flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH)
--
2.9.3
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v8 2/2] rcar-vin: implement EDID control ioctls
2016-09-15 13:24 [PATCH v8 0/2] rcar-vin EDID control ioctls Ulrich Hecht
2016-09-15 13:24 ` [PATCH v8 1/2] media: adv7604: automatic "default-input" selection Ulrich Hecht
@ 2016-09-15 13:24 ` Ulrich Hecht
2016-09-15 13:59 ` Niklas Söderlund
2016-09-15 16:47 ` Laurent Pinchart
2016-09-15 13:25 ` [PATCH v8 0/2] rcar-vin " Hans Verkuil
2 siblings, 2 replies; 18+ messages in thread
From: Ulrich Hecht @ 2016-09-15 13:24 UTC (permalink / raw)
To: hans.verkuil, niklas.soderlund
Cc: linux-media, linux-renesas-soc, magnus.damm,
ulrich.hecht+renesas, laurent.pinchart, william.towle
Adds G_EDID and S_EDID.
Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
---
drivers/media/platform/rcar-vin/rcar-v4l2.c | 42 +++++++++++++++++++++++++++++
drivers/media/platform/rcar-vin/rcar-vin.h | 1 +
2 files changed, 43 insertions(+)
diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
index 62ca7e3..f679182 100644
--- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
@@ -557,6 +557,38 @@ static int rvin_dv_timings_cap(struct file *file, void *priv_fh,
return ret;
}
+static int rvin_g_edid(struct file *file, void *fh, struct v4l2_edid *edid)
+{
+ struct rvin_dev *vin = video_drvdata(file);
+ struct v4l2_subdev *sd = vin_to_source(vin);
+ int input, ret;
+
+ input = edid->pad;
+ edid->pad = vin->sink_pad_idx;
+
+ ret = v4l2_subdev_call(sd, pad, get_edid, edid);
+
+ edid->pad = input;
+
+ return ret;
+}
+
+static int rvin_s_edid(struct file *file, void *fh, struct v4l2_edid *edid)
+{
+ struct rvin_dev *vin = video_drvdata(file);
+ struct v4l2_subdev *sd = vin_to_source(vin);
+ int input, ret;
+
+ input = edid->pad;
+ edid->pad = vin->sink_pad_idx;
+
+ ret = v4l2_subdev_call(sd, pad, set_edid, edid);
+
+ edid->pad = input;
+
+ return ret;
+}
+
static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
.vidioc_querycap = rvin_querycap,
.vidioc_try_fmt_vid_cap = rvin_try_fmt_vid_cap,
@@ -579,6 +611,9 @@ static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
.vidioc_s_dv_timings = rvin_s_dv_timings,
.vidioc_query_dv_timings = rvin_query_dv_timings,
+ .vidioc_g_edid = rvin_g_edid,
+ .vidioc_s_edid = rvin_s_edid,
+
.vidioc_querystd = rvin_querystd,
.vidioc_g_std = rvin_g_std,
.vidioc_s_std = rvin_s_std,
@@ -832,6 +867,13 @@ int rvin_v4l2_probe(struct rvin_dev *vin)
vin->src_pad_idx = pad_idx;
fmt.pad = vin->src_pad_idx;
+ vin->sink_pad_idx = 0;
+ for (pad_idx = 0; pad_idx < sd->entity.num_pads; pad_idx++)
+ if (sd->entity.pads[pad_idx].flags == MEDIA_PAD_FL_SINK) {
+ vin->sink_pad_idx = pad_idx;
+ break;
+ }
+
/* Try to improve our guess of a reasonable window format */
ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &fmt);
if (ret) {
diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
index 793184d..af815cc 100644
--- a/drivers/media/platform/rcar-vin/rcar-vin.h
+++ b/drivers/media/platform/rcar-vin/rcar-vin.h
@@ -121,6 +121,7 @@ struct rvin_dev {
struct video_device vdev;
struct v4l2_device v4l2_dev;
int src_pad_idx;
+ int sink_pad_idx;
struct v4l2_ctrl_handler ctrl_handler;
struct v4l2_async_notifier notifier;
struct rvin_graph_entity digital;
--
2.9.3
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v8 0/2] rcar-vin EDID control ioctls
2016-09-15 13:24 [PATCH v8 0/2] rcar-vin EDID control ioctls Ulrich Hecht
2016-09-15 13:24 ` [PATCH v8 1/2] media: adv7604: automatic "default-input" selection Ulrich Hecht
2016-09-15 13:24 ` [PATCH v8 2/2] rcar-vin: implement EDID control ioctls Ulrich Hecht
@ 2016-09-15 13:25 ` Hans Verkuil
2016-09-15 13:38 ` Ulrich Hecht
2 siblings, 1 reply; 18+ messages in thread
From: Hans Verkuil @ 2016-09-15 13:25 UTC (permalink / raw)
To: Ulrich Hecht, hans.verkuil, niklas.soderlund
Cc: linux-media, linux-renesas-soc, magnus.damm, laurent.pinchart,
william.towle
On 09/15/2016 03:24 PM, Ulrich Hecht wrote:
> Hi!
>
> This is a spinoff of "Lager/Koelsch board HDMI input support" that excludes
> the DT portions, and that works without the unmerged subdevice abstraction
> layer.
Are you going to post another patch series for the DT portions?
Regards,
Hans
>
> This revision improves over v7 from earlier today in that it does not break
> analog input devices...
>
> CU
> Uli
>
>
> Changes since v7:
> - do not fail if there is no sink pad
>
> Changes since v6:
> - work without subdev abstraction layer
> - split off DT parts, to be handled separately
>
> Changes since v5:
> - implement vin/subdev pad translation
> - move i2c devices
>
> Changes since v4:
> - drop merged patches
> - adv7604: always fall back to input 0 if nothing else is specified
> - rcar-vin: implement G_EDID, S_EDID in place of hard-coded EDID blob
>
> Changes since v3:
> - rvin_enum_dv_timings(): use vin->src_pad_idx
> - rvin_dv_timings_cap(): likewise
> - rvin_s_dv_timings(): update vin->format
> - add Koelsch support
>
> Changes since v2:
> - rebased on top of rcar-vin driver v4
> - removed "adv7604: fix SPA register location for ADV7612" (picked up)
> - changed prefix of dts patch to "ARM: dts: lager: "
>
>
> Ulrich Hecht (2):
> media: adv7604: automatic "default-input" selection
> rcar-vin: implement EDID control ioctls
>
> drivers/media/i2c/adv7604.c | 5 +++-
> drivers/media/platform/rcar-vin/rcar-v4l2.c | 42 +++++++++++++++++++++++++++++
> drivers/media/platform/rcar-vin/rcar-vin.h | 1 +
> 3 files changed, 47 insertions(+), 1 deletion(-)
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v8 0/2] rcar-vin EDID control ioctls
2016-09-15 13:25 ` [PATCH v8 0/2] rcar-vin " Hans Verkuil
@ 2016-09-15 13:38 ` Ulrich Hecht
0 siblings, 0 replies; 18+ messages in thread
From: Ulrich Hecht @ 2016-09-15 13:38 UTC (permalink / raw)
To: Hans Verkuil
Cc: hans.verkuil, Niklas Söderlund, Linux Media Mailing List,
Linux-Renesas, Magnus Damm, Laurent, William Towle
On Thu, Sep 15, 2016 at 3:25 PM, Hans Verkuil <hansverk@cisco.com> wrote:
> On 09/15/2016 03:24 PM, Ulrich Hecht wrote:
>>
>> Hi!
>>
>> This is a spinoff of "Lager/Koelsch board HDMI input support" that
>> excludes
>> the DT portions, and that works without the unmerged subdevice abstraction
>> layer.
>
>
> Are you going to post another patch series for the DT portions?
Yes, but not today.
CU
Uli
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v8 2/2] rcar-vin: implement EDID control ioctls
2016-09-15 13:24 ` [PATCH v8 2/2] rcar-vin: implement EDID control ioctls Ulrich Hecht
@ 2016-09-15 13:59 ` Niklas Söderlund
2016-09-15 16:47 ` Laurent Pinchart
1 sibling, 0 replies; 18+ messages in thread
From: Niklas Söderlund @ 2016-09-15 13:59 UTC (permalink / raw)
To: Ulrich Hecht
Cc: hans.verkuil, linux-media, linux-renesas-soc, magnus.damm,
laurent.pinchart, william.towle
Hi Ulrich,
Thanks for your patch.
Can you append another patch ontop of this one whit the following
change:
diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
index ac26738..7ba728d 100644
--- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
@@ -494,7 +494,7 @@ static int rvin_enum_dv_timings(struct file *file, void *priv_fh,
int pad, ret;
pad = timings->pad;
- timings->pad = vin->src_pad_idx;
+ timings->pad = vin->sink_pad_idx;
ret = v4l2_subdev_call(sd, pad, enum_dv_timings, timings);
@@ -548,7 +548,7 @@ static int rvin_dv_timings_cap(struct file *file, void *priv_fh,
int pad, ret;
pad = cap->pad;
- cap->pad = vin->src_pad_idx;
+ cap->pad = vin->sink_pad_idx;
ret = v4l2_subdev_call(sd, pad, dv_timings_cap, cap);
Whit that change the driver pass v4l2-compliance check for the HDMI
input on Koelsch for me.
On 2016-09-15 15:24:08 +0200, Ulrich Hecht wrote:
> Adds G_EDID and S_EDID.
>
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> ---
> drivers/media/platform/rcar-vin/rcar-v4l2.c | 42 +++++++++++++++++++++++++++++
> drivers/media/platform/rcar-vin/rcar-vin.h | 1 +
> 2 files changed, 43 insertions(+)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> index 62ca7e3..f679182 100644
> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> @@ -557,6 +557,38 @@ static int rvin_dv_timings_cap(struct file *file, void *priv_fh,
> return ret;
> }
>
> +static int rvin_g_edid(struct file *file, void *fh, struct v4l2_edid *edid)
> +{
> + struct rvin_dev *vin = video_drvdata(file);
> + struct v4l2_subdev *sd = vin_to_source(vin);
> + int input, ret;
> +
> + input = edid->pad;
> + edid->pad = vin->sink_pad_idx;
> +
> + ret = v4l2_subdev_call(sd, pad, get_edid, edid);
> +
> + edid->pad = input;
> +
> + return ret;
> +}
> +
> +static int rvin_s_edid(struct file *file, void *fh, struct v4l2_edid *edid)
> +{
> + struct rvin_dev *vin = video_drvdata(file);
> + struct v4l2_subdev *sd = vin_to_source(vin);
> + int input, ret;
> +
> + input = edid->pad;
> + edid->pad = vin->sink_pad_idx;
> +
> + ret = v4l2_subdev_call(sd, pad, set_edid, edid);
> +
> + edid->pad = input;
> +
> + return ret;
> +}
> +
> static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
> .vidioc_querycap = rvin_querycap,
> .vidioc_try_fmt_vid_cap = rvin_try_fmt_vid_cap,
> @@ -579,6 +611,9 @@ static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
> .vidioc_s_dv_timings = rvin_s_dv_timings,
> .vidioc_query_dv_timings = rvin_query_dv_timings,
>
> + .vidioc_g_edid = rvin_g_edid,
> + .vidioc_s_edid = rvin_s_edid,
> +
> .vidioc_querystd = rvin_querystd,
> .vidioc_g_std = rvin_g_std,
> .vidioc_s_std = rvin_s_std,
> @@ -832,6 +867,13 @@ int rvin_v4l2_probe(struct rvin_dev *vin)
> vin->src_pad_idx = pad_idx;
> fmt.pad = vin->src_pad_idx;
>
> + vin->sink_pad_idx = 0;
> + for (pad_idx = 0; pad_idx < sd->entity.num_pads; pad_idx++)
> + if (sd->entity.pads[pad_idx].flags == MEDIA_PAD_FL_SINK) {
> + vin->sink_pad_idx = pad_idx;
> + break;
> + }
> +
> /* Try to improve our guess of a reasonable window format */
> ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &fmt);
> if (ret) {
> diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
> index 793184d..af815cc 100644
> --- a/drivers/media/platform/rcar-vin/rcar-vin.h
> +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
> @@ -121,6 +121,7 @@ struct rvin_dev {
> struct video_device vdev;
> struct v4l2_device v4l2_dev;
> int src_pad_idx;
> + int sink_pad_idx;
sink_pad_idx should be documented in the comment above the struct. If
you fix that feel free to add my:
Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> struct v4l2_ctrl_handler ctrl_handler;
> struct v4l2_async_notifier notifier;
> struct rvin_graph_entity digital;
> --
> 2.9.3
>
--
Regards,
Niklas Söderlund
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v8 2/2] rcar-vin: implement EDID control ioctls
@ 2016-09-15 13:59 ` Niklas Söderlund
0 siblings, 0 replies; 18+ messages in thread
From: Niklas Söderlund @ 2016-09-15 13:59 UTC (permalink / raw)
To: Ulrich Hecht
Cc: hans.verkuil, linux-media, linux-renesas-soc, magnus.damm,
laurent.pinchart, william.towle
Hi Ulrich,
Thanks for your patch.
Can you append another patch ontop of this one whit the following
change:
diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
index ac26738..7ba728d 100644
--- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
@@ -494,7 +494,7 @@ static int rvin_enum_dv_timings(struct file *file, void *priv_fh,
int pad, ret;
pad = timings->pad;
- timings->pad = vin->src_pad_idx;
+ timings->pad = vin->sink_pad_idx;
ret = v4l2_subdev_call(sd, pad, enum_dv_timings, timings);
@@ -548,7 +548,7 @@ static int rvin_dv_timings_cap(struct file *file, void *priv_fh,
int pad, ret;
pad = cap->pad;
- cap->pad = vin->src_pad_idx;
+ cap->pad = vin->sink_pad_idx;
ret = v4l2_subdev_call(sd, pad, dv_timings_cap, cap);
Whit that change the driver pass v4l2-compliance check for the HDMI
input on Koelsch for me.
On 2016-09-15 15:24:08 +0200, Ulrich Hecht wrote:
> Adds G_EDID and S_EDID.
>
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> ---
> drivers/media/platform/rcar-vin/rcar-v4l2.c | 42 +++++++++++++++++++++++++++++
> drivers/media/platform/rcar-vin/rcar-vin.h | 1 +
> 2 files changed, 43 insertions(+)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> index 62ca7e3..f679182 100644
> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> @@ -557,6 +557,38 @@ static int rvin_dv_timings_cap(struct file *file, void *priv_fh,
> return ret;
> }
>
> +static int rvin_g_edid(struct file *file, void *fh, struct v4l2_edid *edid)
> +{
> + struct rvin_dev *vin = video_drvdata(file);
> + struct v4l2_subdev *sd = vin_to_source(vin);
> + int input, ret;
> +
> + input = edid->pad;
> + edid->pad = vin->sink_pad_idx;
> +
> + ret = v4l2_subdev_call(sd, pad, get_edid, edid);
> +
> + edid->pad = input;
> +
> + return ret;
> +}
> +
> +static int rvin_s_edid(struct file *file, void *fh, struct v4l2_edid *edid)
> +{
> + struct rvin_dev *vin = video_drvdata(file);
> + struct v4l2_subdev *sd = vin_to_source(vin);
> + int input, ret;
> +
> + input = edid->pad;
> + edid->pad = vin->sink_pad_idx;
> +
> + ret = v4l2_subdev_call(sd, pad, set_edid, edid);
> +
> + edid->pad = input;
> +
> + return ret;
> +}
> +
> static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
> .vidioc_querycap = rvin_querycap,
> .vidioc_try_fmt_vid_cap = rvin_try_fmt_vid_cap,
> @@ -579,6 +611,9 @@ static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
> .vidioc_s_dv_timings = rvin_s_dv_timings,
> .vidioc_query_dv_timings = rvin_query_dv_timings,
>
> + .vidioc_g_edid = rvin_g_edid,
> + .vidioc_s_edid = rvin_s_edid,
> +
> .vidioc_querystd = rvin_querystd,
> .vidioc_g_std = rvin_g_std,
> .vidioc_s_std = rvin_s_std,
> @@ -832,6 +867,13 @@ int rvin_v4l2_probe(struct rvin_dev *vin)
> vin->src_pad_idx = pad_idx;
> fmt.pad = vin->src_pad_idx;
>
> + vin->sink_pad_idx = 0;
> + for (pad_idx = 0; pad_idx < sd->entity.num_pads; pad_idx++)
> + if (sd->entity.pads[pad_idx].flags == MEDIA_PAD_FL_SINK) {
> + vin->sink_pad_idx = pad_idx;
> + break;
> + }
> +
> /* Try to improve our guess of a reasonable window format */
> ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &fmt);
> if (ret) {
> diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
> index 793184d..af815cc 100644
> --- a/drivers/media/platform/rcar-vin/rcar-vin.h
> +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
> @@ -121,6 +121,7 @@ struct rvin_dev {
> struct video_device vdev;
> struct v4l2_device v4l2_dev;
> int src_pad_idx;
> + int sink_pad_idx;
sink_pad_idx should be documented in the comment above the struct. If
you fix that feel free to add my:
Acked-by: Niklas S�derlund <niklas.soderlund+renesas@ragnatech.se>
> struct v4l2_ctrl_handler ctrl_handler;
> struct v4l2_async_notifier notifier;
> struct rvin_graph_entity digital;
> --
> 2.9.3
>
--
Regards,
Niklas S�derlund
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v8 2/2] rcar-vin: implement EDID control ioctls
2016-09-15 13:59 ` Niklas Söderlund
@ 2016-09-15 14:02 ` Hans Verkuil
-1 siblings, 0 replies; 18+ messages in thread
From: Hans Verkuil @ 2016-09-15 14:02 UTC (permalink / raw)
To: Niklas Söderlund, Ulrich Hecht
Cc: hans.verkuil, linux-media, linux-renesas-soc, magnus.damm,
laurent.pinchart, william.towle
Hi Ulrich,
A patch series on top of my tree with Niklas' patches already merged:
https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=rcar
would be ideal. Since otherwise I have to resolve a conflict.
Regards,
Hans
On 09/15/2016 03:59 PM, Niklas Söderlund wrote:
> Hi Ulrich,
>
> Thanks for your patch.
>
> Can you append another patch ontop of this one whit the following
> change:
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> index ac26738..7ba728d 100644
> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> @@ -494,7 +494,7 @@ static int rvin_enum_dv_timings(struct file *file, void *priv_fh,
> int pad, ret;
>
> pad = timings->pad;
> - timings->pad = vin->src_pad_idx;
> + timings->pad = vin->sink_pad_idx;
>
> ret = v4l2_subdev_call(sd, pad, enum_dv_timings, timings);
>
> @@ -548,7 +548,7 @@ static int rvin_dv_timings_cap(struct file *file, void *priv_fh,
> int pad, ret;
>
> pad = cap->pad;
> - cap->pad = vin->src_pad_idx;
> + cap->pad = vin->sink_pad_idx;
>
> ret = v4l2_subdev_call(sd, pad, dv_timings_cap, cap);
>
>
> Whit that change the driver pass v4l2-compliance check for the HDMI
> input on Koelsch for me.
>
> On 2016-09-15 15:24:08 +0200, Ulrich Hecht wrote:
>> Adds G_EDID and S_EDID.
>>
>> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
>> ---
>> drivers/media/platform/rcar-vin/rcar-v4l2.c | 42 +++++++++++++++++++++++++++++
>> drivers/media/platform/rcar-vin/rcar-vin.h | 1 +
>> 2 files changed, 43 insertions(+)
>>
>> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
>> index 62ca7e3..f679182 100644
>> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
>> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
>> @@ -557,6 +557,38 @@ static int rvin_dv_timings_cap(struct file *file, void *priv_fh,
>> return ret;
>> }
>>
>> +static int rvin_g_edid(struct file *file, void *fh, struct v4l2_edid *edid)
>> +{
>> + struct rvin_dev *vin = video_drvdata(file);
>> + struct v4l2_subdev *sd = vin_to_source(vin);
>> + int input, ret;
>> +
>> + input = edid->pad;
>> + edid->pad = vin->sink_pad_idx;
>> +
>> + ret = v4l2_subdev_call(sd, pad, get_edid, edid);
>> +
>> + edid->pad = input;
>> +
>> + return ret;
>> +}
>> +
>> +static int rvin_s_edid(struct file *file, void *fh, struct v4l2_edid *edid)
>> +{
>> + struct rvin_dev *vin = video_drvdata(file);
>> + struct v4l2_subdev *sd = vin_to_source(vin);
>> + int input, ret;
>> +
>> + input = edid->pad;
>> + edid->pad = vin->sink_pad_idx;
>> +
>> + ret = v4l2_subdev_call(sd, pad, set_edid, edid);
>> +
>> + edid->pad = input;
>> +
>> + return ret;
>> +}
>> +
>> static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
>> .vidioc_querycap = rvin_querycap,
>> .vidioc_try_fmt_vid_cap = rvin_try_fmt_vid_cap,
>> @@ -579,6 +611,9 @@ static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
>> .vidioc_s_dv_timings = rvin_s_dv_timings,
>> .vidioc_query_dv_timings = rvin_query_dv_timings,
>>
>> + .vidioc_g_edid = rvin_g_edid,
>> + .vidioc_s_edid = rvin_s_edid,
>> +
>> .vidioc_querystd = rvin_querystd,
>> .vidioc_g_std = rvin_g_std,
>> .vidioc_s_std = rvin_s_std,
>> @@ -832,6 +867,13 @@ int rvin_v4l2_probe(struct rvin_dev *vin)
>> vin->src_pad_idx = pad_idx;
>> fmt.pad = vin->src_pad_idx;
>>
>> + vin->sink_pad_idx = 0;
>> + for (pad_idx = 0; pad_idx < sd->entity.num_pads; pad_idx++)
>> + if (sd->entity.pads[pad_idx].flags == MEDIA_PAD_FL_SINK) {
>> + vin->sink_pad_idx = pad_idx;
>> + break;
>> + }
>> +
>> /* Try to improve our guess of a reasonable window format */
>> ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &fmt);
>> if (ret) {
>> diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
>> index 793184d..af815cc 100644
>> --- a/drivers/media/platform/rcar-vin/rcar-vin.h
>> +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
>> @@ -121,6 +121,7 @@ struct rvin_dev {
>> struct video_device vdev;
>> struct v4l2_device v4l2_dev;
>> int src_pad_idx;
>> + int sink_pad_idx;
>
> sink_pad_idx should be documented in the comment above the struct. If
> you fix that feel free to add my:
>
> Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
>
>> struct v4l2_ctrl_handler ctrl_handler;
>> struct v4l2_async_notifier notifier;
>> struct rvin_graph_entity digital;
>> --
>> 2.9.3
>>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v8 2/2] rcar-vin: implement EDID control ioctls
@ 2016-09-15 14:02 ` Hans Verkuil
0 siblings, 0 replies; 18+ messages in thread
From: Hans Verkuil @ 2016-09-15 14:02 UTC (permalink / raw)
To: Niklas Söderlund, Ulrich Hecht
Cc: hans.verkuil, linux-media, linux-renesas-soc, magnus.damm,
laurent.pinchart, william.towle
Hi Ulrich,
A patch series on top of my tree with Niklas' patches already merged:
https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=rcar
would be ideal. Since otherwise I have to resolve a conflict.
Regards,
Hans
On 09/15/2016 03:59 PM, Niklas S�derlund wrote:
> Hi Ulrich,
>
> Thanks for your patch.
>
> Can you append another patch ontop of this one whit the following
> change:
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> index ac26738..7ba728d 100644
> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> @@ -494,7 +494,7 @@ static int rvin_enum_dv_timings(struct file *file, void *priv_fh,
> int pad, ret;
>
> pad = timings->pad;
> - timings->pad = vin->src_pad_idx;
> + timings->pad = vin->sink_pad_idx;
>
> ret = v4l2_subdev_call(sd, pad, enum_dv_timings, timings);
>
> @@ -548,7 +548,7 @@ static int rvin_dv_timings_cap(struct file *file, void *priv_fh,
> int pad, ret;
>
> pad = cap->pad;
> - cap->pad = vin->src_pad_idx;
> + cap->pad = vin->sink_pad_idx;
>
> ret = v4l2_subdev_call(sd, pad, dv_timings_cap, cap);
>
>
> Whit that change the driver pass v4l2-compliance check for the HDMI
> input on Koelsch for me.
>
> On 2016-09-15 15:24:08 +0200, Ulrich Hecht wrote:
>> Adds G_EDID and S_EDID.
>>
>> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
>> ---
>> drivers/media/platform/rcar-vin/rcar-v4l2.c | 42 +++++++++++++++++++++++++++++
>> drivers/media/platform/rcar-vin/rcar-vin.h | 1 +
>> 2 files changed, 43 insertions(+)
>>
>> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
>> index 62ca7e3..f679182 100644
>> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
>> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
>> @@ -557,6 +557,38 @@ static int rvin_dv_timings_cap(struct file *file, void *priv_fh,
>> return ret;
>> }
>>
>> +static int rvin_g_edid(struct file *file, void *fh, struct v4l2_edid *edid)
>> +{
>> + struct rvin_dev *vin = video_drvdata(file);
>> + struct v4l2_subdev *sd = vin_to_source(vin);
>> + int input, ret;
>> +
>> + input = edid->pad;
>> + edid->pad = vin->sink_pad_idx;
>> +
>> + ret = v4l2_subdev_call(sd, pad, get_edid, edid);
>> +
>> + edid->pad = input;
>> +
>> + return ret;
>> +}
>> +
>> +static int rvin_s_edid(struct file *file, void *fh, struct v4l2_edid *edid)
>> +{
>> + struct rvin_dev *vin = video_drvdata(file);
>> + struct v4l2_subdev *sd = vin_to_source(vin);
>> + int input, ret;
>> +
>> + input = edid->pad;
>> + edid->pad = vin->sink_pad_idx;
>> +
>> + ret = v4l2_subdev_call(sd, pad, set_edid, edid);
>> +
>> + edid->pad = input;
>> +
>> + return ret;
>> +}
>> +
>> static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
>> .vidioc_querycap = rvin_querycap,
>> .vidioc_try_fmt_vid_cap = rvin_try_fmt_vid_cap,
>> @@ -579,6 +611,9 @@ static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
>> .vidioc_s_dv_timings = rvin_s_dv_timings,
>> .vidioc_query_dv_timings = rvin_query_dv_timings,
>>
>> + .vidioc_g_edid = rvin_g_edid,
>> + .vidioc_s_edid = rvin_s_edid,
>> +
>> .vidioc_querystd = rvin_querystd,
>> .vidioc_g_std = rvin_g_std,
>> .vidioc_s_std = rvin_s_std,
>> @@ -832,6 +867,13 @@ int rvin_v4l2_probe(struct rvin_dev *vin)
>> vin->src_pad_idx = pad_idx;
>> fmt.pad = vin->src_pad_idx;
>>
>> + vin->sink_pad_idx = 0;
>> + for (pad_idx = 0; pad_idx < sd->entity.num_pads; pad_idx++)
>> + if (sd->entity.pads[pad_idx].flags == MEDIA_PAD_FL_SINK) {
>> + vin->sink_pad_idx = pad_idx;
>> + break;
>> + }
>> +
>> /* Try to improve our guess of a reasonable window format */
>> ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &fmt);
>> if (ret) {
>> diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
>> index 793184d..af815cc 100644
>> --- a/drivers/media/platform/rcar-vin/rcar-vin.h
>> +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
>> @@ -121,6 +121,7 @@ struct rvin_dev {
>> struct video_device vdev;
>> struct v4l2_device v4l2_dev;
>> int src_pad_idx;
>> + int sink_pad_idx;
>
> sink_pad_idx should be documented in the comment above the struct. If
> you fix that feel free to add my:
>
> Acked-by: Niklas S�derlund <niklas.soderlund+renesas@ragnatech.se>
>
>> struct v4l2_ctrl_handler ctrl_handler;
>> struct v4l2_async_notifier notifier;
>> struct rvin_graph_entity digital;
>> --
>> 2.9.3
>>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v8 2/2] rcar-vin: implement EDID control ioctls
2016-09-15 14:02 ` Hans Verkuil
@ 2016-09-15 15:22 ` Hans Verkuil
-1 siblings, 0 replies; 18+ messages in thread
From: Hans Verkuil @ 2016-09-15 15:22 UTC (permalink / raw)
To: Niklas Söderlund, Ulrich Hecht
Cc: hans.verkuil, linux-media, linux-renesas-soc, magnus.damm,
laurent.pinchart, william.towle
I've added patch v8 1/2 here as well. All I need is a correct patch 2/2 on top of
this. If you can do this tomorrow morning, then it will get into 4.9.
Hans
On 09/15/2016 04:02 PM, Hans Verkuil wrote:
> Hi Ulrich,
>
> A patch series on top of my tree with Niklas' patches already merged:
>
> https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=rcar
>
> would be ideal. Since otherwise I have to resolve a conflict.
>
> Regards,
>
> Hans
>
> On 09/15/2016 03:59 PM, Niklas Söderlund wrote:
>> Hi Ulrich,
>>
>> Thanks for your patch.
>>
>> Can you append another patch ontop of this one whit the following
>> change:
>>
>> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
>> index ac26738..7ba728d 100644
>> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
>> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
>> @@ -494,7 +494,7 @@ static int rvin_enum_dv_timings(struct file *file, void *priv_fh,
>> int pad, ret;
>>
>> pad = timings->pad;
>> - timings->pad = vin->src_pad_idx;
>> + timings->pad = vin->sink_pad_idx;
>>
>> ret = v4l2_subdev_call(sd, pad, enum_dv_timings, timings);
>>
>> @@ -548,7 +548,7 @@ static int rvin_dv_timings_cap(struct file *file, void *priv_fh,
>> int pad, ret;
>>
>> pad = cap->pad;
>> - cap->pad = vin->src_pad_idx;
>> + cap->pad = vin->sink_pad_idx;
>>
>> ret = v4l2_subdev_call(sd, pad, dv_timings_cap, cap);
>>
>>
>> Whit that change the driver pass v4l2-compliance check for the HDMI
>> input on Koelsch for me.
>>
>> On 2016-09-15 15:24:08 +0200, Ulrich Hecht wrote:
>>> Adds G_EDID and S_EDID.
>>>
>>> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
>>> ---
>>> drivers/media/platform/rcar-vin/rcar-v4l2.c | 42 +++++++++++++++++++++++++++++
>>> drivers/media/platform/rcar-vin/rcar-vin.h | 1 +
>>> 2 files changed, 43 insertions(+)
>>>
>>> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
>>> index 62ca7e3..f679182 100644
>>> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
>>> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
>>> @@ -557,6 +557,38 @@ static int rvin_dv_timings_cap(struct file *file, void *priv_fh,
>>> return ret;
>>> }
>>>
>>> +static int rvin_g_edid(struct file *file, void *fh, struct v4l2_edid *edid)
>>> +{
>>> + struct rvin_dev *vin = video_drvdata(file);
>>> + struct v4l2_subdev *sd = vin_to_source(vin);
>>> + int input, ret;
>>> +
>>> + input = edid->pad;
>>> + edid->pad = vin->sink_pad_idx;
>>> +
>>> + ret = v4l2_subdev_call(sd, pad, get_edid, edid);
>>> +
>>> + edid->pad = input;
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static int rvin_s_edid(struct file *file, void *fh, struct v4l2_edid *edid)
>>> +{
>>> + struct rvin_dev *vin = video_drvdata(file);
>>> + struct v4l2_subdev *sd = vin_to_source(vin);
>>> + int input, ret;
>>> +
>>> + input = edid->pad;
>>> + edid->pad = vin->sink_pad_idx;
>>> +
>>> + ret = v4l2_subdev_call(sd, pad, set_edid, edid);
>>> +
>>> + edid->pad = input;
>>> +
>>> + return ret;
>>> +}
>>> +
>>> static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
>>> .vidioc_querycap = rvin_querycap,
>>> .vidioc_try_fmt_vid_cap = rvin_try_fmt_vid_cap,
>>> @@ -579,6 +611,9 @@ static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
>>> .vidioc_s_dv_timings = rvin_s_dv_timings,
>>> .vidioc_query_dv_timings = rvin_query_dv_timings,
>>>
>>> + .vidioc_g_edid = rvin_g_edid,
>>> + .vidioc_s_edid = rvin_s_edid,
>>> +
>>> .vidioc_querystd = rvin_querystd,
>>> .vidioc_g_std = rvin_g_std,
>>> .vidioc_s_std = rvin_s_std,
>>> @@ -832,6 +867,13 @@ int rvin_v4l2_probe(struct rvin_dev *vin)
>>> vin->src_pad_idx = pad_idx;
>>> fmt.pad = vin->src_pad_idx;
>>>
>>> + vin->sink_pad_idx = 0;
>>> + for (pad_idx = 0; pad_idx < sd->entity.num_pads; pad_idx++)
>>> + if (sd->entity.pads[pad_idx].flags == MEDIA_PAD_FL_SINK) {
>>> + vin->sink_pad_idx = pad_idx;
>>> + break;
>>> + }
>>> +
>>> /* Try to improve our guess of a reasonable window format */
>>> ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &fmt);
>>> if (ret) {
>>> diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
>>> index 793184d..af815cc 100644
>>> --- a/drivers/media/platform/rcar-vin/rcar-vin.h
>>> +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
>>> @@ -121,6 +121,7 @@ struct rvin_dev {
>>> struct video_device vdev;
>>> struct v4l2_device v4l2_dev;
>>> int src_pad_idx;
>>> + int sink_pad_idx;
>>
>> sink_pad_idx should be documented in the comment above the struct. If
>> you fix that feel free to add my:
>>
>> Acked-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
>>
>>> struct v4l2_ctrl_handler ctrl_handler;
>>> struct v4l2_async_notifier notifier;
>>> struct rvin_graph_entity digital;
>>> --
>>> 2.9.3
>>>
>>
> --
> 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] 18+ messages in thread
* Re: [PATCH v8 2/2] rcar-vin: implement EDID control ioctls
@ 2016-09-15 15:22 ` Hans Verkuil
0 siblings, 0 replies; 18+ messages in thread
From: Hans Verkuil @ 2016-09-15 15:22 UTC (permalink / raw)
To: Niklas Söderlund, Ulrich Hecht
Cc: hans.verkuil, linux-media, linux-renesas-soc, magnus.damm,
laurent.pinchart, william.towle
I've added patch v8 1/2 here as well. All I need is a correct patch 2/2 on top of
this. If you can do this tomorrow morning, then it will get into 4.9.
Hans
On 09/15/2016 04:02 PM, Hans Verkuil wrote:
> Hi Ulrich,
>
> A patch series on top of my tree with Niklas' patches already merged:
>
> https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=rcar
>
> would be ideal. Since otherwise I have to resolve a conflict.
>
> Regards,
>
> Hans
>
> On 09/15/2016 03:59 PM, Niklas S�derlund wrote:
>> Hi Ulrich,
>>
>> Thanks for your patch.
>>
>> Can you append another patch ontop of this one whit the following
>> change:
>>
>> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
>> index ac26738..7ba728d 100644
>> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
>> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
>> @@ -494,7 +494,7 @@ static int rvin_enum_dv_timings(struct file *file, void *priv_fh,
>> int pad, ret;
>>
>> pad = timings->pad;
>> - timings->pad = vin->src_pad_idx;
>> + timings->pad = vin->sink_pad_idx;
>>
>> ret = v4l2_subdev_call(sd, pad, enum_dv_timings, timings);
>>
>> @@ -548,7 +548,7 @@ static int rvin_dv_timings_cap(struct file *file, void *priv_fh,
>> int pad, ret;
>>
>> pad = cap->pad;
>> - cap->pad = vin->src_pad_idx;
>> + cap->pad = vin->sink_pad_idx;
>>
>> ret = v4l2_subdev_call(sd, pad, dv_timings_cap, cap);
>>
>>
>> Whit that change the driver pass v4l2-compliance check for the HDMI
>> input on Koelsch for me.
>>
>> On 2016-09-15 15:24:08 +0200, Ulrich Hecht wrote:
>>> Adds G_EDID and S_EDID.
>>>
>>> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
>>> ---
>>> drivers/media/platform/rcar-vin/rcar-v4l2.c | 42 +++++++++++++++++++++++++++++
>>> drivers/media/platform/rcar-vin/rcar-vin.h | 1 +
>>> 2 files changed, 43 insertions(+)
>>>
>>> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
>>> index 62ca7e3..f679182 100644
>>> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
>>> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
>>> @@ -557,6 +557,38 @@ static int rvin_dv_timings_cap(struct file *file, void *priv_fh,
>>> return ret;
>>> }
>>>
>>> +static int rvin_g_edid(struct file *file, void *fh, struct v4l2_edid *edid)
>>> +{
>>> + struct rvin_dev *vin = video_drvdata(file);
>>> + struct v4l2_subdev *sd = vin_to_source(vin);
>>> + int input, ret;
>>> +
>>> + input = edid->pad;
>>> + edid->pad = vin->sink_pad_idx;
>>> +
>>> + ret = v4l2_subdev_call(sd, pad, get_edid, edid);
>>> +
>>> + edid->pad = input;
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static int rvin_s_edid(struct file *file, void *fh, struct v4l2_edid *edid)
>>> +{
>>> + struct rvin_dev *vin = video_drvdata(file);
>>> + struct v4l2_subdev *sd = vin_to_source(vin);
>>> + int input, ret;
>>> +
>>> + input = edid->pad;
>>> + edid->pad = vin->sink_pad_idx;
>>> +
>>> + ret = v4l2_subdev_call(sd, pad, set_edid, edid);
>>> +
>>> + edid->pad = input;
>>> +
>>> + return ret;
>>> +}
>>> +
>>> static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
>>> .vidioc_querycap = rvin_querycap,
>>> .vidioc_try_fmt_vid_cap = rvin_try_fmt_vid_cap,
>>> @@ -579,6 +611,9 @@ static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
>>> .vidioc_s_dv_timings = rvin_s_dv_timings,
>>> .vidioc_query_dv_timings = rvin_query_dv_timings,
>>>
>>> + .vidioc_g_edid = rvin_g_edid,
>>> + .vidioc_s_edid = rvin_s_edid,
>>> +
>>> .vidioc_querystd = rvin_querystd,
>>> .vidioc_g_std = rvin_g_std,
>>> .vidioc_s_std = rvin_s_std,
>>> @@ -832,6 +867,13 @@ int rvin_v4l2_probe(struct rvin_dev *vin)
>>> vin->src_pad_idx = pad_idx;
>>> fmt.pad = vin->src_pad_idx;
>>>
>>> + vin->sink_pad_idx = 0;
>>> + for (pad_idx = 0; pad_idx < sd->entity.num_pads; pad_idx++)
>>> + if (sd->entity.pads[pad_idx].flags == MEDIA_PAD_FL_SINK) {
>>> + vin->sink_pad_idx = pad_idx;
>>> + break;
>>> + }
>>> +
>>> /* Try to improve our guess of a reasonable window format */
>>> ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &fmt);
>>> if (ret) {
>>> diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
>>> index 793184d..af815cc 100644
>>> --- a/drivers/media/platform/rcar-vin/rcar-vin.h
>>> +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
>>> @@ -121,6 +121,7 @@ struct rvin_dev {
>>> struct video_device vdev;
>>> struct v4l2_device v4l2_dev;
>>> int src_pad_idx;
>>> + int sink_pad_idx;
>>
>> sink_pad_idx should be documented in the comment above the struct. If
>> you fix that feel free to add my:
>>
>> Acked-by: Niklas S�derlund <niklas.soderlund+renesas@ragnatech.se>
>>
>>> struct v4l2_ctrl_handler ctrl_handler;
>>> struct v4l2_async_notifier notifier;
>>> struct rvin_graph_entity digital;
>>> --
>>> 2.9.3
>>>
>>
> --
> 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] 18+ messages in thread
* Re: [PATCH v8 1/2] media: adv7604: automatic "default-input" selection
2016-09-15 13:24 ` [PATCH v8 1/2] media: adv7604: automatic "default-input" selection Ulrich Hecht
@ 2016-09-15 16:42 ` Laurent Pinchart
2016-09-16 8:44 ` Hans Verkuil
0 siblings, 1 reply; 18+ messages in thread
From: Laurent Pinchart @ 2016-09-15 16:42 UTC (permalink / raw)
To: Ulrich Hecht
Cc: hans.verkuil, niklas.soderlund, linux-media, linux-renesas-soc,
magnus.damm, william.towle
Hi Ulrich,
Thank you for the patch.
On Thursday 15 Sep 2016 15:24:07 Ulrich Hecht wrote:
> Fall back to input 0 if "default-input" property is not present.
>
> Additionally, documentation in commit bf9c82278c34 ("[media]
> media: adv7604: ability to read default input port from DT") states
> that the "default-input" property should reside directly in the node
> for adv7612.
Actually it doesn't. The DT bindings specifies "default-input" as an endpoint
property, even though the example sets it in the device node. That's
inconsistent so the DT bindings document should be fixed. I believe the
property should be set in the device node, it doesn't make much sense to have
different default inputs per port.
> Hence, also adjust the parsing to make the implementation
> consistent with this.
>
> Based on patch by William Towle <william.towle@codethink.co.uk>.
>
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> ---
> drivers/media/i2c/adv7604.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
> index 4003831..055c9df 100644
> --- a/drivers/media/i2c/adv7604.c
> +++ b/drivers/media/i2c/adv7604.c
> @@ -3077,10 +3077,13 @@ static int adv76xx_parse_dt(struct adv76xx_state
> *state) if (!of_property_read_u32(endpoint, "default-input", &v))
> state->pdata.default_input = v;
> else
> - state->pdata.default_input = -1;
> + state->pdata.default_input = 0;
>
> of_node_put(endpoint);
>
> + if (!of_property_read_u32(np, "default-input", &v))
> + state->pdata.default_input = v;
> +
> flags = bus_cfg.bus.parallel.flags;
>
> if (flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH)
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v8 2/2] rcar-vin: implement EDID control ioctls
2016-09-15 13:24 ` [PATCH v8 2/2] rcar-vin: implement EDID control ioctls Ulrich Hecht
2016-09-15 13:59 ` Niklas Söderlund
@ 2016-09-15 16:47 ` Laurent Pinchart
2016-09-15 17:01 ` Hans Verkuil
1 sibling, 1 reply; 18+ messages in thread
From: Laurent Pinchart @ 2016-09-15 16:47 UTC (permalink / raw)
To: Ulrich Hecht
Cc: hans.verkuil, niklas.soderlund, linux-media, linux-renesas-soc,
magnus.damm, william.towle
Hi Ulrich,
Thank you for the patch.
On Thursday 15 Sep 2016 15:24:08 Ulrich Hecht wrote:
> Adds G_EDID and S_EDID.
>
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> ---
> drivers/media/platform/rcar-vin/rcar-v4l2.c | 42 ++++++++++++++++++++++++++
> drivers/media/platform/rcar-vin/rcar-vin.h | 1 +
> 2 files changed, 43 insertions(+)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> b/drivers/media/platform/rcar-vin/rcar-v4l2.c index 62ca7e3..f679182 100644
> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> @@ -557,6 +557,38 @@ static int rvin_dv_timings_cap(struct file *file, void
> *priv_fh, return ret;
> }
>
> +static int rvin_g_edid(struct file *file, void *fh, struct v4l2_edid *edid)
> +{
> + struct rvin_dev *vin = video_drvdata(file);
> + struct v4l2_subdev *sd = vin_to_source(vin);
> + int input, ret;
> +
> + input = edid->pad;
> + edid->pad = vin->sink_pad_idx;
> +
> + ret = v4l2_subdev_call(sd, pad, get_edid, edid);
> +
> + edid->pad = input;
> +
> + return ret;
> +}
> +
> +static int rvin_s_edid(struct file *file, void *fh, struct v4l2_edid *edid)
> +{
> + struct rvin_dev *vin = video_drvdata(file);
> + struct v4l2_subdev *sd = vin_to_source(vin);
> + int input, ret;
> +
> + input = edid->pad;
> + edid->pad = vin->sink_pad_idx;
> +
> + ret = v4l2_subdev_call(sd, pad, set_edid, edid);
> +
> + edid->pad = input;
> +
> + return ret;
> +}
> +
> static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
> .vidioc_querycap = rvin_querycap,
> .vidioc_try_fmt_vid_cap = rvin_try_fmt_vid_cap,
> @@ -579,6 +611,9 @@ static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
> .vidioc_s_dv_timings = rvin_s_dv_timings,
> .vidioc_query_dv_timings = rvin_query_dv_timings,
>
> + .vidioc_g_edid = rvin_g_edid,
> + .vidioc_s_edid = rvin_s_edid,
> +
> .vidioc_querystd = rvin_querystd,
> .vidioc_g_std = rvin_g_std,
> .vidioc_s_std = rvin_s_std,
> @@ -832,6 +867,13 @@ int rvin_v4l2_probe(struct rvin_dev *vin)
> vin->src_pad_idx = pad_idx;
> fmt.pad = vin->src_pad_idx;
>
> + vin->sink_pad_idx = 0;
> + for (pad_idx = 0; pad_idx < sd->entity.num_pads; pad_idx++)
> + if (sd->entity.pads[pad_idx].flags == MEDIA_PAD_FL_SINK) {
> + vin->sink_pad_idx = pad_idx;
> + break;
> + }
> +
What if the subdev has multiple sink pads ? Shouldn't the pad number be
instead computed in the get and set EDID handlers based on the input number
passed in the struct v4l2_edid::pad field ?
> /* Try to improve our guess of a reasonable window format */
> ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &fmt);
> if (ret) {
> diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h
> b/drivers/media/platform/rcar-vin/rcar-vin.h index 793184d..af815cc 100644
> --- a/drivers/media/platform/rcar-vin/rcar-vin.h
> +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
> @@ -121,6 +121,7 @@ struct rvin_dev {
> struct video_device vdev;
> struct v4l2_device v4l2_dev;
> int src_pad_idx;
> + int sink_pad_idx;
> struct v4l2_ctrl_handler ctrl_handler;
> struct v4l2_async_notifier notifier;
> struct rvin_graph_entity digital;
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v8 2/2] rcar-vin: implement EDID control ioctls
2016-09-15 16:47 ` Laurent Pinchart
@ 2016-09-15 17:01 ` Hans Verkuil
2016-09-15 17:28 ` Niklas Söderlund
2016-09-15 17:34 ` Laurent Pinchart
0 siblings, 2 replies; 18+ messages in thread
From: Hans Verkuil @ 2016-09-15 17:01 UTC (permalink / raw)
To: Laurent Pinchart, Ulrich Hecht
Cc: hans.verkuil, niklas.soderlund, linux-media, linux-renesas-soc,
magnus.damm, william.towle
On 09/15/2016 06:47 PM, Laurent Pinchart wrote:
> Hi Ulrich,
>
> Thank you for the patch.
>
> On Thursday 15 Sep 2016 15:24:08 Ulrich Hecht wrote:
>> Adds G_EDID and S_EDID.
>>
>> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
>> ---
>> drivers/media/platform/rcar-vin/rcar-v4l2.c | 42 ++++++++++++++++++++++++++
>> drivers/media/platform/rcar-vin/rcar-vin.h | 1 +
>> 2 files changed, 43 insertions(+)
>>
>> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
>> b/drivers/media/platform/rcar-vin/rcar-v4l2.c index 62ca7e3..f679182 100644
>> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
>> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
>> @@ -557,6 +557,38 @@ static int rvin_dv_timings_cap(struct file *file, void
>> *priv_fh, return ret;
>> }
>>
>> +static int rvin_g_edid(struct file *file, void *fh, struct v4l2_edid *edid)
>> +{
>> + struct rvin_dev *vin = video_drvdata(file);
>> + struct v4l2_subdev *sd = vin_to_source(vin);
>> + int input, ret;
>> +
>> + input = edid->pad;
>> + edid->pad = vin->sink_pad_idx;
>> +
>> + ret = v4l2_subdev_call(sd, pad, get_edid, edid);
>> +
>> + edid->pad = input;
>> +
>> + return ret;
>> +}
>> +
>> +static int rvin_s_edid(struct file *file, void *fh, struct v4l2_edid *edid)
>> +{
>> + struct rvin_dev *vin = video_drvdata(file);
>> + struct v4l2_subdev *sd = vin_to_source(vin);
>> + int input, ret;
>> +
>> + input = edid->pad;
>> + edid->pad = vin->sink_pad_idx;
>> +
>> + ret = v4l2_subdev_call(sd, pad, set_edid, edid);
>> +
>> + edid->pad = input;
>> +
>> + return ret;
>> +}
>> +
>> static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
>> .vidioc_querycap = rvin_querycap,
>> .vidioc_try_fmt_vid_cap = rvin_try_fmt_vid_cap,
>> @@ -579,6 +611,9 @@ static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
>> .vidioc_s_dv_timings = rvin_s_dv_timings,
>> .vidioc_query_dv_timings = rvin_query_dv_timings,
>>
>> + .vidioc_g_edid = rvin_g_edid,
>> + .vidioc_s_edid = rvin_s_edid,
>> +
>> .vidioc_querystd = rvin_querystd,
>> .vidioc_g_std = rvin_g_std,
>> .vidioc_s_std = rvin_s_std,
>> @@ -832,6 +867,13 @@ int rvin_v4l2_probe(struct rvin_dev *vin)
>> vin->src_pad_idx = pad_idx;
>> fmt.pad = vin->src_pad_idx;
>>
>> + vin->sink_pad_idx = 0;
>> + for (pad_idx = 0; pad_idx < sd->entity.num_pads; pad_idx++)
>> + if (sd->entity.pads[pad_idx].flags == MEDIA_PAD_FL_SINK) {
>> + vin->sink_pad_idx = pad_idx;
>> + break;
>> + }
>> +
>
> What if the subdev has multiple sink pads ? Shouldn't the pad number be
> instead computed in the get and set EDID handlers based on the input number
> passed in the struct v4l2_edid::pad field ?
But there is only one input (VIDIOC_ENUM_INPUT), so this would not make sense.
What is wrong is that g/s_edid should check the pad and return -EINVAL if it
is non-zero. Odd that I missed that in the earlier reviews...
Regards,
Hans
>
>> /* Try to improve our guess of a reasonable window format */
>> ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &fmt);
>> if (ret) {
>> diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h
>> b/drivers/media/platform/rcar-vin/rcar-vin.h index 793184d..af815cc 100644
>> --- a/drivers/media/platform/rcar-vin/rcar-vin.h
>> +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
>> @@ -121,6 +121,7 @@ struct rvin_dev {
>> struct video_device vdev;
>> struct v4l2_device v4l2_dev;
>> int src_pad_idx;
>> + int sink_pad_idx;
>> struct v4l2_ctrl_handler ctrl_handler;
>> struct v4l2_async_notifier notifier;
>> struct rvin_graph_entity digital;
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v8 2/2] rcar-vin: implement EDID control ioctls
2016-09-15 17:01 ` Hans Verkuil
@ 2016-09-15 17:28 ` Niklas Söderlund
2016-09-15 17:34 ` Laurent Pinchart
1 sibling, 0 replies; 18+ messages in thread
From: Niklas Söderlund @ 2016-09-15 17:28 UTC (permalink / raw)
To: Hans Verkuil
Cc: Laurent Pinchart, Ulrich Hecht, hans.verkuil, linux-media,
linux-renesas-soc, magnus.damm, william.towle
On 2016-09-15 19:01:06 +0200, Hans Verkuil wrote:
>
>
> On 09/15/2016 06:47 PM, Laurent Pinchart wrote:
> > Hi Ulrich,
> >
> > Thank you for the patch.
> >
> > On Thursday 15 Sep 2016 15:24:08 Ulrich Hecht wrote:
> >> Adds G_EDID and S_EDID.
> >>
> >> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> >> ---
> >> drivers/media/platform/rcar-vin/rcar-v4l2.c | 42 ++++++++++++++++++++++++++
> >> drivers/media/platform/rcar-vin/rcar-vin.h | 1 +
> >> 2 files changed, 43 insertions(+)
> >>
> >> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> >> b/drivers/media/platform/rcar-vin/rcar-v4l2.c index 62ca7e3..f679182 100644
> >> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> >> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> >> @@ -557,6 +557,38 @@ static int rvin_dv_timings_cap(struct file *file, void
> >> *priv_fh, return ret;
> >> }
> >>
> >> +static int rvin_g_edid(struct file *file, void *fh, struct v4l2_edid *edid)
> >> +{
> >> + struct rvin_dev *vin = video_drvdata(file);
> >> + struct v4l2_subdev *sd = vin_to_source(vin);
> >> + int input, ret;
> >> +
> >> + input = edid->pad;
> >> + edid->pad = vin->sink_pad_idx;
> >> +
> >> + ret = v4l2_subdev_call(sd, pad, get_edid, edid);
> >> +
> >> + edid->pad = input;
> >> +
> >> + return ret;
> >> +}
> >> +
> >> +static int rvin_s_edid(struct file *file, void *fh, struct v4l2_edid *edid)
> >> +{
> >> + struct rvin_dev *vin = video_drvdata(file);
> >> + struct v4l2_subdev *sd = vin_to_source(vin);
> >> + int input, ret;
> >> +
> >> + input = edid->pad;
> >> + edid->pad = vin->sink_pad_idx;
> >> +
> >> + ret = v4l2_subdev_call(sd, pad, set_edid, edid);
> >> +
> >> + edid->pad = input;
> >> +
> >> + return ret;
> >> +}
> >> +
> >> static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
> >> .vidioc_querycap = rvin_querycap,
> >> .vidioc_try_fmt_vid_cap = rvin_try_fmt_vid_cap,
> >> @@ -579,6 +611,9 @@ static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
> >> .vidioc_s_dv_timings = rvin_s_dv_timings,
> >> .vidioc_query_dv_timings = rvin_query_dv_timings,
> >>
> >> + .vidioc_g_edid = rvin_g_edid,
> >> + .vidioc_s_edid = rvin_s_edid,
> >> +
> >> .vidioc_querystd = rvin_querystd,
> >> .vidioc_g_std = rvin_g_std,
> >> .vidioc_s_std = rvin_s_std,
> >> @@ -832,6 +867,13 @@ int rvin_v4l2_probe(struct rvin_dev *vin)
> >> vin->src_pad_idx = pad_idx;
> >> fmt.pad = vin->src_pad_idx;
> >>
> >> + vin->sink_pad_idx = 0;
> >> + for (pad_idx = 0; pad_idx < sd->entity.num_pads; pad_idx++)
> >> + if (sd->entity.pads[pad_idx].flags == MEDIA_PAD_FL_SINK) {
> >> + vin->sink_pad_idx = pad_idx;
> >> + break;
> >> + }
> >> +
> >
> > What if the subdev has multiple sink pads ? Shouldn't the pad number be
> > instead computed in the get and set EDID handlers based on the input number
> > passed in the struct v4l2_edid::pad field ?
>
> But there is only one input (VIDIOC_ENUM_INPUT), so this would not make sense.
>
> What is wrong is that g/s_edid should check the pad and return -EINVAL if it
> is non-zero. Odd that I missed that in the earlier reviews...
Both Hans and Laurents comments are correct in this case I think.
The original patches was based on top of the Gen3 work where just as
Laurent states the input number passed in the v4l2_edid::pad needs to be
translated to a sink pad number of the subdevice. But since this is for
Gen2 only there are only one input so no mapping is needed. All that is
required is as Hans state to check that v4l2_edid::pad is a valid input
number (equal to zero) from the rcar-vin perspective.
I do however still think there are value to find the subdevice sink pad
id in rvin_v4l2_probe() and then use it in EDID handlers. The driver
still need to use a sink pad number which is correct from the subdevice
point of view.
>
> Regards,
>
> Hans
>
> >
> >> /* Try to improve our guess of a reasonable window format */
> >> ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &fmt);
> >> if (ret) {
> >> diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h
> >> b/drivers/media/platform/rcar-vin/rcar-vin.h index 793184d..af815cc 100644
> >> --- a/drivers/media/platform/rcar-vin/rcar-vin.h
> >> +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
> >> @@ -121,6 +121,7 @@ struct rvin_dev {
> >> struct video_device vdev;
> >> struct v4l2_device v4l2_dev;
> >> int src_pad_idx;
> >> + int sink_pad_idx;
> >> struct v4l2_ctrl_handler ctrl_handler;
> >> struct v4l2_async_notifier notifier;
> >> struct rvin_graph_entity digital;
> >
--
Regards,
Niklas Söderlund
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v8 2/2] rcar-vin: implement EDID control ioctls
@ 2016-09-15 17:28 ` Niklas Söderlund
0 siblings, 0 replies; 18+ messages in thread
From: Niklas Söderlund @ 2016-09-15 17:28 UTC (permalink / raw)
To: Hans Verkuil
Cc: Laurent Pinchart, Ulrich Hecht, hans.verkuil, linux-media,
linux-renesas-soc, magnus.damm, william.towle
On 2016-09-15 19:01:06 +0200, Hans Verkuil wrote:
>
>
> On 09/15/2016 06:47 PM, Laurent Pinchart wrote:
> > Hi Ulrich,
> >
> > Thank you for the patch.
> >
> > On Thursday 15 Sep 2016 15:24:08 Ulrich Hecht wrote:
> >> Adds G_EDID and S_EDID.
> >>
> >> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> >> ---
> >> drivers/media/platform/rcar-vin/rcar-v4l2.c | 42 ++++++++++++++++++++++++++
> >> drivers/media/platform/rcar-vin/rcar-vin.h | 1 +
> >> 2 files changed, 43 insertions(+)
> >>
> >> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> >> b/drivers/media/platform/rcar-vin/rcar-v4l2.c index 62ca7e3..f679182 100644
> >> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> >> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> >> @@ -557,6 +557,38 @@ static int rvin_dv_timings_cap(struct file *file, void
> >> *priv_fh, return ret;
> >> }
> >>
> >> +static int rvin_g_edid(struct file *file, void *fh, struct v4l2_edid *edid)
> >> +{
> >> + struct rvin_dev *vin = video_drvdata(file);
> >> + struct v4l2_subdev *sd = vin_to_source(vin);
> >> + int input, ret;
> >> +
> >> + input = edid->pad;
> >> + edid->pad = vin->sink_pad_idx;
> >> +
> >> + ret = v4l2_subdev_call(sd, pad, get_edid, edid);
> >> +
> >> + edid->pad = input;
> >> +
> >> + return ret;
> >> +}
> >> +
> >> +static int rvin_s_edid(struct file *file, void *fh, struct v4l2_edid *edid)
> >> +{
> >> + struct rvin_dev *vin = video_drvdata(file);
> >> + struct v4l2_subdev *sd = vin_to_source(vin);
> >> + int input, ret;
> >> +
> >> + input = edid->pad;
> >> + edid->pad = vin->sink_pad_idx;
> >> +
> >> + ret = v4l2_subdev_call(sd, pad, set_edid, edid);
> >> +
> >> + edid->pad = input;
> >> +
> >> + return ret;
> >> +}
> >> +
> >> static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
> >> .vidioc_querycap = rvin_querycap,
> >> .vidioc_try_fmt_vid_cap = rvin_try_fmt_vid_cap,
> >> @@ -579,6 +611,9 @@ static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
> >> .vidioc_s_dv_timings = rvin_s_dv_timings,
> >> .vidioc_query_dv_timings = rvin_query_dv_timings,
> >>
> >> + .vidioc_g_edid = rvin_g_edid,
> >> + .vidioc_s_edid = rvin_s_edid,
> >> +
> >> .vidioc_querystd = rvin_querystd,
> >> .vidioc_g_std = rvin_g_std,
> >> .vidioc_s_std = rvin_s_std,
> >> @@ -832,6 +867,13 @@ int rvin_v4l2_probe(struct rvin_dev *vin)
> >> vin->src_pad_idx = pad_idx;
> >> fmt.pad = vin->src_pad_idx;
> >>
> >> + vin->sink_pad_idx = 0;
> >> + for (pad_idx = 0; pad_idx < sd->entity.num_pads; pad_idx++)
> >> + if (sd->entity.pads[pad_idx].flags == MEDIA_PAD_FL_SINK) {
> >> + vin->sink_pad_idx = pad_idx;
> >> + break;
> >> + }
> >> +
> >
> > What if the subdev has multiple sink pads ? Shouldn't the pad number be
> > instead computed in the get and set EDID handlers based on the input number
> > passed in the struct v4l2_edid::pad field ?
>
> But there is only one input (VIDIOC_ENUM_INPUT), so this would not make sense.
>
> What is wrong is that g/s_edid should check the pad and return -EINVAL if it
> is non-zero. Odd that I missed that in the earlier reviews...
Both Hans and Laurents comments are correct in this case I think.
The original patches was based on top of the Gen3 work where just as
Laurent states the input number passed in the v4l2_edid::pad needs to be
translated to a sink pad number of the subdevice. But since this is for
Gen2 only there are only one input so no mapping is needed. All that is
required is as Hans state to check that v4l2_edid::pad is a valid input
number (equal to zero) from the rcar-vin perspective.
I do however still think there are value to find the subdevice sink pad
id in rvin_v4l2_probe() and then use it in EDID handlers. The driver
still need to use a sink pad number which is correct from the subdevice
point of view.
>
> Regards,
>
> Hans
>
> >
> >> /* Try to improve our guess of a reasonable window format */
> >> ret = v4l2_subdev_call(sd, pad, get_fmt, NULL, &fmt);
> >> if (ret) {
> >> diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h
> >> b/drivers/media/platform/rcar-vin/rcar-vin.h index 793184d..af815cc 100644
> >> --- a/drivers/media/platform/rcar-vin/rcar-vin.h
> >> +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
> >> @@ -121,6 +121,7 @@ struct rvin_dev {
> >> struct video_device vdev;
> >> struct v4l2_device v4l2_dev;
> >> int src_pad_idx;
> >> + int sink_pad_idx;
> >> struct v4l2_ctrl_handler ctrl_handler;
> >> struct v4l2_async_notifier notifier;
> >> struct rvin_graph_entity digital;
> >
--
Regards,
Niklas S�derlund
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v8 2/2] rcar-vin: implement EDID control ioctls
2016-09-15 17:01 ` Hans Verkuil
2016-09-15 17:28 ` Niklas Söderlund
@ 2016-09-15 17:34 ` Laurent Pinchart
1 sibling, 0 replies; 18+ messages in thread
From: Laurent Pinchart @ 2016-09-15 17:34 UTC (permalink / raw)
To: Hans Verkuil
Cc: Ulrich Hecht, hans.verkuil, niklas.soderlund, linux-media,
linux-renesas-soc, magnus.damm, william.towle
Hi Hans,
On Thursday 15 Sep 2016 19:01:06 Hans Verkuil wrote:
> On 09/15/2016 06:47 PM, Laurent Pinchart wrote:
> > On Thursday 15 Sep 2016 15:24:08 Ulrich Hecht wrote:
> >> Adds G_EDID and S_EDID.
> >>
> >> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> >> ---
> >>
> >> drivers/media/platform/rcar-vin/rcar-v4l2.c | 42 +++++++++++++++++++++++
> >> drivers/media/platform/rcar-vin/rcar-vin.h | 1 +
> >> 2 files changed, 43 insertions(+)
> >>
> >> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> >> b/drivers/media/platform/rcar-vin/rcar-v4l2.c index 62ca7e3..f679182
> >> 100644
> >> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> >> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> >> @@ -557,6 +557,38 @@ static int rvin_dv_timings_cap(struct file *file,
> >> void *priv_fh, return ret;
> >> }
> >>
> >> +static int rvin_g_edid(struct file *file, void *fh, struct v4l2_edid
> >> *edid) +{
> >> + struct rvin_dev *vin = video_drvdata(file);
> >> + struct v4l2_subdev *sd = vin_to_source(vin);
> >> + int input, ret;
> >> +
> >> + input = edid->pad;
> >> + edid->pad = vin->sink_pad_idx;
> >> +
> >> + ret = v4l2_subdev_call(sd, pad, get_edid, edid);
> >> +
> >> + edid->pad = input;
> >> +
> >> + return ret;
> >> +}
> >> +
> >> +static int rvin_s_edid(struct file *file, void *fh, struct v4l2_edid
> >> *edid) +{
> >> + struct rvin_dev *vin = video_drvdata(file);
> >> + struct v4l2_subdev *sd = vin_to_source(vin);
> >> + int input, ret;
> >> +
> >> + input = edid->pad;
> >> + edid->pad = vin->sink_pad_idx;
> >> +
> >> + ret = v4l2_subdev_call(sd, pad, set_edid, edid);
> >> +
> >> + edid->pad = input;
> >> +
> >> + return ret;
> >> +}
> >> +
> >> static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
> >> .vidioc_querycap = rvin_querycap,
> >> .vidioc_try_fmt_vid_cap = rvin_try_fmt_vid_cap,
> >> @@ -579,6 +611,9 @@ static const struct v4l2_ioctl_ops rvin_ioctl_ops = {
> >> .vidioc_s_dv_timings = rvin_s_dv_timings,
> >> .vidioc_query_dv_timings = rvin_query_dv_timings,
> >>
> >> + .vidioc_g_edid = rvin_g_edid,
> >> + .vidioc_s_edid = rvin_s_edid,
> >> +
> >> .vidioc_querystd = rvin_querystd,
> >> .vidioc_g_std = rvin_g_std,
> >> .vidioc_s_std = rvin_s_std,
> >> @@ -832,6 +867,13 @@ int rvin_v4l2_probe(struct rvin_dev *vin)
> >> vin->src_pad_idx = pad_idx;
> >> fmt.pad = vin->src_pad_idx;
> >>
> >> + vin->sink_pad_idx = 0;
> >> + for (pad_idx = 0; pad_idx < sd->entity.num_pads; pad_idx++)
> >> + if (sd->entity.pads[pad_idx].flags == MEDIA_PAD_FL_SINK) {
> >> + vin->sink_pad_idx = pad_idx;
> >> + break;
> >> + }
> >> +
> >
> > What if the subdev has multiple sink pads ? Shouldn't the pad number be
> > instead computed in the get and set EDID handlers based on the input
> > number passed in the struct v4l2_edid::pad field ?
>
> But there is only one input (VIDIOC_ENUM_INPUT), so this would not make
> sense.
Indeed, my bad. We'll address that when implementing input selection support
then.
> What is wrong is that g/s_edid should check the pad and return -EINVAL if it
> is non-zero. Odd that I missed that in the earlier reviews...
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v8 1/2] media: adv7604: automatic "default-input" selection
2016-09-15 16:42 ` Laurent Pinchart
@ 2016-09-16 8:44 ` Hans Verkuil
0 siblings, 0 replies; 18+ messages in thread
From: Hans Verkuil @ 2016-09-16 8:44 UTC (permalink / raw)
To: Laurent Pinchart, Ulrich Hecht
Cc: niklas.soderlund, linux-media, linux-renesas-soc, magnus.damm,
william.towle
Hi Ulrich,
What should I do with this? I dropped it for now.
I'm just going ahead and post the pull request without this patch as I
don't want this to be a blocker.
Regards,
Hans
On 09/15/2016 06:42 PM, Laurent Pinchart wrote:
> Hi Ulrich,
>
> Thank you for the patch.
>
> On Thursday 15 Sep 2016 15:24:07 Ulrich Hecht wrote:
>> Fall back to input 0 if "default-input" property is not present.
>>
>> Additionally, documentation in commit bf9c82278c34 ("[media]
>> media: adv7604: ability to read default input port from DT") states
>> that the "default-input" property should reside directly in the node
>> for adv7612.
>
> Actually it doesn't. The DT bindings specifies "default-input" as an endpoint
> property, even though the example sets it in the device node. That's
> inconsistent so the DT bindings document should be fixed. I believe the
> property should be set in the device node, it doesn't make much sense to have
> different default inputs per port.
>
>> Hence, also adjust the parsing to make the implementation
>> consistent with this.
>>
>> Based on patch by William Towle <william.towle@codethink.co.uk>.
>>
>> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
>> ---
>> drivers/media/i2c/adv7604.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/i2c/adv7604.c b/drivers/media/i2c/adv7604.c
>> index 4003831..055c9df 100644
>> --- a/drivers/media/i2c/adv7604.c
>> +++ b/drivers/media/i2c/adv7604.c
>> @@ -3077,10 +3077,13 @@ static int adv76xx_parse_dt(struct adv76xx_state
>> *state) if (!of_property_read_u32(endpoint, "default-input", &v))
>> state->pdata.default_input = v;
>> else
>> - state->pdata.default_input = -1;
>> + state->pdata.default_input = 0;
>>
>> of_node_put(endpoint);
>>
>> + if (!of_property_read_u32(np, "default-input", &v))
>> + state->pdata.default_input = v;
>> +
>> flags = bus_cfg.bus.parallel.flags;
>>
>> if (flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH)
>
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2016-09-16 8:44 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-15 13:24 [PATCH v8 0/2] rcar-vin EDID control ioctls Ulrich Hecht
2016-09-15 13:24 ` [PATCH v8 1/2] media: adv7604: automatic "default-input" selection Ulrich Hecht
2016-09-15 16:42 ` Laurent Pinchart
2016-09-16 8:44 ` Hans Verkuil
2016-09-15 13:24 ` [PATCH v8 2/2] rcar-vin: implement EDID control ioctls Ulrich Hecht
2016-09-15 13:59 ` Niklas Söderlund
2016-09-15 13:59 ` Niklas Söderlund
2016-09-15 14:02 ` Hans Verkuil
2016-09-15 14:02 ` Hans Verkuil
2016-09-15 15:22 ` Hans Verkuil
2016-09-15 15:22 ` Hans Verkuil
2016-09-15 16:47 ` Laurent Pinchart
2016-09-15 17:01 ` Hans Verkuil
2016-09-15 17:28 ` Niklas Söderlund
2016-09-15 17:28 ` Niklas Söderlund
2016-09-15 17:34 ` Laurent Pinchart
2016-09-15 13:25 ` [PATCH v8 0/2] rcar-vin " Hans Verkuil
2016-09-15 13:38 ` Ulrich Hecht
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.