linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/9] Add support for Wave6 video codec driver
@ 2025-08-29  8:46 Nas Chung
  2025-08-29  8:46 ` [PATCH v3 1/9] media: v4l2-common: Add YUV24 format info Nas Chung
                   ` (6 more replies)
  0 siblings, 7 replies; 19+ messages in thread
From: Nas Chung @ 2025-08-29  8:46 UTC (permalink / raw)
  To: mchehab, hverkuil, robh, krzk+dt, conor+dt, shawnguo, s.hauer
  Cc: linux-media, devicetree, linux-kernel, linux-imx,
	linux-arm-kernel, jackson.lee, lafley.kim, Nas Chung

This patch series introduces support for the Chips&Media Wave6 video
codec IP, a completely different hardware architecture compared to Wave5.

The wave6 driver is a M2M stateful encoder/decoder driver.
It supports various video formats, including H.264 and H.265,
for both encoding and decoding.
While other versions of the Wave6 IP may support VP9 decoding and
AV1 decoding and encoding those formats are not implemented or validated
in this driver at this time.

On NXP i.MX SoCs, the Wave6 IP functionality is split between two regions:
VPU Control region, Manages shared resources such as firmware memory.
VPU Core region, Provides encoding and decoding capabilities.
The VPU core cannot operate independently without the VPU control region.

The firmware tested by this driver has been upstreamed in linux-firmware:
- Path: cnm/wave633c_imx9_codec_fw.bin

This driver has been tested with GStreamer on:
- NXP i.MX95 board
- pre-silicon FPGA environment

Test results for decoder fluster:
- JVT-AVC_V1, Ran 77/135 tests successfully              in 43.050 secs
- JVT-FR-EXT, Ran 25/69 tests successfully               in 20.654 secs
- JCT-VC-HEVC_V1, Ran 132/147 tests successfully         in 87.940 secs
- All failures are due to unsupported hardware features:
-- 10bit, Resolutions higher than 4K, FMO, MBAFF
-- Extended profile, Field encoding and High422 sreams.

Test results for v4l2-compliance:
v4l2-compliance 1.31.0-5386, 64 bits, 64-bit time_t
v4l2-compliance SHA: 48316b8a20aa 2025-08-12 12:44:56

Compliance test for wave6-dec device /dev/video0:
                fail: v4l2-test-controls.cpp(1204): !have_source_change || !have_eos
        test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: FAIL
Total for wave6-dec device /dev/video0: 48, Succeeded: 47, Failed: 1, Warnings: 0

Compliance test for wave6-enc device /dev/video1:
                fail: v4l2-test-controls.cpp(1193): node->codec_mask & STATEFUL_ENCODER
        test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: FAIL
Total for wave6-enc device /dev/video1: 48, Succeeded: 47, Failed: 1, Warnings: 0

Note: the failures are all related with the eos event.

Changelog:

v3:
- Removed ambiguous SUPPORT_FOLLOWER feature
- Used WARN_ON() for unexpected programming errors
- Split thermal device code into wave6-vpu-thermal.c/h
- Dropped wave6_cooling_disable module parameter
- Replaced mutex_lock() with guard()
- Added lockdep_assert_held() to clarify locking regions
- Removed exported function due to dual-license and used function pointer
- Added documentation and validation for state transitions
- Added documentation for device structures
- Added patch to enable VPU device in imx95 DTS
- Updated DT bindings and driver to align with parent(vpu) and child(vpu-core)
- Replaced magic numbers with mask and offset macros when accessing registers
- Placed goto statements after an empty line
- Printed HW info (e.g. product_code) via dev_dbg() for debugging
- Replaced wave6_vpu_dec_give_command() with dedicated functions

v2:
- Refined DT bindings to better represent the hardware
- Reworked driver to align with the parent(VPU) and child(CTRL, CORE)
- Fixed build issues reported by CI tools (Smatch, Sparse, TRACE)
- Improved commit messages with clearer descriptions
- Added kernel-doc for exported functions
- Removed redundant print statements and unused code
- Reordered patches to prevent build failures

Nas Chung (9):
  media: v4l2-common: Add YUV24 format info
  dt-bindings: media: nxp: Add Wave6 video codec device
  media: chips-media: wave6: Add Wave6 VPU interface
  media: chips-media: wave6: Add v4l2 m2m driver support
  media: chips-media: wave6: Add Wave6 core driver
  media: chips-media: wave6: Improve debugging capabilities
  media: chips-media: wave6: Add Wave6 thermal cooling device
  media: chips-media: wave6: Add Wave6 control driver
  arm64: dts: freescale: imx95: Add video codec node

 .../bindings/media/nxp,imx95-vpu.yaml         |  145 +
 MAINTAINERS                                   |    8 +
 .../boot/dts/freescale/imx95-15x15-evk.dts    |    5 +
 .../boot/dts/freescale/imx95-19x19-evk.dts    |   10 +
 .../dts/freescale/imx95-phycore-fpsc.dtsi     |   10 +
 .../boot/dts/freescale/imx95-tqma9596sa.dtsi  |    5 +
 arch/arm64/boot/dts/freescale/imx95.dtsi      |   43 +
 drivers/media/platform/chips-media/Kconfig    |    1 +
 drivers/media/platform/chips-media/Makefile   |    1 +
 .../media/platform/chips-media/wave6/Kconfig  |   17 +
 .../media/platform/chips-media/wave6/Makefile |   17 +
 .../platform/chips-media/wave6/wave6-hw.c     | 2929 +++++++++++++++++
 .../platform/chips-media/wave6/wave6-hw.h     |   73 +
 .../chips-media/wave6/wave6-regdefine.h       |  638 ++++
 .../platform/chips-media/wave6/wave6-trace.h  |  286 ++
 .../platform/chips-media/wave6/wave6-vdi.h    |   92 +
 .../chips-media/wave6/wave6-vpu-core.c        |  406 +++
 .../chips-media/wave6/wave6-vpu-core.h        |  133 +
 .../chips-media/wave6/wave6-vpu-dbg.c         |  225 ++
 .../chips-media/wave6/wave6-vpu-dbg.h         |   14 +
 .../chips-media/wave6/wave6-vpu-dec.c         | 1863 +++++++++++
 .../chips-media/wave6/wave6-vpu-enc.c         | 2690 +++++++++++++++
 .../chips-media/wave6/wave6-vpu-thermal.c     |  136 +
 .../chips-media/wave6/wave6-vpu-thermal.h     |   25 +
 .../chips-media/wave6/wave6-vpu-v4l2.c        |  507 +++
 .../platform/chips-media/wave6/wave6-vpu.c    |  654 ++++
 .../platform/chips-media/wave6/wave6-vpu.h    |  131 +
 .../platform/chips-media/wave6/wave6-vpuapi.c |  725 ++++
 .../platform/chips-media/wave6/wave6-vpuapi.h | 1010 ++++++
 .../chips-media/wave6/wave6-vpuconfig.h       |   71 +
 .../chips-media/wave6/wave6-vpuerror.h        |  262 ++
 drivers/media/v4l2-core/v4l2-common.c         |    1 +
 32 files changed, 13133 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/nxp,imx95-vpu.yaml
 create mode 100644 drivers/media/platform/chips-media/wave6/Kconfig
 create mode 100644 drivers/media/platform/chips-media/wave6/Makefile
 create mode 100644 drivers/media/platform/chips-media/wave6/wave6-hw.c
 create mode 100644 drivers/media/platform/chips-media/wave6/wave6-hw.h
 create mode 100644 drivers/media/platform/chips-media/wave6/wave6-regdefine.h
 create mode 100644 drivers/media/platform/chips-media/wave6/wave6-trace.h
 create mode 100644 drivers/media/platform/chips-media/wave6/wave6-vdi.h
 create mode 100644 drivers/media/platform/chips-media/wave6/wave6-vpu-core.c
 create mode 100644 drivers/media/platform/chips-media/wave6/wave6-vpu-core.h
 create mode 100644 drivers/media/platform/chips-media/wave6/wave6-vpu-dbg.c
 create mode 100644 drivers/media/platform/chips-media/wave6/wave6-vpu-dbg.h
 create mode 100644 drivers/media/platform/chips-media/wave6/wave6-vpu-dec.c
 create mode 100644 drivers/media/platform/chips-media/wave6/wave6-vpu-enc.c
 create mode 100644 drivers/media/platform/chips-media/wave6/wave6-vpu-thermal.c
 create mode 100644 drivers/media/platform/chips-media/wave6/wave6-vpu-thermal.h
 create mode 100644 drivers/media/platform/chips-media/wave6/wave6-vpu-v4l2.c
 create mode 100644 drivers/media/platform/chips-media/wave6/wave6-vpu.c
 create mode 100644 drivers/media/platform/chips-media/wave6/wave6-vpu.h
 create mode 100644 drivers/media/platform/chips-media/wave6/wave6-vpuapi.c
 create mode 100644 drivers/media/platform/chips-media/wave6/wave6-vpuapi.h
 create mode 100644 drivers/media/platform/chips-media/wave6/wave6-vpuconfig.h
 create mode 100644 drivers/media/platform/chips-media/wave6/wave6-vpuerror.h

-- 
2.31.1



^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v3 1/9] media: v4l2-common: Add YUV24 format info
  2025-08-29  8:46 [PATCH v3 0/9] Add support for Wave6 video codec driver Nas Chung
@ 2025-08-29  8:46 ` Nas Chung
  2025-08-29  8:46 ` [PATCH v3 2/9] dt-bindings: media: nxp: Add Wave6 video codec device Nas Chung
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 19+ messages in thread
From: Nas Chung @ 2025-08-29  8:46 UTC (permalink / raw)
  To: mchehab, hverkuil, robh, krzk+dt, conor+dt, shawnguo, s.hauer
  Cc: linux-media, devicetree, linux-kernel, linux-imx,
	linux-arm-kernel, jackson.lee, lafley.kim, Nas Chung,
	Nicolas Dufresne

The YUV24 format is missing an entry in the v4l2_format_info().
The YUV24 format is the packed YUV 4:4:4 formats with 8 bits
per component.

Fixes: 0376a51fbe5e ("media: v4l: Add packed YUV444 24bpp pixel format")
Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com>
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
---
 drivers/media/v4l2-core/v4l2-common.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c
index 0574f5d685f8..8733a8ed5b2a 100644
--- a/drivers/media/v4l2-core/v4l2-common.c
+++ b/drivers/media/v4l2-core/v4l2-common.c
@@ -281,6 +281,7 @@ const struct v4l2_format_info *v4l2_format_info(u32 format)
 		{ .format = V4L2_PIX_FMT_Y212,    .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 1 },
 		{ .format = V4L2_PIX_FMT_Y216,    .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 1, .bpp = { 4, 0, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 2, .vdiv = 1 },
 		{ .format = V4L2_PIX_FMT_YUV48_12, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 1, .bpp = { 6, 0, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 1, .vdiv = 1 },
+		{ .format = V4L2_PIX_FMT_YUV24,   .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 1, .comp_planes = 1, .bpp = { 3, 0, 0, 0 }, .bpp_div = { 1, 1, 1, 1 }, .hdiv = 1, .vdiv = 1 },
 		{ .format = V4L2_PIX_FMT_MT2110T, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 5, 10, 0, 0 }, .bpp_div = { 4, 4, 1, 1 }, .hdiv = 2, .vdiv = 2,
 		  .block_w = { 16, 8, 0, 0 }, .block_h = { 32, 16, 0, 0 }},
 		{ .format = V4L2_PIX_FMT_MT2110R, .pixel_enc = V4L2_PIXEL_ENC_YUV, .mem_planes = 2, .comp_planes = 2, .bpp = { 5, 10, 0, 0 }, .bpp_div = { 4, 4, 1, 1 }, .hdiv = 2, .vdiv = 2,
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v3 2/9] dt-bindings: media: nxp: Add Wave6 video codec device
  2025-08-29  8:46 [PATCH v3 0/9] Add support for Wave6 video codec driver Nas Chung
  2025-08-29  8:46 ` [PATCH v3 1/9] media: v4l2-common: Add YUV24 format info Nas Chung
@ 2025-08-29  8:46 ` Nas Chung
  2025-08-29 13:57   ` Krzysztof Kozlowski
  2025-08-29  8:46 ` [PATCH v3 5/9] media: chips-media: wave6: Add Wave6 core driver Nas Chung
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 19+ messages in thread
From: Nas Chung @ 2025-08-29  8:46 UTC (permalink / raw)
  To: mchehab, hverkuil, robh, krzk+dt, conor+dt, shawnguo, s.hauer
  Cc: linux-media, devicetree, linux-kernel, linux-imx,
	linux-arm-kernel, jackson.lee, lafley.kim, Nas Chung

Add documents for the Wave6 video codec on NXP i.MX SoCs.

Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com>
---
 .../bindings/media/nxp,imx95-vpu.yaml         | 145 ++++++++++++++++++
 MAINTAINERS                                   |   7 +
 2 files changed, 152 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/nxp,imx95-vpu.yaml

diff --git a/Documentation/devicetree/bindings/media/nxp,imx95-vpu.yaml b/Documentation/devicetree/bindings/media/nxp,imx95-vpu.yaml
new file mode 100644
index 000000000000..34917997f8d0
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/nxp,imx95-vpu.yaml
@@ -0,0 +1,145 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/nxp,imx95-vpu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Chips&Media Wave6 Series multi-standard codec IP on NXP i.MX SoCs
+
+maintainers:
+  - Nas Chung <nas.chung@chipsnmedia.com>
+  - Jackson Lee <jackson.lee@chipsnmedia.com>
+
+description:
+  The Chips&Media Wave6 codec IP is a multi-standard video encoder/decoder.
+  On NXP i.MX SoCs, Wave6 codec IP functionality is split between
+  the VPU control region and the VPU core region.
+  The VPU control region manages shared resources such as firmware memory,
+  while the VPU core region provides encoding and decoding
+  capabilities. The VPU core cannot operate independently without
+  the VPU control region.
+
+properties:
+  compatible:
+    enum:
+      - nxp,imx95-vpu
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  power-domains:
+    maxItems: 1
+
+  memory-region:
+    maxItems: 1
+
+  sram:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: phandle of the SRAM memory region node.
+
+  "#cooling-cells":
+    const: 2
+
+  "#address-cells":
+    const: 2
+
+  "#size-cells":
+    const: 2
+
+  ranges: true
+
+patternProperties:
+  "^video-core@[0-9a-f]+$":
+    type: object
+    additionalProperties: false
+
+    properties:
+      compatible:
+        enum:
+          - nxp,imx95-vpu-core
+
+      reg:
+        maxItems: 1
+
+      clocks:
+        maxItems: 1
+
+      power-domains:
+        maxItems: 1
+
+      interrupts:
+        maxItems: 1
+
+    required:
+      - compatible
+      - reg
+      - clocks
+      - power-domains
+      - interrupts
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - power-domains
+  - memory-region
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/nxp,imx95-clock.h>
+
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+
+      vpu: video-codec@4c4c0000 {
+        compatible = "nxp,imx95-vpu";
+        reg = <0x0 0x4c4c0000 0x0 0x10000>;
+        clocks = <&vpu_blk_ctrl IMX95_CLK_VPUBLK_WAVE>;
+        power-domains = <&scmi_perf 10>;
+        memory-region = <&vpu_boot>;
+        sram = <&sram1>;
+        #cooling-cells = <2>;
+        #address-cells = <2>;
+        #size-cells = <2>;
+        ranges;
+
+        vpucore0: video-core@4c480000 {
+          compatible = "nxp,imx95-vpu-core";
+          reg = <0x0 0x4c480000 0x0 0x10000>;
+          clocks = <&scmi_clk 115>;
+          power-domains = <&scmi_devpd 21>;
+          interrupts = <GIC_SPI 299 IRQ_TYPE_LEVEL_HIGH>;
+        };
+
+        vpucore1: video-core@4c490000 {
+          compatible = "nxp,imx95-vpu-core";
+          reg = <0x0 0x4c490000 0x0 0x10000>;
+          clocks = <&scmi_clk 115>;
+          power-domains = <&scmi_devpd 21>;
+          interrupts = <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>;
+        };
+
+        vpucore2: video-core@4c4a0000 {
+          compatible = "nxp,imx95-vpu-core";
+          reg = <0x0 0x4c4a0000 0x0 0x10000>;
+          clocks = <&scmi_clk 115>;
+          power-domains = <&scmi_devpd 21>;
+          interrupts = <GIC_SPI 301 IRQ_TYPE_LEVEL_HIGH>;
+        };
+
+        vpucore3: video-core@4c4b0000 {
+          compatible = "nxp,imx95-vpu-core";
+          reg = <0x0 0x4c4b0000 0x0 0x10000>;
+          clocks = <&scmi_clk 115>;
+          power-domains = <&scmi_devpd 21>;
+          interrupts = <GIC_SPI 302 IRQ_TYPE_LEVEL_HIGH>;
+        };
+      };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 41e9014db574..c0e1eb867758 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -27070,6 +27070,13 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/media/cnm,wave521c.yaml
 F:	drivers/media/platform/chips-media/wave5/
 
+WAVE6 VPU CODEC DRIVER
+M:	Nas Chung <nas.chung@chipsnmedia.com>
+M:	Jackson Lee <jackson.lee@chipsnmedia.com>
+L:	linux-media@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/media/nxp,imx95-vpu.yaml
+
 WHISKEYCOVE PMIC GPIO DRIVER
 M:	Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
 L:	linux-gpio@vger.kernel.org
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v3 5/9] media: chips-media: wave6: Add Wave6 core driver
  2025-08-29  8:46 [PATCH v3 0/9] Add support for Wave6 video codec driver Nas Chung
  2025-08-29  8:46 ` [PATCH v3 1/9] media: v4l2-common: Add YUV24 format info Nas Chung
  2025-08-29  8:46 ` [PATCH v3 2/9] dt-bindings: media: nxp: Add Wave6 video codec device Nas Chung
