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@xxxxxxxxx>
>> ---
>> 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;
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
