Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1467617 > unrolled thread
| Started by | Stanimir Varbanov <stanimir.varbanov@linaro.org> |
|---|---|
| First post | 2016-08-22 15:20 +0200 |
| Last post | 2016-08-23 05:40 +0200 |
| Articles | 5 — 3 participants |
Back to article view | Back to linux.kernel
[PATCH 0/8] Qualcomm video decoder/encoder driver Stanimir Varbanov <stanimir.varbanov@linaro.org> - 2016-08-22 15:20 +0200
Re: [PATCH 2/8] media: vidc: adding core part and helper functions Hans Verkuil <hverkuil@xs4all.nl> - 2016-08-22 15:50 +0200
Re: [PATCH 2/8] media: vidc: adding core part and helper functions Stanimir Varbanov <stanimir.varbanov@linaro.org> - 2016-08-22 18:10 +0200
Re: [PATCH 5/8] media: vidc: add Host Firmware Interface (HFI) Bjorn Andersson <bjorn.andersson@linaro.org> - 2016-08-23 05:30 +0200
Re: [PATCH 6/8] media: vidc: add Venus HFI files Bjorn Andersson <bjorn.andersson@linaro.org> - 2016-08-23 05:40 +0200
| From | Stanimir Varbanov <stanimir.varbanov@linaro.org> |
|---|---|
| Date | 2016-08-22 15:20 +0200 |
| Subject | [PATCH 0/8] Qualcomm video decoder/encoder driver |
| Message-ID | <s8XiV-6LP-3@gated-at.bofh.it> |
This patchset introduces a basic support for Qualcomm video
acceleration hardware used for video stream decoding/encoding.
The video IP can found on various qcom SoCs like apq8084, msm8916
and msm8996, hence it is widly distributed but the driver is
missing in the mainline.
The v4l2 driver is something like a wrapper over Host Firmware
Interface. The HFI itself is a set of command and message packets
send/received through shared memory, and its purpose is to
comunicate with the firmware which is run on remote processor.
The Venus is the name of the video hardware IP that doing the
video acceleration.
From the software point of view the HFI interface is implemented
in the files with prefix hfi_xxx. It acts as a translation layer
between HFI and v4l2 layer. There is one special file in the
driver called hfi_venus which doing most of the driver
orchestration work. Something more it setups Venus core, run it
and handle commands and messages from low-level point of view with
the help of provided functions by HFI interface.
I think that the driver is in good shape for mainline kernel, and
I hope the review comments will help to improve it, so please
do review and make comments.
The driver depends on:
- venus remoteproc driver posted at [1].
- out-of-tree qcom IOMMU driver and IOMMU probe deferral support
at [2].
The driver has been tested on db410c (with apq8016 SoC) with simple
v4l2 test applications and with gstreamer v4l2 videodec plugin,
and v4l2 h264 out-of-tree gstreamer videoenc plugin.
The output of v4l2-compliance test looks like:
root@dragonboard-410c:/home/linaro# ./v4l2-compliance -d /dev/video0
v4l2-compliance SHA : ee1ab491019f80052834d14c76bdd1c1b46f2158
Driver Info:
Driver name : vidc
Card type : video decoder
Bus info : platform:vidc
Driver version: 4.8.0
Capabilities : 0x84204000
Video Memory-to-Memory Multiplanar
Streaming
Extended Pix Format
Device Capabilities
Device Caps : 0x04204000
Video Memory-to-Memory Multiplanar
Streaming
Extended Pix Format
Compliance test for device /dev/video0 (not using libv4l2):
Required ioctls:
test VIDIOC_QUERYCAP: OK
Allow for multiple opens:
test second video open: OK
test VIDIOC_QUERYCAP: OK
test VIDIOC_G/S_PRIORITY: OK
test for unlimited opens: OK
Debug ioctls:
test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
test VIDIOC_LOG_STATUS: OK (Not Supported)
Input ioctls:
test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
test VIDIOC_ENUMAUDIO: OK (Not Supported)
test VIDIOC_G/S/ENUMINPUT: OK (Not Supported)
test VIDIOC_G/S_AUDIO: OK (Not Supported)
Inputs: 0 Audio Inputs: 0 Tuners: 0
Output ioctls:
test VIDIOC_G/S_MODULATOR: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_ENUMAUDOUT: OK (Not Supported)
test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
test VIDIOC_G/S_AUDOUT: OK (Not Supported)
Outputs: 0 Audio Outputs: 0 Modulators: 0
Input/Output configuration ioctls:
test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
test VIDIOC_G/S_EDID: OK (Not Supported)
Control ioctls:
test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
test VIDIOC_QUERYCTRL: OK
test VIDIOC_G/S_CTRL: OK
test VIDIOC_G/S/TRY_EXT_CTRLS: OK
test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK
test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
Standard Controls: 7 Private Controls: 0
Format ioctls:
test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
test VIDIOC_G/S_PARM: OK
test VIDIOC_G_FBUF: OK (Not Supported)
test VIDIOC_G_FMT: OK
test VIDIOC_TRY_FMT: OK
test VIDIOC_S_FMT: OK
test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
test Cropping: OK (Not Supported)
test Composing: OK (Not Supported)
test Scaling: OK
Codec ioctls:
test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
test VIDIOC_G_ENC_INDEX: OK (Not Supported)
test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)
Buffer ioctls:
test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
test VIDIOC_EXPBUF: OK
Test input 0:
Total: 43, Succeeded: 43, Failed: 0, Warnings: 0
root@dragonboard-410c:/home/linaro# ./v4l2-compliance -d /dev/video1
v4l2-compliance SHA : ee1ab491019f80052834d14c76bdd1c1b46f2158
Driver Info:
Driver name : vidc
Card type : video encoder
Bus info : platform:vidc
Driver version: 4.8.0
Capabilities : 0x84204000
Video Memory-to-Memory Multiplanar
Streaming
Extended Pix Format
Device Capabilities
Device Caps : 0x04204000
Video Memory-to-Memory Multiplanar
Streaming
Extended Pix Format
Compliance test for device /dev/video1 (not using libv4l2):
Required ioctls:
test VIDIOC_QUERYCAP: OK
Allow for multiple opens:
test second video open: OK
test VIDIOC_QUERYCAP: OK
test VIDIOC_G/S_PRIORITY: OK
test for unlimited opens: OK
Debug ioctls:
test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
test VIDIOC_LOG_STATUS: OK (Not Supported)
Input ioctls:
test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
test VIDIOC_ENUMAUDIO: OK (Not Supported)
test VIDIOC_G/S/ENUMINPUT: OK (Not Supported)
test VIDIOC_G/S_AUDIO: OK (Not Supported)
Inputs: 0 Audio Inputs: 0 Tuners: 0
Output ioctls:
test VIDIOC_G/S_MODULATOR: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_ENUMAUDOUT: OK (Not Supported)
test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
test VIDIOC_G/S_AUDOUT: OK (Not Supported)
Outputs: 0 Audio Outputs: 0 Modulators: 0
Input/Output configuration ioctls:
test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
test VIDIOC_G/S_EDID: OK (Not Supported)
Control ioctls:
test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
test VIDIOC_QUERYCTRL: OK
test VIDIOC_G/S_CTRL: OK
test VIDIOC_G/S/TRY_EXT_CTRLS: OK
test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK
test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
Standard Controls: 32 Private Controls: 0
Format ioctls:
test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
test VIDIOC_G/S_PARM: OK
test VIDIOC_G_FBUF: OK (Not Supported)
test VIDIOC_G_FMT: OK
test VIDIOC_TRY_FMT: OK
test VIDIOC_S_FMT: OK
test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
test Cropping: OK
test Composing: OK (Not Supported)
test Scaling: OK
Codec ioctls:
test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
test VIDIOC_G_ENC_INDEX: OK (Not Supported)
test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)
Buffer ioctls:
test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
test VIDIOC_EXPBUF: OK
Test input 0:
Total: 43, Succeeded: 43, Failed: 0, Warnings: 0
regards,
Stan
[1] https://lkml.org/lkml/2016/8/19/570
[2] https://www.spinics.net/lists/arm-kernel/msg522505.html
Stanimir Varbanov (8):
doc: DT: vidc: binding document for Qualcomm video driver
media: vidc: adding core part and helper functions
media: vidc: decoder: add video decoder files
media: vidc: encoder: add video encoder files
media: vidc: add Host Firmware Interface (HFI)
media: vidc: add Venus HFI files
media: vidc: add Makefiles and Kconfig files
media: vidc: enable building of the video codec driver
.../devicetree/bindings/media/qcom,vidc.txt | 61 +
drivers/media/platform/Kconfig | 1 +
drivers/media/platform/Makefile | 1 +
drivers/media/platform/qcom/Kconfig | 8 +
drivers/media/platform/qcom/Makefile | 6 +
drivers/media/platform/qcom/vidc/Makefile | 19 +
drivers/media/platform/qcom/vidc/core.c | 548 +++++++
drivers/media/platform/qcom/vidc/core.h | 196 +++
drivers/media/platform/qcom/vidc/helpers.c | 394 +++++
drivers/media/platform/qcom/vidc/helpers.h | 43 +
drivers/media/platform/qcom/vidc/hfi.c | 622 ++++++++
drivers/media/platform/qcom/vidc/hfi.h | 272 ++++
drivers/media/platform/qcom/vidc/hfi_cmds.c | 1261 ++++++++++++++++
drivers/media/platform/qcom/vidc/hfi_cmds.h | 338 +++++
drivers/media/platform/qcom/vidc/hfi_helper.h | 1143 +++++++++++++++
drivers/media/platform/qcom/vidc/hfi_msgs.c | 1072 ++++++++++++++
drivers/media/platform/qcom/vidc/hfi_msgs.h | 298 ++++
drivers/media/platform/qcom/vidc/hfi_venus.c | 1539 ++++++++++++++++++++
drivers/media/platform/qcom/vidc/hfi_venus.h | 25 +
drivers/media/platform/qcom/vidc/hfi_venus_io.h | 98 ++
drivers/media/platform/qcom/vidc/int_bufs.c | 325 +++++
drivers/media/platform/qcom/vidc/int_bufs.h | 23 +
drivers/media/platform/qcom/vidc/load.c | 104 ++
drivers/media/platform/qcom/vidc/load.h | 22 +
drivers/media/platform/qcom/vidc/mem.c | 64 +
drivers/media/platform/qcom/vidc/mem.h | 32 +
drivers/media/platform/qcom/vidc/resources.c | 46 +
drivers/media/platform/qcom/vidc/resources.h | 46 +
drivers/media/platform/qcom/vidc/vdec.c | 1100 ++++++++++++++
drivers/media/platform/qcom/vidc/vdec.h | 27 +
drivers/media/platform/qcom/vidc/vdec_ctrls.c | 200 +++
drivers/media/platform/qcom/vidc/vdec_ctrls.h | 21 +
drivers/media/platform/qcom/vidc/venc.c | 1261 ++++++++++++++++
drivers/media/platform/qcom/vidc/venc.h | 27 +
drivers/media/platform/qcom/vidc/venc_ctrls.c | 396 +++++
drivers/media/platform/qcom/vidc/venc_ctrls.h | 23 +
36 files changed, 11662 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/qcom,vidc.txt
create mode 100644 drivers/media/platform/qcom/Kconfig
create mode 100644 drivers/media/platform/qcom/Makefile
create mode 100644 drivers/media/platform/qcom/vidc/Makefile
create mode 100644 drivers/media/platform/qcom/vidc/core.c
create mode 100644 drivers/media/platform/qcom/vidc/core.h
create mode 100644 drivers/media/platform/qcom/vidc/helpers.c
create mode 100644 drivers/media/platform/qcom/vidc/helpers.h
create mode 100644 drivers/media/platform/qcom/vidc/hfi.c
create mode 100644 drivers/media/platform/qcom/vidc/hfi.h
create mode 100644 drivers/media/platform/qcom/vidc/hfi_cmds.c
create mode 100644 drivers/media/platform/qcom/vidc/hfi_cmds.h
create mode 100644 drivers/media/platform/qcom/vidc/hfi_helper.h
create mode 100644 drivers/media/platform/qcom/vidc/hfi_msgs.c
create mode 100644 drivers/media/platform/qcom/vidc/hfi_msgs.h
create mode 100644 drivers/media/platform/qcom/vidc/hfi_venus.c
create mode 100644 drivers/media/platform/qcom/vidc/hfi_venus.h
create mode 100644 drivers/media/platform/qcom/vidc/hfi_venus_io.h
create mode 100644 drivers/media/platform/qcom/vidc/int_bufs.c
create mode 100644 drivers/media/platform/qcom/vidc/int_bufs.h
create mode 100644 drivers/media/platform/qcom/vidc/load.c
create mode 100644 drivers/media/platform/qcom/vidc/load.h
create mode 100644 drivers/media/platform/qcom/vidc/mem.c
create mode 100644 drivers/media/platform/qcom/vidc/mem.h
create mode 100644 drivers/media/platform/qcom/vidc/resources.c
create mode 100644 drivers/media/platform/qcom/vidc/resources.h
create mode 100644 drivers/media/platform/qcom/vidc/vdec.c
create mode 100644 drivers/media/platform/qcom/vidc/vdec.h
create mode 100644 drivers/media/platform/qcom/vidc/vdec_ctrls.c
create mode 100644 drivers/media/platform/qcom/vidc/vdec_ctrls.h
create mode 100644 drivers/media/platform/qcom/vidc/venc.c
create mode 100644 drivers/media/platform/qcom/vidc/venc.h
create mode 100644 drivers/media/platform/qcom/vidc/venc_ctrls.c
create mode 100644 drivers/media/platform/qcom/vidc/venc_ctrls.h
--
2.7.4
[toc] | [next] | [standalone]
| From | Hans Verkuil <hverkuil@xs4all.nl> |
|---|---|
| Date | 2016-08-22 15:50 +0200 |
| Subject | Re: [PATCH 2/8] media: vidc: adding core part and helper functions |
| Message-ID | <s8XLY-6Xy-13@gated-at.bofh.it> |
| In reply to | #1467617 |
Hi Stanimir,
Thanks for this patch series!
I have some review comments:
On 08/22/2016 03:13 PM, Stanimir Varbanov wrote:
<snip>
> diff --git a/drivers/media/platform/qcom/vidc/core.h b/drivers/media/platform/qcom/vidc/core.h
> new file mode 100644
> index 000000000000..5dc8e05f8c36
> --- /dev/null
> +++ b/drivers/media/platform/qcom/vidc/core.h
> @@ -0,0 +1,196 @@
> +/*
> + * Copyright (c) 2012-2015, The Linux Foundation. All rights reserved.
> + * Copyright (C) 2016 Linaro Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * 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.
> + *
> + */
> +
> +#ifndef __VIDC_CORE_H_
> +#define __VIDC_CORE_H_
> +
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/videobuf2-core.h>
> +
> +#include "resources.h"
> +#include "hfi.h"
> +
> +#define VIDC_DRV_NAME "vidc"
> +
> +struct vidc_list {
> + struct list_head list;
> + struct mutex lock;
> +};
> +
> +struct vidc_format {
> + u32 pixfmt;
> + int num_planes;
> + u32 type;
> +};
> +
> +struct vidc_core {
> + struct list_head list;
> + void __iomem *base;
> + int irq;
> + struct clk *clks[VIDC_CLKS_NUM_MAX];
> + struct mutex lock;
> + struct hfi_core hfi;
> + struct video_device vdev_dec;
> + struct video_device vdev_enc;
I know that many drivers embed struct video_device, but this can cause subtle
refcounting problems. I recommend changing this to a pointer and using video_device_alloc().
I have plans to reorganize the way video_devices are allocated and registered in
the near future, and you might just as well prepare this driver for that by switching
to a pointer.
> + struct v4l2_device v4l2_dev;
> + struct list_head instances;
> + const struct vidc_resources *res;
> + struct rproc *rproc;
> + bool rproc_booted;
> + struct device *dev;
> +};
> +
> +struct vdec_controls {
> + u32 post_loop_deb_mode;
> + u32 profile;
> + u32 level;
> +};
> +
> +struct venc_controls {
> + u16 gop_size;
> + u32 idr_period;
> + u32 num_p_frames;
> + u32 num_b_frames;
> + u32 bitrate_mode;
> + u32 bitrate;
> + u32 bitrate_peak;
> +
> + u32 h264_i_period;
> + u32 h264_entropy_mode;
> + u32 h264_i_qp;
> + u32 h264_p_qp;
> + u32 h264_b_qp;
> + u32 h264_min_qp;
> + u32 h264_max_qp;
> + u32 h264_loop_filter_mode;
> + u32 h264_loop_filter_alpha;
> + u32 h264_loop_filter_beta;
> +
> + u32 vp8_min_qp;
> + u32 vp8_max_qp;
> +
> + u32 multi_slice_mode;
> + u32 multi_slice_max_bytes;
> + u32 multi_slice_max_mb;
> +
> + u32 header_mode;
> +
> + u32 profile;
> + u32 level;
> +};
> +
> +struct vidc_inst {
> + struct list_head list;
> + struct mutex lock;
> + struct vidc_core *core;
> +
> + struct vidc_list scratchbufs;
> + struct vidc_list persistbufs;
> + struct vidc_list registeredbufs;
> +
> + struct list_head bufqueue;
> + struct mutex bufqueue_lock;
> +
> + int streamoff;
> + int streamon;
> + struct vb2_queue bufq_out;
> + struct vb2_queue bufq_cap;
> +
> + struct v4l2_ctrl_handler ctrl_handler;
> + union {
> + struct vdec_controls dec;
> + struct venc_controls enc;
> + } controls;
> + struct v4l2_fh fh;
> +
> + struct hfi_inst *hfi_inst;
> +
> + /* session fields */
> + u32 session_type;
> + u32 width;
> + u32 height;
> + u32 out_width;
> + u32 out_height;
> + u32 colorspace;
> + u8 ycbcr_enc;
> + u8 quantization;
> + u8 xfer_func;
> + u64 fps;
> + struct v4l2_fract timeperframe;
> + const struct vidc_format *fmt_out;
> + const struct vidc_format *fmt_cap;
> + unsigned int num_input_bufs;
> + unsigned int num_output_bufs;
> + bool in_reconfig;
> + u32 reconfig_width;
> + u32 reconfig_height;
> + u64 sequence;
> +};
> +
> +#define ctrl_to_inst(ctrl) \
> + container_of(ctrl->handler, struct vidc_inst, ctrl_handler)
> +
> +struct vidc_ctrl {
> + u32 id;
> + enum v4l2_ctrl_type type;
> + s32 min;
> + s32 max;
> + s32 def;
> + u32 step;
> + u64 menu_skip_mask;
> + u32 flags;
> + const char * const *qmenu;
> +};
> +
> +/*
> + * Offset base for buffers on the destination queue - used to distinguish
> + * between source and destination buffers when mmapping - they receive the same
> + * offsets but for different queues
> + */
> +#define DST_QUEUE_OFF_BASE (1 << 30)
> +
> +extern const struct v4l2_file_operations vidc_fops;
> +
> +static inline void INIT_VIDC_LIST(struct vidc_list *mlist)
> +{
> + mutex_init(&mlist->lock);
> + INIT_LIST_HEAD(&mlist->list);
> +}
> +
> +static inline struct vidc_inst *to_inst(struct file *filp)
> +{
> + return container_of(filp->private_data, struct vidc_inst, fh);
> +}
> +
> +static inline struct hfi_inst *to_hfi_inst(struct file *filp)
> +{
> + return to_inst(filp)->hfi_inst;
> +}
> +
> +static inline struct vb2_queue *
> +vidc_to_vb2q(struct file *file, enum v4l2_buf_type type)
> +{
> + struct vidc_inst *inst = to_inst(file);
> +
> + if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
> + return &inst->bufq_cap;
> + else if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> + return &inst->bufq_out;
> +
> + return NULL;
> +}
> +
> +#endif
> diff --git a/drivers/media/platform/qcom/vidc/helpers.c b/drivers/media/platform/qcom/vidc/helpers.c
> new file mode 100644
> index 000000000000..81079f2b5ed1
> --- /dev/null
> +++ b/drivers/media/platform/qcom/vidc/helpers.c
> @@ -0,0 +1,394 @@
> +/*
> + * Copyright (c) 2012-2015, The Linux Foundation. All rights reserved.
> + * Copyright (C) 2016 Linaro Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * 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.
> + *
> + */
> +#include <linux/list.h>
> +#include <linux/mutex.h>
> +#include <linux/pm_runtime.h>
> +#include <media/videobuf2-dma-sg.h>
> +
> +#include "helpers.h"
> +#include "int_bufs.h"
> +#include "load.h"
> +#include "hfi_helper.h"
> +
> +static int session_set_buf(struct vb2_buffer *vb)
> +{
> + struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> + struct vb2_queue *q = vb->vb2_queue;
> + struct vidc_inst *inst = vb2_get_drv_priv(q);
> + struct vidc_core *core = inst->core;
> + struct device *dev = core->dev;
> + struct hfi_core *hfi = &core->hfi;
> + struct vidc_buffer *buf = to_vidc_buffer(vbuf);
> + struct hfi_frame_data fdata;
> + int ret;
> +
> + memset(&fdata, 0, sizeof(fdata));
> +
> + fdata.alloc_len = vb2_plane_size(vb, 0);
> + fdata.device_addr = buf->dma_addr;
> + fdata.timestamp = vb->timestamp;
> + fdata.flags = 0;
> + fdata.clnt_data = buf->dma_addr;
> +
> + if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
> + fdata.buffer_type = HFI_BUFFER_INPUT;
> + fdata.filled_len = vb2_get_plane_payload(vb, 0);
> + fdata.offset = vb->planes[0].data_offset;
> +
> + if (vbuf->flags & V4L2_BUF_FLAG_LAST || !fdata.filled_len)
> + fdata.flags |= HFI_BUFFERFLAG_EOS;
> +
> + ret = vidc_hfi_session_etb(hfi, inst->hfi_inst, &fdata);
> + } else if (q->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
> + fdata.buffer_type = HFI_BUFFER_OUTPUT;
> + fdata.filled_len = 0;
> + fdata.offset = 0;
> +
> + ret = vidc_hfi_session_ftb(hfi, inst->hfi_inst, &fdata);
> + } else {
> + ret = -EINVAL;
> + }
> +
> + if (ret) {
> + dev_err(dev, "failed to set session buffer (%d)\n", ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int session_unregister_bufs(struct vidc_inst *inst)
> +{
> + struct device *dev = inst->core->dev;
> + struct hfi_core *hfi = &inst->core->hfi;
> + struct hfi_buffer_desc *bd;
> + struct vidc_buffer *buf, *tmp;
> + int ret = 0;
> +
> + mutex_lock(&inst->registeredbufs.lock);
> + list_for_each_entry_safe(buf, tmp, &inst->registeredbufs.list,
> + hfi_list) {
> + list_del(&buf->hfi_list);
> + bd = &buf->bd;
> + bd->response_required = 1;
> + ret = vidc_hfi_session_unset_buffers(hfi, inst->hfi_inst, bd);
> + if (ret) {
> + dev_err(dev, "%s: session release buffers failed\n",
> + __func__);
> + break;
> + }
> + }
> + mutex_unlock(&inst->registeredbufs.lock);
> +
> + return ret;
> +}
> +
> +static int session_register_bufs(struct vidc_inst *inst)
> +{
> + struct device *dev = inst->core->dev;
> + struct hfi_core *hfi = &inst->core->hfi;
> + struct hfi_buffer_desc *bd;
> + struct vidc_buffer *buf, *tmp;
> + int ret = 0;
> +
> + mutex_lock(&inst->registeredbufs.lock);
> + list_for_each_entry_safe(buf, tmp, &inst->registeredbufs.list,
> + hfi_list) {
> + bd = &buf->bd;
> + ret = vidc_hfi_session_set_buffers(hfi, inst->hfi_inst, bd);
> + if (ret) {
> + dev_err(dev, "%s: session: set buffer failed\n",
> + __func__);
> + break;
> + }
> + }
> + mutex_unlock(&inst->registeredbufs.lock);
> +
> + return ret;
> +}
> +
> +int vidc_buf_descs(struct vidc_inst *inst, u32 type,
> + struct hfi_buffer_requirements *out)
> +{
> + struct hfi_core *hfi = &inst->core->hfi;
> + u32 ptype = HFI_PROPERTY_CONFIG_BUFFER_REQUIREMENTS;
> + union hfi_get_property hprop;
> + int ret, i;
> +
> + if (out)
> + memset(out, 0, sizeof(*out));
> +
> + ret = vidc_hfi_session_get_property(hfi, inst->hfi_inst, ptype, &hprop);
> + if (ret)
> + return ret;
> +
> + ret = -EINVAL;
> +
> + for (i = 0; i < HFI_BUFFER_TYPE_MAX; i++) {
> + if (hprop.bufreq[i].type != type)
> + continue;
> +
> + if (out)
> + memcpy(out, &hprop.bufreq[i], sizeof(*out));
> + ret = 0;
> + break;
> + }
> +
> + return ret;
> +}
> +
> +int vidc_set_color_format(struct vidc_inst *inst, u32 type, u32 pixfmt)
> +{
> + struct hfi_uncompressed_format_select fmt;
> + struct hfi_core *hfi = &inst->core->hfi;
> + u32 ptype = HFI_PROPERTY_PARAM_UNCOMPRESSED_FORMAT_SELECT;
> + int ret;
> +
> + fmt.buffer_type = type;
> +
> + switch (pixfmt) {
> + case V4L2_PIX_FMT_NV12:
> + fmt.format = HFI_COLOR_FORMAT_NV12;
> + break;
> + case V4L2_PIX_FMT_NV21:
> + fmt.format = HFI_COLOR_FORMAT_NV21;
> + break;
> + default:
> + return -ENOTSUPP;
I'm not really sure how this error code is used, but normally -EINVAL is returned
for invalid pixel formats. -ENOTSUPP is not used by V4L2.
> + }
> +
> + ret = vidc_hfi_session_set_property(hfi, inst->hfi_inst, ptype, &fmt);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +struct vb2_v4l2_buffer *
> +vidc_vb2_find_buf(struct vidc_inst *inst, dma_addr_t addr)
> +{
> + struct vidc_buffer *buf;
> + struct vb2_v4l2_buffer *vb = NULL;
> +
> + mutex_lock(&inst->bufqueue_lock);
> +
> + list_for_each_entry(buf, &inst->bufqueue, list) {
> + if (buf->dma_addr == addr) {
> + vb = &buf->vb;
> + break;
> + }
> + }
> +
> + if (vb)
> + list_del(&buf->list);
> +
> + mutex_unlock(&inst->bufqueue_lock);
> +
> + return vb;
> +}
> +
> +int vidc_vb2_buf_init(struct vb2_buffer *vb)
> +{
> + struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> + struct vb2_queue *q = vb->vb2_queue;
> + struct vidc_inst *inst = vb2_get_drv_priv(q);
> + struct vidc_buffer *buf = to_vidc_buffer(vbuf);
> + struct hfi_buffer_desc *bd = &buf->bd;
> + struct sg_table *sgt;
> +
> + memset(bd, 0, sizeof(*bd));
> +
> + if (q->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
> + return 0;
> +
> + sgt = vb2_dma_sg_plane_desc(vb, 0);
> + if (!sgt)
> + return -EINVAL;
> +
> + bd->buffer_size = vb2_plane_size(vb, 0);
> + bd->buffer_type = HFI_BUFFER_OUTPUT;
> + bd->num_buffers = 1;
> + bd->device_addr = sg_dma_address(sgt->sgl);
> +
> + mutex_lock(&inst->registeredbufs.lock);
> + list_add_tail(&buf->hfi_list, &inst->registeredbufs.list);
> + mutex_unlock(&inst->registeredbufs.lock);
> +
> + return 0;
> +}
> +
> +int vidc_vb2_buf_prepare(struct vb2_buffer *vb)
> +{
> + struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> + struct vidc_buffer *buf = to_vidc_buffer(vbuf);
> + struct sg_table *sgt;
> +
> + sgt = vb2_dma_sg_plane_desc(vb, 0);
> + if (!sgt)
> + return -EINVAL;
> +
> + buf->dma_addr = sg_dma_address(sgt->sgl);
> +
> + return 0;
> +}
> +
> +void vidc_vb2_buf_queue(struct vb2_buffer *vb)
> +{
> + struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> + struct vidc_inst *inst = vb2_get_drv_priv(vb->vb2_queue);
> + struct vidc_core *core = inst->core;
> + struct device *dev = core->dev;
> + struct vidc_buffer *buf = to_vidc_buffer(vbuf);
> + unsigned int state;
> + int ret;
> +
> + mutex_lock(&inst->hfi_inst->lock);
> + state = inst->hfi_inst->state;
> + mutex_unlock(&inst->hfi_inst->lock);
> +
> + if (state == INST_INVALID || state >= INST_STOP) {
> + vb2_buffer_done(vb, VB2_BUF_STATE_ERROR);
> + dev_dbg(dev, "%s: type:%d, invalid instance state\n", __func__,
> + vb->type);
> + return;
> + }
> +
> + mutex_lock(&inst->bufqueue_lock);
> + list_add_tail(&buf->list, &inst->bufqueue);
> + mutex_unlock(&inst->bufqueue_lock);
> +
> + if (!vb2_is_streaming(&inst->bufq_cap) ||
> + !vb2_is_streaming(&inst->bufq_out))
> + return;
> +
> + ret = session_set_buf(vb);
> + if (ret)
> + vb2_buffer_done(vb, VB2_BUF_STATE_ERROR);
> +}
> +
> +void vidc_vb2_stop_streaming(struct vb2_queue *q)
> +{
> + struct vidc_inst *inst = vb2_get_drv_priv(q);
> + struct hfi_inst *hfi_inst = inst->hfi_inst;
> + struct vidc_core *core = inst->core;
> + struct device *dev = core->dev;
> + struct hfi_core *hfi = &core->hfi;
> + int ret, streamoff;
> +
> + mutex_lock(&inst->lock);
> + streamoff = inst->streamoff;
> + mutex_unlock(&inst->lock);
> +
> + if (streamoff)
> + return;
> +
> + mutex_lock(&inst->lock);
> + if (inst->streamon == 0) {
> + mutex_unlock(&inst->lock);
> + return;
> + }
> + mutex_unlock(&inst->lock);
> +
> + ret = vidc_hfi_session_stop(hfi, inst->hfi_inst);
> + if (ret) {
> + dev_err(dev, "session: stop failed (%d)\n", ret);
> + goto abort;
> + }
> +
> + ret = vidc_hfi_session_unload_res(hfi, inst->hfi_inst);
> + if (ret) {
> + dev_err(dev, "session: release resources failed (%d)\n", ret);
> + goto abort;
> + }
> +
> + ret = session_unregister_bufs(inst);
> + if (ret) {
> + dev_err(dev, "failed to release capture buffers: %d\n", ret);
> + goto abort;
> + }
> +
> + ret = internal_bufs_free(inst);
> +
> + if (hfi_inst->state == INST_INVALID || hfi->state == CORE_INVALID) {
> + ret = -EINVAL;
> + goto abort;
> + }
> +
> +abort:
> + if (ret)
> + vidc_hfi_session_abort(hfi, inst->hfi_inst);
> +
> + vidc_scale_clocks(inst->core);
> +
> + ret = vidc_hfi_session_deinit(hfi, inst->hfi_inst);
> +
> + mutex_lock(&inst->lock);
> + inst->streamoff = 1;
> + mutex_unlock(&inst->lock);
> +
> + if (ret)
> + dev_err(dev, "stop streaming failed type: %d, ret: %d\n",
> + q->type, ret);
> +
> + ret = pm_runtime_put_sync(dev);
> + if (ret < 0)
> + dev_err(dev, "%s: pm_runtime_put_sync (%d)\n", __func__, ret);
> +}
> +
> +int vidc_vb2_start_streaming(struct vidc_inst *inst)
> +{
> + struct device *dev = inst->core->dev;
> + struct hfi_core *hfi = &inst->core->hfi;
> + struct vidc_buffer *buf, *n;
> + int ret;
> +
> + ret = session_register_bufs(inst);
> + if (ret)
> + return ret;
> +
> + ret = internal_bufs_alloc(inst);
> + if (ret)
> + return ret;
> +
> + vidc_scale_clocks(inst->core);
> +
> + ret = vidc_hfi_session_load_res(hfi, inst->hfi_inst);
> + if (ret) {
> + dev_err(dev, "session: load resources (%d)\n", ret);
> + return ret;
> + }
> +
> + ret = vidc_hfi_session_start(hfi, inst->hfi_inst);
> + if (ret) {
> + dev_err(dev, "session: start failed (%d)\n", ret);
> + return ret;
> + }
> +
> + mutex_lock(&inst->bufqueue_lock);
> + list_for_each_entry_safe(buf, n, &inst->bufqueue, list) {
> + ret = session_set_buf(&buf->vb.vb2_buf);
> + if (ret)
> + break;
> + }
> + mutex_unlock(&inst->bufqueue_lock);
> +
> + if (!ret) {
> + mutex_lock(&inst->lock);
> + inst->streamon = 1;
> + mutex_unlock(&inst->lock);
> + }
> +
> + return ret;
> +}
Regards,
Hans
[toc] | [prev] | [next] | [standalone]
| From | Stanimir Varbanov <stanimir.varbanov@linaro.org> |
|---|---|
| Date | 2016-08-22 18:10 +0200 |
| Subject | Re: [PATCH 2/8] media: vidc: adding core part and helper functions |
| Message-ID | <s8ZXr-8vn-13@gated-at.bofh.it> |
| In reply to | #1467636 |
Hi Hans,
Thanks for the express comments!
<cut>
>> +
>> +struct vidc_core {
>> + struct list_head list;
>> + void __iomem *base;
>> + int irq;
>> + struct clk *clks[VIDC_CLKS_NUM_MAX];
>> + struct mutex lock;
>> + struct hfi_core hfi;
>> + struct video_device vdev_dec;
>> + struct video_device vdev_enc;
>
> I know that many drivers embed struct video_device, but this can cause subtle
> refcounting problems. I recommend changing this to a pointer and using video_device_alloc().
>
> I have plans to reorganize the way video_devices are allocated and registered in
> the near future, and you might just as well prepare this driver for that by switching
> to a pointer.
OK, thanks for the info, I will change to pointers.
<cut>
>> +
>> +int vidc_set_color_format(struct vidc_inst *inst, u32 type, u32 pixfmt)
>> +{
>> + struct hfi_uncompressed_format_select fmt;
>> + struct hfi_core *hfi = &inst->core->hfi;
>> + u32 ptype = HFI_PROPERTY_PARAM_UNCOMPRESSED_FORMAT_SELECT;
>> + int ret;
>> +
>> + fmt.buffer_type = type;
>> +
>> + switch (pixfmt) {
>> + case V4L2_PIX_FMT_NV12:
>> + fmt.format = HFI_COLOR_FORMAT_NV12;
>> + break;
>> + case V4L2_PIX_FMT_NV21:
>> + fmt.format = HFI_COLOR_FORMAT_NV21;
>> + break;
>> + default:
>> + return -ENOTSUPP;
>
> I'm not really sure how this error code is used, but normally -EINVAL is returned
> for invalid pixel formats. -ENOTSUPP is not used by V4L2.
>
you are right, I need to change this to EINVAL.
--
regards,
Stan
[toc] | [prev] | [next] | [standalone]
| From | Bjorn Andersson <bjorn.andersson@linaro.org> |
|---|---|
| Date | 2016-08-23 05:30 +0200 |
| Subject | Re: [PATCH 5/8] media: vidc: add Host Firmware Interface (HFI) |
| Message-ID | <s9azv-6Qj-5@gated-at.bofh.it> |
| In reply to | #1467617 |
On Mon 22 Aug 06:13 PDT 2016, Stanimir Varbanov wrote:
> This is the implementation of HFI. It is loaded with the
> responsibility to comunicate with the firmware through an
> interface commands and messages.
>
> - hfi.c has interface functions used by the core, decoder
> and encoder parts to comunicate with the firmware. For example
> there are functions for session and core initialisation.
>
I can't help feeling that the split between core.c and hfi.c is a
remnant of a vidc driver supporting both HFI and pre-HFI with the same
v4l code.
What do you think about merging vidc_core with hfi_core and vidc_inst
with hfi_inst? Both seems to be in a 1:1 relationship.
> - hfi_cmds has packetization operations which preparing
> packets to be send from host to firmware.
>
> - hfi_msgs takes care of messages sent from firmware to the
> host.
>
[..]
> diff --git a/drivers/media/platform/qcom/vidc/hfi_cmds.c b/drivers/media/platform/qcom/vidc/hfi_cmds.c
[..]
> +
> +static const struct hfi_packetization_ops hfi_default = {
> + .sys_init = pkt_sys_init,
> + .sys_pc_prep = pkt_sys_pc_prep,
> + .sys_idle_indicator = pkt_sys_idle_indicator,
> + .sys_power_control = pkt_sys_power_control,
> + .sys_set_resource = pkt_sys_set_resource,
> + .sys_release_resource = pkt_sys_unset_resource,
> + .sys_debug_config = pkt_sys_debug_config,
> + .sys_coverage_config = pkt_sys_coverage_config,
> + .sys_ping = pkt_sys_ping,
> + .sys_image_version = pkt_sys_image_version,
> + .ssr_cmd = pkt_ssr_cmd,
> + .session_init = pkt_session_init,
> + .session_cmd = pkt_session_cmd,
> + .session_set_buffers = pkt_session_set_buffers,
> + .session_release_buffers = pkt_session_release_buffers,
> + .session_etb_decoder = pkt_session_etb_decoder,
> + .session_etb_encoder = pkt_session_etb_encoder,
> + .session_ftb = pkt_session_ftb,
> + .session_parse_seq_header = pkt_session_parse_seq_header,
> + .session_get_seq_hdr = pkt_session_get_seq_hdr,
> + .session_flush = pkt_session_flush,
> + .session_get_property = pkt_session_get_property,
> + .session_set_property = pkt_session_set_property,
> +};
> +
> +static const struct hfi_packetization_ops *get_3xx_ops(void)
> +{
> + static struct hfi_packetization_ops hfi_3xx;
> +
> + hfi_3xx = hfi_default;
> + hfi_3xx.session_set_property = pkt_session_set_property_3xx;
> +
> + return &hfi_3xx;
> +}
> +
> +const struct hfi_packetization_ops *
> +hfi_get_pkt_ops(enum hfi_packetization_type type)
The only reasonable argument I can come up with for not just exposing
these as global functions would be that there are 23 of them... Can we
skip the jump table?
> +{
> + switch (type) {
> + case HFI_PACKETIZATION_LEGACY:
> + return &hfi_default;
> + case HFI_PACKETIZATION_3XX:
> + return get_3xx_ops();
> + }
> +
> + return NULL;
> +}
Regards,
Bjorn
[toc] | [prev] | [next] | [standalone]
| From | Bjorn Andersson <bjorn.andersson@linaro.org> |
|---|---|
| Date | 2016-08-23 05:40 +0200 |
| Subject | Re: [PATCH 6/8] media: vidc: add Venus HFI files |
| Message-ID | <s9aJb-6Tl-1@gated-at.bofh.it> |
| In reply to | #1467617 |
On Mon 22 Aug 06:13 PDT 2016, Stanimir Varbanov wrote:
> Here is the implementation of Venus video accelerator low-level
> functionality. It contanins code which setup the registers and
> startup uthe processor, allocate and manipulates with the shared
> memory used for sending commands and receiving messages.
>
> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
> ---
> drivers/media/platform/qcom/vidc/hfi_venus.c | 1539 +++++++++++++++++++++++
> drivers/media/platform/qcom/vidc/hfi_venus.h | 25 +
> drivers/media/platform/qcom/vidc/hfi_venus_io.h | 98 ++
> 3 files changed, 1662 insertions(+)
> create mode 100644 drivers/media/platform/qcom/vidc/hfi_venus.c
> create mode 100644 drivers/media/platform/qcom/vidc/hfi_venus.h
> create mode 100644 drivers/media/platform/qcom/vidc/hfi_venus_io.h
>
> diff --git a/drivers/media/platform/qcom/vidc/hfi_venus.c b/drivers/media/platform/qcom/vidc/hfi_venus.c
[..]
> +
> +static const struct hfi_ops venus_hfi_ops = {
> + .core_init = venus_hfi_core_init,
> + .core_deinit = venus_hfi_core_deinit,
> + .core_ping = venus_hfi_core_ping,
> + .core_trigger_ssr = venus_hfi_core_trigger_ssr,
> +
> + .session_init = venus_hfi_session_init,
> + .session_end = venus_hfi_session_end,
> + .session_abort = venus_hfi_session_abort,
> + .session_flush = venus_hfi_session_flush,
> + .session_start = venus_hfi_session_start,
> + .session_stop = venus_hfi_session_stop,
> + .session_etb = venus_hfi_session_etb,
> + .session_ftb = venus_hfi_session_ftb,
> + .session_set_buffers = venus_hfi_session_set_buffers,
> + .session_release_buffers = venus_hfi_session_release_buffers,
> + .session_load_res = venus_hfi_session_load_res,
> + .session_release_res = venus_hfi_session_release_res,
> + .session_parse_seq_hdr = venus_hfi_session_parse_seq_hdr,
> + .session_get_seq_hdr = venus_hfi_session_get_seq_hdr,
> + .session_set_property = venus_hfi_session_set_property,
> + .session_get_property = venus_hfi_session_get_property,
> +
> + .resume = venus_hfi_resume,
> + .suspend = venus_hfi_suspend,
> +
> + .isr = venus_isr,
> + .isr_thread = venus_isr_thread,
> +};
> +
> +void venus_hfi_destroy(struct hfi_core *hfi)
> +{
> + struct venus_hfi_device *hdev = to_hfi_priv(hfi);
> +
> + venus_interface_queues_release(hdev);
> + mutex_destroy(&hdev->lock);
> + kfree(hdev);
> +}
> +
> +int venus_hfi_create(struct hfi_core *hfi, const struct vidc_resources *res,
> + void __iomem *base)
> +{
Rather than having the core figure out which *_hfi_create() to call I
think this should be the probe() entry point, calling into the core
registering the venus_hfi_ops - a common-probe() in the hfi/vidc core
could still do most of the heavy lifting.
Probing the driver up from the transport rather than for the highest
logical layer allows us to inject a separate hfi_ops for the apr tal
case.
> + struct venus_hfi_device *hdev;
> + int ret;
> +
> + hdev = kzalloc(sizeof(*hdev), GFP_KERNEL);
> + if (!hdev)
> + return -ENOMEM;
> +
> + mutex_init(&hdev->lock);
> +
> + hdev->res = res;
> + hdev->pkt_ops = hfi->pkt_ops;
> + hdev->packetization_type = HFI_PACKETIZATION_LEGACY;
> + hdev->base = base;
> + hdev->dev = hfi->dev;
> + hdev->suspended = true;
> +
> + hfi->priv = hdev;
> + hfi->ops = &venus_hfi_ops;
> + hfi->core_caps = VIDC_ENC_ROTATION_CAPABILITY |
> + VIDC_ENC_SCALING_CAPABILITY |
> + VIDC_ENC_DEINTERLACE_CAPABILITY |
> + VIDC_DEC_MULTI_STREAM_CAPABILITY;
> +
> + ret = venus_interface_queues_init(hdev);
> + if (ret)
> + goto err_kfree;
> +
> + return 0;
> +
> +err_kfree:
> + kfree(hdev);
> + hfi->priv = NULL;
> + hfi->ops = NULL;
> + return ret;
> +}
I'll try to find some time to do a more detailed review of the
implementation.
Regards,
Bjorn
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web