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+rene...@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

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to