| Message ID | 20210507074604.15079-1-louis.kuo@mediatek.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | media: some framework interface extension for new feature of Mediatek Camsys driver | expand |
Hi Louis, On 07/05/2021 09:46, Louis Kuo wrote: > Hello, > > This is the first version of the patch series extending V4L2 and media > framework to support some advanced camera function, for example, to change > the sensor when ISP is still streaming. A typical scenario is the wide-angle > sensor and telephoto sensor switching in camera application. When the user > is using the zooming UI, the application needs to switch the sensor from > wide-angle sensor to telephoto sensor smoothly. > > To finish the function, we may need to modify the links of a pipeline and > the format of pad and video device per request. Currently, the link, > pad and video device format and selection settings are not involved in > media request's design. Therefore, we try to extend the related interface > to support the request-based operations. In the early version, we added > request fd to the parameters of MEDIA_IOC_SETUP_LINK, > VIDIOC_S_FMT, VIDIOC_SUBDEV_S_SELECTION, VIDIOC_SUBDEV_S_FMT. > The driver uses media_request_get_by_fd() to retrieve the media request > and save the pending change in it, so that we can apply the pending change > in req_queue() callback then. > > Here is an example: > > int mtk_cam_vidioc_s_selection(struct file *file, void *fh, > struct v4l2_selection *s) > { > struct mtk_cam_device *cam = video_drvdata(file); > struct mtk_cam_video_device *node = file_to_mtk_cam_node(file); > struct mtk_cam_request_stream_data *stream_data; > struct mtk_cam_request *cam_req; > struct media_request *req; > s32 fd; > > fd = s->request_fd; > if (fd < 0) > return -EINVAL; > > req = media_request_get_by_fd(&cam->media_dev, fd); > > /* .... */ > > cam_req = to_mtk_cam_req(req); > stream_data = &cam_req->stream_data[node->uid.pipe_id]; > stream_data->vdev_selection_update |= (1 << node->desc.id); > stream_data->vdev_selection[node->desc.id] = *s; > > /* .... */ > > media_request_put(req); > > return 0; > } > > I posted interface change to discuss first and would like some > review comments. > > Thank you very much. Just adding a request_fd in several places is the easy bit. The much harder part is where to store that information, and even harder is an outstanding issue with the request framework: Currently the request framework is only used with decoder drivers, so there are no subdev drivers involved. I suspect that there is a fair amount of work to do to make it work well if part of the request configuration is for subdev drivers. Ideally I would like to see a proof-of-concept with the vimc driver. I think getting this right is quite a lot of work. The public API part is just a minor part of that since the public API was designed with support for this in mind. It's the internal kernel support that is lacking. If you want to pursue this (and that would be great!), then start with vimc and initially just support controls in a request. The core problem is likely to be how to keep track of the request data if it is spread out between the bridge driver and subdev drivers, and that can be tested with just supporting controls. Adding support for formats and selection rectangles is, I think, much less difficult and can be addressed later. Changing the topology in a request is a separate issue as well, and I would suggest that you postpone that. There is some low-level work going on that might make this easier in the near future (1), we'll have to wait and see. Regards, Hans (1): https://patchwork.linuxtv.org/project/linux-media/cover/20210524104408.599645-1-tomi.valkeinen@ideasonboard.com/ > > media: v4l2-core: extend the v4l2 format to support request > media: subdev: support which in v4l2_subdev_frame_interval > media: v4l2-ctrl: Add ISP Camsys user control > media: pixfmt: Add ISP Camsys formats > > drivers/media/mc/mc-device.c | 7 +- > drivers/media/v4l2-core/v4l2-ioctl.c | 153 ++++++++++++++++++++++++++- > include/media/media-entity.h | 3 + > include/uapi/linux/media.h | 3 +- > include/uapi/linux/v4l2-controls.h | 4 + > include/uapi/linux/v4l2-subdev.h | 8 +- > include/uapi/linux/videodev2.h | 109 ++++++++++++++++++- > 7 files changed, 275 insertions(+), 12 deletions(-) > >
On 14/06/2021 12:56, Hans Verkuil wrote: > Hi Louis, > > On 07/05/2021 09:46, Louis Kuo wrote: >> Hello, >> >> This is the first version of the patch series extending V4L2 and media >> framework to support some advanced camera function, for example, to change >> the sensor when ISP is still streaming. A typical scenario is the wide-angle >> sensor and telephoto sensor switching in camera application. When the user >> is using the zooming UI, the application needs to switch the sensor from >> wide-angle sensor to telephoto sensor smoothly. >> >> To finish the function, we may need to modify the links of a pipeline and >> the format of pad and video device per request. Currently, the link, >> pad and video device format and selection settings are not involved in >> media request's design. Therefore, we try to extend the related interface >> to support the request-based operations. In the early version, we added >> request fd to the parameters of MEDIA_IOC_SETUP_LINK, >> VIDIOC_S_FMT, VIDIOC_SUBDEV_S_SELECTION, VIDIOC_SUBDEV_S_FMT. >> The driver uses media_request_get_by_fd() to retrieve the media request >> and save the pending change in it, so that we can apply the pending change >> in req_queue() callback then. >> >> Here is an example: >> >> int mtk_cam_vidioc_s_selection(struct file *file, void *fh, >> struct v4l2_selection *s) >> { >> struct mtk_cam_device *cam = video_drvdata(file); >> struct mtk_cam_video_device *node = file_to_mtk_cam_node(file); >> struct mtk_cam_request_stream_data *stream_data; >> struct mtk_cam_request *cam_req; >> struct media_request *req; >> s32 fd; >> >> fd = s->request_fd; >> if (fd < 0) >> return -EINVAL; >> >> req = media_request_get_by_fd(&cam->media_dev, fd); >> >> /* .... */ >> >> cam_req = to_mtk_cam_req(req); >> stream_data = &cam_req->stream_data[node->uid.pipe_id]; >> stream_data->vdev_selection_update |= (1 << node->desc.id); >> stream_data->vdev_selection[node->desc.id] = *s; >> >> /* .... */ >> >> media_request_put(req); >> >> return 0; >> } >> >> I posted interface change to discuss first and would like some >> review comments. >> >> Thank you very much. > > Just adding a request_fd in several places is the easy bit. The much > harder part is where to store that information, and even harder is an > outstanding issue with the request framework: > > Currently the request framework is only used with decoder drivers, so > there are no subdev drivers involved. I suspect that there is a fair > amount of work to do to make it work well if part of the request configuration > is for subdev drivers. > > Ideally I would like to see a proof-of-concept with the vimc driver. > > I think getting this right is quite a lot of work. The public API part > is just a minor part of that since the public API was designed with support > for this in mind. It's the internal kernel support that is lacking. > > If you want to pursue this (and that would be great!), then start with > vimc and initially just support controls in a request. The core problem > is likely to be how to keep track of the request data if it is spread > out between the bridge driver and subdev drivers, and that can be tested > with just supporting controls. > > Adding support for formats and selection rectangles is, I think, much less > difficult and can be addressed later. Changing the topology in a request > is a separate issue as well, and I would suggest that you postpone that. > There is some low-level work going on that might make this easier in the > near future (1), we'll have to wait and see. Just FYI: I have not heard anything about this since my reply, so I am marking this series as RFC in patchwork. Regards, Hans > > Regards, > > Hans > > (1): https://patchwork.linuxtv.org/project/linux-media/cover/20210524104408.599645-1-tomi.valkeinen@ideasonboard.com/ > >> >> media: v4l2-core: extend the v4l2 format to support request >> media: subdev: support which in v4l2_subdev_frame_interval >> media: v4l2-ctrl: Add ISP Camsys user control >> media: pixfmt: Add ISP Camsys formats >> >> drivers/media/mc/mc-device.c | 7 +- >> drivers/media/v4l2-core/v4l2-ioctl.c | 153 ++++++++++++++++++++++++++- >> include/media/media-entity.h | 3 + >> include/uapi/linux/media.h | 3 +- >> include/uapi/linux/v4l2-controls.h | 4 + >> include/uapi/linux/v4l2-subdev.h | 8 +- >> include/uapi/linux/videodev2.h | 109 ++++++++++++++++++- >> 7 files changed, 275 insertions(+), 12 deletions(-) >> >> >
Hello, This is the first version of the patch series extending V4L2 and media framework to support some advanced camera function, for example, to change the sensor when ISP is still streaming. A typical scenario is the wide-angle sensor and telephoto sensor switching in camera application. When the user is using the zooming UI, the application needs to switch the sensor from wide-angle sensor to telephoto sensor smoothly. To finish the function, we may need to modify the links of a pipeline and the format of pad and video device per request. Currently, the link, pad and video device format and selection settings are not involved in media request's design. Therefore, we try to extend the related interface to support the request-based operations. In the early version, we added request fd to the parameters of MEDIA_IOC_SETUP_LINK, VIDIOC_S_FMT, VIDIOC_SUBDEV_S_SELECTION, VIDIOC_SUBDEV_S_FMT. The driver uses media_request_get_by_fd() to retrieve the media request and save the pending change in it, so that we can apply the pending change in req_queue() callback then. Here is an example: int mtk_cam_vidioc_s_selection(struct file *file, void *fh, struct v4l2_selection *s) { struct mtk_cam_device *cam = video_drvdata(file); struct mtk_cam_video_device *node = file_to_mtk_cam_node(file); struct mtk_cam_request_stream_data *stream_data; struct mtk_cam_request *cam_req; struct media_request *req; s32 fd; fd = s->request_fd; if (fd < 0) return -EINVAL; req = media_request_get_by_fd(&cam->media_dev, fd); /* .... */ cam_req = to_mtk_cam_req(req); stream_data = &cam_req->stream_data[node->uid.pipe_id]; stream_data->vdev_selection_update |= (1 << node->desc.id); stream_data->vdev_selection[node->desc.id] = *s; /* .... */ media_request_put(req); return 0; } I posted interface change to discuss first and would like some review comments. Thank you very much. media: v4l2-core: extend the v4l2 format to support request media: subdev: support which in v4l2_subdev_frame_interval media: v4l2-ctrl: Add ISP Camsys user control media: pixfmt: Add ISP Camsys formats drivers/media/mc/mc-device.c | 7 +- drivers/media/v4l2-core/v4l2-ioctl.c | 153 ++++++++++++++++++++++++++- include/media/media-entity.h | 3 + include/uapi/linux/media.h | 3 +- include/uapi/linux/v4l2-controls.h | 4 + include/uapi/linux/v4l2-subdev.h | 8 +- include/uapi/linux/videodev2.h | 109 ++++++++++++++++++- 7 files changed, 275 insertions(+), 12 deletions(-)