@ 2025-08-29  8:46 ` Nas Chung
  2025-08-29 14:02   ` Krzysztof Kozlowski
  2025-08-29  8:46 ` [PATCH v3 6/9] media: chips-media: wave6: Improve debugging capabilities Nas Chung
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 19+ messages in thread
From: Nas Chung @ 2025-08-29  8:46 UTC (permalink / raw)
  To: mchehab, hverkuil, robh, krzk+dt, conor+dt, shawnguo, s.hauer
  Cc: linux-media, devicetree, linux-kernel, linux-imx,
	linux-arm-kernel, jackson.lee, lafley.kim, Nas Chung, Ming Qian

This adds the core driver for the Chips&Media Wave6 video codec IP.

The core region provides the encoding and decoding capabilities of
the VPU and depends on the control region for firmware and shared
resource management.

This driver configure the v4l2 m2m video device and handles
communication with the Wave6 hardware to perform video processing tasks.

Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com>
Tested-by: Ming Qian <ming.qian@oss.nxp.com>
---
 .../chips-media/wave6/wave6-vpu-core.c        | 406 ++++++++++++++++++
 .../chips-media/wave6/wave6-vpu-core.h        | 133 ++++++
 2 files changed, 539 insertions(+)
 create mode 100644 drivers/media/platform/chips-media/wave6/wave6-vpu-core.c
 create mode 100644 drivers/media/platform/chips-media/wave6/wave6-vpu-core.h

diff --git a/drivers/media/platform/chips-media/wave6/wave6-vpu-core.c b/drivers/media/platform/chips-media/wave6/wave6-vpu-core.c
new file mode 100644
index 000000000000..35c749ab1a14
--- /dev/null
+++ b/drivers/media/platform/chips-media/wave6/wave6-vpu-core.c
@@ -0,0 +1,406 @@
+// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
+/*
+ * Wave6 series multi-standard codec IP - wave6 core driver
+ *
+ * Copyright (C) 2025 CHIPS&MEDIA INC
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+#include <linux/firmware.h>
+#include <linux/interrupt.h>
+#include <linux/pm_runtime.h>
+#include <linux/debugfs.h>
+#include <linux/iopoll.h>
+#include "wave6-vpu-core.h"
+#include "wave6-regdefine.h"
+#include "wave6-vpuconfig.h"
+#include "wave6-hw.h"
+#include "wave6-vpu-dbg.h"
+
+#define CREATE_TRACE_POINTS
+#include "wave6-trace.h"
+
+#define WAVE6_VPU_CORE_PLATFORM_DRIVER_NAME "wave6-vpu-core"
+#define WAVE6_VPU_DEBUGFS_DIR "wave6"
+
+#define WAVE6_IS_ENC BIT(0)
+#define WAVE6_IS_DEC BIT(1)
+
+static const struct wave6_vpu_core_resource wave633c_core_data = {
+	.codec_types = WAVE6_IS_ENC | WAVE6_IS_DEC,
+	.compatible_fw_version = 0x2010000,
+};
+
+static irqreturn_t wave6_vpu_core_irq(int irq, void *dev_id)
+{
+	struct vpu_core_device *core = dev_id;
+	u32 irq_status;
+
+	if (vpu_read_reg(core, W6_VPU_VPU_INT_STS)) {
+		irq_status = vpu_read_reg(core, W6_VPU_VINT_REASON);
+
+		vpu_write_reg(core, W6_VPU_VINT_REASON_CLR, irq_status);
+		vpu_write_reg(core, W6_VPU_VINT_CLEAR, 0x1);
+
+		trace_irq(core, irq_status);
+
+		kfifo_in(&core->irq_status, &irq_status, sizeof(int));
+
+		return IRQ_WAKE_THREAD;
+	}
+
+	return IRQ_HANDLED;
+}
+
+static void wave6_vpu_core_response_work_buffer(struct vpu_core_device *core)
+{
+	int ret;
+
+	ret = pm_runtime_resume_and_get(core->dev);
+	if (ret)
+		return;
+
+	if (core->vpu)
+		core->vpu->req_work_buffer(core->vpu, core);
+
+	pm_runtime_put_sync(core->dev);
+}
+
+static irqreturn_t wave6_vpu_core_irq_thread(int irq, void *dev_id)
+{
+	struct vpu_core_device *core = dev_id;
+	struct vpu_instance *inst;
+	int irq_status, ret;
+
+	while (kfifo_len(&core->irq_status)) {
+		bool error = false;
+
+		ret = kfifo_out(&core->irq_status, &irq_status, sizeof(int));
+		if (!ret)
+			break;
+
+		if (irq_status & BIT(W6_INT_BIT_REQ_WORK_BUF)) {
+			wave6_vpu_core_response_work_buffer(core);
+			continue;
+		}
+
+		if ((irq_status & BIT(W6_INT_BIT_INIT_SEQ)) ||
+		    (irq_status & BIT(W6_INT_BIT_ENC_SET_PARAM))) {
+			complete(&core->irq_done);
+			continue;
+		}
+
+		if (irq_status & BIT(W6_INT_BIT_BSBUF_ERROR))
+			error = true;
+
+		inst = v4l2_m2m_get_curr_priv(core->m2m_dev);
+		if (inst && inst->ops && inst->ops->finish_process)
+			inst->ops->finish_process(inst, error);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static void wave6_vpu_core_check_state(struct vpu_core_device *core)
+{
+	u32 val;
+	int ret;
+
+	guard(mutex)(&core->hw_lock);
+
+	ret = read_poll_timeout(vpu_read_reg, val, val != 0,
+				W6_VPU_POLL_DELAY_US, W6_VPU_POLL_TIMEOUT,
+				false, core, W6_VPU_VCPU_CUR_PC);
+	if (ret)
+		return;
+
+	wave6_vpu_enable_interrupt(core);
+	ret = wave6_vpu_get_version(core);
+	if (ret) {
+		dev_err(core->dev, "wave6_vpu_get_version fail\n");
+		return;
+	}
+
+	dev_dbg(core->dev, "product 0x%x, fw_ver %d.%d.%d(r%d), hw_ver 0x%x\n",
+		core->attr.product_code,
+		FW_VERSION_MAJOR(core->attr.fw_version),
+		FW_VERSION_MINOR(core->attr.fw_version),
+		FW_VERSION_REL(core->attr.fw_version),
+		core->attr.fw_revision,
+		core->attr.hw_version);
+
+	if (core->attr.fw_version < core->res->compatible_fw_version)
+		dev_err(core->dev, "fw version is too low (< v%d.%d.%d)\n",
+			FW_VERSION_MAJOR(core->res->compatible_fw_version),
+			FW_VERSION_MINOR(core->res->compatible_fw_version),
+			FW_VERSION_REL(core->res->compatible_fw_version));
+}
+
+void wave6_vpu_core_activate(struct vpu_core_device *core)
+{
+	core->active = true;
+}
+
+static void wave6_vpu_core_wait_activated(struct vpu_core_device *core)
+{
+	if (core->active)
+		wave6_vpu_core_check_state(core);
+}
+
+static int wave6_vpu_core_probe(struct platform_device *pdev)
+{
+	struct vpu_core_device *core;
+	const struct wave6_vpu_core_resource *res;
+	int ret;
+	int irq;
+
+	res = device_get_match_data(&pdev->dev);
+	if (!res) {
+		dev_err(&pdev->dev, "missing resource\n");
+		return -EINVAL;
+	}
+
+	ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
+	if (ret < 0) {
+		dev_err(&pdev->dev, "dma_set_mask_and_coherent failed: %d\n", ret);
+		return ret;
+	}
+
+	core = devm_kzalloc(&pdev->dev, sizeof(*core), GFP_KERNEL);
+	if (!core)
+		return -ENOMEM;
+
+	ret = devm_mutex_init(&pdev->dev, &core->dev_lock);
+	if (ret)
+		return ret;
+
+	ret = devm_mutex_init(&pdev->dev, &core->hw_lock);
+	if (ret)
+		return ret;
+
+	init_completion(&core->irq_done);
+	dev_set_drvdata(&pdev->dev, core);
+	core->dev = &pdev->dev;
+	core->res = res;
+
+	if (pdev->dev.parent->driver && pdev->dev.parent->driver->name &&
+	    !strcmp(pdev->dev.parent->driver->name, WAVE6_VPU_PLATFORM_DRIVER_NAME))
+		core->vpu = dev_get_drvdata(pdev->dev.parent);
+
+	core->reg_base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(core->reg_base))
+		return PTR_ERR(core->reg_base);
+
+	ret = devm_clk_bulk_get_all(&pdev->dev, &core->clks);
+	if (ret < 0) {
+		dev_warn(&pdev->dev, "unable to get clocks: %d\n", ret);
+		ret = 0;
+	}
+	core->num_clks = ret;
+
+	ret = v4l2_device_register(&pdev->dev, &core->v4l2_dev);
+	if (ret) {
+		dev_err(&pdev->dev, "v4l2_device_register fail: %d\n", ret);
+		return ret;
+	}
+
+	ret = wave6_vpu_init_m2m_dev(core);
+	if (ret)
+		goto err_v4l2_unregister;
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(&pdev->dev, "failed to get irq resource\n");
+		ret = -ENXIO;
+		goto err_m2m_dev_release;
+	}
+
+	ret = kfifo_alloc(&core->irq_status, 16 * sizeof(int), GFP_KERNEL);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to allocate fifo\n");
+		goto err_m2m_dev_release;
+	}
+
+	ret = devm_request_threaded_irq(&pdev->dev, irq,
+					wave6_vpu_core_irq,
+					wave6_vpu_core_irq_thread,
+					0, "vpu_irq", core);
+	if (ret) {
+		dev_err(&pdev->dev, "fail to register interrupt handler: %d\n", ret);
+		goto err_kfifo_free;
+	}
+
+	core->temp_vbuf.size = ALIGN(W6_TEMPBUF_SIZE, 4096);
+	ret = wave6_vdi_alloc_dma(core->dev, &core->temp_vbuf);
+	if (ret) {
+		dev_err(&pdev->dev, "alloc temp of size %zu failed\n",
+			core->temp_vbuf.size);
+		goto err_kfifo_free;
+	}
+
+	core->debugfs = debugfs_lookup(WAVE6_VPU_DEBUGFS_DIR, NULL);
+	if (IS_ERR_OR_NULL(core->debugfs))
+		core->debugfs = debugfs_create_dir(WAVE6_VPU_DEBUGFS_DIR, NULL);
+
+	pm_runtime_enable(&pdev->dev);
+
+	if (core->res->codec_types & WAVE6_IS_DEC) {
+		ret = wave6_vpu_dec_register_device(core);
+		if (ret) {
+			dev_err(&pdev->dev, "wave6_vpu_dec_register_device fail: %d\n", ret);
+			goto err_temp_vbuf_free;
+		}
+	}
+	if (core->res->codec_types & WAVE6_IS_ENC) {
+		ret = wave6_vpu_enc_register_device(core);
+		if (ret) {
+			dev_err(&pdev->dev, "wave6_vpu_enc_register_device fail: %d\n", ret);
+			goto err_dec_unreg;
+		}
+	}
+
+	dev_dbg(&pdev->dev, "Added wave6 driver with caps %s %s\n",
+		core->res->codec_types & WAVE6_IS_ENC ? "'ENCODE'" : "",
+		core->res->codec_types & WAVE6_IS_DEC ? "'DECODE'" : "");
+
+	return 0;
+
+err_dec_unreg:
+	if (core->res->codec_types & WAVE6_IS_DEC)
+		wave6_vpu_dec_unregister_device(core);
+err_temp_vbuf_free:
+	wave6_vdi_free_dma(&core->temp_vbuf);
+err_kfifo_free:
+	kfifo_free(&core->irq_status);
+err_m2m_dev_release:
+	wave6_vpu_release_m2m_dev(core);
+err_v4l2_unregister:
+	v4l2_device_unregister(&core->v4l2_dev);
+
+	return ret;
+}
+
+static void wave6_vpu_core_remove(struct platform_device *pdev)
+{
+	struct vpu_core_device *core = dev_get_drvdata(&pdev->dev);
+
+	pm_runtime_disable(&pdev->dev);
+
+	wave6_vpu_enc_unregister_device(core);
+	wave6_vpu_dec_unregister_device(core);
+	wave6_vdi_free_dma(&core->temp_vbuf);
+	kfifo_free(&core->irq_status);
+	wave6_vpu_release_m2m_dev(core);
+	v4l2_device_unregister(&core->v4l2_dev);
+}
+
+static int __maybe_unused wave6_vpu_core_runtime_suspend(struct device *dev)
+{
+	struct vpu_core_device *core = dev_get_drvdata(dev);
+
+	if (WARN_ON(!core))
+		return -ENODEV;
+
+	/*
+	 * Only call parent VPU put_vpu if the core has a parent and is active.
+	 * - core->vpu: prevent access in core without parent VPU.
+	 * - core->active: execute sleep only after m2m streaming is started.
+	 */
+	if (core->vpu && core->active)
+		core->vpu->put_vpu(core->vpu, core);
+
+	if (core->num_clks)
+		clk_bulk_disable_unprepare(core->num_clks, core->clks);
+
+	return 0;
+}
+
+static int __maybe_unused wave6_vpu_core_runtime_resume(struct device *dev)
+{
+	struct vpu_core_device *core = dev_get_drvdata(dev);
+	int ret = 0;
+
+	if (WARN_ON(!core))
+		return -ENODEV;
+
+	if (core->num_clks) {
+		ret = clk_bulk_prepare_enable(core->num_clks, core->clks);
+		if (ret) {
+			dev_err(dev, "failed to enable clocks: %d\n", ret);
+			return ret;
+		}
+	}
+
+	/*
+	 * Only call parent VPU get_vpu if the core has a parent and is active.
+	 * - core->vpu: prevent access in core without parent VPU.
+	 * - core->active: execute boot only after m2m streaming is started.
+	 */
+	if (core->vpu && core->active)
+		ret = core->vpu->get_vpu(core->vpu, core);
+
+	if (!ret)
+		wave6_vpu_core_wait_activated(core);
+	else if (core->num_clks)
+		clk_bulk_disable_unprepare(core->num_clks, core->clks);
+
+	return ret;
+}
+
+static int __maybe_unused wave6_vpu_core_suspend(struct device *dev)
+{
+	struct vpu_core_device *core = dev_get_drvdata(dev);
+	int ret;
+
+	v4l2_m2m_suspend(core->m2m_dev);
+
+	ret = pm_runtime_force_suspend(dev);
+	if (ret)
+		v4l2_m2m_resume(core->m2m_dev);
+
+	return ret;
+}
+
+static int __maybe_unused wave6_vpu_core_resume(struct device *dev)
+{
+	struct vpu_core_device *core = dev_get_drvdata(dev);
+	int ret;
+
+	ret = pm_runtime_force_resume(dev);
+	if (ret)
+		return ret;
+
+	v4l2_m2m_resume(core->m2m_dev);
+
+	return 0;
+}
+
+static const struct dev_pm_ops wave6_vpu_core_pm_ops = {
+	SET_RUNTIME_PM_OPS(wave6_vpu_core_runtime_suspend,
+			   wave6_vpu_core_runtime_resume, NULL)
+	SET_SYSTEM_SLEEP_PM_OPS(wave6_vpu_core_suspend,
+				wave6_vpu_core_resume)
+};
+
+static const struct of_device_id wave6_vpu_core_ids[] = {
+	{ .compatible = "nxp,imx95-vpu-core", .data = &wave633c_core_data },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, wave6_vpu_core_ids);
+
+static struct platform_driver wave6_vpu_core_driver = {
+	.driver = {
+		.name = WAVE6_VPU_CORE_PLATFORM_DRIVER_NAME,
+		.of_match_table = wave6_vpu_core_ids,
+		.pm = &wave6_vpu_core_pm_ops,
+	},
+	.probe = wave6_vpu_core_probe,
+	.remove = wave6_vpu_core_remove,
+};
+
+module_platform_driver(wave6_vpu_core_driver);
+MODULE_DESCRIPTION("chips&media Wave6 VPU CORE V4L2 driver");
+MODULE_LICENSE("Dual BSD/GPL");
diff --git a/drivers/media/platform/chips-media/wave6/wave6-vpu-core.h b/drivers/media/platform/chips-media/wave6/wave6-vpu-core.h
new file mode 100644
index 000000000000..9b247e5c3c9f
--- /dev/null
+++ b/drivers/media/platform/chips-media/wave6/wave6-vpu-core.h
@@ -0,0 +1,133 @@
+/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) */
+/*
+ * Wave6 series multi-standard codec IP - wave6 core driver
+ *
+ * Copyright (C) 2025 CHIPS&MEDIA INC
+ */
+
+#ifndef __WAVE6_VPU_CORE_H__
+#define __WAVE6_VPU_CORE_H__
+
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-ioctl.h>
+#include <media/v4l2-event.h>
+#include <media/v4l2-fh.h>
+#include <media/videobuf2-v4l2.h>
+#include <media/videobuf2-dma-contig.h>
+#include "wave6-vpuconfig.h"
+#include "wave6-vpuapi.h"
+
+#define vpu_write_reg(CORE, ADDR, DATA) wave6_vpu_writel(CORE, ADDR, DATA)
+#define vpu_read_reg(CORE, ADDR) wave6_vpu_readl(CORE, ADDR)
+
+struct vpu_buffer {
+	struct v4l2_m2m_buffer v4l2_m2m_buf;
+	bool consumed;
+	bool used;
+	bool error;
+	bool force_key_frame;
+	bool force_frame_qp;
+	u32 force_i_frame_qp;
+	u32 force_p_frame_qp;
+	u32 force_b_frame_qp;
+	ktime_t ts_input;
+	ktime_t ts_start;
+	ktime_t ts_finish;
+	ktime_t ts_output;
+	u64 hw_time;
+	u32 average_qp;
+};
+
+enum vpu_fmt_type {
+	VPU_FMT_TYPE_CODEC	= 0,
+	VPU_FMT_TYPE_RAW	= 1
+};
+
+#define VPU_FMT_FLAG_CBCR_INTERLEAVED	BIT(0)
+#define VPU_FMT_FLAG_CRCB_ORDER		BIT(1)
+#define VPU_FMT_FLAG_10BIT		BIT(2)
+#define VPU_FMT_FLAG_RGB		BIT(3)
+
+struct vpu_format {
+	unsigned int v4l2_pix_fmt;
+	unsigned int max_width;
+	unsigned int min_width;
+	unsigned int max_height;
+	unsigned int min_height;
+	unsigned int num_planes;
+	enum frame_buffer_format fb_fmt;
+	enum endian_mode endian;
+	enum csc_format_order csc_fmt_order;
+	unsigned int flags;
+};
+
+/**
+ * struct wave6_vpu_core_resource - VPU CORE device compatible data
+ * @codec_types:		Bitmask of supported codec types
+ * @compatible_fw_version:	Firmware version compatible with driver
+ */
+struct wave6_vpu_core_resource {
+	int codec_types;
+	u32 compatible_fw_version;
+};
+
+static inline struct vpu_instance *wave6_fh_to_vpu_inst(struct v4l2_fh *vfh)
+{
+	return container_of(vfh, struct vpu_instance, v4l2_fh);
+}
+
+static inline struct vpu_instance *wave6_file_to_vpu_inst(struct file *filp)
+{
+	return wave6_fh_to_vpu_inst(file_to_v4l2_fh(filp));
+}
+
+static inline struct vpu_instance *wave6_ctrl_to_vpu_inst(struct v4l2_ctrl *vctrl)
+{
+	return container_of(vctrl->handler, struct vpu_instance, v4l2_ctrl_hdl);
+}
+
+static inline struct vpu_buffer *wave6_to_vpu_buf(struct vb2_v4l2_buffer *vbuf)
+{
+	return container_of(vbuf, struct vpu_buffer, v4l2_m2m_buf.vb);
+}
+
+static inline bool wave6_vpu_both_queues_are_streaming(struct vpu_instance *inst)
+{
+	struct vb2_queue *vq_cap = v4l2_m2m_get_dst_vq(inst->v4l2_fh.m2m_ctx);
+	struct vb2_queue *vq_out = v4l2_m2m_get_src_vq(inst->v4l2_fh.m2m_ctx);
+
+	return vb2_is_streaming(vq_cap) && vb2_is_streaming(vq_out);
+}
+
+u32 wave6_vpu_get_consumed_fb_num(struct vpu_instance *inst);
+void wave6_vpu_core_activate(struct vpu_core_device *core);
+void wave6_update_pix_fmt(struct v4l2_pix_format_mplane *pix_mp,
+			  unsigned int width,
+			  unsigned int height);
+struct vb2_v4l2_buffer *wave6_get_dst_buf_by_addr(struct vpu_instance *inst,
+						  dma_addr_t addr);
+dma_addr_t wave6_get_dma_addr(struct vb2_v4l2_buffer *buf,
+			      unsigned int plane_no);
+enum codec_std wave6_to_codec_std(enum vpu_instance_type type, unsigned int v4l2_pix_fmt);
+const char *wave6_vpu_instance_state_name(enum vpu_instance_state state);
+void wave6_vpu_set_instance_state(struct vpu_instance *inst,
+				  enum vpu_instance_state state);
+u64 wave6_vpu_cycle_to_ns(struct vpu_core_device *core, u64 cycle);
+int wave6_vpu_wait_interrupt(struct vpu_instance *inst, unsigned int timeout);
+int  wave6_vpu_dec_register_device(struct vpu_core_device *core);
+void wave6_vpu_dec_unregister_device(struct vpu_core_device *core);
+int  wave6_vpu_enc_register_device(struct vpu_core_device *core);
+void wave6_vpu_enc_unregister_device(struct vpu_core_device *core);
+void wave6_vpu_finish_job(struct vpu_instance *inst);
+void wave6_vpu_record_performance_timestamps(struct vpu_instance *inst);
+void wave6_vpu_handle_performance(struct vpu_instance *inst,
+				  struct vpu_buffer *vpu_buf);
+void wave6_vpu_reset_performance(struct vpu_instance *inst);
+int wave6_vpu_init_m2m_dev(struct vpu_core_device *core);
+void wave6_vpu_release_m2m_dev(struct vpu_core_device *core);
+int wave6_vpu_subscribe_event(struct v4l2_fh *fh,
+			      const struct v4l2_event_subscription *sub);
+void wave6_vpu_return_buffers(struct vpu_instance *inst,
+			      unsigned int type, enum vb2_buffer_state state);
+
+#endif /* __WAVE6_VPU_CORE_H__ */
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v3 6/9] media: chips-media: wave6: Improve debugging capabilities
  2025-08-29  8:46 [PATCH v3 0/9] Add support for Wave6 video codec driver Nas Chung
                   ` (2 preceding siblings ...)
  2025-08-29  8:46 ` [PATCH v3 5/9] media: chips-media: wave6: Add Wave6 core driver Nas Chung
@ 2025-08-29  8:46 ` Nas Chung
  2025-08-29  8:46 ` [PATCH v3 7/9] media: chips-media: wave6: Add Wave6 thermal cooling device Nas Chung
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 19+ messages in thread
From: Nas Chung @ 2025-08-29  8:46 UTC (permalink / raw)
  To: mchehab, hverkuil, robh, krzk+dt, conor+dt, shawnguo, s.hauer
  Cc: linux-media, devicetree, linux-kernel, linux-imx,
	linux-arm-kernel, jackson.lee, lafley.kim, Nas Chung, Ming Qian

This adds debugfs entries and trace events to provide detailed
debugging information.
These enhancements help diagnose issues and improve debugging
capabilities for the Wave6 core driver.

Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com>
Tested-by: Ming Qian <ming.qian@oss.nxp.com>
---
 .../platform/chips-media/wave6/wave6-trace.h  | 286 ++++++++++++++++++
 .../chips-media/wave6/wave6-vpu-dbg.c         | 225 ++++++++++++++
 .../chips-media/wave6/wave6-vpu-dbg.h         |  14 +
 3 files changed, 525 insertions(+)
 create mode 100644 drivers/media/platform/chips-media/wave6/wave6-trace.h
 create mode 100644 drivers/media/platform/chips-media/wave6/wave6-vpu-dbg.c
 create mode 100644 drivers/media/platform/chips-media/wave6/wave6-vpu-dbg.h

diff --git a/drivers/media/platform/chips-media/wave6/wave6-trace.h b/drivers/media/platform/chips-media/wave6/wave6-trace.h
new file mode 100644
index 000000000000..74227a059ca8
--- /dev/null
+++ b/drivers/media/platform/chips-media/wave6/wave6-trace.h
@@ -0,0 +1,286 @@
+/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) */
+/*
+ * Wave6 series multi-standard codec IP - wave6 driver tracer
+ *
+ * Copyright (C) 2025 CHIPS&MEDIA INC
+ */
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM wave6
+
+#if !defined(__WAVE6_TRACE_H__) || defined(TRACE_HEADER_MULTI_READ)
+#define __WAVE6_TRACE_H__
+
+#include <linux/tracepoint.h>
+#include <media/videobuf2-v4l2.h>
+
+DECLARE_EVENT_CLASS(register_access,
+		    TP_PROTO(struct device *dev, u32 addr, u32 value),
+		    TP_ARGS(dev, addr, value),
+		    TP_STRUCT__entry(__string(name, dev_name(dev))
+				     __field(u32, addr)
+				     __field(u32, value)),
+		    TP_fast_assign(__assign_str(name);
+				   __entry->addr = addr;
+				   __entry->value = value;),
+		    TP_printk("%s:0x%03x 0x%08x",
+			      __get_str(name), __entry->addr, __entry->value));
+
+DEFINE_EVENT(register_access, writel,
+	     TP_PROTO(struct device *dev, u32 addr, u32 value),
+	     TP_ARGS(dev, addr, value));
+DEFINE_EVENT(register_access, readl,
+	     TP_PROTO(struct device *dev, u32 addr, u32 value),
+	     TP_ARGS(dev, addr, value));
+
+TRACE_EVENT(send_command,
+	    TP_PROTO(struct vpu_core_device *core, u32 id, u32 std, u32 cmd),
+	    TP_ARGS(core, id, std, cmd),
+	    TP_STRUCT__entry(__string(name, dev_name(core->dev))
+			     __field(u32, id)
+			     __field(u32, std)
+			     __field(u32, cmd)),
+	    TP_fast_assign(__assign_str(name);
+			   __entry->id = id;
+			   __entry->std = std;
+			   __entry->cmd = cmd;),
+	    TP_printk("%s: inst id %d, std 0x%x, cmd 0x%x",
+		      __get_str(name), __entry->id,
+		      __entry->std, __entry->cmd));
+
+TRACE_EVENT(irq,
+	    TP_PROTO(struct vpu_core_device *core, u32 irq),
+	    TP_ARGS(core, irq),
+	    TP_STRUCT__entry(__string(name, dev_name(core->dev))
+			     __field(u32, irq)),
+	    TP_fast_assign(__assign_str(name);
+			   __entry->irq = irq;),
+	    TP_printk("%s: irq 0x%x", __get_str(name), __entry->irq));
+
+TRACE_EVENT(set_state,
+	    TP_PROTO(struct vpu_instance *inst, u32 state),
+	    TP_ARGS(inst, state),
+	    TP_STRUCT__entry(__string(name, dev_name(inst->dev->dev))
+			     __field(u32, id)
+			     __string(cur_state, wave6_vpu_instance_state_name(inst->state))
+			     __string(nxt_state, wave6_vpu_instance_state_name(state))),
+	    TP_fast_assign(__assign_str(name);
+			   __entry->id = inst->id;
+			   __assign_str(cur_state);
+			   __assign_str(nxt_state);),
+	    TP_printk("%s: inst[%d] set state %s -> %s",
+		      __get_str(name), __entry->id,
+		      __get_str(cur_state), __get_str(nxt_state)));
+
+DECLARE_EVENT_CLASS(inst_internal,
+		    TP_PROTO(struct vpu_instance *inst, bool is_out),
+		    TP_ARGS(inst, is_out),
+		    TP_STRUCT__entry(__string(name, dev_name(inst->dev->dev))
+				     __field(u32, id)
+				     __string(type, is_out ? "output" : "capture")
+				     __field(u32, pixelformat)
+				     __field(u32, width)
+				     __field(u32, height)
+				     __field(u32, buf_cnt_src)
+				     __field(u32, buf_cnt_dst)
+				     __field(u32, processed_cnt)
+				     __field(u32, error_cnt)),
+		    TP_fast_assign(__assign_str(name);
+				   __entry->id = inst->id;
+				   __assign_str(type);
+				   __entry->pixelformat = is_out ? inst->src_fmt.pixelformat :
+								   inst->dst_fmt.pixelformat;
+				   __entry->width = is_out ? inst->src_fmt.width :
+							     inst->dst_fmt.width;
+				   __entry->height = is_out ? inst->src_fmt.height :
+							      inst->dst_fmt.height;
+				   __entry->buf_cnt_src = inst->queued_src_buf_num;
+				   __entry->buf_cnt_dst = inst->queued_dst_buf_num;
+				   __entry->processed_cnt = inst->processed_buf_num;
+				   __entry->error_cnt = inst->error_buf_num;),
+		    TP_printk("%s: inst[%d] %s %c%c%c%c %dx%d, input %d, %d, process %d, error %d",
+			      __get_str(name), __entry->id, __get_str(type),
+			      __entry->pixelformat,
+			      __entry->pixelformat >> 8,
+			      __entry->pixelformat >> 16,
+			      __entry->pixelformat >> 24,
+			      __entry->width, __entry->height,
+			      __entry->buf_cnt_src, __entry->buf_cnt_dst,
+			      __entry->processed_cnt, __entry->error_cnt));
+
+DEFINE_EVENT(inst_internal, start_streaming,
+	     TP_PROTO(struct vpu_instance *inst, bool is_out),
+	     TP_ARGS(inst, is_out));
+
+DEFINE_EVENT(inst_internal, stop_streaming,
+	     TP_PROTO(struct vpu_instance *inst, bool is_out),
+	     TP_ARGS(inst, is_out));
+
+TRACE_EVENT(dec_pic,
+	    TP_PROTO(struct vpu_instance *inst, u32 srcidx, u32 size),
+	    TP_ARGS(inst, srcidx, size),
+	    TP_STRUCT__entry(__string(name, dev_name(inst->dev->dev))
+			     __field(u32, id)
+			     __field(u32, srcidx)
+			     __field(u32, start)
+			     __field(u32, size)),
+	    TP_fast_assign(__assign_str(name);
+			   __entry->id = inst->id;
+			   __entry->srcidx = srcidx;
+			   __entry->start = inst->codec_info->dec_info.stream_rd_ptr;
+			   __entry->size = size;),
+	    TP_printk("%s: inst[%d] src[%2d] %8x, %d",
+		      __get_str(name), __entry->id,
+		      __entry->srcidx, __entry->start, __entry->size));
+
+TRACE_EVENT(source_change,
+	    TP_PROTO(struct vpu_instance *inst, struct dec_seq_info *info),
+	    TP_ARGS(inst, info),
+	    TP_STRUCT__entry(__string(name, dev_name(inst->dev->dev))
+			     __field(u32, id)
+			     __field(u32, width)
+			     __field(u32, height)
+			     __field(u32, profile)
+			     __field(u32, level)
+			     __field(u32, tier)
+			     __field(u32, min_fb_cnt)
+			     __field(u32, disp_delay)
+			     __field(u32, quantization)
+			     __field(u32, colorspace)
+			     __field(u32, xfer_func)
+			     __field(u32, ycbcr_enc)),
+	    TP_fast_assign(__assign_str(name);
+			   __entry->id = inst->id;
+			   __entry->width = info->pic_width,
+			   __entry->height = info->pic_height,
+			   __entry->profile = info->profile,
+			   __entry->level = info->level;
+			   __entry->tier = info->tier;
+			   __entry->min_fb_cnt = info->min_frame_buffer_count;
+			   __entry->disp_delay = info->frame_buf_delay;
+			   __entry->quantization = inst->quantization;
+			   __entry->colorspace = inst->colorspace;
+			   __entry->xfer_func = inst->xfer_func;
+			   __entry->ycbcr_enc = inst->ycbcr_enc;),
+	    TP_printk("%s: inst[%d] %dx%d profile %d %d %d min_fb %d delay %d color %d %d %d %d",
+		      __get_str(name), __entry->id,
+		      __entry->width, __entry->height,
+		      __entry->profile, __entry->level, __entry->tier,
+		      __entry->min_fb_cnt, __entry->disp_delay,
+		      __entry->quantization, __entry->colorspace,
+		      __entry->xfer_func, __entry->ycbcr_enc));
+
+TRACE_EVENT(dec_done,
+	    TP_PROTO(struct vpu_instance *inst, struct dec_output_info *info),
+	    TP_ARGS(inst, info),
+	    TP_STRUCT__entry(__string(name, dev_name(inst->dev->dev))
+			     __field(u32, id)
+			     __field(u32, dec_flag)
+			     __field(u32, dec_poc)
+			     __field(u32, disp_flag)
+			     __field(u32, disp_cnt)
+			     __field(u32, rel_cnt)
+			     __field(u32, src_ch)
+			     __field(u32, eos)
+			     __field(u32, error)
+			     __field(u32, warn)),
+	    TP_fast_assign(__assign_str(name);
+			   __entry->id = inst->id;
+			   __entry->dec_flag = info->frame_decoded;
+			   __entry->dec_poc = info->decoded_poc;
+			   __entry->disp_flag = info->frame_display;
+			   __entry->disp_cnt = info->disp_frame_num;
+			   __entry->rel_cnt = info->release_disp_frame_num;
+			   __entry->src_ch = info->notification_flags & DEC_NOTI_FLAG_SEQ_CHANGE;
+			   __entry->eos = info->stream_end;
+			   __entry->error = info->error_reason;
+			   __entry->warn = info->warn_info;),
+	    TP_printk("%s: inst[%d] dec %d %d disp %d(%d) rel %d src_ch %d eos %d error 0x%x 0x%x",
+		      __get_str(name), __entry->id,
+		      __entry->dec_flag, __entry->dec_poc,
+		      __entry->disp_flag, __entry->disp_cnt,
+		      __entry->rel_cnt,
+		      __entry->src_ch, __entry->eos,
+		      __entry->error, __entry->warn));
+
+TRACE_EVENT(enc_pic,
+	    TP_PROTO(struct vpu_instance *inst, struct enc_param *param),
+	    TP_ARGS(inst, param),
+	    TP_STRUCT__entry(__string(name, dev_name(inst->dev->dev))
+			     __field(u32, id)
+			     __field(u32, srcidx)
+			     __field(u32, buf_y)
+			     __field(u32, buf_cb)
+			     __field(u32, buf_cr)
+			     __field(u32, stride)
+			     __field(u32, buf_strm)
+			     __field(u32, size_strm)
+			     __field(u32, force_type_enable)
+			     __field(u32, force_type)
+			     __field(u32, end_flag)),
+	    TP_fast_assign(__assign_str(name);
+			   __entry->id = inst->id;
+			   __entry->srcidx = param->src_idx;
+			   __entry->buf_y = param->source_frame->buf_y;
+			   __entry->buf_cb = param->source_frame->buf_cb;
+			   __entry->buf_cr = param->source_frame->buf_cr;
+			   __entry->stride = param->source_frame->stride;
+			   __entry->buf_strm = param->pic_stream_buffer_addr;
+			   __entry->size_strm = param->pic_stream_buffer_size;
+			   __entry->force_type_enable = param->force_pic;
+			   __entry->force_type = param->force_pic_type;
+			   __entry->end_flag = param->src_end;),
+	    TP_printk("%s: inst[%d] src[%2d] %8x %8x %8x(%d) dst %8x(%d) force type %d(%d) end %d",
+		      __get_str(name), __entry->id, __entry->srcidx,
+		      __entry->buf_y, __entry->buf_cb, __entry->buf_cr,
+		      __entry->stride, __entry->buf_strm, __entry->size_strm,
+		      __entry->force_type_enable, __entry->force_type,
+		      __entry->end_flag));
+
+TRACE_EVENT(enc_done,
+	    TP_PROTO(struct vpu_instance *inst, struct enc_output_info *info),
+	    TP_ARGS(inst, info),
+	    TP_STRUCT__entry(__string(name, dev_name(inst->dev->dev))
+			     __field(u32, id)
+			     __field(u32, srcidx)
+			     __field(u32, frmidx)
+			     __field(u32, size)
+			     __field(u32, type)
+			     __field(u32, avg_qp)),
+	    TP_fast_assign(__assign_str(name);
+			   __entry->id = inst->id;
+			   __entry->srcidx = info->enc_src_idx;
+			   __entry->frmidx = info->recon_frame_index;
+			   __entry->size = info->bitstream_size;
+			   __entry->type = info->pic_type;
+			   __entry->avg_qp = info->avg_ctu_qp;),
+	    TP_printk("%s: inst[%d] src %d, frame %d, size %d, type %d, qp %d, eos %d",
+		      __get_str(name), __entry->id,
+		      __entry->srcidx, __entry->frmidx,
+		      __entry->size, __entry->type, __entry->avg_qp,
+		      __entry->frmidx == RECON_IDX_FLAG_ENC_END));
+
+TRACE_EVENT(s_ctrl,
+	    TP_PROTO(struct vpu_instance *inst, struct v4l2_ctrl *ctrl),
+	    TP_ARGS(inst, ctrl),
+	    TP_STRUCT__entry(__string(name, dev_name(inst->dev->dev))
+			     __field(u32, id)
+			     __string(ctrl_name, ctrl->name)
+			     __field(u32, val)),
+	    TP_fast_assign(__assign_str(name);
+			   __entry->id = inst->id;
+			   __assign_str(ctrl_name);
+			   __entry->val = ctrl->val;),
+	    TP_printk("%s: inst[%d] %s = %d",
+		      __get_str(name), __entry->id,
+		      __get_str(ctrl_name), __entry->val));
+
+#endif /* __WAVE6_TRACE_H__ */
+
+#undef TRACE_INCLUDE_PATH
+#define TRACE_INCLUDE_PATH .
+#undef TRACE_INCLUDE_FILE
+#define TRACE_INCLUDE_FILE wave6-trace
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
diff --git a/drivers/media/platform/chips-media/wave6/wave6-vpu-dbg.c b/drivers/media/platform/chips-media/wave6/wave6-vpu-dbg.c
new file mode 100644
index 000000000000..7f04060f0aea
--- /dev/null
+++ b/drivers/media/platform/chips-media/wave6/wave6-vpu-dbg.c
@@ -0,0 +1,225 @@
+// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
+/*
+ * Wave6 series multi-standard codec IP - debug interface
+ *
+ * Copyright (C) 2025 CHIPS&MEDIA INC
+ */
+
+#include <linux/types.h>
+#include <linux/debugfs.h>
+#include "wave6-vpu-core.h"
+#include "wave6-vpu-dbg.h"
+
+static int wave6_vpu_dbg_instance(struct seq_file *s, void *data)
+{
+	struct vpu_instance *inst = s->private;
+	struct vpu_performance_info *perf = &inst->performance;
+	struct vb2_queue *vq;
+	char str[128];
+	int num;
+	s64 tmp;
+	s64 fps;
+
+	if (!inst->v4l2_fh.m2m_ctx)
+		return 0;
+
+	num = scnprintf(str, sizeof(str), "[%s]\n",
+			inst->type == VPU_INST_TYPE_DEC ? "Decoder" : "Encoder");
+	if (seq_write(s, str, num))
+		return 0;
+
+	num = scnprintf(str, sizeof(str), "%s : product 0x%x, fw_ver %d.%d.%d(r%d), hw_ver 0x%x\n",
+			dev_name(inst->dev->dev),
+			inst->dev->attr.product_code,
+			FW_VERSION_MAJOR(inst->dev->attr.fw_version),
+			FW_VERSION_MINOR(inst->dev->attr.fw_version),
+			FW_VERSION_REL(inst->dev->attr.fw_version),
+			inst->dev->attr.fw_revision,
+			inst->dev->attr.hw_version);
+	if (seq_write(s, str, num))
+		return 0;
+
+	num = scnprintf(str, sizeof(str), "state = %s\n",
+			wave6_vpu_instance_state_name(inst->state));
+	if (seq_write(s, str, num))
+		return 0;
+
+	vq = v4l2_m2m_get_src_vq(inst->v4l2_fh.m2m_ctx);
+	num = scnprintf(str, sizeof(str),
+			"output (%2d, %2d): fmt = %c%c%c%c %d x %d, %d;\n",
+			vb2_is_streaming(vq),
+			vb2_get_num_buffers(vq),
+			inst->src_fmt.pixelformat,
+			inst->src_fmt.pixelformat >> 8,
+			inst->src_fmt.pixelformat >> 16,
+			inst->src_fmt.pixelformat >> 24,
+			inst->src_fmt.width,
+			inst->src_fmt.height,
+			vq->last_buffer_dequeued);
+	if (seq_write(s, str, num))
+		return 0;
+
+	vq = v4l2_m2m_get_dst_vq(inst->v4l2_fh.m2m_ctx);
+	num = scnprintf(str, sizeof(str),
+			"capture(%2d, %2d): fmt = %c%c%c%c %d x %d, %d;\n",
+			vb2_is_streaming(vq),
+			vb2_get_num_buffers(vq),
+			inst->dst_fmt.pixelformat,
+			inst->dst_fmt.pixelformat >> 8,
+			inst->dst_fmt.pixelformat >> 16,
+			inst->dst_fmt.pixelformat >> 24,
+			inst->dst_fmt.width,
+			inst->dst_fmt.height,
+			vq->last_buffer_dequeued);
+	if (seq_write(s, str, num))
+		return 0;
+
+	num = scnprintf(str, sizeof(str), "crop: (%d, %d) %d x %d\n",
+			inst->crop.left,
+			inst->crop.top,
+			inst->crop.width,
+			inst->crop.height);
+	if (seq_write(s, str, num))
+		return 0;
+
+	if (inst->scaler_info.enable) {
+		num = scnprintf(str, sizeof(str), "scale: %d x %d\n",
+				inst->scaler_info.width, inst->scaler_info.height);
+		if (seq_write(s, str, num))
+			return 0;
+	}
+
+	num = scnprintf(str, sizeof(str),
+			"queued src %d, dst %d, process %d, sequence %d, error %d, drain %d:%d\n",
+			inst->queued_src_buf_num,
+			inst->queued_dst_buf_num,
+			inst->processed_buf_num,
+			inst->sequence,
+			inst->error_buf_num,
+			inst->v4l2_fh.m2m_ctx->out_q_ctx.buffered,
+			inst->eos);
+	if (seq_write(s, str, num))
+		return 0;
+
+	num = scnprintf(str, sizeof(str), "fps");
+	if (seq_write(s, str, num))
+		return 0;
+	tmp = MSEC_PER_SEC * inst->processed_buf_num;
+	if (perf->ts_last > perf->ts_first + NSEC_PER_MSEC) {
+		fps = DIV_ROUND_CLOSEST(tmp, (perf->ts_last - perf->ts_first) / NSEC_PER_MSEC);
+		num = scnprintf(str, sizeof(str), " actual: %lld;", fps);
+		if (seq_write(s, str, num))
+			return 0;
+	}
+	if (perf->total_sw_time) {
+		fps = DIV_ROUND_CLOSEST(tmp, perf->total_sw_time / NSEC_PER_MSEC);
+		num = scnprintf(str, sizeof(str), " sw: %lld;", fps);
+		if (seq_write(s, str, num))
+			return 0;
+	}
+	if (perf->total_hw_time) {
+		fps = DIV_ROUND_CLOSEST(tmp, perf->total_hw_time / NSEC_PER_MSEC);
+		num = scnprintf(str, sizeof(str), " hw: %lld", fps);
+		if (seq_write(s, str, num))
+			return 0;
+	}
+	num = scnprintf(str, sizeof(str), "\n");
+	if (seq_write(s, str, num))
+		return 0;
+
+	num = scnprintf(str, sizeof(str),
+			"latency(ms) first: %llu.%06llu, max %llu.%06llu, setup %llu.%06llu\n",
+			perf->latency_first / NSEC_PER_MSEC,
+			perf->latency_first % NSEC_PER_MSEC,
+			perf->latency_max / NSEC_PER_MSEC,
+			perf->latency_max % NSEC_PER_MSEC,
+			(perf->ts_first - perf->ts_start) / NSEC_PER_MSEC,
+			(perf->ts_first - perf->ts_start) % NSEC_PER_MSEC);
+	if (seq_write(s, str, num))
+		return 0;
+
+	num = scnprintf(str, sizeof(str),
+			"process frame time(ms) min: %llu.%06llu, max %llu.%06llu\n",
+			perf->min_process_time / NSEC_PER_MSEC,
+			perf->min_process_time % NSEC_PER_MSEC,
+			perf->max_process_time / NSEC_PER_MSEC,
+			perf->max_process_time % NSEC_PER_MSEC);
+	if (seq_write(s, str, num))
+		return 0;
+
+	if (inst->type == VPU_INST_TYPE_DEC) {
+		num = scnprintf(str, sizeof(str), "%s order\n",
+				inst->disp_mode == DISP_MODE_DISP_ORDER ? "display" : "decode");
+		if (seq_write(s, str, num))
+			return 0;
+	} else {
+		struct enc_info *p_enc_info = &inst->codec_info->enc_info;
+		struct enc_codec_param *param = &p_enc_info->open_param.codec_param;
+
+		num = scnprintf(str, sizeof(str), "profile %d, level %d, tier %d\n",
+				param->profile, param->level, param->tier);
+		if (seq_write(s, str, num))
+			return 0;
+
+		num = scnprintf(str, sizeof(str), "frame_rate %d, idr_period %d, intra_period %d\n",
+				param->frame_rate, param->idr_period, param->intra_period);
+		if (seq_write(s, str, num))
+			return 0;
+
+		num = scnprintf(str, sizeof(str), "rc %d, mode %d, bitrate %d\n",
+				param->en_rate_control,
+				param->rc_mode,
+				param->bitrate);
+		if (seq_write(s, str, num))
+			return 0;
+
+		num = scnprintf(str, sizeof(str),
+				"qp %d, i_qp [%d, %d], p_qp [%d, %d], b_qp [%d, %d]\n",
+				param->qp,
+				param->min_qp_i, param->max_qp_i,
+				param->min_qp_p, param->max_qp_p,
+				param->min_qp_b, param->max_qp_b);
+		if (seq_write(s, str, num))
+			return 0;
+	}
+
+	return 0;
+}
+
+static int wave6_vpu_dbg_open(struct inode *inode, struct file *filp)
+{
+	return single_open(filp, wave6_vpu_dbg_instance, inode->i_private);
+}
+
+static const struct file_operations wave6_vpu_dbg_fops = {
+	.owner = THIS_MODULE,
+	.open = wave6_vpu_dbg_open,
+	.release = single_release,
+	.read = seq_read,
+};
+
+int wave6_vpu_create_dbgfs_file(struct vpu_instance *inst)
+{
+	char name[64];
+
+	if (WARN_ON(!inst || !inst->dev || IS_ERR_OR_NULL(inst->dev->debugfs)))
+		return -EINVAL;
+
+	scnprintf(name, sizeof(name), "instance.%d", inst->id);
+	inst->debugfs = debugfs_create_file((const char *)name,
+					    VERIFY_OCTAL_PERMISSIONS(0444),
+					    inst->dev->debugfs,
+					    inst,
+					    &wave6_vpu_dbg_fops);
+
+	return 0;
+}
+
+void wave6_vpu_remove_dbgfs_file(struct vpu_instance *inst)
+{
+	if (WARN_ON(!inst || !inst->debugfs))
+		return;
+
+	debugfs_remove(inst->debugfs);
+	inst->debugfs = NULL;
+}
diff --git a/drivers/media/platform/chips-media/wave6/wave6-vpu-dbg.h b/drivers/media/platform/chips-media/wave6/wave6-vpu-dbg.h
new file mode 100644
index 000000000000..6453eb2de76f
--- /dev/null
+++ b/drivers/media/platform/chips-media/wave6/wave6-vpu-dbg.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) */
+/*
+ * Wave6 series multi-standard codec IP - debug interface
+ *
+ * Copyright (C) 2025 CHIPS&MEDIA INC
+ */
+
+#ifndef __WAVE6_VPU_DBG_H__
+#define __WAVE6_VPU_DBG_H__
+
+int wave6_vpu_create_dbgfs_file(struct vpu_instance *inst);
+void wave6_vpu_remove_dbgfs_file(struct vpu_instance *inst);
+
+#endif /* __WAVE6_VPU_DBG_H__ */
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v3 7/9] media: chips-media: wave6: Add Wave6 thermal cooling device
  2025-08-29  8:46 [PATCH v3 0/9] Add support for Wave6 video codec driver Nas Chung
                   ` (3 preceding siblings ...)
  2025-08-29  8:46 ` [PATCH v3 6/9] media: chips-media: wave6: Improve debugging capabilities Nas Chung
@ 2025-08-29  8:46 ` Nas Chung
  2025-08-29  8:46 ` [PATCH v3 8/9] media: chips-media: wave6: Add Wave6 control driver Nas Chung
  2025-08-29  8:46 ` [PATCH v3 9/9] arm64: dts: freescale: imx95: Add video codec node Nas Chung
  6 siblings, 0 replies; 19+ messages in thread
