Hi Detlev,
Am 20.06.24 um 16:19 schrieb Detlev Casanova:
This driver supports the second generation of the Rockchip Video
decoder, also known as vdpu34x.
It is currently only used on the RK3588(s) SoC.
There are 2 decoders on the RK3588 SoC that can work in pair to decode
8K video at 30 FPS but currently, only using one core at a time is
supported.
Scheduling requests between the two cores will be implemented later.
The core supports H264, HEVC, VP9 and AVS2 decoding but this driver
currently only supports H264.
The driver is based on rkvdec and they may share some code in the
future.
The decision to make a different driver is mainly because rkvdec2 has
more features and can work with multiple cores.
The registers are mapped in a struct in RAM using bitfields. It is IO
copied to the HW when all values are configured.
The decision to use such a struct instead of writing buffers one by one
is based on the following reasons:
- Rockchip cores are known to misbehave when registers are not written
in address order,
- Those cores also need the software to write all registers, even if
they are written their default values or are not related to the task
(this core will not start decoding some H264 frames if some VP9
registers are not written to 0)
- In the future, to support multiple cores, the scheduler could be
optimized by storing the precomputed registers values and copy them
to the HW as soos as a core becomes available.
This makes the code more readable and may bring performance improvements
in future features.
Signed-off-by: Detlev Casanova <detlev.casanova@xxxxxxxxxxxxx>
---
drivers/staging/media/Kconfig | 1 +
drivers/staging/media/Makefile | 1 +
drivers/staging/media/rkvdec2/Kconfig | 15 +
drivers/staging/media/rkvdec2/Makefile | 3 +
drivers/staging/media/rkvdec2/TODO | 9 +
drivers/staging/media/rkvdec2/rkvdec2-h264.c | 739 +++++++++++
drivers/staging/media/rkvdec2/rkvdec2-regs.h | 345 +++++
drivers/staging/media/rkvdec2/rkvdec2.c | 1253 ++++++++++++++++++
drivers/staging/media/rkvdec2/rkvdec2.h | 130 ++
9 files changed, 2496 insertions(+)
create mode 100644 drivers/staging/media/rkvdec2/Kconfig
create mode 100644 drivers/staging/media/rkvdec2/Makefile
create mode 100644 drivers/staging/media/rkvdec2/TODO
create mode 100644 drivers/staging/media/rkvdec2/rkvdec2-h264.c
create mode 100644 drivers/staging/media/rkvdec2/rkvdec2-regs.h
create mode 100644 drivers/staging/media/rkvdec2/rkvdec2.c
create mode 100644 drivers/staging/media/rkvdec2/rkvdec2.h
...
+static inline void rkvdec2_memcpy_toio(void __iomem *dst, void *src, size_t len)
+{
+#ifdef CONFIG_ARM64
+ __iowrite32_copy(dst, src, len);
+#elif defined(CONFIG_ARM)
I guess that can get an "#else" since memcpy_toio exists for all archs.
+ memcpy_toio(dst, src, len);
+#endif
+}
+
...
+ /* Set timeout threshold */
+ if (pixels < RKVDEC2_1080P_PIXELS)
+ regs->common.timeout_threshold = RKVDEC2_TIMEOUT_1080p;
+ else if (pixels < RKVDEC2_4K_PIXELS)
+ regs->common.timeout_threshold = RKVDEC2_TIMEOUT_4K;
+ else if (pixels < RKVDEC2_8K_PIXELS)
+ regs->common.timeout_threshold = RKVDEC2_TIMEOUT_8K;
+
Did you test if it works with anything > 8K? If so, you propably want to
make the check above
+ else
+ regs->common.timeout_threshold = RKVDEC2_TIMEOUT_8K;
Otherwise the timeout may not be set/contain invalid values from any former stream.
...
+
+static const struct rkvdec2_coded_fmt_desc rkvdec2_coded_fmts[] = {
+ {
+ .fourcc = V4L2_PIX_FMT_H264_SLICE,
+ .frmsize = {
+ .min_width = 16,
+ .max_width = 65520,
+ .step_width = 16,
+ .min_height = 16,
+ .max_height = 65520,
+ .step_height = 16,
+ },
+ .ctrls = &rkvdec2_h264_ctrls,
+ .ops = &rkvdec2_h264_fmt_ops,
+ .num_decoded_fmts = ARRAY_SIZE(rkvdec2_h264_decoded_fmts),
+ .decoded_fmts = rkvdec2_h264_decoded_fmts,
+ .subsystem_flags = VB2_V4L2_FL_SUPPORTS_M2M_HOLD_CAPTURE_BUF,
+ },
+};
+
Note, that this is also given to userspace (VIDIOC_ENUM_FRAMESIZES) and
this is already incorrect in the old rkvdec driver (and hantro): From
userspace perspective we do not have a restriction in
step_width/step_width, as we are aligning any given width/height to HW
requirements in the driver - what we should give to userspace is
fsize->type = V4L2_FRMSIZE_TYPE_CONTINUOUS; fsize->stepwise.min_height =
1; fsize->stepwise.min_width = 1; fsize->stepwise.max_height = 65520;
fsize->stepwise.max_width = 65520; I guess this new driver should be an
opportunity to fix that and distinguish between internal and external
frame size requirements and the .vidioc_enum_framesizes callback should
adapted accordingly. Regards, Alex
