virtio video driver
Subsystems (2)
Hello,
My apologies for the long delay. The driver code is now available and provided as a follow-up to the discussion from this thread [1].
The reference Linux kernel 5.4 driver implementation is located here:
https://github.com/OpenSynergy/linux/tree/virtio-video-draft-v1
The driver is implemented using the V4L2 API. It allocates a v4l2 device for each probed virtio device and then creates a video device for each function within the respective virtio device. The driver implements the stateful decoder interface [2] and the stateful encoder interface (WIP) [3].
The DMA SG memory allocator tries to map buffers right away. As it is not always suitable, and some implementations might need just a physical address, we had to introduce a set of simple dma ops directly in the driver.
The driver is in the RFC state and currently a bit ahead of the spec that was proposed in the discussion mentioned above. On the other hand, the driver unfortunately does not yet include changes proposed in the recent comments [4]. The driver currently supports encoder and decoder functions. Also, it does not fully pass the v4l2-compliance yet, it has been a bit out of the focus so far.
Any feedback and contribution would be greatly appreciated.
[1] https://markmail.org/message/gc6h25acct22niut
[2] https://www.kernel.org/doc/html/v5.4/media/uapi/v4l/dev-decoder.html
[3] https://hverkuil.home.xs4all.nl/codec-api/uapi/v4l/dev-encoder.html
[4] https://markmail.org/message/yy67elx2adbivdsp
Best regards,
Dmitry.
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
From: Kiran Pawar <[email protected]>
This adds a Virtio based video driver for video streaming device that operates input and/or output data buffers to share video devices with several guests. The current implementation consist of V4L2 based video driver supporting video functions of decoder and encoder. This can be extended for other functions e.g. processor, capture and output. The device uses descriptor structures to advertise and negotiate stream formats and controls. This allows the driver to modify the processing logic of the device on a per stream basis.
--- drivers/media/Kconfig | 1 + drivers/media/Makefile | 2 + drivers/media/virtio/Kconfig | 11 + drivers/media/virtio/Makefile | 12 + drivers/media/virtio/virtio_video.h | 372 +++++++ drivers/media/virtio/virtio_video_caps.c | 618 +++++++++++ drivers/media/virtio/virtio_video_dec.c | 947 +++++++++++++++++ drivers/media/virtio/virtio_video_dec.h | 30 + drivers/media/virtio/virtio_video_device.c | 747 +++++++++++++ drivers/media/virtio/virtio_video_driver.c | 278 +++++ drivers/media/virtio/virtio_video_enc.c | 1124 ++++++++++++++++++++ drivers/media/virtio/virtio_video_enc.h | 30 + drivers/media/virtio/virtio_video_vq.c | 950 +++++++++++++++++ include/uapi/linux/virtio_ids.h | 2 + include/uapi/linux/virtio_video.h | 456 ++++++++ 15 files changed, 5580 insertions(+) create mode 100644 drivers/media/virtio/Kconfig create mode 100644 drivers/media/virtio/Makefile create mode 100644 drivers/media/virtio/virtio_video.h create mode 100644 drivers/media/virtio/virtio_video_caps.c create mode 100644 drivers/media/virtio/virtio_video_dec.c create mode 100644 drivers/media/virtio/virtio_video_dec.h create mode 100644 drivers/media/virtio/virtio_video_device.c create mode 100644 drivers/media/virtio/virtio_video_driver.c create mode 100644 drivers/media/virtio/virtio_video_enc.c create mode 100644 drivers/media/virtio/virtio_video_enc.h create mode 100644 drivers/media/virtio/virtio_video_vq.c create mode 100644 include/uapi/linux/virtio_video.h
drivers/media/Kconfig +1 -0
@@ -170,6 +170,7 @@ source "drivers/media/pci/Kconfig" source "drivers/media/platform/Kconfig" source "drivers/media/mmc/Kconfig" source "drivers/media/radio/Kconfig" +source "drivers/media/virtio/Kconfig" comment "Supported FireWire (IEEE 1394) Adapters" depends on DVB_CORE && FIREWIRE
drivers/media/Makefile +2 -0
@@ -25,6 +25,8 @@ obj-y += rc/ obj-$(CONFIG_CEC_CORE) += cec/ +obj-$(CONFIG_VIDEO_VIRTIO) += virtio/ + # # Finally, merge the drivers that require the core #
drivers/media/virtio/Kconfig new file +11 -0
@@ -0,0 +1,11 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Video driver for virtio + +config VIDEO_VIRTIO + tristate "Virtio video V4L2 driver" + depends on VIRTIO && VIDEO_DEV && VIDEO_V4L2 + select VIDEOBUF2_DMA_SG + select V4L2_MEM2MEM_DEV + help + This is the virtual video driver for virtio. + Say Y or M.
drivers/media/virtio/Makefile new file +12 -0
@@ -0,0 +1,12 @@ +# SPDX-License-Identifier: GPL-2.0+ + +obj-$(CONFIG_VIDEO_VIRTIO) += virtio-video.o + +virtio-video-objs := \ + virtio_video_driver.o \ + virtio_video_vq.o \ + virtio_video_device.o \ + virtio_video_dec.o \ + virtio_video_enc.o \ + virtio_video_caps.o +
drivers/media/virtio/virtio_video.h new file +372 -0
372 lines not on this page · raw message
drivers/media/virtio/virtio_video_caps.c new file +618 -0
618 lines not on this page · raw message
drivers/media/virtio/virtio_video_dec.c new file +947 -0
947 lines not on this page · raw message
drivers/media/virtio/virtio_video_dec.h new file +30 -0
@@ -0,0 +1,30 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* Decoder header for virtio video driver. +* +* Copyright 2019 OpenSynergy GmbH. +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation; either version 2 of the License, or +* (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, see <http://www.gnu.org/licenses/>. +*/ + +#ifndef _VIRTIO_VIDEO_DEC_H +#define _VIRTIO_VIDEO_DEC_H + +#include "virtio_video.h" + +int virtio_video_dec_init (struct virtio_video_device *vvd); +int virtio_video_init_dec_ctrls (struct virtio_video_stream *stream); +int virtio_video_init_dec_queues (void *priv,struct vb2_queue *src_vq, +struct vb2_queue *dst_vq); + +#endif /* _VIRTIO_VIDEO_DEC_H */
drivers/media/virtio/virtio_video_device.c new file +747 -0
747 lines not on this page · raw message
drivers/media/virtio/virtio_video_driver.c new file +278 -0
248 lines not on this page · raw message
drivers/media/virtio/virtio_video_enc.c new file +1124 -0
1,124 lines not on this page · raw message
drivers/media/virtio/virtio_video_enc.h new file +30 -0
Lines of this file are not shown. View the raw message.
drivers/media/virtio/virtio_video_vq.c new file +950 -0
950 lines not on this page · raw message
include/uapi/linux/virtio_video.h new file +456 -0
456 lines not on this page · raw message
The rest of this diff is not shown. View the raw message.
Signature · 48 lines
2.24.0 ________________________________________ From: Dmitry Sepp Sent: Thursday, December 05, 2019 17:11 To: [email protected] Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected] Subject: [RFC] virtio video driver Hello, My apologies for the long delay. The driver code is now available and provided as a follow-up to the discussion from this thread [1]. The reference Linux kernel 5.4 driver implementation is located here: https://github.com/OpenSynergy/linux/tree/virtio-video-draft-v1 The driver is implemented using the V4L2 API. It allocates a v4l2 device for each probed virtio device and then creates a video device for each function within the respective virtio device. The driver implements the stateful decoder interface [2] and the stateful encoder interface (WIP) [3]. The DMA SG memory allocator tries to map buffers right away. As it is not always suitable, and some implementations might need just a physical address, we had to introduce a set of simple dma ops directly in the driver. The driver is in the RFC state and currently a bit ahead of the spec that was proposed in the discussion mentioned above. On the other hand, the driver unfortunately does not yet include changes proposed in the recent comments [4]. The driver currently supports encoder and decoder functions. Also, it does not fully pass the v4l2-compliance yet, it has been a bit out of the focus so far. Any feedback and contribution would be greatly appreciated. [1] https://markmail.org/message/gc6h25acct22niut [2] https://www.kernel.org/doc/html/v5.4/media/uapi/v4l/dev-decoder.html [3] https://hverkuil.home.xs4all.nl/codec-api/uapi/v4l/dev-encoder.html [4] https://markmail.org/message/yy67elx2adbivdsp Best regards, Dmitry. --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Part of the diff is not shown here. View the raw message.
Hi Dmitry, Hans,
Quoted 11 lines · Dmitry Sepp
From: Kiran Pawar <[email protected]>
This adds a Virtio based video driver for video streaming device that operates input and/or output data buffers to share video devices with several guests. The current implementation consist of V4L2 based video driver supporting video functions of decoder and encoder. This can be extended for other functions e.g. processor, capture and output. The device uses descriptor structures to advertise and negotiate stream formats and controls. This allows the driver to modify the processing logic of the device on a per stream basis.
Dmitry, just a note that some people may have email filters specifically for "PATCH" in the subject line.
Hans, do you think you could have some time to take a look at this regardless of that?
Best regards,
Tomasz
Quoted 5,754 lines
Signed-off-by Dmitry Sepp <[email protected]>Signed-off-by Kiran Pawar <[email protected]>Signed-off-by Nikolay Martyanov <[email protected]>--- drivers/media/Kconfig | 1 + drivers/media/Makefile | 2 + drivers/media/virtio/Kconfig | 11 + drivers/media/virtio/Makefile | 12 + drivers/media/virtio/virtio_video.h | 372 +++++++ drivers/media/virtio/virtio_video_caps.c | 618 +++++++++++ drivers/media/virtio/virtio_video_dec.c | 947 +++++++++++++++++ drivers/media/virtio/virtio_video_dec.h | 30 + drivers/media/virtio/virtio_video_device.c | 747 +++++++++++++ drivers/media/virtio/virtio_video_driver.c | 278 +++++ drivers/media/virtio/virtio_video_enc.c | 1124 ++++++++++++++++++++ drivers/media/virtio/virtio_video_enc.h | 30 + drivers/media/virtio/virtio_video_vq.c | 950 +++++++++++++++++ include/uapi/linux/virtio_ids.h | 2 + include/uapi/linux/virtio_video.h | 456 ++++++++ 15 files changed, 5580 insertions(+) create mode 100644 drivers/media/virtio/Kconfig create mode 100644 drivers/media/virtio/Makefile create mode 100644 drivers/media/virtio/virtio_video.h create mode 100644 drivers/media/virtio/virtio_video_caps.c create mode 100644 drivers/media/virtio/virtio_video_dec.c create mode 100644 drivers/media/virtio/virtio_video_dec.h create mode 100644 drivers/media/virtio/virtio_video_device.c create mode 100644 drivers/media/virtio/virtio_video_driver.c create mode 100644 drivers/media/virtio/virtio_video_enc.c create mode 100644 drivers/media/virtio/virtio_video_enc.h create mode 100644 drivers/media/virtio/virtio_video_vq.c create mode 100644 include/uapi/linux/virtio_video.hdrivers/media/Kconfig +1
-0@@ -170,6 +170,7 @@ source "drivers/media/pci/Kconfig" source "drivers/media/platform/Kconfig" source "drivers/media/mmc/Kconfig" source "drivers/media/radio/Kconfig" +source "drivers/media/virtio/Kconfig" comment "Supported FireWire (IEEE 1394) Adapters" depends on DVB_CORE && FIREWIREdrivers/media/Makefile +2
-0@@ -25,6 +25,8 @@ obj-y += rc/ obj-$(CONFIG_CEC_CORE) += cec/ +obj-$(CONFIG_VIDEO_VIRTIO) += virtio/ + # # Finally, merge the drivers that require the core #drivers/media/virtio/Kconfig new file +11
-0@@ -0,0 +1,11 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Video driver for virtio + +config VIDEO_VIRTIO + tristate "Virtio video V4L2 driver" + depends on VIRTIO && VIDEO_DEV && VIDEO_V4L2 + select VIDEOBUF2_DMA_SG + select V4L2_MEM2MEM_DEV + help + This is the virtual video driver for virtio. + Say Y or M.drivers/media/virtio/Makefile new file +12
-0@@ -0,0 +1,12 @@ +# SPDX-License-Identifier: GPL-2.0+ + +obj-$(CONFIG_VIDEO_VIRTIO) += virtio-video.o + +virtio-video-objs := \ + virtio_video_driver.o \ + virtio_video_vq.o \ + virtio_video_device.o \ + virtio_video_dec.o \ + virtio_video_enc.o \ + virtio_video_caps.o +drivers/media/virtio/virtio_video.h new file +372
-0@@ -0,0 +1,372 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* Common header for virtio video driver. +* +* Copyright 2019 OpenSynergy GmbH. +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation; either version 2 of the License, or +* (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, see <http://www.gnu.org/licenses/>. +*/ + +#ifndef _VIRTIO_VIDEO_H +#define _VIRTIO_VIDEO_H + +#include <linux/virtio.h> +#include <linux/virtio_ids.h> +#include <linux/virtio_config.h> +#include <linux/virtio_video.h> +#include <linux/list.h> +#include <media/v4l2-device.h> +#include <media/v4l2-mem2mem.h> +#include <media/v4l2-ctrls.h> + +#define DRIVER_NAME "virtio-video" + +#ifndef VIRTIO_ID_VIDEO +#define VIRTIO_ID_VIDEO 29 +#endif + +enum video_pin_type { +VIDEO_PIN_TYPE_INPUT =0 , +VIDEO_PIN_TYPE_OUTPUT , +}; + +enum video_params_scope { +VIDEO_PARAMS_SCOPE_STREAM =0 , +VIDEO_PARAMS_SCOPE_DEVICE , +}; + +#define MIN_BUFS_MIN 0 +#define MIN_BUFS_MAX 32 +#define MIN_BUFS_STEP 1 +#define MIN_BUFS_DEF 1 + +struct video_frame_rate { +unsigned int min_rate; +unsigned int max_rate; +unsigned int step; +}; + +struct video_frame_size { +unsigned int min_width; +unsigned int max_width; +unsigned int step_width; +unsigned int min_height; +unsigned int max_height; +unsigned int step_height; +unsigned int num_rates; +struct video_frame_rate *frame_rates; +}; + +struct video_pix_format { +uint32_t fourcc_format; +unsigned int num_sizes; +struct video_frame_size *frame_sizes; +}; + +struct video_frame_format { +int pin_type;/* VIRTIO_VIDEO_PIN_ */ +unsigned int num_formats; +struct video_pix_format *pix_formats; +}; + +struct video_control { +uint32_t control_type; +uint32_t step; +uint64_t min; +uint64_t max; +uint64_t def; +}; + +struct video_controls { +unsigned int num_controls; +struct video_control *control; +}; + +struct video_capability { +struct list_head caps_list_entry; +int cap_type;/* VIRTIO_VIDEO_CAP_ */ +unsigned int cap_id; +union { +struct video_frame_format frame_format; +struct video_controls controls; + } u; +}; + +struct virtio_video ; +struct virtio_video_vbuffer ; + +typedef void (*virtio_video_resp_cb )(struct virtio_video *vv, +struct virtio_video_vbuffer *vbuf); + +struct virtio_video_vbuffer { +char *buf; +int size; + +void *data_buf; +uint32_t data_size; + +char *resp_buf; +int resp_size; + +void *priv; +virtio_video_resp_cb resp_cb; + +struct list_head list; +}; + +struct virtio_video_queue { +struct virtqueue *vq; +spinlock_t qlock; +wait_queue_head_t ack_queue; +struct work_struct dequeue_work; +}; + +struct virtio_video { +struct v4l2_device v4l2_dev; +int instance; + +struct virtio_device *vdev; +struct virtio_video_queue ctrlq; +struct virtio_video_queue eventq; +wait_queue_head_t wq; +bool vq_ready; + +struct kmem_cache *vbufs; + +struct idr resource_idr; +spinlock_t resource_idr_lock; +struct idr stream_idr; +spinlock_t stream_idr_lock; + +uint32_t num_devices; +uint32_t funcs_size; +bool got_funcs; + +bool has_iommu; +struct list_head devices_list; + +int debug; +}; + +struct video_plane_format { +uint32_t channel; +uint32_t plane_size; +uint32_t stride; +uint32_t padding; +}; + +struct video_format_info { +unsigned int frame_rate; +unsigned int frame_width; +unsigned int frame_height; +unsigned int min_buffers; +uint32_t fourcc_format; +uint32_t num_planes; +struct video_plane_format plane_format[VIRTIO_VIDEO_MAX_PLANES ]; +bool is_updated; +}; + +enum video_stream_state { +STREAM_STATE_IDLE =0 , +STREAM_STATE_INIT , +STREAM_STATE_METADATA ,/* specific to decoder */ +STREAM_STATE_RUNNING , +STREAM_STATE_DRAIN , +STREAM_STATE_STOPPED , +STREAM_STATE_RESET ,/* specific to encoder */ +}; + +struct virtio_video_device { +struct virtio_video *vv; +struct video_device video_dev; +struct mutex video_dev_mutex; + +struct v4l2_m2m_dev *m2m_dev; + +struct workqueue_struct *workqueue; + +struct list_head devices_list_entry; +/* VIRTIO_VIDEO_FUNC_ */ +int type; +unsigned int id; +/* List of control capabilities */ +struct list_head ctrl_caps_list; +/* List of frame formats capabilities */ +struct list_head fmt_caps_list; + +/* The following 2 arrays contain pointers to pixel formats that are +* stored in 'fmt_caps_list' (as a part of the 'video_frame_format' +* structure). They are necessary to simplify indexing +* through pixel formats in the implementation of ENUM_FMT callbacks +*/ + +/* Array of pointers to pixel formats of CAPTURE pin */ +unsigned int num_capture_formats; +struct video_pix_format **capture_fmts; + +/* Array of pointers to pixel formats of OUTPUT pin */ +unsigned int num_output_formats; +struct video_pix_format **output_fmts; + +struct video_format_info in_info; +struct video_format_info out_info; +}; + +int virtio_video_alloc_vbufs (struct virtio_video *vv); +void virtio_video_free_vbufs (struct virtio_video *vv); +int virtio_video_alloc_events (struct virtio_video *vv,size_t num); + +int virtio_video_devices_init (struct virtio_video *vv,void *funcs_buf); +void virtio_video_devices_deinit (struct virtio_video *vv); + +struct virtio_video_stream { +uint32_t stream_id; +enum video_stream_state state; +struct video_device *video_dev; +struct v4l2_fh fh; +struct mutex vq_mutex; +struct v4l2_ctrl_handler ctrl_handler; +struct video_format_info in_info; +struct video_format_info out_info; +bool src_cleared; +bool dst_cleared; +bool mark_last_buffer_pending; +bool check_drain_sequence_pending; +struct work_struct work; +struct video_frame_size *current_frame_size; +}; + +struct virtio_video_buffer { +struct v4l2_m2m_buffer v4l2_m2m_vb; +uint32_t resource_id; +bool detached; +bool queued; +}; + +static inline struct virtio_video_device * +to_virtio_vd (struct video_device *video_dev) +{ + return container_of(video_dev, struct virtio_video_device, + video_dev); +} + +static inline struct virtio_video_stream *file2stream (struct file *file) +{ + return container_of(file->private_data, struct virtio_video_stream, fh); +} + +static inline struct virtio_video_stream *ctrl2stream (struct v4l2_ctrl *ctrl) +{ + return container_of(ctrl->handler, struct virtio_video_stream, + ctrl_handler); +} + +static inline struct virtio_video_stream *work2stream (struct work_struct *work) +{ + return container_of(work, struct virtio_video_stream, work); +} + +static inline struct virtio_video_buffer *to_virtio_vb (struct vb2_buffer *vb) +{ +struct vb2_v4l2_buffer *v4l2_vb =to_vb2_v4l2_buffer (vb); + + return container_of(v4l2_vb, struct virtio_video_buffer, + v4l2_m2m_vb.vb); +} + +void virtio_video_stream_id_get (struct virtio_video *vv, +struct virtio_video_stream *stream, +uint32_t *id); +void virtio_video_stream_id_put (struct virtio_video *vv,uint32_t id); +void virtio_video_resource_id_get (struct virtio_video *vv,uint32_t *id); +void virtio_video_resource_id_put (struct virtio_video *vv,uint32_t id); + +int virtio_video_req_stream_create (struct virtio_video *vv, +uint32_t function_id,uint32_t stream_id, +const char *name); +int virtio_video_req_stream_destroy (struct virtio_video *vv, +uint32_t function_id,uint32_t stream_id); +int virtio_video_req_stream_start (struct virtio_video *vv, +uint32_t function_id,uint32_t stream_id); +int virtio_video_req_stream_stop (struct virtio_video *vv, +uint32_t function_id, +struct virtio_video_stream *stream); +int virtio_video_req_stream_drain (struct virtio_video *vv, +uint32_t function_id,uint32_t stream_id); +int virtio_video_req_resource_create (struct virtio_video *vv, +uint32_t function_id,uint32_t stream_id, +uint32_t resource_id); +int virtio_video_req_resource_destroy (struct virtio_video *vv, +uint32_t function_id,uint32_t stream_id, +uint32_t resource_id); +int virtio_video_req_resource_queue (struct virtio_video *vv, +uint32_t function_id,uint32_t stream_id, +struct virtio_video_buffer *virtio_vb, +uint32_t data_size[], +uint8_t num_data_size,bool is_in); +int virtio_video_req_queue_clear (struct virtio_video *vv,uint32_t function_id, +struct virtio_video_stream *stream, +bool is_in); +int +virtio_video_req_resource_attach_backing (struct virtio_video *vv, +uint32_t function_id, +uint32_t stream_id, +uint32_t resource_id, +struct virtio_video_mem_entry *ents, +uint32_t nents); +int +virtio_video_req_resource_detach_backing (struct virtio_video *vv, +uint32_t function_id, +uint32_t stream_id, +struct virtio_video_buffer *virtio_vb); +int virtio_video_req_funcs (struct virtio_video *vv,void *resp_buf, +size_t resp_size); +int virtio_video_req_set_params (struct virtio_video *vv,uint32_t function_id, +struct video_format_info *format_info, +enum video_pin_type pin_type, +enum video_params_scope scope, +struct virtio_video_stream *stream); +int virtio_video_req_get_params (struct virtio_video *vv,uint32_t function_id, +enum video_pin_type pin_type, +enum video_params_scope scope, +struct virtio_video_stream *stream); +int virtio_video_req_set_control (struct virtio_video *vv, +uint32_t function_id,uint32_t stream_id, +uint32_t control,uint32_t val); + +void virtio_video_queue_res_chg_event (struct virtio_video_stream *stream); +void virtio_video_queue_eos_event (struct virtio_video_stream *stream); +void virtio_video_ctrl_ack (struct virtqueue *vq); +void virtio_video_event_ack (struct virtqueue *vq); +void virtio_video_dequeue_ctrl_func (struct work_struct *work); +void virtio_video_dequeue_event_func (struct work_struct *work); +void virtio_video_buf_done (struct virtio_video_buffer *virtio_vb, +uint32_t flags,uint64_t timestamp,uint32_t size); +void virtio_video_mark_drain_complete (struct virtio_video_stream *stream, +struct vb2_v4l2_buffer *v4l2_vb); + +void virtio_video_free_caps_list (struct list_head *caps_list); +size_t virtio_video_parse_virtio_function (void *func_buf, +struct virtio_video_device *vvd); +void virtio_video_clean_virtio_function (struct virtio_video_device *vvd); + +uint32_t virtio_video_format_to_v4l2 (uint32_t pixel_format); +uint32_t virtio_video_v4l2_fourcc_to_virtio (uint32_t fourcc); +uint32_t virtio_video_control_to_v4l2 (uint32_t control_type); +uint32_t virtio_video_profile_to_v4l2 (uint32_t profile); +uint32_t virtio_video_level_to_v4l2 (uint32_t level); +uint32_t virtio_video_v4l2_control_to_virtio (uint32_t v4l2_control); +uint32_t virtio_video_v4l2_profile_to_virtio (uint32_t v4l2_profile); +uint32_t virtio_video_v4l2_level_to_virtio (uint32_t v4l2_level); + +#endif /* _VIRTIO_VIDEO_H */ drivers/media/virtio/virtio_video_caps.c new file +618
-0@@ -0,0 +1,618 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* Driver for virtio video device. +* +* Copyright 2019 OpenSynergy GmbH. +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation; either version 2 of the License, or +* (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, see <http://www.gnu.org/licenses/>. +*/ + +#include <media/v4l2-ioctl.h> +#include <media/videobuf2-dma-sg.h> + +#include "virtio_video.h" + +static void pix_format_free (struct video_pix_format *pix_fmt) +{ +int size_idx =0 ; + +if (!pix_fmt) +return ; + +for (size_idx =0 ; size_idx < pix_fmt->num_sizes; size_idx++) +kfree (pix_fmt->frame_sizes[size_idx].frame_rates); +kfree (pix_fmt->frame_sizes); +} + +static void fmt_capability_free (struct video_frame_format *frame_fmt) +{ +int fmt_idx =0 ; + +if (!frame_fmt) +return ; + +for (fmt_idx =0 ; fmt_idx < frame_fmt->num_formats; fmt_idx++) +pix_format_free (&frame_fmt->pix_formats[fmt_idx]); +kfree (frame_fmt->pix_formats); +} + +static void ctrl_capability_free (struct video_controls *controls) +{ +if (!controls) +return ; + +kfree (controls->control); +} + +static void capability_free (struct video_capability *cap) +{ +if (!cap) +return ; + +switch (cap->cap_type) { +case VIRTIO_VIDEO_CAP_CONTROL : +ctrl_capability_free (&cap->u.controls); +break ; +case VIRTIO_VIDEO_CAP_PIN_FORMATS : +fmt_capability_free (&cap->u.frame_format); +break ; +default : +return ; + } +kfree (cap); +} + +static size_t parse_virtio_frame_rate (struct video_frame_rate *frame_rate, +void *frame_rate_buf, +struct virtio_video_device *vvd) +{ +struct virtio_video *vv =NULL ; +struct virtio_video_frame_rate *virtio_frame_rate =NULL ; +size_t frame_rate_size =sizeof (struct virtio_video_frame_rate ); + +if (!frame_rate || !frame_rate_buf || !vvd) +return 0 ; + + vv = vvd->vv; + virtio_frame_rate = frame_rate_buf; + +if (le32_to_cpu (virtio_frame_rate->desc.type) != +VIRTIO_VIDEO_DESC_FRAME_RATE ) { +v4l2_err (&vv->v4l2_dev, +"failed to read frame rate descriptor\n" ); +return 0 ; + } + + frame_rate->min_rate =le32_to_cpu (virtio_frame_rate->min_rate); + frame_rate->max_rate =le32_to_cpu (virtio_frame_rate->max_rate); + frame_rate->step =le32_to_cpu (virtio_frame_rate->step); + +return frame_rate_size; +} + +static size_t parse_virtio_frame_size (struct video_frame_size *frame_size, +void *frame_size_buf, +struct virtio_video_device *vvd) +{ +struct virtio_video *vv =NULL ; +size_t frame_size_size =0 ; +struct virtio_video_frame_size *virtio_frame_size =NULL ; +int rate_idx =0 ; +size_t offset =sizeof (struct virtio_video_frame_size ); + +if (!frame_size || !frame_size_buf || !vvd) +return 0 ; + + vv = vvd->vv; + virtio_frame_size = frame_size_buf; + +if (le32_to_cpu (virtio_frame_size->desc.type) != +VIRTIO_VIDEO_DESC_FRAME_SIZE ) { +v4l2_err (&vv->v4l2_dev, +"failed to read frame size descriptor\n" ); +return 0 ; + } + + frame_size->min_height =le32_to_cpu (virtio_frame_size->min_height); + frame_size->max_height =le32_to_cpu (virtio_frame_size->max_height); + frame_size->step_height =le32_to_cpu (virtio_frame_size->step_height); + frame_size->min_width =le32_to_cpu (virtio_frame_size->min_width); + frame_size->max_width =le32_to_cpu (virtio_frame_size->max_width); + frame_size->step_width =le32_to_cpu (virtio_frame_size->step_width); + frame_size->num_rates =le32_to_cpu (virtio_frame_size->num_rates); + + frame_size->frame_rates =kcalloc (frame_size->num_rates, +sizeof (struct video_frame_rate ), +GFP_KERNEL ); +if (!frame_size->frame_rates) { +v4l2_err (&vv->v4l2_dev,"failed to alloc frame rates\n" ); +return 0 ; + } + +for (rate_idx =0 ; rate_idx < frame_size->num_rates; rate_idx++) { +struct video_frame_rate *frame_rate = + &frame_size->frame_rates[rate_idx]; +size_t rate_size =0 ; + + rate_size =parse_virtio_frame_rate (frame_rate, + frame_size_buf + offset, + vvd); +if (rate_size ==0 ) { +v4l2_err (&vv->v4l2_dev,"failed to parse frame rate\n" ); +kfree (frame_size->frame_rates); +return 0 ; + } + offset += rate_size; + } + + frame_size_size = offset; + +return frame_size_size; +} + +static size_t parse_virtio_pix_fmt (struct video_pix_format *pix_fmt, +void *pix_buf, +struct virtio_video_device *vvd) +{ +struct virtio_video *vv =NULL ; +size_t pix_fmt_size =0 ; +struct virtio_video_pix_format *virtio_pix_fmt =NULL ; +int size_idx =0 ; +size_t offset =sizeof (struct virtio_video_pix_format ); + +if (!pix_fmt || !pix_buf || !vvd) +return 0 ; + + vv = vvd->vv; + virtio_pix_fmt = pix_buf; + +if (le32_to_cpu (virtio_pix_fmt->desc.type) != +VIRTIO_VIDEO_DESC_PIX_FORMAT ) { +v4l2_err (&vv->v4l2_dev,"failed to read pix fmt descriptor\n" ); +return 0 ; + } + + pix_fmt->fourcc_format = +virtio_video_format_to_v4l2 ( +le32_to_cpu (virtio_pix_fmt->pixel_format)); + pix_fmt->num_sizes =le32_to_cpu (virtio_pix_fmt->num_sizes); + + pix_fmt->frame_sizes =kcalloc (pix_fmt->num_sizes, +sizeof (struct video_frame_size ), +GFP_KERNEL ); +if (!pix_fmt->frame_sizes) { +v4l2_err (&vv->v4l2_dev,"failed to alloc frame sizes\n" ); +return 0 ; + } + +for (size_idx =0 ; size_idx < pix_fmt->num_sizes; size_idx++) { +struct video_frame_size *frame_size =NULL ; +size_t size_size =0 ; + + frame_size = &pix_fmt->frame_sizes[size_idx]; + size_size =parse_virtio_frame_size (frame_size, + pix_buf + offset, vvd); +if (size_size ==0 ) { +v4l2_err (&vv->v4l2_dev,"failed to parse frame size\n" ); +kfree (pix_fmt->frame_sizes); +return 0 ; + } + offset += size_size; + } + + pix_fmt_size = offset; + +return pix_fmt_size; +} + +static size_t parse_virtio_fmts_cap (struct video_frame_format *frame_fmt, +void *cap_buf, +struct virtio_video_device *vvd) +{ +struct virtio_video *vv =NULL ; +size_t fmts_size =0 ; +struct virtio_video_frame_format *virtio_fmt =NULL ; +int fmt_idx =0 ; +size_t offset =sizeof (struct virtio_video_frame_format ); +struct virtio_video_capability dummy; + +if (!frame_fmt || !cap_buf || !vvd) +return 0 ; + + vv = vvd->vv; + virtio_fmt = cap_buf; + + frame_fmt->pin_type =le32_to_cpu (virtio_fmt->pin_type); + frame_fmt->num_formats =le32_to_cpu (virtio_fmt->num_formats); + + frame_fmt->pix_formats =kcalloc (frame_fmt->num_formats, +sizeof (struct video_pix_format ), +GFP_KERNEL ); +if (!frame_fmt->pix_formats) { +v4l2_err (&vv->v4l2_dev,"failed to alloc pix formats\n" ); +return 0 ; + } + +for (fmt_idx =0 ; fmt_idx < frame_fmt->num_formats; fmt_idx++) { +struct video_pix_format *pix_fmt =NULL ; +size_t fmt_size =0 ; + + pix_fmt = &frame_fmt->pix_formats[fmt_idx]; + fmt_size =parse_virtio_pix_fmt (pix_fmt, cap_buf + offset, vvd); +if (fmt_size ==0 ) { +v4l2_err (&vv->v4l2_dev, +"failed to parse pixel format\n" ); +fmt_capability_free (frame_fmt); +return 0 ; + } + offset += fmt_size; + } + +switch (frame_fmt->pin_type) { +case VIRTIO_VIDEO_PIN_INPUT : + vvd->num_output_formats += frame_fmt->num_formats; +break ; +case VIRTIO_VIDEO_PIN_OUTPUT : + vvd->num_capture_formats += frame_fmt->num_formats; +break ; +default : +v4l2_err (&vv->v4l2_dev,"failed to parse a pin type\n" ); +return 0 ; + } + + fmts_size = offset -sizeof (dummy.u); + +return fmts_size; +} + +static size_t parse_virtio_ctrl (struct video_control *control, +void *control_buf, +struct virtio_video_device *vvd) +{ +struct virtio_video *vv =NULL ; +struct virtio_video_control *virtio_control =NULL ; +size_t control_size =sizeof (struct virtio_video_control ); + +if (!control || !control_buf || !vvd) +return 0 ; + + vv = vvd->vv; + virtio_control = control_buf; + +if (le32_to_cpu (virtio_control->desc.type) != +VIRTIO_VIDEO_DESC_CONTROL ) { +v4l2_err (&vv->v4l2_dev,"failed to read control descriptor\n" ); +return 0 ; + } + + control->control_type = +virtio_video_control_to_v4l2 + (le32_to_cpu (virtio_control->control_type)); + +switch (control->control_type) { +case V4L2_CID_MPEG_VIDEO_H264_PROFILE : + control->min = +virtio_video_profile_to_v4l2 + (le64_to_cpu (virtio_control->min)); + control->max = +virtio_video_profile_to_v4l2 + (le64_to_cpu (virtio_control->max)); + control->step = +virtio_video_profile_to_v4l2 + (le32_to_cpu (virtio_control->step)); + control->def = +virtio_video_profile_to_v4l2 + (le64_to_cpu (virtio_control->def)); +break ; +case V4L2_CID_MPEG_VIDEO_H264_LEVEL : + control->min = +virtio_video_level_to_v4l2 + (le64_to_cpu (virtio_control->min)); + control->max = +virtio_video_level_to_v4l2 + (le64_to_cpu (virtio_control->max)); + control->step = +virtio_video_level_to_v4l2 + (le32_to_cpu (virtio_control->step)); + control->def = +virtio_video_level_to_v4l2 + (le64_to_cpu (virtio_control->def)); +break ; +default : + control->min =le64_to_cpu (virtio_control->min); + control->max =le64_to_cpu (virtio_control->max); + control->step =le32_to_cpu (virtio_control->step); + control->def =le64_to_cpu (virtio_control->def); +break ; + } + +return control_size; +} + +static size_t parse_virtio_ctrls_cap (struct video_controls *controls, +void *cap_buf, +struct virtio_video_device *vvd) +{ +struct virtio_video_controls *virtio_controls =NULL ; +struct virtio_video *vv =NULL ; +size_t ctrls_size =0 ; +int ctrl_idx =0 ; +size_t offset =sizeof (struct virtio_video_controls ); +struct virtio_video_capability dummy; + +if (!controls || !cap_buf || !vvd) +return 0 ; + + vv = vvd->vv; + virtio_controls = cap_buf; + + controls->num_controls =le32_to_cpu (virtio_controls->num_controls); + controls->control =kcalloc (controls->num_controls, +sizeof (struct video_control ), +GFP_KERNEL ); +if (!controls->control) { +v4l2_err (&vv->v4l2_dev,"failed to alloc controls\n" ); +return 0 ; + } + +for (ctrl_idx =0 ; ctrl_idx < controls->num_controls; ctrl_idx++) { +struct video_control *ctrl =NULL ; +size_t ctrl_size =0 ; + + ctrl = &controls->control[ctrl_idx]; + ctrl_size =parse_virtio_ctrl (ctrl, cap_buf + offset, vvd); +if (ctrl_size ==0 ) { +v4l2_err (&vv->v4l2_dev,"failed to parse control\n" ); +ctrl_capability_free (controls); +return 0 ; + } + offset += ctrl_size; + } + + ctrls_size = offset -sizeof (dummy.u); + +return ctrls_size; +} + +static size_t parse_virtio_capability (struct video_capability *cap, +void *cap_buf, +struct virtio_video_device *vvd) +{ +struct virtio_video *vv =NULL ; +struct virtio_video_capability *virtio_cap =NULL ; +size_t offset =0 ; +size_t extra_size =0 ; +size_t cap_size =0 ; + +if (!cap || !cap_buf || !vvd) +return 0 ; + + vv = vvd->vv; + virtio_cap = cap_buf; + offset =sizeof (struct virtio_video_capability ) -sizeof (virtio_cap->u); + +if (le32_to_cpu (virtio_cap->desc.type) !=VIRTIO_VIDEO_DESC_CAP ) { +v4l2_err (&vv->v4l2_dev, +"failed to read capability descriptor\n" ); +return 0 ; + } + + cap->cap_id =le32_to_cpu (virtio_cap->cap_id); + cap->cap_type =le32_to_cpu (virtio_cap->cap_type); + +switch (cap->cap_type) { +case VIRTIO_VIDEO_CAP_CONTROL : + extra_size =parse_virtio_ctrls_cap (&cap->u.controls, + cap_buf + offset, vvd); +if (extra_size ==0 ) { +v4l2_err (&vv->v4l2_dev,"failed to parse ctrl cap\n" ); +return 0 ; + } +break ; +case VIRTIO_VIDEO_CAP_PIN_FORMATS : + extra_size =parse_virtio_fmts_cap (&cap->u.frame_format, + cap_buf + offset, vvd); +if (extra_size ==0 ) { +v4l2_err (&vv->v4l2_dev,"failed to parse fmts cap\n" ); +return 0 ; + } +break ; +default : +v4l2_err (&vv->v4l2_dev,"undefined capability type\n" ); +return 0 ; + } + +if (extra_size <0 ) + extra_size =0 ; + + cap_size =sizeof (struct virtio_video_capability ) + extra_size; + +return cap_size; + +} + +void virtio_video_free_caps_list (struct list_head *caps_list) +{ +struct video_capability *cap =NULL ; +struct video_capability *tmp =NULL ; + +list_for_each_entry_safe (cap, tmp, caps_list, caps_list_entry) { +list_del (&cap->caps_list_entry); +capability_free (cap); + } + +} + +static int virtio_video_copy_params (struct video_format_info *dst, +const struct virtio_video_params *src) +{ +if (!dst || !src) +return -EINVAL ; + + dst->fourcc_format = +virtio_video_format_to_v4l2 ( +le32_to_cpu (src->pixel_format)); + dst->frame_height =le32_to_cpu (src->frame_height); + dst->frame_width =le32_to_cpu (src->frame_width); + dst->frame_rate =le32_to_cpu (src->frame_rate); + dst->min_buffers =le32_to_cpu (src->min_buffers); +return 0 ; +} + +size_t virtio_video_parse_virtio_function (void *func_buf, +struct virtio_video_device *vvd) +{ +struct virtio_video *vv =NULL ; +size_t func_size =0 ; +struct virtio_video_function *virtio_func =NULL ; +uint32_t num_caps =0 ; +int cap_idx =0 ; +size_t offset =sizeof (struct virtio_video_function ); +struct video_capability *cap =NULL ; +int capture_idx =0 ; +int output_idx =0 ; +int ret =0 ; + +if (!func_buf || !vvd) +return 0 ; + + vv = vvd->vv; + virtio_func = func_buf; + +if (le32_to_cpu (virtio_func->desc.type) !=VIRTIO_VIDEO_DESC_FUNC ) { +v4l2_err (&vv->v4l2_dev,"failed to read function descriptor\n" ); +return 0 ; + } + + vvd->id =le32_to_cpu (virtio_func->function_id); + vvd->type =le32_to_cpu (virtio_func->function_type); + +if (le32_to_cpu (virtio_func->in_params.desc.type) != +VIRTIO_VIDEO_DESC_PARAMS ) { +v4l2_err (&vv->v4l2_dev,"failed to read function params\n" ); +return 0 ; + } + + ret =virtio_video_copy_params (&vvd->in_info, + &virtio_func->in_params); +if (ret) { +v4l2_err (&vv->v4l2_dev,"failed to read 'in' params\n" ); +return 0 ; + } + +if (le32_to_cpu (virtio_func->out_params.desc.type) != +VIRTIO_VIDEO_DESC_PARAMS ) { +v4l2_err (&vv->v4l2_dev,"failed to read function params\n" ); +return 0 ; + } + + ret =virtio_video_copy_params (&vvd->out_info, + &virtio_func->out_params); +if (ret) { +v4l2_err (&vv->v4l2_dev,"failed to read 'out' params\n" ); +return 0 ; + } + + num_caps =le32_to_cpu (virtio_func->num_caps); + +for (cap_idx =0 ; cap_idx < num_caps; cap_idx++) { +size_t cap_size =0 ; + + cap =kzalloc (sizeof (*cap),GFP_KERNEL ); + +if (!cap) { +virtio_video_free_caps_list (&vvd->ctrl_caps_list); +virtio_video_free_caps_list (&vvd->fmt_caps_list); +return 0 ; + } + + cap_size =parse_virtio_capability (cap, func_buf + offset, vvd); +if (cap_size ==0 ) { +v4l2_err (&vv->v4l2_dev, +"failed to parse a capability\n" ); +virtio_video_free_caps_list (&vvd->ctrl_caps_list); +virtio_video_free_caps_list (&vvd->fmt_caps_list); +kfree (cap); +return 0 ; + } + offset += cap_size; + +switch (cap->cap_type) { +case VIRTIO_VIDEO_CAP_PIN_FORMATS : +list_add (&cap->caps_list_entry, &vvd->fmt_caps_list); +break ; +case VIRTIO_VIDEO_CAP_CONTROL : +list_add (&cap->caps_list_entry, &vvd->ctrl_caps_list); +break ; +default : +virtio_video_free_caps_list (&vvd->ctrl_caps_list); +virtio_video_free_caps_list (&vvd->fmt_caps_list); +capability_free (cap); +break ; + } + } + + vvd->capture_fmts =kcalloc (vvd->num_capture_formats, +sizeof (struct video_capability *), +GFP_KERNEL ); + vvd->output_fmts =kcalloc (vvd->num_output_formats, +sizeof (struct video_capability *), +GFP_KERNEL ); +if (!vvd->capture_fmts || !vvd->output_fmts) { +v4l2_err (&vv->v4l2_dev,"failed to alloc capability arrays\n" ); +virtio_video_free_caps_list (&vvd->ctrl_caps_list); +virtio_video_free_caps_list (&vvd->fmt_caps_list); + } + + cap =NULL ; +list_for_each_entry (cap, &vvd->fmt_caps_list, caps_list_entry) { +struct video_frame_format *frame_fmt = &cap->u.frame_format; +int pix_idx =0 ; + +switch (le32_to_cpu (frame_fmt->pin_type)) { +case VIRTIO_VIDEO_PIN_INPUT : +for (pix_idx =0 ; pix_idx < frame_fmt->num_formats; + pix_idx++) { + vvd->output_fmts[pix_idx + output_idx] = + &frame_fmt->pix_formats[pix_idx]; + } + output_idx += pix_idx; +break ; +case VIRTIO_VIDEO_PIN_OUTPUT : +for (pix_idx =0 ; pix_idx < frame_fmt->num_formats; + pix_idx++) { + vvd->capture_fmts[pix_idx + capture_idx] = + &frame_fmt->pix_formats[pix_idx]; + } + capture_idx += pix_idx; +break ; +default : +v4l2_err (&vv->v4l2_dev,"failed to parse a pin type\n" ); +return 0 ; + } + } + + func_size = offset; +return func_size; +} + +void virtio_video_clean_virtio_function (struct virtio_video_device *vvd) +{ +if (!vvd) +return ; + +kfree (vvd->capture_fmts); +kfree (vvd->output_fmts); +virtio_video_free_caps_list (&vvd->ctrl_caps_list); +virtio_video_free_caps_list (&vvd->fmt_caps_list); +}drivers/media/virtio/virtio_video_dec.c new file +947
-0@@ -0,0 +1,947 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* Decoder for virtio video device. +* +* Copyright 2019 OpenSynergy GmbH. +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation; either version 2 of the License, or +* (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, see <http://www.gnu.org/licenses/>. +*/ + +#include <media/v4l2-event.h> +#include <media/v4l2-ioctl.h> +#include <media/videobuf2-dma-sg.h> + +#include "virtio_video.h" + +static int virtio_video_queue_setup (struct vb2_queue *vq, +unsigned int *num_buffers, +unsigned int *num_planes, +unsigned int sizes[], +struct device *alloc_devs[]) +{ +int i; +struct virtio_video_stream *stream =vb2_get_drv_priv (vq); +struct video_format_info *p_info; + +if (*num_planes) +return 0 ; + +if (V4L2_TYPE_IS_OUTPUT (vq->type)) + p_info = &stream->in_info; +else + p_info = &stream->out_info; + + *num_planes = p_info->num_planes; + +for (i =0 ; i < p_info->num_planes; i++) + sizes[i] = p_info->plane_format[i].plane_size; + +return 0 ; +} + +static int virtio_video_buf_plane_init (uint32_t idx, +uint32_t resource_id, +struct virtio_video_device *vvd, +struct virtio_video_stream *stream, +struct vb2_buffer *vb) +{ +int ret; +unsigned int i; +struct virtio_video *vv = vvd->vv; +struct scatterlist *sg; +struct virtio_video_mem_entry *ents; +struct sg_table *sgt =vb2_dma_sg_plane_desc (vb, idx); + +/* Freed when the request has been completed */ + ents =kcalloc (sgt->nents,sizeof (*ents),GFP_KERNEL ); +for_each_sg (sgt->sgl, sg, sgt->nents, i) { + ents[i].addr =cpu_to_le64 (vv->has_iommu + ?sg_dma_address (sg) + :sg_phys (sg)); + ents[i].length =cpu_to_le32 (sg->length); + } + +v4l2_dbg (1 , vv->debug, &vv->v4l2_dev,"mem entries:\n" ); +if (vv->debug >=1 ) { +for (i =0 ; i < sgt->nents; i++) +pr_debug ("\t%03i: addr=%llx length=%u\n" , i, + ents[i].addr, ents[i].length); + } + + ret =virtio_video_req_resource_attach_backing (vv, vvd->id, + stream->stream_id, + resource_id, ents, + sgt->nents); +if (ret) +kfree (ents); + +return ret; +} + +static int virtio_video_buf_init (struct vb2_buffer *vb) +{ +int ret =0 ; +unsigned int i; +uint32_t resource_id; +struct virtio_video_stream *stream =vb2_get_drv_priv (vb->vb2_queue); +struct virtio_video_buffer *virtio_vb =to_virtio_vb (vb); +struct virtio_video_device *vvd =to_virtio_vd (stream->video_dev); +struct virtio_video *vv = vvd->vv; + +virtio_video_resource_id_get (vv, &resource_id); + ret =virtio_video_req_resource_create (vv, vvd->id, stream->stream_id, + resource_id); +if (ret) +return ret; + +for (i =0 ; i < vb->num_planes; ++i) { + ret =virtio_video_buf_plane_init (i, + resource_id, vvd, stream, vb); +if (ret) +break ; + } + +if (ret) { +virtio_video_req_resource_destroy (vvd->vv, vvd->id, + stream->stream_id, + resource_id); +virtio_video_resource_id_put (vvd->vv, resource_id); +return ret; + } + + virtio_vb->queued = false; + virtio_vb->detached = false; + virtio_vb->resource_id = resource_id; + +return 0 ; +} + +static void virtio_video_buf_cleanup (struct vb2_buffer *vb) +{ +int ret; +struct virtio_video_stream *stream =vb2_get_drv_priv (vb->vb2_queue); +struct virtio_video_buffer *virtio_vb =to_virtio_vb (vb); +struct virtio_video_device *vvd =to_virtio_vd (stream->video_dev); +struct virtio_video *vv = vvd->vv; + + ret =virtio_video_req_resource_detach_backing (vv, vvd->id, + stream->stream_id, + virtio_vb); +if (ret) +return ; + + ret =wait_event_timeout (vv->wq, virtio_vb->detached,5 *HZ ); +if (ret ==0 ) { +v4l2_err (&vv->v4l2_dev,"timed out waiting for detach\n" ); +return ; + } + +virtio_video_req_resource_destroy (vv, vvd->id, stream->stream_id, + virtio_vb->resource_id); +virtio_video_resource_id_put (vv, virtio_vb->resource_id); +} + +static void virtio_video_buf_queue (struct vb2_buffer *vb) +{ +int i, ret; +struct vb2_buffer *src_buf; +struct virtio_video_buffer *virtio_vb; +uint32_t data_size[VB2_MAX_PLANES ] = {0 }; +struct vb2_v4l2_buffer *v4l2_vb =to_vb2_v4l2_buffer (vb); +struct vb2_v4l2_buffer *src_vb; +struct virtio_video_stream *stream =vb2_get_drv_priv (vb->vb2_queue); +struct virtio_video_device *vvd =to_virtio_vd (stream->video_dev); +struct virtio_video *vv = vvd->vv; + +v4l2_m2m_buf_queue (stream->fh.m2m_ctx, v4l2_vb); + +if ((stream->state !=STREAM_STATE_INIT ) || + !V4L2_TYPE_IS_OUTPUT (vb->vb2_queue->type)) +return ; + + src_vb =v4l2_m2m_next_src_buf (stream->fh.m2m_ctx); +if (!src_vb) { +v4l2_err (&vv->v4l2_dev,"no src buf during initialization\n" ); +return ; + } + + src_buf = &src_vb->vb2_buf; +for (i =0 ; i < src_buf->num_planes; ++i) + data_size[i] = src_buf->planes[i].bytesused; + + virtio_vb =to_virtio_vb (src_buf); + + ret =virtio_video_req_resource_queue (vv, vvd->id, stream->stream_id, + virtio_vb, data_size, + src_buf->num_planes, + true); +if (ret) { +v4l2_err (&vv->v4l2_dev,"failed to queue an src buffer\n" ); +return ; + } + + virtio_vb->queued = true; + stream->src_cleared = false; + src_vb =v4l2_m2m_src_buf_remove (stream->fh.m2m_ctx); +} + +static int virtio_video_start_streaming (struct vb2_queue *vq, +unsigned int count) +{ +int ret; +struct virtio_video_stream *stream =vb2_get_drv_priv (vq); +struct virtio_video_device *vvd =to_virtio_vd (stream->video_dev); +struct virtio_video *vv = vvd->vv; + +if ((V4L2_TYPE_IS_OUTPUT (vq->type) && + (stream->state ==STREAM_STATE_INIT )) || + (stream->state ==STREAM_STATE_STOPPED )) { + ret =virtio_video_req_stream_start (vv, vvd->id, + stream->stream_id); +if (ret) +return ret; + } + +if (!V4L2_TYPE_IS_OUTPUT (vq->type) && + (stream->state >=STREAM_STATE_INIT )) + stream->state =STREAM_STATE_RUNNING ; + +return 0 ; +} + +static void virtio_video_stop_streaming (struct vb2_queue *vq) +{ +int ret; +bool *cleared; +bool is_output =V4L2_TYPE_IS_OUTPUT (vq->type); +struct virtio_video_stream *stream =vb2_get_drv_priv (vq); +struct virtio_video_device *vvd =to_virtio_vd (stream->video_dev); +struct virtio_video *vv = vvd->vv; +struct vb2_v4l2_buffer *v4l2_vb; + +if (is_output) + cleared = &stream->src_cleared; +else + cleared = &stream->dst_cleared; + + ret =virtio_video_req_queue_clear (vv, vvd->id, stream, is_output); +if (ret) { +v4l2_err (&vv->v4l2_dev,"failed to clear queue\n" ); +return ; + } + + ret =wait_event_timeout (vv->wq, *cleared,5 *HZ ); +if (ret ==0 ) { +v4l2_err (&vv->v4l2_dev,"timed out waiting for queue clear\n" ); +return ; + } + +for (;;) { +if (is_output) + v4l2_vb =v4l2_m2m_src_buf_remove (stream->fh.m2m_ctx); +else + v4l2_vb =v4l2_m2m_dst_buf_remove (stream->fh.m2m_ctx); +if (!v4l2_vb) +break ; +v4l2_m2m_buf_done (v4l2_vb,VB2_BUF_STATE_ERROR ); + } +} + +static const struct vb2_ops virtio_video_qops = { + .queue_setup = virtio_video_queue_setup, + .buf_init = virtio_video_buf_init, + .buf_cleanup = virtio_video_buf_cleanup, + .buf_queue = virtio_video_buf_queue, + .start_streaming = virtio_video_start_streaming, + .stop_streaming = virtio_video_stop_streaming, + .wait_prepare = vb2_ops_wait_prepare, + .wait_finish = vb2_ops_wait_finish, +}; + +static int virtio_video_g_volatile_ctrl (struct v4l2_ctrl *ctrl) +{ +int ret =0 ; +struct virtio_video_stream *stream =ctrl2stream (ctrl); +struct virtio_video_device *vvd =to_virtio_vd (stream->video_dev); + +switch (ctrl->id) { +case V4L2_CID_MIN_BUFFERS_FOR_CAPTURE : +if (stream->state >=STREAM_STATE_METADATA ) + ctrl->val = vvd->out_info.min_buffers; +else + ctrl->val =0 ; +break ; +default : + ret = -EINVAL ; + } + +return ret; +} + +static const struct v4l2_ctrl_ops virtio_video_ctrl_ops = { + .g_volatile_ctrl = virtio_video_g_volatile_ctrl, +}; + +int virtio_video_init_dec_ctrls (struct virtio_video_stream *stream) +{ +struct v4l2_ctrl *ctrl; + +v4l2_ctrl_handler_init (&stream->ctrl_handler,1 ); + + ctrl =v4l2_ctrl_new_std (&stream->ctrl_handler, + &virtio_video_ctrl_ops, +V4L2_CID_MIN_BUFFERS_FOR_CAPTURE , +MIN_BUFS_MIN ,MIN_BUFS_MAX ,MIN_BUFS_STEP , +MIN_BUFS_DEF ); + ctrl->flags |=V4L2_CTRL_FLAG_VOLATILE ; + +if (stream->ctrl_handler.error) +return stream->ctrl_handler.error; + +v4l2_ctrl_handler_setup (&stream->ctrl_handler); + +return 0 ; +} + +int virtio_video_init_dec_queues (void *priv,struct vb2_queue *src_vq, +struct vb2_queue *dst_vq) +{ +int ret; +struct virtio_video_stream *stream = priv; +struct virtio_video_device *vvd =to_virtio_vd (stream->video_dev); +struct device *dev = vvd->vv->v4l2_dev.dev; + + src_vq->type =V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE ; + src_vq->io_modes =VB2_MMAP |VB2_DMABUF ; + src_vq->drv_priv = stream; + src_vq->buf_struct_size =sizeof (struct virtio_video_buffer ); + src_vq->ops = &virtio_video_qops; + src_vq->mem_ops = &vb2_dma_sg_memops; + src_vq->min_buffers_needed = vvd->in_info.min_buffers; + src_vq->timestamp_flags =V4L2_BUF_FLAG_TIMESTAMP_COPY ; + src_vq->lock = &stream->vq_mutex; + src_vq->dev = dev; + + ret =vb2_queue_init (src_vq); +if (ret) +return ret; + + dst_vq->type =V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE ; + dst_vq->io_modes =VB2_MMAP |VB2_DMABUF ; + dst_vq->drv_priv = stream; + dst_vq->buf_struct_size =sizeof (struct virtio_video_buffer ); + dst_vq->ops = &virtio_video_qops; + dst_vq->mem_ops = &vb2_dma_sg_memops; + dst_vq->min_buffers_needed = vvd->out_info.min_buffers; + dst_vq->timestamp_flags =V4L2_BUF_FLAG_TIMESTAMP_COPY ; + dst_vq->lock = &stream->vq_mutex; + dst_vq->dev = dev; + +return vb2_queue_init (dst_vq); +} + +static int virtio_video_querycap (struct file *file,void *fh, +struct v4l2_capability *cap) +{ +struct video_device *video_dev =video_devdata (file); + +strncpy (cap->driver,DRIVER_NAME ,sizeof (cap->driver)); +strncpy (cap->card, video_dev->name,sizeof (cap->card)); +snprintf (cap->bus_info,sizeof (cap->bus_info),"virtio:%s" , + video_dev->name); + + cap->device_caps =V4L2_CAP_VIDEO_M2M_MPLANE |V4L2_CAP_STREAMING ; + cap->capabilities = cap->device_caps |V4L2_CAP_DEVICE_CAPS ; + +return 0 ; +} + +static int virtio_video_enum_fmt_vid_cap (struct file *file,void *fh, +struct v4l2_fmtdesc *f) +{ +struct virtio_video_stream *stream =file2stream (file); +struct virtio_video_device *vvd =to_virtio_vd (stream->video_dev); + +if (f->type !=V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE ) +return -EINVAL ; + +if (f->index >= vvd->num_capture_formats) +return -EINVAL ; + + f->pixelformat = vvd->capture_fmts[f->index]->fourcc_format; + +return 0 ; +} + +static struct video_pix_format *find_pix_format (struct video_pix_format **list, +uint32_t fourcc,int num) +{ +int idx =0 ; + +for (idx =0 ; idx < num; idx++) { +if (list[idx]->fourcc_format == fourcc) +return list[idx]; + } +return NULL ; +} + +static int virtio_video_try_decoder_cmd (struct file *file,void *fh, +struct v4l2_decoder_cmd *cmd) +{ +struct virtio_video_stream *stream =file2stream (file); +struct virtio_video_device *vvd =video_drvdata (file); +struct virtio_video *vv = vvd->vv; + +if (stream->state ==STREAM_STATE_DRAIN ) +return -EBUSY ; + +switch (cmd->cmd) { +case V4L2_DEC_CMD_STOP : +case V4L2_DEC_CMD_START : +if (cmd->flags !=0 ) { +v4l2_err (&vv->v4l2_dev,"flags=%u are not supported" , + cmd->flags); +return -EINVAL ; + } +break ; +default : +return -EINVAL ; + } + +return 0 ; +} + +static int virtio_video_decoder_cmd (struct file *file,void *fh, +struct v4l2_decoder_cmd *cmd) +{ +int ret; +struct vb2_queue *src_vq, *dst_vq; +struct virtio_video_stream *stream =file2stream (file); +struct virtio_video_device *vvd =video_drvdata (file); +struct virtio_video *vv = vvd->vv; + + ret =virtio_video_try_decoder_cmd (file, fh, cmd); +if (ret <0 ) +return ret; + + dst_vq =v4l2_m2m_get_vq (stream->fh.m2m_ctx, +V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE ); + +switch (cmd->cmd) { +case V4L2_DEC_CMD_START : +vb2_clear_last_buffer_dequeued (dst_vq); + ret =virtio_video_req_stream_start (vv, vvd->id, + stream->stream_id); +if (ret) +return ret; +break ; +case V4L2_DEC_CMD_STOP : + src_vq =v4l2_m2m_get_vq (stream->fh.m2m_ctx, +V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE ); + +if (!vb2_is_streaming (src_vq)) { +v4l2_dbg (1 , vv->debug, + &vv->v4l2_dev,"output is not streaming\n" ); +return 0 ; + } + +if (!vb2_is_streaming (dst_vq)) { +v4l2_dbg (1 , vv->debug, + &vv->v4l2_dev,"capture is not streaming\n" ); +return 0 ; + } + + ret =virtio_video_req_stream_drain (vv, vvd->id, + stream->stream_id); +if (ret) { +v4l2_err (&vv->v4l2_dev,"failed to drain stream\n" ); +return ret; + } + stream->state =STREAM_STATE_DRAIN ; +break ; +default : +return -EINVAL ; + } + +return 0 ; +} + +static int virtio_video_enum_framesizes (struct file *file,void *fh, +struct v4l2_frmsizeenum *f) +{ +struct virtio_video_stream *stream =file2stream (file); +struct virtio_video_device *vvd =to_virtio_vd (stream->video_dev); +struct video_frame_size *frame_size =NULL ; +int i =0 ; +bool fake_non_discrete = false; +int idx = f->index; +struct video_pix_format *fmt =NULL ; + + fmt =find_pix_format (vvd->output_fmts, f->pixel_format, + vvd->num_output_formats); +if (fmt ==NULL ) + fmt =find_pix_format (vvd->capture_fmts, f->pixel_format, + vvd->num_capture_formats); +if (fmt ==NULL ) +return -EINVAL ; + +if (idx >= fmt->num_sizes) +return -EINVAL ; + +/* If the index is 0 - it is the first call of ENUM_FRAMESIZES, that +* defines a type of all the frame sizes. +* +* Indexes > 0 can be used later only in case of the type is discrete. +* But, if there is at least one non-discrete type later in the array - +* it may be misinterpreted as a discrete one. +* +* Hence, check, whether there is a non-discrete frame size, and if yes +* - return the first of them. +*/ +if (!idx) +for (i =0 ; i < fmt->num_sizes; i++) { + frame_size = &fmt->frame_sizes[i]; +if (frame_size->min_width != frame_size->max_width || + frame_size->min_height != frame_size->max_height) { + idx = i; + fake_non_discrete = true; +break ; + } + } + +/* Index > 0 can be used only for discrete frame sizes. Type of the +* frame sizes is equal to type of the first frame size. +*/ +if (idx && !fake_non_discrete) { + frame_size = &fmt->frame_sizes[0 ]; +if (frame_size->min_width != frame_size->max_width || + frame_size->min_height != frame_size->max_height) +return -EINVAL ; + } + + frame_size = &fmt->frame_sizes[idx]; + +if (frame_size->min_width == frame_size->max_width && + frame_size->min_height == frame_size->max_height) { + f->type =V4L2_FRMSIZE_TYPE_DISCRETE ; + f->discrete.width = frame_size->min_width; + f->discrete.height = frame_size->min_height; + }else { +if (idx && !fake_non_discrete) +return -EINVAL ; + f->stepwise.min_width = frame_size->min_width; + f->stepwise.min_height = frame_size->min_height; + f->stepwise.step_height = frame_size->step_height; + f->stepwise.step_width = frame_size->step_width; + f->stepwise.max_height = frame_size->max_height; + f->stepwise.max_width = frame_size->max_width; +if (frame_size->step_width ==1 && + frame_size->min_height ==1 ) { + f->type =V4L2_FRMSIZE_TYPE_CONTINUOUS ; + }else { + f->type =V4L2_FRMSIZE_TYPE_STEPWISE ; + } + } + +return 0 ; +} + +static bool in_stepped_interval (uint32_t int_start,uint32_t int_end, +uint32_t step,uint32_t point) +{ +if (point < int_start || point > int_end) +return false; + +if (step ==0 && int_start == int_end && int_start == point) +return true; + +if (step !=0 && (point - int_start) % step ==0 ) +return true; + +return false; +} + +static int virtio_video_enum_framemintervals (struct file *file,void *fh, +struct v4l2_frmivalenum *f) +{ +struct virtio_video_stream *stream =file2stream (file); +struct virtio_video_device *vvd =to_virtio_vd (stream->video_dev); +struct video_frame_size *fsize =NULL ; +int fsize_idx =0 ; +int i =0 ; +bool fake_non_discrete = false; +int idx = f->index; +struct video_pix_format *fmt =NULL ; +struct video_frame_rate *frate =NULL ; + + fmt =find_pix_format (vvd->output_fmts, f->pixel_format, + vvd->num_output_formats); +if (fmt ==NULL ) + fmt =find_pix_format (vvd->capture_fmts, f->pixel_format, + vvd->num_capture_formats); +if (fmt ==NULL ) +return -EINVAL ; + +for (fsize_idx =0 ; fsize_idx <= fmt->num_sizes; fsize_idx++) { + fsize = &fmt->frame_sizes[fsize_idx]; +if (in_stepped_interval (fsize->min_width, fsize->max_width, + fsize->step_width, f->width) && +in_stepped_interval (fsize->min_height, fsize->max_height, + fsize->step_height, f->height)) +break ; + } + +if (fsize ==NULL ) +return -EINVAL ; + +if (idx >= fsize->num_rates) +return -EINVAL ; + +/* If the index is 0 - it is the first call of ENUM_FRAMEIVALS, that +* defines a type of all the frame intervals. +* +* Indexes > 0 can be used later only in case of the type is discrete. +* But, if there is at least one non-discrete type later in the array - +* it may be misinterpreted as a discrete one. +* +* Hence, check, whether there is a non-discrete frame rate, and if yes +* - return the first of them. +*/ +if (!idx) +for (i =0 ; i < fsize->num_rates; i++) { + frate = &fsize->frame_rates[i]; +if (frate->min_rate != frate->max_rate) { + fake_non_discrete = true; + idx = i; +break ; + } + } + +/* Index > 0 can be used only for discrete frame rates. Type of the +* frame rate is equal to the type of the first frame size. +*/ +if (idx && !fake_non_discrete) { + frate = &fsize->frame_rates[0 ]; +if (frate->max_rate != frate->min_rate) +return -EINVAL ; + } + + frate = &fsize->frame_rates[idx]; +if (frate->max_rate == frate->min_rate) { + f->type =V4L2_FRMIVAL_TYPE_DISCRETE ; + f->discrete.numerator =1 ; + f->discrete.denominator = frate->max_rate; + }else { +if (idx && !fake_non_discrete) +return -EINVAL ; +/* If A > B, then 1/A < 1/B, so max denominator = min_rate +* and vise versa +*/ + f->stepwise.min.numerator =1 ; + f->stepwise.min.denominator = frate->max_rate; + f->stepwise.max.numerator =1 ; + f->stepwise.max.denominator = frate->min_rate; + f->stepwise.step.numerator =1 ; + f->stepwise.step.denominator = frate->step; +if (frate->step ==1 ) + f->type =V4L2_FRMIVAL_TYPE_CONTINUOUS ; +else + f->type =V4L2_FRMIVAL_TYPE_STEPWISE ; + } + +return 0 ; +} + +static int virtio_video_enum_fmt_vid_out (struct file *file,void *fh, +struct v4l2_fmtdesc *f) +{ +struct virtio_video_stream *stream =file2stream (file); +struct virtio_video_device *vvd =to_virtio_vd (stream->video_dev); + +if (f->type !=V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE ) +return -EINVAL ; + +if (f->index >= vvd->num_output_formats) +return -EINVAL ; + + f->pixelformat = vvd->output_fmts[f->index]->fourcc_format; + +return 0 ; +} + +static void fill_v4l2_format_from_info (struct video_format_info *info, +struct v4l2_pix_format_mplane *pix_mp) +{ +int i; + + pix_mp->width = info->frame_width; + pix_mp->height = info->frame_height; + pix_mp->field =V4L2_FIELD_NONE ; + pix_mp->colorspace =V4L2_COLORSPACE_REC709 ; + pix_mp->xfer_func =0 ; + pix_mp->ycbcr_enc =0 ; + pix_mp->quantization =0 ; +memset (pix_mp->reserved,0 ,sizeof (pix_mp->reserved)); +memset (pix_mp->plane_fmt[0 ].reserved,0 , +sizeof (pix_mp->plane_fmt[0 ].reserved)); + + pix_mp->num_planes = info->num_planes; + pix_mp->pixelformat = info->fourcc_format; + +for (i =0 ; i < info->num_planes; i++) { + pix_mp->plane_fmt[i].bytesperline = + info->plane_format[i].stride; + pix_mp->plane_fmt[i].sizeimage = + info->plane_format[i].plane_size; + } +} + +static int virtio_video_g_fmt (struct virtio_video_stream *stream, +struct v4l2_format *f) +{ +struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp; +struct video_format_info *info; + +if (!V4L2_TYPE_IS_OUTPUT (f->type)) + info = &stream->out_info; +else + info = &stream->in_info; + +fill_v4l2_format_from_info (info, pix_mp); +return 0 ; +} + +static int virtio_video_g_fmt_vid_out (struct file *file,void *fh, +struct v4l2_format *f) +{ +return virtio_video_g_fmt (file2stream (file), f); +} + +static int virtio_video_g_fmt_vid_cap (struct file *file,void *fh, +struct v4l2_format *f) +{ +return virtio_video_g_fmt (file2stream (file), f); +} + +static inline bool within_range (uint32_t min,uint32_t val,uint32_t max) +{ +return ((val - min) <= (max - min)); +} + +static inline bool needs_alignment (uint32_t val,uint32_t a) +{ +if (a ==0 ||IS_ALIGNED (val, a)) +return false; + +return true; +} + +static int virtio_video_try_fmt (struct virtio_video_stream *stream, +struct v4l2_format *f) +{ +int i, idx =0 ; +struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp; +struct virtio_video_device *vvd =to_virtio_vd (stream->video_dev); +struct video_pix_format *fmt =NULL ; +struct video_frame_size *frm_sz =NULL ; +bool found = false; + +if (V4L2_TYPE_IS_OUTPUT (f->type)) + fmt =find_pix_format (vvd->output_fmts, pix_mp->pixelformat, + vvd->num_output_formats); +else + fmt =find_pix_format (vvd->capture_fmts, pix_mp->pixelformat, + vvd->num_capture_formats); + +if (!fmt) { +if (f->type ==V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE ) +fill_v4l2_format_from_info (&stream->out_info, pix_mp); +else if (f->type ==V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE ) +fill_v4l2_format_from_info (&stream->in_info, pix_mp); +else +return -EINVAL ; +return 0 ; + } + +for (i =0 ; i < fmt->num_sizes && !found; i++) { + frm_sz = &fmt->frame_sizes[i]; +if (!within_range (frm_sz->min_width, pix_mp->width, + frm_sz->max_width)) +continue ; + +if (!within_range (frm_sz->min_height, pix_mp->height, + frm_sz->max_height)) +continue ; + + idx = i; +/* +* Try to find a more suitable frame size. Go with the current +* one otherwise. +*/ +if (needs_alignment (pix_mp->width, frm_sz->step_width)) +continue ; + +if (needs_alignment (pix_mp->height, frm_sz->step_height)) +continue ; + + found = true; + } + +if (!found) { + frm_sz = &fmt->frame_sizes[idx]; + pix_mp->width =clamp (pix_mp->width, frm_sz->min_width, + frm_sz->max_width); +if (frm_sz->step_width !=0 ) + pix_mp->width =ALIGN (pix_mp->width, + frm_sz->step_width); + + pix_mp->height =clamp (pix_mp->height, frm_sz->min_height, + frm_sz->max_height); +if (frm_sz->step_height !=0 ) + pix_mp->height =ALIGN (pix_mp->height, + frm_sz->step_height); + } + +return 0 ; +} + +static int virtio_video_s_fmt (struct virtio_video_stream *stream, +struct v4l2_format *f) +{ +int i, ret; +struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp; +struct virtio_video_device *vvd =to_virtio_vd (stream->video_dev); +struct virtio_video *vv = vvd->vv; +struct video_format_info info; +struct video_format_info *p_info; +enum video_pin_type pin =VIDEO_PIN_TYPE_INPUT ; + + ret =virtio_video_try_fmt (stream, f); +if (ret) +return ret; + + info.frame_width = pix_mp->width; + info.frame_height = pix_mp->height; + info.num_planes = pix_mp->num_planes; + info.fourcc_format = pix_mp->pixelformat; + +for (i =0 ; i < info.num_planes; i++) { + info.plane_format[i].stride = + pix_mp->plane_fmt[i].bytesperline; + info.plane_format[i].plane_size = + pix_mp->plane_fmt[i].sizeimage; + } + +if (!V4L2_TYPE_IS_OUTPUT (f->type)) + pin =VIDEO_PIN_TYPE_OUTPUT ; + +virtio_video_req_set_params (vv, vvd->id, &info, pin, +VIDEO_PARAMS_SCOPE_STREAM , stream); + +virtio_video_req_get_params (vv, vvd->id,VIDEO_PIN_TYPE_INPUT , +VIDEO_PARAMS_SCOPE_STREAM , stream); + +virtio_video_req_get_params (vv, vvd->id,VIDEO_PIN_TYPE_OUTPUT , +VIDEO_PARAMS_SCOPE_STREAM , stream); + +if (V4L2_TYPE_IS_OUTPUT (f->type)) + p_info = &stream->in_info; +else + p_info = &stream->out_info; + +fill_v4l2_format_from_info (p_info, pix_mp); + +if (V4L2_TYPE_IS_OUTPUT (f->type)) { +if (stream->state ==STREAM_STATE_IDLE ) + stream->state =STREAM_STATE_INIT ; + } + +return 0 ; +} + +static int virtio_video_s_fmt_vid_cap (struct file *file,void *fh, +struct v4l2_format *f) +{ +struct virtio_video_stream *stream =file2stream (file); + +return virtio_video_s_fmt (stream, f); +} + +static int virtio_video_s_fmt_vid_out (struct file *file,void *fh, +struct v4l2_format *f) +{ +struct virtio_video_stream *stream =file2stream (file); + +return virtio_video_s_fmt (stream, f); +} + +static int +virtio_video_subscribe_event (struct v4l2_fh *fh, +const struct v4l2_event_subscription *sub) +{ +switch (sub->type) { +case V4L2_EVENT_SOURCE_CHANGE : +return v4l2_src_change_event_subscribe (fh, sub); +default : +return -EINVAL ; + } +} + +static const struct v4l2_ioctl_ops virtio_video_device_dec_ioctl_ops = { + .vidioc_querycap = virtio_video_querycap, + + .vidioc_enum_fmt_vid_cap = virtio_video_enum_fmt_vid_cap, + .vidioc_g_fmt_vid_cap = virtio_video_g_fmt_vid_cap, + .vidioc_s_fmt_vid_cap = virtio_video_s_fmt_vid_cap, + + .vidioc_g_fmt_vid_cap_mplane = virtio_video_g_fmt_vid_cap, + .vidioc_s_fmt_vid_cap_mplane = virtio_video_s_fmt_vid_cap, + + .vidioc_enum_fmt_vid_out = virtio_video_enum_fmt_vid_out, + .vidioc_g_fmt_vid_out = virtio_video_g_fmt_vid_out, + .vidioc_s_fmt_vid_out = virtio_video_s_fmt_vid_out, + + .vidioc_g_fmt_vid_out_mplane = virtio_video_g_fmt_vid_out, + .vidioc_s_fmt_vid_out_mplane = virtio_video_s_fmt_vid_out, + + .vidioc_try_decoder_cmd = virtio_video_try_decoder_cmd, + .vidioc_decoder_cmd = virtio_video_decoder_cmd, + .vidioc_enum_frameintervals = virtio_video_enum_framemintervals, + .vidioc_enum_framesizes = virtio_video_enum_framesizes, + + .vidioc_reqbufs = v4l2_m2m_ioctl_reqbufs, + .vidioc_querybuf = v4l2_m2m_ioctl_querybuf, + .vidioc_qbuf = v4l2_m2m_ioctl_qbuf, + .vidioc_dqbuf = v4l2_m2m_ioctl_dqbuf, + .vidioc_prepare_buf = v4l2_m2m_ioctl_prepare_buf, + .vidioc_create_bufs = v4l2_m2m_ioctl_create_bufs, + .vidioc_expbuf = v4l2_m2m_ioctl_expbuf, + + .vidioc_streamon = v4l2_m2m_ioctl_streamon, + .vidioc_streamoff = v4l2_m2m_ioctl_streamoff, + + .vidioc_subscribe_event = virtio_video_subscribe_event, + .vidioc_unsubscribe_event = v4l2_event_unsubscribe, +}; + +int virtio_video_dec_init (struct virtio_video_device *vvd) +{ +struct video_device *vd =NULL ; + +if (!vvd) +return -EINVAL ; + + vd = &vvd->video_dev; + vd->ioctl_ops = &virtio_video_device_dec_ioctl_ops; +return 0 ; +}drivers/media/virtio/virtio_video_dec.h new file +30
-0@@ -0,0 +1,30 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* Decoder header for virtio video driver. +* +* Copyright 2019 OpenSynergy GmbH. +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation; either version 2 of the License, or +* (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, see <http://www.gnu.org/licenses/>. +*/ + +#ifndef _VIRTIO_VIDEO_DEC_H +#define _VIRTIO_VIDEO_DEC_H + +#include "virtio_video.h" + +int virtio_video_dec_init (struct virtio_video_device *vvd); +int virtio_video_init_dec_ctrls (struct virtio_video_stream *stream); +int virtio_video_init_dec_queues (void *priv,struct vb2_queue *src_vq, +struct vb2_queue *dst_vq); + +#endif /* _VIRTIO_VIDEO_DEC_H */ drivers/media/virtio/virtio_video_device.c new file +747
-0@@ -0,0 +1,747 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* Driver for virtio video device. +* +* Copyright 2019 OpenSynergy GmbH. +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation; either version 2 of the License, or +* (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, see <http://www.gnu.org/licenses/>. +*/ + +#include <media/v4l2-event.h> +#include <media/v4l2-ioctl.h> +#include <media/videobuf2-dma-sg.h> + +#include "virtio_video.h" +#include "virtio_video_dec.h" +#include "virtio_video_enc.h" + +void virtio_video_queue_eos_event (struct virtio_video_stream *stream) +{ +static const struct v4l2_event eos_event = { + .type =V4L2_EVENT_EOS + }; + +v4l2_event_queue_fh (&stream->fh, &eos_event); +} + +void virtio_video_queue_res_chg_event (struct virtio_video_stream *stream) +{ +static const struct v4l2_event ev_src_ch = { + .type =V4L2_EVENT_SOURCE_CHANGE , + .u.src_change.changes = +V4L2_EVENT_SRC_CH_RESOLUTION , + }; + +v4l2_event_queue_fh (&stream->fh, &ev_src_ch); +} + +void virtio_video_mark_drain_complete (struct virtio_video_stream *stream, +struct vb2_v4l2_buffer *v4l2_vb) +{ +struct vb2_buffer *vb2_buf; + + v4l2_vb->flags |=V4L2_BUF_FLAG_LAST ; + + vb2_buf = &v4l2_vb->vb2_buf; + vb2_buf->planes[0 ].bytesused =0 ; + +v4l2_m2m_buf_done (v4l2_vb,VB2_BUF_STATE_DONE ); + stream->state =STREAM_STATE_STOPPED ; +} + +void virtio_video_buf_done (struct virtio_video_buffer *virtio_vb, +uint32_t flags,uint64_t timestamp,uint32_t size) +{ +int i, ret; +enum vb2_buffer_state done_state =VB2_BUF_STATE_DONE ; +struct vb2_v4l2_buffer *v4l2_vb = &virtio_vb->v4l2_m2m_vb.vb; +struct vb2_buffer *vb = &v4l2_vb->vb2_buf; +struct vb2_queue *vb2_queue = vb->vb2_queue; +struct virtio_video_stream *stream =vb2_get_drv_priv (vb2_queue); +struct virtio_video_device *vvd =to_virtio_vd (stream->video_dev); +struct virtio_video *vv = vvd->vv; +struct video_format_info *p_info; + + virtio_vb->queued = false; + +if (!V4L2_TYPE_IS_OUTPUT (vb2_queue->type) && + stream->check_drain_sequence_pending) { +virtio_video_mark_drain_complete (stream, v4l2_vb); + stream->check_drain_sequence_pending = false; +return ; + } + +if (flags &VIRTIO_VIDEO_BUFFER_F_ERR ) + done_state =VB2_BUF_STATE_ERROR ; + +if (flags &VIRTIO_VIDEO_BUFFER_IFRAME ) + v4l2_vb->flags |=V4L2_BUF_FLAG_KEYFRAME ; + +if (flags &VIRTIO_VIDEO_BUFFER_BFRAME ) + v4l2_vb->flags |=V4L2_BUF_FLAG_BFRAME ; + +if (flags &VIRTIO_VIDEO_BUFFER_PFRAME ) + v4l2_vb->flags |=V4L2_BUF_FLAG_PFRAME ; + +if (flags &VIRTIO_VIDEO_BUFFER_F_EOS ) { + v4l2_vb->flags |=V4L2_BUF_FLAG_LAST ; + ret =virtio_video_req_stream_stop (vv, vvd->id, stream); +if (ret) +v4l2_err (&vv->v4l2_dev,"failed to stop stream\n" ); +else + stream->state =STREAM_STATE_STOPPED ; +virtio_video_queue_eos_event (stream); + } + +if (!V4L2_TYPE_IS_OUTPUT (vb2_queue->type)) { +if (vvd->type ==VIRTIO_VIDEO_FUNC_ENCODER ) { + vb->planes[0 ].bytesused = size; + }else if (vvd->type ==VIRTIO_VIDEO_FUNC_DECODER ) { + p_info = &stream->out_info; +for (i =0 ; i < p_info->num_planes; i++) + vb->planes[i].bytesused = + p_info->plane_format[i].plane_size; + } + + vb->timestamp = timestamp; + } + +v4l2_m2m_buf_done (v4l2_vb, done_state); +} + + +static void virtio_video_worker (struct work_struct *work) +{ +unsigned int i; +int ret; +struct vb2_buffer *vb2_buf; +struct vb2_v4l2_buffer *src_vb, *dst_vb; +struct virtio_video_buffer *virtio_vb; +struct virtio_video_stream *stream =work2stream (work); +struct virtio_video_device *vvd =to_virtio_vd (stream->video_dev); +struct vb2_queue *src_vq = +v4l2_m2m_get_vq (stream->fh.m2m_ctx, +V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE ); +struct vb2_queue *dst_vq = +v4l2_m2m_get_vq (stream->fh.m2m_ctx, +V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE ); +struct virtio_video *vv = vvd->vv; +uint32_t data_size[VB2_MAX_PLANES ] = {0 }; + + +mutex_lock (dst_vq->lock); +for (;;) { + dst_vb =v4l2_m2m_next_dst_buf (stream->fh.m2m_ctx); +if (dst_vb ==NULL ) +break ; + + vb2_buf = &dst_vb->vb2_buf; + virtio_vb =to_virtio_vb (vb2_buf); + +for (i =0 ; i < vb2_buf->num_planes; ++i) + data_size[i] = vb2_buf->planes[i].bytesused; + + ret =virtio_video_req_resource_queue (vv, vvd->id, + stream->stream_id, + virtio_vb, data_size, + vb2_buf->num_planes, + false); +if (ret) { +v4l2_info (&vv->v4l2_dev, +"failed to queue a dst buffer\n" ); +v4l2_m2m_job_finish (vvd->m2m_dev, stream->fh.m2m_ctx); +mutex_unlock (dst_vq->lock); +return ; + } + + virtio_vb->queued = true; + stream->dst_cleared = false; + dst_vb =v4l2_m2m_dst_buf_remove (stream->fh.m2m_ctx); + } +mutex_unlock (dst_vq->lock); + +mutex_lock (src_vq->lock); +for (;;) { +if (stream->state ==STREAM_STATE_DRAIN ) +break ; + + src_vb =v4l2_m2m_next_src_buf (stream->fh.m2m_ctx); +if (src_vb ==NULL ) +break ; + + vb2_buf = &src_vb->vb2_buf; + virtio_vb =to_virtio_vb (vb2_buf); + +for (i =0 ; i < vb2_buf->num_planes; ++i) + data_size[i] = vb2_buf->planes[i].bytesused; + + ret =virtio_video_req_resource_queue (vv, vvd->id, + stream->stream_id, + virtio_vb, + data_size, + vb2_buf->num_planes, + true); +if (ret) { +v4l2_info (&vv->v4l2_dev, +"failed to queue an src buffer\n" ); +v4l2_m2m_job_finish (vvd->m2m_dev, stream->fh.m2m_ctx); +mutex_unlock (src_vq->lock); +return ; + } + + virtio_vb->queued = true; + stream->src_cleared = false; + src_vb =v4l2_m2m_src_buf_remove (stream->fh.m2m_ctx); + } +mutex_unlock (src_vq->lock); + +v4l2_m2m_job_finish (vvd->m2m_dev, stream->fh.m2m_ctx); +} + +static int virtio_video_device_open (struct file *file) +{ +int ret; +uint32_t stream_id; +char name[TASK_COMM_LEN ]; +struct virtio_video_stream *stream; +struct video_device *video_dev =video_devdata (file); +struct virtio_video_device *vvd =video_drvdata (file); +struct virtio_video *vv = vvd->vv; + + stream =kzalloc (sizeof (*stream),GFP_KERNEL ); +if (!stream) +return -ENOMEM ; + +get_task_comm (name, current); +virtio_video_stream_id_get (vv, stream, &stream_id); + ret =virtio_video_req_stream_create (vv, vvd->id, stream_id, name); +if (ret) { +v4l2_err (&vv->v4l2_dev,"failed to create stream\n" ); + goto err_stream_create; + } + + stream->video_dev = video_dev; + stream->stream_id = stream_id; + stream->state =STREAM_STATE_IDLE ; +mutex_init (&stream->vq_mutex); +INIT_WORK (&stream->work, virtio_video_worker); +v4l2_fh_init (&stream->fh, video_dev); + stream->fh.ctrl_handler = &stream->ctrl_handler; + +if (vvd->type ==VIRTIO_VIDEO_FUNC_DECODER ) { + stream->fh.m2m_ctx = +v4l2_m2m_ctx_init (vvd->m2m_dev, stream, + &virtio_video_init_dec_queues); + }else if (vvd->type ==VIRTIO_VIDEO_FUNC_ENCODER ) { + stream->fh.m2m_ctx = +v4l2_m2m_ctx_init (vvd->m2m_dev, stream, + &virtio_video_init_enc_queues); + }else { +v4l2_err (&vv->v4l2_dev,"unsupported device type\n" ); + goto err_stream_create; + } + +v4l2_m2m_set_src_buffered (stream->fh.m2m_ctx, true); +v4l2_m2m_set_dst_buffered (stream->fh.m2m_ctx, true); + file->private_data = &stream->fh; +v4l2_fh_add (&stream->fh); + +if (vvd->type ==VIRTIO_VIDEO_FUNC_DECODER ) + ret =virtio_video_init_dec_ctrls (stream); +else if (vvd->type ==VIRTIO_VIDEO_FUNC_ENCODER ) + ret =virtio_video_init_enc_ctrls (stream); + +if (ret) { +v4l2_err (&vv->v4l2_dev,"failed to init controls\n" ); + goto err_init_ctrls; + } + + ret =virtio_video_req_get_params (vv, vvd->id,VIDEO_PIN_TYPE_INPUT , +VIDEO_PARAMS_SCOPE_STREAM , stream); +if (ret) { +v4l2_err (&vv->v4l2_dev,"failed to get stream in params\n" ); + goto err_init_ctrls; + } + + ret =virtio_video_req_get_params (vv, vvd->id,VIDEO_PIN_TYPE_OUTPUT , +VIDEO_PARAMS_SCOPE_STREAM , stream); +if (ret) { +v4l2_err (&vv->v4l2_dev,"failed to get stream out params\n" ); + goto err_init_ctrls; + } + +return 0 ; + +err_init_ctrls: +v4l2_fh_del (&stream->fh); +v4l2_fh_exit (&stream->fh); +mutex_lock (video_dev->lock); +v4l2_m2m_ctx_release (stream->fh.m2m_ctx); +mutex_unlock (video_dev->lock); +err_stream_create: +virtio_video_stream_id_put (vv, stream_id); +kfree (stream); + +return ret; +} + +static int virtio_video_device_release (struct file *file) +{ +struct virtio_video_stream *stream =file2stream (file); +struct video_device *video_dev =video_devdata (file); +struct virtio_video_device *vvd =video_drvdata (file); +struct virtio_video *vv = vvd->vv; + +v4l2_fh_del (&stream->fh); +v4l2_fh_exit (&stream->fh); +mutex_lock (video_dev->lock); +v4l2_m2m_ctx_release (stream->fh.m2m_ctx); +mutex_unlock (video_dev->lock); + +virtio_video_req_stream_destroy (vv, vvd->id, stream->stream_id); +virtio_video_stream_id_put (vv, stream->stream_id); + +kfree (stream); + +return 0 ; +} + +static const struct v4l2_file_operations virtio_video_device_fops = { + .owner =THIS_MODULE , + .open = virtio_video_device_open, + .release = virtio_video_device_release, + .poll = v4l2_m2m_fop_poll, + .unlocked_ioctl = video_ioctl2, + .mmap = v4l2_m2m_fop_mmap, +}; + +static void virtio_video_device_run (void *priv) +{ +struct virtio_video_stream *stream = priv; +struct virtio_video_device *vvd =to_virtio_vd (stream->video_dev); + +queue_work (vvd->workqueue, &stream->work); +} + +static int virtio_video_device_job_ready (void *priv) +{ +struct virtio_video_stream *stream = priv; + +if (stream->state ==STREAM_STATE_STOPPED ) +return 0 ; + +if (v4l2_m2m_num_src_bufs_ready (stream->fh.m2m_ctx) >0 || +v4l2_m2m_num_dst_bufs_ready (stream->fh.m2m_ctx) >0 ) +return 1 ; + +return 0 ; +} + +static void virtio_video_device_job_abort (void *priv) +{ +struct virtio_video_stream *stream = priv; +struct virtio_video_device *vvd =to_virtio_vd (stream->video_dev); + +v4l2_m2m_job_finish (vvd->m2m_dev, stream->fh.m2m_ctx); +} + +static const struct v4l2_m2m_ops virtio_video_device_m2m_ops = { + .device_run = virtio_video_device_run, + .job_ready = virtio_video_device_job_ready, + .job_abort = virtio_video_device_job_abort, +}; + +uint32_t virtio_video_control_to_v4l2 (uint32_t control_type) +{ +switch (control_type) { +case VIRTIO_VIDEO_CONTROL_BITRATE : +return V4L2_CID_MPEG_VIDEO_BITRATE ; +case VIRTIO_VIDEO_CONTROL_PROFILE : +return V4L2_CID_MPEG_VIDEO_H264_PROFILE ; +case VIRTIO_VIDEO_CONTROL_LEVEL : +return V4L2_CID_MPEG_VIDEO_H264_LEVEL ; +default : +return 0 ; + } +} + +uint32_t virtio_video_profile_to_v4l2 (uint32_t profile) +{ +switch (profile) { +case VIRTIO_MPEG_VIDEO_H264_PROFILE_BASELINE : +return V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE ; +case VIRTIO_MPEG_VIDEO_H264_PROFILE_CONSTRAINED_BASELINE : +return V4L2_MPEG_VIDEO_H264_PROFILE_CONSTRAINED_BASELINE ; +case VIRTIO_MPEG_VIDEO_H264_PROFILE_MAIN : +return V4L2_MPEG_VIDEO_H264_PROFILE_MAIN ; +case VIRTIO_MPEG_VIDEO_H264_PROFILE_EXTENDED : +return V4L2_MPEG_VIDEO_H264_PROFILE_EXTENDED ; +case VIRTIO_MPEG_VIDEO_H264_PROFILE_HIGH : +return V4L2_MPEG_VIDEO_H264_PROFILE_HIGH ; +case VIRTIO_MPEG_VIDEO_H264_PROFILE_HIGH_10 : +return V4L2_MPEG_VIDEO_H264_PROFILE_HIGH_10 ; +case VIRTIO_MPEG_VIDEO_H264_PROFILE_HIGH_422 : +return V4L2_MPEG_VIDEO_H264_PROFILE_HIGH_422 ; +case VIRTIO_MPEG_VIDEO_H264_PROFILE_HIGH_444_PREDICTIVE : +return V4L2_MPEG_VIDEO_H264_PROFILE_HIGH_444_PREDICTIVE ; +case VIRTIO_MPEG_VIDEO_H264_PROFILE_HIGH_10_INTRA : +return V4L2_MPEG_VIDEO_H264_PROFILE_HIGH_10_INTRA ; +case VIRTIO_MPEG_VIDEO_H264_PROFILE_HIGH_422_INTRA : +return V4L2_MPEG_VIDEO_H264_PROFILE_HIGH_422_INTRA ; +case VIRTIO_MPEG_VIDEO_H264_PROFILE_HIGH_444_INTRA : +return V4L2_MPEG_VIDEO_H264_PROFILE_HIGH_444_INTRA ; +case VIRTIO_MPEG_VIDEO_H264_PROFILE_CAVLC_444_INTRA : +return V4L2_MPEG_VIDEO_H264_PROFILE_CAVLC_444_INTRA ; +case VIRTIO_MPEG_VIDEO_H264_PROFILE_SCALABLE_BASELINE : +return V4L2_MPEG_VIDEO_H264_PROFILE_SCALABLE_BASELINE ; +case VIRTIO_MPEG_VIDEO_H264_PROFILE_SCALABLE_HIGH : +return V4L2_MPEG_VIDEO_H264_PROFILE_SCALABLE_HIGH ; +case VIRTIO_MPEG_VIDEO_H264_PROFILE_SCALABLE_HIGH_INTRA : +return V4L2_MPEG_VIDEO_H264_PROFILE_SCALABLE_HIGH_INTRA ; +case VIRTIO_MPEG_VIDEO_H264_PROFILE_STEREO_HIGH : +return V4L2_MPEG_VIDEO_H264_PROFILE_STEREO_HIGH ; +case VIRTIO_MPEG_VIDEO_H264_PROFILE_MULTIVIEW_HIGH : +return V4L2_MPEG_VIDEO_H264_PROFILE_MULTIVIEW_HIGH ; +default : +return 0 ; + } +} + +uint32_t virtio_video_level_to_v4l2 (uint32_t level) +{ +switch (level) { +case VIRTIO_MPEG_VIDEO_H264_LEVEL_1_0 : +return V4L2_MPEG_VIDEO_H264_LEVEL_1_0 ; +case VIRTIO_MPEG_VIDEO_H264_LEVEL_2_0 : +return V4L2_MPEG_VIDEO_H264_LEVEL_2_0 ; +case VIRTIO_MPEG_VIDEO_H264_LEVEL_4_0 : +return V4L2_MPEG_VIDEO_H264_LEVEL_4_0 ; +case VIRTIO_MPEG_VIDEO_H264_LEVEL_4_1 : +return V4L2_MPEG_VIDEO_H264_LEVEL_4_1 ; +case VIRTIO_MPEG_VIDEO_H264_LEVEL_4_2 : +return V4L2_MPEG_VIDEO_H264_LEVEL_4_2 ; +case VIRTIO_MPEG_VIDEO_H264_LEVEL_5_0 : +return V4L2_MPEG_VIDEO_H264_LEVEL_5_0 ; +case VIRTIO_MPEG_VIDEO_H264_LEVEL_5_1 : +return V4L2_MPEG_VIDEO_H264_LEVEL_5_1 ; +default : +return 0 ; + } +} + +uint32_t virtio_video_v4l2_control_to_virtio (uint32_t v4l2_control) +{ +switch (v4l2_control) { +case V4L2_CID_MPEG_VIDEO_BITRATE : +return VIRTIO_VIDEO_CONTROL_BITRATE ; +case V4L2_CID_MPEG_VIDEO_H264_PROFILE : +return VIRTIO_VIDEO_CONTROL_PROFILE ; +case V4L2_CID_MPEG_VIDEO_H264_LEVEL : +return VIRTIO_VIDEO_CONTROL_LEVEL ; +default : +return 0 ; + } +} + +uint32_t virtio_video_v4l2_profile_to_virtio (uint32_t v4l2_profile) +{ +switch (v4l2_profile) { +case V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE : +return VIRTIO_MPEG_VIDEO_H264_PROFILE_BASELINE ; +case V4L2_MPEG_VIDEO_H264_PROFILE_MAIN : +return VIRTIO_MPEG_VIDEO_H264_PROFILE_MAIN ; +case V4L2_MPEG_VIDEO_H264_PROFILE_EXTENDED : +return VIRTIO_MPEG_VIDEO_H264_PROFILE_EXTENDED ; +case V4L2_MPEG_VIDEO_H264_PROFILE_HIGH : +return VIRTIO_MPEG_VIDEO_H264_PROFILE_HIGH ; +case V4L2_MPEG_VIDEO_H264_PROFILE_HIGH_10 : +return VIRTIO_MPEG_VIDEO_H264_PROFILE_HIGH_10 ; +case V4L2_MPEG_VIDEO_H264_PROFILE_HIGH_422 : +return VIRTIO_MPEG_VIDEO_H264_PROFILE_HIGH_422 ; +case V4L2_MPEG_VIDEO_H264_PROFILE_HIGH_444_PREDICTIVE : +return VIRTIO_MPEG_VIDEO_H264_PROFILE_HIGH_444_PREDICTIVE ; +case V4L2_MPEG_VIDEO_H264_PROFILE_HIGH_10_INTRA : +return VIRTIO_MPEG_VIDEO_H264_PROFILE_HIGH_10_INTRA ; +case V4L2_MPEG_VIDEO_H264_PROFILE_HIGH_422_INTRA : +return VIRTIO_MPEG_VIDEO_H264_PROFILE_HIGH_422_INTRA ; +case V4L2_MPEG_VIDEO_H264_PROFILE_HIGH_444_INTRA : +return VIRTIO_MPEG_VIDEO_H264_PROFILE_HIGH_444_INTRA ; +case V4L2_MPEG_VIDEO_H264_PROFILE_CAVLC_444_INTRA : +return VIRTIO_MPEG_VIDEO_H264_PROFILE_CAVLC_444_INTRA ; +case V4L2_MPEG_VIDEO_H264_PROFILE_SCALABLE_BASELINE : +return VIRTIO_MPEG_VIDEO_H264_PROFILE_SCALABLE_BASELINE ; +case V4L2_MPEG_VIDEO_H264_PROFILE_SCALABLE_HIGH : +return VIRTIO_MPEG_VIDEO_H264_PROFILE_SCALABLE_HIGH ; +case V4L2_MPEG_VIDEO_H264_PROFILE_SCALABLE_HIGH_INTRA : +return VIRTIO_MPEG_VIDEO_H264_PROFILE_SCALABLE_HIGH_INTRA ; +case V4L2_MPEG_VIDEO_H264_PROFILE_STEREO_HIGH : +return VIRTIO_MPEG_VIDEO_H264_PROFILE_STEREO_HIGH ; +case V4L2_MPEG_VIDEO_H264_PROFILE_MULTIVIEW_HIGH : +return VIRTIO_MPEG_VIDEO_H264_PROFILE_MULTIVIEW_HIGH ; +default : +return VIRTIO_MPEG_VIDEO_H264_PROFILE_UNDEFINED ; + } +} + +uint32_t virtio_video_v4l2_level_to_virtio (uint32_t v4l2_level) +{ +switch (v4l2_level) { +case V4L2_MPEG_VIDEO_H264_LEVEL_1_0 : +return VIRTIO_MPEG_VIDEO_H264_LEVEL_1_0 ; +case V4L2_MPEG_VIDEO_H264_LEVEL_2_0 : +return VIRTIO_MPEG_VIDEO_H264_LEVEL_2_0 ; +case V4L2_MPEG_VIDEO_H264_LEVEL_4_0 : +return VIRTIO_MPEG_VIDEO_H264_LEVEL_4_0 ; +case V4L2_MPEG_VIDEO_H264_LEVEL_4_1 : +return VIRTIO_MPEG_VIDEO_H264_LEVEL_4_1 ; +case V4L2_MPEG_VIDEO_H264_LEVEL_4_2 : +return VIRTIO_MPEG_VIDEO_H264_LEVEL_4_2 ; +case V4L2_MPEG_VIDEO_H264_LEVEL_5_0 : +return VIRTIO_MPEG_VIDEO_H264_LEVEL_5_0 ; +case V4L2_MPEG_VIDEO_H264_LEVEL_5_1 : +return VIRTIO_MPEG_VIDEO_H264_LEVEL_5_1 ; +default : +return VIRTIO_MPEG_VIDEO_H264_LEVEL_UNDEFINED ; + } +} + +uint32_t virtio_video_format_to_v4l2 (uint32_t pixel_format) +{ + +switch (pixel_format) { +case VIRTIO_VIDEO_PIX_FMT_H264 : +return V4L2_PIX_FMT_H264 ; +case VIRTIO_VIDEO_PIX_FMT_H265 : +return V4L2_PIX_FMT_HEVC ; +case VIRTIO_VIDEO_PIX_FMT_MPEG4 : +return V4L2_PIX_FMT_MPEG4 ; +case VIRTIO_VIDEO_PIX_FMT_MPEG2 : +return V4L2_PIX_FMT_MPEG2 ; +case VIRTIO_VIDEO_PIX_FMT_NV12 : +return V4L2_PIX_FMT_NV12 ; +case VIRTIO_VIDEO_PIX_FMT_I420 : +return V4L2_PIX_FMT_YUV420 ; +default : +return 0 ; + } +} + +uint32_t virtio_video_v4l2_fourcc_to_virtio (uint32_t fourcc) +{ +switch (fourcc) { +case V4L2_PIX_FMT_H264 : +return VIRTIO_VIDEO_PIX_FMT_H264 ; +case V4L2_PIX_FMT_HEVC : +return VIRTIO_VIDEO_PIX_FMT_H265 ; +case V4L2_PIX_FMT_MPEG4 : +return VIRTIO_VIDEO_PIX_FMT_MPEG4 ; +case V4L2_PIX_FMT_MPEG2 : +return VIRTIO_VIDEO_PIX_FMT_MPEG2 ; +case V4L2_PIX_FMT_NV12 : +return VIRTIO_VIDEO_PIX_FMT_NV12 ; +case V4L2_PIX_FMT_YUV420 : +return VIRTIO_VIDEO_PIX_FMT_I420 ; +default : +return VIRTIO_VIDEO_PIX_FMT_UNKNOWN ; + } +} + +static int virtio_video_device_init (struct virtio_video_device *vvd) +{ +int ret =0 ; +const char *device_name =NULL ; +struct video_device *vd =NULL ; +struct virtio_video *vv =NULL ; + +if (!vvd) +return -EINVAL ; + + vd = &vvd->video_dev; + vv = vvd->vv; + +switch (vvd->type) { +case VIRTIO_VIDEO_FUNC_ENCODER : + device_name ="stateful-encoder" ; + ret =virtio_video_enc_init (vvd); +break ; +case VIRTIO_VIDEO_FUNC_DECODER : + device_name ="stateful-decoder" ; + ret =virtio_video_dec_init (vvd); +break ; +case VIRTIO_VIDEO_FUNC_PROCESSOR : +case VIRTIO_VIDEO_FUNC_CAPTURE : +case VIRTIO_VIDEO_FUNC_OUTPUT : +default : + ret = -EINVAL ; +break ; + } + +if (ret) { +v4l2_err (&vv->v4l2_dev,"failed to init device type" ); +return ret; + } + + ret =video_register_device (vd,VFL_TYPE_GRABBER ,0 ); +if (ret) { +v4l2_err (&vv->v4l2_dev,"failed to register video device\n" ); +return ret; + } + + vvd->workqueue =alloc_ordered_workqueue (vd->name, +WQ_MEM_RECLAIM |WQ_FREEZABLE ); +if (!vvd->workqueue) { +v4l2_err (&vv->v4l2_dev,"failed to create a workqueue" ); +video_unregister_device (vd); +return -ENOMEM ; + } + +list_add (&vvd->devices_list_entry, &vv->devices_list); +v4l2_info (&vv->v4l2_dev,"Device '%s' registered as /dev/video%d\n" , + device_name, vd->num); + +return 0 ; +} + +static void virtio_video_device_deinit (struct virtio_video_device *vvd) +{ +if (!vvd) +return ; + +list_del (&vvd->devices_list_entry); +flush_workqueue (vvd->workqueue); +destroy_workqueue (vvd->workqueue); +video_unregister_device (&vvd->video_dev); +} + +static struct virtio_video_device * +virtio_video_device_create (struct virtio_video *vv) +{ +struct device *dev =NULL ; +struct video_device *vd =NULL ; +struct v4l2_m2m_dev *m2m_dev =NULL ; +struct virtio_video_device *vvd =NULL ; + +if (!vv) +return ERR_PTR (-EINVAL ); + + dev = &vv->vdev->dev; + + vvd =devm_kzalloc (dev,sizeof (*vvd),GFP_KERNEL ); +if (!vvd) +return ERR_PTR (-ENOMEM ); + + m2m_dev =v4l2_m2m_init (&virtio_video_device_m2m_ops); +if (IS_ERR (m2m_dev)) { +v4l2_err (&vv->v4l2_dev,"failed to init m2m device\n" ); + goto err; + } + + vvd->vv = vv; + vvd->m2m_dev = m2m_dev; +mutex_init (&vvd->video_dev_mutex); + vd = &vvd->video_dev; + vd->lock = &vvd->video_dev_mutex; + vd->v4l2_dev = &vv->v4l2_dev; + vd->vfl_dir =VFL_DIR_M2M ; + vd->ioctl_ops =NULL ; + vd->fops = &virtio_video_device_fops; + vd->device_caps =V4L2_CAP_STREAMING |V4L2_CAP_VIDEO_M2M_MPLANE ; + vd->release = video_device_release_empty; + +memset (vd->name,0 ,sizeof (vd->name)); + +video_set_drvdata (vd, vvd); + +INIT_LIST_HEAD (&vvd->ctrl_caps_list); +INIT_LIST_HEAD (&vvd->fmt_caps_list); + + vvd->num_output_formats =0 ; + vvd->num_capture_formats =0 ; + +return vvd; + +err: +devm_kfree (dev, vvd); + +return ERR_CAST (m2m_dev); +} + +void virtio_video_device_destroy (struct virtio_video_device *vvd) +{ +if (!vvd) +return ; + +v4l2_m2m_release (vvd->m2m_dev); +devm_kfree (&vvd->vv->vdev->dev, vvd); +} + +int virtio_video_devices_init (struct virtio_video *vv,void *funcs_buf) +{ +int ret =0 ; +int fun_idx =0 ; +size_t offset =0 ; + +if (!vv || !funcs_buf) +return -EINVAL ; + +for (fun_idx =0 ; fun_idx < vv->num_devices; fun_idx++) { +struct virtio_video_device *vvd =NULL ; +size_t func_size =0 ; + + vvd =virtio_video_device_create (vv); +if (IS_ERR (vvd)) { +v4l2_err (&vv->v4l2_dev, +"failed to create virtio video device\n" ); + ret =PTR_ERR (vvd); + goto failed; + } + + func_size =virtio_video_parse_virtio_function (funcs_buf + + offset, vvd); +if (func_size ==0 ) { +v4l2_err (&vv->v4l2_dev,"failed to parse a function\n" ); +virtio_video_device_destroy (vvd); + ret = -EINVAL ; + goto failed; + } + offset += func_size; + + ret =virtio_video_device_init (vvd); +if (ret !=0 ) { +v4l2_err (&vv->v4l2_dev, +"failed to init virtio video device\n" ); +virtio_video_clean_virtio_function (vvd); +virtio_video_device_destroy (vvd); + goto failed; + } + } + +return 0 ; + +failed: +virtio_video_devices_deinit (vv); + +return ret; +} + +void virtio_video_devices_deinit (struct virtio_video *vv) +{ +struct virtio_video_device *vvd =NULL , *tmp =NULL ; + +list_for_each_entry_safe (vvd, tmp, &vv->devices_list, + devices_list_entry) { +virtio_video_device_deinit (vvd); +virtio_video_clean_virtio_function (vvd); +virtio_video_device_destroy (vvd); + } +}drivers/media/virtio/virtio_video_driver.c new file +278
-0@@ -0,0 +1,278 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* +* Driver for virtio video device. +* +* Copyright 2019 OpenSynergy GmbH. +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation; either version 2 of the License, or +* (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, see <http://www.gnu.org/licenses/>. +*/ + +#include <linux/module.h> +#include <linux/version.h> +#include <linux/dma-mapping.h> + +#include "virtio_video.h" + +static unsigned int debug; +module_param (debug, uint,0644 ); + +static void virtio_video_init_vq (struct virtio_video_queue *vvq, +void (*work_func)(struct work_struct *work)) +{ +spin_lock_init (&vvq->qlock); +init_waitqueue_head (&vvq->ack_queue); +INIT_WORK (&vvq->dequeue_work, work_func); +} + +static void *dma_phys_alloc (struct device *dev,size_t size, +dma_addr_t *dma_handle,gfp_t gfp, +unsigned long attrs) +{ +void *ret; + + ret = (void *)__get_free_pages (gfp,get_order (size)); +if (ret) + *dma_handle =virt_to_phys (ret) -PFN_PHYS (dev->dma_pfn_offset); + +return ret; +} + +static void dma_phys_free (struct device *dev,size_t size, +void *cpu_addr,dma_addr_t dma_addr, +unsigned long attrs) +{ +free_pages ((unsigned long )cpu_addr,get_order (size)); +} + +static dma_addr_t dma_phys_map_page (struct device *dev,struct page *page, +unsigned long offset,size_t size, +enum dma_data_direction dir, +unsigned long attrs) +{ +return page_to_phys (page) + offset -PFN_PHYS (dev->dma_pfn_offset); +} + +static int dma_phys_map_sg (struct device *dev,struct scatterlist *sgl, +int nents,enum dma_data_direction dir, +unsigned long attrs) +{ +int i; +struct scatterlist *sg; + +for_each_sg (sgl, sg, nents, i) { +dma_addr_t offset =PFN_PHYS (dev->dma_pfn_offset); +void *va; + +BUG_ON (!sg_page (sg)); + va =sg_virt (sg); +sg_dma_address (sg) = (dma_addr_t )virt_to_phys (va) - offset; +sg_dma_len (sg) = sg->length; + } + +return nents; +} + +const struct dma_map_ops dma_phys_ops = { + .alloc = dma_phys_alloc, + .free = dma_phys_free, + .map_page = dma_phys_map_page, + .map_sg = dma_phys_map_sg, +}; + +static int virtio_video_init (struct virtio_video *vv) +{ +int ret =0 ; +void *resp_buf =NULL ; +void *funcs_buf =NULL ; +size_t total_resp_size =0 ; + +if (!vv) +return -EINVAL ; + + total_resp_size = vv->funcs_size + +sizeof (struct virtio_video_get_functions ); + resp_buf =kzalloc (total_resp_size,GFP_KERNEL ); +if (IS_ERR (resp_buf)) +return -ENOMEM ; + + ret =virtio_video_req_funcs (vv, resp_buf, total_resp_size); +if (ret) { +v4l2_err (&vv->v4l2_dev, +"failed to get devices from the host\n" ); + goto err; + } + + ret =wait_event_timeout (vv->wq, vv->got_funcs,5 *HZ ); +if (ret ==0 ) { +v4l2_err (&vv->v4l2_dev,"timed out waiting for get_funcs\n" ); + ret = -EIO ; + goto err; + } + + funcs_buf = resp_buf +sizeof (struct virtio_video_ctrl_hdr ); + ret =virtio_video_devices_init (vv, funcs_buf); +if (ret) { +v4l2_err (&vv->v4l2_dev,"failed to initialize devices\n" ); + goto err; + } + +kfree (resp_buf); +return 0 ; + +err: +kfree (resp_buf); +return ret; +}; + +static int virtio_video_probe (struct virtio_device *vdev) +{ +int ret; +struct virtio_video *vv; +struct virtqueue *vqs[2 ]; +struct device *dev = &vdev->dev; + +static const char *const names[] = {"control" ,"event" }; +static vq_callback_t *callbacks[] = { + virtio_video_ctrl_ack, + virtio_video_event_ack + }; + + vv =devm_kzalloc (dev,sizeof (*vv),GFP_KERNEL ); +if (!vv) +return -ENOMEM ; + vv->vdev = vdev; + vv->debug = debug; + vdev->priv = vv; + +spin_lock_init (&vv->resource_idr_lock); +idr_init (&vv->resource_idr); +spin_lock_init (&vv->stream_idr_lock); +idr_init (&vv->stream_idr); + +init_waitqueue_head (&vv->wq); + + vv->has_iommu = !virtio_has_iommu_quirk (vdev); +if (!vv->has_iommu) +set_dma_ops (dev, &dma_phys_ops); + +dev_set_name (dev,DRIVER_NAME ); + ret =v4l2_device_register (dev, &vv->v4l2_dev); +if (ret) + goto err_v4l2_reg; + +virtio_video_init_vq (&vv->ctrlq, virtio_video_dequeue_ctrl_func); +virtio_video_init_vq (&vv->eventq, virtio_video_dequeue_event_func); + + ret =virtio_find_vqs (vdev,2 , vqs, callbacks, names,NULL ); +if (ret) { +v4l2_err (&vv->v4l2_dev,"failed to find virt queues\n" ); + goto err_vqs; + } + + vv->ctrlq.vq = vqs[0 ]; + vv->eventq.vq = vqs[1 ]; + + ret =virtio_video_alloc_vbufs (vv); +if (ret) { +v4l2_err (&vv->v4l2_dev,"failed to alloc vbufs\n" ); + goto err_vbufs; + } + + virtio_cread(vdev, struct virtio_video_config, total_functions_size, + &vv->funcs_size); +if (!vv->funcs_size) { +v4l2_err (&vv->v4l2_dev,"virtio_functions_size is zero\n" ); + ret = -EINVAL ; + goto err_config; + } + + virtio_cread(vdev, struct virtio_video_config, num_functions, + &vv->num_devices); +if (!vv->num_devices) { +v4l2_err (&vv->v4l2_dev,"num_devices is zero\n" ); + ret = -EINVAL ; + goto err_config; + } + + ret =virtio_video_alloc_events (vv, vv->eventq.vq->num_free); +if (ret) + goto err_events; + +virtio_device_ready (vdev); + vv->vq_ready = true; + vv->got_funcs = false; + +INIT_LIST_HEAD (&vv->devices_list); + + ret =virtio_video_init (vv); +if (ret) { +v4l2_err (&vv->v4l2_dev, +"failed to init virtio video\n" ); + goto err_init; + } + +return 0 ; + +err_init: +err_events: +err_config: +virtio_video_free_vbufs (vv); +err_vbufs: + vdev->config->del_vqs (vdev); +err_vqs: +v4l2_device_unregister (&vv->v4l2_dev); +err_v4l2_reg: +devm_kfree (&vdev->dev, vv); + +return ret; +} + +static void virtio_video_remove (struct virtio_device *vdev) +{ +struct virtio_video *vv = vdev->priv; + +virtio_video_devices_deinit (vv); +virtio_video_free_vbufs (vv); + vdev->config->del_vqs (vdev); +v4l2_device_unregister (&vv->v4l2_dev);Lines of this file are not shown. View the raw message.
drivers/media/virtio/virtio_video_enc.c new file +1124
-0Lines of this file are not shown. View the raw message.
drivers/media/virtio/virtio_video_enc.h new file +30
-0Lines of this file are not shown. View the raw message.
drivers/media/virtio/virtio_video_vq.c new file +950
-0Lines of this file are not shown. View the raw message.
include/uapi/linux/virtio_video.h new file +456
-1Lines of this file are not shown. View the raw message.
The rest of this diff is not shown. View the raw message.
2.24.0
________________________________________
From: Dmitry Sepp
Sent: Thursday, December 05, 2019 17:11
To: [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: [RFC] virtio video driverHello,
My apologies for the long delay. The driver code is now available and provided as a follow-up to the discussion from this thread [1].
The reference Linux kernel 5.4 driver implementation is located here:
https://github.com/OpenSynergy/linux/tree/virtio-video-draft-v1The driver is implemented using the V4L2 API. It allocates a v4l2 device for each probed virtio device and then creates a video device for each function within the respective virtio device. The driver implements the stateful decoder interface [2] and the stateful encoder interface (WIP) [3].
The DMA SG memory allocator tries to map buffers right away. As it is not always suitable, and some implementations might need just a physical address, we had to introduce a set of simple dma ops directly in the driver.
The driver is in the RFC state and currently a bit ahead of the spec that was proposed in the discussion mentioned above. On the other hand, the driver unfortunately does not yet include changes proposed in the recent comments [4]. The driver currently supports encoder and decoder functions. Also, it does not fully pass the v4l2-compliance yet, it has been a bit out of the focus so far.
Any feedback and contribution would be greatly appreciated.
Quoted 8 lines
[1] https://markmail.org/message/gc6h25acct22niut
[2] https://www.kernel.org/doc/html/v5.4/media/uapi/v4l/dev-decoder.html
[3] https://hverkuil.home.xs4all.nl/codec-api/uapi/v4l/dev-encoder.html
[4] https://markmail.org/message/yy67elx2adbivdspBest regards,
Dmitry.
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Part of the diff is not shown here. View the raw message.
Hi Dmitry,
Thanks for sharing the driver implementation.
Since the virtio protocol specification is still under discussion in virtio-dev@ ML separately, let me add comments only about implementation that are irrelevant to the protocol details. Please find my comments inline.
Quoted 76 lines · Dmitry Sepp
From: Kiran Pawar <[email protected]>
This adds a Virtio based video driver for video streaming device that operates input and/or output data buffers to share video devices with several guests. The current implementation consist of V4L2 based video driver supporting video functions of decoder and encoder. This can be extended for other functions e.g. processor, capture and output. The device uses descriptor structures to advertise and negotiate stream formats and controls. This allows the driver to modify the processing logic of the device on a per stream basis.
Signed-off-by Dmitry Sepp <[email protected]>Signed-off-by Kiran Pawar <[email protected]>Signed-off-by Nikolay Martyanov <[email protected]>--- drivers/media/Kconfig | 1 + drivers/media/Makefile | 2 + drivers/media/virtio/Kconfig | 11 + drivers/media/virtio/Makefile | 12 + drivers/media/virtio/virtio_video.h | 372 +++++++ drivers/media/virtio/virtio_video_caps.c | 618 +++++++++++ drivers/media/virtio/virtio_video_dec.c | 947 +++++++++++++++++ drivers/media/virtio/virtio_video_dec.h | 30 + drivers/media/virtio/virtio_video_device.c | 747 +++++++++++++ drivers/media/virtio/virtio_video_driver.c | 278 +++++ drivers/media/virtio/virtio_video_enc.c | 1124 ++++++++++++++++++++ drivers/media/virtio/virtio_video_enc.h | 30 + drivers/media/virtio/virtio_video_vq.c | 950 +++++++++++++++++ include/uapi/linux/virtio_ids.h | 2 + include/uapi/linux/virtio_video.h | 456 ++++++++ 15 files changed, 5580 insertions(+) create mode 100644 drivers/media/virtio/Kconfig create mode 100644 drivers/media/virtio/Makefile create mode 100644 drivers/media/virtio/virtio_video.h create mode 100644 drivers/media/virtio/virtio_video_caps.c create mode 100644 drivers/media/virtio/virtio_video_dec.c create mode 100644 drivers/media/virtio/virtio_video_dec.h create mode 100644 drivers/media/virtio/virtio_video_device.c create mode 100644 drivers/media/virtio/virtio_video_driver.c create mode 100644 drivers/media/virtio/virtio_video_enc.c create mode 100644 drivers/media/virtio/virtio_video_enc.h create mode 100644 drivers/media/virtio/virtio_video_vq.c create mode 100644 include/uapi/linux/virtio_video.hdrivers/media/Kconfig +1
-0@@ -170,6 +170,7 @@ source "drivers/media/pci/Kconfig" source "drivers/media/platform/Kconfig" source "drivers/media/mmc/Kconfig" source "drivers/media/radio/Kconfig" +source "drivers/media/virtio/Kconfig" comment "Supported FireWire (IEEE 1394) Adapters" depends on DVB_CORE && FIREWIREdrivers/media/Makefile +2
-0@@ -25,6 +25,8 @@ obj-y += rc/ obj-$(CONFIG_CEC_CORE) += cec/ +obj-$(CONFIG_VIDEO_VIRTIO) += virtio/ + # # Finally, merge the drivers that require the core #drivers/media/virtio/Kconfig new file +0
-0
drivers/media/virtio/Kconfig line 4
Quoted 4 lines
drivers/media/virtio/Kconfig new file +4
-0@@ -0,0 +1,11 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Video driver for virtio + +config VIDEO_VIRTIO
"VIRTIO_VIDEO" would be better like VIRTIO_NET and VIRTIO_PCI.
drivers/media/virtio/Kconfig line 8
Quoted 4 lines
drivers/media/virtio/Kconfig new file +4
-0+ tristate "Virtio video V4L2 driver" + depends on VIRTIO && VIDEO_DEV && VIDEO_V4L2 + select VIDEOBUF2_DMA_SG + select V4L2_MEM2MEM_DEV
I suppose you also need "select VIDEOBUF2_V4L2", as struct vb2_v4l2_buffer is used.
Quoted 58 lines
drivers/media/virtio/Kconfig new file +3
-0+ help + This is the virtual video driver for virtio. + Say Y or M.drivers/media/virtio/Makefile new file +12
-0@@ -0,0 +1,12 @@ +# SPDX-License-Identifier: GPL-2.0+ + +obj-$(CONFIG_VIDEO_VIRTIO) += virtio-video.o + +virtio-video-objs := \ + virtio_video_driver.o \ + virtio_video_vq.o \ + virtio_video_device.o \ + virtio_video_dec.o \ + virtio_video_enc.o \ + virtio_video_caps.o +drivers/media/virtio/virtio_video.h new file +31
-0@@ -0,0 +1,372 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* Common header for virtio video driver. +* +* Copyright 2019 OpenSynergy GmbH. +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation; either version 2 of the License, or +* (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, see <http://www.gnu.org/licenses/>. +*/ + +#ifndef _VIRTIO_VIDEO_H +#define _VIRTIO_VIDEO_H + +#include <linux/virtio.h> +#include <linux/virtio_ids.h> +#include <linux/virtio_config.h> +#include <linux/virtio_video.h> +#include <linux/list.h> +#include <media/v4l2-device.h> +#include <media/v4l2-mem2mem.h> +#include <media/v4l2-ctrls.h> +
drivers/media/virtio/virtio_video.h line 41
Quoted 10 lines
drivers/media/virtio/virtio_video.h new file +10
-0+#define DRIVER_NAME "virtio-video" + +#ifndef VIRTIO_ID_VIDEO +#define VIRTIO_ID_VIDEO 29 +#endif + +enum video_pin_type { +VIDEO_PIN_TYPE_INPUT =0 , +VIDEO_PIN_TYPE_OUTPUT , +};
Why don't you use enums and structs defined in /include/uapi/linux/virtio_video.h?
For example, I suppose we don't need this video_pin_type, as we can use enum virtio_video_pin_type defined there.
Same for other enums and structs in this file.
[snip]
Quoted 26 lines
drivers/media/virtio/virtio_video_dec.c new file +20
-0@@ -0,0 +1,947 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* Decoder for virtio video device. +* +* Copyright 2019 OpenSynergy GmbH. +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation; either version 2 of the License, or +* (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, see <http://www.gnu.org/licenses/>. +*/ + +#include <media/v4l2-event.h>
drivers/media/virtio/virtio_video_dec.c line 30
Quoted 10 lines
drivers/media/virtio/virtio_video_dec.c new file +10
-0+#include <media/v4l2-ioctl.h> +#include <media/videobuf2-dma-sg.h> + +#include "virtio_video.h" + +static int virtio_video_queue_setup (struct vb2_queue *vq, +unsigned int *num_buffers, +unsigned int *num_planes, +unsigned int sizes[], +struct device *alloc_devs[])
We have functions named "virtio_video_queue_setup" both in virtio_video_dec.c and virtio_video_enc.c.
How about calling them "virtio_video_{dec,enc}_queue_setup" or "virtvideo_{dec,enc}_queue_setup"?
Though the naming conflicts of static functions shouldn't cause any problem when building a kernel, indexing tools, such as ctags, should be confused.
Same for other functions.
[snip]
drivers/media/virtio/virtio_video_dec.c line 325
Quoted 10 lines
drivers/media/virtio/virtio_video_dec.c new file +10
-0+int virtio_video_init_dec_queues (void *priv,struct vb2_queue *src_vq, +struct vb2_queue *dst_vq) +{ +int ret; +struct virtio_video_stream *stream = priv; +struct virtio_video_device *vvd =to_virtio_vd (stream->video_dev); +struct device *dev = vvd->vv->v4l2_dev.dev; + + src_vq->type =V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE ; + src_vq->io_modes =VB2_MMAP |VB2_DMABUF ;
I suppose this driver doesn't support DMABUF mode yet. IIUC, VB_DMABUF should be specified only when a driver supports DMA buffer importing.
[snip]
Quoted 29 lines
drivers/media/virtio/virtio_video_dec.c new file +29
-0+static int virtio_video_s_fmt (struct virtio_video_stream *stream, +struct v4l2_format *f) +{ +int i, ret; +struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp; +struct virtio_video_device *vvd =to_virtio_vd (stream->video_dev); +struct virtio_video *vv = vvd->vv; +struct video_format_info info; +struct video_format_info *p_info; +enum video_pin_type pin =VIDEO_PIN_TYPE_INPUT ; + + ret =virtio_video_try_fmt (stream, f); +if (ret) +return ret; + + info.frame_width = pix_mp->width; + info.frame_height = pix_mp->height; + info.num_planes = pix_mp->num_planes; + info.fourcc_format = pix_mp->pixelformat; + +for (i =0 ; i < info.num_planes; i++) { + info.plane_format[i].stride = + pix_mp->plane_fmt[i].bytesperline; + info.plane_format[i].plane_size = + pix_mp->plane_fmt[i].sizeimage; + } + +if (!V4L2_TYPE_IS_OUTPUT (f->type)) + pin =VIDEO_PIN_TYPE_OUTPUT ;
drivers/media/virtio/virtio_video_dec.c line 856
Quoted 10 lines
drivers/media/virtio/virtio_video_dec.c new file +10
-0+ +virtio_video_req_set_params (vv, vvd->id, &info, pin, +VIDEO_PARAMS_SCOPE_STREAM , stream); + +virtio_video_req_get_params (vv, vvd->id,VIDEO_PIN_TYPE_INPUT , +VIDEO_PARAMS_SCOPE_STREAM , stream); + +virtio_video_req_get_params (vv, vvd->id,VIDEO_PIN_TYPE_OUTPUT , +VIDEO_PARAMS_SCOPE_STREAM , stream); +
nit: GET_PARAMS must be called for both queues after SET_PARAMS. So, how about defining a function to call these three functions?
[snip]
Quoted 7 lines
drivers/media/virtio/virtio_video_dec.c new file +7
-0+static int virtio_video_device_open (struct file *file) +{ +int ret; +uint32_t stream_id; +char name[TASK_COMM_LEN ]; +struct virtio_video_stream *stream; +struct video_device *video_dev =video_devdata (file);
drivers/media/virtio/virtio_video_dec.c line 96
Quoted 10 lines
drivers/media/virtio/virtio_video_dec.c new file +10
-0+struct virtio_video_device *vvd =video_drvdata (file); +struct virtio_video *vv = vvd->vv; + + stream =kzalloc (sizeof (*stream),GFP_KERNEL ); +if (!stream) +return -ENOMEM ; + +get_task_comm (name, current); +virtio_video_stream_id_get (vv, stream, &stream_id); + ret =virtio_video_req_stream_create (vv, vvd->id, stream_id, name);
It'd be better not to send virtio requests in open callback to avoid unnecessary overhead. For example, we often open and close /dev/videoX just to check its capabilities. In that case, we don't need to create a stream.
Instead, it'd make more sense to send the request when REQBUFS is called by passing a non-zero value as "count" and no stream is created yet.
Please see comments for stream_destroy in release callback, too.
Quoted 4 lines
drivers/media/virtio/virtio_video_dec.c new file +4
-0+if (ret) { +v4l2_err (&vv->v4l2_dev,"failed to create stream\n" ); + goto err_stream_create; + }
drivers/media/virtio/virtio_video_dec.c line 110
Quoted 10 lines
drivers/media/virtio/virtio_video_dec.c new file +10
-0+ + stream->video_dev = video_dev; + stream->stream_id = stream_id; + stream->state =STREAM_STATE_IDLE ; +mutex_init (&stream->vq_mutex); +INIT_WORK (&stream->work, virtio_video_worker); +v4l2_fh_init (&stream->fh, video_dev); + stream->fh.ctrl_handler = &stream->ctrl_handler; + +if (vvd->type ==VIRTIO_VIDEO_FUNC_DECODER ) {
nit: How about using switch statements? Same for other if-else statements for vvd->type.
[snip]
Quoted 5 lines
drivers/media/virtio/virtio_video_dec.c new file +5
-0+ +static int virtio_video_device_release (struct file *file) +{ +struct virtio_video_stream *stream =file2stream (file); +struct video_device *video_dev =video_devdata (file);
drivers/media/virtio/virtio_video_dec.c line 125
Quoted 10 lines
drivers/media/virtio/virtio_video_dec.c new file +10
-0+struct virtio_video_device *vvd =video_drvdata (file); +struct virtio_video *vv = vvd->vv; + +v4l2_fh_del (&stream->fh); +v4l2_fh_exit (&stream->fh); +mutex_lock (video_dev->lock); +v4l2_m2m_ctx_release (stream->fh.m2m_ctx); +mutex_unlock (video_dev->lock); + +virtio_video_req_stream_destroy (vv, vvd->id, stream->stream_id);
As with STREAM_CREATE requests, STREAM_DESTROY should be sent in REQBUFS callback as well.
We can call it in REQBUFS when "v4l2_requestbuffers.count" is 0 and an active stream already exists (i.e. stream_id > 0).
Still, it'd be good to keep STREAM_DESTROY here, as users can forget to free buffers via REQBUFS.
[snip]
drivers/media/virtio/virtio_video_dec.c line 132
Quoted 7 lines
drivers/media/virtio/virtio_video_dec.c new file +7
-0+void virtio_video_resource_id_get (struct virtio_video *vv,uint32_t *id) +{ +int handle; + +idr_preload (GFP_KERNEL ); +spin_lock (&vv->resource_idr_lock); + handle =idr_alloc (&vv->resource_idr,NULL ,1 ,0 ,GFP_NOWAIT );
We should check if idr_alloc returns no error here or caller-side.
[snip]
drivers/media/virtio/virtio_video_dec.c line 140
Quoted 8 lines
drivers/media/virtio/virtio_video_dec.c new file +8
-0+static int +virtio_video_queue_ctrl_buffer_locked (struct virtio_video *vv, +struct virtio_video_vbuffer *vbuf) +{ +struct virtqueue *vq = vv->ctrlq.vq; +struct scatterlist *sgs[3 ], vreq, vout, vresp; +int outcnt =0 , incnt =0 ; +int ret;
How about adding WARN_ON(!mutex_is_locked(&vv->ctrlq.qlock)) to prevent misuse and show which lock must be used?
Quoted 15 lines
drivers/media/virtio/virtio_video_dec.c new file +15
-0+ +if (!vv->vq_ready) +return -ENODEV ; + +sg_init_one (&vreq, vbuf->buf, vbuf->size); + sgs[outcnt + incnt] = &vreq; + outcnt++; + +if (vbuf->data_size) { +sg_init_one (&vout, vbuf->data_buf, vbuf->data_size); + sgs[outcnt + incnt] = &vout; + outcnt++; + } + +if (vbuf->resp_size) {
drivers/media/virtio/virtio_video_dec.c line 165
Quoted 10 lines
drivers/media/virtio/virtio_video_dec.c new file +10
-0+sg_init_one (&vresp, vbuf->resp_buf, vbuf->resp_size); + sgs[outcnt + incnt] = &vresp; + incnt++; + } + +retry: + ret =virtqueue_add_sgs (vq, sgs, outcnt, incnt, vbuf,GFP_ATOMIC ); +if (ret == -ENOSPC ) { +spin_unlock (&vv->ctrlq.qlock); +wait_event (vv->ctrlq.ack_queue, vq->num_free);
wait_event_timeout would be better to avoid getting stuck.
drivers/media/virtio/virtio_video_dec.c line 167
Quoted 2 lines
drivers/media/virtio/virtio_video_dec.c new file +2
-0+spin_lock (&vv->ctrlq.qlock); + goto retry;
I'd use while-loop instead of goto.
[snip]
drivers/media/virtio/virtio_video_dec.c line 178
Quoted 11 lines
drivers/media/virtio/virtio_video_dec.c new file +11
-0+int virtio_video_req_stream_create (struct virtio_video *vv, +uint32_t function_id,uint32_t stream_id, +const char *name) +{ +struct virtio_video_stream_create *req_p; +struct virtio_video_vbuffer *vbuf; + + req_p =virtio_video_alloc_req (vv, &vbuf,sizeof (*req_p)); +if (IS_ERR (req_p)) +return PTR_ERR (req_p); +memset (req_p,0 ,sizeof (*req_p));
I'm wondering if we don't need to call memset here, as virtio_video_get_vbuf calls memset for vbuf.
If it's not true, it'd make sense to call memset in virtio_video_alloc_req instead because we always want to have zero-initialized structs.
[snip]
Best regards,
Keiichi
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Hi Keiichi,
Quoted 9 lines · On Mittwoch, 25. Dezember 2019 12:38:55 CET Keiichi Watanabe wrote:
Hi Dmitry,
Thanks for sharing the driver implementation.
Since the virtio protocol specification is still under discussion in virtio-dev@ ML separately, let me add comments only about implementation that are irrelevant to the protocol details. Please find my comments inline.On Fri, Dec 6, 2019 at 1:33 AM Dmitry Sepp <[email protected]>
Quoted 76 lines · wrote:
Quoted 76 lines
From: Kiran Pawar <[email protected]>
This adds a Virtio based video driver for video streaming device that operates input and/or output data buffers to share video devices with several guests. The current implementation consist of V4L2 based video driver supporting video functions of decoder and encoder. This can be extended for other functions e.g. processor, capture and output. The device uses descriptor structures to advertise and negotiate stream formats and controls. This allows the driver to modify the processing logic of the device on a per stream basis.
Signed-off-by Dmitry Sepp <[email protected]>Signed-off-by Kiran Pawar <[email protected]>Signed-off-by Nikolay Martyanov <[email protected]>---
drivers/media/Kconfig | 1 + drivers/media/Makefile | 2 + drivers/media/virtio/Kconfig | 11 + drivers/media/virtio/Makefile | 12 + drivers/media/virtio/virtio_video.h | 372 +++++++ drivers/media/virtio/virtio_video_caps.c | 618 +++++++++++ drivers/media/virtio/virtio_video_dec.c | 947 +++++++++++++++++ drivers/media/virtio/virtio_video_dec.h | 30 + drivers/media/virtio/virtio_video_device.c | 747 +++++++++++++ drivers/media/virtio/virtio_video_driver.c | 278 +++++ drivers/media/virtio/virtio_video_enc.c | 1124 ++++++++++++++++++++ drivers/media/virtio/virtio_video_enc.h | 30 + drivers/media/virtio/virtio_video_vq.c | 950 +++++++++++++++++ include/uapi/linux/virtio_ids.h | 2 + include/uapi/linux/virtio_video.h | 456 ++++++++ 15 files changed, 5580 insertions(+) create mode 100644 drivers/media/virtio/Kconfig create mode 100644 drivers/media/virtio/Makefile create mode 100644 drivers/media/virtio/virtio_video.h create mode 100644 drivers/media/virtio/virtio_video_caps.c create mode 100644 drivers/media/virtio/virtio_video_dec.c create mode 100644 drivers/media/virtio/virtio_video_dec.h create mode 100644 drivers/media/virtio/virtio_video_device.c create mode 100644 drivers/media/virtio/virtio_video_driver.c create mode 100644 drivers/media/virtio/virtio_video_enc.c create mode 100644 drivers/media/virtio/virtio_video_enc.h create mode 100644 drivers/media/virtio/virtio_video_vq.c create mode 100644 include/uapi/linux/virtio_video.hdrivers/media/Kconfig +1
-0@@ -170,6 +170,7 @@ source "drivers/media/pci/Kconfig" source "drivers/media/platform/Kconfig" source "drivers/media/mmc/Kconfig" source "drivers/media/radio/Kconfig" +source "drivers/media/virtio/Kconfig"comment "Supported FireWire (IEEE 1394) Adapters"
depends on DVB_CORE && FIREWIREdrivers/media/Makefile +2
-0@@ -25,6 +25,8 @@ obj-y += rc/ obj-$(CONFIG_CEC_CORE) += cec/ +obj-$(CONFIG_VIDEO_VIRTIO) += virtio/ + # # Finally, merge the drivers that require the core#
Quoted 13 lines
drivers/media/virtio/Kconfig line 4
Quoted 10 lines
drivers/media/virtio/Kconfig new file +4
-0@@ -0,0 +1,11 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Video driver for virtio + +config VIDEO_VIRTIO"
VIRTIO_VIDEO" would be better likeVIRTIO_NETandVIRTIO_PCI.
Ok, I agree.
Quoted 8 lines
drivers/media/virtio/Kconfig line 8
Quoted 4 lines
drivers/media/virtio/Kconfig new file +4
-0+ tristate "Virtio video V4L2 driver" + depends on VIRTIO && VIDEO_DEV && VIDEO_V4L2 + select VIDEOBUF2_DMA_SG + select V4L2_MEM2MEM_DEVI suppose you also need "select
VIDEOBUF2_V4L2", as structvb2_v4l2_bufferis used.
It is selected automatically by VIDEO_V4L2. Also, there are no drivers that set it explicitly.
Quoted 66 lines
Quoted 66 lines
drivers/media/virtio/Kconfig new file +3
-0+ help + This is the virtual video driver for virtio. + Say Y or M.drivers/media/virtio/Makefile new file +12
-0@@ -0,0 +1,12 @@ +# SPDX-License-Identifier: GPL-2.0+ + +obj-$(CONFIG_VIDEO_VIRTIO) += virtio-video.o + +virtio-video-objs := \ + virtio_video_driver.o \ + virtio_video_vq.o \ + virtio_video_device.o \ + virtio_video_dec.o \ + virtio_video_enc.o \ + virtio_video_caps.o +(quoted diff) +0
-0
b/drivers/media/virtio/virtio_video.hnew file mode 100644 index 000000000000..23c77dc0cb93drivers/media/virtio/virtio_video.h new file +39
-0@@ -0,0 +1,372 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* Common header for virtio video driver. +* +* Copyright 2019 OpenSynergy GmbH. +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation; either version 2 of the License, or +* (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, see <http://www.gnu.org/licenses/>. +*/ + +#ifndef _VIRTIO_VIDEO_H +#define _VIRTIO_VIDEO_H + +#include <linux/virtio.h> +#include <linux/virtio_ids.h> +#include <linux/virtio_config.h> +#include <linux/virtio_video.h> +#include <linux/list.h> +#include <media/v4l2-device.h> +#include <media/v4l2-mem2mem.h> +#include <media/v4l2-ctrls.h> + +#define DRIVER_NAME "virtio-video" + +#ifndef VIRTIO_ID_VIDEO +#define VIRTIO_ID_VIDEO 29 +#endif + +enum video_pin_type { + VIDEO_PIN_TYPE_INPUT = 0,
Quoted 8 lines
drivers/media/virtio/virtio_video.h line 41
Quoted 2 lines
drivers/media/virtio/virtio_video.h new file +2
-0+ VIDEO_PIN_TYPE_OUTPUT, +};Why don't you use enums and structs defined in /include/uapi/linux/virtio_video.h?
For example, I suppose we don't need thisvideo_pin_type, as we can use enumvirtio_video_pin_typedefined there.
Same for other enums and structs in this file.
The idea was to have some extra layer of parameter verification. But I also agree this might be excessive.
Quoted 48 lines
[snip]
Quoted 26 lines
(quoted diff) +0
-0
b/drivers/media/virtio/virtio_video_dec.cnew file mode 100644 index 000000000000..c2ad62229d21drivers/media/virtio/virtio_video_dec.c new file +20
-0@@ -0,0 +1,947 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* Decoder for virtio video device. +* +* Copyright 2019 OpenSynergy GmbH. +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation; either version 2 of the License, or +* (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, see <http://www.gnu.org/licenses/>. +*/ + +#include <media/v4l2-event.h> drivers/media/virtio/virtio_video_dec.c line 30
Quoted 10 lines
drivers/media/virtio/virtio_video_dec.c new file +10
-0+#include <media/v4l2-ioctl.h> +#include <media/videobuf2-dma-sg.h> + +#include "virtio_video.h" + +static int virtio_video_queue_setup (struct vb2_queue *vq, +unsigned int *num_buffers, +unsigned int *num_planes, +unsigned int sizes[], +struct device *alloc_devs[])We have functions named "
virtio_video_queue_setup" both invirtio_video_dec.candvirtio_video_enc.c.
How about calling them "virtio_video_{dec,enc}_queue_setup" or "virtvideo_{dec,enc}_queue_setup"?
Though the naming conflicts of static functions shouldn't cause any problem when building a kernel, indexing tools, such as ctags, should be confused.
Quoted 1 line
Same for other functions.
There are some functions that simply can be moved to common code, we'll look into this as well.
Quoted 17 lines
[snip]
drivers/media/virtio/virtio_video_dec.c line 325
Quoted 10 lines
drivers/media/virtio/virtio_video_dec.c new file +10
-0+int virtio_video_init_dec_queues (void *priv,struct vb2_queue *src_vq, +struct vb2_queue *dst_vq) +{ +int ret; +struct virtio_video_stream *stream = priv; +struct virtio_video_device *vvd =to_virtio_vd (stream->video_dev); +struct device *dev = vvd->vv->v4l2_dev.dev; + + src_vq->type =V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE ; + src_vq->io_modes =VB2_MMAP |VB2_DMABUF ;I suppose this driver doesn't support DMABUF mode yet. IIUC,
VB_DMABUFshould be specified only when a driver supports DMA buffer importing.
We do support DMABUF. This is exactly how virtio-video works in Android with gralloc handles. But ideally the driver needs to know device restrictions and alloc accordingly. This is why we needed struct virtio_video_plane_format. I guess we'll need to get it back.
Quoted 38 lines
[snip]
Quoted 35 lines
drivers/media/virtio/virtio_video_dec.c new file +23
-0+static int virtio_video_s_fmt (struct virtio_video_stream *stream, +struct v4l2_format *f) +{ +int i, ret; +struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp; +struct virtio_video_device *vvd =to_virtio_vd (stream->video_dev); +struct virtio_video *vv = vvd->vv; +struct video_format_info info; +struct video_format_info *p_info; +enum video_pin_type pin =VIDEO_PIN_TYPE_INPUT ; + + ret =virtio_video_try_fmt (stream, f); +if (ret) +return ret; + + info.frame_width = pix_mp->width; + info.frame_height = pix_mp->height; + info.num_planes = pix_mp->num_planes; + info.fourcc_format = pix_mp->pixelformat; + +for (i =0 ; i < info.num_planes; i++) { + info.plane_format[i].stride = +pix_mp->plane_fmt[i].bytesperline;
drivers/media/virtio/virtio_video_dec.c new file +11
-0+ info.plane_format[i].plane_size = + pix_mp->plane_fmt[i].sizeimage; + } + +if (!V4L2_TYPE_IS_OUTPUT (f->type)) + pin =VIDEO_PIN_TYPE_OUTPUT ; + +virtio_video_req_set_params (vv, vvd->id, &info, pin, +VIDEO_PARAMS_SCOPE_STREAM , stream); + + virtio_video_req_get_params(vv, vvd->id, VIDEO_PIN_TYPE_INPUT,
Quoted 8 lines
drivers/media/virtio/virtio_video_dec.c line 856
Quoted 5 lines
drivers/media/virtio/virtio_video_dec.c new file +5
-0+ VIDEO_PARAMS_SCOPE_STREAM, stream); + +virtio_video_req_get_params (vv, vvd->id,VIDEO_PIN_TYPE_OUTPUT , +VIDEO_PARAMS_SCOPE_STREAM , stream); +nit:
GET_PARAMSmust be called for both queues afterSET_PARAMS. So, how about defining a function to call these three functions?
Yes, might be useful.
Quoted 27 lines
[snip]
Quoted 17 lines
drivers/media/virtio/virtio_video_dec.c new file +17
-0+static int virtio_video_device_open (struct file *file) +{ +int ret; +uint32_t stream_id; +char name[TASK_COMM_LEN ]; +struct virtio_video_stream *stream; +struct video_device *video_dev =video_devdata (file); +struct virtio_video_device *vvd =video_drvdata (file); +struct virtio_video *vv = vvd->vv; + + stream =kzalloc (sizeof (*stream),GFP_KERNEL ); +if (!stream) +return -ENOMEM ; + +get_task_comm (name, current); +virtio_video_stream_id_get (vv, stream, &stream_id); + ret = virtio_video_req_stream_create(vv, vvd->id, stream_id,Quoted 1 line
name);
It'd be better not to send virtio requests in open callback to avoid unnecessary overhead. For example, we often open and close
/dev/videoXjust to check its capabilities. In that case, we don't need to create a stream.
Quoted 5 lines
Instead, it'd make more sense to send the request when REQBUFS is called by passing a non-zero value as "count" and no stream is created yet.
Please see comments forstream_destroyin release callback, too.
We probably just don't need to open too often. I know udev does this once on boot. This is pretty much it.
Quoted 9 lines
Quoted 9 lines
drivers/media/virtio/virtio_video_dec.c new file +9
-0+if (ret) { +v4l2_err (&vv->v4l2_dev,"failed to create stream\n" ); + goto err_stream_create; + } + + stream->video_dev = video_dev; + stream->stream_id = stream_id; + stream->state =STREAM_STATE_IDLE ; +mutex_init (&stream->vq_mutex);
Quoted 8 lines
drivers/media/virtio/virtio_video_dec.c
Quoted 5 lines
drivers/media/virtio/virtio_video_dec.c new file +5
-0+INIT_WORK (&stream->work, virtio_video_worker); +v4l2_fh_init (&stream->fh, video_dev); + stream->fh.ctrl_handler = &stream->ctrl_handler; + +if (vvd->type ==VIRTIO_VIDEO_FUNC_DECODER ) {nit: How about using switch statements? Same for other if-else statements for
vvd->type.
I agree. It will look more neat.
Quoted 40 lines
[snip]
Quoted 5 lines
drivers/media/virtio/virtio_video_dec.c new file +5
-0+ +static int virtio_video_device_release (struct file *file) +{ +struct virtio_video_stream *stream =file2stream (file); +struct video_device *video_dev =video_devdata (file);drivers/media/virtio/virtio_video_dec.c
Quoted 10 lines
drivers/media/virtio/virtio_video_dec.c new file +10
-0+struct virtio_video_device *vvd =video_drvdata (file); +struct virtio_video *vv = vvd->vv; + +v4l2_fh_del (&stream->fh); +v4l2_fh_exit (&stream->fh); +mutex_lock (video_dev->lock); +v4l2_m2m_ctx_release (stream->fh.m2m_ctx); +mutex_unlock (video_dev->lock); + +virtio_video_req_stream_destroy (vv, vvd->id, stream->stream_id);As with
STREAM_CREATErequests,STREAM_DESTROYshould be sent in REQBUFS callback as well.
We can call it in REQBUFS when "v4l2_requestbuffers.count" is 0 and an active stream already exists (i.e.stream_id> 0).
Still, it'd be good to keepSTREAM_DESTROYhere, as users can forget to free buffers via REQBUFS.[snip]
drivers/media/virtio/virtio_video_dec.c
Quoted 7 lines
drivers/media/virtio/virtio_video_dec.c new file +7
-0+void virtio_video_resource_id_get (struct virtio_video *vv,uint32_t *id) +{ +int handle; + +idr_preload (GFP_KERNEL ); +spin_lock (&vv->resource_idr_lock); + handle =idr_alloc (&vv->resource_idr,NULL ,1 ,0 ,GFP_NOWAIT );We should check if
idr_allocreturns no error here or caller-side.[snip]
Quoted 11 lines
drivers/media/virtio/virtio_video_dec.c
Quoted 8 lines
drivers/media/virtio/virtio_video_dec.c new file +8
-0+static int +virtio_video_queue_ctrl_buffer_locked (struct virtio_video *vv, +struct virtio_video_vbuffer *vbuf) +{ +struct virtqueue *vq = vv->ctrlq.vq; +struct scatterlist *sgs[3 ], vreq, vout, vresp; +int outcnt =0 , incnt =0 ; +int ret;How about adding WARN_ON(!mutex_is_locked(&vv->ctrlq.qlock)) to prevent misuse and show which lock must be used?
Makes sense.
Quoted 20 lines
Quoted 20 lines
drivers/media/virtio/virtio_video_dec.c new file +19
-0+ +if (!vv->vq_ready) +return -ENODEV ; + +sg_init_one (&vreq, vbuf->buf, vbuf->size); + sgs[outcnt + incnt] = &vreq; + outcnt++; + +if (vbuf->data_size) { +sg_init_one (&vout, vbuf->data_buf, vbuf->data_size); + sgs[outcnt + incnt] = &vout; + outcnt++; + } + +if (vbuf->resp_size) { +sg_init_one (&vresp, vbuf->resp_buf, vbuf->resp_size); + sgs[outcnt + incnt] = &vresp; + incnt++; + }
Quoted 8 lines
drivers/media/virtio/virtio_video_dec.c
Quoted 6 lines
drivers/media/virtio/virtio_video_dec.c new file +6
-0+ +retry: + ret =virtqueue_add_sgs (vq, sgs, outcnt, incnt, vbuf,GFP_ATOMIC ); +if (ret == -ENOSPC ) { +spin_unlock (&vv->ctrlq.qlock); +wait_event (vv->ctrlq.ack_queue, vq->num_free);
wait_event_timeoutwould be better to avoid getting stuck.
True.
Quoted 6 lines
drivers/media/virtio/virtio_video_dec.c
Quoted 3 lines
drivers/media/virtio/virtio_video_dec.c new file +2
-0+spin_lock (&vv->ctrlq.qlock); + goto retry;I'd use while-loop instead of goto.
The code was derived from virtio-gpu. Doesn't make too much difference in the end.
Quoted 11 lines
[snip]
Quoted 9 lines
drivers/media/virtio/virtio_video_dec.c new file +2
-0+int virtio_video_req_stream_create (struct virtio_video *vv, + uint32_t function_id, uint32_t
stream_id, + const char *name)drivers/media/virtio/virtio_video_dec.c new file +6
-0+{ +struct virtio_video_stream_create *req_p; +struct virtio_video_vbuffer *vbuf; + + req_p =virtio_video_alloc_req (vv, &vbuf,sizeof (*req_p)); +if (IS_ERR (req_p))
Quoted 9 lines
drivers/media/virtio/virtio_video_dec.c
Quoted 2 lines
drivers/media/virtio/virtio_video_dec.c new file +2
-0+return PTR_ERR (req_p); +memset (req_p,0 ,sizeof (*req_p));I'm wondering if we don't need to call memset here, as
virtio_video_get_vbufcalls memset for vbuf.
If it's not true, it'd make sense to call memset invirtio_video_alloc_reqinstead because we always want to have zero-initialized structs.
I agree.
Best regards,
Dmitry.
Quoted 4 lines
[snip]
Best regards,
Keiichi
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Hi Dmitry,
Quoted 110 lines · Dmitry Sepp
Hi Keiichi,
Quoted 9 lines · On Mittwoch, 25. Dezember 2019 12:38:55 CET Keiichi Watanabe wrote:
Hi Dmitry,
Thanks for sharing the driver implementation.
Since the virtio protocol specification is still under discussion in virtio-dev@ ML separately, let me add comments only about implementation that are irrelevant to the protocol details. Please find my comments inline.On Fri, Dec 6, 2019 at 1:33 AM Dmitry Sepp <[email protected]>
Quoted 76 lines · wrote:
Quoted 76 lines
From: Kiran Pawar <[email protected]>
This adds a Virtio based video driver for video streaming device that operates input and/or output data buffers to share video devices with several guests. The current implementation consist of V4L2 based video driver supporting video functions of decoder and encoder. This can be extended for other functions e.g. processor, capture and output. The device uses descriptor structures to advertise and negotiate stream formats and controls. This allows the driver to modify the processing logic of the device on a per stream basis.
Signed-off-by Dmitry Sepp <[email protected]>Signed-off-by Kiran Pawar <[email protected]>Signed-off-by Nikolay Martyanov <[email protected]>---
drivers/media/Kconfig | 1 + drivers/media/Makefile | 2 + drivers/media/virtio/Kconfig | 11 + drivers/media/virtio/Makefile | 12 + drivers/media/virtio/virtio_video.h | 372 +++++++ drivers/media/virtio/virtio_video_caps.c | 618 +++++++++++ drivers/media/virtio/virtio_video_dec.c | 947 +++++++++++++++++ drivers/media/virtio/virtio_video_dec.h | 30 + drivers/media/virtio/virtio_video_device.c | 747 +++++++++++++ drivers/media/virtio/virtio_video_driver.c | 278 +++++ drivers/media/virtio/virtio_video_enc.c | 1124 ++++++++++++++++++++ drivers/media/virtio/virtio_video_enc.h | 30 + drivers/media/virtio/virtio_video_vq.c | 950 +++++++++++++++++ include/uapi/linux/virtio_ids.h | 2 + include/uapi/linux/virtio_video.h | 456 ++++++++ 15 files changed, 5580 insertions(+) create mode 100644 drivers/media/virtio/Kconfig create mode 100644 drivers/media/virtio/Makefile create mode 100644 drivers/media/virtio/virtio_video.h create mode 100644 drivers/media/virtio/virtio_video_caps.c create mode 100644 drivers/media/virtio/virtio_video_dec.c create mode 100644 drivers/media/virtio/virtio_video_dec.h create mode 100644 drivers/media/virtio/virtio_video_device.c create mode 100644 drivers/media/virtio/virtio_video_driver.c create mode 100644 drivers/media/virtio/virtio_video_enc.c create mode 100644 drivers/media/virtio/virtio_video_enc.h create mode 100644 drivers/media/virtio/virtio_video_vq.c create mode 100644 include/uapi/linux/virtio_video.hdrivers/media/Kconfig +1
-0@@ -170,6 +170,7 @@ source "drivers/media/pci/Kconfig" source "drivers/media/platform/Kconfig" source "drivers/media/mmc/Kconfig" source "drivers/media/radio/Kconfig" +source "drivers/media/virtio/Kconfig"comment "Supported FireWire (IEEE 1394) Adapters"
depends on DVB_CORE && FIREWIREdrivers/media/Makefile +2
-0@@ -25,6 +25,8 @@ obj-y += rc/ obj-$(CONFIG_CEC_CORE) += cec/ +obj-$(CONFIG_VIDEO_VIRTIO) += virtio/ + # # Finally, merge the drivers that require the core#
Quoted 13 lines
drivers/media/virtio/Kconfig line 4
Quoted 10 lines
drivers/media/virtio/Kconfig new file +4
-0@@ -0,0 +1,11 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Video driver for virtio + +config VIDEO_VIRTIO"
VIRTIO_VIDEO" would be better likeVIRTIO_NETandVIRTIO_PCI.Ok, I agree.
Quoted 5 lines
Quoted 4 lines
drivers/media/virtio/Kconfig new file +4
-0+ tristate "Virtio video V4L2 driver" + depends on VIRTIO && VIDEO_DEV && VIDEO_V4L2 + select VIDEOBUF2_DMA_SG + select V4L2_MEM2MEM_DEV
Quoted 5 lines
Quoted 3 lines
I suppose you also need "select
VIDEOBUF2_V4L2", as structvb2_v4l2_bufferis used.It is selected automatically by
VIDEO_V4L2. Also, there are no drivers that set it explicitly.
Ah, you're right.VIDEO_V4L2 selects VIDEOBUF2_V4L2 if VIDEOBUF2_CORE is set. And, VIDEOBUF2_CORE should be set due to V4L2_MEM2MEM_DEV. So, this looks okay as is.
Quoted 227 lines
Quoted 67 lines
Quoted 67 lines
drivers/media/virtio/Kconfig new file +3
-0+ help + This is the virtual video driver for virtio. + Say Y or M.drivers/media/virtio/Makefile new file +12
-0@@ -0,0 +1,12 @@ +# SPDX-License-Identifier: GPL-2.0+ + +obj-$(CONFIG_VIDEO_VIRTIO) += virtio-video.o + +virtio-video-objs := \ + virtio_video_driver.o \ + virtio_video_vq.o \ + virtio_video_device.o \ + virtio_video_dec.o \ + virtio_video_enc.o \ + virtio_video_caps.o +(quoted diff) +0
-0
b/drivers/media/virtio/virtio_video.hnew file mode 100644 index 000000000000..23c77dc0cb93drivers/media/virtio/virtio_video.h new file +39
-0@@ -0,0 +1,372 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* Common header for virtio video driver. +* +* Copyright 2019 OpenSynergy GmbH. +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation; either version 2 of the License, or +* (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, see <http://www.gnu.org/licenses/>. +*/ + +#ifndef _VIRTIO_VIDEO_H +#define _VIRTIO_VIDEO_H + +#include <linux/virtio.h> +#include <linux/virtio_ids.h> +#include <linux/virtio_config.h> +#include <linux/virtio_video.h> +#include <linux/list.h> +#include <media/v4l2-device.h> +#include <media/v4l2-mem2mem.h> +#include <media/v4l2-ctrls.h> + +#define DRIVER_NAME "virtio-video" + +#ifndef VIRTIO_ID_VIDEO +#define VIRTIO_ID_VIDEO 29 +#endif + +enum video_pin_type { + VIDEO_PIN_TYPE_INPUT = 0,Quoted 8 lines
drivers/media/virtio/virtio_video.h line 41
Quoted 2 lines
drivers/media/virtio/virtio_video.h new file +2
-0+ VIDEO_PIN_TYPE_OUTPUT, +};Why don't you use enums and structs defined in /include/uapi/linux/virtio_video.h?
For example, I suppose we don't need thisvideo_pin_type, as we can use enumvirtio_video_pin_typedefined there.
Same for other enums and structs in this file.The idea was to have some extra layer of parameter verification. But I also agree this might be excessive.
Quoted 48 lines
[snip]
Quoted 26 lines
(quoted diff) +0
-0
b/drivers/media/virtio/virtio_video_dec.cnew file mode 100644 index 000000000000..c2ad62229d21drivers/media/virtio/virtio_video_dec.c new file +20
-0@@ -0,0 +1,947 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* Decoder for virtio video device. +* +* Copyright 2019 OpenSynergy GmbH. +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation; either version 2 of the License, or +* (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, see <http://www.gnu.org/licenses/>. +*/ + +#include <media/v4l2-event.h> drivers/media/virtio/virtio_video_dec.c line 30
Quoted 10 lines
drivers/media/virtio/virtio_video_dec.c new file +10
-0+#include <media/v4l2-ioctl.h> +#include <media/videobuf2-dma-sg.h> + +#include "virtio_video.h" + +static int virtio_video_queue_setup (struct vb2_queue *vq, +unsigned int *num_buffers, +unsigned int *num_planes, +unsigned int sizes[], +struct device *alloc_devs[])We have functions named "
virtio_video_queue_setup" both invirtio_video_dec.candvirtio_video_enc.c.
How about calling them "virtio_video_{dec,enc}_queue_setup" or "virtvideo_{dec,enc}_queue_setup"?
Though the naming conflicts of static functions shouldn't cause any problem when building a kernel, indexing tools, such as ctags, should be confused.Quoted 1 line
Same for other functions.
There are some functions that simply can be moved to common code, we'll look into this as well.
Quoted 17 lines
[snip]
drivers/media/virtio/virtio_video_dec.c line 325
Quoted 10 lines
drivers/media/virtio/virtio_video_dec.c new file +10
-0+int virtio_video_init_dec_queues (void *priv,struct vb2_queue *src_vq, +struct vb2_queue *dst_vq) +{ +int ret; +struct virtio_video_stream *stream = priv; +struct virtio_video_device *vvd =to_virtio_vd (stream->video_dev); +struct device *dev = vvd->vv->v4l2_dev.dev; + + src_vq->type =V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE ; + src_vq->io_modes =VB2_MMAP |VB2_DMABUF ;I suppose this driver doesn't support DMABUF mode yet. IIUC,
VB_DMABUFshould be specified only when a driver supports DMA buffer importing.We do support DMABUF. This is exactly how virtio-video works in Android with gralloc handles. But ideally the driver needs to know device restrictions and alloc accordingly. This is why we needed struct
virtio_video_plane_format. I guess we'll need to get it back.Quoted 38 lines
[snip]
Quoted 35 lines
drivers/media/virtio/virtio_video_dec.c new file +23
-0+static int virtio_video_s_fmt (struct virtio_video_stream *stream, +struct v4l2_format *f) +{ +int i, ret; +struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp; +struct virtio_video_device *vvd =to_virtio_vd (stream->video_dev); +struct virtio_video *vv = vvd->vv; +struct video_format_info info; +struct video_format_info *p_info; +enum video_pin_type pin =VIDEO_PIN_TYPE_INPUT ; + + ret =virtio_video_try_fmt (stream, f); +if (ret) +return ret; + + info.frame_width = pix_mp->width; + info.frame_height = pix_mp->height; + info.num_planes = pix_mp->num_planes; + info.fourcc_format = pix_mp->pixelformat; + +for (i =0 ; i < info.num_planes; i++) { + info.plane_format[i].stride = +pix_mp->plane_fmt[i].bytesperline;
drivers/media/virtio/virtio_video_dec.c new file +11
-0+ info.plane_format[i].plane_size = + pix_mp->plane_fmt[i].sizeimage; + } + +if (!V4L2_TYPE_IS_OUTPUT (f->type)) + pin =VIDEO_PIN_TYPE_OUTPUT ; + +virtio_video_req_set_params (vv, vvd->id, &info, pin, +VIDEO_PARAMS_SCOPE_STREAM , stream); + + virtio_video_req_get_params(vv, vvd->id, VIDEO_PIN_TYPE_INPUT,Quoted 8 lines
drivers/media/virtio/virtio_video_dec.c line 856
Quoted 5 lines
drivers/media/virtio/virtio_video_dec.c new file +5
-0+ VIDEO_PARAMS_SCOPE_STREAM, stream); + +virtio_video_req_get_params (vv, vvd->id,VIDEO_PIN_TYPE_OUTPUT , +VIDEO_PARAMS_SCOPE_STREAM , stream); +nit:
GET_PARAMSmust be called for both queues afterSET_PARAMS. So, how about defining a function to call these three functions?Yes, might be useful.
Quoted 27 lines
[snip]
Quoted 17 lines
drivers/media/virtio/virtio_video_dec.c new file +17
-0+static int virtio_video_device_open (struct file *file) +{ +int ret; +uint32_t stream_id; +char name[TASK_COMM_LEN ]; +struct virtio_video_stream *stream; +struct video_device *video_dev =video_devdata (file); +struct virtio_video_device *vvd =video_drvdata (file); +struct virtio_video *vv = vvd->vv; + + stream =kzalloc (sizeof (*stream),GFP_KERNEL ); +if (!stream) +return -ENOMEM ; + +get_task_comm (name, current); +virtio_video_stream_id_get (vv, stream, &stream_id); + ret = virtio_video_req_stream_create(vv, vvd->id, stream_id,Quoted 1 line
name);
It'd be better not to send virtio requests in open callback to avoid unnecessary overhead. For example, we often open and close
/dev/videoXjust to check its capabilities. In that case, we don't need to create a stream.
Quoted 7 lines
Quoted 5 lines
Instead, it'd make more sense to send the request when REQBUFS is called by passing a non-zero value as "count" and no stream is created yet.
Please see comments forstream_destroyin release callback, too.We probably just don't need to open too often. I know udev does this once on boot. This is pretty much it.
Hmm, a documentation says that V4L2 open() has no side effects. So, I wonder if the device state shouldn't be changed by open().
https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/func-open.html#description
Also, asking users to call open() less often doesn't sound like a good idea. If we can avoid a performance issue in the driver side, we should do so.
Delaying the requests shouldn't require so much code changes, I guess.
Best regards,
Keiichi
Quoted 143 lines
Quoted 10 lines
Quoted 10 lines
drivers/media/virtio/virtio_video_dec.c new file +9
-0+if (ret) { +v4l2_err (&vv->v4l2_dev,"failed to create stream\n" ); + goto err_stream_create; + } + + stream->video_dev = video_dev; + stream->stream_id = stream_id; + stream->state =STREAM_STATE_IDLE ; +mutex_init (&stream->vq_mutex);Quoted 8 lines
drivers/media/virtio/virtio_video_dec.c
Quoted 5 lines
drivers/media/virtio/virtio_video_dec.c new file +5
-0+INIT_WORK (&stream->work, virtio_video_worker); +v4l2_fh_init (&stream->fh, video_dev); + stream->fh.ctrl_handler = &stream->ctrl_handler; + +if (vvd->type ==VIRTIO_VIDEO_FUNC_DECODER ) {nit: How about using switch statements? Same for other if-else statements for
vvd->type.I agree. It will look more neat.
Quoted 40 lines
[snip]
Quoted 5 lines
drivers/media/virtio/virtio_video_dec.c new file +5
-0+ +static int virtio_video_device_release (struct file *file) +{ +struct virtio_video_stream *stream =file2stream (file); +struct video_device *video_dev =video_devdata (file);drivers/media/virtio/virtio_video_dec.c
Quoted 10 lines
drivers/media/virtio/virtio_video_dec.c new file +10
-0+struct virtio_video_device *vvd =video_drvdata (file); +struct virtio_video *vv = vvd->vv; + +v4l2_fh_del (&stream->fh); +v4l2_fh_exit (&stream->fh); +mutex_lock (video_dev->lock); +v4l2_m2m_ctx_release (stream->fh.m2m_ctx); +mutex_unlock (video_dev->lock); + +virtio_video_req_stream_destroy (vv, vvd->id, stream->stream_id);As with
STREAM_CREATErequests,STREAM_DESTROYshould be sent in REQBUFS callback as well.
We can call it in REQBUFS when "v4l2_requestbuffers.count" is 0 and an active stream already exists (i.e.stream_id> 0).
Still, it'd be good to keepSTREAM_DESTROYhere, as users can forget to free buffers via REQBUFS.[snip]
drivers/media/virtio/virtio_video_dec.c
Quoted 7 lines
drivers/media/virtio/virtio_video_dec.c new file +7
-0+void virtio_video_resource_id_get (struct virtio_video *vv,uint32_t *id) +{ +int handle; + +idr_preload (GFP_KERNEL ); +spin_lock (&vv->resource_idr_lock); + handle =idr_alloc (&vv->resource_idr,NULL ,1 ,0 ,GFP_NOWAIT );We should check if
idr_allocreturns no error here or caller-side.[snip]
Quoted 11 lines
drivers/media/virtio/virtio_video_dec.c
Quoted 8 lines
drivers/media/virtio/virtio_video_dec.c new file +8
-0+static int +virtio_video_queue_ctrl_buffer_locked (struct virtio_video *vv, +struct virtio_video_vbuffer *vbuf) +{ +struct virtqueue *vq = vv->ctrlq.vq; +struct scatterlist *sgs[3 ], vreq, vout, vresp; +int outcnt =0 , incnt =0 ; +int ret;How about adding WARN_ON(!mutex_is_locked(&vv->ctrlq.qlock)) to prevent misuse and show which lock must be used?
Makes sense.
Quoted 20 lines
Quoted 20 lines
drivers/media/virtio/virtio_video_dec.c new file +19
-0+ +if (!vv->vq_ready) +return -ENODEV ; + +sg_init_one (&vreq, vbuf->buf, vbuf->size); + sgs[outcnt + incnt] = &vreq; + outcnt++; + +if (vbuf->data_size) { +sg_init_one (&vout, vbuf->data_buf, vbuf->data_size); + sgs[outcnt + incnt] = &vout; + outcnt++; + } + +if (vbuf->resp_size) { +sg_init_one (&vresp, vbuf->resp_buf, vbuf->resp_size); + sgs[outcnt + incnt] = &vresp; + incnt++; + }Quoted 8 lines
drivers/media/virtio/virtio_video_dec.c
Quoted 6 lines
drivers/media/virtio/virtio_video_dec.c new file +6
-0+ +retry: + ret =virtqueue_add_sgs (vq, sgs, outcnt, incnt, vbuf,GFP_ATOMIC ); +if (ret == -ENOSPC ) { +spin_unlock (&vv->ctrlq.qlock); +wait_event (vv->ctrlq.ack_queue, vq->num_free);
wait_event_timeoutwould be better to avoid getting stuck.True.
Quoted 6 lines
drivers/media/virtio/virtio_video_dec.c
Quoted 3 lines
drivers/media/virtio/virtio_video_dec.c new file +2
-0+spin_lock (&vv->ctrlq.qlock); + goto retry;I'd use while-loop instead of goto.
The code was derived from virtio-gpu. Doesn't make too much difference in the end.
Quoted 11 lines
[snip]
Quoted 9 lines
drivers/media/virtio/virtio_video_dec.c new file +2
-0+int virtio_video_req_stream_create (struct virtio_video *vv, + uint32_t function_id, uint32_t
stream_id, + const char *name)drivers/media/virtio/virtio_video_dec.c new file +6
-0+{ +struct virtio_video_stream_create *req_p; +struct virtio_video_vbuffer *vbuf; + + req_p =virtio_video_alloc_req (vv, &vbuf,sizeof (*req_p)); +if (IS_ERR (req_p))Quoted 9 lines
drivers/media/virtio/virtio_video_dec.c
Quoted 2 lines
drivers/media/virtio/virtio_video_dec.c new file +2
-0+return PTR_ERR (req_p); +memset (req_p,0 ,sizeof (*req_p));I'm wondering if we don't need to call memset here, as
virtio_video_get_vbufcalls memset for vbuf.
If it's not true, it'd make sense to call memset invirtio_video_alloc_reqinstead because we always want to have zero-initialized structs.I agree.
Best regards,
Dmitry.Quoted 4 lines
[snip]
Best regards,
Keiichi
Hi Keiichi, Tomasz,
Quoted 385 lines · On Montag, 6. Januar 2020 07:44:51 CET Keiichi Watanabe wrote:
Hi Dmitry,
On Fri, Dec 27, 2019 at 11:05 PM Dmitry Sepp
Quoted 113 lines · <[email protected]> wrote:
Hi Keiichi,
Quoted 9 lines · On Mittwoch, 25. Dezember 2019 12:38:55 CET Keiichi Watanabe wrote:
Hi Dmitry,
Thanks for sharing the driver implementation.
Since the virtio protocol specification is still under discussion in virtio-dev@ ML separately, let me add comments only about implementation that are irrelevant to the protocol details. Please find my comments inline.On Fri, Dec 6, 2019 at 1:33 AM Dmitry Sepp <[email protected]>
Quoted 78 lines · wrote:
Quoted 78 lines
From: Kiran Pawar <[email protected]>
This adds a Virtio based video driver for video streaming device that operates input and/or output data buffers to share video devices with several guests. The current implementation consist of V4L2 based video driver supporting video functions of decoder and encoder. This can be extended for other functions e.g. processor, capture and output. The device uses descriptor structures to advertise and negotiate stream
formats and controls. This allows the driver to modify the processing logic of the device on a per stream basis.Signed-off-by Dmitry Sepp <[email protected]>Signed-off-by Kiran Pawar <[email protected]>Signed-off-by Nikolay Martyanov <[email protected]>---
drivers/media/Kconfig | 1 + drivers/media/Makefile | 2 + drivers/media/virtio/Kconfig | 11 + drivers/media/virtio/Makefile | 12 + drivers/media/virtio/virtio_video.h | 372 +++++++ drivers/media/virtio/virtio_video_caps.c | 618 +++++++++++ drivers/media/virtio/virtio_video_dec.c | 947 +++++++++++++++++ drivers/media/virtio/virtio_video_dec.h | 30 + drivers/media/virtio/virtio_video_device.c | 747 +++++++++++++ drivers/media/virtio/virtio_video_driver.c | 278 +++++ drivers/media/virtio/virtio_video_enc.c | 1124 ++++++++++++++++++++ drivers/media/virtio/virtio_video_enc.h | 30 + drivers/media/virtio/virtio_video_vq.c | 950 +++++++++++++++++ include/uapi/linux/virtio_ids.h | 2 + include/uapi/linux/virtio_video.h | 456 ++++++++ 15 files changed, 5580 insertions(+) create mode 100644 drivers/media/virtio/Kconfig create mode 100644 drivers/media/virtio/Makefile create mode 100644 drivers/media/virtio/virtio_video.h create mode 100644 drivers/media/virtio/virtio_video_caps.c create mode 100644 drivers/media/virtio/virtio_video_dec.c create mode 100644 drivers/media/virtio/virtio_video_dec.h create mode 100644 drivers/media/virtio/virtio_video_device.c create mode 100644 drivers/media/virtio/virtio_video_driver.c create mode 100644 drivers/media/virtio/virtio_video_enc.c create mode 100644 drivers/media/virtio/virtio_video_enc.h create mode 100644 drivers/media/virtio/virtio_video_vq.c create mode 100644 include/uapi/linux/virtio_video.hdrivers/media/Kconfig +1
-0@@ -170,6 +170,7 @@ source "drivers/media/pci/Kconfig" source "drivers/media/platform/Kconfig" source "drivers/media/mmc/Kconfig" source "drivers/media/radio/Kconfig" +source "drivers/media/virtio/Kconfig"comment "Supported FireWire (IEEE 1394) Adapters"
depends on DVB_CORE && FIREWIREdrivers/media/Makefile +2
-0@@ -25,6 +25,8 @@ obj-y += rc/ obj-$(CONFIG_CEC_CORE) += cec/ +obj-$(CONFIG_VIDEO_VIRTIO) += virtio/ + # # Finally, merge the drivers that require the core#
Quoted 13 lines
drivers/media/virtio/Kconfig line 4
Quoted 11 lines
drivers/media/virtio/Kconfig new file +4
-0@@ -0,0 +1,11 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Video driver for virtio + +config VIDEO_VIRTIO"
VIRTIO_VIDEO" would be better likeVIRTIO_NETandVIRTIO_PCI.Ok, I agree.
Quoted 5 lines
Quoted 4 lines
drivers/media/virtio/Kconfig new file +4
-0+ tristate "Virtio video V4L2 driver" + depends on VIRTIO && VIDEO_DEV && VIDEO_V4L2 + select VIDEOBUF2_DMA_SG + select V4L2_MEM2MEM_DEVQuoted 6 lines
Quoted 2 lines
I suppose you also need "select
VIDEOBUF2_V4L2", as structvb2_v4l2_bufferis used.It is selected automatically by
VIDEO_V4L2. Also, there are no drivers that
set it explicitly.Ah, you're right.
VIDEO_V4L2selectsVIDEOBUF2_V4L2ifVIDEOBUF2_COREis set. And,VIDEOBUF2_COREshould be set due toV4L2_MEM2MEM_DEV. So, this looks okay as is.Quoted 242 lines
Quoted 70 lines
Quoted 70 lines
drivers/media/virtio/Kconfig new file +3
-0+ help + This is the virtual video driver for virtio. + Say Y or M.drivers/media/virtio/Makefile new file +12
-0@@ -0,0 +1,12 @@ +# SPDX-License-Identifier: GPL-2.0+ + +obj-$(CONFIG_VIDEO_VIRTIO) += virtio-video.o + +virtio-video-objs := \ + virtio_video_driver.o \ + virtio_video_vq.o \ + virtio_video_device.o \ + virtio_video_dec.o \ + virtio_video_enc.o \ + virtio_video_caps.o +(quoted diff) +0
-0
b/drivers/media/virtio/virtio_video.hnew file mode 100644 index 000000000000..23c77dc0cb93drivers/media/virtio/virtio_video.h new file +6
-0@@ -0,0 +1,372 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* Common header for virtio video driver. +* +* Copyright 2019 OpenSynergy GmbH. +* +* This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see
<http://www.gnu.org/licenses/>.
+ */drivers/media/virtio/virtio_video.h new file +21
-0+ +#ifndef _VIRTIO_VIDEO_H +#define _VIRTIO_VIDEO_H + +#include <linux/virtio.h> +#include <linux/virtio_ids.h> +#include <linux/virtio_config.h> +#include <linux/virtio_video.h> +#include <linux/list.h> +#include <media/v4l2-device.h> +#include <media/v4l2-mem2mem.h> +#include <media/v4l2-ctrls.h> + +#define DRIVER_NAME "virtio-video" + +#ifndef VIRTIO_ID_VIDEO +#define VIRTIO_ID_VIDEO 29 +#endif + +enum video_pin_type { + VIDEO_PIN_TYPE_INPUT = 0,Quoted 8 lines
drivers/media/virtio/virtio_video.h
Quoted 2 lines
drivers/media/virtio/virtio_video.h new file +2
-0+ VIDEO_PIN_TYPE_OUTPUT, +};Why don't you use enums and structs defined in /include/uapi/linux/virtio_video.h?
For example, I suppose we don't need thisvideo_pin_type, as we can use enumvirtio_video_pin_typedefined there.
Same for other enums and structs in this file.The idea was to have some extra layer of parameter verification. But I also
agree this might be excessive.Quoted 50 lines
[snip]
Quoted 29 lines
(quoted diff) +0
-0
b/drivers/media/virtio/virtio_video_dec.cnew file mode 100644 index 000000000000..c2ad62229d21drivers/media/virtio/virtio_video_dec.c new file +6
-0@@ -0,0 +1,947 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* Decoder for virtio video device. +* +* Copyright 2019 OpenSynergy GmbH. +* +* This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see
<http://www.gnu.org/licenses/>.
+ */drivers/media/virtio/virtio_video_dec.c new file +2
-0+ +#include <media/v4l2-event.h> drivers/media/virtio/virtio_video_dec.c line 30
Quoted 10 lines
drivers/media/virtio/virtio_video_dec.c new file +10
-0+#include <media/v4l2-ioctl.h> +#include <media/videobuf2-dma-sg.h> + +#include "virtio_video.h" + +static int virtio_video_queue_setup (struct vb2_queue *vq, +unsigned int *num_buffers, +unsigned int *num_planes, +unsigned int sizes[], +struct device *alloc_devs[])We have functions named "
virtio_video_queue_setup" both invirtio_video_dec.candvirtio_video_enc.c.
How about calling them "virtio_video_{dec,enc}_queue_setup" or "virtvideo_{dec,enc}_queue_setup"?
Though the naming conflicts of static functions shouldn't cause any problem when building a kernel, indexing tools, such as ctags, should be confused.Quoted 1 line
Same for other functions.
There are some functions that simply can be moved to common code, we'll look into this as well.
Quoted 18 lines
[snip]
drivers/media/virtio/virtio_video_dec.c line 325
Quoted 12 lines
+int virtio_video_init_dec_queues(void *priv, struct
vb2_queue*src_vq,drivers/media/virtio/virtio_video_dec.c new file +5
-0+ struct vb2_queue *dst_vq) +{ +int ret; +struct virtio_video_stream *stream = priv; + struct virtio_video_device *vvd =
to_virtio_vd(stream->video_dev);drivers/media/virtio/virtio_video_dec.c new file +4
-0+struct device *dev = vvd->vv->v4l2_dev.dev; + + src_vq->type =V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE ; + src_vq->io_modes =VB2_MMAP |VB2_DMABUF ;I suppose this driver doesn't support DMABUF mode yet. IIUC,
VB_DMABUFshould be specified only when a driver supports DMA buffer importing.We do support DMABUF. This is exactly how virtio-video works in Android with gralloc handles. But ideally the driver needs to know device restrictions and alloc accordingly. This is why we needed struct
virtio_video_plane_format. I guess we'll need to get it back.Quoted 43 lines
[snip]
Quoted 41 lines
drivers/media/virtio/virtio_video_dec.c new file +6
-0+static int virtio_video_s_fmt (struct virtio_video_stream *stream, +struct v4l2_format *f) +{ +int i, ret; +struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp; + struct virtio_video_device *vvd =
to_virtio_vd(stream->video_dev);drivers/media/virtio/virtio_video_dec.c new file +17
-0+struct virtio_video *vv = vvd->vv; +struct video_format_info info; +struct video_format_info *p_info; +enum video_pin_type pin =VIDEO_PIN_TYPE_INPUT ; + + ret =virtio_video_try_fmt (stream, f); +if (ret) +return ret; + + info.frame_width = pix_mp->width; + info.frame_height = pix_mp->height; + info.num_planes = pix_mp->num_planes; + info.fourcc_format = pix_mp->pixelformat; + +for (i =0 ; i < info.num_planes; i++) { + info.plane_format[i].stride = +pix_mp->plane_fmt[i].bytesperline;
drivers/media/virtio/virtio_video_dec.c new file +2
-0+ info.plane_format[i].plane_size = +pix_mp->plane_fmt[i].sizeimage;
drivers/media/virtio/virtio_video_dec.c new file +7
-0+ } + +if (!V4L2_TYPE_IS_OUTPUT (f->type)) + pin =VIDEO_PIN_TYPE_OUTPUT ; + + virtio_video_req_set_params(vv, vvd->id, &info, pin, + VIDEO_PARAMS_SCOPE_STREAM,stream);
drivers/media/virtio/virtio_video_dec.c new file +3
-0+ + virtio_video_req_get_params(vv, vvd->id, VIDEO_PIN_TYPE_INPUT, + VIDEO_PARAMS_SCOPE_STREAM,stream);
+Quoted 8 lines
Quoted 5 lines
+ virtio_video_req_get_params(vv,
vvd->id,VIDEO_PIN_TYPE_OUTPUT,
+VIDEO_PARAMS_SCOPE_STREAM,
stream);
+nit:
GET_PARAMSmust be called for both queues afterSET_PARAMS. So, how about defining a function to call these three functions?Yes, might be useful.
Quoted 26 lines
[snip]
Quoted 17 lines
drivers/media/virtio/virtio_video_dec.c new file +17
-0+static int virtio_video_device_open (struct file *file) +{ +int ret; +uint32_t stream_id; +char name[TASK_COMM_LEN ]; +struct virtio_video_stream *stream; +struct video_device *video_dev =video_devdata (file); +struct virtio_video_device *vvd =video_drvdata (file); +struct virtio_video *vv = vvd->vv; + + stream =kzalloc (sizeof (*stream),GFP_KERNEL ); +if (!stream) +return -ENOMEM ; + +get_task_comm (name, current); +virtio_video_stream_id_get (vv, stream, &stream_id); + ret = virtio_video_req_stream_create(vv, vvd->id, stream_id,Quoted 1 line
name);
It'd be better not to send virtio requests in open callback to avoid unnecessary overhead. For example, we often open and close
/dev/videoXjust to check its capabilities. In that case, we don't need to create a stream.Quoted 7 lines
Quoted 4 lines
Instead, it'd make more sense to send the request when REQBUFS is called by passing a non-zero value as "count" and no stream is created yet.
Please see comments forstream_destroyin release callback, too.We probably just don't need to open too often. I know udev does this once on boot. This is pretty much it.
Hmm, a documentation says that V4L2
open()has no side effects. So, I wonder if the device state shouldn't be changed byopen().
https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/func-open.html#descripti
on
Quoted 5 lines
Also, asking users to call
open()less often doesn't sound like a good idea. If we can avoid a performance issue in the driver side, we should do so.
Delaying the requests shouldn't require so much code changes, I guess.
Thinking of this in conjunction with this:
enum virtio_video_planes_layout {
VIRTIO_VIDEO_PLANES_LAYOUT_UNSPEC = 0, /* no special requirement */ VIRTIO_VIDEO_PLANES_LAYOUT_CONTIG,
};
In my eyes a good place to handle both would be queue_setup(). Different formats can have different mem requirements. So it could be nice to set mem_ops depending on the format requirements inside queue_setup().
But the problem is that by the time we get this call the queue must be already initialized, and the mem_ops field must be set prior to calling vb2_queue_init(). The vb2_queue_init() function is in turn usually called from a driver callback provided as a parameter to v4l2_m2m_ctx_init(), which is usually (always?) called from open().
Any ideas?
A poor man's approach would be to have a separate video devices depending on the layout requirements.
Best regards,
Dmitry.
Quoted 147 lines
Best regards,
KeiichiQuoted 144 lines
Quoted 9 lines
Quoted 9 lines
drivers/media/virtio/virtio_video_dec.c new file +9
-0+if (ret) { +v4l2_err (&vv->v4l2_dev,"failed to create stream\n" ); + goto err_stream_create; + } + + stream->video_dev = video_dev; + stream->stream_id = stream_id; + stream->state =STREAM_STATE_IDLE ; +mutex_init (&stream->vq_mutex);Quoted 8 lines
drivers/media/virtio/virtio_video_dec.c
Quoted 5 lines
drivers/media/virtio/virtio_video_dec.c new file +5
-0+INIT_WORK (&stream->work, virtio_video_worker); +v4l2_fh_init (&stream->fh, video_dev); + stream->fh.ctrl_handler = &stream->ctrl_handler; + +if (vvd->type ==VIRTIO_VIDEO_FUNC_DECODER ) {nit: How about using switch statements? Same for other if-else statements for
vvd->type.I agree. It will look more neat.
Quoted 41 lines
[snip]
Quoted 15 lines
drivers/media/virtio/virtio_video_dec.c new file +15
-0+ +static int virtio_video_device_release (struct file *file) +{ +struct virtio_video_stream *stream =file2stream (file); +struct video_device *video_dev =video_devdata (file); +struct virtio_video_device *vvd =video_drvdata (file); +struct virtio_video *vv = vvd->vv; + +v4l2_fh_del (&stream->fh); +v4l2_fh_exit (&stream->fh); +mutex_lock (video_dev->lock); +v4l2_m2m_ctx_release (stream->fh.m2m_ctx); +mutex_unlock (video_dev->lock); + + virtio_video_req_stream_destroy(vv, vvd->id,Quoted 1 line
stream->stream_id);As with
STREAM_CREATErequests,STREAM_DESTROYshould be sent in REQBUFS callback as well.
We can call it in REQBUFS when "v4l2_requestbuffers.count" is 0 and an active stream already exists (i.e.stream_id> 0).
Still, it'd be good to keepSTREAM_DESTROYhere, as users can forget to free buffers via REQBUFS.[snip]
drivers/media/virtio/virtio_video_dec.c
Quoted 8 lines
+void
virtio_video_resource_id_get(struct virtio_video *vv, uint32_t *id)drivers/media/virtio/virtio_video_dec.c new file +6
-0+{ +int handle; + +idr_preload (GFP_KERNEL ); +spin_lock (&vv->resource_idr_lock); + handle =idr_alloc (&vv->resource_idr,NULL ,1 ,0 ,GFP_NOWAIT );We should check if
idr_allocreturns no error here or caller-side.[snip]
Quoted 12 lines
drivers/media/virtio/virtio_video_dec.c
Quoted 9 lines
drivers/media/virtio/virtio_video_dec.c new file +3
-0+static int +virtio_video_queue_ctrl_buffer_locked (struct virtio_video *vv, + struct virtio_video_vbuffer*vbuf)
drivers/media/virtio/virtio_video_dec.c new file +5
-0+{ +struct virtqueue *vq = vv->ctrlq.vq; +struct scatterlist *sgs[3 ], vreq, vout, vresp; +int outcnt =0 , incnt =0 ; +int ret;How about adding WARN_ON(!mutex_is_locked(&vv->ctrlq.qlock)) to prevent misuse and show which lock must be used?
Makes sense.
Quoted 20 lines
Quoted 20 lines
drivers/media/virtio/virtio_video_dec.c new file +20
-0+ +if (!vv->vq_ready) +return -ENODEV ; + +sg_init_one (&vreq, vbuf->buf, vbuf->size); + sgs[outcnt + incnt] = &vreq; + outcnt++; + +if (vbuf->data_size) { +sg_init_one (&vout, vbuf->data_buf, vbuf->data_size); + sgs[outcnt + incnt] = &vout; + outcnt++; + } + +if (vbuf->resp_size) { +sg_init_one (&vresp, vbuf->resp_buf, vbuf->resp_size); + sgs[outcnt + incnt] = &vresp; + incnt++; + } +Quoted 8 lines
drivers/media/virtio/virtio_video_dec.c
Quoted 6 lines
drivers/media/virtio/virtio_video_dec.c new file +2
-0+retry: + ret = virtqueue_add_sgs(vq, sgs, outcnt, incnt, vbuf,
GFP_ATOMIC);drivers/media/virtio/virtio_video_dec.c new file +3
-0+if (ret == -ENOSPC ) { +spin_unlock (&vv->ctrlq.qlock); +wait_event (vv->ctrlq.ack_queue, vq->num_free);
wait_event_timeoutwould be better to avoid getting stuck.True.
Quoted 4 lines
drivers/media/virtio/virtio_video_dec.c
Quoted 2 lines
drivers/media/virtio/virtio_video_dec.c new file +2
-0+spin_lock (&vv->ctrlq.qlock); + goto retry;I'd use while-loop instead of goto.
The code was derived from virtio-gpu. Doesn't make too much difference in the end.
Quoted 11 lines
[snip]
Quoted 9 lines
drivers/media/virtio/virtio_video_dec.c new file +2
-0+int virtio_video_req_stream_create (struct virtio_video *vv, + uint32_t function_id, uint32_t
stream_id, + const char *name)drivers/media/virtio/virtio_video_dec.c new file +6
-0+{ +struct virtio_video_stream_create *req_p; +struct virtio_video_vbuffer *vbuf; + + req_p =virtio_video_alloc_req (vv, &vbuf,sizeof (*req_p)); +if (IS_ERR (req_p))Quoted 8 lines
drivers/media/virtio/virtio_video_dec.c
Quoted 2 lines
drivers/media/virtio/virtio_video_dec.c new file +2
-0+return PTR_ERR (req_p); +memset (req_p,0 ,sizeof (*req_p));I'm wondering if we don't need to call memset here, as
virtio_video_get_vbufcalls memset for vbuf.
If it's not true, it'd make sense to call memset invirtio_video_alloc_reqinstead because we always want to have zero-initialized structs.I agree.
Best regards,
Dmitry.Quoted 4 lines
[snip]
Best regards,
Keiichi
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]