From: Nas Chung @ 2025-08-29  8:46 UTC (permalink / raw)
  To: mchehab, hverkuil, robh, krzk+dt, conor+dt, shawnguo, s.hauer
  Cc: linux-media, devicetree, linux-kernel, linux-imx,
	linux-arm-kernel, jackson.lee, lafley.kim, Nas Chung, Ming Qian

This adds a thermal cooling device for the Wave6 VPU.
The device operates within the Linux thermal framework,
adjusting the VPU performance state based on thermal conditions.

Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com>
Tested-by: Ming Qian <ming.qian@oss.nxp.com>
---
 .../chips-media/wave6/wave6-vpu-thermal.c     | 136 ++++++++++++++++++
 .../chips-media/wave6/wave6-vpu-thermal.h     |  25 ++++
 2 files changed, 161 insertions(+)
 create mode 100644 drivers/media/platform/chips-media/wave6/wave6-vpu-thermal.c
 create mode 100644 drivers/media/platform/chips-media/wave6/wave6-vpu-thermal.h

diff --git a/drivers/media/platform/chips-media/wave6/wave6-vpu-thermal.c b/drivers/media/platform/chips-media/wave6/wave6-vpu-thermal.c
new file mode 100644
index 000000000000..24d897dd72bb
--- /dev/null
+++ b/drivers/media/platform/chips-media/wave6/wave6-vpu-thermal.c
@@ -0,0 +1,136 @@
+// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
+/*
+ * Wave6 series multi-standard codec IP - wave6 thermal cooling interface
+ *
+ * Copyright (C) 2025 CHIPS&MEDIA INC
+ *
+ */
+
+#include <linux/pm_domain.h>
+#include <linux/pm_opp.h>
+#include <linux/units.h>
+#include "wave6-vpu-thermal.h"
+
+static int wave6_vpu_thermal_cooling_update(struct vpu_thermal_cooling *thermal,
+					    int state)
+{
+	unsigned long new_clock_rate;
+	int ret;
+
+	if (state > thermal->thermal_max || !thermal->cooling)
+		return 0;
+
+	new_clock_rate = DIV_ROUND_UP(thermal->freq_table[state], HZ_PER_KHZ);
+	dev_dbg(thermal->dev, "receive cooling state: %d, new clock rate %ld\n",
+		state, new_clock_rate);
+
+	ret = dev_pm_genpd_set_performance_state(thermal->dev, new_clock_rate);
+	if (ret && !((ret == -ENODEV) || (ret == -EOPNOTSUPP))) {
+		dev_err(thermal->dev, "failed to set perf to %lu, ret = %d\n",
+			new_clock_rate, ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int wave6_vpu_cooling_get_max_state(struct thermal_cooling_device *cdev,
+					   unsigned long *state)
+{
+	struct vpu_thermal_cooling *thermal = cdev->devdata;
+
+	*state = thermal->thermal_max;
+
+	return 0;
+}
+
+static int wave6_vpu_cooling_get_cur_state(struct thermal_cooling_device *cdev,
+					   unsigned long *state)
+{
+	struct vpu_thermal_cooling *thermal = cdev->devdata;
+
+	*state = thermal->thermal_event;
+
+	return 0;
+}
+
+static int wave6_vpu_cooling_set_cur_state(struct thermal_cooling_device *cdev,
+					   unsigned long state)
+{
+	struct vpu_thermal_cooling *thermal = cdev->devdata;
+
+	thermal->thermal_event = state;
+	wave6_vpu_thermal_cooling_update(thermal, state);
+
+	return 0;
+}
+
+static struct thermal_cooling_device_ops wave6_cooling_ops = {
+	.get_max_state = wave6_vpu_cooling_get_max_state,
+	.get_cur_state = wave6_vpu_cooling_get_cur_state,
+	.set_cur_state = wave6_vpu_cooling_set_cur_state,
+};
+
+int wave6_vpu_cooling_init(struct vpu_thermal_cooling *thermal)
+{
+	int i;
+	int num_opps;
+	unsigned long freq;
+
+	if (WARN_ON(!thermal || !thermal->dev))
+		return -EINVAL;
+
+	num_opps = dev_pm_opp_get_opp_count(thermal->dev);
+	if (num_opps <= 0) {
+		dev_err(thermal->dev, "fail to get pm opp count, ret = %d\n", num_opps);
+		return -ENODEV;
+	}
+
+	thermal->freq_table = kcalloc(num_opps, sizeof(*thermal->freq_table), GFP_KERNEL);
+	if (!thermal->freq_table)
+		goto error;
+
+	for (i = 0, freq = ULONG_MAX; i < num_opps; i++, freq--) {
+		struct dev_pm_opp *opp;
+
+		opp = dev_pm_opp_find_freq_floor(thermal->dev, &freq);
+		if (IS_ERR(opp))
+			break;
+
+		dev_pm_opp_put(opp);
+
+		dev_dbg(thermal->dev, "[%d] = %ld\n", i, freq);
+		if (freq < 100 * HZ_PER_MHZ)
+			break;
+
+		thermal->freq_table[i] = freq;
+		thermal->thermal_max = i;
+	}
+
+	if (!thermal->thermal_max)
+		goto error;
+
+	thermal->thermal_event = 0;
+	thermal->cooling = thermal_of_cooling_device_register(thermal->dev->of_node,
+							      dev_name(thermal->dev),
+							      thermal,
+							      &wave6_cooling_ops);
+	if (IS_ERR(thermal->cooling)) {
+		dev_err(thermal->dev, "register cooling device failed\n");
+		goto error;
+	}
+
+	return 0;
+
+error:
+	wave6_vpu_cooling_remove(thermal);
+
+	return -EINVAL;
+}
+
+void wave6_vpu_cooling_remove(struct vpu_thermal_cooling *thermal)
+{
+	thermal_cooling_device_unregister(thermal->cooling);
+	kfree(thermal->freq_table);
+	thermal->freq_table = NULL;
+}
diff --git a/drivers/media/platform/chips-media/wave6/wave6-vpu-thermal.h b/drivers/media/platform/chips-media/wave6/wave6-vpu-thermal.h
new file mode 100644
index 000000000000..b8c45f6785c5
--- /dev/null
+++ b/drivers/media/platform/chips-media/wave6/wave6-vpu-thermal.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) */
+/*
+ * Wave6 series multi-standard codec IP - wave6 thermal cooling interface
+ *
+ * Copyright (C) 2025 CHIPS&MEDIA INC
+ *
+ */
+
+#ifndef __WAVE6_VPU_THERMAL_H__
+#define __WAVE6_VPU_THERMAL_H__
+
+#include <linux/thermal.h>
+
+struct vpu_thermal_cooling {
+	struct device *dev;
+	int thermal_event;
+	int thermal_max;
+	struct thermal_cooling_device *cooling;
+	unsigned long *freq_table;
+};
+
+int wave6_vpu_cooling_init(struct vpu_thermal_cooling *thermal);
+void wave6_vpu_cooling_remove(struct vpu_thermal_cooling *thermal);
+
+#endif /* __WAVE6_VPU_THERMAL_H__ */
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v3 8/9] media: chips-media: wave6: Add Wave6 control driver
  2025-08-29  8:46 [PATCH v3 0/9] Add support for Wave6 video codec driver Nas Chung
                   ` (4 preceding siblings ...)
  2025-08-29  8:46 ` [PATCH v3 7/9] media: chips-media: wave6: Add Wave6 thermal cooling device Nas Chung
@ 2025-08-29  8:46 ` Nas Chung
  2025-08-29 14:06   ` Krzysztof Kozlowski
  2025-08-29  8:46 ` [PATCH v3 9/9] arm64: dts: freescale: imx95: Add video codec node Nas Chung
  6 siblings, 1 reply; 19+ messages in thread
From: Nas Chung @ 2025-08-29  8:46 UTC (permalink / raw)
  To: mchehab, hverkuil, robh, krzk+dt, conor+dt, shawnguo, s.hauer
  Cc: linux-media, devicetree, linux-kernel, linux-imx,
	linux-arm-kernel, jackson.lee, lafley.kim, Nas Chung, Ming Qian

This adds the control driver for the Chips&Media Wave6 video codec IP.

On NXP i.MX platforms, the Wave6 consists of two functional regions:
a control region responsible for firmware and shared resources,
and a core region for encoding and decoding.

The control driver manages shared resources such as firmware loading,
firmware memory allocation, and synchronization required by the core.

It also binds the `wave6-core` sub-device and coordinates with it
for firmware and power state management.

Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com>
Tested-by: Ming Qian <ming.qian@oss.nxp.com>
---
 drivers/media/platform/chips-media/Kconfig    |   1 +
 drivers/media/platform/chips-media/Makefile   |   1 +
 .../media/platform/chips-media/wave6/Kconfig  |  17 +
 .../media/platform/chips-media/wave6/Makefile |  17 +
 .../platform/chips-media/wave6/wave6-vpu.c    | 654 ++++++++++++++++++
 .../platform/chips-media/wave6/wave6-vpu.h    | 131 ++++
 6 files changed, 821 insertions(+)
 create mode 100644 drivers/media/platform/chips-media/wave6/Kconfig
 create mode 100644 drivers/media/platform/chips-media/wave6/Makefile
 create mode 100644 drivers/media/platform/chips-media/wave6/wave6-vpu.c
 create mode 100644 drivers/media/platform/chips-media/wave6/wave6-vpu.h

diff --git a/drivers/media/platform/chips-media/Kconfig b/drivers/media/platform/chips-media/Kconfig
index ad350eb6b1fc..8ef7fc8029a4 100644
--- a/drivers/media/platform/chips-media/Kconfig
+++ b/drivers/media/platform/chips-media/Kconfig
@@ -4,3 +4,4 @@ comment "Chips&Media media platform drivers"
 
 source "drivers/media/platform/chips-media/coda/Kconfig"
 source "drivers/media/platform/chips-media/wave5/Kconfig"
+source "drivers/media/platform/chips-media/wave6/Kconfig"
diff --git a/drivers/media/platform/chips-media/Makefile b/drivers/media/platform/chips-media/Makefile
index 6b5d99de8b54..b9a07a91c9d6 100644
--- a/drivers/media/platform/chips-media/Makefile
+++ b/drivers/media/platform/chips-media/Makefile
@@ -2,3 +2,4 @@
 
 obj-y += coda/
 obj-y += wave5/
+obj-y += wave6/
diff --git a/drivers/media/platform/chips-media/wave6/Kconfig b/drivers/media/platform/chips-media/wave6/Kconfig
new file mode 100644
index 000000000000..63d79c56c7fc
--- /dev/null
+++ b/drivers/media/platform/chips-media/wave6/Kconfig
@@ -0,0 +1,17 @@
+# SPDX-License-Identifier: GPL-2.0
+
+config VIDEO_WAVE6_VPU
+	tristate "Chips&Media Wave6 Codec Driver"
+	depends on V4L_MEM2MEM_DRIVERS
+	depends on VIDEO_DEV && OF
+	depends on ARCH_MXC || COMPILE_TEST
+	select VIDEOBUF2_DMA_CONTIG
+	select V4L2_MEM2MEM_DEV
+	select GENERIC_ALLOCATOR
+	help
+	  Chips&Media Wave6 stateful codec driver.
+	  The wave6 driver manages shared resources such as firmware memory.
+	  The wave6-core driver provides encoding and decoding capabilities
+	  for H.264, HEVC, and other video formats.
+	  To compile this driver as modules, choose M here: the
+	  modules will be called wave6 and wave6-core.
diff --git a/drivers/media/platform/chips-media/wave6/Makefile b/drivers/media/platform/chips-media/wave6/Makefile
new file mode 100644
index 000000000000..06f8ac9bef14
--- /dev/null
+++ b/drivers/media/platform/chips-media/wave6/Makefile
@@ -0,0 +1,17 @@
+# SPDX-License-Identifier: GPL-2.0
+
+# tell define_trace.h where to find the trace header
+CFLAGS_wave6-vpu-core.o := -I$(src)
+
+wave6-objs += wave6-vpu.o \
+	      wave6-vpu-thermal.o
+obj-$(CONFIG_VIDEO_WAVE6_VPU) += wave6.o
+
+wave6-core-objs += wave6-vpu-core.o \
+		   wave6-vpu-v4l2.o \
+		   wave6-vpu-dbg.o \
+		   wave6-vpuapi.o \
+		   wave6-vpu-dec.o \
+		   wave6-vpu-enc.o \
+		   wave6-hw.o
+obj-$(CONFIG_VIDEO_WAVE6_VPU) += wave6-core.o
diff --git a/drivers/media/platform/chips-media/wave6/wave6-vpu.c b/drivers/media/platform/chips-media/wave6/wave6-vpu.c
new file mode 100644
index 000000000000..2b8c3a5fec33
--- /dev/null
+++ b/drivers/media/platform/chips-media/wave6/wave6-vpu.c
@@ -0,0 +1,654 @@
+// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
+/*
+ * Wave6 series multi-standard codec IP - wave6 driver
+ *
+ * Copyright (C) 2025 CHIPS&MEDIA INC
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/of_platform.h>
+#include <linux/firmware.h>
+#include <linux/interrupt.h>
+#include <linux/pm_runtime.h>
+#include <linux/pm_domain.h>
+#include <linux/dma-mapping.h>
+#include <linux/iopoll.h>
+#include <linux/genalloc.h>
+
+#include "wave6-vpuconfig.h"
+#include "wave6-regdefine.h"
+#include "wave6-vpu.h"
+
+static const struct wave6_vpu_resource wave633c_data = {
+	.fw_name = "cnm/wave633c_imx9_codec_fw.bin",
+	/* For HEVC, AVC, 4096x4096, 8bit */
+	.sram_size = 0x14800,
+};
+
+static const char *wave6_vpu_state_name(enum wave6_vpu_state state)
+{
+	switch (state) {
+	case WAVE6_VPU_STATE_OFF:
+		return "off";
+	case WAVE6_VPU_STATE_PREPARE:
+		return "prepare";
+	case WAVE6_VPU_STATE_ON:
+		return "on";
+	case WAVE6_VPU_STATE_SLEEP:
+		return "sleep";
+	default:
+		return "unknown";
+	}
+}
+
+static bool wave6_vpu_valid_transition(struct wave6_vpu_device *vpu,
+				       enum wave6_vpu_state next)
+{
+	switch (vpu->state) {
+	case WAVE6_VPU_STATE_OFF:
+		/* to PREPARE: first boot attempt */
+		/* to ON: already booted before, skipping boot */
+		if (next == WAVE6_VPU_STATE_PREPARE ||
+		    next == WAVE6_VPU_STATE_ON)
+			return true;
+		break;
+	case WAVE6_VPU_STATE_PREPARE:
+		/* to OFF: boot failed */
+		/* to ON: boot successful */
+		if (next == WAVE6_VPU_STATE_OFF ||
+		    next == WAVE6_VPU_STATE_ON)
+			return true;
+		break;
+	case WAVE6_VPU_STATE_ON:
+		/* to OFF: sleep failed */
+		/* to SLEEP: sleep successful */
+		if (next == WAVE6_VPU_STATE_OFF ||
+		    next == WAVE6_VPU_STATE_SLEEP)
+			return true;
+		break;
+	case WAVE6_VPU_STATE_SLEEP:
+		/* to OFF: resume failed */
+		/* to ON: resume successful */
+		if (next == WAVE6_VPU_STATE_OFF ||
+		    next == WAVE6_VPU_STATE_ON)
+			return true;
+		break;
+	}
+
+	dev_err(vpu->dev, "invalid transition: %s -> %s\n",
+		wave6_vpu_state_name(vpu->state), wave6_vpu_state_name(next));
+
+	return false;
+}
+
+static void wave6_vpu_set_state(struct wave6_vpu_device *vpu,
+				enum wave6_vpu_state state)
+{
+	if (!wave6_vpu_valid_transition(vpu, state))
+		return;
+
+	dev_dbg(vpu->dev, "set state: %s -> %s\n",
+		wave6_vpu_state_name(vpu->state), wave6_vpu_state_name(state));
+
+	vpu->state = state;
+}
+
+static int wave6_vpu_wait_busy(struct vpu_core_device *core)
+{
+	u32 val;
+
+	return read_poll_timeout(wave6_vdi_readl, val, !val,
+				 W6_VPU_POLL_DELAY_US, W6_VPU_POLL_TIMEOUT,
+				 false, core->reg_base, W6_VPU_BUSY_STATUS);
+}
+
+static int wave6_vpu_check_result(struct vpu_core_device *core)
+{
+	if (wave6_vdi_readl(core->reg_base, W6_RET_SUCCESS))
+		return 0;
+
+	return wave6_vdi_readl(core->reg_base, W6_RET_FAIL_REASON);
+}
+
+static u32 wave6_vpu_get_code_buf_size(struct wave6_vpu_device *vpu)
+{
+	return min_t(u32, vpu->code_buf.size, W6_MAX_CODE_BUF_SIZE);
+}
+
+static void wave6_vpu_remap_code_buf(struct wave6_vpu_device *vpu)
+{
+	dma_addr_t code_base = vpu->code_buf.dma_addr;
+	u32 i, reg_val;
+
+	for (i = 0; i < wave6_vpu_get_code_buf_size(vpu) / W6_MAX_REMAP_PAGE_SIZE; i++) {
+		reg_val = REMAP_CTRL_ON |
+			  REMAP_CTRL_INDEX(i) |
+			  REMAP_CTRL_PAGE_SIZE_ON |
+			  REMAP_CTRL_PAGE_SIZE(W6_MAX_REMAP_PAGE_SIZE);
+		wave6_vdi_writel(vpu->reg_base, W6_VPU_REMAP_CTRL_GB, reg_val);
+		wave6_vdi_writel(vpu->reg_base, W6_VPU_REMAP_VADDR_GB,
+				 i * W6_MAX_REMAP_PAGE_SIZE);
+		wave6_vdi_writel(vpu->reg_base, W6_VPU_REMAP_PADDR_GB,
+				 code_base + i * W6_MAX_REMAP_PAGE_SIZE);
+	}
+}
+
+static void wave6_vpu_init_code_buf(struct wave6_vpu_device *vpu)
+{
+	if (vpu->code_buf.size < W6_CODE_BUF_SIZE) {
+		dev_warn(vpu->dev,
+			 "code buf size (%zu) is too small\n", vpu->code_buf.size);
+		vpu->code_buf.phys_addr = 0;
+		vpu->code_buf.size = 0;
+		memset(&vpu->code_buf, 0, sizeof(vpu->code_buf));
+		return;
+	}
+
+	vpu->code_buf.vaddr = devm_memremap(vpu->dev,
+					    vpu->code_buf.phys_addr,
+					    vpu->code_buf.size,
+					    MEMREMAP_WC);
+	if (!vpu->code_buf.vaddr) {
+		memset(&vpu->code_buf, 0, sizeof(vpu->code_buf));
+		return;
+	}
+
+	vpu->code_buf.dma_addr = dma_map_resource(vpu->dev,
+						  vpu->code_buf.phys_addr,
+						  vpu->code_buf.size,
+						  DMA_BIDIRECTIONAL,
+						  0);
+	if (!vpu->code_buf.dma_addr) {
+		memset(&vpu->code_buf, 0, sizeof(vpu->code_buf));
+		return;
+	}
+}
+
+static void wave6_vpu_init_work_buf(struct wave6_vpu_device *vpu,
+				    struct vpu_core_device *core)
+{
+	struct wave6_vpu_work_buf *pbuf, *tmp;
+	int ret;
+
+	lockdep_assert_held(&vpu->lock);
+
+	if (!core)
+		goto init_work_buf_done;
+
+	wave6_vdi_writel(core->reg_base, W6_VPU_BUSY_STATUS, BUSY_STATUS_SET);
+	wave6_vdi_writel(core->reg_base, W6_COMMAND, W6_CMD_INIT_WORK_BUF);
+	wave6_vdi_writel(core->reg_base, W6_VPU_HOST_INT_REQ, HOST_INT_REQ_ON);
+
+	ret = wave6_vpu_wait_busy(core);
+	if (ret) {
+		dev_err(vpu->dev, "init work buf failed\n");
+		return;
+	}
+
+	ret = wave6_vpu_check_result(core);
+	if (ret) {
+		dev_err(vpu->dev, "init work buf failed, reason 0x%x\n", ret);
+		return;
+	}
+
+init_work_buf_done:
+	list_for_each_entry_safe(pbuf, tmp, &vpu->work_buffers, list) {
+		list_del(&pbuf->list);
+		wave6_vdi_free_dma(&pbuf->buf);
+		kfree(pbuf);
+	}
+}
+
+static int wave6_vpu_init_vpu(struct wave6_vpu_device *vpu,
+			      struct vpu_core_device *core)
+{
+	int ret;
+
+	lockdep_assert_held(&vpu->lock);
+
+	/* try init directly as firmware is running */
+	if (wave6_vdi_readl(core->reg_base, W6_VPU_VCPU_CUR_PC))
+		goto init_done;
+
+	wave6_vpu_set_state(vpu, WAVE6_VPU_STATE_PREPARE);
+
+	wave6_vpu_remap_code_buf(vpu);
+
+	wave6_vdi_writel(core->reg_base, W6_VPU_BUSY_STATUS, BUSY_STATUS_SET);
+	wave6_vdi_writel(core->reg_base, W6_CMD_INIT_VPU_SEC_AXI_BASE_CORE0,
+			 vpu->sram_buf.dma_addr);
+	wave6_vdi_writel(core->reg_base, W6_CMD_INIT_VPU_SEC_AXI_SIZE_CORE0,
+			 vpu->sram_buf.size);
+	wave6_vdi_writel(vpu->reg_base, W6_COMMAND_GB, W6_CMD_INIT_VPU);
+	wave6_vdi_writel(vpu->reg_base, W6_VPU_REMAP_CORE_START_GB,
+			 REMAP_CORE_START_ON);
+
+	ret = wave6_vpu_wait_busy(core);
+	if (ret) {
+		dev_err(vpu->dev, "init vpu timeout\n");
+		wave6_vpu_set_state(vpu, WAVE6_VPU_STATE_OFF);
+		return -EINVAL;
+	}
+
+	ret = wave6_vpu_check_result(core);
+	if (ret) {
+		dev_err(vpu->dev, "init vpu fail, reason 0x%x\n", ret);
+		wave6_vpu_set_state(vpu, WAVE6_VPU_STATE_OFF);
+		return -EIO;
+	}
+
+init_done:
+	wave6_vpu_init_work_buf(vpu, core);
+	wave6_vpu_set_state(vpu, WAVE6_VPU_STATE_ON);
+
+	return 0;
+}
+
+static int wave6_vpu_sleep(struct wave6_vpu_device *vpu,
+			   struct vpu_core_device *core)
+{
+	int ret;
+
+	lockdep_assert_held(&vpu->lock);
+
+	if (!wave6_vdi_readl(core->reg_base, W6_VPU_VCPU_CUR_PC)) {
+		wave6_vpu_set_state(vpu, WAVE6_VPU_STATE_OFF);
+		return 0;
+	}
+
+	wave6_vdi_writel(core->reg_base, W6_VPU_BUSY_STATUS, BUSY_STATUS_SET);
+	wave6_vdi_writel(core->reg_base, W6_COMMAND, W6_CMD_SLEEP_VPU);
+	wave6_vdi_writel(core->reg_base, W6_VPU_HOST_INT_REQ, HOST_INT_REQ_ON);
+
+	ret = wave6_vpu_wait_busy(core);
+	if (ret) {
+		dev_err(vpu->dev, "sleep vpu timeout\n");
+		wave6_vpu_set_state(vpu, WAVE6_VPU_STATE_OFF);
+		return -EINVAL;
+	}
+
+	ret = wave6_vpu_check_result(core);
+	if (ret) {
+		dev_err(vpu->dev, "sleep vpu fail, reason 0x%x\n", ret);
+		wave6_vpu_set_state(vpu, WAVE6_VPU_STATE_OFF);
+		return -EIO;
+	}
+
+	wave6_vpu_set_state(vpu, WAVE6_VPU_STATE_SLEEP);
+
+	return 0;
+}
+
+static int wave6_vpu_wakeup(struct wave6_vpu_device *vpu,
+			    struct vpu_core_device *core)
+{
+	int ret;
+
+	lockdep_assert_held(&vpu->lock);
+
+	/* try wakeup directly as firmware is running */
+	if (wave6_vdi_readl(core->reg_base, W6_VPU_VCPU_CUR_PC))
+		goto wakeup_done;
+
+	wave6_vpu_remap_code_buf(vpu);
+
+	wave6_vdi_writel(core->reg_base, W6_VPU_BUSY_STATUS, BUSY_STATUS_SET);
+	wave6_vdi_writel(core->reg_base, W6_CMD_INIT_VPU_SEC_AXI_BASE_CORE0,
+			 vpu->sram_buf.dma_addr);
+	wave6_vdi_writel(core->reg_base, W6_CMD_INIT_VPU_SEC_AXI_SIZE_CORE0,
+			 vpu->sram_buf.size);
+	wave6_vdi_writel(vpu->reg_base, W6_COMMAND_GB, W6_CMD_WAKEUP_VPU);
+	wave6_vdi_writel(vpu->reg_base, W6_VPU_REMAP_CORE_START_GB,
+			 REMAP_CORE_START_ON);
+
+	ret = wave6_vpu_wait_busy(core);
+	if (ret) {
+		dev_err(vpu->dev, "wakeup vpu timeout\n");
+		wave6_vpu_set_state(vpu, WAVE6_VPU_STATE_OFF);
+		return -EINVAL;
+	}
+
+	ret = wave6_vpu_check_result(core);
+	if (ret) {
+		dev_err(vpu->dev, "wakeup vpu fail, reason 0x%x\n", ret);
+		wave6_vpu_set_state(vpu, WAVE6_VPU_STATE_OFF);
+		return -EIO;
+	}
+
+wakeup_done:
+	wave6_vpu_set_state(vpu, WAVE6_VPU_STATE_ON);
+
+	return 0;
+}
+
+static int wave6_vpu_try_boot(struct wave6_vpu_device *vpu,
+			      struct vpu_core_device *core)
+{
+	u32 product_code;
+	int ret;
+
+	lockdep_assert_held(&vpu->lock);
+
+	if (vpu->state != WAVE6_VPU_STATE_OFF && vpu->state != WAVE6_VPU_STATE_SLEEP)
+		return 0;
+
+	product_code = wave6_vdi_readl(core->reg_base, W6_VPU_RET_PRODUCT_CODE);
+	if (!PRODUCT_CODE_W_SERIES(product_code)) {
+		dev_err(vpu->dev, "unknown product : %08x\n", product_code);
+		return -EINVAL;
+	}
+
+	if (vpu->state == WAVE6_VPU_STATE_SLEEP) {
+		ret = wave6_vpu_wakeup(vpu, core);
+		return ret;
+	}
+
+	ret = wave6_vpu_init_vpu(vpu, core);
+
+	return ret;
+}
+
+static int wave6_vpu_get(struct wave6_vpu_device *vpu,
+			 struct vpu_core_device *core)
+{
+	int ret;
+
+	if (WARN_ON(!vpu || !core))
+		return -EINVAL;
+
+	guard(mutex)(&vpu->lock);
+
+	if (!vpu->fw_available)
+		return -EINVAL;
+
+	/* Only the first core executes boot; others return */
+	if (atomic_inc_return(&vpu->core_count) > 1)
+		return 0;
+
+	ret = pm_runtime_resume_and_get(vpu->dev);
+	if (ret)
+		goto error_pm;
+
+	ret = wave6_vpu_try_boot(vpu, core);
+	if (ret)
+		goto error_boot;
+
+	return 0;
+
+error_boot:
+	pm_runtime_put_sync(vpu->dev);
+error_pm:
+	atomic_dec(&vpu->core_count);
+
+	return ret;
+}
+
+static void wave6_vpu_put(struct wave6_vpu_device *vpu,
+			  struct vpu_core_device *core)
+{
+	if (WARN_ON(!vpu || !core))
+		return;
+
+	guard(mutex)(&vpu->lock);
+
+	if (!vpu->fw_available)
+		return;
+
+	/* Only the last core executes sleep; others return */
+	if (atomic_dec_return(&vpu->core_count) > 0)
+		return;
+
+	wave6_vpu_sleep(vpu, core);
+
+	if (!pm_runtime_suspended(vpu->dev))
+		pm_runtime_put_sync(vpu->dev);
+}
+
+static void wave6_vpu_require_work_buffer(struct wave6_vpu_device *vpu,
+					  struct vpu_core_device *core)
+{
+	struct wave6_vpu_work_buf *pbuf;
+	u32 size;
+	int ret;
+
+	if (WARN_ON(!vpu || !core))
+		return;
+
+	guard(mutex)(&vpu->lock);
+
+	size = wave6_vdi_readl(core->reg_base, W6_CMD_SET_WORK_BUF_SIZE);
+	if (!size)
+		return;
+
+	pbuf = kzalloc(sizeof(*pbuf), GFP_KERNEL);
+	if (!pbuf)
+		goto exit;
+
+	pbuf->buf.size = size;
+	ret = wave6_vdi_alloc_dma(vpu->dev, &pbuf->buf);
+	if (ret) {
+		dev_warn(vpu->dev, "Failed to allocate work_buf memory\n");
+		kfree(pbuf);
+		goto exit;
+	}
+
+	list_add_tail(&pbuf->list, &vpu->work_buffers);
+	wave6_vdi_writel(core->reg_base, W6_CMD_SET_WORK_BUF_ADDR, pbuf->buf.daddr);
+
+exit:
+	wave6_vdi_writel(core->reg_base, W6_CMD_SET_WORK_BUF_SIZE, SET_WORK_BUF_SIZE_ACK);
+}
+
+static void wave6_vpu_release(struct wave6_vpu_device *vpu)
+{
+	guard(mutex)(&vpu->lock);
+
+	vpu->fw_available = false;
+	wave6_vpu_init_work_buf(vpu, NULL);
+	if (vpu->sram_pool && vpu->sram_buf.vaddr) {
+		dma_unmap_resource(vpu->dev,
+				   vpu->sram_buf.dma_addr,
+				   vpu->sram_buf.size,
+				   DMA_BIDIRECTIONAL,
+				   0);
+		gen_pool_free(vpu->sram_pool,
+			      (unsigned long)vpu->sram_buf.vaddr,
+			      vpu->sram_buf.size);
+	}
+	if (vpu->code_buf.dma_addr)
+		dma_unmap_resource(vpu->dev,
+				   vpu->code_buf.dma_addr,
+				   vpu->code_buf.size,
+				   DMA_BIDIRECTIONAL,
+				   0);
+}
+
+static void wave6_vpu_load_firmware(const struct firmware *fw, void *context)
+{
+	struct wave6_vpu_device *vpu = context;
+
+	guard(mutex)(&vpu->lock);
+
+	if (!fw || !fw->data) {
+		dev_err(vpu->dev, "No firmware.\n");
+		return;
+	}
+
+	if (!vpu->fw_available)
+		goto exit;
+
+	if (fw->size + W6_EXTRA_CODE_BUF_SIZE > wave6_vpu_get_code_buf_size(vpu)) {
+		dev_err(vpu->dev, "firmware size (%ld > %zd) is too big\n",
+			fw->size, vpu->code_buf.size);
+		vpu->fw_available = false;
+		goto exit;
+	}
+
+	memcpy(vpu->code_buf.vaddr, fw->data, fw->size);
+
+	vpu->get_vpu = wave6_vpu_get;
+	vpu->put_vpu = wave6_vpu_put;
+	vpu->req_work_buffer = wave6_vpu_require_work_buffer;
+	of_platform_populate(vpu->dev->of_node, NULL, NULL, vpu->dev);
+
+exit:
+	release_firmware(fw);
+}
+
+static int wave6_vpu_probe(struct platform_device *pdev)
+{
+	struct device_node *np;
+	struct wave6_vpu_device *vpu;
+	const struct wave6_vpu_resource *res;
+	int ret;
+
+	ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
+	if (ret < 0) {
+		dev_err(&pdev->dev, "dma_set_mask_and_coherent failed: %d\n", ret);
+		return ret;
+	}
+
+	res = of_device_get_match_data(&pdev->dev);
+	if (!res)
+		return -ENODEV;
+
+	vpu = devm_kzalloc(&pdev->dev, sizeof(*vpu), GFP_KERNEL);
+	if (!vpu)
+		return -ENOMEM;
+
+	ret = devm_mutex_init(&pdev->dev, &vpu->lock);
+	if (ret)
+		return ret;
+
+	atomic_set(&vpu->core_count, 0);
+	INIT_LIST_HEAD(&vpu->work_buffers);
+	dev_set_drvdata(&pdev->dev, vpu);
+	vpu->dev = &pdev->dev;
+	vpu->res = res;
+	vpu->reg_base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(vpu->reg_base))
+		return PTR_ERR(vpu->reg_base);
+
+	ret = devm_clk_bulk_get_all(&pdev->dev, &vpu->clks);
+	if (ret < 0) {
+		dev_warn(&pdev->dev, "unable to get clocks: %d\n", ret);
+		ret = 0;
+	}
+	vpu->num_clks = ret;
+
+	np = of_parse_phandle(pdev->dev.of_node, "memory-region", 0);
+	if (np) {
+		struct resource mem;
+
+		ret = of_address_to_resource(np, 0, &mem);
+		of_node_put(np);
+		if (!ret) {
+			vpu->code_buf.phys_addr = mem.start;
+			vpu->code_buf.size = resource_size(&mem);
+			wave6_vpu_init_code_buf(vpu);
+		} else {
+			dev_warn(&pdev->dev, "memory-region is not available.\n");
+		}
+	}
+
+	vpu->sram_pool = of_gen_pool_get(pdev->dev.of_node, "sram", 0);
+	if (vpu->sram_pool) {
+		vpu->sram_buf.size = vpu->res->sram_size;
+		vpu->sram_buf.vaddr = gen_pool_dma_alloc(vpu->sram_pool,
+							 vpu->sram_buf.size,
+							 &vpu->sram_buf.phys_addr);
+		if (!vpu->sram_buf.vaddr)
+			vpu->sram_buf.size = 0;
+		else
+			vpu->sram_buf.dma_addr = dma_map_resource(&pdev->dev,
+								  vpu->sram_buf.phys_addr,
+								  vpu->sram_buf.size,
+								  DMA_BIDIRECTIONAL,
+								  0);
+	}
+
+	vpu->thermal.dev = &pdev->dev;
+	ret = wave6_vpu_cooling_init(&vpu->thermal);
+	if (ret)
+		dev_err(&pdev->dev, "failed to initialize thermal cooling, ret = %d\n", ret);
+
+	pm_runtime_enable(&pdev->dev);
+	vpu->fw_available = true;
+
+	ret = firmware_request_nowait_nowarn(THIS_MODULE,
+					     vpu->res->fw_name,
+					     &pdev->dev,
+					     GFP_KERNEL,
+					     vpu,
+					     wave6_vpu_load_firmware);
+	if (ret) {
+		dev_err(&pdev->dev, "request firmware fail, ret = %d\n", ret);
+		goto error;
+	}
+
+	return 0;
+
+error:
+	wave6_vpu_release(vpu);
+	wave6_vpu_cooling_remove(&vpu->thermal);
+	pm_runtime_disable(&pdev->dev);
+
+	return ret;
+}
+
+static void wave6_vpu_remove(struct platform_device *pdev)
+{
+	struct wave6_vpu_device *vpu = dev_get_drvdata(&pdev->dev);
+
+	wave6_vpu_release(vpu);
+	wave6_vpu_cooling_remove(&vpu->thermal);
+	of_platform_depopulate(vpu->dev);
+	pm_runtime_disable(vpu->dev);
+}
+
+static int __maybe_unused wave6_vpu_runtime_suspend(struct device *dev)
+{
+	struct wave6_vpu_device *vpu = dev_get_drvdata(dev);
+
+	clk_bulk_disable_unprepare(vpu->num_clks, vpu->clks);
+
+	return 0;
+}
+
+static int __maybe_unused wave6_vpu_runtime_resume(struct device *dev)
+{
+	struct wave6_vpu_device *vpu = dev_get_drvdata(dev);
+
+	return clk_bulk_prepare_enable(vpu->num_clks, vpu->clks);
+}
+
+static const struct dev_pm_ops wave6_vpu_pm_ops = {
+	SET_RUNTIME_PM_OPS(wave6_vpu_runtime_suspend,
+			   wave6_vpu_runtime_resume, NULL)
+};
+
+static const struct of_device_id wave6_vpu_ids[] = {
+	{ .compatible = "nxp,imx95-vpu", .data = &wave633c_data },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, wave6_vpu_ids);
+
+static struct platform_driver wave6_vpu_driver = {
+	.driver = {
+		.name = WAVE6_VPU_PLATFORM_DRIVER_NAME,
+		.of_match_table = wave6_vpu_ids,
+		.pm = &wave6_vpu_pm_ops,
+	},
+	.probe = wave6_vpu_probe,
+	.remove = wave6_vpu_remove,
+};
+
+module_platform_driver(wave6_vpu_driver);
+MODULE_DESCRIPTION("chips&media Wave6 VPU driver");
+MODULE_LICENSE("Dual BSD/GPL");
diff --git a/drivers/media/platform/chips-media/wave6/wave6-vpu.h b/drivers/media/platform/chips-media/wave6/wave6-vpu.h
new file mode 100644
index 000000000000..01a2e985dd54
--- /dev/null
+++ b/drivers/media/platform/chips-media/wave6/wave6-vpu.h
@@ -0,0 +1,131 @@
+/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) */
+/*
+ * Wave6 series multi-standard codec IP - wave6 driver
+ *
+ * Copyright (C) 2025 CHIPS&MEDIA INC
+ */
+
+#ifndef __WAVE6_VPU_H__
+#define __WAVE6_VPU_H__
+
+#include <linux/device.h>
+#include "wave6-vpu-thermal.h"
+#include "wave6-vdi.h"
+#include "wave6-vpuapi.h"
+
+#define WAVE6_VPU_PLATFORM_DRIVER_NAME "wave6-vpu"
+
+struct wave6_vpu_device;
+struct vpu_core_device;
+
+/**
+ * enum wave6_vpu_state - VPU states
+ * @WAVE6_VPU_STATE_OFF:	VPU is powered off
+ * @WAVE6_VPU_STATE_PREPARE:	VPU is booting
+ * @WAVE6_VPU_STATE_ON:		VPU is running
+ * @WAVE6_VPU_STATE_SLEEP:	VPU is in a sleep mode
+ */
+enum wave6_vpu_state {
+	WAVE6_VPU_STATE_OFF,
+	WAVE6_VPU_STATE_PREPARE,
+	WAVE6_VPU_STATE_ON,
+	WAVE6_VPU_STATE_SLEEP
+};
+
+/**
+ * struct wave6_vpu_dma_buf - VPU buffer from reserved memory or gen_pool
+ * @size:	Buffer size
+ * @dma_addr:	Mapped address for device access
+ * @vaddr:	Kernel virtual address
+ * @phys_addr:	Physical address of the reserved memory region or gen_pool
+ *
+ * Represents a buffer allocated from pre-reserved device memory regions or
+ * SRAM via gen_pool_dma_alloc(). Used for code and SRAM buffers only.
+ * Managed by the VPU device.
+ */
+struct wave6_vpu_dma_buf {
+	size_t size;
+	dma_addr_t dma_addr;
+	void *vaddr;
+	phys_addr_t phys_addr;
+};
+
+/**
+ * struct wave6_vpu_work_buf - VPU buffer for a coherent DMA work buffer
+ * @list:	Linked list node
+ * @buf:	VPU buffer for a coherent DMA buffer
+ *
+ * Represents a single work buffer needed by a VPU instance.
+ * Managed by the VPU device in a linked list, allocated upon request
+ * when a VPU core device creates an instance.
+ */
+struct wave6_vpu_work_buf {
+	struct list_head list;
+	struct vpu_buf buf;
+};
+
+/**
+ * struct wave6_vpu_resource - VPU device compatible data
+ * @fw_name:	Firmware name for the device
+ * @sram_size:	Required SRAM size
+ */
+struct wave6_vpu_resource {
+	const char *fw_name;
+	u32 sram_size;
+};
+
+/**
+ * struct wave6_vpu_device - VPU driver structure
+ * @get_vpu:		Function pointer, boot or wake the device
+ * @put_vpu:		Function pointer, power off or suspend the device
+ * @req_work_buffer:	Function pointer, request allocation of a work buffer
+ * @dev:		Platform device pointer
+ * @reg_base:		Base address of MMIO registers
+ * @clks:		Array of clock handles
+ * @num_clks:		Number of entries in @clks
+ * @state:		Device state
+ * @lock:		Mutex protecting device data, register access
+ * @fw_available:	Firmware availability flag
+ * @res:		Device compatible data
+ * @sram_pool:		Genalloc pool for SRAM allocations
+ * @sram_buf:		Optional SRAM buffer
+ * @code_buf:		Firmware code buffer
+ * @work_buffers:	Linked list of work buffers
+ * @thermal:		Thermal cooling device
+ * @core_count:		Number of available VPU core devices
+ *
+ * @get_vpu, @put_vpu, @req_work_buffer are called by VPU core devices.
+ *
+ * Buffers such as @sram_buf, @code_buf, and @work_buffers are managed
+ * by the VPU device and accessed exclusively by the firmware.
+ */
+struct wave6_vpu_device {
+	int (*get_vpu)(struct wave6_vpu_device *vpu,
+		       struct vpu_core_device *core);
+	void (*put_vpu)(struct wave6_vpu_device *vpu,
+			struct vpu_core_device *core);
+	void (*req_work_buffer)(struct wave6_vpu_device *vpu,
+				struct vpu_core_device *core);
+	struct device *dev;
+	void __iomem *reg_base;
+	struct clk_bulk_data *clks;
+	int num_clks;
+	enum wave6_vpu_state state;
+	struct mutex lock; /* Protects device data, register access */
+
+	/* Prevents boot or sleep sequence if firmware is unavailable. */
+	bool fw_available;
+
+	const struct wave6_vpu_resource *res;
+	struct gen_pool *sram_pool;
+	struct wave6_vpu_dma_buf sram_buf;
+	struct wave6_vpu_dma_buf code_buf;
+
+	/* Allocates per-instance, used for storing instance-specific data. */
+	struct list_head work_buffers;
+
+	struct vpu_thermal_cooling thermal;
+	atomic_t core_count;
+};
+
+#endif /* __WAVE6_VPU_H__ */
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v3 9/9] arm64: dts: freescale: imx95: Add video codec node
  2025-08-29  8:46 [PATCH v3 0/9] Add support for Wave6 video codec driver Nas Chung
                   ` (5 preceding siblings ...)
  2025-08-29  8:46 ` [PATCH v3 8/9] media: chips-media: wave6: Add Wave6 control driver Nas Chung
@ 2025-08-29  8:46 ` Nas Chung
  2025-08-29 14:07   ` Krzysztof Kozlowski
  6 siblings, 1 reply; 19+ messages in thread
From: Nas Chung @ 2025-08-29  8:46 UTC (permalink / raw)
  To: mchehab, hverkuil, robh, krzk+dt, conor+dt, shawnguo, s.hauer
  Cc: linux-media, devicetree, linux-kernel, linux-imx,
	linux-arm-kernel, jackson.lee, lafley.kim, Nas Chung

Add the Chips and Media wave633 video codec node on IMX95 SoCs.

Signed-off-by: Nas Chung <nas.chung@chipsnmedia.com>
---
 .../boot/dts/freescale/imx95-15x15-evk.dts    |  5 +++
 .../boot/dts/freescale/imx95-19x19-evk.dts    | 10 +++++
 .../dts/freescale/imx95-phycore-fpsc.dtsi     | 10 +++++
 .../boot/dts/freescale/imx95-tqma9596sa.dtsi  |  5 +++
 arch/arm64/boot/dts/freescale/imx95.dtsi      | 43 +++++++++++++++++++
 5 files changed, 73 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx95-15x15-evk.dts b/arch/arm64/boot/dts/freescale/imx95-15x15-evk.dts
index 46f6e0fbf2b0..07e070ae92b7 100644
--- a/arch/arm64/boot/dts/freescale/imx95-15x15-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx95-15x15-evk.dts
@@ -1137,6 +1137,11 @@ &wdog3 {
 	status = "okay";
 };
 
+&vpu {
+	memory-region = <&vpu_boot>;
+	sram = <&sram1>;
+};
+
 &xcvr {
 	clocks = <&scmi_clk IMX95_CLK_BUSWAKEUP>,
 		 <&scmi_clk IMX95_CLK_SPDIF>,
diff --git a/arch/arm64/boot/dts/freescale/imx95-19x19-evk.dts b/arch/arm64/boot/dts/freescale/imx95-19x19-evk.dts
index 2f949a0d48d2..c9d8b78d5768 100644
--- a/arch/arm64/boot/dts/freescale/imx95-19x19-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx95-19x19-evk.dts
@@ -75,6 +75,11 @@ linux_cma: linux,cma {
 			linux,cma-default;
 			reusable;
 		};
+
+		vpu_boot: vpu_boot@a0000000 {
+			reg = <0 0xa0000000 0 0x100000>;
+			no-map;
+		};
 	};
 
 	flexcan1_phy: can-phy0 {
@@ -1044,3 +1049,8 @@ &tpm6 {
 	pinctrl-0 = <&pinctrl_tpm6>;
 	status = "okay";
 };
+
+&vpu {
+	memory-region = <&vpu_boot>;
+	sram = <&sram1>;
+};
diff --git a/arch/arm64/boot/dts/freescale/imx95-phycore-fpsc.dtsi b/arch/arm64/boot/dts/freescale/imx95-phycore-fpsc.dtsi
index 7519d5bd06ba..73c84ab60dfd 100644
--- a/arch/arm64/boot/dts/freescale/imx95-phycore-fpsc.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx95-phycore-fpsc.dtsi
@@ -59,6 +59,11 @@ linux,cma {
 			size = <0 0x3c000000>;
 			linux,cma-default;
 		};
+
+		vpu_boot: vpu_boot@a0000000 {
+			reg = <0 0xa0000000 0 0x100000>;
+			no-map;
+		};
 	};
 };
 
@@ -654,3 +659,8 @@ &usdhc3 { /* FPSC SDIO */
 	pinctrl-0 = <&pinctrl_usdhc3>;
 	pinctrl-names = "default";
 };
+
+&vpu {
+	memory-region = <&vpu_boot>;
+	sram = <&sram1>;
+};
diff --git a/arch/arm64/boot/dts/freescale/imx95-tqma9596sa.dtsi b/arch/arm64/boot/dts/freescale/imx95-tqma9596sa.dtsi
index 180124cc5bce..f7c7e676a077 100644
--- a/arch/arm64/boot/dts/freescale/imx95-tqma9596sa.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx95-tqma9596sa.dtsi
@@ -696,3 +696,8 @@ pinctrl_usdhc2_200mhz: usdhc2-200mhzgrp {
 			   <IMX95_PAD_SD2_VSELECT__USDHC2_VSELECT		0x51e>;
 	};
 };
+
+&vpu {
+	memory-region = <&vpu_boot>;
+	sram = <&sram1>;
+};
diff --git a/arch/arm64/boot/dts/freescale/imx95.dtsi b/arch/arm64/boot/dts/freescale/imx95.dtsi
index 4ca6a7ea586e..4c14bf9437b8 100644
--- a/arch/arm64/boot/dts/freescale/imx95.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx95.dtsi
@@ -1820,6 +1820,49 @@ vpu_blk_ctrl: clock-controller@4c410000 {
 			assigned-clock-rates = <133333333>, <667000000>, <500000000>;
 		};
 
+		vpu: video-codec@4c4c0000 {
+			compatible = "nxp,imx95-vpu";
+			reg = <0x0 0x4c4c0000 0x0 0x10000>;
+			clocks = <&vpu_blk_ctrl IMX95_CLK_VPUBLK_WAVE>;
+			power-domains = <&scmi_perf IMX95_PERF_VPU>;
+			#cooling-cells = <2>;
+			#address-cells = <2>;
+			#size-cells = <2>;
+			ranges;
+
+			vpucore0: video-core@4c480000 {
+				compatible = "nxp,imx95-vpu-core";
+				reg = <0x0 0x4c480000 0x0 0x10000>;
+				clocks = <&scmi_clk IMX95_CLK_VPU>;
+				power-domains = <&scmi_devpd IMX95_PD_VPU>;
+				interrupts = <GIC_SPI 299 IRQ_TYPE_LEVEL_HIGH>;
+			};
+
+			vpucore1: video-core@4c490000 {
+				compatible = "nxp,imx95-vpu-core";
+				reg = <0x0 0x4c490000 0x0 0x10000>;
+				clocks = <&scmi_clk IMX95_CLK_VPU>;
+				power-domains = <&scmi_devpd IMX95_PD_VPU>;
+				interrupts = <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>;
+			};
+
+			vpucore2: video-core@4c4a0000 {
+				compatible = "nxp,imx95-vpu-core";
+				reg = <0x0 0x4c4a0000 0x0 0x10000>;
+				clocks = <&scmi_clk IMX95_CLK_VPU>;
+				power-domains = <&scmi_devpd IMX95_PD_VPU>;
+				interrupts = <GIC_SPI 301 IRQ_TYPE_LEVEL_HIGH>;
+			};
+
+			vpucore3: video-core@4c4b0000 {
+				compatible = "nxp,imx95-vpu-core";
+				reg = <0x0 0x4c4b0000 0x0 0x10000>;
+				clocks = <&scmi_clk IMX95_CLK_VPU>;
+				power-domains = <&scmi_devpd IMX95_PD_VPU>;
+				interrupts = <GIC_SPI 302 IRQ_TYPE_LEVEL_HIGH>;
+			};
+		};
+
 		jpegdec: jpegdec@4c500000 {
 			compatible = "nxp,imx95-jpgdec", "nxp,imx8qxp-jpgdec";
 			reg = <0x0 0x4C500000 0x0 0x00050000>;
-- 
2.31.1



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 2/9] dt-bindings: media: nxp: Add Wave6 video codec device
  2025-08-29  8:46 ` [PATCH v3 2/9] dt-bindings: media: nxp: Add Wave6 video codec device Nas Chung
@ 2025-08-29 13:57   ` Krzysztof Kozlowski
  2025-09-02  5:45     ` Nas Chung
  0 siblings, 1 reply; 19+ messages in thread
From: Krzysztof Kozlowski @ 2025-08-29 13:57 UTC (permalink / raw)
  To: Nas Chung, mchehab, hverkuil, robh, krzk+dt, conor+dt, shawnguo,
	s.hauer
  Cc: linux-media, devicetree, linux-kernel, linux-imx,
	linux-arm-kernel, jackson.lee, lafley.kim

On 29/08/2025 10:46, Nas Chung wrote:
> Add documents for the Wave6 video codec on NXP i.MX SoCs.
Pretty incomplete commit msg. Nothing explaining hardware, nothing
documenting resolution of previous discussions (where is all this
chip&media?).

...


> +
> +properties:
> +  compatible:
> +    enum:
> +      - nxp,imx95-vpu
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  memory-region:
> +    maxItems: 1
> +
> +  sram:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: phandle of the SRAM memory region node.
> +
> +  "#cooling-cells":
> +    const: 2
> +
> +  "#address-cells":
> +    const: 2
> +
> +  "#size-cells":
> +    const: 2
> +
> +  ranges: true
> +
> +patternProperties:
> +  "^video-core@[0-9a-f]+$":
> +    type: object

Missing description.

> +    additionalProperties: false
> +
> +    properties:
> +      compatible:
> +        enum:
> +          - nxp,imx95-vpu-core

Why do you need here compatible? Can this child be anything else? Can it
be re-used? Is it actually a separate block?

Your example suggests that the only distinctive resource are the
interrupt and address space and that's on the edge of calling it a
separate device.

There is some tendency to call such "pseudo-cores" a separate devices in
case of video codec bindings and experience shows these are usually
fake. It's not the same as DP or HDMI sub-block of display pipeline.

That's why you should come here with strong argument what separate piece
of hardware this is.

> +
> +      reg:
> +        maxItems: 1
> +
> +      clocks:
> +        maxItems: 1
> +
> +      power-domains:
> +        maxItems: 1
> +
> +      interrupts:
> +        maxItems: 1
> +
> +    required:
> +      - compatible
> +      - reg
> +      - clocks
> +      - power-domains
> +      - interrupts
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - power-domains
> +  - memory-region
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/nxp,imx95-clock.h>
> +
> +    soc {
> +      #address-cells = <2>;
> +      #size-cells = <2>;
> +
> +      vpu: video-codec@4c4c0000 {

Unused label, drop

> +        compatible = "nxp,imx95-vpu";
> +        reg = <0x0 0x4c4c0000 0x0 0x10000>;
> +        clocks = <&vpu_blk_ctrl IMX95_CLK_VPUBLK_WAVE>;
> +        power-domains = <&scmi_perf 10>;
> +        memory-region = <&vpu_boot>;
> +        sram = <&sram1>;
> +        #cooling-cells = <2>;
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +        ranges;
> +
> +        vpucore0: video-core@4c480000 {

None of these labels are used, drop.

> +          compatible = "nxp,imx95-vpu-core";
> +          reg = <0x0 0x4c480000 0x0 0x10000>;
> +          clocks = <&scmi_clk 115>;
> +          power-domains = <&scmi_devpd 21>;
> +          interrupts = <GIC_SPI 299 IRQ_TYPE_LEVEL_HIGH>;
> +        };
> +
> +        vpucore1: video-core@4c490000 {
> +          compatible = "nxp,imx95-vpu-core";
> +          reg = <0x0 0x4c490000 0x0 0x10000>;
> +          clocks = <&scmi_clk 115>;
> +          power-domains = <&scmi_devpd 21>;
> +          interrupts = <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>;
> +        };
> +



Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 5/9] media: chips-media: wave6: Add Wave6 core driver
  2025-08-29  8:46 ` [PATCH v3 5/9] media: chips-media: wave6: Add Wave6 core driver Nas Chung
@ 2025-08-29 14:02   ` Krzysztof Kozlowski
  2025-09-01  8:34     ` Nas Chung
  0 siblings, 1 reply; 19+ messages in thread
From: Krzysztof Kozlowski @ 2025-08-29 14:02 UTC (permalink / raw)
  To: Nas Chung, mchehab, hverkuil, robh, krzk+dt, conor+dt, shawnguo,
	s.hauer
  Cc: linux-media, devicetree, linux-kernel, linux-imx,
	linux-arm-kernel, jackson.lee, lafley.kim, Ming Qian

On 29/08/2025 10:46, Nas Chung wrote:
> This adds the core driver for the Chips&Media Wave6 video codec IP.

Please do not use "This commit/patch/change", but imperative mood. See
longer explanation here:
https://elixir.bootlin.com/linux/v6.16/source/Documentation/process/submitting-patches.rst#L94

> 
> The core region provides the encoding and decoding capabilities of
> the VPU and depends on the control region for firmware and shared
> resource management.
> 


...

> +
> +static int wave6_vpu_core_probe(struct platform_device *pdev)
> +{
> +	struct vpu_core_device *core;
> +	const struct wave6_vpu_core_resource *res;
> +	int ret;
> +	int irq;
> +
> +	res = device_get_match_data(&pdev->dev);
> +	if (!res) {
> +		dev_err(&pdev->dev, "missing resource\n");

This is almost impossible condition. Not worth printing errors.

> +		return -EINVAL;
> +	}
> +
> +	ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "dma_set_mask_and_coherent failed: %d\n", ret);
> +		return ret;
> +	}
> +
> +	core = devm_kzalloc(&pdev->dev, sizeof(*core), GFP_KERNEL);
> +	if (!core)
> +		return -ENOMEM;
> +
> +	ret = devm_mutex_init(&pdev->dev, &core->dev_lock);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_mutex_init(&pdev->dev, &core->hw_lock);
> +	if (ret)
> +		return ret;
> +
> +	init_completion(&core->irq_done);
> +	dev_set_drvdata(&pdev->dev, core);
> +	core->dev = &pdev->dev;
> +	core->res = res;
> +
> +	if (pdev->dev.parent->driver && pdev->dev.parent->driver->name &&
> +	    !strcmp(pdev->dev.parent->driver->name, WAVE6_VPU_PLATFORM_DRIVER_NAME))
> +		core->vpu = dev_get_drvdata(pdev->dev.parent);
> +
> +	core->reg_base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(core->reg_base))
> +		return PTR_ERR(core->reg_base);
> +
> +	ret = devm_clk_bulk_get_all(&pdev->dev, &core->clks);
> +	if (ret < 0) {
> +		dev_warn(&pdev->dev, "unable to get clocks: %d\n", ret);

You should handle deferred probe instead.

> +		ret = 0;
> +	}
> +	core->num_clks = ret;
> +
> +	ret = v4l2_device_register(&pdev->dev, &core->v4l2_dev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "v4l2_device_register fail: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = wave6_vpu_init_m2m_dev(core);
> +	if (ret)
> +		goto err_v4l2_unregister;
> +
> +	irq = platform_get_irq(pdev, 0);
> +	if (irq < 0) {
> +		dev_err(&pdev->dev, "failed to get irq resource\n");

Syntax is: dev_err_probe

> +		ret = -ENXIO;

Don't override error codes.

> +		goto err_m2m_dev_release;
> +	}
> +
> +	ret = kfifo_alloc(&core->irq_status, 16 * sizeof(int), GFP_KERNEL);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to allocate fifo\n");
> +		goto err_m2m_dev_release;
> +	}
> +
> +	ret = devm_request_threaded_irq(&pdev->dev, irq,
> +					wave6_vpu_core_irq,
> +					wave6_vpu_core_irq_thread,
> +					0, "vpu_irq", core);
> +	if (ret) {
> +		dev_err(&pdev->dev, "fail to register interrupt handler: %d\n", ret);
> +		goto err_kfifo_free;
> +	}
> +
> +	core->temp_vbuf.size = ALIGN(W6_TEMPBUF_SIZE, 4096);
> +	ret = wave6_vdi_alloc_dma(core->dev, &core->temp_vbuf);
> +	if (ret) {
> +		dev_err(&pdev->dev, "alloc temp of size %zu failed\n",
> +			core->temp_vbuf.size);
> +		goto err_kfifo_free;
> +	}
> +
> +	core->debugfs = debugfs_lookup(WAVE6_VPU_DEBUGFS_DIR, NULL);
> +	if (IS_ERR_OR_NULL(core->debugfs))
> +		core->debugfs = debugfs_create_dir(WAVE6_VPU_DEBUGFS_DIR, NULL);
> +
> +	pm_runtime_enable(&pdev->dev);
> +
> +	if (core->res->codec_types & WAVE6_IS_DEC) {
> +		ret = wave6_vpu_dec_register_device(core);
> +		if (ret) {
> +			dev_err(&pdev->dev, "wave6_vpu_dec_register_device fail: %d\n", ret);
> +			goto err_temp_vbuf_free;
> +		}
> +	}
> +	if (core->res->codec_types & WAVE6_IS_ENC) {
> +		ret = wave6_vpu_enc_register_device(core);
> +		if (ret) {
> +			dev_err(&pdev->dev, "wave6_vpu_enc_register_device fail: %d\n", ret);
> +			goto err_dec_unreg;
> +		}
> +	}
Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 8/9] media: chips-media: wave6: Add Wave6 control driver
  2025-08-29  8:46 ` [PATCH v3 8/9] media: chips-media: wave6: Add Wave6 control driver Nas Chung
@ 2025-08-29 14:06   ` Krzysztof Kozlowski
  2025-09-01  8:13     ` Nas Chung
  0 siblings, 1 reply; 19+ messages in thread
From: Krzysztof Kozlowski @ 2025-08-29 14:06 UTC (permalink / raw)
  To: Nas Chung, mchehab, hverkuil, robh, krzk+dt, conor+dt, shawnguo,
	s.hauer
  Cc: linux-media, devicetree, linux-kernel, linux-imx,
	linux-arm-kernel, jackson.lee, lafley.kim, Ming Qian

On 29/08/2025 10:46, Nas Chung wrote:
> +
> +static void wave6_vpu_load_firmware(const struct firmware *fw, void *context)
> +{
> +	struct wave6_vpu_device *vpu = context;
> +
> +	guard(mutex)(&vpu->lock);

Why? How could this be called in parallel, before the probe?

> +
> +	if (!fw || !fw->data) {
> +		dev_err(vpu->dev, "No firmware.\n");
> +		return;
> +	}
> +
> +	if (!vpu->fw_available)
> +		goto exit;
> +
> +	if (fw->size + W6_EXTRA_CODE_BUF_SIZE > wave6_vpu_get_code_buf_size(vpu)) {
> +		dev_err(vpu->dev, "firmware size (%ld > %zd) is too big\n",
> +			fw->size, vpu->code_buf.size);
> +		vpu->fw_available = false;
> +		goto exit;
> +	}
> +
> +	memcpy(vpu->code_buf.vaddr, fw->data, fw->size);
> +
> +	vpu->get_vpu = wave6_vpu_get;
> +	vpu->put_vpu = wave6_vpu_put;
> +	vpu->req_work_buffer = wave6_vpu_require_work_buffer;
> +	of_platform_populate(vpu->dev->of_node, NULL, NULL, vpu->dev);
> +
> +exit:
> +	release_firmware(fw);
> +}
> +
> +static int wave6_vpu_probe(struct platform_device *pdev)
> +{
> +	struct device_node *np;
> +	struct wave6_vpu_device *vpu;
> +	const struct wave6_vpu_resource *res;
> +	int ret;
> +
> +	ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "dma_set_mask_and_coherent failed: %d\n", ret);
> +		return ret;
> +	}
> +
> +	res = of_device_get_match_data(&pdev->dev);
> +	if (!res)
> +		return -ENODEV;
> +
> +	vpu = devm_kzalloc(&pdev->dev, sizeof(*vpu), GFP_KERNEL);
> +	if (!vpu)
> +		return -ENOMEM;
> +
> +	ret = devm_mutex_init(&pdev->dev, &vpu->lock);
> +	if (ret)
> +		return ret;
> +
> +	atomic_set(&vpu->core_count, 0);
> +	INIT_LIST_HEAD(&vpu->work_buffers);
> +	dev_set_drvdata(&pdev->dev, vpu);
> +	vpu->dev = &pdev->dev;
> +	vpu->res = res;
> +	vpu->reg_base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(vpu->reg_base))
> +		return PTR_ERR(vpu->reg_base);
> +
> +	ret = devm_clk_bulk_get_all(&pdev->dev, &vpu->clks);
> +	if (ret < 0) {
> +		dev_warn(&pdev->dev, "unable to get clocks: %d\n", ret);

You need to handle deferred probe.

> +		ret = 0;
> +	}
> +	vpu->num_clks = ret;
Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 9/9] arm64: dts: freescale: imx95: Add video codec node
  2025-08-29  8:46 ` [PATCH v3 9/9] arm64: dts: freescale: imx95: Add video codec node Nas Chung
@ 2025-08-29 14:07   ` Krzysztof Kozlowski
  2025-09-01  7:38     ` Nas Chung
  0 siblings, 1 reply; 19+ messages in thread
From: Krzysztof Kozlowski @ 2025-08-29 14:07 UTC (permalink / raw)
  To: Nas Chung, mchehab, hverkuil, robh, krzk+dt, conor+dt, shawnguo,
	s.hauer
  Cc: linux-media, devicetree, linux-kernel, linux-imx,
	linux-arm-kernel, jackson.lee, lafley.kim

On 29/08/2025 10:46, Nas Chung wrote:
> diff --git a/arch/arm64/boot/dts/freescale/imx95-19x19-evk.dts b/arch/arm64/boot/dts/freescale/imx95-19x19-evk.dts
> index 2f949a0d48d2..c9d8b78d5768 100644
> --- a/arch/arm64/boot/dts/freescale/imx95-19x19-evk.dts
> +++ b/arch/arm64/boot/dts/freescale/imx95-19x19-evk.dts
> @@ -75,6 +75,11 @@ linux_cma: linux,cma {
>  			linux,cma-default;
>  			reusable;
>  		};
> +
> +		vpu_boot: vpu_boot@a0000000 {

Follow DTS coding style.

> +			reg = <0 0xa0000000 0 0x100000>;
> +			no-map;
> +		};
>  	};
>  
>  	flexcan1_phy: can-phy0 {
> @@ -1044,3 +1049,8 @@ &tpm6 {
>  	pinctrl-0 = <&pinctrl_tpm6>;
>  	status = "okay";
>  };
> +
> +&vpu {
> +	memory-region = <&vpu_boot>;
> +	sram = <&sram1>;
> +};
> diff --git a/arch/arm64/boot/dts/freescale/imx95-phycore-fpsc.dtsi b/arch/arm64/boot/dts/freescale/imx95-phycore-fpsc.dtsi
> index 7519d5bd06ba..73c84ab60dfd 100644
> --- a/arch/arm64/boot/dts/freescale/imx95-phycore-fpsc.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx95-phycore-fpsc.dtsi
> @@ -59,6 +59,11 @@ linux,cma {
>  			size = <0 0x3c000000>;
>  			linux,cma-default;
>  		};
> +
> +		vpu_boot: vpu_boot@a0000000 {

Same problem.

> +			reg = <0 0xa0000000 0 0x100000>;
> +			no-map;
> +		};
>  	};

> +
Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [PATCH v3 9/9] arm64: dts: freescale: imx95: Add video codec node
  2025-08-29 14:07   ` Krzysztof Kozlowski
