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@xxxxxxxxxxxxxxxx/
>
>>
>> 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(-)
>>
>>
>
