Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1632723 > unrolled thread

[PATCH v8 00/10] Qualcomm video decoder/encoder driver

Started byStanimir Varbanov <stanimir.varbanov@linaro.org>
First post2017-04-28 11:20 +0200
Last post2017-05-05 15:50 +0200
Articles 14 — 5 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH v8 00/10] Qualcomm video decoder/encoder driver Stanimir Varbanov <stanimir.varbanov@linaro.org> - 2017-04-28 11:20 +0200
    [PATCH v8 06/10] media: venus: vdec: add video decoder files Stanimir Varbanov <stanimir.varbanov@linaro.org> - 2017-04-28 11:20 +0200
    [PATCH v8 02/10] media: v4l2-mem2mem: extend m2m APIs for more accurate buffer management Stanimir Varbanov <stanimir.varbanov@linaro.org> - 2017-04-28 11:20 +0200
      Re: [PATCH v8 02/10] media: v4l2-mem2mem: extend m2m APIs for more  accurate buffer management Hans Verkuil <hverkuil@xs4all.nl> - 2017-05-05 13:40 +0200
    Re: [PATCH v8 05/10] media: venus: adding core part and helper  functions Jordan Crouse <jcrouse@codeaurora.org> - 2017-04-29 00:10 +0200
      Re: [PATCH v8 05/10] media: venus: adding core part and helper  functions Bjorn Andersson <bjorn.andersson@linaro.org> - 2017-04-29 22:30 +0200
        Re: [PATCH v8 05/10] media: venus: adding core part and helper  functions Stanimir Varbanov <stanimir.varbanov@linaro.org> - 2017-05-02 11:20 +0200
          Re: [PATCH v8 05/10] media: venus: adding core part and helper  functions Jordan Crouse <jcrouse@codeaurora.org> - 2017-05-03 15:50 +0200
    Re: [PATCH v8 05/10] media: venus: adding core part and helper  functions Sakari Ailus <sakari.ailus@iki.fi> - 2017-04-30 00:30 +0200
      Re: [PATCH v8 05/10] media: venus: adding core part and helper  functions Stanimir Varbanov <stanimir.varbanov@linaro.org> - 2017-05-02 11:00 +0200
        Re: [PATCH v8 05/10] media: venus: adding core part and helper  functions Bjorn Andersson <bjorn.andersson@linaro.org> - 2017-05-02 21:00 +0200
          Re: [PATCH v8 05/10] media: venus: adding core part and helper  functions Stanimir Varbanov <stanimir.varbanov@linaro.org> - 2017-05-05 15:30 +0200
    Re: [PATCH v8 00/10] Qualcomm video decoder/encoder driver Hans Verkuil <hverkuil@xs4all.nl> - 2017-05-05 14:50 +0200
      Re: [PATCH v8 00/10] Qualcomm video decoder/encoder driver Stanimir Varbanov <stanimir.varbanov@linaro.org> - 2017-05-05 15:50 +0200

#1632723 — [PATCH v8 00/10] Qualcomm video decoder/encoder driver

FromStanimir Varbanov <stanimir.varbanov@linaro.org>
Date2017-04-28 11:20 +0200
Subject[PATCH v8 00/10] Qualcomm video decoder/encoder driver
Message-ID<tBaed-5ps-5@gated-at.bofh.it>
Hi everyone,

The changes since v7 are:
  * fixed error path in recovery handler.
  * fixed the logic in helper_vb2_buf_prepare.
  * added comments over venus_format arrays why MPLANE formats are used.
  * added sequence for output queue as well.
  * added COMPILE_TEST Kconfig option for the venus driver. To make
  compile testing of the venus driver possible I had to create a patch
  01/10 which fixing the qcom SCM driver.

I have made various fixes and improvements of the decoder and encoder
to make them work on Venus hw versions 1xx & 3xx (Venus hw v.1xx is found
on SoC apq8016 / db410c SBC board, and Venus hw v.3xx on apq8096).
A brief of the changes:
  * implemented buffer reference handling. This is adding delayed process
  of the newly queued buffers until the firmware release them completely.
  With this in place now vidioc_create_bufs op works properly.
  * implemented vidioc_try_decoder_cmd and vidioc_decoder_cmd v4l2 ioctl
  ops.
  * cleanups and run checkpatch --strict

The patchset is based on next-20170426 and applies cleanly on media_tree
as well.

The report of v4l2-compliance is below patchset diff status.

regards,
Stan
  
Stanimir Varbanov (10):
  firmware: qcom_scm: Fix to allow COMPILE_TEST-ing
  media: v4l2-mem2mem: extend m2m APIs for more accurate buffer
    management
  doc: DT: venus: binding document for Qualcomm video driver
  MAINTAINERS: Add Qualcomm Venus video accelerator driver
  media: venus: adding core part and helper functions
  media: venus: vdec: add video decoder files
  media: venus: venc: add video encoder files
  media: venus: hfi: add Host Firmware Interface (HFI)
  media: venus: hfi: add Venus HFI files
  media: venus: enable building of Venus video driver

 .../devicetree/bindings/media/qcom,venus.txt       |  107 ++
 MAINTAINERS                                        |    8 +
 drivers/firmware/Kconfig                           |    2 +-
 drivers/firmware/qcom_scm.h                        |   72 +-
 drivers/media/platform/Kconfig                     |   13 +
 drivers/media/platform/Makefile                    |    2 +
 drivers/media/platform/qcom/venus/Makefile         |   11 +
 drivers/media/platform/qcom/venus/core.c           |  388 +++++
 drivers/media/platform/qcom/venus/core.h           |  323 ++++
 drivers/media/platform/qcom/venus/firmware.c       |  107 ++
 drivers/media/platform/qcom/venus/firmware.h       |   22 +
 drivers/media/platform/qcom/venus/helpers.c        |  725 +++++++++
 drivers/media/platform/qcom/venus/helpers.h        |   44 +
 drivers/media/platform/qcom/venus/hfi.c            |  522 +++++++
 drivers/media/platform/qcom/venus/hfi.h            |  175 +++
 drivers/media/platform/qcom/venus/hfi_cmds.c       | 1255 ++++++++++++++++
 drivers/media/platform/qcom/venus/hfi_cmds.h       |  304 ++++
 drivers/media/platform/qcom/venus/hfi_helper.h     | 1050 +++++++++++++
 drivers/media/platform/qcom/venus/hfi_msgs.c       | 1056 +++++++++++++
 drivers/media/platform/qcom/venus/hfi_msgs.h       |  283 ++++
 drivers/media/platform/qcom/venus/hfi_venus.c      | 1571 ++++++++++++++++++++
 drivers/media/platform/qcom/venus/hfi_venus.h      |   23 +
 drivers/media/platform/qcom/venus/hfi_venus_io.h   |  113 ++
 drivers/media/platform/qcom/venus/vdec.c           | 1152 ++++++++++++++
 drivers/media/platform/qcom/venus/vdec.h           |   23 +
 drivers/media/platform/qcom/venus/vdec_ctrls.c     |  149 ++
 drivers/media/platform/qcom/venus/venc.c           | 1281 ++++++++++++++++
 drivers/media/platform/qcom/venus/venc.h           |   23 +
 drivers/media/platform/qcom/venus/venc_ctrls.c     |  269 ++++
 drivers/media/v4l2-core/v4l2-mem2mem.c             |   37 +
 include/linux/qcom_scm.h                           |   32 -
 include/media/v4l2-mem2mem.h                       |   92 ++
 32 files changed, 11190 insertions(+), 44 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/qcom,venus.txt
 create mode 100644 drivers/media/platform/qcom/venus/Makefile
 create mode 100644 drivers/media/platform/qcom/venus/core.c
 create mode 100644 drivers/media/platform/qcom/venus/core.h
 create mode 100644 drivers/media/platform/qcom/venus/firmware.c
 create mode 100644 drivers/media/platform/qcom/venus/firmware.h
 create mode 100644 drivers/media/platform/qcom/venus/helpers.c
 create mode 100644 drivers/media/platform/qcom/venus/helpers.h
 create mode 100644 drivers/media/platform/qcom/venus/hfi.c
 create mode 100644 drivers/media/platform/qcom/venus/hfi.h
 create mode 100644 drivers/media/platform/qcom/venus/hfi_cmds.c
 create mode 100644 drivers/media/platform/qcom/venus/hfi_cmds.h
 create mode 100644 drivers/media/platform/qcom/venus/hfi_helper.h
 create mode 100644 drivers/media/platform/qcom/venus/hfi_msgs.c
 create mode 100644 drivers/media/platform/qcom/venus/hfi_msgs.h
 create mode 100644 drivers/media/platform/qcom/venus/hfi_venus.c
 create mode 100644 drivers/media/platform/qcom/venus/hfi_venus.h
 create mode 100644 drivers/media/platform/qcom/venus/hfi_venus_io.h
 create mode 100644 drivers/media/platform/qcom/venus/vdec.c
 create mode 100644 drivers/media/platform/qcom/venus/vdec.h
 create mode 100644 drivers/media/platform/qcom/venus/vdec_ctrls.c
 create mode 100644 drivers/media/platform/qcom/venus/venc.c
 create mode 100644 drivers/media/platform/qcom/venus/venc.h
 create mode 100644 drivers/media/platform/qcom/venus/venc_ctrls.c

dragonboard-410c:~$ ./v4l2-compliance -d /dev/video0
v4l2-compliance SHA   : 8fc88615b49843acb82cd8316d0bc4ab8474cba2

Driver Info:
        Driver name   : qcom-venus
        Card type     : Qualcomm Venus video decoder
        Bus info      : platform:qcom-venus
        Driver version: 4.9.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 (Not Supported)
                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

        Buffer ioctls:
                test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
                test VIDIOC_EXPBUF: OK

Test input 0:


Total: 43, Succeeded: 43, Failed: 0, Warnings: 0



dragonboard-410c:~$ ./v4l2-compliance -d /dev/video1
v4l2-compliance SHA   : 8fc88615b49843acb82cd8316d0bc4ab8474cba2

Driver Info:
        Driver name   : qcom-venus
        Card type     : Qualcomm Venus video encoder
        Bus info      : platform:qcom-venus
        Driver version: 4.9.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: 28 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
 
-- 
2.7.4

[toc] | [next] | [standalone]


#1632725 — [PATCH v8 06/10] media: venus: vdec: add video decoder files

FromStanimir Varbanov <stanimir.varbanov@linaro.org>
Date2017-04-28 11:20 +0200
Subject[PATCH v8 06/10] media: venus: vdec: add video decoder files
Message-ID<tBaef-5ps-37@gated-at.bofh.it>
In reply to#1632723
This consists of video decoder implementation plus decoder
controls.

Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
---
 drivers/media/platform/qcom/venus/vdec.c       | 1152 ++++++++++++++++++++++++
 drivers/media/platform/qcom/venus/vdec.h       |   23 +
 drivers/media/platform/qcom/venus/vdec_ctrls.c |  149 +++
 3 files changed, 1324 insertions(+)
 create mode 100644 drivers/media/platform/qcom/venus/vdec.c
 create mode 100644 drivers/media/platform/qcom/venus/vdec.h
 create mode 100644 drivers/media/platform/qcom/venus/vdec_ctrls.c

diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
new file mode 100644
index 000000000000..3e606b77c8fb
--- /dev/null
+++ b/drivers/media/platform/qcom/venus/vdec.c
@@ -0,0 +1,1152 @@
+/*
+ * Copyright (c) 2012-2016, The Linux Foundation. All rights reserved.
+ * Copyright (C) 2017 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/clk.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/slab.h>
+#include <media/v4l2-ioctl.h>
+#include <media/v4l2-event.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-mem2mem.h>
+#include <media/videobuf2-dma-sg.h>
+
+#include "hfi_venus_io.h"
+#include "core.h"
+#include "helpers.h"
+#include "vdec.h"
+
+static u32 get_framesize_uncompressed(unsigned int plane, u32 width, u32 height)
+{
+	u32 y_stride, uv_stride, y_plane;
+	u32 y_sclines, uv_sclines, uv_plane;
+	u32 size;
+
+	y_stride = ALIGN(width, 128);
+	uv_stride = ALIGN(width, 128);
+	y_sclines = ALIGN(height, 32);
+	uv_sclines = ALIGN(((height + 1) >> 1), 16);
+
+	y_plane = y_stride * y_sclines;
+	uv_plane = uv_stride * uv_sclines + SZ_4K;
+	size = y_plane + uv_plane + SZ_8K;
+
+	return ALIGN(size, SZ_4K);
+}
+
+static u32 get_framesize_compressed(unsigned int width, unsigned int height)
+{
+	return ((width * height * 3 / 2) / 2) + 128;
+}
+
+/*
+ * Three resons to keep MPLANE formats (despite that the number of planes
+ * currently is one):
+ * - the MPLANE formats allow only one plane to be used
+ * - the downstream driver use MPLANE formats too
+ * - future firmware versions could add support for >1 planes
+ */
+static const struct venus_format vdec_formats[] = {
+	{
+		.pixfmt = V4L2_PIX_FMT_NV12,
+		.num_planes = 1,
+		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
+	}, {
+		.pixfmt = V4L2_PIX_FMT_MPEG4,
+		.num_planes = 1,
+		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+	}, {
+		.pixfmt = V4L2_PIX_FMT_MPEG2,
+		.num_planes = 1,
+		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+	}, {
+		.pixfmt = V4L2_PIX_FMT_H263,
+		.num_planes = 1,
+		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+	}, {
+		.pixfmt = V4L2_PIX_FMT_VC1_ANNEX_G,
+		.num_planes = 1,
+		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+	}, {
+		.pixfmt = V4L2_PIX_FMT_VC1_ANNEX_L,
+		.num_planes = 1,
+		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+	}, {
+		.pixfmt = V4L2_PIX_FMT_H264,
+		.num_planes = 1,
+		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+	}, {
+		.pixfmt = V4L2_PIX_FMT_VP8,
+		.num_planes = 1,
+		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+	}, {
+		.pixfmt = V4L2_PIX_FMT_VP9,
+		.num_planes = 1,
+		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+	}, {
+		.pixfmt = V4L2_PIX_FMT_XVID,
+		.num_planes = 1,
+		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
+	},
+};
+
+static const struct venus_format *find_format(u32 pixfmt, u32 type)
+{
+	const struct venus_format *fmt = vdec_formats;
+	unsigned int size = ARRAY_SIZE(vdec_formats);
+	unsigned int i;
+
+	for (i = 0; i < size; i++) {
+		if (fmt[i].pixfmt == pixfmt)
+			break;
+	}
+
+	if (i == size || fmt[i].type != type)
+		return NULL;
+
+	return &fmt[i];
+}
+
+static const struct venus_format *find_format_by_index(int index, u32 type)
+{
+	const struct venus_format *fmt = vdec_formats;
+	unsigned int size = ARRAY_SIZE(vdec_formats);
+	int i, k = 0;
+
+	if (index < 0 || index > size)
+		return NULL;
+
+	for (i = 0; i < size; i++) {
+		if (fmt[i].type != type)
+			continue;
+		if (k == index)
+			break;
+		k++;
+	}
+
+	if (i == size)
+		return NULL;
+
+	return &fmt[i];
+}
+
+static const struct venus_format *
+vdec_try_fmt_common(struct venus_inst *inst, struct v4l2_format *f)
+{
+	struct v4l2_pix_format_mplane *pixmp = &f->fmt.pix_mp;
+	struct v4l2_plane_pix_format *pfmt = pixmp->plane_fmt;
+	const struct venus_format *fmt;
+	unsigned int p;
+
+	memset(pfmt[0].reserved, 0, sizeof(pfmt[0].reserved));
+	memset(pixmp->reserved, 0, sizeof(pixmp->reserved));
+
+	fmt = find_format(pixmp->pixelformat, f->type);
+	if (!fmt) {
+		if (f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
+			pixmp->pixelformat = V4L2_PIX_FMT_NV12;
+		else if (f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
+			pixmp->pixelformat = V4L2_PIX_FMT_H264;
+		else
+			return NULL;
+		fmt = find_format(pixmp->pixelformat, f->type);
+		pixmp->width = 1280;
+		pixmp->height = 720;
+	}
+
+	pixmp->width = clamp(pixmp->width, inst->cap_width.min,
+			     inst->cap_width.max);
+	pixmp->height = clamp(pixmp->height, inst->cap_height.min,
+			      inst->cap_height.max);
+
+	if (f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
+		pixmp->height = ALIGN(pixmp->height, 32);
+
+	if (pixmp->field == V4L2_FIELD_ANY)
+		pixmp->field = V4L2_FIELD_NONE;
+	pixmp->num_planes = fmt->num_planes;
+	pixmp->flags = 0;
+
+	if (f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
+		for (p = 0; p < pixmp->num_planes; p++) {
+			pfmt[p].sizeimage =
+				get_framesize_uncompressed(p, pixmp->width,
+							   pixmp->height);
+			pfmt[p].bytesperline = ALIGN(pixmp->width, 128);
+		}
+	} else {
+		pfmt[0].sizeimage = get_framesize_compressed(pixmp->width,
+							     pixmp->height);
+		pfmt[0].bytesperline = 0;
+	}
+
+	return fmt;
+}
+
+static int vdec_try_fmt(struct file *file, void *fh, struct v4l2_format *f)
+{
+	struct venus_inst *inst = to_inst(file);
+
+	vdec_try_fmt_common(inst, f);
+
+	return 0;
+}
+
+static int vdec_g_fmt(struct file *file, void *fh, struct v4l2_format *f)
+{
+	struct venus_inst *inst = to_inst(file);
+	const struct venus_format *fmt = NULL;
+	struct v4l2_pix_format_mplane *pixmp = &f->fmt.pix_mp;
+
+	if (f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
+		fmt = inst->fmt_cap;
+	else if (f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
+		fmt = inst->fmt_out;
+
+	if (inst->reconfig) {
+		struct v4l2_format format = {};
+
+		inst->out_width = inst->reconfig_width;
+		inst->out_height = inst->reconfig_height;
+		inst->reconfig = false;
+
+		format.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
+		format.fmt.pix_mp.pixelformat = inst->fmt_cap->pixfmt;
+		format.fmt.pix_mp.width = inst->out_width;
+		format.fmt.pix_mp.height = inst->out_height;
+
+		vdec_try_fmt_common(inst, &format);
+
+		inst->width = format.fmt.pix_mp.width;
+		inst->height = format.fmt.pix_mp.height;
+	}
+
+	pixmp->pixelformat = fmt->pixfmt;
+
+	if (f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
+		pixmp->width = inst->width;
+		pixmp->height = inst->height;
+		pixmp->colorspace = inst->colorspace;
+		pixmp->ycbcr_enc = inst->ycbcr_enc;
+		pixmp->quantization = inst->quantization;
+		pixmp->xfer_func = inst->xfer_func;
+	} else if (f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
+		pixmp->width = inst->out_width;
+		pixmp->height = inst->out_height;
+	}
+
+	vdec_try_fmt_common(inst, f);
+
+	return 0;
+}
+
+static int vdec_s_fmt(struct file *file, void *fh, struct v4l2_format *f)
+{
+	struct venus_inst *inst = to_inst(file);
+	struct v4l2_pix_format_mplane *pixmp = &f->fmt.pix_mp;
+	struct v4l2_pix_format_mplane orig_pixmp;
+	const struct venus_format *fmt;
+	struct v4l2_format format;
+	u32 pixfmt_out = 0, pixfmt_cap = 0;
+
+	orig_pixmp = *pixmp;
+
+	fmt = vdec_try_fmt_common(inst, f);
+
+	if (f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
+		pixfmt_out = pixmp->pixelformat;
+		pixfmt_cap = inst->fmt_cap->pixfmt;
+	} else if (f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
+		pixfmt_cap = pixmp->pixelformat;
+		pixfmt_out = inst->fmt_out->pixfmt;
+	}
+
+	memset(&format, 0, sizeof(format));
+
+	format.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
+	format.fmt.pix_mp.pixelformat = pixfmt_out;
+	format.fmt.pix_mp.width = orig_pixmp.width;
+	format.fmt.pix_mp.height = orig_pixmp.height;
+	vdec_try_fmt_common(inst, &format);
+
+	if (f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
+		inst->out_width = format.fmt.pix_mp.width;
+		inst->out_height = format.fmt.pix_mp.height;
+		inst->colorspace = pixmp->colorspace;
+		inst->ycbcr_enc = pixmp->ycbcr_enc;
+		inst->quantization = pixmp->quantization;
+		inst->xfer_func = pixmp->xfer_func;
+	}
+
+	memset(&format, 0, sizeof(format));
+
+	format.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
+	format.fmt.pix_mp.pixelformat = pixfmt_cap;
+	format.fmt.pix_mp.width = orig_pixmp.width;
+	format.fmt.pix_mp.height = orig_pixmp.height;
+	vdec_try_fmt_common(inst, &format);
+
+	inst->width = format.fmt.pix_mp.width;
+	inst->height = format.fmt.pix_mp.height;
+
+	if (f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
+		inst->fmt_out = fmt;
+	else if (f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
+		inst->fmt_cap = fmt;
+
+	return 0;
+}
+
+static int
+vdec_g_selection(struct file *file, void *fh, struct v4l2_selection *s)
+{
+	struct venus_inst *inst = to_inst(file);
+
+	if (s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE &&
+	    s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
+		return -EINVAL;
+
+	switch (s->target) {
+	case V4L2_SEL_TGT_CROP_BOUNDS:
+	case V4L2_SEL_TGT_CROP_DEFAULT:
+	case V4L2_SEL_TGT_CROP:
+		if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
+			return -EINVAL;
+		s->r.width = inst->out_width;
+		s->r.height = inst->out_height;
+		break;
+	case V4L2_SEL_TGT_COMPOSE_BOUNDS:
+	case V4L2_SEL_TGT_COMPOSE_PADDED:
+		if (s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+			return -EINVAL;
+		s->r.width = inst->width;
+		s->r.height = inst->height;
+		break;
+	case V4L2_SEL_TGT_COMPOSE_DEFAULT:
+	case V4L2_SEL_TGT_COMPOSE:
+		if (s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+			return -EINVAL;
+		s->r.width = inst->out_width;
+		s->r.height = inst->out_height;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	s->r.top = 0;
+	s->r.left = 0;
+
+	return 0;
+}
+
+static int
+vdec_querycap(struct file *file, void *fh, struct v4l2_capability *cap)
+{
+	strlcpy(cap->driver, "qcom-venus", sizeof(cap->driver));
+	strlcpy(cap->card, "Qualcomm Venus video decoder", sizeof(cap->card));
+	strlcpy(cap->bus_info, "platform:qcom-venus", sizeof(cap->bus_info));
+
+	return 0;
+}
+
+static int vdec_enum_fmt(struct file *file, void *fh, struct v4l2_fmtdesc *f)
+{
+	const struct venus_format *fmt;
+
+	memset(f->reserved, 0, sizeof(f->reserved));
+
+	fmt = find_format_by_index(f->index, f->type);
+	if (!fmt)
+		return -EINVAL;
+
+	f->pixelformat = fmt->pixfmt;
+
+	return 0;
+}
+
+static int vdec_s_parm(struct file *file, void *fh, struct v4l2_streamparm *a)
+{
+	struct venus_inst *inst = to_inst(file);
+	struct v4l2_captureparm *cap = &a->parm.capture;
+	struct v4l2_fract *timeperframe = &cap->timeperframe;
+	u64 us_per_frame, fps;
+
+	if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE &&
+	    a->type != V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
+		return -EINVAL;
+
+	memset(cap->reserved, 0, sizeof(cap->reserved));
+	if (!timeperframe->denominator)
+		timeperframe->denominator = inst->timeperframe.denominator;
+	if (!timeperframe->numerator)
+		timeperframe->numerator = inst->timeperframe.numerator;
+	cap->readbuffers = 0;
+	cap->extendedmode = 0;
+	cap->capability = V4L2_CAP_TIMEPERFRAME;
+	us_per_frame = timeperframe->numerator * (u64)USEC_PER_SEC;
+	do_div(us_per_frame, timeperframe->denominator);
+
+	if (!us_per_frame)
+		return -EINVAL;
+
+	fps = (u64)USEC_PER_SEC;
+	do_div(fps, us_per_frame);
+
+	inst->fps = fps;
+	inst->timeperframe = *timeperframe;
+
+	return 0;
+}
+
+static int vdec_enum_framesizes(struct file *file, void *fh,
+				struct v4l2_frmsizeenum *fsize)
+{
+	struct venus_inst *inst = to_inst(file);
+	const struct venus_format *fmt;
+
+	fmt = find_format(fsize->pixel_format,
+			  V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
+	if (!fmt) {
+		fmt = find_format(fsize->pixel_format,
+				  V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
+		if (!fmt)
+			return -EINVAL;
+	}
+
+	if (fsize->index)
+		return -EINVAL;
+
+	fsize->type = V4L2_FRMSIZE_TYPE_STEPWISE;
+
+	fsize->stepwise.min_width = inst->cap_width.min;
+	fsize->stepwise.max_width = inst->cap_width.max;
+	fsize->stepwise.step_width = inst->cap_width.step_size;
+	fsize->stepwise.min_height = inst->cap_height.min;
+	fsize->stepwise.max_height = inst->cap_height.max;
+	fsize->stepwise.step_height = inst->cap_height.step_size;
+
+	return 0;
+}
+
+static int vdec_subscribe_event(struct v4l2_fh *fh,
+				const struct v4l2_event_subscription *sub)
+{
+	switch (sub->type) {
+	case V4L2_EVENT_EOS:
+		return v4l2_event_subscribe(fh, sub, 2, NULL);
+	case V4L2_EVENT_SOURCE_CHANGE:
+		return v4l2_src_change_event_subscribe(fh, sub);
+	case V4L2_EVENT_CTRL:
+		return v4l2_ctrl_subscribe_event(fh, sub);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int
+vdec_try_decoder_cmd(struct file *file, void *fh, struct v4l2_decoder_cmd *cmd)
+{
+	if (cmd->cmd != V4L2_DEC_CMD_STOP)
+		return -EINVAL;
+
+	return 0;
+}
+
+static int
+vdec_decoder_cmd(struct file *file, void *fh, struct v4l2_decoder_cmd *cmd)
+{
+	struct venus_inst *inst = to_inst(file);
+	int ret;
+
+	ret = vdec_try_decoder_cmd(file, fh, cmd);
+	if (ret)
+		return ret;
+
+	mutex_lock(&inst->lock);
+	inst->cmd_stop = true;
+	mutex_unlock(&inst->lock);
+
+	hfi_session_flush(inst);
+
+	return 0;
+}
+
+static const struct v4l2_ioctl_ops vdec_ioctl_ops = {
+	.vidioc_querycap = vdec_querycap,
+	.vidioc_enum_fmt_vid_cap_mplane = vdec_enum_fmt,
+	.vidioc_enum_fmt_vid_out_mplane = vdec_enum_fmt,
+	.vidioc_s_fmt_vid_cap_mplane = vdec_s_fmt,
+	.vidioc_s_fmt_vid_out_mplane = vdec_s_fmt,
+	.vidioc_g_fmt_vid_cap_mplane = vdec_g_fmt,
+	.vidioc_g_fmt_vid_out_mplane = vdec_g_fmt,
+	.vidioc_try_fmt_vid_cap_mplane = vdec_try_fmt,
+	.vidioc_try_fmt_vid_out_mplane = vdec_try_fmt,
+	.vidioc_g_selection = vdec_g_selection,
+	.vidioc_reqbufs = v4l2_m2m_ioctl_reqbufs,
+	.vidioc_querybuf = v4l2_m2m_ioctl_querybuf,
+	.vidioc_create_bufs = v4l2_m2m_ioctl_create_bufs,
+	.vidioc_prepare_buf = v4l2_m2m_ioctl_prepare_buf,
+	.vidioc_qbuf = v4l2_m2m_ioctl_qbuf,
+	.vidioc_expbuf = v4l2_m2m_ioctl_expbuf,
+	.vidioc_dqbuf = v4l2_m2m_ioctl_dqbuf,
+	.vidioc_streamon = v4l2_m2m_ioctl_streamon,
+	.vidioc_streamoff = v4l2_m2m_ioctl_streamoff,
+	.vidioc_s_parm = vdec_s_parm,
+	.vidioc_enum_framesizes = vdec_enum_framesizes,
+	.vidioc_subscribe_event = vdec_subscribe_event,
+	.vidioc_unsubscribe_event = v4l2_event_unsubscribe,
+	.vidioc_try_decoder_cmd = vdec_try_decoder_cmd,
+	.vidioc_decoder_cmd = vdec_decoder_cmd,
+};
+
+static int vdec_set_properties(struct venus_inst *inst)
+{
+	struct vdec_controls *ctr = &inst->controls.dec;
+	struct venus_core *core = inst->core;
+	struct hfi_enable en = { .enable = 1 };
+	u32 ptype;
+	int ret;
+
+	if (core->res->hfi_version == HFI_VERSION_1XX) {
+		ptype = HFI_PROPERTY_PARAM_VDEC_CONTINUE_DATA_TRANSFER;
+		ret = hfi_session_set_property(inst, ptype, &en);
+		if (ret)
+			return ret;
+	}
+
+	if (core->res->hfi_version == HFI_VERSION_3XX ||
+	    inst->cap_bufs_mode_dynamic) {
+		struct hfi_buffer_alloc_mode mode;
+
+		ptype = HFI_PROPERTY_PARAM_BUFFER_ALLOC_MODE;
+		mode.type = HFI_BUFFER_OUTPUT;
+		mode.mode = HFI_BUFFER_MODE_DYNAMIC;
+
+		ret = hfi_session_set_property(inst, ptype, &mode);
+		if (ret)
+			return ret;
+	}
+
+	if (ctr->post_loop_deb_mode) {
+		ptype = HFI_PROPERTY_CONFIG_VDEC_POST_LOOP_DEBLOCKER;
+		en.enable = 1;
+		ret = hfi_session_set_property(inst, ptype, &en);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int vdec_init_session(struct venus_inst *inst)
+{
+	int ret;
+
+	ret = hfi_session_init(inst, inst->fmt_out->pixfmt);
+	if (ret)
+		return ret;
+
+	ret = helper_set_input_resolution(inst, inst->out_width,
+					  inst->out_height);
+	if (ret)
+		goto deinit;
+
+	ret = helper_set_color_format(inst, inst->fmt_cap->pixfmt);
+	if (ret)
+		goto deinit;
+
+	return 0;
+deinit:
+	hfi_session_deinit(inst);
+	return ret;
+}
+
+static int vdec_cap_num_buffers(struct venus_inst *inst, unsigned int *num)
+{
+	struct hfi_buffer_requirements bufreq;
+	int ret;
+
+	ret = vdec_init_session(inst);
+	if (ret)
+		return ret;
+
+	ret = helper_get_bufreq(inst, HFI_BUFFER_OUTPUT, &bufreq);
+
+	*num = bufreq.count_actual;
+
+	hfi_session_deinit(inst);
+
+	return ret;
+}
+
+static int vdec_queue_setup(struct vb2_queue *q,
+			    unsigned int *num_buffers, unsigned int *num_planes,
+			    unsigned int sizes[], struct device *alloc_devs[])
+{
+	struct venus_inst *inst = vb2_get_drv_priv(q);
+	unsigned int p, num;
+	int ret = 0;
+
+	if (*num_planes) {
+		if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE &&
+		    *num_planes != inst->fmt_out->num_planes)
+			return -EINVAL;
+
+		if (q->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE &&
+		    *num_planes != inst->fmt_cap->num_planes)
+			return -EINVAL;
+
+		if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE &&
+		    sizes[0] < inst->input_buf_size)
+			return -EINVAL;
+
+		if (q->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE &&
+		    sizes[0] < inst->output_buf_size)
+			return -EINVAL;
+
+		return 0;
+	}
+
+	switch (q->type) {
+	case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
+		*num_planes = inst->fmt_out->num_planes;
+		sizes[0] = get_framesize_compressed(inst->out_width,
+						    inst->out_height);
+		inst->input_buf_size = sizes[0];
+		inst->num_input_bufs = *num_buffers;
+		break;
+	case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
+		*num_planes = inst->fmt_cap->num_planes;
+
+		ret = vdec_cap_num_buffers(inst, &num);
+		if (ret)
+			break;
+
+		*num_buffers = max(*num_buffers, num);
+
+		for (p = 0; p < *num_planes; p++)
+			sizes[p] = get_framesize_uncompressed(p, inst->width,
+							      inst->height);
+
+		inst->num_output_bufs = *num_buffers;
+		inst->output_buf_size = sizes[0];
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	return ret;
+}
+
+static int vdec_verify_conf(struct venus_inst *inst)
+{
+	struct hfi_buffer_requirements bufreq;
+	int ret;
+
+	if (!inst->num_input_bufs || !inst->num_output_bufs)
+		return -EINVAL;
+
+	ret = helper_get_bufreq(inst, HFI_BUFFER_OUTPUT, &bufreq);
+	if (ret)
+		return ret;
+
+	if (inst->num_output_bufs < bufreq.count_actual ||
+	    inst->num_output_bufs < bufreq.count_min)
+		return -EINVAL;
+
+	ret = helper_get_bufreq(inst, HFI_BUFFER_INPUT, &bufreq);
+	if (ret)
+		return ret;
+
+	if (inst->num_input_bufs < bufreq.count_min)
+		return -EINVAL;
+
+	return 0;
+}
+
+static int vdec_start_streaming(struct vb2_queue *q, unsigned int count)
+{
+	struct venus_inst *inst = vb2_get_drv_priv(q);
+	struct venus_core *core = inst->core;
+	u32 ptype;
+	int ret;
+
+	mutex_lock(&inst->lock);
+
+	if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
+		inst->streamon_out = 1;
+	else
+		inst->streamon_cap = 1;
+
+	if (!(inst->streamon_out & inst->streamon_cap)) {
+		mutex_unlock(&inst->lock);
+		return 0;
+	}
+
+	helper_init_instance(inst);
+
+	inst->reconfig = false;
+	inst->sequence_cap = 0;
+	inst->sequence_out = 0;
+	inst->cmd_stop = false;
+
+	ret = vdec_init_session(inst);
+	if (ret)
+		goto bufs_done;
+
+	ret = vdec_set_properties(inst);
+	if (ret)
+		goto deinit_sess;
+
+	if (core->res->hfi_version == HFI_VERSION_3XX) {
+		struct hfi_buffer_size_actual buf_sz;
+
+		ptype = HFI_PROPERTY_PARAM_BUFFER_SIZE_ACTUAL;
+		buf_sz.type = HFI_BUFFER_OUTPUT;
+		buf_sz.size = inst->output_buf_size;
+
+		ret = hfi_session_set_property(inst, ptype, &buf_sz);
+		if (ret)
+			goto deinit_sess;
+	}
+
+	ret = vdec_verify_conf(inst);
+	if (ret)
+		goto deinit_sess;
+
+	ret = helper_set_num_bufs(inst, inst->num_input_bufs, VB2_MAX_FRAME);
+	if (ret)
+		goto deinit_sess;
+
+	ret = helper_vb2_start_streaming(inst);
+	if (ret)
+		goto deinit_sess;
+
+	mutex_unlock(&inst->lock);
+
+	return 0;
+
+deinit_sess:
+	hfi_session_deinit(inst);
+bufs_done:
+	helper_buffers_done(inst, VB2_BUF_STATE_QUEUED);
+	if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
+		inst->streamon_out = 0;
+	else
+		inst->streamon_cap = 0;
+	mutex_unlock(&inst->lock);
+	return ret;
+}
+
+static const struct vb2_ops vdec_vb2_ops = {
+	.queue_setup = vdec_queue_setup,
+	.buf_init = helper_vb2_buf_init,
+	.buf_prepare = helper_vb2_buf_prepare,
+	.start_streaming = vdec_start_streaming,
+	.stop_streaming = helper_vb2_stop_streaming,
+	.buf_queue = helper_vb2_buf_queue,
+};
+
+static void vdec_buf_done(struct venus_inst *inst, unsigned int buf_type,
+			  u32 tag, u32 bytesused, u32 data_offset, u32 flags,
+			  u32 hfi_flags, u64 timestamp_us)
+{
+	enum vb2_buffer_state state = VB2_BUF_STATE_DONE;
+	struct vb2_v4l2_buffer *vbuf;
+	struct vb2_buffer *vb;
+	unsigned int type;
+
+	if (buf_type == HFI_BUFFER_INPUT)
+		type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
+	else
+		type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
+
+	vbuf = helper_find_buf(inst, type, tag);
+	if (!vbuf)
+		return;
+
+	vbuf->flags = flags;
+	vbuf->field = V4L2_FIELD_NONE;
+
+	if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
+		vb = &vbuf->vb2_buf;
+		vb->planes[0].bytesused =
+			max_t(unsigned int, inst->output_buf_size, bytesused);
+		vb->planes[0].data_offset = data_offset;
+		vb->timestamp = timestamp_us * NSEC_PER_USEC;
+		vbuf->sequence = inst->sequence_cap++;
+
+		if (inst->cmd_stop) {
+			vbuf->flags |= V4L2_BUF_FLAG_LAST;
+			inst->cmd_stop = false;
+		}
+
+		if (vbuf->flags & V4L2_BUF_FLAG_LAST) {
+			const struct v4l2_event ev = { .type = V4L2_EVENT_EOS };
+
+			v4l2_event_queue_fh(&inst->fh, &ev);
+		}
+	} else {
+		vbuf->sequence = inst->sequence_out++;
+	}
+
+	if (hfi_flags & HFI_BUFFERFLAG_READONLY)
+		helper_acquire_buf_ref(vbuf);
+
+	if (hfi_flags & HFI_BUFFERFLAG_DATACORRUPT)
+		state = VB2_BUF_STATE_ERROR;
+
+	v4l2_m2m_buf_done(vbuf, state);
+}
+
+static void vdec_event_notify(struct venus_inst *inst, u32 event,
+			      struct hfi_event_data *data)
+{
+	struct venus_core *core = inst->core;
+	struct device *dev = core->dev_dec;
+	static const struct v4l2_event ev = {
+		.type = V4L2_EVENT_SOURCE_CHANGE,
+		.u.src_change.changes = V4L2_EVENT_SRC_CH_RESOLUTION };
+
+	switch (event) {
+	case EVT_SESSION_ERROR:
+		inst->session_error = true;
+		dev_err(dev, "dec: event session error %x\n", inst->error);
+		break;
+	case EVT_SYS_EVENT_CHANGE:
+		switch (data->event_type) {
+		case HFI_EVENT_DATA_SEQUENCE_CHANGED_SUFFICIENT_BUF_RESOURCES:
+			hfi_session_continue(inst);
+			dev_dbg(dev, "event sufficient resources\n");
+			break;
+		case HFI_EVENT_DATA_SEQUENCE_CHANGED_INSUFFICIENT_BUF_RESOURCES:
+			inst->reconfig_height = data->height;
+			inst->reconfig_width = data->width;
+			inst->reconfig = true;
+
+			v4l2_event_queue_fh(&inst->fh, &ev);
+
+			dev_dbg(dev, "event not sufficient resources (%ux%u)\n",
+				data->width, data->height);
+			break;
+		case HFI_EVENT_RELEASE_BUFFER_REFERENCE:
+			helper_release_buf_ref(inst, data->tag);
+			break;
+		default:
+			break;
+		}
+		break;
+	default:
+		break;
+	}
+}
+
+static const struct hfi_inst_ops vdec_hfi_ops = {
+	.buf_done = vdec_buf_done,
+	.event_notify = vdec_event_notify,
+};
+
+static void vdec_inst_init(struct venus_inst *inst)
+{
+	inst->fmt_out = &vdec_formats[6];
+	inst->fmt_cap = &vdec_formats[0];
+	inst->width = 1280;
+	inst->height = ALIGN(720, 32);
+	inst->out_width = 1280;
+	inst->out_height = 720;
+	inst->fps = 30;
+	inst->timeperframe.numerator = 1;
+	inst->timeperframe.denominator = 30;
+
+	inst->cap_width.min = 64;
+	inst->cap_width.max = 1920;
+	if (inst->core->res->hfi_version == HFI_VERSION_3XX)
+		inst->cap_width.max = 3840;
+	inst->cap_width.step_size = 1;
+	inst->cap_height.min = 64;
+	inst->cap_height.max = ALIGN(1080, 32);
+	if (inst->core->res->hfi_version == HFI_VERSION_3XX)
+		inst->cap_height.max = ALIGN(2160, 32);
+	inst->cap_height.step_size = 1;
+	inst->cap_framerate.min = 1;
+	inst->cap_framerate.max = 30;
+	inst->cap_framerate.step_size = 1;
+	inst->cap_mbs_per_frame.min = 16;
+	inst->cap_mbs_per_frame.max = 8160;
+}
+
+static const struct v4l2_m2m_ops vdec_m2m_ops = {
+	.device_run = helper_m2m_device_run,
+	.job_abort = helper_m2m_job_abort,
+};
+
+static int m2m_queue_init(void *priv, struct vb2_queue *src_vq,
+			  struct vb2_queue *dst_vq)
+{
+	struct venus_inst *inst = priv;
+	int ret;
+
+	src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
+	src_vq->io_modes = VB2_MMAP | VB2_DMABUF;
+	src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
+	src_vq->ops = &vdec_vb2_ops;
+	src_vq->mem_ops = &vb2_dma_sg_memops;
+	src_vq->drv_priv = inst;
+	src_vq->buf_struct_size = sizeof(struct venus_buffer);
+	src_vq->allow_zero_bytesused = 1;
+	src_vq->min_buffers_needed = 1;
+	src_vq->dev = inst->core->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->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
+	dst_vq->ops = &vdec_vb2_ops;
+	dst_vq->mem_ops = &vb2_dma_sg_memops;
+	dst_vq->drv_priv = inst;
+	dst_vq->buf_struct_size = sizeof(struct venus_buffer);
+	dst_vq->allow_zero_bytesused = 1;
+	dst_vq->min_buffers_needed = 1;
+	dst_vq->dev = inst->core->dev;
+	ret = vb2_queue_init(dst_vq);
+	if (ret) {
+		vb2_queue_release(src_vq);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int vdec_open(struct file *file)
+{
+	struct venus_core *core = video_drvdata(file);
+	struct venus_inst *inst;
+	int ret;
+
+	inst = kzalloc(sizeof(*inst), GFP_KERNEL);
+	if (!inst)
+		return -ENOMEM;
+
+	INIT_LIST_HEAD(&inst->registeredbufs);
+	INIT_LIST_HEAD(&inst->internalbufs);
+	INIT_LIST_HEAD(&inst->list);
+	mutex_init(&inst->lock);
+
+	inst->core = core;
+	inst->session_type = VIDC_SESSION_TYPE_DEC;
+
+	helper_init_instance(inst);
+
+	ret = pm_runtime_get_sync(core->dev_dec);
+	if (ret < 0)
+		goto err_free_inst;
+
+	ret = vdec_ctrl_init(inst);
+	if (ret)
+		goto err_put_sync;
+
+	ret = hfi_session_create(inst, &vdec_hfi_ops);
+	if (ret)
+		goto err_ctrl_deinit;
+
+	vdec_inst_init(inst);
+
+	/*
+	 * create m2m device for every instance, the m2m context scheduling
+	 * is made by firmware side so we do not need to care about.
+	 */
+	inst->m2m_dev = v4l2_m2m_init(&vdec_m2m_ops);
+	if (IS_ERR(inst->m2m_dev)) {
+		ret = PTR_ERR(inst->m2m_dev);
+		goto err_session_destroy;
+	}
+
+	inst->m2m_ctx = v4l2_m2m_ctx_init(inst->m2m_dev, inst, m2m_queue_init);
+	if (IS_ERR(inst->m2m_ctx)) {
+		ret = PTR_ERR(inst->m2m_ctx);
+		goto err_m2m_release;
+	}
+
+	v4l2_fh_init(&inst->fh, core->vdev_dec);
+
+	inst->fh.ctrl_handler = &inst->ctrl_handler;
+	v4l2_fh_add(&inst->fh);
+	inst->fh.m2m_ctx = inst->m2m_ctx;
+	file->private_data = &inst->fh;
+
+	return 0;
+
+err_m2m_release:
+	v4l2_m2m_release(inst->m2m_dev);
+err_session_destroy:
+	hfi_session_destroy(inst);
+err_ctrl_deinit:
+	vdec_ctrl_deinit(inst);
+err_put_sync:
+	pm_runtime_put_sync(core->dev_dec);
+err_free_inst:
+	kfree(inst);
+	return ret;
+}
+
+static int vdec_close(struct file *file)
+{
+	struct venus_inst *inst = to_inst(file);
+
+	v4l2_m2m_ctx_release(inst->m2m_ctx);
+	v4l2_m2m_release(inst->m2m_dev);
+	vdec_ctrl_deinit(inst);
+	hfi_session_destroy(inst);
+	mutex_destroy(&inst->lock);
+	v4l2_fh_del(&inst->fh);
+	v4l2_fh_exit(&inst->fh);
+	kfree(inst);
+
+	pm_runtime_put_sync(inst->core->dev_dec);
+	return 0;
+}
+
+static const struct v4l2_file_operations vdec_fops = {
+	.owner = THIS_MODULE,
+	.open = vdec_open,
+	.release = vdec_close,
+	.unlocked_ioctl = video_ioctl2,
+	.poll = v4l2_m2m_fop_poll,
+	.mmap = v4l2_m2m_fop_mmap,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl32 = v4l2_compat_ioctl32,
+#endif
+};
+
+static int vdec_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct video_device *vdev;
+	struct venus_core *core;
+	int ret;
+
+	if (!dev->parent)
+		return -EPROBE_DEFER;
+
+	core = dev_get_drvdata(dev->parent);
+	if (!core)
+		return -EPROBE_DEFER;
+
+	if (core->res->hfi_version == HFI_VERSION_3XX) {
+		core->core0_clk = devm_clk_get(dev, "core");
+		if (IS_ERR(core->core0_clk))
+			return PTR_ERR(core->core0_clk);
+	}
+
+	platform_set_drvdata(pdev, core);
+
+	vdev = video_device_alloc();
+	if (!vdev)
+		return -ENOMEM;
+
+	vdev->release = video_device_release;
+	vdev->fops = &vdec_fops;
+	vdev->ioctl_ops = &vdec_ioctl_ops;
+	vdev->vfl_dir = VFL_DIR_M2M;
+	vdev->v4l2_dev = &core->v4l2_dev;
+	vdev->device_caps = V4L2_CAP_VIDEO_M2M_MPLANE | V4L2_CAP_STREAMING;
+
+	ret = video_register_device(vdev, VFL_TYPE_GRABBER, -1);
+	if (ret)
+		goto err_vdev_release;
+
+	core->vdev_dec = vdev;
+	core->dev_dec = dev;
+
+	video_set_drvdata(vdev, core);
+	pm_runtime_enable(dev);
+
+	return 0;
+
+err_vdev_release:
+	video_device_release(vdev);
+	return ret;
+}
+
+static int vdec_remove(struct platform_device *pdev)
+{
+	struct venus_core *core = dev_get_drvdata(pdev->dev.parent);
+
+	video_unregister_device(core->vdev_dec);
+	pm_runtime_disable(core->dev_dec);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM
+static int vdec_runtime_suspend(struct device *dev)
+{
+	struct venus_core *core = dev_get_drvdata(dev);
+
+	if (core->res->hfi_version == HFI_VERSION_1XX)
+		return 0;
+
+	writel(0, core->base + WRAPPER_VDEC_VCODEC_POWER_CONTROL);
+	clk_disable_unprepare(core->core0_clk);
+	writel(1, core->base + WRAPPER_VDEC_VCODEC_POWER_CONTROL);
+
+	return 0;
+}
+
+static int vdec_runtime_resume(struct device *dev)
+{
+	struct venus_core *core = dev_get_drvdata(dev);
+	int ret;
+
+	if (core->res->hfi_version == HFI_VERSION_1XX)
+		return 0;
+
+	writel(0, core->base + WRAPPER_VDEC_VCODEC_POWER_CONTROL);
+	ret = clk_prepare_enable(core->core0_clk);
+	writel(1, core->base + WRAPPER_VDEC_VCODEC_POWER_CONTROL);
+
+	return ret;
+}
+#endif
+
+static const struct dev_pm_ops vdec_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+				pm_runtime_force_resume)
+	SET_RUNTIME_PM_OPS(vdec_runtime_suspend, vdec_runtime_resume, NULL)
+};
+
+static const struct of_device_id vdec_dt_match[] = {
+	{ .compatible = "venus-decoder" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, vdec_dt_match);
+
+static struct platform_driver qcom_venus_dec_driver = {
+	.probe = vdec_probe,
+	.remove = vdec_remove,
+	.driver = {
+		.name = "qcom-venus-decoder",
+		.of_match_table = vdec_dt_match,
+		.pm = &vdec_pm_ops,
+	},
+};
+module_platform_driver(qcom_venus_dec_driver);
+
+MODULE_ALIAS("platform:qcom-venus-decoder");
+MODULE_DESCRIPTION("Qualcomm Venus video decoder driver");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/media/platform/qcom/venus/vdec.h b/drivers/media/platform/qcom/venus/vdec.h
new file mode 100644
index 000000000000..84b672c54d02
--- /dev/null
+++ b/drivers/media/platform/qcom/venus/vdec.h
@@ -0,0 +1,23 @@
+/*
+ * Copyright (c) 2012-2016, The Linux Foundation. All rights reserved.
+ * Copyright (C) 2017 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 __VENUS_VDEC_H__
+#define __VENUS_VDEC_H__
+
+struct venus_inst;
+
+int vdec_ctrl_init(struct venus_inst *inst);
+void vdec_ctrl_deinit(struct venus_inst *inst);
+
+#endif
diff --git a/drivers/media/platform/qcom/venus/vdec_ctrls.c b/drivers/media/platform/qcom/venus/vdec_ctrls.c
new file mode 100644
index 000000000000..c91e2ec37ea4
--- /dev/null
+++ b/drivers/media/platform/qcom/venus/vdec_ctrls.c
@@ -0,0 +1,149 @@
+/*
+ * Copyright (c) 2012-2016, The Linux Foundation. All rights reserved.
+ * Copyright (C) 2017 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/types.h>
+#include <media/v4l2-ctrls.h>
+
+#include "core.h"
+
+static int vdec_op_s_ctrl(struct v4l2_ctrl *ctrl)
+{
+	struct venus_inst *inst = ctrl_to_inst(ctrl);
+	struct vdec_controls *ctr = &inst->controls.dec;
+
+	switch (ctrl->id) {
+	case V4L2_CID_MPEG_VIDEO_DECODER_MPEG4_DEBLOCK_FILTER:
+		ctr->post_loop_deb_mode = ctrl->val;
+		break;
+	case V4L2_CID_MPEG_VIDEO_H264_PROFILE:
+	case V4L2_CID_MPEG_VIDEO_MPEG4_PROFILE:
+	case V4L2_CID_MPEG_VIDEO_VPX_PROFILE:
+		ctr->profile = ctrl->val;
+		break;
+	case V4L2_CID_MPEG_VIDEO_H264_LEVEL:
+	case V4L2_CID_MPEG_VIDEO_MPEG4_LEVEL:
+		ctr->level = ctrl->val;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int vdec_op_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
+{
+	struct venus_inst *inst = ctrl_to_inst(ctrl);
+	struct vdec_controls *ctr = &inst->controls.dec;
+	union hfi_get_property hprop;
+	u32 ptype = HFI_PROPERTY_PARAM_PROFILE_LEVEL_CURRENT;
+	int ret;
+
+	switch (ctrl->id) {
+	case V4L2_CID_MPEG_VIDEO_H264_PROFILE:
+	case V4L2_CID_MPEG_VIDEO_MPEG4_PROFILE:
+	case V4L2_CID_MPEG_VIDEO_VPX_PROFILE:
+		ret = hfi_session_get_property(inst, ptype, &hprop);
+		if (!ret)
+			ctr->profile = hprop.profile_level.profile;
+		ctrl->val = ctr->profile;
+		break;
+	case V4L2_CID_MPEG_VIDEO_H264_LEVEL:
+	case V4L2_CID_MPEG_VIDEO_MPEG4_LEVEL:
+		ret = hfi_session_get_property(inst, ptype, &hprop);
+		if (!ret)
+			ctr->level = hprop.profile_level.level;
+		ctrl->val = ctr->level;
+		break;
+	case V4L2_CID_MPEG_VIDEO_DECODER_MPEG4_DEBLOCK_FILTER:
+		ctrl->val = ctr->post_loop_deb_mode;
+		break;
+	default:
+		return -EINVAL;
+	};
+
+	return 0;
+}
+
+static const struct v4l2_ctrl_ops vdec_ctrl_ops = {
+	.s_ctrl = vdec_op_s_ctrl,
+	.g_volatile_ctrl = vdec_op_g_volatile_ctrl,
+};
+
+int vdec_ctrl_init(struct venus_inst *inst)
+{
+	struct v4l2_ctrl *ctrl;
+	int ret;
+
+	ret = v4l2_ctrl_handler_init(&inst->ctrl_handler, 6);
+	if (ret)
+		return ret;
+
+	ctrl = v4l2_ctrl_new_std_menu(&inst->ctrl_handler, &vdec_ctrl_ops,
+		V4L2_CID_MPEG_VIDEO_MPEG4_PROFILE,
+		V4L2_MPEG_VIDEO_MPEG4_PROFILE_ADVANCED_CODING_EFFICIENCY,
+		~((1 << V4L2_MPEG_VIDEO_MPEG4_PROFILE_SIMPLE) |
+		  (1 << V4L2_MPEG_VIDEO_MPEG4_PROFILE_ADVANCED_SIMPLE)),
+		V4L2_MPEG_VIDEO_MPEG4_PROFILE_SIMPLE);
+	if (ctrl)
+		ctrl->flags |= V4L2_CTRL_FLAG_VOLATILE;
+
+	ctrl = v4l2_ctrl_new_std_menu(&inst->ctrl_handler, &vdec_ctrl_ops,
+				      V4L2_CID_MPEG_VIDEO_MPEG4_LEVEL,
+				      V4L2_MPEG_VIDEO_MPEG4_LEVEL_5,
+				      0, V4L2_MPEG_VIDEO_MPEG4_LEVEL_0);
+	if (ctrl)
+		ctrl->flags |= V4L2_CTRL_FLAG_VOLATILE;
+
+	ctrl = v4l2_ctrl_new_std_menu(&inst->ctrl_handler, &vdec_ctrl_ops,
+		V4L2_CID_MPEG_VIDEO_H264_PROFILE,
+		V4L2_MPEG_VIDEO_H264_PROFILE_MULTIVIEW_HIGH,
+		~((1 << V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE) |
+		  (1 << V4L2_MPEG_VIDEO_H264_PROFILE_CONSTRAINED_BASELINE) |
+		  (1 << V4L2_MPEG_VIDEO_H264_PROFILE_MAIN) |
+		  (1 << V4L2_MPEG_VIDEO_H264_PROFILE_HIGH) |
+		  (1 << V4L2_MPEG_VIDEO_H264_PROFILE_STEREO_HIGH) |
+		  (1 << V4L2_MPEG_VIDEO_H264_PROFILE_MULTIVIEW_HIGH)),
+		V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE);
+	if (ctrl)
+		ctrl->flags |= V4L2_CTRL_FLAG_VOLATILE;
+
+	ctrl = v4l2_ctrl_new_std_menu(&inst->ctrl_handler, &vdec_ctrl_ops,
+				      V4L2_CID_MPEG_VIDEO_H264_LEVEL,
+				      V4L2_MPEG_VIDEO_H264_LEVEL_5_1,
+				      0, V4L2_MPEG_VIDEO_H264_LEVEL_1_0);
+	if (ctrl)
+		ctrl->flags |= V4L2_CTRL_FLAG_VOLATILE;
+
+	ctrl = v4l2_ctrl_new_std(&inst->ctrl_handler, &vdec_ctrl_ops,
+				 V4L2_CID_MPEG_VIDEO_VPX_PROFILE, 0, 3, 1, 0);
+	if (ctrl)
+		ctrl->flags |= V4L2_CTRL_FLAG_VOLATILE;
+
+	v4l2_ctrl_new_std(&inst->ctrl_handler, &vdec_ctrl_ops,
+		V4L2_CID_MPEG_VIDEO_DECODER_MPEG4_DEBLOCK_FILTER, 0, 1, 1, 0);
+
+	ret = inst->ctrl_handler.error;
+	if (ret) {
+		v4l2_ctrl_handler_free(&inst->ctrl_handler);
+		return ret;
+	}
+
+	return 0;
+}
+
+void vdec_ctrl_deinit(struct venus_inst *inst)
+{
+	v4l2_ctrl_handler_free(&inst->ctrl_handler);
+}
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]


#1632726 — [PATCH v8 02/10] media: v4l2-mem2mem: extend m2m APIs for more accurate buffer management

FromStanimir Varbanov <stanimir.varbanov@linaro.org>
Date2017-04-28 11:20 +0200
Subject[PATCH v8 02/10] media: v4l2-mem2mem: extend m2m APIs for more accurate buffer management
Message-ID<tBaef-5ps-43@gated-at.bofh.it>
In reply to#1632723
this add functions for:
  - remove buffers from src/dst queue by index
  - remove exact buffer from src/dst queue

also extends m2m API to iterate over a list of src/dst buffers
in safely and non-safely manner.

Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
---
 drivers/media/v4l2-core/v4l2-mem2mem.c | 37 ++++++++++++++
 include/media/v4l2-mem2mem.h           | 92 ++++++++++++++++++++++++++++++++++
 2 files changed, 129 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c
index 6bc27e7b2a33..f62e68aa04c4 100644
--- a/drivers/media/v4l2-core/v4l2-mem2mem.c
+++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
@@ -126,6 +126,43 @@ void *v4l2_m2m_buf_remove(struct v4l2_m2m_queue_ctx *q_ctx)
 }
 EXPORT_SYMBOL_GPL(v4l2_m2m_buf_remove);
 
+void v4l2_m2m_buf_remove_by_buf(struct v4l2_m2m_queue_ctx *q_ctx,
+				struct vb2_v4l2_buffer *vbuf)
+{
+	struct v4l2_m2m_buffer *b;
+	unsigned long flags;
+
+	spin_lock_irqsave(&q_ctx->rdy_spinlock, flags);
+	b = container_of(vbuf, struct v4l2_m2m_buffer, vb);
+	list_del(&b->list);
+	q_ctx->num_rdy--;
+	spin_unlock_irqrestore(&q_ctx->rdy_spinlock, flags);
+}
+EXPORT_SYMBOL_GPL(v4l2_m2m_buf_remove_by_buf);
+
+struct vb2_v4l2_buffer *
+v4l2_m2m_buf_remove_by_idx(struct v4l2_m2m_queue_ctx *q_ctx, unsigned int idx)
+
+{
+	struct v4l2_m2m_buffer *b, *tmp;
+	struct vb2_v4l2_buffer *ret = NULL;
+	unsigned long flags;
+
+	spin_lock_irqsave(&q_ctx->rdy_spinlock, flags);
+	list_for_each_entry_safe(b, tmp, &q_ctx->rdy_queue, list) {
+		if (b->vb.vb2_buf.index == idx) {
+			list_del(&b->list);
+			q_ctx->num_rdy--;
+			ret = &b->vb;
+			break;
+		}
+	}
+	spin_unlock_irqrestore(&q_ctx->rdy_spinlock, flags);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(v4l2_m2m_buf_remove_by_idx);
+
 /*
  * Scheduling handlers
  */
diff --git a/include/media/v4l2-mem2mem.h b/include/media/v4l2-mem2mem.h
index 3ccd01bd245e..e157d5c9b224 100644
--- a/include/media/v4l2-mem2mem.h
+++ b/include/media/v4l2-mem2mem.h
@@ -437,6 +437,47 @@ static inline void *v4l2_m2m_next_dst_buf(struct v4l2_m2m_ctx *m2m_ctx)
 }
 
 /**
+ * v4l2_m2m_for_each_dst_buf() - iterate over a list of destination ready
+ * buffers
+ *
+ * @m2m_ctx: m2m context assigned to the instance given by struct &v4l2_m2m_ctx
+ * @b: current buffer of type struct v4l2_m2m_buffer
+ */
+#define v4l2_m2m_for_each_dst_buf(m2m_ctx, b)	\
+	list_for_each_entry(b, &m2m_ctx->cap_q_ctx.rdy_queue, list)
+
+/**
+ * v4l2_m2m_for_each_src_buf() - iterate over a list of source ready buffers
+ *
+ * @m2m_ctx: m2m context assigned to the instance given by struct &v4l2_m2m_ctx
+ * @b: current buffer of type struct v4l2_m2m_buffer
+ */
+#define v4l2_m2m_for_each_src_buf(m2m_ctx, b)	\
+	list_for_each_entry(b, &m2m_ctx->out_q_ctx.rdy_queue, list)
+
+/**
+ * v4l2_m2m_for_each_dst_buf_safe() - iterate over a list of destination ready
+ * buffers safely
+ *
+ * @m2m_ctx: m2m context assigned to the instance given by struct &v4l2_m2m_ctx
+ * @b: current buffer of type struct v4l2_m2m_buffer
+ * @n: used as temporary storage
+ */
+#define v4l2_m2m_for_each_dst_buf_safe(m2m_ctx, b, n)	\
+	list_for_each_entry_safe(b, n, &m2m_ctx->cap_q_ctx.rdy_queue, list)
+
+/**
+ * v4l2_m2m_for_each_src_buf_safe() - iterate over a list of source ready
+ * buffers safely
+ *
+ * @m2m_ctx: m2m context assigned to the instance given by struct &v4l2_m2m_ctx
+ * @b: current buffer of type struct v4l2_m2m_buffer
+ * @n: used as temporary storage
+ */
+#define v4l2_m2m_for_each_src_buf_safe(m2m_ctx, b, n)	\
+	list_for_each_entry_safe(b, n, &m2m_ctx->out_q_ctx.rdy_queue, list)
+
+/**
  * v4l2_m2m_get_src_vq() - return vb2_queue for source buffers
  *
  * @m2m_ctx: m2m context assigned to the instance given by struct &v4l2_m2m_ctx
@@ -488,6 +529,57 @@ static inline void *v4l2_m2m_dst_buf_remove(struct v4l2_m2m_ctx *m2m_ctx)
 	return v4l2_m2m_buf_remove(&m2m_ctx->cap_q_ctx);
 }
 
+/**
+ * v4l2_m2m_buf_remove_by_buf() - take off exact buffer from the list of ready
+ * buffers
+ *
+ * @q_ctx: pointer to struct @v4l2_m2m_queue_ctx
+ * @vbuf: the buffer to be removed
+ */
+void v4l2_m2m_buf_remove_by_buf(struct v4l2_m2m_queue_ctx *q_ctx,
+				struct vb2_v4l2_buffer *vbuf);
+
+/**
+ * v4l2_m2m_src_buf_remove_by_buf() - take off exact source buffer from the list
+ * of ready buffers
+ *
+ * @m2m_ctx: m2m context assigned to the instance given by struct &v4l2_m2m_ctx
+ * @vbuf: the buffer to be removed
+ */
+static inline void v4l2_m2m_src_buf_remove_by_buf(struct v4l2_m2m_ctx *m2m_ctx,
+						  struct vb2_v4l2_buffer *vbuf)
+{
+	v4l2_m2m_buf_remove_by_buf(&m2m_ctx->out_q_ctx, vbuf);
+}
+
+/**
+ * v4l2_m2m_dst_buf_remove_by_buf() - take off exact destination buffer from the
+ * list of ready buffers
+ *
+ * @m2m_ctx: m2m context assigned to the instance given by struct &v4l2_m2m_ctx
+ * @vbuf: the buffer to be removed
+ */
+static inline void v4l2_m2m_dst_buf_remove_by_buf(struct v4l2_m2m_ctx *m2m_ctx,
+						  struct vb2_v4l2_buffer *vbuf)
+{
+	v4l2_m2m_buf_remove_by_buf(&m2m_ctx->cap_q_ctx, vbuf);
+}
+
+struct vb2_v4l2_buffer *
+v4l2_m2m_buf_remove_by_idx(struct v4l2_m2m_queue_ctx *q_ctx, unsigned int idx);
+
+static inline struct vb2_v4l2_buffer *
+v4l2_m2m_src_buf_remove_by_idx(struct v4l2_m2m_ctx *m2m_ctx, unsigned int idx)
+{
+	return v4l2_m2m_buf_remove_by_idx(&m2m_ctx->out_q_ctx, idx);
+}
+
+static inline struct vb2_v4l2_buffer *
+v4l2_m2m_dst_buf_remove_by_idx(struct v4l2_m2m_ctx *m2m_ctx, unsigned int idx)
+{
+	return v4l2_m2m_buf_remove_by_idx(&m2m_ctx->cap_q_ctx, idx);
+}
+
 /* v4l2 ioctl helpers */
 
 int v4l2_m2m_ioctl_reqbufs(struct file *file, void *priv,
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]


#1636322 — Re: [PATCH v8 02/10] media: v4l2-mem2mem: extend m2m APIs for more accurate buffer management

FromHans Verkuil <hverkuil@xs4all.nl>
Date2017-05-05 13:40 +0200
SubjectRe: [PATCH v8 02/10] media: v4l2-mem2mem: extend m2m APIs for more accurate buffer management
Message-ID<tDJKy-15q-1@gated-at.bofh.it>
In reply to#1632726
On 04/28/17 11:13, Stanimir Varbanov wrote:
> this add functions for:
>   - remove buffers from src/dst queue by index
>   - remove exact buffer from src/dst queue
> 
> also extends m2m API to iterate over a list of src/dst buffers
> in safely and non-safely manner.
> 
> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>

Reviewed-by: Hans Verkuil <hans.verkuil@cisco.com>

Thanks!

	Hans

> ---
>  drivers/media/v4l2-core/v4l2-mem2mem.c | 37 ++++++++++++++
>  include/media/v4l2-mem2mem.h           | 92 ++++++++++++++++++++++++++++++++++
>  2 files changed, 129 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c
> index 6bc27e7b2a33..f62e68aa04c4 100644
> --- a/drivers/media/v4l2-core/v4l2-mem2mem.c
> +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
> @@ -126,6 +126,43 @@ void *v4l2_m2m_buf_remove(struct v4l2_m2m_queue_ctx *q_ctx)
>  }
>  EXPORT_SYMBOL_GPL(v4l2_m2m_buf_remove);
>  
> +void v4l2_m2m_buf_remove_by_buf(struct v4l2_m2m_queue_ctx *q_ctx,
> +				struct vb2_v4l2_buffer *vbuf)
> +{
> +	struct v4l2_m2m_buffer *b;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&q_ctx->rdy_spinlock, flags);
> +	b = container_of(vbuf, struct v4l2_m2m_buffer, vb);
> +	list_del(&b->list);
> +	q_ctx->num_rdy--;
> +	spin_unlock_irqrestore(&q_ctx->rdy_spinlock, flags);
> +}
> +EXPORT_SYMBOL_GPL(v4l2_m2m_buf_remove_by_buf);
> +
> +struct vb2_v4l2_buffer *
> +v4l2_m2m_buf_remove_by_idx(struct v4l2_m2m_queue_ctx *q_ctx, unsigned int idx)
> +
> +{
> +	struct v4l2_m2m_buffer *b, *tmp;
> +	struct vb2_v4l2_buffer *ret = NULL;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&q_ctx->rdy_spinlock, flags);
> +	list_for_each_entry_safe(b, tmp, &q_ctx->rdy_queue, list) {
> +		if (b->vb.vb2_buf.index == idx) {
> +			list_del(&b->list);
> +			q_ctx->num_rdy--;
> +			ret = &b->vb;
> +			break;
> +		}
> +	}
> +	spin_unlock_irqrestore(&q_ctx->rdy_spinlock, flags);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_m2m_buf_remove_by_idx);
> +
>  /*
>   * Scheduling handlers
>   */
> diff --git a/include/media/v4l2-mem2mem.h b/include/media/v4l2-mem2mem.h
> index 3ccd01bd245e..e157d5c9b224 100644
> --- a/include/media/v4l2-mem2mem.h
> +++ b/include/media/v4l2-mem2mem.h
> @@ -437,6 +437,47 @@ static inline void *v4l2_m2m_next_dst_buf(struct v4l2_m2m_ctx *m2m_ctx)
>  }
>  
>  /**
> + * v4l2_m2m_for_each_dst_buf() - iterate over a list of destination ready
> + * buffers
> + *
> + * @m2m_ctx: m2m context assigned to the instance given by struct &v4l2_m2m_ctx
> + * @b: current buffer of type struct v4l2_m2m_buffer
> + */
> +#define v4l2_m2m_for_each_dst_buf(m2m_ctx, b)	\
> +	list_for_each_entry(b, &m2m_ctx->cap_q_ctx.rdy_queue, list)
> +
> +/**
> + * v4l2_m2m_for_each_src_buf() - iterate over a list of source ready buffers
> + *
> + * @m2m_ctx: m2m context assigned to the instance given by struct &v4l2_m2m_ctx
> + * @b: current buffer of type struct v4l2_m2m_buffer
> + */
> +#define v4l2_m2m_for_each_src_buf(m2m_ctx, b)	\
> +	list_for_each_entry(b, &m2m_ctx->out_q_ctx.rdy_queue, list)
> +
> +/**
> + * v4l2_m2m_for_each_dst_buf_safe() - iterate over a list of destination ready
> + * buffers safely
> + *
> + * @m2m_ctx: m2m context assigned to the instance given by struct &v4l2_m2m_ctx
> + * @b: current buffer of type struct v4l2_m2m_buffer
> + * @n: used as temporary storage
> + */
> +#define v4l2_m2m_for_each_dst_buf_safe(m2m_ctx, b, n)	\
> +	list_for_each_entry_safe(b, n, &m2m_ctx->cap_q_ctx.rdy_queue, list)
> +
> +/**
> + * v4l2_m2m_for_each_src_buf_safe() - iterate over a list of source ready
> + * buffers safely
> + *
> + * @m2m_ctx: m2m context assigned to the instance given by struct &v4l2_m2m_ctx
> + * @b: current buffer of type struct v4l2_m2m_buffer
> + * @n: used as temporary storage
> + */
> +#define v4l2_m2m_for_each_src_buf_safe(m2m_ctx, b, n)	\
> +	list_for_each_entry_safe(b, n, &m2m_ctx->out_q_ctx.rdy_queue, list)
> +
> +/**
>   * v4l2_m2m_get_src_vq() - return vb2_queue for source buffers
>   *
>   * @m2m_ctx: m2m context assigned to the instance given by struct &v4l2_m2m_ctx
> @@ -488,6 +529,57 @@ static inline void *v4l2_m2m_dst_buf_remove(struct v4l2_m2m_ctx *m2m_ctx)
>  	return v4l2_m2m_buf_remove(&m2m_ctx->cap_q_ctx);
>  }
>  
> +/**
> + * v4l2_m2m_buf_remove_by_buf() - take off exact buffer from the list of ready
> + * buffers
> + *
> + * @q_ctx: pointer to struct @v4l2_m2m_queue_ctx
> + * @vbuf: the buffer to be removed
> + */
> +void v4l2_m2m_buf_remove_by_buf(struct v4l2_m2m_queue_ctx *q_ctx,
> +				struct vb2_v4l2_buffer *vbuf);
> +
> +/**
> + * v4l2_m2m_src_buf_remove_by_buf() - take off exact source buffer from the list
> + * of ready buffers
> + *
> + * @m2m_ctx: m2m context assigned to the instance given by struct &v4l2_m2m_ctx
> + * @vbuf: the buffer to be removed
> + */
> +static inline void v4l2_m2m_src_buf_remove_by_buf(struct v4l2_m2m_ctx *m2m_ctx,
> +						  struct vb2_v4l2_buffer *vbuf)
> +{
> +	v4l2_m2m_buf_remove_by_buf(&m2m_ctx->out_q_ctx, vbuf);
> +}
> +
> +/**
> + * v4l2_m2m_dst_buf_remove_by_buf() - take off exact destination buffer from the
> + * list of ready buffers
> + *
> + * @m2m_ctx: m2m context assigned to the instance given by struct &v4l2_m2m_ctx
> + * @vbuf: the buffer to be removed
> + */
> +static inline void v4l2_m2m_dst_buf_remove_by_buf(struct v4l2_m2m_ctx *m2m_ctx,
> +						  struct vb2_v4l2_buffer *vbuf)
> +{
> +	v4l2_m2m_buf_remove_by_buf(&m2m_ctx->cap_q_ctx, vbuf);
> +}
> +
> +struct vb2_v4l2_buffer *
> +v4l2_m2m_buf_remove_by_idx(struct v4l2_m2m_queue_ctx *q_ctx, unsigned int idx);
> +
> +static inline struct vb2_v4l2_buffer *
> +v4l2_m2m_src_buf_remove_by_idx(struct v4l2_m2m_ctx *m2m_ctx, unsigned int idx)
> +{
> +	return v4l2_m2m_buf_remove_by_idx(&m2m_ctx->out_q_ctx, idx);
> +}
> +
> +static inline struct vb2_v4l2_buffer *
> +v4l2_m2m_dst_buf_remove_by_idx(struct v4l2_m2m_ctx *m2m_ctx, unsigned int idx)
> +{
> +	return v4l2_m2m_buf_remove_by_idx(&m2m_ctx->cap_q_ctx, idx);
> +}
> +
>  /* v4l2 ioctl helpers */
>  
>  int v4l2_m2m_ioctl_reqbufs(struct file *file, void *priv,
> 

[toc] | [prev] | [next] | [standalone]


#1633179 — Re: [PATCH v8 05/10] media: venus: adding core part and helper functions

FromJordan Crouse <jcrouse@codeaurora.org>
Date2017-04-29 00:10 +0200
SubjectRe: [PATCH v8 05/10] media: venus: adding core part and helper functions
Message-ID<tBmfo-5oV-5@gated-at.bofh.it>
In reply to#1632723
On Fri, Apr 28, 2017 at 12:13:52PM +0300, Stanimir Varbanov wrote:
> +int venus_boot(struct device *parent, struct device *fw_dev)
> +{
> +	const struct firmware *mdt;
> +	phys_addr_t mem_phys;
> +	ssize_t fw_size;
> +	size_t mem_size;
> +	void *mem_va;
> +	int ret;
> +
> +	if (!qcom_scm_is_available())
> +		return -EPROBE_DEFER;
> +
> +	fw_dev->parent = parent;
> +	fw_dev->release = device_release_dummy;
> +
> +	ret = dev_set_name(fw_dev, "%s:%s", dev_name(parent), "firmware");
> +	if (ret)
> +		return ret;
> +
> +	ret = device_register(fw_dev);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = of_reserved_mem_device_init_by_idx(fw_dev, parent->of_node, 0);
> +	if (ret)
> +		goto err_unreg_device;
> +
> +	mem_size = VENUS_FW_MEM_SIZE;
> +
> +	mem_va = dmam_alloc_coherent(fw_dev, mem_size, &mem_phys, GFP_KERNEL);
> +	if (!mem_va) {
> +		ret = -ENOMEM;
> +		goto err_unreg_device;
> +	}
> +
> +	ret = request_firmware(&mdt, VENUS_FIRMWARE_NAME, fw_dev);
> +	if (ret < 0)
> +		goto err_unreg_device;
> +
> +	fw_size = qcom_mdt_get_size(mdt);
> +	if (fw_size < 0) {
> +		ret = fw_size;
> +		release_firmware(mdt);
> +		goto err_unreg_device;
> +	}
> +
> +	ret = qcom_mdt_load(fw_dev, mdt, VENUS_FIRMWARE_NAME, VENUS_PAS_ID,
> +			    mem_va, mem_phys, mem_size);
> +
> +	release_firmware(mdt);
> +
> +	if (ret)
> +		goto err_unreg_device;
> +
> +	ret = qcom_scm_pas_auth_and_reset(VENUS_PAS_ID);
> +	if (ret)
> +		goto err_unreg_device;
> +
> +	return 0;
> +
> +err_unreg_device:
> +	device_unregister(fw_dev);
> +	return ret;
> +}

Hey, this looks familiar - almost line for line identical to what we'll need to
do for GPU.

Bjorn - Is this enough to qualify for generic status in the mdt_loader code?
I know its just two consumers, but it would save 50 or 60 lines of code between
the two drivers and be easier to maintain.

Jordan

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [prev] | [next] | [standalone]


#1633339 — Re: [PATCH v8 05/10] media: venus: adding core part and helper functions

FromBjorn Andersson <bjorn.andersson@linaro.org>
Date2017-04-29 22:30 +0200
SubjectRe: [PATCH v8 05/10] media: venus: adding core part and helper functions
Message-ID<tBHa9-2lV-9@gated-at.bofh.it>
In reply to#1633179
On Fri 28 Apr 15:02 PDT 2017, Jordan Crouse wrote:

> On Fri, Apr 28, 2017 at 12:13:52PM +0300, Stanimir Varbanov wrote:
> > +int venus_boot(struct device *parent, struct device *fw_dev)
> > +{
> > +	const struct firmware *mdt;
> > +	phys_addr_t mem_phys;
> > +	ssize_t fw_size;
> > +	size_t mem_size;
> > +	void *mem_va;
> > +	int ret;
> > +
> > +	if (!qcom_scm_is_available())
> > +		return -EPROBE_DEFER;
> > +
> > +	fw_dev->parent = parent;
> > +	fw_dev->release = device_release_dummy;
> > +
> > +	ret = dev_set_name(fw_dev, "%s:%s", dev_name(parent), "firmware");
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = device_register(fw_dev);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	ret = of_reserved_mem_device_init_by_idx(fw_dev, parent->of_node, 0);
> > +	if (ret)
> > +		goto err_unreg_device;
> > +
> > +	mem_size = VENUS_FW_MEM_SIZE;
> > +
> > +	mem_va = dmam_alloc_coherent(fw_dev, mem_size, &mem_phys, GFP_KERNEL);
> > +	if (!mem_va) {
> > +		ret = -ENOMEM;
> > +		goto err_unreg_device;
> > +	}
> > +
> > +	ret = request_firmware(&mdt, VENUS_FIRMWARE_NAME, fw_dev);
> > +	if (ret < 0)
> > +		goto err_unreg_device;
> > +
> > +	fw_size = qcom_mdt_get_size(mdt);
> > +	if (fw_size < 0) {
> > +		ret = fw_size;
> > +		release_firmware(mdt);
> > +		goto err_unreg_device;
> > +	}
> > +
> > +	ret = qcom_mdt_load(fw_dev, mdt, VENUS_FIRMWARE_NAME, VENUS_PAS_ID,
> > +			    mem_va, mem_phys, mem_size);
> > +
> > +	release_firmware(mdt);
> > +
> > +	if (ret)
> > +		goto err_unreg_device;
> > +
> > +	ret = qcom_scm_pas_auth_and_reset(VENUS_PAS_ID);
> > +	if (ret)
> > +		goto err_unreg_device;
> > +
> > +	return 0;
> > +
> > +err_unreg_device:
> > +	device_unregister(fw_dev);
> > +	return ret;
> > +}
> 
> Hey, this looks familiar - almost line for line identical to what we'll need to
> do for GPU.
> 
> Bjorn - Is this enough to qualify for generic status in the mdt_loader code?
> I know its just two consumers, but it would save 50 or 60 lines of code between
> the two drivers and be easier to maintain.
> 

I think the code setting up the struct device for memory allocation
should be done during probe of the parent, so that I don't think should
be shared.

The part that allocates memory from a device, loads the mdt into that
memory and calls auth_and_reset() sounds like a useful thing to move to
the mdt_loader.c

Regards,
Bjorn

[toc] | [prev] | [next] | [standalone]


#1634274 — Re: [PATCH v8 05/10] media: venus: adding core part and helper functions

FromStanimir Varbanov <stanimir.varbanov@linaro.org>
Date2017-05-02 11:20 +0200
SubjectRe: [PATCH v8 05/10] media: venus: adding core part and helper functions
Message-ID<tCC8q-4Fn-17@gated-at.bofh.it>
In reply to#1633339
Hi,

On 04/29/2017 11:22 PM, Bjorn Andersson wrote:
> On Fri 28 Apr 15:02 PDT 2017, Jordan Crouse wrote:
> 
>> On Fri, Apr 28, 2017 at 12:13:52PM +0300, Stanimir Varbanov wrote:
>>> +int venus_boot(struct device *parent, struct device *fw_dev)
>>> +{
>>> +	const struct firmware *mdt;
>>> +	phys_addr_t mem_phys;
>>> +	ssize_t fw_size;
>>> +	size_t mem_size;
>>> +	void *mem_va;
>>> +	int ret;
>>> +
>>> +	if (!qcom_scm_is_available())
>>> +		return -EPROBE_DEFER;
>>> +
>>> +	fw_dev->parent = parent;
>>> +	fw_dev->release = device_release_dummy;
>>> +
>>> +	ret = dev_set_name(fw_dev, "%s:%s", dev_name(parent), "firmware");
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	ret = device_register(fw_dev);
>>> +	if (ret < 0)
>>> +		return ret;
>>> +
>>> +	ret = of_reserved_mem_device_init_by_idx(fw_dev, parent->of_node, 0);
>>> +	if (ret)
>>> +		goto err_unreg_device;
>>> +
>>> +	mem_size = VENUS_FW_MEM_SIZE;
>>> +
>>> +	mem_va = dmam_alloc_coherent(fw_dev, mem_size, &mem_phys, GFP_KERNEL);
>>> +	if (!mem_va) {
>>> +		ret = -ENOMEM;
>>> +		goto err_unreg_device;
>>> +	}
>>> +
>>> +	ret = request_firmware(&mdt, VENUS_FIRMWARE_NAME, fw_dev);
>>> +	if (ret < 0)
>>> +		goto err_unreg_device;
>>> +
>>> +	fw_size = qcom_mdt_get_size(mdt);
>>> +	if (fw_size < 0) {
>>> +		ret = fw_size;
>>> +		release_firmware(mdt);
>>> +		goto err_unreg_device;
>>> +	}
>>> +
>>> +	ret = qcom_mdt_load(fw_dev, mdt, VENUS_FIRMWARE_NAME, VENUS_PAS_ID,
>>> +			    mem_va, mem_phys, mem_size);
>>> +
>>> +	release_firmware(mdt);
>>> +
>>> +	if (ret)
>>> +		goto err_unreg_device;
>>> +
>>> +	ret = qcom_scm_pas_auth_and_reset(VENUS_PAS_ID);
>>> +	if (ret)
>>> +		goto err_unreg_device;
>>> +
>>> +	return 0;
>>> +
>>> +err_unreg_device:
>>> +	device_unregister(fw_dev);
>>> +	return ret;
>>> +}
>>
>> Hey, this looks familiar - almost line for line identical to what we'll need to
>> do for GPU.
>>
>> Bjorn - Is this enough to qualify for generic status in the mdt_loader code?
>> I know its just two consumers, but it would save 50 or 60 lines of code between
>> the two drivers and be easier to maintain.
>>
> 
> I think the code setting up the struct device for memory allocation
> should be done during probe of the parent, so that I don't think should
> be shared.
> 
> The part that allocates memory from a device, loads the mdt into that
> memory and calls auth_and_reset() sounds like a useful thing to move to
> the mdt_loader.c

I agree, who is going to do that?

-- 
regards,
Stan

[toc] | [prev] | [next] | [standalone]


#1634980 — Re: [PATCH v8 05/10] media: venus: adding core part and helper functions

FromJordan Crouse <jcrouse@codeaurora.org>
Date2017-05-03 15:50 +0200
SubjectRe: [PATCH v8 05/10] media: venus: adding core part and helper functions
Message-ID<tD2Pg-636-15@gated-at.bofh.it>
In reply to#1634274
On Tue, May 02, 2017 at 12:17:20PM +0300, Stanimir Varbanov wrote:
> Hi,
> 
> On 04/29/2017 11:22 PM, Bjorn Andersson wrote:
> > On Fri 28 Apr 15:02 PDT 2017, Jordan Crouse wrote:
> > 
> >> On Fri, Apr 28, 2017 at 12:13:52PM +0300, Stanimir Varbanov wrote:
> >>> +int venus_boot(struct device *parent, struct device *fw_dev)
> >>> +{
> >>> +	const struct firmware *mdt;
> >>> +	phys_addr_t mem_phys;
> >>> +	ssize_t fw_size;
> >>> +	size_t mem_size;
> >>> +	void *mem_va;
> >>> +	int ret;
> >>> +
> >>> +	if (!qcom_scm_is_available())
> >>> +		return -EPROBE_DEFER;
> >>> +
> >>> +	fw_dev->parent = parent;
> >>> +	fw_dev->release = device_release_dummy;
> >>> +
> >>> +	ret = dev_set_name(fw_dev, "%s:%s", dev_name(parent), "firmware");
> >>> +	if (ret)
> >>> +		return ret;
> >>> +
> >>> +	ret = device_register(fw_dev);
> >>> +	if (ret < 0)
> >>> +		return ret;
> >>> +
> >>> +	ret = of_reserved_mem_device_init_by_idx(fw_dev, parent->of_node, 0);
> >>> +	if (ret)
> >>> +		goto err_unreg_device;
> >>> +
> >>> +	mem_size = VENUS_FW_MEM_SIZE;
> >>> +
> >>> +	mem_va = dmam_alloc_coherent(fw_dev, mem_size, &mem_phys, GFP_KERNEL);
> >>> +	if (!mem_va) {
> >>> +		ret = -ENOMEM;
> >>> +		goto err_unreg_device;
> >>> +	}
> >>> +
> >>> +	ret = request_firmware(&mdt, VENUS_FIRMWARE_NAME, fw_dev);
> >>> +	if (ret < 0)
> >>> +		goto err_unreg_device;
> >>> +
> >>> +	fw_size = qcom_mdt_get_size(mdt);
> >>> +	if (fw_size < 0) {
> >>> +		ret = fw_size;
> >>> +		release_firmware(mdt);
> >>> +		goto err_unreg_device;
> >>> +	}
> >>> +
> >>> +	ret = qcom_mdt_load(fw_dev, mdt, VENUS_FIRMWARE_NAME, VENUS_PAS_ID,
> >>> +			    mem_va, mem_phys, mem_size);
> >>> +
> >>> +	release_firmware(mdt);
> >>> +
> >>> +	if (ret)
> >>> +		goto err_unreg_device;
> >>> +
> >>> +	ret = qcom_scm_pas_auth_and_reset(VENUS_PAS_ID);
> >>> +	if (ret)
> >>> +		goto err_unreg_device;
> >>> +
> >>> +	return 0;
> >>> +
> >>> +err_unreg_device:
> >>> +	device_unregister(fw_dev);
> >>> +	return ret;
> >>> +}
> >>
> >> Hey, this looks familiar - almost line for line identical to what we'll need to
> >> do for GPU.
> >>
> >> Bjorn - Is this enough to qualify for generic status in the mdt_loader code?
> >> I know its just two consumers, but it would save 50 or 60 lines of code between
> >> the two drivers and be easier to maintain.
> >>
> > 
> > I think the code setting up the struct device for memory allocation
> > should be done during probe of the parent, so that I don't think should
> > be shared.
> > 
> > The part that allocates memory from a device, loads the mdt into that
> > memory and calls auth_and_reset() sounds like a useful thing to move to
> > the mdt_loader.c
> 
> I agree, who is going to do that?

I'll volunteer with the caveat that sometimes I'm not as fast on turning around
code as I would like to be given then organization I'm affiliated with. 

That said I do have a stack with my zap shader code pending so it would make
sense to stick it in there.

If the worst happens we can alway merge separately and then consolidate.

Jordan

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [prev] | [next] | [standalone]


#1633359 — Re: [PATCH v8 05/10] media: venus: adding core part and helper functions

FromSakari Ailus <sakari.ailus@iki.fi>
Date2017-04-30 00:30 +0200
SubjectRe: [PATCH v8 05/10] media: venus: adding core part and helper functions
Message-ID<tBJ2h-3vo-5@gated-at.bofh.it>
In reply to#1632723
Hi, Stan!!

On Fri, Apr 28, 2017 at 12:13:52PM +0300, Stanimir Varbanov wrote:
...
> +int helper_get_bufreq(struct venus_inst *inst, u32 type,
> +		      struct hfi_buffer_requirements *req)
> +{
> +	u32 ptype = HFI_PROPERTY_CONFIG_BUFFER_REQUIREMENTS;
> +	union hfi_get_property hprop;
> +	int ret, i;

unsigned int i ? It's an array index...

> +
> +	if (req)
> +		memset(req, 0, sizeof(*req));
> +
> +	ret = hfi_session_get_property(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 (req)
> +			memcpy(req, &hprop.bufreq[i], sizeof(*req));
> +		ret = 0;
> +		break;
> +	}
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(helper_get_bufreq);

As these are global symbols but still specific to a single driver, it'd be
good to have them prefixed with a common prefix. How about "venus"? You
actually already have that in a macro in the header. :-)

> +
> +int helper_set_input_resolution(struct venus_inst *inst, unsigned int width,
> +				unsigned int height)
> +{
> +	u32 ptype = HFI_PROPERTY_PARAM_FRAME_SIZE;
> +	struct hfi_framesize fs;
> +
> +	fs.buffer_type = HFI_BUFFER_INPUT;
> +	fs.width = width;
> +	fs.height = height;
> +
> +	return hfi_session_set_property(inst, ptype, &fs);
> +}
> +EXPORT_SYMBOL_GPL(helper_set_input_resolution);
> +
> +int helper_set_output_resolution(struct venus_inst *inst, unsigned int width,
> +				 unsigned int height)
> +{
> +	u32 ptype = HFI_PROPERTY_PARAM_FRAME_SIZE;
> +	struct hfi_framesize fs;
> +
> +	fs.buffer_type = HFI_BUFFER_OUTPUT;
> +	fs.width = width;
> +	fs.height = height;
> +
> +	return hfi_session_set_property(inst, ptype, &fs);
> +}
> +EXPORT_SYMBOL_GPL(helper_set_output_resolution);
> +
> +int helper_set_num_bufs(struct venus_inst *inst, unsigned int input_bufs,
> +			unsigned int output_bufs)
> +{
> +	u32 ptype = HFI_PROPERTY_PARAM_BUFFER_COUNT_ACTUAL;
> +	struct hfi_buffer_count_actual buf_count;
> +	int ret;
> +
> +	buf_count.type = HFI_BUFFER_INPUT;
> +	buf_count.count_actual = input_bufs;
> +
> +	ret = hfi_session_set_property(inst, ptype, &buf_count);
> +	if (ret)
> +		return ret;
> +
> +	buf_count.type = HFI_BUFFER_OUTPUT;
> +	buf_count.count_actual = output_bufs;
> +
> +	return hfi_session_set_property(inst, ptype, &buf_count);
> +}
> +EXPORT_SYMBOL_GPL(helper_set_num_bufs);
> +
> +int helper_set_color_format(struct venus_inst *inst, u32 pixfmt)
> +{
> +	struct hfi_uncompressed_format_select fmt;
> +	u32 ptype = HFI_PROPERTY_PARAM_UNCOMPRESSED_FORMAT_SELECT;
> +	int ret;
> +
> +	if (inst->session_type == VIDC_SESSION_TYPE_DEC)
> +		fmt.buffer_type = HFI_BUFFER_OUTPUT;
> +	else if (inst->session_type == VIDC_SESSION_TYPE_ENC)
> +		fmt.buffer_type = HFI_BUFFER_INPUT;
> +	else
> +		return -EINVAL;
> +
> +	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 -EINVAL;
> +	}
> +
> +	ret = hfi_session_set_property(inst, ptype, &fmt);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(helper_set_color_format);
> +
> +static void delayed_process_buf_func(struct work_struct *work)
> +{
> +	struct venus_buffer *buf, *n;
> +	struct venus_inst *inst;
> +	int ret;
> +
> +	inst = container_of(work, struct venus_inst, delayed_process_work);
> +
> +	mutex_lock(&inst->lock);
> +
> +	if (!(inst->streamon_out & inst->streamon_cap))
> +		goto unlock;
> +
> +	list_for_each_entry_safe(buf, n, &inst->delayed_process, ref_list) {
> +		if (buf->flags & HFI_BUFFERFLAG_READONLY)
> +			continue;
> +
> +		ret = session_process_buf(inst, &buf->vb);
> +		if (ret)
> +			return_buf_error(inst, &buf->vb);
> +
> +		list_del_init(&buf->ref_list);
> +	}
> +unlock:
> +	mutex_unlock(&inst->lock);
> +}
> +
> +void helper_release_buf_ref(struct venus_inst *inst, unsigned int idx)
> +{
> +	struct venus_buffer *buf;
> +
> +	list_for_each_entry(buf, &inst->registeredbufs, reg_list) {
> +		if (buf->vb.vb2_buf.index == idx) {
> +			buf->flags &= ~HFI_BUFFERFLAG_READONLY;
> +			schedule_work(&inst->delayed_process_work);
> +			break;
> +		}
> +	}
> +}
> +EXPORT_SYMBOL_GPL(helper_release_buf_ref);
> +
> +void helper_acquire_buf_ref(struct vb2_v4l2_buffer *vbuf)
> +{
> +	struct venus_buffer *buf = to_venus_buffer(vbuf);
> +
> +	buf->flags |= HFI_BUFFERFLAG_READONLY;
> +}
> +EXPORT_SYMBOL_GPL(helper_acquire_buf_ref);
> +
> +static int is_buf_refed(struct venus_inst *inst, struct vb2_v4l2_buffer *vbuf)
> +{
> +	struct venus_buffer *buf = to_venus_buffer(vbuf);
> +
> +	if (buf->flags & HFI_BUFFERFLAG_READONLY) {
> +		list_add_tail(&buf->ref_list, &inst->delayed_process);
> +		schedule_work(&inst->delayed_process_work);
> +		return 1;
> +	}
> +
> +	return 0;
> +}
> +
> +struct vb2_v4l2_buffer *
> +helper_find_buf(struct venus_inst *inst, unsigned int type, u32 idx)
> +{
> +	struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;
> +
> +	if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> +		return v4l2_m2m_src_buf_remove_by_idx(m2m_ctx, idx);
> +	else
> +		return v4l2_m2m_dst_buf_remove_by_idx(m2m_ctx, idx);
> +}
> +EXPORT_SYMBOL_GPL(helper_find_buf);
> +
> +int helper_vb2_buf_init(struct vb2_buffer *vb)
> +{
> +	struct venus_inst *inst = vb2_get_drv_priv(vb->vb2_queue);
> +	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> +	struct venus_buffer *buf = to_venus_buffer(vbuf);
> +	struct sg_table *sgt;
> +
> +	sgt = vb2_dma_sg_plane_desc(vb, 0);
> +	if (!sgt)
> +		return -EFAULT;
> +
> +	buf->size = vb2_plane_size(vb, 0);
> +	buf->dma_addr = sg_dma_address(sgt->sgl);
> +
> +	if (vb->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
> +		list_add_tail(&buf->reg_list, &inst->registeredbufs);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(helper_vb2_buf_init);
> +
> +int helper_vb2_buf_prepare(struct vb2_buffer *vb)
> +{
> +	struct venus_inst *inst = vb2_get_drv_priv(vb->vb2_queue);
> +
> +	if (vb->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE &&
> +	    vb2_plane_size(vb, 0) < inst->output_buf_size)
> +		return -EINVAL;
> +	if (vb->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE &&
> +	    vb2_plane_size(vb, 0) < inst->input_buf_size)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(helper_vb2_buf_prepare);
> +
> +void helper_vb2_buf_queue(struct vb2_buffer *vb)
> +{
> +	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> +	struct venus_inst *inst = vb2_get_drv_priv(vb->vb2_queue);
> +	struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;
> +	int ret;
> +
> +	mutex_lock(&inst->lock);
> +
> +	if (inst->cmd_stop) {
> +		vbuf->flags |= V4L2_BUF_FLAG_LAST;
> +		v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_DONE);
> +		inst->cmd_stop = false;
> +		goto unlock;
> +	}
> +
> +	v4l2_m2m_buf_queue(m2m_ctx, vbuf);
> +
> +	if (!(inst->streamon_out & inst->streamon_cap))
> +		goto unlock;
> +
> +	ret = is_buf_refed(inst, vbuf);
> +	if (ret)
> +		goto unlock;
> +
> +	ret = session_process_buf(inst, vbuf);
> +	if (ret)
> +		return_buf_error(inst, vbuf);
> +
> +unlock:
> +	mutex_unlock(&inst->lock);
> +}
> +EXPORT_SYMBOL_GPL(helper_vb2_buf_queue);
> +
> +void helper_buffers_done(struct venus_inst *inst, enum vb2_buffer_state state)
> +{
> +	struct vb2_v4l2_buffer *buf;
> +
> +	while ((buf = v4l2_m2m_src_buf_remove(inst->m2m_ctx)))
> +		v4l2_m2m_buf_done(buf, state);
> +	while ((buf = v4l2_m2m_dst_buf_remove(inst->m2m_ctx)))
> +		v4l2_m2m_buf_done(buf, state);
> +}
> +EXPORT_SYMBOL_GPL(helper_buffers_done);
> +
> +void helper_vb2_stop_streaming(struct vb2_queue *q)
> +{
> +	struct venus_inst *inst = vb2_get_drv_priv(q);
> +	struct venus_core *core = inst->core;
> +	int ret;
> +
> +	mutex_lock(&inst->lock);
> +
> +	if (inst->streamon_out & inst->streamon_cap) {
> +		ret = hfi_session_stop(inst);
> +		ret |= hfi_session_unload_res(inst);
> +		ret |= session_unregister_bufs(inst);
> +		ret |= intbufs_free(inst);
> +		ret |= hfi_session_deinit(inst);
> +
> +		if (inst->session_error || core->sys_error)
> +			ret = -EIO;
> +
> +		if (ret)
> +			hfi_session_abort(inst);
> +
> +		load_scale_clocks(core);
> +	}
> +
> +	helper_buffers_done(inst, VB2_BUF_STATE_ERROR);
> +
> +	if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> +		inst->streamon_out = 0;
> +	else
> +		inst->streamon_cap = 0;
> +
> +	mutex_unlock(&inst->lock);
> +}
> +EXPORT_SYMBOL_GPL(helper_vb2_stop_streaming);
> +
> +int helper_vb2_start_streaming(struct venus_inst *inst)
> +{
> +	struct venus_core *core = inst->core;
> +	int ret;
> +
> +	ret = intbufs_alloc(inst);
> +	if (ret)
> +		return ret;
> +
> +	ret = session_register_bufs(inst);
> +	if (ret)
> +		goto err_bufs_free;
> +
> +	load_scale_clocks(core);
> +
> +	ret = hfi_session_load_res(inst);
> +	if (ret)
> +		goto err_unreg_bufs;
> +
> +	ret = hfi_session_start(inst);
> +	if (ret)
> +		goto err_unload_res;
> +
> +	return 0;
> +
> +err_unload_res:
> +	hfi_session_unload_res(inst);
> +err_unreg_bufs:
> +	session_unregister_bufs(inst);
> +err_bufs_free:
> +	intbufs_free(inst);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(helper_vb2_start_streaming);
> +
> +void helper_m2m_device_run(void *priv)
> +{
> +	struct venus_inst *inst = priv;
> +	struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;
> +	struct v4l2_m2m_buffer *buf, *n;
> +	int ret;
> +
> +	mutex_lock(&inst->lock);
> +
> +	v4l2_m2m_for_each_dst_buf_safe(m2m_ctx, buf, n) {
> +		ret = session_process_buf(inst, &buf->vb);
> +		if (ret)
> +			return_buf_error(inst, &buf->vb);
> +	}
> +
> +	v4l2_m2m_for_each_src_buf_safe(m2m_ctx, buf, n) {
> +		ret = session_process_buf(inst, &buf->vb);
> +		if (ret)
> +			return_buf_error(inst, &buf->vb);
> +	}
> +
> +	mutex_unlock(&inst->lock);
> +}
> +EXPORT_SYMBOL_GPL(helper_m2m_device_run);
> +
> +void helper_m2m_job_abort(void *priv)
> +{
> +	struct venus_inst *inst = priv;
> +
> +	v4l2_m2m_job_finish(inst->m2m_dev, inst->m2m_ctx);
> +}
> +EXPORT_SYMBOL_GPL(helper_m2m_job_abort);
> +
> +void helper_init_instance(struct venus_inst *inst)
> +{
> +	if (inst->session_type == VIDC_SESSION_TYPE_DEC) {
> +		INIT_LIST_HEAD(&inst->delayed_process);
> +		INIT_WORK(&inst->delayed_process_work,
> +			  delayed_process_buf_func);
> +	}
> +}
> +EXPORT_SYMBOL_GPL(helper_init_instance);
> diff --git a/drivers/media/platform/qcom/venus/helpers.h b/drivers/media/platform/qcom/venus/helpers.h
> new file mode 100644
> index 000000000000..1ff5005b5add
> --- /dev/null
> +++ b/drivers/media/platform/qcom/venus/helpers.h
> @@ -0,0 +1,44 @@
> +/*
> + * Copyright (c) 2012-2016, The Linux Foundation. All rights reserved.
> + * Copyright (C) 2017 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 __VENUS_HELPERS_H__
> +#define __VENUS_HELPERS_H__
> +
> +#include <media/videobuf2-v4l2.h>
> +
> +struct venus_inst;
> +
> +struct vb2_v4l2_buffer *helper_find_buf(struct venus_inst *inst,
> +					unsigned int type, u32 idx);
> +void helper_buffers_done(struct venus_inst *inst, enum vb2_buffer_state state);
> +int helper_vb2_buf_init(struct vb2_buffer *vb);
> +int helper_vb2_buf_prepare(struct vb2_buffer *vb);
> +void helper_vb2_buf_queue(struct vb2_buffer *vb);
> +void helper_vb2_stop_streaming(struct vb2_queue *q);
> +int helper_vb2_start_streaming(struct venus_inst *inst);
> +void helper_m2m_device_run(void *priv);
> +void helper_m2m_job_abort(void *priv);
> +int helper_get_bufreq(struct venus_inst *inst, u32 type,
> +		      struct hfi_buffer_requirements *req);
> +int helper_set_input_resolution(struct venus_inst *inst, unsigned int width,
> +				unsigned int height);
> +int helper_set_output_resolution(struct venus_inst *inst, unsigned int width,
> +				 unsigned int height);
> +int helper_set_num_bufs(struct venus_inst *inst, unsigned int input_bufs,
> +			unsigned int output_bufs);
> +int helper_set_color_format(struct venus_inst *inst, u32 fmt);
> +void helper_acquire_buf_ref(struct vb2_v4l2_buffer *vbuf);
> +void helper_release_buf_ref(struct venus_inst *inst, unsigned int idx);
> +void helper_init_instance(struct venus_inst *inst);
> +#endif
> 

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

[toc] | [prev] | [next] | [standalone]


#1634265 — Re: [PATCH v8 05/10] media: venus: adding core part and helper functions

FromStanimir Varbanov <stanimir.varbanov@linaro.org>
Date2017-05-02 11:00 +0200
SubjectRe: [PATCH v8 05/10] media: venus: adding core part and helper functions
Message-ID<tCBP4-4jy-21@gated-at.bofh.it>
In reply to#1633359
Hei Sakari,

On 04/30/2017 01:21 AM, Sakari Ailus wrote:
> Hi, Stan!!
> 
> On Fri, Apr 28, 2017 at 12:13:52PM +0300, Stanimir Varbanov wrote:
> ...
>> +int helper_get_bufreq(struct venus_inst *inst, u32 type,
>> +		      struct hfi_buffer_requirements *req)
>> +{
>> +	u32 ptype = HFI_PROPERTY_CONFIG_BUFFER_REQUIREMENTS;
>> +	union hfi_get_property hprop;
>> +	int ret, i;
> 
> unsigned int i ? It's an array index...

Thanks for pointing that out, I have to revisit all similar places as
well ...

> 
>> +
>> +	if (req)
>> +		memset(req, 0, sizeof(*req));
>> +
>> +	ret = hfi_session_get_property(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 (req)
>> +			memcpy(req, &hprop.bufreq[i], sizeof(*req));
>> +		ret = 0;
>> +		break;
>> +	}
>> +
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(helper_get_bufreq);
> 
> As these are global symbols but still specific to a single driver, it'd be
> good to have them prefixed with a common prefix. How about "venus"? You
> actually already have that in a macro in the header. :-)

You are damned right, will rework that in next version.

<snip>

-- 
regards,
Stan

[toc] | [prev] | [next] | [standalone]


#1634584 — Re: [PATCH v8 05/10] media: venus: adding core part and helper functions

FromBjorn Andersson <bjorn.andersson@linaro.org>
Date2017-05-02 21:00 +0200
SubjectRe: [PATCH v8 05/10] media: venus: adding core part and helper functions
Message-ID<tCLbI-1Zs-13@gated-at.bofh.it>
In reply to#1634265
On Tue 02 May 01:52 PDT 2017, Stanimir Varbanov wrote:

> Hei Sakari,
> 
> On 04/30/2017 01:21 AM, Sakari Ailus wrote:
> > Hi, Stan!!
> > 
> > On Fri, Apr 28, 2017 at 12:13:52PM +0300, Stanimir Varbanov wrote:
> > ...
> >> +int helper_get_bufreq(struct venus_inst *inst, u32 type,
> >> +		      struct hfi_buffer_requirements *req)
> >> +{
> >> +	u32 ptype = HFI_PROPERTY_CONFIG_BUFFER_REQUIREMENTS;
> >> +	union hfi_get_property hprop;
> >> +	int ret, i;
> > 
> > unsigned int i ? It's an array index...
> 
> Thanks for pointing that out, I have to revisit all similar places as
> well ...
> 

It's perfectly fine to index an array with an int and you are comparing
the index with a integer constant in the loop - so don't clutter the
code unnecessarily.

Regards,
Bjorn

[toc] | [prev] | [next] | [standalone]


#1636372 — Re: [PATCH v8 05/10] media: venus: adding core part and helper functions

FromStanimir Varbanov <stanimir.varbanov@linaro.org>
Date2017-05-05 15:30 +0200
SubjectRe: [PATCH v8 05/10] media: venus: adding core part and helper functions
Message-ID<tDLsZ-2gd-7@gated-at.bofh.it>
In reply to#1634584
Hi Bjorn

On 05/02/2017 09:52 PM, Bjorn Andersson wrote:
> On Tue 02 May 01:52 PDT 2017, Stanimir Varbanov wrote:
> 
>> Hei Sakari,
>>
>> On 04/30/2017 01:21 AM, Sakari Ailus wrote:
>>> Hi, Stan!!
>>>
>>> On Fri, Apr 28, 2017 at 12:13:52PM +0300, Stanimir Varbanov wrote:
>>> ...
>>>> +int helper_get_bufreq(struct venus_inst *inst, u32 type,
>>>> +		      struct hfi_buffer_requirements *req)
>>>> +{
>>>> +	u32 ptype = HFI_PROPERTY_CONFIG_BUFFER_REQUIREMENTS;
>>>> +	union hfi_get_property hprop;
>>>> +	int ret, i;
>>>
>>> unsigned int i ? It's an array index...
>>
>> Thanks for pointing that out, I have to revisit all similar places as
>> well ...
>>
> 
> It's perfectly fine to index an array with an int and you are comparing
> the index with a integer constant in the loop - so don't clutter the
> code unnecessarily.

I personally prefer unsigned for iterator variable type (because
unsigned type has defined behavior on overflow), but having the fact
that I'm comparing with int I will keep it int.

Also it seems that -Wsign-compare is not enabled by default in kernel,
no? So I have modified my Makefile and catch few occurrences of warnings
about signed with unsigned compare and fixed them.

-- 
regards,
Stan

[toc] | [prev] | [next] | [standalone]


#1636359

FromHans Verkuil <hverkuil@xs4all.nl>
Date2017-05-05 14:50 +0200
Message-ID<tDKQi-1Mu-7@gated-at.bofh.it>
In reply to#1632723
Hi Stanimir,

It looks good to me. I do think that patch 01/10 shouldn't go through
media. This might mean that we have to drop the COMPILE_TEST dependency
on the media driver until this firmware driver patch gets merged, which
is fine with me as long as this is clearly stated in the commit log for
the media Kconfig. Let me know what you want to do with this.

I also saw some comments for patch 05/10, but I'm not sure if that would
block merging this driver or can be fixed afterwards.

Regards,

	Hans

On 04/28/17 11:13, Stanimir Varbanov wrote:
> Hi everyone,
> 
> The changes since v7 are:
>   * fixed error path in recovery handler.
>   * fixed the logic in helper_vb2_buf_prepare.
>   * added comments over venus_format arrays why MPLANE formats are used.
>   * added sequence for output queue as well.
>   * added COMPILE_TEST Kconfig option for the venus driver. To make
>   compile testing of the venus driver possible I had to create a patch
>   01/10 which fixing the qcom SCM driver.
> 
> I have made various fixes and improvements of the decoder and encoder
> to make them work on Venus hw versions 1xx & 3xx (Venus hw v.1xx is found
> on SoC apq8016 / db410c SBC board, and Venus hw v.3xx on apq8096).
> A brief of the changes:
>   * implemented buffer reference handling. This is adding delayed process
>   of the newly queued buffers until the firmware release them completely.
>   With this in place now vidioc_create_bufs op works properly.
>   * implemented vidioc_try_decoder_cmd and vidioc_decoder_cmd v4l2 ioctl
>   ops.
>   * cleanups and run checkpatch --strict
> 
> The patchset is based on next-20170426 and applies cleanly on media_tree
> as well.
> 
> The report of v4l2-compliance is below patchset diff status.
> 
> regards,
> Stan
>   
> Stanimir Varbanov (10):
>   firmware: qcom_scm: Fix to allow COMPILE_TEST-ing
>   media: v4l2-mem2mem: extend m2m APIs for more accurate buffer
>     management
>   doc: DT: venus: binding document for Qualcomm video driver
>   MAINTAINERS: Add Qualcomm Venus video accelerator driver
>   media: venus: adding core part and helper functions
>   media: venus: vdec: add video decoder files
>   media: venus: venc: add video encoder files
>   media: venus: hfi: add Host Firmware Interface (HFI)
>   media: venus: hfi: add Venus HFI files
>   media: venus: enable building of Venus video driver
> 
>  .../devicetree/bindings/media/qcom,venus.txt       |  107 ++
>  MAINTAINERS                                        |    8 +
>  drivers/firmware/Kconfig                           |    2 +-
>  drivers/firmware/qcom_scm.h                        |   72 +-
>  drivers/media/platform/Kconfig                     |   13 +
>  drivers/media/platform/Makefile                    |    2 +
>  drivers/media/platform/qcom/venus/Makefile         |   11 +
>  drivers/media/platform/qcom/venus/core.c           |  388 +++++
>  drivers/media/platform/qcom/venus/core.h           |  323 ++++
>  drivers/media/platform/qcom/venus/firmware.c       |  107 ++
>  drivers/media/platform/qcom/venus/firmware.h       |   22 +
>  drivers/media/platform/qcom/venus/helpers.c        |  725 +++++++++
>  drivers/media/platform/qcom/venus/helpers.h        |   44 +
>  drivers/media/platform/qcom/venus/hfi.c            |  522 +++++++
>  drivers/media/platform/qcom/venus/hfi.h            |  175 +++
>  drivers/media/platform/qcom/venus/hfi_cmds.c       | 1255 ++++++++++++++++
>  drivers/media/platform/qcom/venus/hfi_cmds.h       |  304 ++++
>  drivers/media/platform/qcom/venus/hfi_helper.h     | 1050 +++++++++++++
>  drivers/media/platform/qcom/venus/hfi_msgs.c       | 1056 +++++++++++++
>  drivers/media/platform/qcom/venus/hfi_msgs.h       |  283 ++++
>  drivers/media/platform/qcom/venus/hfi_venus.c      | 1571 ++++++++++++++++++++
>  drivers/media/platform/qcom/venus/hfi_venus.h      |   23 +
>  drivers/media/platform/qcom/venus/hfi_venus_io.h   |  113 ++
>  drivers/media/platform/qcom/venus/vdec.c           | 1152 ++++++++++++++
>  drivers/media/platform/qcom/venus/vdec.h           |   23 +
>  drivers/media/platform/qcom/venus/vdec_ctrls.c     |  149 ++
>  drivers/media/platform/qcom/venus/venc.c           | 1281 ++++++++++++++++
>  drivers/media/platform/qcom/venus/venc.h           |   23 +
>  drivers/media/platform/qcom/venus/venc_ctrls.c     |  269 ++++
>  drivers/media/v4l2-core/v4l2-mem2mem.c             |   37 +
>  include/linux/qcom_scm.h                           |   32 -
>  include/media/v4l2-mem2mem.h                       |   92 ++
>  32 files changed, 11190 insertions(+), 44 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/media/qcom,venus.txt
>  create mode 100644 drivers/media/platform/qcom/venus/Makefile
>  create mode 100644 drivers/media/platform/qcom/venus/core.c
>  create mode 100644 drivers/media/platform/qcom/venus/core.h
>  create mode 100644 drivers/media/platform/qcom/venus/firmware.c
>  create mode 100644 drivers/media/platform/qcom/venus/firmware.h
>  create mode 100644 drivers/media/platform/qcom/venus/helpers.c
>  create mode 100644 drivers/media/platform/qcom/venus/helpers.h
>  create mode 100644 drivers/media/platform/qcom/venus/hfi.c
>  create mode 100644 drivers/media/platform/qcom/venus/hfi.h
>  create mode 100644 drivers/media/platform/qcom/venus/hfi_cmds.c
>  create mode 100644 drivers/media/platform/qcom/venus/hfi_cmds.h
>  create mode 100644 drivers/media/platform/qcom/venus/hfi_helper.h
>  create mode 100644 drivers/media/platform/qcom/venus/hfi_msgs.c
>  create mode 100644 drivers/media/platform/qcom/venus/hfi_msgs.h
>  create mode 100644 drivers/media/platform/qcom/venus/hfi_venus.c
>  create mode 100644 drivers/media/platform/qcom/venus/hfi_venus.h
>  create mode 100644 drivers/media/platform/qcom/venus/hfi_venus_io.h
>  create mode 100644 drivers/media/platform/qcom/venus/vdec.c
>  create mode 100644 drivers/media/platform/qcom/venus/vdec.h
>  create mode 100644 drivers/media/platform/qcom/venus/vdec_ctrls.c
>  create mode 100644 drivers/media/platform/qcom/venus/venc.c
>  create mode 100644 drivers/media/platform/qcom/venus/venc.h
>  create mode 100644 drivers/media/platform/qcom/venus/venc_ctrls.c
> 
> dragonboard-410c:~$ ./v4l2-compliance -d /dev/video0
> v4l2-compliance SHA   : 8fc88615b49843acb82cd8316d0bc4ab8474cba2
> 
> Driver Info:
>         Driver name   : qcom-venus
>         Card type     : Qualcomm Venus video decoder
>         Bus info      : platform:qcom-venus
>         Driver version: 4.9.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 (Not Supported)
>                 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
> 
>         Buffer ioctls:
>                 test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
>                 test VIDIOC_EXPBUF: OK
> 
> Test input 0:
> 
> 
> Total: 43, Succeeded: 43, Failed: 0, Warnings: 0
> 
> 
> 
> dragonboard-410c:~$ ./v4l2-compliance -d /dev/video1
> v4l2-compliance SHA   : 8fc88615b49843acb82cd8316d0bc4ab8474cba2
> 
> Driver Info:
>         Driver name   : qcom-venus
>         Card type     : Qualcomm Venus video encoder
>         Bus info      : platform:qcom-venus
>         Driver version: 4.9.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: 28 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
>  
> 

[toc] | [prev] | [next] | [standalone]


#1636389

FromStanimir Varbanov <stanimir.varbanov@linaro.org>
Date2017-05-05 15:50 +0200
Message-ID<tDLMl-2qU-11@gated-at.bofh.it>
In reply to#1636359
Hi Hans,

On 05/05/2017 03:44 PM, Hans Verkuil wrote:
> Hi Stanimir,
> 
> It looks good to me. I do think that patch 01/10 shouldn't go through
> media. This might mean that we have to drop the COMPILE_TEST dependency
> on the media driver until this firmware driver patch gets merged, which
> is fine with me as long as this is clearly stated in the commit log for
> the media Kconfig. Let me know what you want to do with this.

OK, the best I can do is to drop COMPILE_TEST for Venus driver in this
patch set and work separately on qcom_scm firmware driver patching. Thus
I will repost v9 version next week.

> 
> I also saw some comments for patch 05/10, but I'm not sure if that would
> block merging this driver or can be fixed afterwards.

I will prefix the exported symbols from venus-core driver as pointed by
Sakari in next v9 version plus fixes for few signed-unsigned compare
warnings.

-- 
regards,
Stan

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web