@ 2025-09-01  7:38     ` Nas Chung
  0 siblings, 0 replies; 19+ messages in thread
From: Nas Chung @ 2025-09-01  7:38 UTC (permalink / raw)
  To: Krzysztof Kozlowski, mchehab@kernel.org, hverkuil@xs4all.nl,
	robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
	shawnguo@kernel.org, s.hauer@pengutronix.de
  Cc: linux-media@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-imx@nxp.com,
	linux-arm-kernel@lists.infradead.org, jackson.lee, lafley.kim

Hi, Krzysztof.

>-----Original Message-----
>From: Krzysztof Kozlowski <krzk@kernel.org>
>Sent: Friday, August 29, 2025 11:08 PM
>To: Nas Chung <nas.chung@chipsnmedia.com>; mchehab@kernel.org;
>hverkuil@xs4all.nl; robh@kernel.org; krzk+dt@kernel.org;
>conor+dt@kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de
>Cc: linux-media@vger.kernel.org; devicetree@vger.kernel.org; linux-
>kernel@vger.kernel.org; linux-imx@nxp.com; linux-arm-
>kernel@lists.infradead.org; jackson.lee <jackson.lee@chipsnmedia.com>;
>lafley.kim <lafley.kim@chipsnmedia.com>
>Subject: Re: [PATCH v3 9/9] arm64: dts: freescale: imx95: Add video codec
>node
>
>On 29/08/2025 10:46, Nas Chung wrote:
>> diff --git a/arch/arm64/boot/dts/freescale/imx95-19x19-evk.dts
>b/arch/arm64/boot/dts/freescale/imx95-19x19-evk.dts
>> index 2f949a0d48d2..c9d8b78d5768 100644
>> --- a/arch/arm64/boot/dts/freescale/imx95-19x19-evk.dts
>> +++ b/arch/arm64/boot/dts/freescale/imx95-19x19-evk.dts
>> @@ -75,6 +75,11 @@ linux_cma: linux,cma {
>>  			linux,cma-default;
>>  			reusable;
>>  		};
>> +
>> +		vpu_boot: vpu_boot@a0000000 {
>
>Follow DTS coding style.

Okay, I will change the node name to "memory", e.g.:

		vpu_boot: memory@a0000000 {

>
>> +			reg = <0 0xa0000000 0 0x100000>;
>> +			no-map;
>> +		};
>>  	};
>>
>>  	flexcan1_phy: can-phy0 {
>> @@ -1044,3 +1049,8 @@ &tpm6 {
>>  	pinctrl-0 = <&pinctrl_tpm6>;
>>  	status = "okay";
>>  };
>> +
>> +&vpu {
>> +	memory-region = <&vpu_boot>;
>> +	sram = <&sram1>;
>> +};
>> diff --git a/arch/arm64/boot/dts/freescale/imx95-phycore-fpsc.dtsi
>b/arch/arm64/boot/dts/freescale/imx95-phycore-fpsc.dtsi
>> index 7519d5bd06ba..73c84ab60dfd 100644
>> --- a/arch/arm64/boot/dts/freescale/imx95-phycore-fpsc.dtsi
>> +++ b/arch/arm64/boot/dts/freescale/imx95-phycore-fpsc.dtsi
>> @@ -59,6 +59,11 @@ linux,cma {
>>  			size = <0 0x3c000000>;
>>  			linux,cma-default;
>>  		};
>> +
>> +		vpu_boot: vpu_boot@a0000000 {
>
>Same problem.

I will fix it in the same way.

Thanks,
Nas.

>
>> +			reg = <0 0xa0000000 0 0x100000>;
>> +			no-map;
>> +		};
>>  	};
>
>> +
>Best regards,
>Krzysztof


^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [PATCH v3 8/9] media: chips-media: wave6: Add Wave6 control driver
  2025-08-29 14:06   ` Krzysztof Kozlowski
