Re: [PATCH v3 4/9] media: uapi: Add a control for HANTRO driver
From: Ezequiel Garcia
Date: Thu Feb 25 2021 - 09:06:39 EST
Hi Benjamin,
On Mon, 2021年02月22日 at 13:24 +0100, Benjamin Gaignard wrote:
>
The HEVC HANTRO driver needs to know the number of bits to skip at
s/HANTRO/Hantro
>
the beginning of the slice header.
As discussed in a different thread, we should describe exactly
what the hardware is expecting, so applications can parse that
and pass a correct value.
>
That is a hardware specific requirement so create a dedicated control
>
that this purpose.
>
>
Signed-off-by: Benjamin Gaignard <benjamin.gaignard@xxxxxxxxxxxxx>
>
---
>
version 3:
>
- Fix typo in field name
>
>
include/uapi/linux/hantro-v4l2-controls.h | 20 ++++++++++++++++++++
>
include/uapi/linux/v4l2-controls.h | 5 +++++
>
2 files changed, 25 insertions(+)
>
create mode 100644 include/uapi/linux/hantro-v4l2-controls.h
>
>
diff --git a/include/uapi/linux/hantro-v4l2-controls.h b/include/uapi/linux/hantro-v4l2-controls.h
>
new file mode 100644
>
index 000000000000..a8dfd6b1a2a9
>
--- /dev/null
>
+++ b/include/uapi/linux/hantro-v4l2-controls.h
>
@@ -0,0 +1,20 @@
>
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
>
+
>
+#ifndef __UAPI_HANTRO_V4L2_CONYTROLS_H__
>
+#define __UAPI_HANTRO_V4L2_CONYTROLS_H__
>
+
>
+#include <linux/v4l2-controls.h>
>
+#include <media/hevc-ctrls.h>
>
+
>
+#define V4L2_CID_HANTRO_HEVC_EXTRA_DECODE_PARAMS (V4L2_CID_USER_HANTRO_BASE + 0)
>
+
>
+/**
>
+ * struct hantro_hevc_extra_decode_params - extra decode parameters for hantro driver
>
+ * @hevc_hdr_skip_length: header first bits offset
>
+ */
>
+struct hantro_hevc_extra_decode_params {
>
+ __u32 hevc_hdr_skip_length;
>
+ __u8 padding[4];
>
+};
>
+
I think we can get away with a simpler solution. Since it's just one integer
we need, there's no need for a compound control. Something like this:
.codec = HANTRO_HEVC_DECODER,
.cfg = {
.id = V4L2_CID_HANTRO_HEVC_SLICE_HEADER_SKIP,
.name = "Hantro HEVC slice header skip bytes",
.type = V4L2_CTRL_TYPE_INTEGER,
.min = 0,
.max = 0x7fffffff,
.step = 1,
},
Also see V4L2_CID_CODA_MB_ERR_CNT which is defined in drivers/media/platform/coda/coda.h.
The control is sufficiently special that it could be kept in an internal driver header.
>
+#endif
>
diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
>
index 039c0d7add1b..ced7486c7f46 100644
>
--- a/include/uapi/linux/v4l2-controls.h
>
+++ b/include/uapi/linux/v4l2-controls.h
>
@@ -209,6 +209,11 @@ enum v4l2_colorfx {
>
* We reserve 128 controls for this driver.
>
*/
>
#define V4L2_CID_USER_CCS_BASE (V4L2_CID_USER_BASE + 0x10f0)
>
+/*
>
+ * The base for HANTRO driver controls.
>
+ * We reserve 32 controls for this driver.
>
+ */
>
+#define V4L2_CID_USER_HANTRO_BASE (V4L2_CID_USER_BASE + 0x1170)
>
>
/* MPEG-class control IDs */
>
/* The MPEG controls are applicable to all codec controls
Thanks,
Ezequiel