* [PATCH v3 0/5] Raspberry Pi HEVC decoder driver
@ 2025-04-23 17:20 Dave Stevenson
2025-04-23 17:20 ` [PATCH v3 1/5] docs: uapi: media: Document Raspberry Pi NV12 column format Dave Stevenson
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Dave Stevenson @ 2025-04-23 17:20 UTC (permalink / raw)
To: Sakari Ailus, Laurent Pinchart, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
Broadcom internal kernel review list, John Cox, Dom Cobley,
review list, Ezequiel Garcia
Cc: John Cox, linux-media, linux-kernel, devicetree, linux-rpi-kernel,
linux-arm-kernel, Dave Stevenson
Hi All
This has been in the pipeline for a while, but I've finally cleaned
up our HEVC decoder driver to be in a shape to at least get a first
review.
John Cox has done almost all of the work under contract to Raspberry
Pi, and I'm largely just doing the process of patch curation and
sending.
There are a couple of questions raised in frameworks.
The main one is that the codec has 2 independent phases to the decode,
CABAC and reconstruction. To keep the decoder operating optimally
means that two requests need to be in process at once, whilst the
current frameworks don't want to allow as there is an implicit
assumption of only a single job being active at once, and
completition returns both buffers and releases the media request.
The OUTPUT queue buffer is finished with and can be returned at the
end of phase 1, but the media request is still required for phase 2.
The frameworks currently force the driver to be returning both
together via v4l2_m2m_buf_done_and_job_finish. v4l2_m2m_job_finish
would complete the job without returning the buffer as we need,
however if the driver has set VB2_V4L2_FL_SUPPORTS_M2M_HOLD_CAPTURE_BUF
then we have a WARN in v4l2_m2m_job_finish.
Dropping the WARN as this series is currently doing isn't going to be
the right answer, but it isn't obvious what the right answer is.
Discussion required.
We also have a need to hold on to the media request for phase 2. John
had discussed this with Ezequiel (and others) a couple of years back,
and hence suggested a patch that adds media_request_{pin,unpin} to
grab references on the media request. Discussion required on that
or a better way of handling it.
I will apologise in advance for sending this V1 just before I head off
on the Christmas break, but will respond to things as soon as possible.
Thanks
Dave
v4l2-compliance output:
$ ./v4l-utils/build/utils/v4l2-compliance/v4l2-compliance
v4l2-compliance 1.29.0-5324, 64 bits, 64-bit time_t
v4l2-compliance SHA: 3116b579c38d 2025-02-05 14:42:29
Compliance test for rpi-hevc-dec device /dev/video0:
Driver Info:
Driver name : rpi-hevc-dec
Card type : rpi-hevc-dec
Bus info : platform:rpi-hevc-dec
Driver version : 6.13.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
Detected Stateless Decoder
Media Driver Info:
Driver name : rpi-hevc-dec
Model : rpi-hevc-dec
Serial :
Bus info : platform:rpi-hevc-dec
Media version : 6.13.0
Hardware revision: 0x00000000 (0)
Driver version : 6.13.0
Interface Info:
ID : 0x0300000c
Type : V4L Video
Entity Info:
ID : 0x00000001 (1)
Name : rpi-hevc-dec-source
Function : V4L2 I/O
Pad 0x01000002 : 0: Source
Link 0x02000008: to remote pad 0x1000004 of entity
'rpi-hevc-dec-proc' (Video Decoder): Data, Enabled, Immutable
Required ioctls:
test MC information (see 'Media Driver Info' above): OK
test VIDIOC_QUERYCAP: OK
test invalid ioctls: OK
Allow for multiple opens:
test second /dev/video0 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
fail: v4l2-test-controls.cpp(939): try_ext_ctrls
returned an error (22)
test VIDIOC_G/S/TRY_EXT_CTRLS: FAIL
test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK
test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
Standard Controls: 3 Private Controls: 0
Standard Compound Controls: 5 Private Compound 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 (Not Supported)
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 CREATE_BUFS maximum buffers: OK
test VIDIOC_REMOVE_BUFS: OK
test VIDIOC_EXPBUF: OK
test Requests: OK
test blocking wait: OK
Total for rpi-hevc-dec device /dev/video0: 49, Succeeded: 48, Failed: 1,
Warnings: 0
I'm still working on getting Fluster running by adding support for SAND.
First stumbling block is finding all the points to add the new format -
pointers to an example would be very welcome. I have had it decoding with
Robert Mader's opaque DRM type and modifiers patches, but in a slightly
hacky manner.
Testing is mainly with a downstream patchset to FFmpeg. I'm told FFmpeg
currently has no stateless decode support, but we will be reviewing
works that have been in progress and our downstream patches to see
whether that can be pushed onwards.
Downstream tree is at
https://github.com/jc-kynesim/rpi-ffmpeg/tree/dev/5.1.6/sandm_1
Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
---
Changes in v3:
- Updated the dtbinding with SoC specific compatible strings (Rob).
- Reordered hevc_dec and v3d in bcm2711.dtsi to keep them in ascending
register order (Stefan).
- Reordered hevc_dec in bcm2711-rpi.dtsi to keep them in alphabetical
order (Stefan).
- Tested on top of Nicolas' revised version of Hans' patch set for
manual request completion.
https://lore.kernel.org/all/20250410-sebastianfricke-vcodec_manual_request_completion_with_state_machine-v2-0-5b99ec0450e6@collabora.com/
- Link to v2: https://lore.kernel.org/r/20250206-media-rpi-hevc-dec-v2-0-69353c8805b2@raspberrypi.com
Changes in v2:
- Rebased to use Hans' manual request completion scheme.
https://lore.kernel.org/linux-media/cover.1724928939.git.hverkuil-cisco@xs4all.nl/
- Require all slices for a frame to be submitted in one request.
- Added the missing header file.
- Used the full macro name for pix format docs (Sakari)
- Dropped unneeded |- from dtbinding (Rob)
- Made reg and reg-names match in order (Rob)
- Removed clock-names from dtbinding (Rob)
- Driver changed to not request the clock by name
- Dropped clock-names from DTS file
- Minor fixes for compliance failures
fail: v4l2-test-formats.cpp(958): fmt_cap.g_colorspace() != col
fail: v4l2-test-buffers.cpp(901): q.create_bufs(node, 1, &fmt) != EINVAL
- v4l2-compliance output added to cover letter (Nicholas)
I believe the "fail: v4l2-test-controls.cpp(939): try_ext_ctrls
returned an error (22)" is expected as it is validating the SPS.
Hantro and Cedrus certainly both appear to return errors in the same place
- Link to v1: https://lore.kernel.org/r/20241220-media-rpi-hevc-dec-v1-0-0ebcc04ed42e@raspberrypi.com
---
Dave Stevenson (4):
docs: uapi: media: Document Raspberry Pi NV12 column format
media: ioctl: Add pixel formats NV12MT_COL128 and NV12MT_10_COL128
media: dt-bindings: media: Add binding for the Raspberry Pi HEVC decoder
arm: dts: bcm2711-rpi: Add HEVC decoder node
John Cox (1):
media: platform: Add Raspberry Pi HEVC decoder driver
.../bindings/media/raspberrypi,hevc-dec.yaml | 70 +
.../userspace-api/media/v4l/pixfmt-yuv-planar.rst | 42 +
MAINTAINERS | 10 +
arch/arm/boot/dts/broadcom/bcm2711-rpi.dtsi | 4 +
arch/arm/boot/dts/broadcom/bcm2711.dtsi | 9 +
drivers/media/platform/raspberrypi/Kconfig | 1 +
drivers/media/platform/raspberrypi/Makefile | 1 +
.../media/platform/raspberrypi/hevc_dec/Kconfig | 17 +
.../media/platform/raspberrypi/hevc_dec/Makefile | 5 +
.../media/platform/raspberrypi/hevc_dec/hevc_d.c | 450 ++++
.../media/platform/raspberrypi/hevc_dec/hevc_d.h | 189 ++
.../platform/raspberrypi/hevc_dec/hevc_d_h265.c | 2542 ++++++++++++++++++++
.../platform/raspberrypi/hevc_dec/hevc_d_h265.h | 23 +
.../platform/raspberrypi/hevc_dec/hevc_d_hw.c | 376 +++
.../platform/raspberrypi/hevc_dec/hevc_d_hw.h | 303 +++
.../platform/raspberrypi/hevc_dec/hevc_d_video.c | 688 ++++++
.../platform/raspberrypi/hevc_dec/hevc_d_video.h | 38 +
drivers/media/v4l2-core/v4l2-ioctl.c | 2 +
include/uapi/linux/videodev2.h | 4 +
19 files changed, 4774 insertions(+)
---
base-commit: 3f44deaa552e79966f7d07f4cd4cc2f5216a6999
change-id: 20241212-media-rpi-hevc-dec-3b5be739f3bd
Best regards,
--
Dave Stevenson <dave.stevenson@raspberrypi.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3 1/5] docs: uapi: media: Document Raspberry Pi NV12 column format
2025-04-23 17:20 [PATCH v3 0/5] Raspberry Pi HEVC decoder driver Dave Stevenson
@ 2025-04-23 17:20 ` Dave Stevenson
2025-05-24 0:30 ` Nicolas Dufresne
2025-04-23 17:20 ` [PATCH v3 2/5] media: ioctl: Add pixel formats NV12MT_COL128 and NV12MT_10_COL128 Dave Stevenson
` (2 subsequent siblings)
3 siblings, 1 reply; 13+ messages in thread
From: Dave Stevenson @ 2025-04-23 17:20 UTC (permalink / raw)
To: Sakari Ailus, Laurent Pinchart, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
Broadcom internal kernel review list, John Cox, Dom Cobley,
review list, Ezequiel Garcia
Cc: John Cox, linux-media, linux-kernel, devicetree, linux-rpi-kernel,
linux-arm-kernel, Dave Stevenson
The Raspberry Pi HEVC decoder uses a tiled format based on
columns for 8 and 10 bit YUV images, so document them as
NV12MT_COL128 and NV12MT_10_COL128.
Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
---
.../userspace-api/media/v4l/pixfmt-yuv-planar.rst | 42 ++++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/Documentation/userspace-api/media/v4l/pixfmt-yuv-planar.rst b/Documentation/userspace-api/media/v4l/pixfmt-yuv-planar.rst
index b788f6933855..b5b590f234b0 100644
--- a/Documentation/userspace-api/media/v4l/pixfmt-yuv-planar.rst
+++ b/Documentation/userspace-api/media/v4l/pixfmt-yuv-planar.rst
@@ -827,6 +827,48 @@ Data in the 12 high bits, zeros in the 4 low bits, arranged in little endian ord
- Cb\ :sub:`11`
- Cr\ :sub:`11`
+V4L2_PIX_FMT_NV12MT_COL128 and V4L2_PIX_FMT_NV12MT_10_COL128
+------------------------------------------------------------
+
+``V4L2_PIX_FMT_NV12MT_COL128`` is a tiled version of
+``V4L2_PIX_FMT_NV12M`` where the two planes are split into 128 byte wide columns
+of Y or interleaved CbCr.
+
+V4L2_PIX_FMT_NV12MT_10_COL128 expands that as a 10 bit format where 3 10 bit
+values are packed into a 32bit word. A 128 byte wide column therefore holds 96
+samples (either Y or interleaved CrCb). That effectively makes it 6 values in a
+64 bit word for the CbCr plane, as the values always go in pairs.
+
+Bit-packed representation.
+
+.. tabularcolumns:: |p{1.2cm}||p{1.2cm}||p{1.2cm}||p{1.2cm}|p{3.2cm}|p{3.2cm}|
+
+.. flat-table::
+ :header-rows: 0
+ :stub-columns: 0
+ :widths: 8 8 8 8
+
+ * - Y'\ :sub:`00[7:0]`
+ - Y'\ :sub:`01[5:0] (bits 7--2)` Y'\ :sub:`00[9:8]`\ (bits 1--0)
+ - Y'\ :sub:`02[3:0] (bits 7--4)` Y'\ :sub:`01[9:6]`\ (bits 3--0)
+ - unused (bits 7--6)` Y'\ :sub:`02[9:4]`\ (bits 5--0)
+
+.. tabularcolumns:: |p{1.2cm}||p{1.2cm}||p{1.2cm}||p{1.2cm}|p{3.2cm}|p{3.2cm}|
+
+.. flat-table::
+ :header-rows: 0
+ :stub-columns: 0
+ :widths: 12 12 12 12 12 12 12 12
+
+ * - Cb\ :sub:`00[7:0]`
+ - Cr\ :sub:`00[5:0]`\ (bits 7--2) Cb\ :sub:`00[9:8]`\ (bits 1--0)
+ - Cb\ :sub:`01[3:0]`\ (bits 7--4) Cr\ :sub:`00[9:6]`\ (bits 3--0)
+ - unused (bits 7--6) Cb\ :sub:`02[9:4]`\ (bits 5--0)
+ - Cr\ :sub:`01[7:0]`
+ - Cb\ :sub:`02[5:0]`\ (bits 7--2) Cr\ :sub:`01[9:8]`\ (bits 1--0)
+ - Cr\ :sub:`02[3:0]`\ (bits 7--4) Cb\ :sub:`02[9:6]`\ (bits 3--0)
+ - unused (bits 7--6) Cr\ :sub:`02[9:4]`\ (bits 5--0)
+
Fully Planar YUV Formats
========================
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v3 2/5] media: ioctl: Add pixel formats NV12MT_COL128 and NV12MT_10_COL128
2025-04-23 17:20 [PATCH v3 0/5] Raspberry Pi HEVC decoder driver Dave Stevenson
2025-04-23 17:20 ` [PATCH v3 1/5] docs: uapi: media: Document Raspberry Pi NV12 column format Dave Stevenson
@ 2025-04-23 17:20 ` Dave Stevenson
2025-04-23 17:20 ` [PATCH v3 3/5] media: dt-bindings: media: Add binding for the Raspberry Pi HEVC decoder Dave Stevenson
2025-04-23 17:20 ` [PATCH v3 5/5] arm: dts: bcm2711-rpi: Add HEVC decoder node Dave Stevenson
3 siblings, 0 replies; 13+ messages in thread
From: Dave Stevenson @ 2025-04-23 17:20 UTC (permalink / raw)
To: Sakari Ailus, Laurent Pinchart, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
Broadcom internal kernel review list, John Cox, Dom Cobley,
review list, Ezequiel Garcia
Cc: John Cox, linux-media, linux-kernel, devicetree, linux-rpi-kernel,
linux-arm-kernel, Dave Stevenson
Add V4L2_PIXFMT_NV12MT_COL128 and V4L2_PIXFMT_NV12MT_10_COL128
to describe the Raspberry Pi HEVC decoder NV12 multiplanar formats.
Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
---
drivers/media/v4l2-core/v4l2-ioctl.c | 2 ++
include/uapi/linux/videodev2.h | 4 ++++
2 files changed, 6 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index a16fb44c7246..9405e1391617 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -1379,7 +1379,9 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
case V4L2_PIX_FMT_NV16M: descr = "Y/UV 4:2:2 (N-C)"; break;
case V4L2_PIX_FMT_NV61M: descr = "Y/VU 4:2:2 (N-C)"; break;
case V4L2_PIX_FMT_NV12MT: descr = "Y/UV 4:2:0 (64x32 MB, N-C)"; break;
+ case V4L2_PIX_FMT_NV12MT_COL128: descr = "Y/CbCr 4:2:0 (128b cols)"; break;
case V4L2_PIX_FMT_NV12MT_16X16: descr = "Y/UV 4:2:0 (16x16 MB, N-C)"; break;
+ case V4L2_PIX_FMT_NV12MT_10_COL128: descr = "10-bit Y/CbCr 4:2:0 (128b cols)"; break;
case V4L2_PIX_FMT_P012M: descr = "12-bit Y/UV 4:2:0 (N-C)"; break;
case V4L2_PIX_FMT_YUV420M: descr = "Planar YUV 4:2:0 (N-C)"; break;
case V4L2_PIX_FMT_YVU420M: descr = "Planar YVU 4:2:0 (N-C)"; break;
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index c8cb2796130f..fbccb471e3cb 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -687,6 +687,10 @@ struct v4l2_pix_format {
#define V4L2_PIX_FMT_NV12MT_16X16 v4l2_fourcc('V', 'M', '1', '2') /* 12 Y/CbCr 4:2:0 16x16 tiles */
#define V4L2_PIX_FMT_NV12M_8L128 v4l2_fourcc('N', 'A', '1', '2') /* Y/CbCr 4:2:0 8x128 tiles */
#define V4L2_PIX_FMT_NV12M_10BE_8L128 v4l2_fourcc_be('N', 'T', '1', '2') /* Y/CbCr 4:2:0 10-bit 8x128 tiles */
+#define V4L2_PIX_FMT_NV12MT_COL128 v4l2_fourcc('N', 'c', '1', '2') /* 12 Y/CbCr 4:2:0 128 pixel wide column */
+#define V4L2_PIX_FMT_NV12MT_10_COL128 v4l2_fourcc('N', 'c', '3', '0')
+ /* Y/CbCr 4:2:0 10bpc, 3x10 packed as 4 bytes in a 128 bytes / 96 pixel wide column */
+
/* Bayer formats - see http://www.siliconimaging.com/RGB%20Bayer.htm */
#define V4L2_PIX_FMT_SBGGR8 v4l2_fourcc('B', 'A', '8', '1') /* 8 BGBG.. GRGR.. */
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v3 3/5] media: dt-bindings: media: Add binding for the Raspberry Pi HEVC decoder
2025-04-23 17:20 [PATCH v3 0/5] Raspberry Pi HEVC decoder driver Dave Stevenson
2025-04-23 17:20 ` [PATCH v3 1/5] docs: uapi: media: Document Raspberry Pi NV12 column format Dave Stevenson
2025-04-23 17:20 ` [PATCH v3 2/5] media: ioctl: Add pixel formats NV12MT_COL128 and NV12MT_10_COL128 Dave Stevenson
@ 2025-04-23 17:20 ` Dave Stevenson
2025-04-25 7:53 ` Krzysztof Kozlowski
2025-04-25 7:54 ` Krzysztof Kozlowski
2025-04-23 17:20 ` [PATCH v3 5/5] arm: dts: bcm2711-rpi: Add HEVC decoder node Dave Stevenson
3 siblings, 2 replies; 13+ messages in thread
From: Dave Stevenson @ 2025-04-23 17:20 UTC (permalink / raw)
To: Sakari Ailus, Laurent Pinchart, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
Broadcom internal kernel review list, John Cox, Dom Cobley,
review list, Ezequiel Garcia
Cc: John Cox, linux-media, linux-kernel, devicetree, linux-rpi-kernel,
linux-arm-kernel, Dave Stevenson
Adds a binding for the HEVC decoder found on the BCM2711 / Raspberry Pi 4,
and BCM2712 / Raspberry Pi 5.
Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
---
.../bindings/media/raspberrypi,hevc-dec.yaml | 70 ++++++++++++++++++++++
1 file changed, 70 insertions(+)
diff --git a/Documentation/devicetree/bindings/media/raspberrypi,hevc-dec.yaml b/Documentation/devicetree/bindings/media/raspberrypi,hevc-dec.yaml
new file mode 100644
index 000000000000..b86534f2689f
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/raspberrypi,hevc-dec.yaml
@@ -0,0 +1,70 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/raspberrypi,hevc-dec.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Raspberry Pi HEVC Decoder
+
+maintainers:
+ - John Cox <john.cox@raspberrypi.com>
+ - Dom Cobley <dom@raspberrypi.com>
+ - Dave Stevenson <dave.stevenson@raspberrypi.com>
+ - Raspberry Pi internal review list <kernel-list@raspberrypi.com>
+
+description:
+ The Raspberry Pi HEVC decoder is a hardware video decode accelerator block
+ found in the BCM2711 and BCM2712 processors used on Raspberry Pi 4 and 5
+ boards respectively.
+
+properties:
+ compatible:
+ items:
+ - enum:
+ - brcm,bcm2711-hevc-dec
+ - brcm,bcm2712-hevc-dec
+ - const: raspberrypi,hevc-dec
+
+ reg:
+ items:
+ - description: The HEVC main register region
+ - description: The Interrupt control register region
+
+ reg-names:
+ items:
+ - const: hevc
+ - const: intc
+
+ interrupts:
+ maxItems: 1
+
+ clocks:
+ items:
+ - description: The HEVC block clock
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - interrupts
+ - clocks
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ video-codec@7eb10000 {
+ compatible = "brcm,bcm2711-hevc-dec", "raspberrypi,hevc-dec";
+ reg = <0x7eb00000 0x10000>, /* HEVC */
+ <0x7eb10000 0x1000>; /* INTC */
+ reg-names = "hevc",
+ "intc";
+
+ interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>;
+
+ clocks = <&clk 0>;
+ };
+
+...
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v3 5/5] arm: dts: bcm2711-rpi: Add HEVC decoder node
2025-04-23 17:20 [PATCH v3 0/5] Raspberry Pi HEVC decoder driver Dave Stevenson
` (2 preceding siblings ...)
2025-04-23 17:20 ` [PATCH v3 3/5] media: dt-bindings: media: Add binding for the Raspberry Pi HEVC decoder Dave Stevenson
@ 2025-04-23 17:20 ` Dave Stevenson
2025-04-25 7:56 ` Krzysztof Kozlowski
3 siblings, 1 reply; 13+ messages in thread
From: Dave Stevenson @ 2025-04-23 17:20 UTC (permalink / raw)
To: Sakari Ailus, Laurent Pinchart, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
Broadcom internal kernel review list, John Cox, Dom Cobley,
review list, Ezequiel Garcia
Cc: John Cox, linux-media, linux-kernel, devicetree, linux-rpi-kernel,
linux-arm-kernel, Dave Stevenson
Add the configuration information for the HEVC decoder.
Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
---
arch/arm/boot/dts/broadcom/bcm2711-rpi.dtsi | 4 ++++
arch/arm/boot/dts/broadcom/bcm2711.dtsi | 9 +++++++++
2 files changed, 13 insertions(+)
diff --git a/arch/arm/boot/dts/broadcom/bcm2711-rpi.dtsi b/arch/arm/boot/dts/broadcom/bcm2711-rpi.dtsi
index c78ed064d166..7984caa861e6 100644
--- a/arch/arm/boot/dts/broadcom/bcm2711-rpi.dtsi
+++ b/arch/arm/boot/dts/broadcom/bcm2711-rpi.dtsi
@@ -68,6 +68,10 @@ &hdmi1 {
wifi-2.4ghz-coexistence;
};
+&hevc_dec {
+ clocks = <&firmware_clocks 11>;
+};
+
&hvs {
clocks = <&firmware_clocks 4>;
};
diff --git a/arch/arm/boot/dts/broadcom/bcm2711.dtsi b/arch/arm/boot/dts/broadcom/bcm2711.dtsi
index c06d9f5e53c8..bdb80f3611ca 100644
--- a/arch/arm/boot/dts/broadcom/bcm2711.dtsi
+++ b/arch/arm/boot/dts/broadcom/bcm2711.dtsi
@@ -617,6 +617,15 @@ xhci: usb@7e9c0000 {
status = "disabled";
};
+ hevc_dec: codec@7eb10000 {
+ compatible = "brcm,bcm2711-hevc-dec", "raspberrypi,hevc-dec";
+ reg = <0x0 0x7eb00000 0x10000>, /* HEVC */
+ <0x0 0x7eb10000 0x1000>; /* INTC */
+ reg-names = "hevc",
+ "intc";
+ interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>;
+ };
+
v3d: gpu@7ec00000 {
compatible = "brcm,2711-v3d";
reg = <0x0 0x7ec00000 0x4000>,
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v3 3/5] media: dt-bindings: media: Add binding for the Raspberry Pi HEVC decoder
2025-04-23 17:20 ` [PATCH v3 3/5] media: dt-bindings: media: Add binding for the Raspberry Pi HEVC decoder Dave Stevenson
@ 2025-04-25 7:53 ` Krzysztof Kozlowski
2025-04-28 10:03 ` Dave Stevenson
2025-04-25 7:54 ` Krzysztof Kozlowski
1 sibling, 1 reply; 13+ messages in thread
From: Krzysztof Kozlowski @ 2025-04-25 7:53 UTC (permalink / raw)
To: Dave Stevenson
Cc: Sakari Ailus, Laurent Pinchart, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
Broadcom internal kernel review list, John Cox, Dom Cobley,
review list, Ezequiel Garcia, John Cox, linux-media, linux-kernel,
devicetree, linux-rpi-kernel, linux-arm-kernel
On Wed, Apr 23, 2025 at 06:20:20PM GMT, Dave Stevenson wrote:
> Adds a binding for the HEVC decoder found on th +maintainers:
> + - John Cox <john.cox@raspberrypi.com>
> + - Dom Cobley <dom@raspberrypi.com>
> + - Dave Stevenson <dave.stevenson@raspberrypi.com>
> + - Raspberry Pi internal review list <kernel-list@raspberrypi.com>
Drop, no mailing lists in bindings maintainers. These must be people.
> +
> +description:
> + The Raspberry Pi HEVC decoder is a hardware video decode accelerator block
> + found in the BCM2711 and BCM2712 processors used on Raspberry Pi 4 and 5
> + boards respectively.
> +
> +properties:
> + compatible:
> + items:
> + - enum:
> + - brcm,bcm2711-hevc-dec
> + - brcm,bcm2712-hevc-dec
> + - const: raspberrypi,hevc-dec
Not what Rob asked. You should use specific SoC compatible as fallback.
You referred to file "raspberrypi,pisbe.yaml" before, but there is no
such file in the next.
Before you reply that there is a binding using different rules: well,
there is always poor code. Above two comments are repeated, especially
this about specific compatible - all the time, so these are not new
rules. These are given in reviews since some years.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 3/5] media: dt-bindings: media: Add binding for the Raspberry Pi HEVC decoder
2025-04-23 17:20 ` [PATCH v3 3/5] media: dt-bindings: media: Add binding for the Raspberry Pi HEVC decoder Dave Stevenson
2025-04-25 7:53 ` Krzysztof Kozlowski
@ 2025-04-25 7:54 ` Krzysztof Kozlowski
1 sibling, 0 replies; 13+ messages in thread
From: Krzysztof Kozlowski @ 2025-04-25 7:54 UTC (permalink / raw)
To: Dave Stevenson
Cc: Sakari Ailus, Laurent Pinchart, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
Broadcom internal kernel review list, John Cox, Dom Cobley,
review list, Ezequiel Garcia, John Cox, linux-media, linux-kernel,
devicetree, linux-rpi-kernel, linux-arm-kernel
On Wed, Apr 23, 2025 at 06:20:20PM GMT, Dave Stevenson wrote:
> Adds a binding for the HEVC decoder found on the BCM2711 / Raspberry Pi 4,
> and BCM2712 / Raspberry Pi 5.
>
> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Also:
subject: only one media prefix. You really do not need two prefixes.
See DT submitting patches.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 5/5] arm: dts: bcm2711-rpi: Add HEVC decoder node
2025-04-23 17:20 ` [PATCH v3 5/5] arm: dts: bcm2711-rpi: Add HEVC decoder node Dave Stevenson
@ 2025-04-25 7:56 ` Krzysztof Kozlowski
0 siblings, 0 replies; 13+ messages in thread
From: Krzysztof Kozlowski @ 2025-04-25 7:56 UTC (permalink / raw)
To: Dave Stevenson, Sakari Ailus, Laurent Pinchart,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Florian Fainelli,
Broadcom internal kernel review list, John Cox, Dom Cobley,
review list, Ezequiel Garcia
Cc: John Cox, linux-media, linux-kernel, devicetree, linux-rpi-kernel,
linux-arm-kernel
On 23/04/2025 19:20, Dave Stevenson wrote:
> diff --git a/arch/arm/boot/dts/broadcom/bcm2711.dtsi b/arch/arm/boot/dts/broadcom/bcm2711.dtsi
> index c06d9f5e53c8..bdb80f3611ca 100644
> --- a/arch/arm/boot/dts/broadcom/bcm2711.dtsi
> +++ b/arch/arm/boot/dts/broadcom/bcm2711.dtsi
> @@ -617,6 +617,15 @@ xhci: usb@7e9c0000 {
> status = "disabled";
> };
>
> + hevc_dec: codec@7eb10000 {
It does not look like you tested the DTS against bindings. Please run
`make dtbs_check W=1` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).
Maybe you need to update your dtschema and yamllint. Don't rely on
distro packages for dtschema and be sure you are using the latest
released dtschema.
> + compatible = "brcm,bcm2711-hevc-dec", "raspberrypi,hevc-dec";
> + reg = <0x0 0x7eb00000 0x10000>, /* HEVC */
> + <0x0 0x7eb10000 0x1000>; /* INTC */
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 3/5] media: dt-bindings: media: Add binding for the Raspberry Pi HEVC decoder
2025-04-25 7:53 ` Krzysztof Kozlowski
@ 2025-04-28 10:03 ` Dave Stevenson
2025-04-28 17:56 ` Krzysztof Kozlowski
0 siblings, 1 reply; 13+ messages in thread
From: Dave Stevenson @ 2025-04-28 10:03 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Sakari Ailus, Laurent Pinchart, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
Broadcom internal kernel review list, John Cox, Dom Cobley,
review list, Ezequiel Garcia, John Cox, linux-media, linux-kernel,
devicetree, linux-rpi-kernel, linux-arm-kernel
Hi Krzysztof
On Fri, 25 Apr 2025 at 08:53, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Wed, Apr 23, 2025 at 06:20:20PM GMT, Dave Stevenson wrote:
> > Adds a binding for the HEVC decoder found on th +maintainers:
> > + - John Cox <john.cox@raspberrypi.com>
> > + - Dom Cobley <dom@raspberrypi.com>
> > + - Dave Stevenson <dave.stevenson@raspberrypi.com>
>
> > + - Raspberry Pi internal review list <kernel-list@raspberrypi.com>
>
> Drop, no mailing lists in bindings maintainers. These must be people.
Ack
> > +
> > +description:
> > + The Raspberry Pi HEVC decoder is a hardware video decode accelerator block
> > + found in the BCM2711 and BCM2712 processors used on Raspberry Pi 4 and 5
> > + boards respectively.
> > +
> > +properties:
> > + compatible:
> > + items:
> > + - enum:
> > + - brcm,bcm2711-hevc-dec
> > + - brcm,bcm2712-hevc-dec
> > + - const: raspberrypi,hevc-dec
>
> Not what Rob asked. You should use specific SoC compatible as fallback.
In which case I don't understand what Rob was asking for.
I asked for clarification in [1], but got no reply. Sending a new
version has at least got an answer, but I'm none the wiser.
Staring at this trying to work out your meaning, you want the generic
first, and SoC specific second? ie
+ compatible:
+ items:
+ - const: raspberrypi,hevc-dec
+ - enum:
+ - brcm,bcm2711-hevc-dec
+ - brcm,bcm2712-hevc-dec
> You referred to file "raspberrypi,pisbe.yaml" before, but there is no
> such file in the next.
Typo.
https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/media/raspberrypi%2Cpispbe.yaml
Reviewed by Rob only just over a year ago [2]
> Before you reply that there is a binding using different rules: well,
> there is always poor code. Above two comments are repeated, especially
> this about specific compatible - all the time, so these are not new
> rules. These are given in reviews since some years.
My Google-foo is totally failing with the only directly relevant
mention of "fallback compatible" I find is [3], which just says to use
them.
You're effectively saying I can't take anything in the kernel tree as
being a valid example as it could be poor code, and a layman such as
myself has no way of telling.
Could you please point me at documentation and examples I can rely on,
or educate me with what is wanted in this situation to avoid me having
to guess?
A further mailing list search has brought up [4] which is a thread
with yourself from 2 years ago which looks to be a very similar
situation. Other than missing the const on the SoC strings (although
that isn't in the merged version of cnm,wave521c.yaml), and two SoC
specific strings, I'm not seeing an obvious difference between there
and here either.
Many thanks
Dave
> Best regards,
> Krzysztof
[1] https://lore.kernel.org/linux-media/CAPY8ntD3Frq5HzV06OrS1051QfjJFzvqs9H4mUkVnd4QKqiMhg@mail.gmail.com/
[2] https://www.spinics.net/lists/linux-media/msg250095.html
[3] https://www.kernel.org/doc/html/latest/devicetree/bindings/writing-bindings.html#properties
[4] https://patchwork.kernel.org/project/linux-arm-kernel/patch/20230929-wave5_v13_media_master-v13-6-5ac60ccbf2ce@collabora.com/#25567148
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 3/5] media: dt-bindings: media: Add binding for the Raspberry Pi HEVC decoder
2025-04-28 10:03 ` Dave Stevenson
@ 2025-04-28 17:56 ` Krzysztof Kozlowski
2025-06-23 17:54 ` Dave Stevenson
0 siblings, 1 reply; 13+ messages in thread
From: Krzysztof Kozlowski @ 2025-04-28 17:56 UTC (permalink / raw)
To: Dave Stevenson
Cc: Sakari Ailus, Laurent Pinchart, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
Broadcom internal kernel review list, John Cox, Dom Cobley,
review list, Ezequiel Garcia, John Cox, linux-media, linux-kernel,
devicetree, linux-rpi-kernel, linux-arm-kernel
On 28/04/2025 12:03, Dave Stevenson wrote:
> Hi Krzysztof
>
> On Fri, 25 Apr 2025 at 08:53, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On Wed, Apr 23, 2025 at 06:20:20PM GMT, Dave Stevenson wrote:
>>> Adds a binding for the HEVC decoder found on th +maintainers:
>>> + - John Cox <john.cox@raspberrypi.com>
>>> + - Dom Cobley <dom@raspberrypi.com>
>>> + - Dave Stevenson <dave.stevenson@raspberrypi.com>
>>
>>> + - Raspberry Pi internal review list <kernel-list@raspberrypi.com>
>>
>> Drop, no mailing lists in bindings maintainers. These must be people.
>
> Ack
>
>>> +
>>> +description:
>>> + The Raspberry Pi HEVC decoder is a hardware video decode accelerator block
>>> + found in the BCM2711 and BCM2712 processors used on Raspberry Pi 4 and 5
>>> + boards respectively.
>>> +
>>> +properties:
>>> + compatible:
>>> + items:
>>> + - enum:
>>> + - brcm,bcm2711-hevc-dec
>>> + - brcm,bcm2712-hevc-dec
>>> + - const: raspberrypi,hevc-dec
>>
>> Not what Rob asked. You should use specific SoC compatible as fallback.
>
> In which case I don't understand what Rob was asking for.
> I asked for clarification in [1], but got no reply. Sending a new
> version has at least got an answer, but I'm none the wiser.
>
> Staring at this trying to work out your meaning, you want the generic
> first, and SoC specific second? ie
> + compatible:
> + items:
> + - const: raspberrypi,hevc-dec
Drop
> + - enum:
That's enum, not fallback.
> + - brcm,bcm2711-hevc-dec
> + - brcm,bcm2712-hevc-dec
>
>> You referred to file "raspberrypi,pisbe.yaml" before, but there is no
>> such file in the next.
>
> Typo.
> https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/media/raspberrypi%2Cpispbe.yaml
> Reviewed by Rob only just over a year ago [2]
There were some discussions and reasons with explanations.
Feel free to use same arguments and accurately describe the hardware, so
we won't be needing to ask usual questions.
Anyway, we asked for fallback, so you need items, just like every SoC
compatible (see also example schema).
>
>> Before you reply that there is a binding using different rules: well,
>> there is always poor code. Above two comments are repeated, especially
>> this about specific compatible - all the time, so these are not new
>> rules. These are given in reviews since some years.
>
> My Google-foo is totally failing with the only directly relevant
> mention of "fallback compatible" I find is [3], which just says to use
> them.
>
> You're effectively saying I can't take anything in the kernel tree as
> being a valid example as it could be poor code, and a layman such as
> myself has no way of telling.
No, I am saying that argument "I saw someone doing this, so I am allowed
to do the same" is not correct. There are good and bad examples. For
example in my talks I mentioned good examples. The list of good examples
was not accepted to the kernel so well... I just use as an example any
recent Qcom binding using specific compatibles.
> Could you please point me at documentation and examples I can rely on,
> or educate me with what is wanted in this situation to avoid me having
> to guess?
>
> A further mailing list search has brought up [4] which is a thread
> with yourself from 2 years ago which looks to be a very similar
> situation. Other than missing the const on the SoC strings (although
> that isn't in the merged version of cnm,wave521c.yaml), and two SoC
> specific strings, I'm not seeing an obvious difference between there
> and here either.
How is the [4] relevant? That's IP block from other vendor.
https://www.kernel.org/doc/html/latest/devicetree/bindings/writing-bindings.html#properties
https://elixir.bootlin.com/linux/v6.3-rc6/source/Documentation/devicetree/bindings/sound/nvidia,tegra210-ope.yaml#L31
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 1/5] docs: uapi: media: Document Raspberry Pi NV12 column format
2025-04-23 17:20 ` [PATCH v3 1/5] docs: uapi: media: Document Raspberry Pi NV12 column format Dave Stevenson
@ 2025-05-24 0:30 ` Nicolas Dufresne
0 siblings, 0 replies; 13+ messages in thread
From: Nicolas Dufresne @ 2025-05-24 0:30 UTC (permalink / raw)
To: Dave Stevenson, Sakari Ailus, Laurent Pinchart,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Florian Fainelli,
Broadcom internal kernel review list, John Cox, Dom Cobley,
review list, Ezequiel Garcia
Cc: John Cox, linux-media, linux-kernel, devicetree, linux-rpi-kernel,
linux-arm-kernel
Le mercredi 23 avril 2025 à 18:20 +0100, Dave Stevenson a écrit :
> The Raspberry Pi HEVC decoder uses a tiled format based on
> columns for 8 and 10 bit YUV images, so document them as
> NV12MT_COL128 and NV12MT_10_COL128.
>
> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> ---
> .../userspace-api/media/v4l/pixfmt-yuv-planar.rst | 42 ++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
> diff --git a/Documentation/userspace-api/media/v4l/pixfmt-yuv-planar.rst b/Documentation/userspace-
> api/media/v4l/pixfmt-yuv-planar.rst
> index b788f6933855..b5b590f234b0 100644
> --- a/Documentation/userspace-api/media/v4l/pixfmt-yuv-planar.rst
> +++ b/Documentation/userspace-api/media/v4l/pixfmt-yuv-planar.rst
> @@ -827,6 +827,48 @@ Data in the 12 high bits, zeros in the 4 low bits, arranged in little endian ord
> - Cb\ :sub:`11`
> - Cr\ :sub:`11`
>
> +V4L2_PIX_FMT_NV12MT_COL128 and V4L2_PIX_FMT_NV12MT_10_COL128
> +------------------------------------------------------------
> +
> +``V4L2_PIX_FMT_NV12MT_COL128`` is a tiled version of
> +``V4L2_PIX_FMT_NV12M`` where the two planes are split into 128 byte wide columns
> +of Y or interleaved CbCr.
> +
> +V4L2_PIX_FMT_NV12MT_10_COL128 expands that as a 10 bit format where 3 10 bit
> +values are packed into a 32bit word. A 128 byte wide column therefore holds 96
> +samples (either Y or interleaved CrCb). That effectively makes it 6 values in a
> +64 bit word for the CbCr plane, as the values always go in pairs.
Could be nice to mention that this has a verticalement alignment of 8, allowing
to represent it as a 128x8 tiled format with a column layout. This was handy for
the GStreamer integration your showed me.
Nicolas
> +
> +Bit-packed representation.
> +
> +.. tabularcolumns:: |p{1.2cm}||p{1.2cm}||p{1.2cm}||p{1.2cm}|p{3.2cm}|p{3.2cm}|
> +
> +.. flat-table::
> + :header-rows: 0
> + :stub-columns: 0
> + :widths: 8 8 8 8
> +
> + * - Y'\ :sub:`00[7:0]`
> + - Y'\ :sub:`01[5:0] (bits 7--2)` Y'\ :sub:`00[9:8]`\ (bits 1--0)
> + - Y'\ :sub:`02[3:0] (bits 7--4)` Y'\ :sub:`01[9:6]`\ (bits 3--0)
> + - unused (bits 7--6)` Y'\ :sub:`02[9:4]`\ (bits 5--0)
> +
> +.. tabularcolumns:: |p{1.2cm}||p{1.2cm}||p{1.2cm}||p{1.2cm}|p{3.2cm}|p{3.2cm}|
> +
> +.. flat-table::
> + :header-rows: 0
> + :stub-columns: 0
> + :widths: 12 12 12 12 12 12 12 12
> +
> + * - Cb\ :sub:`00[7:0]`
> + - Cr\ :sub:`00[5:0]`\ (bits 7--2) Cb\ :sub:`00[9:8]`\ (bits 1--0)
> + - Cb\ :sub:`01[3:0]`\ (bits 7--4) Cr\ :sub:`00[9:6]`\ (bits 3--0)
> + - unused (bits 7--6) Cb\ :sub:`02[9:4]`\ (bits 5--0)
> + - Cr\ :sub:`01[7:0]`
> + - Cb\ :sub:`02[5:0]`\ (bits 7--2) Cr\ :sub:`01[9:8]`\ (bits 1--0)
> + - Cr\ :sub:`02[3:0]`\ (bits 7--4) Cb\ :sub:`02[9:6]`\ (bits 3--0)
> + - unused (bits 7--6) Cr\ :sub:`02[9:4]`\ (bits 5--0)
> +
>
> Fully Planar YUV Formats
> ========================
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 3/5] media: dt-bindings: media: Add binding for the Raspberry Pi HEVC decoder
2025-04-28 17:56 ` Krzysztof Kozlowski
@ 2025-06-23 17:54 ` Dave Stevenson
2025-06-24 6:01 ` Krzysztof Kozlowski
0 siblings, 1 reply; 13+ messages in thread
From: Dave Stevenson @ 2025-06-23 17:54 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Sakari Ailus, Laurent Pinchart, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
Broadcom internal kernel review list, John Cox, Dom Cobley,
review list, Ezequiel Garcia, John Cox, linux-media, linux-kernel,
devicetree, linux-rpi-kernel, linux-arm-kernel
Thanks Krzysztof
Sorry for the delay in replying - other priorities.
On Mon, 28 Apr 2025 at 18:57, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 28/04/2025 12:03, Dave Stevenson wrote:
> > Hi Krzysztof
> >
> > On Fri, 25 Apr 2025 at 08:53, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >>
> >> On Wed, Apr 23, 2025 at 06:20:20PM GMT, Dave Stevenson wrote:
> >>> Adds a binding for the HEVC decoder found on th +maintainers:
> >>> + - John Cox <john.cox@raspberrypi.com>
> >>> + - Dom Cobley <dom@raspberrypi.com>
> >>> + - Dave Stevenson <dave.stevenson@raspberrypi.com>
> >>
> >>> + - Raspberry Pi internal review list <kernel-list@raspberrypi.com>
> >>
> >> Drop, no mailing lists in bindings maintainers. These must be people.
> >
> > Ack
> >
> >>> +
> >>> +description:
> >>> + The Raspberry Pi HEVC decoder is a hardware video decode accelerator block
> >>> + found in the BCM2711 and BCM2712 processors used on Raspberry Pi 4 and 5
> >>> + boards respectively.
> >>> +
> >>> +properties:
> >>> + compatible:
> >>> + items:
> >>> + - enum:
> >>> + - brcm,bcm2711-hevc-dec
> >>> + - brcm,bcm2712-hevc-dec
> >>> + - const: raspberrypi,hevc-dec
> >>
> >> Not what Rob asked. You should use specific SoC compatible as fallback.
> >
> > In which case I don't understand what Rob was asking for.
> > I asked for clarification in [1], but got no reply. Sending a new
> > version has at least got an answer, but I'm none the wiser.
> >
> > Staring at this trying to work out your meaning, you want the generic
> > first, and SoC specific second? ie
> > + compatible:
> > + items:
> > + - const: raspberrypi,hevc-dec
>
> Drop
>
> > + - enum:
>
> That's enum, not fallback.
>
> > + - brcm,bcm2711-hevc-dec
> > + - brcm,bcm2712-hevc-dec
> >
> >> You referred to file "raspberrypi,pisbe.yaml" before, but there is no
> >> such file in the next.
> >
> > Typo.
> > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/media/raspberrypi%2Cpispbe.yaml
> > Reviewed by Rob only just over a year ago [2]
>
> There were some discussions and reasons with explanations.
> Feel free to use same arguments and accurately describe the hardware, so
> we won't be needing to ask usual questions.
Sorry if the previous descriptions hadn't been clear. All the same
points as for raspberrypi,pispbe.yaml apply here, which is why I'd
tried to link to it originally.
Describe the hardware:
Raspberry Pi designed and are the sole owners of the IP for this HEVC
decoder block. This is *not* Broadcom IP.
That design was given to Broadcom as Verilog to wire into the
bus/interrupt/clock fabric of BCM2711, and to manufacture it via TSMC
on a 28nm process node.
A few years later the same design was given to Broadcom to wire into
BCM2712, and to manufacture it on a 16nm process node.
As it is Raspberry Pi owned IP it can be used in other places than
Broadcom SoCs.
Broadcom does not have a licence to use the IP in any other of their chips.
It is the same situation as for raspberrypi,pispbe.yaml except the
block already exists in 2 chips rather than just the 1. There also
isn't a version register in the hardware that is different between
those chips (an oversight noted for future chips).
It could be compared to a Synopsis or Cadence IP block dropped into an
SoC. The vendor prefix happens to be raspberrypi instead of snps or
cdns.
Is there any part that needs to be further clarified?
> Anyway, we asked for fallback, so you need items, just like every SoC
> compatible (see also example schema).
>
>
> >
> >> Before you reply that there is a binding using different rules: well,
> >> there is always poor code. Above two comments are repeated, especially
> >> this about specific compatible - all the time, so these are not new
> >> rules. These are given in reviews since some years.
> >
> > My Google-foo is totally failing with the only directly relevant
> > mention of "fallback compatible" I find is [3], which just says to use
> > them.
> >
> > You're effectively saying I can't take anything in the kernel tree as
> > being a valid example as it could be poor code, and a layman such as
> > myself has no way of telling.
>
> No, I am saying that argument "I saw someone doing this, so I am allowed
> to do the same" is not correct. There are good and bad examples. For
> example in my talks I mentioned good examples. The list of good examples
> was not accepted to the kernel so well... I just use as an example any
> recent Qcom binding using specific compatibles.
>
> > Could you please point me at documentation and examples I can rely on,
> > or educate me with what is wanted in this situation to avoid me having
> > to guess?
> >
> > A further mailing list search has brought up [4] which is a thread
> > with yourself from 2 years ago which looks to be a very similar
> > situation. Other than missing the const on the SoC strings (although
> > that isn't in the merged version of cnm,wave521c.yaml), and two SoC
> > specific strings, I'm not seeing an obvious difference between there
> > and here either.
>
> How is the [4] relevant? That's IP block from other vendor.
It was another example of an IP block belonging to vendor A being
added into an SoC from vendor B, same as the situation here. Raspberry
Pi != Broadcom.
Thanks
Dave
> https://www.kernel.org/doc/html/latest/devicetree/bindings/writing-bindings.html#properties
>
> https://elixir.bootlin.com/linux/v6.3-rc6/source/Documentation/devicetree/bindings/sound/nvidia,tegra210-ope.yaml#L31
>
> Best regards,
> Krzysztof
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 3/5] media: dt-bindings: media: Add binding for the Raspberry Pi HEVC decoder
2025-06-23 17:54 ` Dave Stevenson
@ 2025-06-24 6:01 ` Krzysztof Kozlowski
0 siblings, 0 replies; 13+ messages in thread
From: Krzysztof Kozlowski @ 2025-06-24 6:01 UTC (permalink / raw)
To: Dave Stevenson
Cc: Sakari Ailus, Laurent Pinchart, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
Broadcom internal kernel review list, John Cox, Dom Cobley,
review list, Ezequiel Garcia, John Cox, linux-media, linux-kernel,
devicetree, linux-rpi-kernel, linux-arm-kernel
On 23/06/2025 19:54, Dave Stevenson wrote:
>>
>>> + - brcm,bcm2711-hevc-dec
>>> + - brcm,bcm2712-hevc-dec
>>>
>>>> You referred to file "raspberrypi,pisbe.yaml" before, but there is no
>>>> such file in the next.
>>>
>>> Typo.
>>> https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/media/raspberrypi%2Cpispbe.yaml
>>> Reviewed by Rob only just over a year ago [2]
>>
>> There were some discussions and reasons with explanations.
>> Feel free to use same arguments and accurately describe the hardware, so
>> we won't be needing to ask usual questions.
>
> Sorry if the previous descriptions hadn't been clear. All the same
> points as for raspberrypi,pispbe.yaml apply here, which is why I'd
> tried to link to it originally.
>
> Describe the hardware:
> Raspberry Pi designed and are the sole owners of the IP for this HEVC
> decoder block. This is *not* Broadcom IP.
>
> That design was given to Broadcom as Verilog to wire into the
> bus/interrupt/clock fabric of BCM2711, and to manufacture it via TSMC
> on a 28nm process node.
>
> A few years later the same design was given to Broadcom to wire into
> BCM2712, and to manufacture it on a 16nm process node.
>
> As it is Raspberry Pi owned IP it can be used in other places than
> Broadcom SoCs.
> Broadcom does not have a licence to use the IP in any other of their chips.
>
> It is the same situation as for raspberrypi,pispbe.yaml except the
> block already exists in 2 chips rather than just the 1. There also
> isn't a version register in the hardware that is different between
> those chips (an oversight noted for future chips).
> It could be compared to a Synopsis or Cadence IP block dropped into an
> SoC. The vendor prefix happens to be raspberrypi instead of snps or
> cdns.
>
> Is there any part that needs to be further clarified?
Dunno, thread is way too old to be in my inbox. Anyway, it is part of
the SoC as you said, so SoC compatibles rules apply here. It's the same
for Synopsys.
>
>> Anyway, we asked for fallback, so you need items, just like every SoC
>> compatible (see also example schema).
Just implement the feedback.
>>
>>
>>>
>>>> Before you reply that there is a binding using different rules: well,
>>>> there is always poor code. Above two comments are repeated, especially
>>>> this about specific compatible - all the time, so these are not new
>>>> rules. These are given in reviews since some years.
>>>
>>> My Google-foo is totally failing with the only directly relevant
>>> mention of "fallback compatible" I find is [3], which just says to use
>>> them.
>>>
>>> You're effectively saying I can't take anything in the kernel tree as
>>> being a valid example as it could be poor code, and a layman such as
>>> myself has no way of telling.
>>
>> No, I am saying that argument "I saw someone doing this, so I am allowed
>> to do the same" is not correct. There are good and bad examples. For
>> example in my talks I mentioned good examples. The list of good examples
>> was not accepted to the kernel so well... I just use as an example any
>> recent Qcom binding using specific compatibles.
>>
>>> Could you please point me at documentation and examples I can rely on,
>>> or educate me with what is wanted in this situation to avoid me having
>>> to guess?
>>>
>>> A further mailing list search has brought up [4] which is a thread
>>> with yourself from 2 years ago which looks to be a very similar
>>> situation. Other than missing the const on the SoC strings (although
>>> that isn't in the merged version of cnm,wave521c.yaml), and two SoC
>>> specific strings, I'm not seeing an obvious difference between there
>>> and here either.
>>
>> How is the [4] relevant? That's IP block from other vendor.
>
> It was another example of an IP block belonging to vendor A being
> added into an SoC from vendor B, same as the situation here. Raspberry
> Pi != Broadcom.
People hide real explanation, real facts, so don't use one example for
other case. I was told *explicitly* that Raspberry ISP or whatetever it
was called, was NOT PART OF the SoC. Now you said:
"A few years later the same design was given to Broadcom to wire into
BCM2712, and to manufacture it on a 16nm process node."
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2025-06-24 6:04 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-23 17:20 [PATCH v3 0/5] Raspberry Pi HEVC decoder driver Dave Stevenson
2025-04-23 17:20 ` [PATCH v3 1/5] docs: uapi: media: Document Raspberry Pi NV12 column format Dave Stevenson
2025-05-24 0:30 ` Nicolas Dufresne
2025-04-23 17:20 ` [PATCH v3 2/5] media: ioctl: Add pixel formats NV12MT_COL128 and NV12MT_10_COL128 Dave Stevenson
2025-04-23 17:20 ` [PATCH v3 3/5] media: dt-bindings: media: Add binding for the Raspberry Pi HEVC decoder Dave Stevenson
2025-04-25 7:53 ` Krzysztof Kozlowski
2025-04-28 10:03 ` Dave Stevenson
2025-04-28 17:56 ` Krzysztof Kozlowski
2025-06-23 17:54 ` Dave Stevenson
2025-06-24 6:01 ` Krzysztof Kozlowski
2025-04-25 7:54 ` Krzysztof Kozlowski
2025-04-23 17:20 ` [PATCH v3 5/5] arm: dts: bcm2711-rpi: Add HEVC decoder node Dave Stevenson
2025-04-25 7:56 ` Krzysztof Kozlowski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).