@ 2025-09-01  8:13     ` Nas Chung
  2025-09-01 10:44       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 19+ messages in thread
From: Nas Chung @ 2025-09-01  8:13 UTC (permalink / raw)
  To: Krzysztof Kozlowski, mchehab@kernel.org, hverkuil@xs4all.nl,
	robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
	shawnguo@kernel.org, s.hauer@pengutronix.de
  Cc: linux-media@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-imx@nxp.com,
	linux-arm-kernel@lists.infradead.org, jackson.lee, lafley.kim,
	Ming Qian

Hi, Krzysztof.

Thanks for the feedback.

>-----Original Message-----
>From: Krzysztof Kozlowski <krzk@kernel.org>
>Sent: Friday, August 29, 2025 11:06 PM
>To: Nas Chung <nas.chung@chipsnmedia.com>; mchehab@kernel.org;
>hverkuil@xs4all.nl; robh@kernel.org; krzk+dt@kernel.org;
>conor+dt@kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de
>Cc: linux-media@vger.kernel.org; devicetree@vger.kernel.org; linux-
>kernel@vger.kernel.org; linux-imx@nxp.com; linux-arm-
>kernel@lists.infradead.org; jackson.lee <jackson.lee@chipsnmedia.com>;
>lafley.kim <lafley.kim@chipsnmedia.com>; Ming Qian <ming.qian@oss.nxp.com>
>Subject: Re: [PATCH v3 8/9] media: chips-media: wave6: Add Wave6 control
>driver
>
>On 29/08/2025 10:46, Nas Chung wrote:
>> +
>> +static void wave6_vpu_load_firmware(const struct firmware *fw, void
>*context)
>> +{
>> +	struct wave6_vpu_device *vpu = context;
>> +
>> +	guard(mutex)(&vpu->lock);
>
>Why? How could this be called in parallel, before the probe?

This callback is called asynchronously via firmware_request_nowait_nowarn().
In practice, I observed a case where the callback was executing
while the device's release() function was being called.

>
>> +
>> +	if (!fw || !fw->data) {
>> +		dev_err(vpu->dev, "No firmware.\n");
>> +		return;
>> +	}
>> +
>> +	if (!vpu->fw_available)
>> +		goto exit;
>> +
>> +	if (fw->size + W6_EXTRA_CODE_BUF_SIZE >
>wave6_vpu_get_code_buf_size(vpu)) {
>> +		dev_err(vpu->dev, "firmware size (%ld > %zd) is too big\n",
>> +			fw->size, vpu->code_buf.size);
>> +		vpu->fw_available = false;
>> +		goto exit;
>> +	}
>> +
>> +	memcpy(vpu->code_buf.vaddr, fw->data, fw->size);
>> +
>> +	vpu->get_vpu = wave6_vpu_get;
>> +	vpu->put_vpu = wave6_vpu_put;
>> +	vpu->req_work_buffer = wave6_vpu_require_work_buffer;
>> +	of_platform_populate(vpu->dev->of_node, NULL, NULL, vpu->dev);
>> +
>> +exit:
>> +	release_firmware(fw);
>> +}
>> +
>> +static int wave6_vpu_probe(struct platform_device *pdev)
>> +{
>> +	struct device_node *np;
>> +	struct wave6_vpu_device *vpu;
>> +	const struct wave6_vpu_resource *res;
>> +	int ret;
>> +
>> +	ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
>> +	if (ret < 0) {
>> +		dev_err(&pdev->dev, "dma_set_mask_and_coherent failed: %d\n",
>ret);
>> +		return ret;
>> +	}
>> +
>> +	res = of_device_get_match_data(&pdev->dev);
>> +	if (!res)
>> +		return -ENODEV;
>> +
>> +	vpu = devm_kzalloc(&pdev->dev, sizeof(*vpu), GFP_KERNEL);
>> +	if (!vpu)
>> +		return -ENOMEM;
>> +
>> +	ret = devm_mutex_init(&pdev->dev, &vpu->lock);
>> +	if (ret)
>> +		return ret;
>> +
>> +	atomic_set(&vpu->core_count, 0);
>> +	INIT_LIST_HEAD(&vpu->work_buffers);
>> +	dev_set_drvdata(&pdev->dev, vpu);
>> +	vpu->dev = &pdev->dev;
>> +	vpu->res = res;
>> +	vpu->reg_base = devm_platform_ioremap_resource(pdev, 0);
>> +	if (IS_ERR(vpu->reg_base))
>> +		return PTR_ERR(vpu->reg_base);
>> +
>> +	ret = devm_clk_bulk_get_all(&pdev->dev, &vpu->clks);
>> +	if (ret < 0) {
>> +		dev_warn(&pdev->dev, "unable to get clocks: %d\n", ret);
>
>You need to handle deferred probe.

Got it, I will address this in v4 using dev_err_probe().

Thanks,
Nas.

>
>> +		ret = 0;
>> +	}
>> +	vpu->num_clks = ret;
>Best regards,
>Krzysztof


^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [PATCH v3 5/9] media: chips-media: wave6: Add Wave6 core driver
  2025-08-29 14:02   ` Krzysztof Kozlowski
