On 12/09/18 20:57, Steve Longerbeam wrote: > From: Steve Longerbeam <slongerbeam@xxxxxxxxx> > > Forward events from a sub-device to its list of reachable video > devices. > > Note this will queue the event to a video device even if there is > no actual _enabled_ media path from the sub-device to the video device. > So a future improvement is to skip the video device if there is no enabled > path to it from the sub-device. The entity->pipe pointer can't be > used for this check because in imx-media a sub-device can be a > member to more than one streaming pipeline at a time. You explain what you are doing, but I am missing an explanation of *why* this is needed. > > Signed-off-by: Steve Longerbeam <slongerbeam@xxxxxxxxx> > --- > drivers/staging/media/imx/imx-media-capture.c | 18 ++++++++++++++ > drivers/staging/media/imx/imx-media-dev.c | 24 +++++++++++++++++++ > 2 files changed, 42 insertions(+) > > diff --git a/drivers/staging/media/imx/imx-media-capture.c b/drivers/staging/media/imx/imx-media-capture.c > index b37e1186eb2f..4dfbe05d203e 100644 > --- a/drivers/staging/media/imx/imx-media-capture.c > +++ b/drivers/staging/media/imx/imx-media-capture.c > @@ -335,6 +335,21 @@ static int capture_s_parm(struct file *file, void *fh, > return 0; > } > > +static int capture_subscribe_event(struct v4l2_fh *fh, > + const struct v4l2_event_subscription *sub) > +{ > + switch (sub->type) { > + case V4L2_EVENT_IMX_FRAME_INTERVAL_ERROR: > + return v4l2_event_subscribe(fh, sub, 0, NULL); > + case V4L2_EVENT_SOURCE_CHANGE: > + return v4l2_src_change_event_subscribe(fh, sub); > + case V4L2_EVENT_CTRL: > + return v4l2_ctrl_subscribe_event(fh, sub); > + default: > + return -EINVAL; > + } > +} > + > static const struct v4l2_ioctl_ops capture_ioctl_ops = { > .vidioc_querycap = vidioc_querycap, > > @@ -362,6 +377,9 @@ static const struct v4l2_ioctl_ops capture_ioctl_ops = { > .vidioc_expbuf = vb2_ioctl_expbuf, > .vidioc_streamon = vb2_ioctl_streamon, > .vidioc_streamoff = vb2_ioctl_streamoff, > + > + .vidioc_subscribe_event = capture_subscribe_event, > + .vidioc_unsubscribe_event = v4l2_event_unsubscribe, > }; This part of the patch adds event support to the capture device, can't this be split up into a separate patch? It seems to be useful in its own right. > > /* > diff --git a/drivers/staging/media/imx/imx-media-dev.c b/drivers/staging/media/imx/imx-media-dev.c > index 4b344a4a3706..25e916562c66 100644 > --- a/drivers/staging/media/imx/imx-media-dev.c > +++ b/drivers/staging/media/imx/imx-media-dev.c > @@ -442,6 +442,29 @@ static const struct media_device_ops imx_media_md_ops = { > .link_notify = imx_media_link_notify, > }; > > +static void imx_media_notify(struct v4l2_subdev *sd, > + unsigned int notification, > + void *arg) > +{ > + struct media_entity *entity = &sd->entity; > + int i; > + > + if (notification != V4L2_DEVICE_NOTIFY_EVENT) > + return; > + > + for (i = 0; i < entity->num_pads; i++) { > + struct media_pad *pad = &entity->pads[i]; > + struct imx_media_pad_vdev *pad_vdev; > + struct list_head *pad_vdev_list; > + > + pad_vdev_list = to_pad_vdev_list(sd, pad->index); > + if (!pad_vdev_list) > + continue; > + list_for_each_entry(pad_vdev, pad_vdev_list, list) > + v4l2_event_queue(pad_vdev->vdev->vfd, arg); Which events do you want to forward? E.g. forwarding control events doesn't seem right, but other events may be useful. Are those events also appearing on the v4l-subdevX device? And if so, should they? > + } > +} > + > static int imx_media_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > @@ -462,6 +485,7 @@ static int imx_media_probe(struct platform_device *pdev) > mutex_init(&imxmd->mutex); > > imxmd->v4l2_dev.mdev = &imxmd->md; > + imxmd->v4l2_dev.notify = imx_media_notify; > strscpy(imxmd->v4l2_dev.name, "imx-media", > sizeof(imxmd->v4l2_dev.name)); > > Regards, Hans