@ 2025-09-01  8:34     ` Nas Chung
  0 siblings, 0 replies; 19+ messages in thread
From: Nas Chung @ 2025-09-01  8:34 UTC (permalink / raw)
  To: Krzysztof Kozlowski, mchehab@kernel.org, hverkuil@xs4all.nl,
	robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
	shawnguo@kernel.org, s.hauer@pengutronix.de
  Cc: linux-media@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-imx@nxp.com,
	linux-arm-kernel@lists.infradead.org, jackson.lee, lafley.kim,
	Ming Qian

Hi, Krzysztof.

>-----Original Message-----
>From: Krzysztof Kozlowski <krzk@kernel.org>
>Sent: Friday, August 29, 2025 11:03 PM
>To: Nas Chung <nas.chung@chipsnmedia.com>; mchehab@kernel.org;
>hverkuil@xs4all.nl; robh@kernel.org; krzk+dt@kernel.org;
>conor+dt@kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de
>Cc: linux-media@vger.kernel.org; devicetree@vger.kernel.org; linux-
>kernel@vger.kernel.org; linux-imx@nxp.com; linux-arm-
>kernel@lists.infradead.org; jackson.lee <jackson.lee@chipsnmedia.com>;
>lafley.kim <lafley.kim@chipsnmedia.com>; Ming Qian <ming.qian@oss.nxp.com>
>Subject: Re: [PATCH v3 5/9] media: chips-media: wave6: Add Wave6 core
>driver
>
>On 29/08/2025 10:46, Nas Chung wrote:
>> This adds the core driver for the Chips&Media Wave6 video codec IP.
>
>Please do not use "This commit/patch/change", but imperative mood. See
>longer explanation here:
>https://elixir.bootlin.com/linux/v6.16/source/Documentation/process/submitt
>ing-patches.rst#L94

Understood, I'll fix the commit message in v4.
Thanks for sharing it.

>
>>
>> The core region provides the encoding and decoding capabilities of
>> the VPU and depends on the control region for firmware and shared
>> resource management.
>>
>
>
>...
>
>> +
>> +static int wave6_vpu_core_probe(struct platform_device *pdev)
>> +{
>> +	struct vpu_core_device *core;
>> +	const struct wave6_vpu_core_resource *res;
>> +	int ret;
>> +	int irq;
>> +
>> +	res = device_get_match_data(&pdev->dev);
>> +	if (!res) {
>> +		dev_err(&pdev->dev, "missing resource\n");
>
>This is almost impossible condition. Not worth printing errors.

I'll remove it in v4.

>
>> +		return -EINVAL;
>> +	}
>> +
>> +	ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
>> +	if (ret < 0) {
>> +		dev_err(&pdev->dev, "dma_set_mask_and_coherent failed: %d\n",
>ret);
>> +		return ret;
>> +	}
>> +
>> +	core = devm_kzalloc(&pdev->dev, sizeof(*core), GFP_KERNEL);
>> +	if (!core)
>> +		return -ENOMEM;
>> +
>> +	ret = devm_mutex_init(&pdev->dev, &core->dev_lock);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = devm_mutex_init(&pdev->dev, &core->hw_lock);
>> +	if (ret)
>> +		return ret;
>> +
>> +	init_completion(&core->irq_done);
>> +	dev_set_drvdata(&pdev->dev, core);
>> +	core->dev = &pdev->dev;
>> +	core->res = res;
>> +
>> +	if (pdev->dev.parent->driver && pdev->dev.parent->driver->name &&
>> +	    !strcmp(pdev->dev.parent->driver->name,
>WAVE6_VPU_PLATFORM_DRIVER_NAME))
>> +		core->vpu = dev_get_drvdata(pdev->dev.parent);
>> +
>> +	core->reg_base = devm_platform_ioremap_resource(pdev, 0);
>> +	if (IS_ERR(core->reg_base))
>> +		return PTR_ERR(core->reg_base);
>> +
>> +	ret = devm_clk_bulk_get_all(&pdev->dev, &core->clks);
>> +	if (ret < 0) {
>> +		dev_warn(&pdev->dev, "unable to get clocks: %d\n", ret);
>
>You should handle deferred probe instead.

Okay, I'll address this in v4 using dev_err_probe().

>
>> +		ret = 0;
>> +	}
>> +	core->num_clks = ret;
>> +
>> +	ret = v4l2_device_register(&pdev->dev, &core->v4l2_dev);
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "v4l2_device_register fail: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	ret = wave6_vpu_init_m2m_dev(core);
>> +	if (ret)
>> +		goto err_v4l2_unregister;
>> +
>> +	irq = platform_get_irq(pdev, 0);
>> +	if (irq < 0) {
>> +		dev_err(&pdev->dev, "failed to get irq resource\n");
>
>Syntax is: dev_err_probe
>
>> +		ret = -ENXIO;
>
>Don't override error codes.

Understood, I'll clean up this part in v4 based on your feedback.

Thanks,
Nas.

>
>> +		goto err_m2m_dev_release;
>> +	}
>> +
>> +	ret = kfifo_alloc(&core->irq_status, 16 * sizeof(int), GFP_KERNEL);
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "failed to allocate fifo\n");
>> +		goto err_m2m_dev_release;
>> +	}
>> +
>> +	ret = devm_request_threaded_irq(&pdev->dev, irq,
>> +					wave6_vpu_core_irq,
>> +					wave6_vpu_core_irq_thread,
>> +					0, "vpu_irq", core);
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "fail to register interrupt
>handler: %d\n", ret);
>> +		goto err_kfifo_free;
>> +	}
>> +
>> +	core->temp_vbuf.size = ALIGN(W6_TEMPBUF_SIZE, 4096);
>> +	ret = wave6_vdi_alloc_dma(core->dev, &core->temp_vbuf);
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "alloc temp of size %zu failed\n",
>> +			core->temp_vbuf.size);
>> +		goto err_kfifo_free;
>> +	}
>> +
>> +	core->debugfs = debugfs_lookup(WAVE6_VPU_DEBUGFS_DIR, NULL);
>> +	if (IS_ERR_OR_NULL(core->debugfs))
>> +		core->debugfs = debugfs_create_dir(WAVE6_VPU_DEBUGFS_DIR,
>NULL);
>> +
>> +	pm_runtime_enable(&pdev->dev);
>> +
>> +	if (core->res->codec_types & WAVE6_IS_DEC) {
>> +		ret = wave6_vpu_dec_register_device(core);
>> +		if (ret) {
>> +			dev_err(&pdev->dev, "wave6_vpu_dec_register_device
>fail: %d\n", ret);
>> +			goto err_temp_vbuf_free;
>> +		}
>> +	}
>> +	if (core->res->codec_types & WAVE6_IS_ENC) {
>> +		ret = wave6_vpu_enc_register_device(core);
>> +		if (ret) {
>> +			dev_err(&pdev->dev, "wave6_vpu_enc_register_device
>fail: %d\n", ret);
>> +			goto err_dec_unreg;
>> +		}
>> +	}
>Best regards,
>Krzysztof


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 8/9] media: chips-media: wave6: Add Wave6 control driver
  2025-09-01  8:13     ` Nas Chung
@ 2025-09-01 10:44       ` Krzysztof Kozlowski
  2025-09-02  2:03         ` Nas Chung
  0 siblings, 1 reply; 19+ messages in thread
From: Krzysztof Kozlowski @ 2025-09-01 10:44 UTC (permalink / raw)
  To: Nas Chung, mchehab@kernel.org, hverkuil@xs4all.nl,
	robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
	shawnguo@kernel.org, s.hauer@pengutronix.de
  Cc: linux-media@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-imx@nxp.com,
	linux-arm-kernel@lists.infradead.org, jackson.lee, lafley.kim,
	Ming Qian

On 01/09/2025 10:13, Nas Chung wrote:
> Hi, Krzysztof.
> 
> Thanks for the feedback.
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzk@kernel.org>
>> Sent: Friday, August 29, 2025 11:06 PM
>> To: Nas Chung <nas.chung@chipsnmedia.com>; mchehab@kernel.org;
>> hverkuil@xs4all.nl; robh@kernel.org; krzk+dt@kernel.org;
>> conor+dt@kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de
>> Cc: linux-media@vger.kernel.org; devicetree@vger.kernel.org; linux-
>> kernel@vger.kernel.org; linux-imx@nxp.com; linux-arm-
>> kernel@lists.infradead.org; jackson.lee <jackson.lee@chipsnmedia.com>;
>> lafley.kim <lafley.kim@chipsnmedia.com>; Ming Qian <ming.qian@oss.nxp.com>
>> Subject: Re: [PATCH v3 8/9] media: chips-media: wave6: Add Wave6 control
>> driver
>>
>> On 29/08/2025 10:46, Nas Chung wrote:
>>> +
>>> +static void wave6_vpu_load_firmware(const struct firmware *fw, void
>> *context)
>>> +{
>>> +	struct wave6_vpu_device *vpu = context;
>>> +
>>> +	guard(mutex)(&vpu->lock);
>>
>> Why? How could this be called in parallel, before the probe?
> 
> This callback is called asynchronously via firmware_request_nowait_nowarn().
> In practice, I observed a case where the callback was executing
> while the device's release() function was being called.

Indeed. Now I wonder how does it protect from concurrent remove()?

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [PATCH v3 8/9] media: chips-media: wave6: Add Wave6 control driver
  2025-09-01 10:44       ` Krzysztof Kozlowski
@ 2025-09-02  2:03         ` Nas Chung
  0 siblings, 0 replies; 19+ messages in thread
From: Nas Chung @ 2025-09-02  2:03 UTC (permalink / raw)
  To: Krzysztof Kozlowski, mchehab@kernel.org, hverkuil@xs4all.nl,
	robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
	shawnguo@kernel.org, s.hauer@pengutronix.de
  Cc: linux-media@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-imx@nxp.com,
	linux-arm-kernel@lists.infradead.org, jackson.lee, lafley.kim,
	Ming Qian

Hi, Krzysztof.

>-----Original Message-----
>From: Krzysztof Kozlowski <krzk@kernel.org>
>Sent: Monday, September 1, 2025 7:44 PM
>To: Nas Chung <nas.chung@chipsnmedia.com>; mchehab@kernel.org;
>hverkuil@xs4all.nl; robh@kernel.org; krzk+dt@kernel.org;
>conor+dt@kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de
>Cc: linux-media@vger.kernel.org; devicetree@vger.kernel.org; linux-
>kernel@vger.kernel.org; linux-imx@nxp.com; linux-arm-
>kernel@lists.infradead.org; jackson.lee <jackson.lee@chipsnmedia.com>;
>lafley.kim <lafley.kim@chipsnmedia.com>; Ming Qian <ming.qian@oss.nxp.com>
>Subject: Re: [PATCH v3 8/9] media: chips-media: wave6: Add Wave6 control
>driver
>
>On 01/09/2025 10:13, Nas Chung wrote:
>> Hi, Krzysztof.
>>
>> Thanks for the feedback.
>>
>>> -----Original Message-----
>>> From: Krzysztof Kozlowski <krzk@kernel.org>
>>> Sent: Friday, August 29, 2025 11:06 PM
>>> To: Nas Chung <nas.chung@chipsnmedia.com>; mchehab@kernel.org;
>>> hverkuil@xs4all.nl; robh@kernel.org; krzk+dt@kernel.org;
>>> conor+dt@kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de
>>> Cc: linux-media@vger.kernel.org; devicetree@vger.kernel.org; linux-
>>> kernel@vger.kernel.org; linux-imx@nxp.com; linux-arm-
>>> kernel@lists.infradead.org; jackson.lee <jackson.lee@chipsnmedia.com>;
>>> lafley.kim <lafley.kim@chipsnmedia.com>; Ming Qian
><ming.qian@oss.nxp.com>
>>> Subject: Re: [PATCH v3 8/9] media: chips-media: wave6: Add Wave6 control
>>> driver
>>>
>>> On 29/08/2025 10:46, Nas Chung wrote:
>>>> +
>>>> +static void wave6_vpu_load_firmware(const struct firmware *fw, void
>>> *context)
>>>> +{
>>>> +	struct wave6_vpu_device *vpu = context;
>>>> +
>>>> +	guard(mutex)(&vpu->lock);
>>>
>>> Why? How could this be called in parallel, before the probe?
>>
>> This callback is called asynchronously via
>firmware_request_nowait_nowarn().
>> In practice, I observed a case where the callback was executing
>> while the device's release() function was being called.
>
>Indeed. Now I wonder how does it protect from concurrent remove()?

The callback checks the fw_available flag and returns immediately if false.
This flag is cleared in the remove() path under the lock.

Thanks,
Nas.

>
>Best regards,
>Krzysztof

^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [PATCH v3 2/9] dt-bindings: media: nxp: Add Wave6 video codec device
  2025-08-29 13:57   ` Krzysztof Kozlowski
@ 2025-09-02  5:45     ` Nas Chung
  2025-09-02  7:42       ` Nas Chung
  0 siblings, 1 reply; 19+ messages in thread
From: Nas Chung @ 2025-09-02  5:45 UTC (permalink / raw)
  To: Krzysztof Kozlowski, mchehab@kernel.org, hverkuil@xs4all.nl,
	robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
	shawnguo@kernel.org, s.hauer@pengutronix.de
  Cc: linux-media@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-imx@nxp.com,
	linux-arm-kernel@lists.infradead.org, jackson.lee, lafley.kim

Hi, Krzysztof.

>-----Original Message-----
>From: Krzysztof Kozlowski <krzk@kernel.org>
>Sent: Friday, August 29, 2025 10:57 PM
>To: Nas Chung <nas.chung@chipsnmedia.com>; mchehab@kernel.org;
>hverkuil@xs4all.nl; robh@kernel.org; krzk+dt@kernel.org;
>conor+dt@kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de
>Cc: linux-media@vger.kernel.org; devicetree@vger.kernel.org; linux-
>kernel@vger.kernel.org; linux-imx@nxp.com; linux-arm-
>kernel@lists.infradead.org; jackson.lee <jackson.lee@chipsnmedia.com>;
>lafley.kim <lafley.kim@chipsnmedia.com>
>Subject: Re: [PATCH v3 2/9] dt-bindings: media: nxp: Add Wave6 video codec
>device
>
>On 29/08/2025 10:46, Nas Chung wrote:
>> Add documents for the Wave6 video codec on NXP i.MX SoCs.
>Pretty incomplete commit msg. Nothing explaining hardware, nothing
>documenting resolution of previous discussions (where is all this
>chip&media?).

I see,  I'll improve the commit message in v4 to include hardware details.

>
>...
>
>
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - nxp,imx95-vpu
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    maxItems: 1
>> +
>> +  power-domains:
>> +    maxItems: 1
>> +
>> +  memory-region:
>> +    maxItems: 1
>> +
>> +  sram:
>> +    $ref: /schemas/types.yaml#/definitions/phandle
>> +    description: phandle of the SRAM memory region node.
>> +
>> +  "#cooling-cells":
>> +    const: 2
>> +
>> +  "#address-cells":
>> +    const: 2
>> +
>> +  "#size-cells":
>> +    const: 2
>> +
>> +  ranges: true
>> +
>> +patternProperties:
>> +  "^video-core@[0-9a-f]+$":
>> +    type: object
>
>Missing description.

I'll add a description in v4.

>
>> +    additionalProperties: false
>> +
>> +    properties:
>> +      compatible:
>> +        enum:
>> +          - nxp,imx95-vpu-core
>
>Why do you need here compatible? Can this child be anything else? Can it
>be re-used? Is it actually a separate block?
>
>Your example suggests that the only distinctive resource are the
>interrupt and address space and that's on the edge of calling it a
>separate device.
>
>There is some tendency to call such "pseudo-cores" a separate devices in
>case of video codec bindings and experience shows these are usually
>fake. It's not the same as DP or HDMI sub-block of display pipeline.
>
>That's why you should come here with strong argument what separate piece
>of hardware this is.

Thanks for your feedback.

As you mentioned, I wanted to represent the interrupts and address space
as separate "cores". This is because, from an external perspective (e.g. multi-VM),
each of these resources is a VPU interface and can be accessed independently
to operate the VPU.

However, there is indeed only one actual VPU processing engine.
I understand your point about "pseudo-cores".

I would appreciate any guidance on the preferred way to represent
these resources in the device tree.

>
>> +
>> +      reg:
>> +        maxItems: 1
>> +
>> +      clocks:
>> +        maxItems: 1
>> +
>> +      power-domains:
>> +        maxItems: 1
>> +
>> +      interrupts:
>> +        maxItems: 1
>> +
>> +    required:
>> +      - compatible
>> +      - reg
>> +      - clocks
>> +      - power-domains
>> +      - interrupts
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - clocks
>> +  - power-domains
>> +  - memory-region
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +    #include <dt-bindings/clock/nxp,imx95-clock.h>
>> +
>> +    soc {
>> +      #address-cells = <2>;
>> +      #size-cells = <2>;
>> +
>> +      vpu: video-codec@4c4c0000 {
>
>Unused label, drop

Okay. I'll drop the unused label.

>
>> +        compatible = "nxp,imx95-vpu";
>> +        reg = <0x0 0x4c4c0000 0x0 0x10000>;
>> +        clocks = <&vpu_blk_ctrl IMX95_CLK_VPUBLK_WAVE>;
>> +        power-domains = <&scmi_perf 10>;
>> +        memory-region = <&vpu_boot>;
>> +        sram = <&sram1>;
>> +        #cooling-cells = <2>;
>> +        #address-cells = <2>;
>> +        #size-cells = <2>;
>> +        ranges;
>> +
>> +        vpucore0: video-core@4c480000 {
>
>None of these labels are used, drop.

I'll drop it.

Thanks,
Nas.

>
>> +          compatible = "nxp,imx95-vpu-core";
>> +          reg = <0x0 0x4c480000 0x0 0x10000>;
>> +          clocks = <&scmi_clk 115>;
>> +          power-domains = <&scmi_devpd 21>;
>> +          interrupts = <GIC_SPI 299 IRQ_TYPE_LEVEL_HIGH>;
>> +        };
>> +
>> +        vpucore1: video-core@4c490000 {
>> +          compatible = "nxp,imx95-vpu-core";
>> +          reg = <0x0 0x4c490000 0x0 0x10000>;
>> +          clocks = <&scmi_clk 115>;
>> +          power-domains = <&scmi_devpd 21>;
>> +          interrupts = <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>;
>> +        };
>> +
>
>
>
>Best regards,
>Krzysztof


^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [PATCH v3 2/9] dt-bindings: media: nxp: Add Wave6 video codec device
  2025-09-02  5:45     ` Nas Chung
@ 2025-09-02  7:42       ` Nas Chung
  0 siblings, 0 replies; 19+ messages in thread
From: Nas Chung @ 2025-09-02  7:42 UTC (permalink / raw)
  To: Nas Chung, Krzysztof Kozlowski, mchehab@kernel.org,
	hverkuil@xs4all.nl, robh@kernel.org, krzk+dt@kernel.org,
	conor+dt@kernel.org, shawnguo@kernel.org, s.hauer@pengutronix.de
  Cc: linux-media@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-imx@nxp.com,
	linux-arm-kernel@lists.infradead.org, jackson.lee, lafley.kim

Hi, Krzysztof.

>-----Original Message-----
>From: Nas Chung <nas.chung@chipsnmedia.com>
>Sent: Tuesday, September 2, 2025 2:46 PM
>To: Krzysztof Kozlowski <krzk@kernel.org>; mchehab@kernel.org;
>hverkuil@xs4all.nl; robh@kernel.org; krzk+dt@kernel.org;
>conor+dt@kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de
>Cc: linux-media@vger.kernel.org; devicetree@vger.kernel.org; linux-
>kernel@vger.kernel.org; linux-imx@nxp.com; linux-arm-
>kernel@lists.infradead.org; jackson.lee <jackson.lee@chipsnmedia.com>;
>lafley.kim <lafley.kim@chipsnmedia.com>
>Subject: RE: [PATCH v3 2/9] dt-bindings: media: nxp: Add Wave6 video codec
>device
>
>Hi, Krzysztof.
>
>>-----Original Message-----
>>From: Krzysztof Kozlowski <krzk@kernel.org>
>>Sent: Friday, August 29, 2025 10:57 PM
>>To: Nas Chung <nas.chung@chipsnmedia.com>; mchehab@kernel.org;
>>hverkuil@xs4all.nl; robh@kernel.org; krzk+dt@kernel.org;
>>conor+dt@kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de
>>Cc: linux-media@vger.kernel.org; devicetree@vger.kernel.org; linux-
>>kernel@vger.kernel.org; linux-imx@nxp.com; linux-arm-
>>kernel@lists.infradead.org; jackson.lee <jackson.lee@chipsnmedia.com>;
>>lafley.kim <lafley.kim@chipsnmedia.com>
>>Subject: Re: [PATCH v3 2/9] dt-bindings: media: nxp: Add Wave6 video codec
>>device
>>
>>On 29/08/2025 10:46, Nas Chung wrote:
>>> Add documents for the Wave6 video codec on NXP i.MX SoCs.
>>Pretty incomplete commit msg. Nothing explaining hardware, nothing
>>documenting resolution of previous discussions (where is all this
>>chip&media?).
>
>I see,  I'll improve the commit message in v4 to include hardware details.
>
>>
>>...
>>
>>
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - nxp,imx95-vpu
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +  clocks:
>>> +    maxItems: 1
>>> +
>>> +  power-domains:
>>> +    maxItems: 1
>>> +
>>> +  memory-region:
>>> +    maxItems: 1
>>> +
>>> +  sram:
>>> +    $ref: /schemas/types.yaml#/definitions/phandle
>>> +    description: phandle of the SRAM memory region node.
>>> +
>>> +  "#cooling-cells":
>>> +    const: 2
>>> +
>>> +  "#address-cells":
>>> +    const: 2
>>> +
>>> +  "#size-cells":
>>> +    const: 2
>>> +
>>> +  ranges: true
>>> +
>>> +patternProperties:
>>> +  "^video-core@[0-9a-f]+$":
>>> +    type: object
>>
>>Missing description.
>
>I'll add a description in v4.
>
>>
>>> +    additionalProperties: false
>>> +
>>> +    properties:
>>> +      compatible:
>>> +        enum:
>>> +          - nxp,imx95-vpu-core
>>
>>Why do you need here compatible? Can this child be anything else? Can it
>>be re-used? Is it actually a separate block?
>>
>>Your example suggests that the only distinctive resource are the
>>interrupt and address space and that's on the edge of calling it a
>>separate device.
>>
>>There is some tendency to call such "pseudo-cores" a separate devices in
>>case of video codec bindings and experience shows these are usually
>>fake. It's not the same as DP or HDMI sub-block of display pipeline.
>>
>>That's why you should come here with strong argument what separate piece
>>of hardware this is.
>
>Thanks for your feedback.
>
>As you mentioned, I wanted to represent the interrupts and address space
>as separate "cores". This is because, from an external perspective (e.g.
>multi-VM),
>each of these resources is a VPU interface and can be accessed
>independently
>to operate the VPU.

Apologies, I forgot to mention one detail in my previous reply.

I did not include the SMMU-related properties in the core nodes.
On this SoC, however, each of these cores has its own SMMU ID
as part of the SoC's isolation policy.
This allows them to be treated as independent interfaces,
even though there is only one actual VPU engine.

Would adding the iommu property be the appropriate way to describe
this in the device tree?

Thanks,
Nas.

>
>However, there is indeed only one actual VPU processing engine.
>I understand your point about "pseudo-cores".
>
>I would appreciate any guidance on the preferred way to represent
>these resources in the device tree.
>
>>
>>> +
>>> +      reg:
>>> +        maxItems: 1
>>> +
>>> +      clocks:
>>> +        maxItems: 1
>>> +
>>> +      power-domains:
>>> +        maxItems: 1
>>> +
>>> +      interrupts:
>>> +        maxItems: 1
>>> +
>>> +    required:
>>> +      - compatible
>>> +      - reg
>>> +      - clocks
>>> +      - power-domains
>>> +      - interrupts
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +  - clocks
>>> +  - power-domains
>>> +  - memory-region
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  - |
>>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>>> +    #include <dt-bindings/clock/nxp,imx95-clock.h>
>>> +
>>> +    soc {
>>> +      #address-cells = <2>;
>>> +      #size-cells = <2>;
>>> +
>>> +      vpu: video-codec@4c4c0000 {
>>
>>Unused label, drop
>
>Okay. I'll drop the unused label.
>
>>
>>> +        compatible = "nxp,imx95-vpu";
>>> +        reg = <0x0 0x4c4c0000 0x0 0x10000>;
>>> +        clocks = <&vpu_blk_ctrl IMX95_CLK_VPUBLK_WAVE>;
>>> +        power-domains = <&scmi_perf 10>;
>>> +        memory-region = <&vpu_boot>;
>>> +        sram = <&sram1>;
>>> +        #cooling-cells = <2>;
>>> +        #address-cells = <2>;
>>> +        #size-cells = <2>;
>>> +        ranges;
>>> +
>>> +        vpucore0: video-core@4c480000 {
>>
>>None of these labels are used, drop.
>
>I'll drop it.
>
>Thanks,
>Nas.
>
>>
>>> +          compatible = "nxp,imx95-vpu-core";
>>> +          reg = <0x0 0x4c480000 0x0 0x10000>;
>>> +          clocks = <&scmi_clk 115>;
>>> +          power-domains = <&scmi_devpd 21>;
>>> +          interrupts = <GIC_SPI 299 IRQ_TYPE_LEVEL_HIGH>;
>>> +        };
>>> +
>>> +        vpucore1: video-core@4c490000 {
>>> +          compatible = "nxp,imx95-vpu-core";
>>> +          reg = <0x0 0x4c490000 0x0 0x10000>;
>>> +          clocks = <&scmi_clk 115>;
>>> +          power-domains = <&scmi_devpd 21>;
>>> +          interrupts = <GIC_SPI 300 IRQ_TYPE_LEVEL_HIGH>;
>>> +        };
>>> +
>>
>>
>>
>>Best regards,
>>Krzysztof



^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2025-09-02  7:46 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-29  8:46 [PATCH v3 0/9] Add support for Wave6 video codec driver Nas Chung
2025-08-29  8:46 ` [PATCH v3 1/9] media: v4l2-common: Add YUV24 format info Nas Chung
2025-08-29  8:46 ` [PATCH v3 2/9] dt-bindings: media: nxp: Add Wave6 video codec device Nas Chung
2025-08-29 13:57   ` Krzysztof Kozlowski
2025-09-02  5:45     ` Nas Chung
2025-09-02  7:42       ` Nas Chung
2025-08-29  8:46 ` [PATCH v3 5/9] media: chips-media: wave6: Add Wave6 core driver Nas Chung
2025-08-29 14:02   ` Krzysztof Kozlowski
2025-09-01  8:34     ` Nas Chung
2025-08-29  8:46 ` [PATCH v3 6/9] media: chips-media: wave6: Improve debugging capabilities Nas Chung
2025-08-29  8:46 ` [PATCH v3 7/9] media: chips-media: wave6: Add Wave6 thermal cooling device Nas Chung
2025-08-29  8:46 ` [PATCH v3 8/9] media: chips-media: wave6: Add Wave6 control driver Nas Chung
2025-08-29 14:06   ` Krzysztof Kozlowski
2025-09-01  8:13     ` Nas Chung
2025-09-01 10:44       ` Krzysztof Kozlowski
2025-09-02  2:03         ` Nas Chung
2025-08-29  8:46 ` [PATCH v3 9/9] arm64: dts: freescale: imx95: Add video codec node Nas Chung
2025-08-29 14:07   ` Krzysztof Kozlowski
2025-09-01  7:38     ` Nas Chung

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).