linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/5] Support Nuvoton NPCM Video Capture/Encode Engine
@ 2022-05-13  3:34 Marvin Lin
  2022-05-13  3:34 ` [PATCH v2 1/5] arm: dts: Add node for " Marvin Lin
                   ` (4 more replies)
  0 siblings, 5 replies; 17+ messages in thread
From: Marvin Lin @ 2022-05-13  3:34 UTC (permalink / raw)
  To: mchehab, linux-media, linux-kernel
  Cc: openbmc, avifishman70, tmaimon77, tali.perry1, kwliu, kflin

Changes in v2: Add Hextile document and locate with vendor formats.

This patch series add DTS node, dt-bindings document and drivers for Video
Capture/Differentiation Engine (VCD) and Encoding Compression Engine (ECE)
present on Nuvoton NPCM SoCs.

VCD can capture/differentiate video data from digital or analog sources,
then ECE will compress the data into HEXTILE format.

HEXTILE compressed format is defined in Remote Framebuffer Protocol (RFC
6143) and is used by VNC features, so we also add a patch to support it.

Marvin Lin (5):
  arm: dts: Add node for NPCM Video Capture/Encode Engine
  dt-bindings: media: Add dt-bindings for NPCM Video Capture/Encode
    Engine
  dt-bindings: arm/npcm: Add dt-bindings for Graphics Core Information
  media: Add HEXTILE compressed format
  drivers: media: platform: Add NPCM Video Capture/Encode Engine driver

 .../bindings/arm/npcm/nuvoton,gfxi.yaml       |   41 +
 .../bindings/media/nuvoton,npcm-video.yaml    |   87 +
 .../media/v4l/pixfmt-reserved.rst             |    7 +
 arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi |   19 +
 drivers/media/platform/Kconfig                |    1 +
 drivers/media/platform/Makefile               |    1 +
 drivers/media/platform/nuvoton/Kconfig        |   12 +
 drivers/media/platform/nuvoton/Makefile       |    2 +
 drivers/media/platform/nuvoton/npcm-video.c   | 2074 +++++++++++++++++
 drivers/media/v4l2-core/v4l2-ioctl.c          |    1 +
 include/uapi/linux/videodev2.h                |    1 +
 11 files changed, 2246 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/npcm/nuvoton,gfxi.yaml
 create mode 100644 Documentation/devicetree/bindings/media/nuvoton,npcm-video.yaml
 create mode 100644 drivers/media/platform/nuvoton/Kconfig
 create mode 100644 drivers/media/platform/nuvoton/Makefile
 create mode 100644 drivers/media/platform/nuvoton/npcm-video.c

-- 
2.17.1


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

* [PATCH v2 1/5] arm: dts: Add node for NPCM Video Capture/Encode Engine
  2022-05-13  3:34 [PATCH v2 0/5] Support Nuvoton NPCM Video Capture/Encode Engine Marvin Lin
@ 2022-05-13  3:34 ` Marvin Lin
  2022-05-13  5:22   ` Paul Menzel
  2022-05-13  3:34 ` [PATCH v2 2/5] dt-bindings: media: Add dt-bindings " Marvin Lin
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: Marvin Lin @ 2022-05-13  3:34 UTC (permalink / raw)
  To: mchehab, linux-media, linux-kernel
  Cc: openbmc, avifishman70, tmaimon77, tali.perry1, kwliu, kflin

Add node for Video Capture/Differentiation Engine (VCD) and Encoding
Compression Engine (ECE) present on Nuvoton NPCM SoCs.

Signed-off-by: Marvin Lin <kflin@nuvoton.com>
---
 arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi b/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi
index 3696980a3da1..0d2df74974bf 100644
--- a/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi
+++ b/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi
@@ -178,6 +178,19 @@
 			status = "disabled";
 		};
 
+		video: video@f0810000 {
+			compatible = "nuvoton,npcm750-video";
+			reg = <0xf0810000 0x10000>, <0xf0820000 0x2000>;
+			reg-names = "vcd", "ece";
+			interrupts = <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>;
+			resets = <&rstc NPCM7XX_RESET_IPSRST2 NPCM7XX_RESET_VCD>,
+				 <&rstc NPCM7XX_RESET_IPSRST2 NPCM7XX_RESET_ECE>;
+			reset-names = "vcd", "ece";
+			nuvoton,syscon-gcr = <&gcr>;
+			nuvoton,syscon-gfxi = <&gfxi>;
+			status = "disabled";
+		};
+
 		apb {
 			#address-cells = <1>;
 			#size-cells = <1>;
@@ -553,6 +566,12 @@
 				pinctrl-0 = <&smb15_pins>;
 				status = "disabled";
 			};
+
+			gfxi: gfxi@e000 {
+				compatible = "nuvoton,npcm750-gfxi", "syscon",
+					     "simple-mfd";
+				reg = <0xe000 0x100>;
+			};
 		};
 	};
 
-- 
2.17.1


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

* [PATCH v2 2/5] dt-bindings: media: Add dt-bindings for NPCM Video Capture/Encode Engine
  2022-05-13  3:34 [PATCH v2 0/5] Support Nuvoton NPCM Video Capture/Encode Engine Marvin Lin
  2022-05-13  3:34 ` [PATCH v2 1/5] arm: dts: Add node for " Marvin Lin
@ 2022-05-13  3:34 ` Marvin Lin
  2022-05-13  3:34 ` [PATCH v2 3/5] dt-bindings: arm/npcm: Add dt-bindings for Graphics Core Information Marvin Lin
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 17+ messages in thread
From: Marvin Lin @ 2022-05-13  3:34 UTC (permalink / raw)
  To: mchehab, linux-media, linux-kernel
  Cc: openbmc, avifishman70, tmaimon77, tali.perry1, kwliu, kflin

Add dt-bindings document for NPCM Video Capture/Encode Engine.

Signed-off-by: Marvin Lin <kflin@nuvoton.com>
---
 .../bindings/media/nuvoton,npcm-video.yaml    | 87 +++++++++++++++++++
 1 file changed, 87 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/nuvoton,npcm-video.yaml

diff --git a/Documentation/devicetree/bindings/media/nuvoton,npcm-video.yaml b/Documentation/devicetree/bindings/media/nuvoton,npcm-video.yaml
new file mode 100644
index 000000000000..b5be7ef09038
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/nuvoton,npcm-video.yaml
@@ -0,0 +1,87 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/nuvoton,npcm-video.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Nuvoton NPCM Video Capture/Encode Engine Device Tree Bindings
+
+maintainers:
+  - Joseph Liu <kwliu@nuvoton.com>
+  - Marvin Lin <kflin@nuvoton.com>
+
+description: |
+  Video Capture/Differentiation Engine (VCD) and Encoding Compression Engine
+  (ECE) present on Nuvoton NPCM SoCs.
+
+properties:
+  compatible:
+    enum:
+      - nuvoton,npcm750-video
+      - nuvoton,npcm845-video
+
+  reg:
+    items:
+      - description: VCD registers
+      - description: ECE registers
+
+  reg-names:
+    items:
+      - const: vcd
+      - const: ece
+
+  interrupts:
+    maxItems: 1
+
+  resets:
+    items:
+      - description: VCD reset control
+      - description: ECE reset control
+
+  reset-names:
+    items:
+      - const: vcd
+      - const: ece
+
+  nuvoton,syscon-gcr:
+    $ref: /schemas/types.yaml#definitions/phandle
+    description: Phandle to the Global Control Register DT node
+
+  nuvoton,syscon-gfxi:
+    $ref: /schemas/types.yaml#definitions/phandle
+    description: Phandle to the Graphics Core Information DT node
+
+  memory-region:
+    description:
+      CMA pool to use for buffers allocation instead of the default CMA pool.
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - interrupts
+  - resets
+  - reset-names
+  - nuvoton,syscon-gcr
+  - nuvoton,syscon-gfxi
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/reset/nuvoton,npcm7xx-reset.h>
+
+    video: video@f0810000 {
+        compatible = "nuvoton,npcm750-video";
+        reg = <0xf0810000 0x10000>,
+              <0xf0820000 0x2000>;
+        reg-names = "vcd", "ece";
+        interrupts = <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>;
+        resets = <&rstc NPCM7XX_RESET_IPSRST2 NPCM7XX_RESET_VCD>,
+                 <&rstc NPCM7XX_RESET_IPSRST2 NPCM7XX_RESET_ECE>;
+        reset-names = "vcd", "ece";
+        nuvoton,syscon-gcr = <&gcr>;
+        nuvoton,syscon-gfxi = <&gfxi>;
+    };
-- 
2.17.1


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

* [PATCH v2 3/5] dt-bindings: arm/npcm: Add dt-bindings for Graphics Core Information
  2022-05-13  3:34 [PATCH v2 0/5] Support Nuvoton NPCM Video Capture/Encode Engine Marvin Lin
  2022-05-13  3:34 ` [PATCH v2 1/5] arm: dts: Add node for " Marvin Lin
  2022-05-13  3:34 ` [PATCH v2 2/5] dt-bindings: media: Add dt-bindings " Marvin Lin
@ 2022-05-13  3:34 ` Marvin Lin
  2022-05-13  3:34 ` [PATCH v2 4/5] media: Add HEXTILE compressed format Marvin Lin
  2022-05-13  3:34 ` [PATCH v2 5/5] drivers: media: platform: Add NPCM Video Capture/Encode Engine driver Marvin Lin
  4 siblings, 0 replies; 17+ messages in thread
From: Marvin Lin @ 2022-05-13  3:34 UTC (permalink / raw)
  To: mchehab, linux-media, linux-kernel
  Cc: openbmc, avifishman70, tmaimon77, tali.perry1, kwliu, kflin

Add dt-bindings document for Graphics Core Information (GFXI) node. It is
necessary for the NPCM Video Capture/Encode Engine driver to retrieve
Graphics core information.

Signed-off-by: Marvin Lin <kflin@nuvoton.com>
---
 .../bindings/arm/npcm/nuvoton,gfxi.yaml       | 41 +++++++++++++++++++
 1 file changed, 41 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/npcm/nuvoton,gfxi.yaml

diff --git a/Documentation/devicetree/bindings/arm/npcm/nuvoton,gfxi.yaml b/Documentation/devicetree/bindings/arm/npcm/nuvoton,gfxi.yaml
new file mode 100644
index 000000000000..5b785bda5739
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/npcm/nuvoton,gfxi.yaml
@@ -0,0 +1,41 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/arm/npcm/nuvoton,gfxi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Graphics Core Information block in Nuvoton SoCs
+
+maintainers:
+  - Joseph Liu <kwliu@nuvoton.com>
+  - Marvin Lin <kflin@nuvoton.com>
+
+description:
+  The Graphics Core Information (GFXI) are a block of registers in Nuvoton SoCs
+  that analyzes Graphics core behavior and provides inforomation in registers.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - nuvoton,npcm750-gfxi
+          - nuvoton,npcm845-gfxi
+      - const: syscon
+      - const: simple-mfd
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    gfxi: gfxi@e000 {
+      compatible = "nuvoton,npcm750-gfxi", "syscon", "simple-mfd";
+      reg = <0xe000 0x100>;
+    };
-- 
2.17.1


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

* [PATCH v2 4/5] media: Add HEXTILE compressed format
  2022-05-13  3:34 [PATCH v2 0/5] Support Nuvoton NPCM Video Capture/Encode Engine Marvin Lin
                   ` (2 preceding siblings ...)
  2022-05-13  3:34 ` [PATCH v2 3/5] dt-bindings: arm/npcm: Add dt-bindings for Graphics Core Information Marvin Lin
@ 2022-05-13  3:34 ` Marvin Lin
  2022-05-13  3:34 ` [PATCH v2 5/5] drivers: media: platform: Add NPCM Video Capture/Encode Engine driver Marvin Lin
  4 siblings, 0 replies; 17+ messages in thread
From: Marvin Lin @ 2022-05-13  3:34 UTC (permalink / raw)
  To: mchehab, linux-media, linux-kernel
  Cc: openbmc, avifishman70, tmaimon77, tali.perry1, kwliu, kflin

Add HEXTILE compressed format. This format is defined in Remote Framebuffer
Protocol (RFC 6143) and is used by Encoding Compression Engine present on
Nuvoton NPCM SoCs.

Signed-off-by: Marvin Lin <kflin@nuvoton.com>
---
 Documentation/userspace-api/media/v4l/pixfmt-reserved.rst | 7 +++++++
 drivers/media/v4l2-core/v4l2-ioctl.c                      | 1 +
 include/uapi/linux/videodev2.h                            | 1 +
 3 files changed, 9 insertions(+)

diff --git a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst
index cabfa34b7db5..bad2b9a2f16e 100644
--- a/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst
+++ b/Documentation/userspace-api/media/v4l/pixfmt-reserved.rst
@@ -239,6 +239,13 @@ please make a proposal on the linux-media mailing list.
 	It remains an opaque intermediate format and the MDP hardware must be
 	used to convert ``V4L2_PIX_FMT_MT21C`` to ``V4L2_PIX_FMT_NV12M``,
 	``V4L2_PIX_FMT_YUV420M`` or ``V4L2_PIX_FMT_YVU420``.
+    * .. _V4L2-PIX-FMT-HEXTILE:
+
+      - ``V4L2_PIX_FMT_HEXTILE``
+      - 'HXTL'
+      - Compressed format used by Nuvoton NPCM video driver. This format is
+        defined in Remote Framebuffer Protocol (RFC 6143, chapter 7.7.4 Hextile
+        Encoding).
 .. raw:: latex
 
     \normalsize
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 96e307fe3aab..9365519d65bb 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -1441,6 +1441,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
 		case V4L2_PIX_FMT_SE401:	descr = "GSPCA SE401"; break;
 		case V4L2_PIX_FMT_S5C_UYVY_JPG:	descr = "S5C73MX interleaved UYVY/JPEG"; break;
 		case V4L2_PIX_FMT_MT21C:	descr = "Mediatek Compressed Format"; break;
+		case V4L2_PIX_FMT_HEXTILE:	descr = "Hextile Compressed Format"; break;
 		default:
 			if (fmt->description[0])
 				return;
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 3768a0a80830..1c7af015b43b 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -746,6 +746,7 @@ struct v4l2_pix_format {
 #define V4L2_PIX_FMT_INZI     v4l2_fourcc('I', 'N', 'Z', 'I') /* Intel Planar Greyscale 10-bit and Depth 16-bit */
 #define V4L2_PIX_FMT_CNF4     v4l2_fourcc('C', 'N', 'F', '4') /* Intel 4-bit packed depth confidence information */
 #define V4L2_PIX_FMT_HI240    v4l2_fourcc('H', 'I', '2', '4') /* BTTV 8-bit dithered RGB */
+#define V4L2_PIX_FMT_HEXTILE  v4l2_fourcc('H', 'X', 'T', 'L') /* Hextile compression */
 
 /* 10bit raw bayer packed, 32 bytes for every 25 pixels, last LSB 6 bits unused */
 #define V4L2_PIX_FMT_IPU3_SBGGR10	v4l2_fourcc('i', 'p', '3', 'b') /* IPU3 packed 10-bit BGGR bayer */
-- 
2.17.1


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

* [PATCH v2 5/5] drivers: media: platform: Add NPCM Video Capture/Encode Engine driver
  2022-05-13  3:34 [PATCH v2 0/5] Support Nuvoton NPCM Video Capture/Encode Engine Marvin Lin
                   ` (3 preceding siblings ...)
  2022-05-13  3:34 ` [PATCH v2 4/5] media: Add HEXTILE compressed format Marvin Lin
@ 2022-05-13  3:34 ` Marvin Lin
  2022-05-13  5:46   ` Paul Menzel
                     ` (4 more replies)
  4 siblings, 5 replies; 17+ messages in thread
From: Marvin Lin @ 2022-05-13  3:34 UTC (permalink / raw)
  To: mchehab, linux-media, linux-kernel
  Cc: openbmc, avifishman70, tmaimon77, tali.perry1, kwliu, kflin

Add driver for Video Capture/Differentiation Engine (VCD) and Encoding
Compression Engine (ECE) present on Nuvoton NPCM SoCs.

Signed-off-by: Marvin Lin <kflin@nuvoton.com>
---
 drivers/media/platform/Kconfig              |    1 +
 drivers/media/platform/Makefile             |    1 +
 drivers/media/platform/nuvoton/Kconfig      |   12 +
 drivers/media/platform/nuvoton/Makefile     |    2 +
 drivers/media/platform/nuvoton/npcm-video.c | 2074 +++++++++++++++++++
 5 files changed, 2090 insertions(+)
 create mode 100644 drivers/media/platform/nuvoton/Kconfig
 create mode 100644 drivers/media/platform/nuvoton/Makefile
 create mode 100644 drivers/media/platform/nuvoton/npcm-video.c

diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index f1056ceaf5a8..21daeedb47c9 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -72,6 +72,7 @@ source "drivers/media/platform/chips-media/Kconfig"
 source "drivers/media/platform/intel/Kconfig"
 source "drivers/media/platform/marvell/Kconfig"
 source "drivers/media/platform/mediatek/Kconfig"
+source "drivers/media/platform/nuvoton/Kconfig"
 source "drivers/media/platform/nvidia/Kconfig"
 source "drivers/media/platform/nxp/Kconfig"
 source "drivers/media/platform/qcom/Kconfig"
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index a881e97bae95..4a1da16c4f8a 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -15,6 +15,7 @@ obj-y += chips-media/
 obj-y += intel/
 obj-y += marvell/
 obj-y += mediatek/
+obj-y += nuvoton/
 obj-y += nvidia/
 obj-y += nxp/
 obj-y += qcom/
diff --git a/drivers/media/platform/nuvoton/Kconfig b/drivers/media/platform/nuvoton/Kconfig
new file mode 100644
index 000000000000..8718631b0424
--- /dev/null
+++ b/drivers/media/platform/nuvoton/Kconfig
@@ -0,0 +1,12 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+comment "Nuvoton media platform drivers"
+
+config VIDEO_NUVOTON
+	tristate "Nuvoton NPCM Video Capture/Encode Engine driver"
+	depends on V4L_PLATFORM_DRIVERS
+	depends on VIDEO_DEV
+	select VIDEOBUF2_DMA_CONTIG
+	help
+	  Support for the Video Capture/Differentiation Engine (VCD) and
+	  Encoding Compression Engine (ECE) present on Nuvoton NPCM SoCs.
diff --git a/drivers/media/platform/nuvoton/Makefile b/drivers/media/platform/nuvoton/Makefile
new file mode 100644
index 000000000000..63041bb526a9
--- /dev/null
+++ b/drivers/media/platform/nuvoton/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0-only
+obj-$(CONFIG_VIDEO_NUVOTON) += npcm-video.o
diff --git a/drivers/media/platform/nuvoton/npcm-video.c b/drivers/media/platform/nuvoton/npcm-video.c
new file mode 100644
index 000000000000..4ed5290f4c5d
--- /dev/null
+++ b/drivers/media/platform/nuvoton/npcm-video.c
@@ -0,0 +1,2074 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for Video Capture/Differentiation Engine (VCD) and Encoding
+ * Compression Engine (ECE) present on Nuvoton NPCM SoCs.
+ *
+ * Copyright (C) 2022 Nuvoton Technologies
+ */
+
+#include <linux/atomic.h>
+#include <linux/bitfield.h>
+#include <linux/bitmap.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/dma-mapping.h>
+#include <linux/interrupt.h>
+#include <linux/jiffies.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/of_reserved_mem.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
+#include <linux/sched.h>
+#include <linux/spinlock.h>
+#include <linux/string.h>
+#include <linux/v4l2-controls.h>
+#include <linux/videodev2.h>
+#include <linux/wait.h>
+#include <linux/workqueue.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-dev.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-dv-timings.h>
+#include <media/v4l2-event.h>
+#include <media/v4l2-ioctl.h>
+#include <media/videobuf2-dma-contig.h>
+
+#define DEVICE_NAME			"nuvoton-video"
+
+#define MAX_FRAME_RATE			60
+#define MAX_WIDTH			1920
+#define MAX_HEIGHT			1200
+#define MIN_WIDTH			320
+#define MIN_HEIGHT			240
+#define MIN_LP				512
+#define MAX_LP				4096
+#define RECT_W				16
+#define RECT_H				16
+#define BITMAP_SIZE			32
+
+#define VCD_MODULE_NAME			"vcd"
+#define ECE_MODULE_NAME			"ece"
+
+/* VCD Registers */
+#define VCD_DIFF_TBL			0x0000
+#define VCD_FBA_ADR			0x8000
+#define VCD_FBB_ADR			0x8004
+
+#define VCD_FB_LP			0x8008
+#define  VCD_FBA_LP			GENMASK(15, 0)
+#define  VCD_FBB_LP			GENMASK(31, 16)
+
+#define VCD_CAP_RES			0x800c
+#define  VCD_CAP_RES_VERT_RES		GENMASK(10, 0)
+#define  VCD_CAP_RES_HOR_RES		GENMASK(26, 16)
+
+#define VCD_MODE			0x8014
+#define  VCD_MODE_VCDE			BIT(0)
+#define  VCD_MODE_CM565			BIT(1)
+#define  VCD_MODE_IDBC			BIT(3)
+#define  VCD_MODE_COLOR_CNVRT		GENMASK(5, 4)
+#define   VCD_MODE_COLOR_CNVRT_NO_CNVRT	0
+#define   VCD_MODE_COLOR_CNVRT_RGB_222	1
+#define   VCD_MODE_COLOR_CNVRT_666_MODE	2
+#define   VCD_MODE_COLOR_CNVRT_RGB_888	3
+#define  VCD_MODE_KVM_BW_SET		BIT(16)
+
+#define VCD_CMD				0x8018
+#define  VCD_CMD_GO			BIT(0)
+#define  VCD_CMD_RST			BIT(1)
+#define  VCD_CMD_OPERATION		GENMASK(6, 4)
+#define   VCD_CMD_OPERATION_CAPTURE	0
+#define   VCD_CMD_OPERATION_COMPARE	2
+
+#define	VCD_STAT			0x801c
+#define	 VCD_STAT_DONE			BIT(0)
+#define	 VCD_STAT_IFOT			BIT(2)
+#define	 VCD_STAT_IFOR			BIT(3)
+#define	 VCD_STAT_BUSY			BIT(30)
+#define	VCD_STAT_CLEAR			0x3fff
+
+#define VCD_INTE			0x8020
+#define  VCD_INTE_DONE_IE		BIT(0)
+#define  VCD_INTE_IFOT_IE		BIT(2)
+#define  VCD_INTE_IFOR_IE		BIT(3)
+
+#define VCD_RCHG			0x8028
+#define VCD_RCHG_TIM_PRSCL		GENMASK(12, 9)
+
+#define VCD_FIFO			0x805c
+#define  VCD_FIFO_TH			0x100350ff
+
+#define VCD_MAX_SRC_BUFFER_SIZE		0x500000 /* 1920 x 1200 x 2 bpp */
+#define VCD_KVM_BW_PCLK			120000000UL
+#define VCD_BUSY_TIMEOUT_US		50000
+
+/* ECE Registers */
+#define ECE_DDA_CTRL			0x0000
+#define  ECE_DDA_CTRL_ECEEN		BIT(0)
+#define  ECE_DDA_CTRL_INTEN		BIT(8)
+
+#define ECE_DDA_STS			0x0004
+#define  ECE_DDA_STS_CDREADY		BIT(8)
+#define  ECE_DDA_STS_ACDRDY		BIT(10)
+
+#define ECE_FBR_BA			0x0008
+#define ECE_ED_BA			0x000c
+#define ECE_RECT_XY			0x0010
+
+#define ECE_RECT_DIMEN			0x0014
+#define  ECE_RECT_DIMEN_WR		GENMASK(10, 0)
+#define  ECE_RECT_DIMEN_WLTR		GENMASK(14, 11)
+#define  ECE_RECT_DIMEN_HR		GENMASK(26, 16)
+#define  ECE_RECT_DIMEN_HLTR		GENMASK(30, 27)
+
+#define ECE_RESOL			0x001c
+#define  ECE_RESOL_FB_LP_512		0
+#define  ECE_RESOL_FB_LP_1024		1
+#define  ECE_RESOL_FB_LP_2048		2
+#define  ECE_RESOL_FB_LP_2560		3
+#define  ECE_RESOL_FB_LP_4096		4
+
+#define ECE_HEX_CTRL			0x0040
+#define  ECE_HEX_CTRL_ENCDIS		BIT(0)
+#define  ECE_HEX_CTRL_ENC_GAP		GENMASK(12, 8)
+
+#define ECE_HEX_RECT_OFFSET		0x0048
+#define  ECE_HEX_RECT_OFFSET_MASK	GENMASK(22, 0)
+
+#define ECE_TILE_W			16
+#define ECE_TILE_H			16
+#define ECE_POLL_TIMEOUT_US		50000
+
+/* GCR Registers */
+#define INTCR				0x3c
+#define  INTCR_GFXIFDIS			GENMASK(9, 8)
+#define  INTCR_DEHS			BIT(27)
+
+#define INTCR2				0x60
+#define  INTCR2_GIRST2			BIT(2)
+#define  INTCR2_GIHCRST			BIT(5)
+#define  INTCR2_GIVCRST			BIT(6)
+
+#define INTCR3				0x9c
+#define  INTCR3_GMMAP			GENMASK(10, 8)
+#define   INTCR3_GMMAP_128MB		0
+#define   INTCR3_GMMAP_256MB		1
+#define   INTCR3_GMMAP_512MB		2
+#define   INTCR3_GMMAP_1GB		3
+#define   INTCR3_GMMAP_2GB		4
+
+#define ADDR_GMMAP_128MB		0x07000000
+#define ADDR_GMMAP_256MB		0x0f000000
+#define ADDR_GMMAP_512MB		0x1f000000
+#define ADDR_GMMAP_1GB			0x3f000000
+#define ADDR_GMMAP_2GB			0x7f000000
+
+#define GMMAP_LENGTH			0xc00000 /* Total 16MB, but 4MB preserved*/
+
+#define MFSEL1				0x0c
+#define  MFSEL1_DVH1SEL			BIT(27)
+
+/* GFXI Register */
+#define DISPST				0x00
+#define  DISPST_HSCROFF			BIT(1)
+#define  DISPST_MGAMODE			BIT(7)
+
+#define HVCNTL				0x10
+#define  HVCNTL_MASK			GENMASK(7, 0)
+
+#define HVCNTH				0x14
+#define  HVCNTH_MASK			GENMASK(2, 0)
+
+#define VVCNTL				0x20
+#define  VVCNTL_MASK			GENMASK(7, 0)
+
+#define VVCNTH				0x24
+#define  VVCNTH_MASK			GENMASK(2, 0)
+
+#define GPLLINDIV			0x40
+#define  GPLLINDIV_MASK			GENMASK(5, 0)
+#define  GPLLINDIV_GPLLFBDV8		BIT(7)
+
+#define GPLLFBDIV			0x44
+#define  GPLLFBDIV_MASK			GENMASK(7, 0)
+
+#define GPLLST				0x48
+#define  GPLLST_PLLOTDIV1		GENMASK(2, 0)
+#define  GPLLST_PLLOTDIV2		GENMASK(5, 3)
+#define  GPLLST_GPLLFBDV109		GENMASK(7, 6)
+
+struct nuvoton_video_addr {
+	unsigned int size;
+	dma_addr_t dma;
+	void *virt;
+};
+
+struct nuvoton_video_buffer {
+	struct vb2_v4l2_buffer vb;
+	struct list_head link;
+};
+
+#define to_nuvoton_video_buffer(x) \
+	container_of((x), struct nuvoton_video_buffer, vb)
+
+enum {
+	VIDEO_STREAMING,
+	VIDEO_FRAME_INPRG,
+	VIDEO_STOPPED,
+};
+
+struct rect_list {
+	struct v4l2_clip clip;
+	struct list_head list;
+};
+
+struct rect_list_info {
+	struct rect_list *list;
+	struct rect_list *first;
+	struct list_head *head;
+	int index;
+	int tile_perline;
+	int tile_perrow;
+	int offset_perline;
+	int tile_size;
+	int tile_cnt;
+};
+
+struct nuvoton_ece {
+	struct regmap *regmap;
+	atomic_t clients;
+	struct reset_control *reset;
+};
+
+struct nuvoton_video {
+	struct regmap *gcr_regmap;
+	struct regmap *gfx_regmap;
+	struct regmap *vcd_regmap;
+
+	struct device *dev;
+	struct v4l2_ctrl_handler ctrl_handler;
+	struct v4l2_device v4l2_dev;
+	struct v4l2_pix_format pix_fmt;
+	struct v4l2_bt_timings active_timings;
+	struct v4l2_bt_timings detected_timings;
+	u32 v4l2_input_status;
+	struct vb2_queue queue;
+	struct video_device vdev;
+	struct mutex video_lock;	/* v4l2 and videobuf2 lock */
+
+	struct list_head buffers;
+	spinlock_t lock;		/* buffer list lock */
+	unsigned long flags;
+	unsigned int sequence;
+
+	unsigned int max_buffer_size;
+	struct nuvoton_video_addr src;
+	struct reset_control *reset;
+	struct nuvoton_ece ece;
+
+	int frame_rate;
+	int vb_index;
+	u32 bytesperline;
+	u8 bytesperpixel;
+	u32 rect_cnt;
+	u8 num_buffers;
+	struct list_head *list;
+	u32 *rect;
+	int ctrl_cmd;
+	int op_cmd;
+};
+
+#define to_nuvoton_video(x) container_of((x), struct nuvoton_video, v4l2_dev)
+
+static const struct v4l2_dv_timings_cap nuvoton_video_timings_cap = {
+	.type = V4L2_DV_BT_656_1120,
+	.bt = {
+		.min_width = MIN_WIDTH,
+		.max_width = MAX_WIDTH,
+		.min_height = MIN_HEIGHT,
+		.max_height = MAX_HEIGHT,
+		.min_pixelclock = 6574080, /* 640 x 480 x 24Hz */
+		.max_pixelclock = 138240000, /* 1920 x 1200 x 60Hz */
+		.standards = V4L2_DV_BT_STD_CEA861 | V4L2_DV_BT_STD_DMT |
+			     V4L2_DV_BT_STD_CVT | V4L2_DV_BT_STD_GTF,
+		.capabilities = V4L2_DV_BT_CAP_PROGRESSIVE |
+				V4L2_DV_BT_CAP_REDUCED_BLANKING |
+				V4L2_DV_BT_CAP_CUSTOM,
+	},
+};
+
+static DECLARE_BITMAP(bitmap, BITMAP_SIZE);
+
+static void nuvoton_video_ece_prepend_rect_header(u8 *addr, u16 x, u16 y, u16 w,
+						  u16 h)
+{
+	__be16 x_pos = cpu_to_be16(x);
+	__be16 y_pos = cpu_to_be16(y);
+	__be16 width = cpu_to_be16(w);
+	__be16 height = cpu_to_be16(h);
+	__be32 encoding = cpu_to_be32(5); /* Hextile encoding */
+
+	memcpy(addr, &x_pos, 2);
+	memcpy(addr + 2, &y_pos, 2);
+	memcpy(addr + 4, &width, 2);
+	memcpy(addr + 6, &height, 2);
+	memcpy(addr + 8, &encoding, 4);
+}
+
+static u32 nuvoton_video_ece_get_ed_size(struct nuvoton_video *video,
+					 u32 offset, void *addr)
+{
+	struct regmap *ece = video->ece.regmap;
+	u32 size, gap, val;
+	int ret;
+
+	ret = regmap_read_poll_timeout(ece, ECE_DDA_STS, val,
+				       (val & ECE_DDA_STS_CDREADY),
+				       0, ECE_POLL_TIMEOUT_US);
+
+	if (ret) {
+		dev_warn(video->dev, "Wait for ECE_DDA_STS_CDREADY timeout\n");
+		return 0;
+	}
+
+	size = readl(addr + offset);
+
+	regmap_read(ece, ECE_HEX_CTRL, &val);
+	gap = FIELD_GET(ECE_HEX_CTRL_ENC_GAP, val);
+
+	dev_dbg(video->dev, "offset = %u, ed_size = %u, gap = %u\n", offset,
+		size, gap);
+
+	return size + gap;
+}
+
+static void nuvoton_video_ece_enc_rect(struct nuvoton_video *video, u32 r_off_x,
+				       u32 r_off_y, u32 r_w, u32 r_h)
+{
+	struct regmap *ece = video->ece.regmap;
+	u32 rect_offset = (r_off_y * video->bytesperline) + (r_off_x * 2);
+	u32 temp;
+	u32 w_tile;
+	u32 h_tile;
+	u32 w_size = ECE_TILE_W;
+	u32 h_size = ECE_TILE_H;
+
+	regmap_update_bits(ece, ECE_DDA_CTRL, ECE_DDA_CTRL_ECEEN, 0);
+	regmap_update_bits(ece, ECE_DDA_CTRL, ECE_DDA_CTRL_ECEEN,
+			   ECE_DDA_CTRL_ECEEN);
+	regmap_write(ece, ECE_DDA_STS, ECE_DDA_STS_CDREADY |
+		     ECE_DDA_STS_ACDRDY);
+	regmap_write(ece, ECE_RECT_XY, rect_offset);
+
+	w_tile = r_w / ECE_TILE_W;
+	h_tile = r_h / ECE_TILE_H;
+
+	if (r_w % ECE_TILE_W) {
+		w_tile += 1;
+		w_size = r_w % ECE_TILE_W;
+	}
+
+	if (r_h % ECE_TILE_H || !h_tile) {
+		h_tile += 1;
+		h_size = r_h % ECE_TILE_H;
+	}
+
+	temp = FIELD_PREP(ECE_RECT_DIMEN_WLTR, w_size - 1) |
+	       FIELD_PREP(ECE_RECT_DIMEN_HLTR, h_size - 1) |
+	       FIELD_PREP(ECE_RECT_DIMEN_WR, w_tile - 1) |
+	       FIELD_PREP(ECE_RECT_DIMEN_HR, h_tile - 1);
+
+	regmap_write(ece, ECE_RECT_DIMEN, temp);
+}
+
+static u32 nuvoton_video_ece_read_rect_offset(struct nuvoton_video *video)
+{
+	struct regmap *ece = video->ece.regmap;
+	u32 offset;
+
+	regmap_read(ece, ECE_HEX_RECT_OFFSET, &offset);
+	return FIELD_GET(ECE_HEX_RECT_OFFSET_MASK, offset);
+}
+
+/*
+ * Set the line pitch (in bytes) for the frame buffers.
+ * Can be on of those values: 512, 1024, 2048, 2560 or 4096 bytes.
+ */
+static void nuvoton_video_ece_set_lp(struct nuvoton_video *video, u32 pitch)
+{
+	u32 lp;
+	struct regmap *ece = video->ece.regmap;
+
+	switch (pitch) {
+	case 512:
+		lp = ECE_RESOL_FB_LP_512;
+		break;
+	case 1024:
+		lp = ECE_RESOL_FB_LP_1024;
+		break;
+	case 2048:
+		lp = ECE_RESOL_FB_LP_2048;
+		break;
+	case 2560:
+		lp = ECE_RESOL_FB_LP_2560;
+		break;
+	case 4096:
+		lp = ECE_RESOL_FB_LP_4096;
+		break;
+	default:
+		return;
+	}
+
+	regmap_write(ece, ECE_RESOL, lp);
+}
+
+static void nuvoton_video_ece_set_fb_addr(struct nuvoton_video *video,
+					  u32 buffer)
+{
+	struct regmap *ece = video->ece.regmap;
+
+	regmap_write(ece, ECE_FBR_BA, buffer);
+}
+
+static void nuvoton_video_ece_set_enc_dba(struct nuvoton_video *video, u32 addr)
+{
+	struct regmap *ece = video->ece.regmap;
+
+	regmap_write(ece, ECE_ED_BA, addr);
+}
+
+static void nuvoton_video_ece_clear_rect_offset(struct nuvoton_video *video)
+{
+	struct regmap *ece = video->ece.regmap;
+
+	regmap_write(ece, ECE_HEX_RECT_OFFSET, 0);
+}
+
+static void nuvoton_video_ece_ctrl_reset(struct nuvoton_video *video)
+{
+	struct regmap *ece = video->ece.regmap;
+
+	regmap_update_bits(ece, ECE_DDA_CTRL, ECE_DDA_CTRL_ECEEN, 0);
+	regmap_update_bits(ece, ECE_HEX_CTRL, ECE_HEX_CTRL_ENCDIS,
+			   ECE_HEX_CTRL_ENCDIS);
+	regmap_update_bits(ece, ECE_DDA_CTRL, ECE_DDA_CTRL_ECEEN,
+			   ECE_DDA_CTRL_ECEEN);
+	regmap_update_bits(ece, ECE_HEX_CTRL, ECE_HEX_CTRL_ENCDIS, 0);
+
+	nuvoton_video_ece_clear_rect_offset(video);
+}
+
+static void nuvoton_video_ece_ip_reset(struct nuvoton_video *video)
+{
+	reset_control_assert(video->ece.reset);
+	msleep(100);
+	reset_control_deassert(video->ece.reset);
+	msleep(100);
+}
+
+static int nuvoton_video_ece_init(struct nuvoton_video *video)
+{
+	nuvoton_video_ece_ip_reset(video);
+	nuvoton_video_ece_ctrl_reset(video);
+
+	return 0;
+}
+
+static int nuvoton_video_ece_stop(struct nuvoton_video *video)
+{
+	struct regmap *ece = video->ece.regmap;
+
+	regmap_update_bits(ece, ECE_DDA_CTRL, ECE_DDA_CTRL_ECEEN, 0);
+	regmap_update_bits(ece, ECE_DDA_CTRL, ECE_DDA_CTRL_INTEN, 0);
+	regmap_update_bits(ece, ECE_HEX_CTRL, ECE_HEX_CTRL_ENCDIS,
+			   ECE_HEX_CTRL_ENCDIS);
+	nuvoton_video_ece_clear_rect_offset(video);
+
+	return 0;
+}
+
+static bool nuvoton_video_alloc_buf(struct nuvoton_video *video,
+				    struct nuvoton_video_addr *addr,
+				    unsigned int size)
+{
+	if (size > VCD_MAX_SRC_BUFFER_SIZE)
+		size = VCD_MAX_SRC_BUFFER_SIZE;
+
+	addr->virt = dma_alloc_coherent(video->dev, size, &addr->dma,
+					GFP_KERNEL);
+
+	if (!addr->virt)
+		return false;
+
+	addr->size = size;
+	return true;
+}
+
+static void nuvoton_video_free_buf(struct nuvoton_video *video,
+				   struct nuvoton_video_addr *addr)
+{
+	dma_free_coherent(video->dev, addr->size, addr->virt, addr->dma);
+	addr->size = 0;
+	addr->dma = 0ULL;
+	addr->virt = NULL;
+}
+
+static void nuvoton_video_free_diff_table(struct nuvoton_video *video)
+{
+	struct list_head *head, *pos, *nx;
+	struct rect_list *tmp;
+	int i;
+
+	for (i = 0; i < video->num_buffers; i++) {
+		head = &video->list[i];
+		list_for_each_safe(pos, nx, head) {
+			tmp = list_entry(pos, struct rect_list, list);
+			if (tmp) {
+				list_del(&tmp->list);
+				kfree(tmp);
+			}
+		}
+	}
+}
+
+static int nuvoton_video_add_rect(struct nuvoton_video *video, int index,
+				  u32 x, u32 y, u32 w, u32 h)
+{
+	struct list_head *head = &video->list[index];
+	struct rect_list *list = NULL;
+	struct v4l2_rect *r;
+
+	list = kzalloc(sizeof(*list), GFP_KERNEL);
+	if (!list)
+		return 0;
+
+	r = &list->clip.c;
+	r->left = x;
+	r->top = y;
+	r->width = w;
+	r->height = h;
+
+	list_add_tail(&list->list, head);
+	return 1;
+}
+
+static void nuvoton_video_merge_rect(struct nuvoton_video *video,
+				     struct rect_list_info *info)
+{
+	struct list_head *head = info->head;
+	struct rect_list *list = info->list;
+	struct rect_list *first = info->first;
+	struct v4l2_rect *r = &list->clip.c;
+	struct v4l2_rect *f = &first->clip.c;
+
+	if (!first) {
+		first = list;
+		info->first = first;
+		list_add_tail(&list->list, head);
+		video->rect_cnt++;
+	} else {
+		if ((r->left == (f->left + f->width)) && r->top == f->top) {
+			f->width += r->width;
+			kfree(list);
+		} else if ((r->top == (f->top + f->height)) &&
+			   (r->left == f->left)) {
+			f->height += r->height;
+			kfree(list);
+		} else if (((r->top > f->top) &&
+			   (r->top < (f->top + f->height))) &&
+			   ((r->left > f->left) &&
+			   (r->left < (f->left + f->width)))) {
+			kfree(list);
+		} else {
+			list_add_tail(&list->list, head);
+			video->rect_cnt++;
+			info->first = list;
+		}
+	}
+}
+
+static struct rect_list *nuvoton_video_new_rect(struct nuvoton_video *video,
+						int offset, int index)
+{
+	struct v4l2_bt_timings *act = &video->active_timings;
+	struct rect_list *list = NULL;
+	struct v4l2_rect *r;
+
+	list = kzalloc(sizeof(*list), GFP_KERNEL);
+	if (!list)
+		return NULL;
+
+	r = &list->clip.c;
+
+	r->left = (offset << 4);
+	r->top = (index >> 2);
+	r->width = RECT_W;
+	r->height = RECT_H;
+	if ((r->left + RECT_W) > act->width)
+		r->width = act->width - r->left;
+	if ((r->top + RECT_H) > act->height)
+		r->height = act->height - r->top;
+
+	return list;
+}
+
+static int nuvoton_video_find_rect(struct nuvoton_video *video,
+				   struct rect_list_info *info, u32 offset)
+{
+	int i = info->index;
+
+	if (offset < info->tile_perline) {
+		info->list = nuvoton_video_new_rect(video, offset, i);
+		if (!info->list)
+			return -ENOMEM;
+
+		nuvoton_video_merge_rect(video, info);
+	}
+	return 0;
+}
+
+static int nuvoton_video_build_table(struct nuvoton_video *video,
+				     struct rect_list_info *info)
+{
+	int i = info->index;
+	int j, ret, bit;
+	u32 value;
+	struct regmap *vcd = video->vcd_regmap;
+
+	for (j = 0; j < info->offset_perline; j += 4) {
+		regmap_read(vcd, VCD_DIFF_TBL + (j + i), &value);
+
+		bitmap_from_arr32(bitmap, &value, BITMAP_SIZE);
+
+		for_each_set_bit(bit, bitmap, BITMAP_SIZE) {
+			ret = nuvoton_video_find_rect(video, info,
+						      bit + (j << 3));
+			if (ret < 0)
+				return ret;
+		}
+	}
+	info->index += 64;
+	return info->tile_perline;
+}
+
+static int nuvoton_video_get_rect_list(struct nuvoton_video *video, int index)
+{
+	struct v4l2_bt_timings *act = &video->active_timings;
+	struct rect_list_info info;
+	int ret = 0;
+	u32 mod, tile_cnt = 0;
+
+	memset(&info, 0, sizeof(struct rect_list_info));
+	info.head = &video->list[index];
+
+	info.tile_perline = act->width >> 4;
+	mod = act->width % RECT_W;
+	if (mod != 0)
+		info.tile_perline += 1;
+
+	info.tile_perrow = act->height >> 4;
+	mod = act->height % RECT_H;
+	if (mod != 0)
+		info.tile_perrow += 1;
+
+	info.tile_size = info.tile_perrow * info.tile_perline;
+
+	info.offset_perline = info.tile_perline >> 5;
+	mod = info.tile_perline % 32;
+	if (mod != 0)
+		info.offset_perline += 1;
+
+	info.offset_perline *= 4;
+
+	do {
+		ret = nuvoton_video_build_table(video, &info);
+		if (ret < 0)
+			return ret;
+		tile_cnt += ret;
+	} while (tile_cnt < info.tile_size);
+
+	return ret;
+}
+
+static u8 nuvoton_video_is_mga(struct nuvoton_video *video)
+{
+	struct regmap *gfxi = video->gfx_regmap;
+	u32 dispst;
+
+	regmap_read(gfxi, DISPST, &dispst);
+	return ((dispst & DISPST_MGAMODE) == DISPST_MGAMODE);
+}
+
+static u32 nuvoton_video_hres(struct nuvoton_video *video)
+{
+	struct regmap *gfxi = video->gfx_regmap;
+	u32 hvcnth, hvcntl, apb_hor_res;
+
+	regmap_read(gfxi, HVCNTH, &hvcnth);
+	regmap_read(gfxi, HVCNTL, &hvcntl);
+	apb_hor_res = (((hvcnth & HVCNTH_MASK) << 8) +
+		       (hvcntl & HVCNTL_MASK) + 1);
+
+	return apb_hor_res;
+}
+
+static u32 nuvoton_video_vres(struct nuvoton_video *video)
+{
+	struct regmap *gfxi = video->gfx_regmap;
+	u32 vvcnth, vvcntl, apb_ver_res;
+
+	regmap_read(gfxi, VVCNTH, &vvcnth);
+	regmap_read(gfxi, VVCNTL, &vvcntl);
+
+	apb_ver_res = (((vvcnth & VVCNTH_MASK) << 8) + (vvcntl & VVCNTL_MASK));
+
+	return apb_ver_res;
+}
+
+static int nuvoton_video_capres(struct nuvoton_video *video, u32 hor_res,
+				u32 vert_res)
+{
+	struct regmap *vcd = video->vcd_regmap;
+	u32 res, cap_res;
+
+	if (hor_res > MAX_WIDTH || vert_res > MAX_HEIGHT)
+		return -EINVAL;
+
+	res = FIELD_PREP(VCD_CAP_RES_VERT_RES, vert_res) |
+	      FIELD_PREP(VCD_CAP_RES_HOR_RES, hor_res);
+
+	regmap_write(vcd, VCD_CAP_RES, res);
+	regmap_read(vcd, VCD_CAP_RES, &cap_res);
+
+	if (cap_res != res)
+		return -EINVAL;
+
+	return 0;
+}
+
+static void nuvoton_video_vcd_ip_reset(struct nuvoton_video *video)
+{
+	reset_control_assert(video->reset);
+	msleep(100);
+	reset_control_deassert(video->reset);
+	msleep(100);
+}
+
+static void nuvoton_video_vcd_state_machine_reset(struct nuvoton_video *video)
+{
+	struct regmap *vcd = video->vcd_regmap;
+	u32 stat;
+	int ret;
+
+	regmap_update_bits(vcd, VCD_MODE, VCD_MODE_VCDE, 0);
+
+	regmap_update_bits(vcd, VCD_MODE, VCD_MODE_IDBC, 0);
+
+	regmap_update_bits(vcd, VCD_CMD, VCD_CMD_RST, VCD_CMD_RST);
+
+	ret = regmap_read_poll_timeout(vcd, VCD_STAT, stat,
+				       (stat & VCD_STAT_DONE), 0,
+				       ECE_POLL_TIMEOUT_US);
+
+	if (ret) {
+		dev_warn(video->dev, "Wait for VCD_STAT_DONE timeout\n");
+		return;
+	}
+
+	regmap_write(vcd, VCD_STAT, VCD_STAT_CLEAR);
+
+	regmap_update_bits(vcd, VCD_MODE, VCD_MODE_VCDE, VCD_MODE_VCDE);
+
+	regmap_update_bits(vcd, VCD_MODE, VCD_MODE_IDBC, VCD_MODE_IDBC);
+}
+
+static int nuvoton_video_gfx_reset(struct nuvoton_video *video)
+{
+	struct regmap *gcr = video->gcr_regmap;
+
+	regmap_update_bits(gcr, INTCR2, INTCR2_GIRST2, INTCR2_GIRST2);
+
+	nuvoton_video_vcd_state_machine_reset(video);
+
+	regmap_update_bits(gcr, INTCR2, INTCR2_GIRST2, 0);
+
+	return 0;
+}
+
+static void nuvoton_video_kvm_bw(struct nuvoton_video *video, u8 bandwidth)
+{
+	struct regmap *vcd = video->vcd_regmap;
+
+	if (!nuvoton_video_is_mga(video))
+		bandwidth = 1;
+
+	if (bandwidth)
+		regmap_update_bits(vcd, VCD_MODE, VCD_MODE_KVM_BW_SET,
+				   VCD_MODE_KVM_BW_SET);
+	else
+		regmap_update_bits(vcd, VCD_MODE, VCD_MODE_KVM_BW_SET, 0);
+}
+
+static u32 nuvoton_video_pclk(struct nuvoton_video *video)
+{
+	struct regmap *gfxi = video->gfx_regmap;
+	u32 tmp, pllfbdiv, pllinotdiv, gpllfbdiv;
+	u8 gpllfbdv109, gpllfbdv8, gpllindiv;
+	u8 gpllst_pllotdiv1, gpllst_pllotdiv2;
+
+	regmap_read(gfxi, GPLLST, &tmp);
+	gpllfbdv109 = FIELD_GET(GPLLST_GPLLFBDV109, tmp);
+	gpllst_pllotdiv1 = FIELD_GET(GPLLST_PLLOTDIV1, tmp);
+	gpllst_pllotdiv2 = FIELD_GET(GPLLST_PLLOTDIV2, tmp);
+
+	regmap_read(gfxi, GPLLINDIV, &tmp);
+	gpllfbdv8 = FIELD_GET(GPLLINDIV_GPLLFBDV8, tmp);
+	gpllindiv = FIELD_GET(GPLLINDIV_MASK, tmp);
+
+	regmap_read(gfxi, GPLLFBDIV, &tmp);
+	gpllfbdiv = FIELD_GET(GPLLFBDIV_MASK, tmp);
+
+	pllfbdiv = (512 * gpllfbdv109 + 256 * gpllfbdv8 + gpllfbdiv);
+	pllinotdiv = (gpllindiv * gpllst_pllotdiv1 * gpllst_pllotdiv2);
+	if (pllfbdiv == 0 || pllinotdiv == 0)
+		return 0;
+
+	return ((pllfbdiv * 25000) / pllinotdiv) * 1000;
+}
+
+static int nuvoton_video_get_bpp(struct nuvoton_video *video)
+{
+	struct regmap *vcd = video->vcd_regmap;
+	u32 mode, color_cnvr;
+
+	regmap_read(vcd, VCD_MODE, &mode);
+
+	color_cnvr = FIELD_GET(VCD_MODE_COLOR_CNVRT, mode);
+
+	switch (color_cnvr) {
+	case VCD_MODE_COLOR_CNVRT_NO_CNVRT:
+		return 2;
+	case VCD_MODE_COLOR_CNVRT_RGB_222:
+	case VCD_MODE_COLOR_CNVRT_666_MODE:
+		return 1;
+	case VCD_MODE_COLOR_CNVRT_RGB_888:
+		return 4;
+	}
+	return 0;
+}
+
+/*
+ * Pitch must be a power of 2, >= linebytes,
+ * at least 512, and no more than 4096.
+ */
+static void nuvoton_video_set_linepitch(struct nuvoton_video *video,
+					u32 linebytes)
+{
+	struct regmap *vcd = video->vcd_regmap;
+	u32 pitch = MIN_LP;
+
+	while ((pitch < linebytes) && (pitch < MAX_LP))
+		pitch *= 2;
+
+	regmap_write(vcd, VCD_FB_LP, FIELD_PREP(VCD_FBA_LP, pitch) |
+		     FIELD_PREP(VCD_FBB_LP, pitch));
+}
+
+static u32 nuvoton_video_get_linepitch(struct nuvoton_video *video)
+{
+	struct regmap *vcd = video->vcd_regmap;
+	u32 linepitch;
+
+	regmap_read(vcd, VCD_FB_LP, &linepitch);
+
+	return FIELD_GET(VCD_FBA_LP, linepitch);
+}
+
+static int nuvoton_video_command(struct nuvoton_video *video, u32 value)
+{
+	struct regmap *vcd = video->vcd_regmap;
+	u32 cmd;
+
+	regmap_write(vcd, VCD_STAT, VCD_STAT_CLEAR);
+
+	regmap_read(vcd, VCD_CMD, &cmd);
+	cmd |= FIELD_PREP(VCD_CMD_OPERATION, value);
+
+	regmap_write(vcd, VCD_CMD, cmd);
+	regmap_update_bits(vcd, VCD_CMD, VCD_CMD_GO, VCD_CMD_GO);
+	video->op_cmd = value;
+
+	return 0;
+}
+
+static int nuvoton_video_init_reg(struct nuvoton_video *video)
+{
+	struct regmap *gcr = video->gcr_regmap;
+	struct regmap *vcd = video->vcd_regmap;
+
+	/* Selects Data Enable */
+	regmap_update_bits(gcr, INTCR, INTCR_DEHS, 0);
+
+	/* Enable display of KVM GFX and access to memory */
+	regmap_update_bits(gcr, INTCR, INTCR_GFXIFDIS, 0);
+
+	/* Active Vertical/Horizontal Counters Reset */
+	regmap_update_bits(gcr, INTCR2, INTCR2_GIHCRST | INTCR2_GIVCRST,
+			   INTCR2_GIHCRST | INTCR2_GIVCRST);
+
+	/* Select KVM GFX input */
+	regmap_update_bits(gcr, MFSEL1, MFSEL1_DVH1SEL, 0);
+
+	/* Reset video modules */
+	nuvoton_video_vcd_ip_reset(video);
+	nuvoton_video_gfx_reset(video);
+
+	/* Set the FIFO thresholds */
+	regmap_write(vcd, VCD_FIFO, VCD_FIFO_TH);
+
+	/* Set video mode */
+	regmap_update_bits(vcd, VCD_MODE, 0xffffffff,
+			   VCD_MODE_VCDE | VCD_MODE_CM565 |
+			   VCD_MODE_IDBC | VCD_MODE_KVM_BW_SET);
+
+	regmap_write(vcd, VCD_RCHG, FIELD_PREP(VCD_RCHG_TIM_PRSCL, 0xf));
+
+	return 0;
+}
+
+static int nuvoton_video_start_frame(struct nuvoton_video *video)
+{
+	unsigned long flags;
+	struct nuvoton_video_buffer *buf;
+	struct regmap *vcd = video->vcd_regmap;
+	u32 val;
+	int ret;
+
+	dev_dbg(video->dev, "%s\n", __func__);
+
+	if (video->v4l2_input_status) {
+		dev_dbg(video->dev, "No video signal; skip capture frame\n");
+		return 0;
+	}
+
+	ret = regmap_read_poll_timeout(vcd, VCD_STAT, val,
+				       !(val & VCD_STAT_BUSY), 1000,
+				       VCD_BUSY_TIMEOUT_US);
+
+	if (ret) {
+		dev_err(video->dev, "Wait for VCD_STAT_BUSY timeout\n");
+		return -EBUSY;
+	}
+
+	spin_lock_irqsave(&video->lock, flags);
+	buf = list_first_entry_or_null(&video->buffers,
+				       struct nuvoton_video_buffer, link);
+	if (!buf) {
+		spin_unlock_irqrestore(&video->lock, flags);
+		dev_dbg(video->dev, "No empty buffers; skip capture frame\n");
+		return 0;
+	}
+
+	set_bit(VIDEO_FRAME_INPRG, &video->flags);
+	spin_unlock_irqrestore(&video->lock, flags);
+
+	nuvoton_video_vcd_state_machine_reset(video);
+
+	regmap_update_bits(vcd, VCD_INTE, VCD_INTE_DONE_IE | VCD_INTE_IFOT_IE |
+			   VCD_INTE_IFOR_IE, VCD_INTE_DONE_IE |
+			   VCD_INTE_IFOT_IE | VCD_INTE_IFOR_IE);
+
+	nuvoton_video_command(video, video->ctrl_cmd);
+
+	return 0;
+}
+
+static void nuvoton_video_bufs_done(struct nuvoton_video *video,
+				    enum vb2_buffer_state state)
+{
+	unsigned long flags;
+	struct nuvoton_video_buffer *buf;
+
+	spin_lock_irqsave(&video->lock, flags);
+	list_for_each_entry(buf, &video->buffers, link)
+		vb2_buffer_done(&buf->vb.vb2_buf, state);
+	INIT_LIST_HEAD(&video->buffers);
+	spin_unlock_irqrestore(&video->lock, flags);
+}
+
+static void nuvoton_video_get_diff_rect(struct nuvoton_video *video, int index)
+{
+	u32 width = video->active_timings.width;
+	u32 height = video->active_timings.height;
+
+	if (video->op_cmd != VCD_CMD_OPERATION_CAPTURE) {
+		video->rect_cnt = 0;
+		nuvoton_video_get_rect_list(video, index);
+		video->rect[index] = video->rect_cnt;
+	} else {
+		video->rect[index] = nuvoton_video_add_rect(video, index, 0, 0,
+							    width, height);
+	}
+}
+
+static irqreturn_t nuvoton_video_irq(int irq, void *arg)
+{
+	struct nuvoton_video *video = arg;
+	struct regmap *vcd = video->vcd_regmap;
+	struct nuvoton_video_buffer *buf;
+	struct rect_list *rect_list;
+	struct v4l2_rect *rect;
+	u32 status, ed_offset, ed_size, total_size;
+	void *addr;
+	dma_addr_t vb_dma_addr;
+	int index;
+
+	regmap_read(vcd, VCD_STAT, &status);
+	dev_dbg(video->dev, "VCD irq status 0x%x\n", status);
+
+	regmap_write(vcd, VCD_INTE, 0);
+	regmap_write(vcd, VCD_STAT, VCD_STAT_CLEAR);
+
+	if (test_bit(VIDEO_STOPPED, &video->flags) ||
+	    !test_bit(VIDEO_STREAMING, &video->flags)) {
+		clear_bit(VIDEO_FRAME_INPRG, &video->flags);
+		return IRQ_NONE;
+	}
+
+	if (status & VCD_STAT_DONE) {
+		spin_lock(&video->lock);
+		buf = list_first_entry_or_null(&video->buffers,
+					       struct nuvoton_video_buffer,
+					       link);
+
+		if (!buf) {
+			spin_unlock(&video->lock);
+			clear_bit(VIDEO_FRAME_INPRG, &video->flags);
+			return IRQ_NONE;
+		}
+
+		addr = vb2_plane_vaddr(&buf->vb.vb2_buf, 0);
+		vb_dma_addr = vb2_dma_contig_plane_dma_addr(&buf->vb.vb2_buf, 0);
+		index = buf->vb.vb2_buf.index;
+
+		nuvoton_video_ece_ctrl_reset(video);
+		nuvoton_video_ece_clear_rect_offset(video);
+
+		nuvoton_video_ece_set_fb_addr(video, video->src.dma);
+
+		/* Set base address of encoded data to video buffer */
+		nuvoton_video_ece_set_enc_dba(video, vb_dma_addr);
+
+		nuvoton_video_ece_set_lp(video, video->bytesperline);
+		nuvoton_video_get_diff_rect(video, index);
+
+		total_size = 0;
+
+		list_for_each_entry(rect_list, &video->list[index], list) {
+			rect = &rect_list->clip.c;
+			ed_offset = nuvoton_video_ece_read_rect_offset(video);
+
+			nuvoton_video_ece_enc_rect(video, rect->left,
+						   rect->top, rect->width,
+						   rect->height);
+			ed_size = nuvoton_video_ece_get_ed_size(video,
+								ed_offset,
+								addr);
+
+			nuvoton_video_ece_prepend_rect_header(addr + ed_offset,
+							      rect->left,
+							      rect->top,
+							      rect->width,
+							      rect->height);
+
+			total_size += ed_size;
+		}
+
+		vb2_set_plane_payload(&buf->vb.vb2_buf, 0, total_size);
+		buf->vb.vb2_buf.timestamp = ktime_get_ns();
+		buf->vb.sequence = video->sequence++;
+		buf->vb.field = V4L2_FIELD_NONE;
+		vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_DONE);
+		list_del(&buf->link);
+
+		spin_unlock(&video->lock);
+
+		clear_bit(VIDEO_FRAME_INPRG, &video->flags);
+	}
+
+	if (status & VCD_STAT_IFOR || status & VCD_STAT_IFOT) {
+		dev_warn(video->dev, "VCD FIFO overrun or over thresholds\n");
+		nuvoton_video_vcd_ip_reset(video);
+		nuvoton_video_gfx_reset(video);
+		nuvoton_video_start_frame(video);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static void nuvoton_video_clear_gmmap(struct nuvoton_video *video)
+{
+	struct regmap *gcr = video->gcr_regmap;
+	u32 intcr3, gmmap;
+	void __iomem *baseptr;
+
+	regmap_read(gcr, INTCR3, &intcr3);
+	gmmap = FIELD_GET(INTCR3_GMMAP, intcr3);
+
+	switch (gmmap) {
+	case INTCR3_GMMAP_128MB:
+		baseptr = ioremap(ADDR_GMMAP_128MB, GMMAP_LENGTH);
+		break;
+	case INTCR3_GMMAP_256MB:
+		baseptr = ioremap(ADDR_GMMAP_256MB, GMMAP_LENGTH);
+		break;
+	case INTCR3_GMMAP_512MB:
+		baseptr = ioremap(ADDR_GMMAP_512MB, GMMAP_LENGTH);
+		break;
+	case INTCR3_GMMAP_1GB:
+		baseptr = ioremap(ADDR_GMMAP_1GB, GMMAP_LENGTH);
+		break;
+	case INTCR3_GMMAP_2GB:
+		baseptr = ioremap(ADDR_GMMAP_2GB, GMMAP_LENGTH);
+		break;
+	}
+
+	memset(baseptr, 0, GMMAP_LENGTH);
+	iounmap(baseptr);
+}
+
+static void nuvoton_video_get_resolution(struct nuvoton_video *video)
+{
+	struct v4l2_bt_timings *act = &video->active_timings;
+	struct v4l2_bt_timings *det = &video->detected_timings;
+	struct regmap *gfxi;
+	u32 dispst;
+
+	video->v4l2_input_status = 0;
+
+	det->width = nuvoton_video_hres(video);
+	det->height = nuvoton_video_vres(video);
+
+	if (act->width != det->width || act->height != det->height) {
+		dev_dbg(video->dev, "Resolution changed\n");
+
+		nuvoton_video_bufs_done(video, VB2_BUF_STATE_ERROR);
+
+		if (nuvoton_video_hres(video) > 0 &&
+		    nuvoton_video_vres(video) > 0) {
+			//struct regmap *gfxi = video->gfx_regmap;
+			gfxi = video->gfx_regmap;
+			//u32 dispst;
+
+			if (test_bit(VIDEO_STREAMING, &video->flags)) {
+				/*
+				 * Wait for resolution is available,
+				 * and it is also captured by host.
+				 */
+				do {
+					mdelay(100);
+					regmap_read(gfxi, DISPST, &dispst);
+				} while (nuvoton_video_vres(video) < 100 ||
+					 nuvoton_video_pclk(video) == 0 ||
+					 (dispst & DISPST_HSCROFF));
+			}
+
+			det->width = nuvoton_video_hres(video);
+			det->height = nuvoton_video_vres(video);
+			det->pixelclock = nuvoton_video_pclk(video);
+		}
+	}
+
+	if (det->width == 0 || det->height == 0) {
+		det->width = MIN_WIDTH;
+		det->height = MIN_HEIGHT;
+		nuvoton_video_clear_gmmap(video);
+		video->v4l2_input_status = V4L2_IN_ST_NO_SIGNAL;
+	}
+
+	dev_dbg(video->dev, "Got resolution[%dx%d] -> [%dx%d], status %d\n",
+		act->width, act->height, det->width, det->height,
+		video->v4l2_input_status);
+}
+
+static void nuvoton_video_set_resolution(struct nuvoton_video *video)
+{
+	struct v4l2_bt_timings *act = &video->active_timings;
+	struct regmap *vcd = video->vcd_regmap;
+	u32 mode;
+
+	/* Set video frame physical address */
+	regmap_write(vcd, VCD_FBA_ADR, video->src.dma);
+	regmap_write(vcd, VCD_FBB_ADR, video->src.dma);
+
+	nuvoton_video_capres(video, act->width, act->height);
+
+	video->bytesperpixel = nuvoton_video_get_bpp(video);
+	nuvoton_video_set_linepitch(video, act->width * video->bytesperpixel);
+
+	video->bytesperline = nuvoton_video_get_linepitch(video);
+
+	nuvoton_video_kvm_bw(video, act->pixelclock > VCD_KVM_BW_PCLK);
+
+	nuvoton_video_gfx_reset(video);
+
+	regmap_read(vcd, VCD_MODE, &mode);
+
+	clear_bit(VIDEO_FRAME_INPRG, &video->flags);
+
+	dev_dbg(video->dev, "VCD mode = 0x%x, %s mode\n", mode,
+		nuvoton_video_is_mga(video) ? "Hi Res" : "VGA");
+
+	dev_dbg(video->dev,
+		"Digital mode: %d x %d x %d, pixelclock %lld, bytesperline %d\n",
+		act->width, act->height, video->bytesperpixel, act->pixelclock,
+		video->bytesperline);
+}
+
+static int nuvoton_video_start(struct nuvoton_video *video)
+{
+	int rc;
+
+	dev_dbg(video->dev, "%s\n", __func__);
+
+	rc = nuvoton_video_init_reg(video);
+	if (rc)
+		return rc;
+
+	nuvoton_video_get_resolution(video);
+
+	video->active_timings = video->detected_timings;
+
+	video->max_buffer_size = VCD_MAX_SRC_BUFFER_SIZE;
+	if (!nuvoton_video_alloc_buf(video, &video->src,
+				     video->max_buffer_size))
+		return -ENOMEM;
+
+	nuvoton_video_set_resolution(video);
+
+	video->pix_fmt.width = video->active_timings.width;
+	video->pix_fmt.height = video->active_timings.height;
+	video->pix_fmt.sizeimage = video->max_buffer_size;
+	video->pix_fmt.bytesperline = video->bytesperline;
+
+	if (atomic_inc_return(&video->ece.clients) == 1) {
+		nuvoton_video_ece_init(video);
+		nuvoton_video_ece_set_fb_addr(video, video->src.dma);
+		nuvoton_video_ece_set_lp(video, video->bytesperline);
+
+		dev_dbg(video->dev, "ECE open: client %d\n",
+			atomic_read(&video->ece.clients));
+	}
+
+	return 0;
+}
+
+static void nuvoton_video_stop(struct nuvoton_video *video)
+{
+	unsigned long flags;
+	struct regmap *vcd = video->vcd_regmap;
+
+	dev_dbg(video->dev, "%s\n", __func__);
+
+	spin_lock_irqsave(&video->lock, flags);
+	set_bit(VIDEO_STOPPED, &video->flags);
+	spin_unlock_irqrestore(&video->lock, flags);
+
+	regmap_write(vcd, VCD_INTE, 0);
+	regmap_write(vcd, VCD_MODE, 0);
+	regmap_write(vcd, VCD_RCHG, 0);
+	regmap_write(vcd, VCD_STAT, VCD_STAT_CLEAR);
+
+	if (video->src.size)
+		nuvoton_video_free_buf(video, &video->src);
+
+	if (video->list)
+		nuvoton_video_free_diff_table(video);
+
+	kfree(video->list);
+	video->list = NULL;
+
+	kfree(video->rect);
+	video->rect = NULL;
+
+	video->v4l2_input_status = V4L2_IN_ST_NO_SIGNAL;
+	video->flags = 0;
+	video->ctrl_cmd = VCD_CMD_OPERATION_CAPTURE;
+
+	if (atomic_dec_return(&video->ece.clients) == 0) {
+		nuvoton_video_ece_stop(video);
+		dev_dbg(video->dev, "ECE close: client %d\n",
+			atomic_read(&video->ece.clients));
+	}
+}
+
+static int nuvoton_video_querycap(struct file *file, void *fh,
+				  struct v4l2_capability *cap)
+{
+	strscpy(cap->driver, DEVICE_NAME, sizeof(cap->driver));
+	strscpy(cap->card, "Nuvoton Video Engine", sizeof(cap->card));
+	snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s",
+		 DEVICE_NAME);
+
+	return 0;
+}
+
+static int nuvoton_video_enum_format(struct file *file, void *fh,
+				     struct v4l2_fmtdesc *f)
+{
+	if (f->index)
+		return -EINVAL;
+
+	f->pixelformat = V4L2_PIX_FMT_RGB565;
+
+	return 0;
+}
+
+static int nuvoton_video_get_format(struct file *file, void *fh,
+				    struct v4l2_format *f)
+{
+	struct nuvoton_video *video = video_drvdata(file);
+
+	f->fmt.pix = video->pix_fmt;
+
+	return 0;
+}
+
+static int nuvoton_video_enum_input(struct file *file, void *fh,
+				    struct v4l2_input *inp)
+{
+	struct nuvoton_video *video = video_drvdata(file);
+
+	if (inp->index)
+		return -EINVAL;
+
+	strlcpy(inp->name, "Host VGA capture", sizeof(inp->name));
+	inp->type = V4L2_INPUT_TYPE_CAMERA;
+	inp->capabilities = V4L2_IN_CAP_DV_TIMINGS;
+	inp->status = video->v4l2_input_status;
+
+	return 0;
+}
+
+static int nuvoton_video_get_input(struct file *file, void *fh, unsigned int *i)
+{
+	*i = 0;
+
+	return 0;
+}
+
+static int nuvoton_video_set_input(struct file *file, void *fh, unsigned int i)
+{
+	if (i)
+		return -EINVAL;
+
+	return 0;
+}
+
+static int nuvoton_video_get_parm(struct file *file, void *fh,
+				  struct v4l2_streamparm *a)
+{
+	struct nuvoton_video *video = video_drvdata(file);
+
+	a->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
+	a->parm.capture.readbuffers = 3;
+	a->parm.capture.timeperframe.numerator = 1;
+	if (!video->frame_rate)
+		a->parm.capture.timeperframe.denominator = MAX_FRAME_RATE;
+	else
+		a->parm.capture.timeperframe.denominator = video->frame_rate;
+
+	return 0;
+}
+
+static int nuvoton_video_set_parm(struct file *file, void *fh,
+				  struct v4l2_streamparm *a)
+{
+	unsigned int frame_rate = 0;
+
+	a->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
+	a->parm.capture.readbuffers = 3;
+
+	if (a->parm.capture.timeperframe.numerator)
+		frame_rate = a->parm.capture.timeperframe.denominator /
+			     a->parm.capture.timeperframe.numerator;
+
+	if (!frame_rate || frame_rate > MAX_FRAME_RATE) {
+		frame_rate = 0;
+		a->parm.capture.timeperframe.denominator = MAX_FRAME_RATE;
+		a->parm.capture.timeperframe.numerator = 1;
+	}
+
+	return 0;
+}
+
+static int nuvoton_video_enum_framesizes(struct file *file, void *fh,
+					 struct v4l2_frmsizeenum *fsize)
+{
+	struct nuvoton_video *video = video_drvdata(file);
+
+	if (fsize->index)
+		return -EINVAL;
+
+	if (fsize->pixel_format != V4L2_PIX_FMT_RGB565)
+		return -EINVAL;
+
+	fsize->discrete.width = video->pix_fmt.width;
+	fsize->discrete.height = video->pix_fmt.height;
+	fsize->type = V4L2_FRMSIZE_TYPE_DISCRETE;
+
+	return 0;
+}
+
+static int nuvoton_video_enum_frameintervals(struct file *file, void *fh,
+					     struct v4l2_frmivalenum *fival)
+{
+	struct nuvoton_video *video = video_drvdata(file);
+
+	if (fival->index)
+		return -EINVAL;
+
+	if (fival->width != video->detected_timings.width ||
+	    fival->height != video->detected_timings.height)
+		return -EINVAL;
+
+	if (fival->pixel_format != V4L2_PIX_FMT_RGB565)
+		return -EINVAL;
+
+	fival->type = V4L2_FRMIVAL_TYPE_CONTINUOUS;
+
+	fival->stepwise.min.denominator = MAX_FRAME_RATE;
+	fival->stepwise.min.numerator = 1;
+	fival->stepwise.max.denominator = 1;
+	fival->stepwise.max.numerator = 1;
+	fival->stepwise.step = fival->stepwise.max;
+
+	return 0;
+}
+
+static int nuvoton_video_get_vid_overlay(struct file *file, void *fh,
+					 struct v4l2_format *fmt)
+{
+	struct nuvoton_video *video = video_drvdata(file);
+	struct v4l2_window *win = &fmt->fmt.win;
+	struct list_head *head, *pos, *nx;
+	struct rect_list *entry, *tmp;
+	struct v4l2_rect *rect;
+
+	if (video->list && video->rect) {
+		win->clipcount = video->rect[video->vb_index];
+		head = &video->list[video->vb_index];
+
+		entry = list_first_entry_or_null(head, struct rect_list, list);
+		if (entry) {
+			//struct v4l2_rect *r = &entry->clip.c;
+			rect = &entry->clip.c;
+
+			win->w.top = rect->top;
+			win->w.left = rect->left;
+			win->w.width = rect->width;
+			win->w.height = rect->height;
+
+			list_del(&entry->list);
+			kfree(entry);
+			if (video->rect[video->vb_index])
+				video->rect[video->vb_index]--;
+		}
+
+		list_for_each_safe(pos, nx, head) {
+			tmp = list_entry(pos, struct rect_list, list);
+			if (tmp) {
+				list_del(&tmp->list);
+				kfree(tmp);
+			}
+		}
+	}
+
+	return 0;
+}
+
+static int nuvoton_video_set_dv_timings(struct file *file, void *fh,
+					struct v4l2_dv_timings *timings)
+{
+	struct nuvoton_video *video = video_drvdata(file);
+
+	if (timings->bt.width == video->active_timings.width &&
+	    timings->bt.height == video->active_timings.height)
+		return 0;
+
+	if (vb2_is_busy(&video->queue)) {
+		dev_err(video->dev, "%s device busy\n", __func__);
+		return -EBUSY;
+	}
+
+	video->active_timings = timings->bt;
+
+	nuvoton_video_set_resolution(video);
+
+	video->pix_fmt.width = timings->bt.width;
+	video->pix_fmt.height = timings->bt.height;
+	video->pix_fmt.sizeimage = video->max_buffer_size;
+	video->pix_fmt.bytesperline = video->bytesperline;
+
+	timings->type = V4L2_DV_BT_656_1120;
+
+	return 0;
+}
+
+static int nuvoton_video_get_dv_timings(struct file *file, void *fh,
+					struct v4l2_dv_timings *timings)
+{
+	struct nuvoton_video *video = video_drvdata(file);
+
+	timings->type = V4L2_DV_BT_656_1120;
+	timings->bt = video->active_timings;
+
+	return 0;
+}
+
+static int nuvoton_video_query_dv_timings(struct file *file, void *fh,
+					  struct v4l2_dv_timings *timings)
+{
+	struct nuvoton_video *video = video_drvdata(file);
+
+	nuvoton_video_get_resolution(video);
+
+	timings->type = V4L2_DV_BT_656_1120;
+	timings->bt = video->detected_timings;
+
+	return video->v4l2_input_status ? -ENOLINK : 0;
+}
+
+static int nuvoton_video_enum_dv_timings(struct file *file, void *fh,
+					 struct v4l2_enum_dv_timings *timings)
+{
+	return v4l2_enum_dv_timings_cap(timings, &nuvoton_video_timings_cap,
+					NULL, NULL);
+}
+
+static int nuvoton_video_dv_timings_cap(struct file *file, void *fh,
+					struct v4l2_dv_timings_cap *cap)
+{
+	*cap = nuvoton_video_timings_cap;
+
+	return 0;
+}
+
+static int nuvoton_video_sub_event(struct v4l2_fh *fh,
+				   const struct v4l2_event_subscription *sub)
+{
+	switch (sub->type) {
+	case V4L2_EVENT_SOURCE_CHANGE:
+		return v4l2_src_change_event_subscribe(fh, sub);
+	}
+
+	return v4l2_ctrl_subscribe_event(fh, sub);
+}
+
+static const struct v4l2_ioctl_ops nuvoton_video_ioctls = {
+	.vidioc_querycap = nuvoton_video_querycap,
+
+	.vidioc_enum_fmt_vid_cap = nuvoton_video_enum_format,
+	.vidioc_g_fmt_vid_cap = nuvoton_video_get_format,
+	.vidioc_s_fmt_vid_cap = nuvoton_video_get_format,
+	.vidioc_try_fmt_vid_cap = nuvoton_video_get_format,
+
+	.vidioc_reqbufs = vb2_ioctl_reqbufs,
+	.vidioc_querybuf = vb2_ioctl_querybuf,
+	.vidioc_qbuf = vb2_ioctl_qbuf,
+	.vidioc_expbuf = vb2_ioctl_expbuf,
+	.vidioc_dqbuf = vb2_ioctl_dqbuf,
+	.vidioc_create_bufs = vb2_ioctl_create_bufs,
+	.vidioc_prepare_buf = vb2_ioctl_prepare_buf,
+	.vidioc_streamon = vb2_ioctl_streamon,
+	.vidioc_streamoff = vb2_ioctl_streamoff,
+
+	.vidioc_enum_input = nuvoton_video_enum_input,
+	.vidioc_g_input = nuvoton_video_get_input,
+	.vidioc_s_input = nuvoton_video_set_input,
+
+	.vidioc_g_parm = nuvoton_video_get_parm,
+	.vidioc_s_parm = nuvoton_video_set_parm,
+	.vidioc_g_fmt_vid_overlay = nuvoton_video_get_vid_overlay,
+	.vidioc_enum_framesizes = nuvoton_video_enum_framesizes,
+	.vidioc_enum_frameintervals = nuvoton_video_enum_frameintervals,
+
+	.vidioc_s_dv_timings = nuvoton_video_set_dv_timings,
+	.vidioc_g_dv_timings = nuvoton_video_get_dv_timings,
+	.vidioc_query_dv_timings = nuvoton_video_query_dv_timings,
+	.vidioc_enum_dv_timings = nuvoton_video_enum_dv_timings,
+	.vidioc_dv_timings_cap = nuvoton_video_dv_timings_cap,
+
+	.vidioc_subscribe_event = nuvoton_video_sub_event,
+	.vidioc_unsubscribe_event = v4l2_event_unsubscribe,
+};
+
+static int nuvoton_video_set_ctrl(struct v4l2_ctrl *ctrl)
+{
+	struct nuvoton_video *video = container_of(ctrl->handler,
+						   struct nuvoton_video,
+						   ctrl_handler);
+
+	switch (ctrl->id) {
+	case V4L2_CID_DETECT_MD_MODE:
+		if (ctrl->val == V4L2_DETECT_MD_MODE_GLOBAL)
+			video->ctrl_cmd = VCD_CMD_OPERATION_CAPTURE;
+		else
+			video->ctrl_cmd = VCD_CMD_OPERATION_COMPARE;
+	break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static const struct v4l2_ctrl_ops nuvoton_video_ctrl_ops = {
+	.s_ctrl = nuvoton_video_set_ctrl,
+};
+
+static int nuvoton_video_open(struct file *file)
+{
+	int rc;
+	struct nuvoton_video *video = video_drvdata(file);
+
+	mutex_lock(&video->video_lock);
+
+	rc = v4l2_fh_open(file);
+	if (rc) {
+		mutex_unlock(&video->video_lock);
+		return rc;
+	}
+
+	if (v4l2_fh_is_singular_file(file))
+		nuvoton_video_start(video);
+
+	mutex_unlock(&video->video_lock);
+
+	return 0;
+}
+
+static int nuvoton_video_release(struct file *file)
+{
+	int rc;
+	struct nuvoton_video *video = video_drvdata(file);
+
+	mutex_lock(&video->video_lock);
+
+	if (v4l2_fh_is_singular_file(file))
+		nuvoton_video_stop(video);
+
+	rc = _vb2_fop_release(file, NULL);
+
+	mutex_unlock(&video->video_lock);
+
+	return rc;
+}
+
+static const struct v4l2_file_operations nuvoton_video_v4l2_fops = {
+	.owner = THIS_MODULE,
+	.read = vb2_fop_read,
+	.poll = vb2_fop_poll,
+	.unlocked_ioctl = video_ioctl2,
+	.mmap = vb2_fop_mmap,
+	.open = nuvoton_video_open,
+	.release = nuvoton_video_release,
+};
+
+static int nuvoton_video_queue_setup(struct vb2_queue *q,
+				     unsigned int *num_buffers,
+				     unsigned int *num_planes,
+				     unsigned int sizes[],
+				     struct device *alloc_devs[])
+{
+	struct nuvoton_video *video = vb2_get_drv_priv(q);
+	int i;
+
+	dev_dbg(video->dev, "%s\n", __func__);
+
+	if (*num_planes) {
+		if (sizes[0] < video->max_buffer_size)
+			return -EINVAL;
+
+		return 0;
+	}
+
+	*num_planes = 1;
+	sizes[0] = video->max_buffer_size;
+
+	kfree(video->rect);
+	video->rect = NULL;
+
+	video->rect = kcalloc(*num_buffers, sizeof(*video->rect), GFP_KERNEL);
+
+	if (video->list) {
+		nuvoton_video_free_diff_table(video);
+		kfree(video->list);
+		video->list = NULL;
+	}
+
+	video->list = kzalloc(sizeof(*video->list) * *num_buffers, GFP_KERNEL);
+
+	for (i = 0; i < *num_buffers; i++)
+		INIT_LIST_HEAD(&video->list[i]);
+
+	video->num_buffers = *num_buffers;
+
+	return 0;
+}
+
+static int nuvoton_video_buf_prepare(struct vb2_buffer *vb)
+{
+	struct nuvoton_video *video = vb2_get_drv_priv(vb->vb2_queue);
+
+	if (vb2_plane_size(vb, 0) < video->max_buffer_size)
+		return -EINVAL;
+
+	return 0;
+}
+
+static int nuvoton_video_start_streaming(struct vb2_queue *q,
+					 unsigned int count)
+{
+	int rc;
+	struct nuvoton_video *video = vb2_get_drv_priv(q);
+
+	dev_dbg(video->dev, "%s\n", __func__);
+
+	video->sequence = 0;
+
+	rc = nuvoton_video_start_frame(video);
+	if (rc) {
+		nuvoton_video_bufs_done(video, VB2_BUF_STATE_QUEUED);
+		return rc;
+	}
+
+	set_bit(VIDEO_STREAMING, &video->flags);
+	return 0;
+}
+
+static void nuvoton_video_stop_streaming(struct vb2_queue *q)
+{
+	struct nuvoton_video *video = vb2_get_drv_priv(q);
+	struct regmap *vcd = video->vcd_regmap;
+
+	dev_dbg(video->dev, "%s\n", __func__);
+
+	clear_bit(VIDEO_STREAMING, &video->flags);
+
+	regmap_write(vcd, VCD_INTE, 0);
+	regmap_write(vcd, VCD_STAT, VCD_STAT_CLEAR);
+	nuvoton_video_gfx_reset(video);
+
+	nuvoton_video_bufs_done(video, VB2_BUF_STATE_ERROR);
+
+	video->ctrl_cmd = VCD_CMD_OPERATION_CAPTURE;
+}
+
+static void nuvoton_video_buf_queue(struct vb2_buffer *vb)
+{
+	int empty;
+	struct nuvoton_video *video = vb2_get_drv_priv(vb->vb2_queue);
+	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
+	struct nuvoton_video_buffer *nvb = to_nuvoton_video_buffer(vbuf);
+	unsigned long flags;
+
+	dev_dbg(video->dev, "%s\n", __func__);
+
+	spin_lock_irqsave(&video->lock, flags);
+	empty = list_empty(&video->buffers);
+	list_add_tail(&nvb->link, &video->buffers);
+	spin_unlock_irqrestore(&video->lock, flags);
+}
+
+static void nuvoton_video_buf_finish(struct vb2_buffer *vb)
+{
+	struct nuvoton_video *video = vb2_get_drv_priv(vb->vb2_queue);
+	struct regmap *vcd = video->vcd_regmap;
+	u32 val;
+	int ret;
+
+	dev_dbg(video->dev, "%s\n", __func__);
+
+	ret = regmap_read_poll_timeout(vcd, VCD_STAT, val,
+				       !(val & VCD_STAT_BUSY), 1000,
+				       VCD_BUSY_TIMEOUT_US);
+
+	if (ret) {
+		dev_warn(video->dev, "Wait for VCD_STAT_BUSY timeout\n");
+		return;
+	}
+
+	/* Capture next frame when a video buffer is dequeued */
+	nuvoton_video_start_frame(video);
+
+	video->vb_index = vb->index;
+}
+
+static const struct vb2_ops nuvoton_video_vb2_ops = {
+	.queue_setup = nuvoton_video_queue_setup,
+	.wait_prepare = vb2_ops_wait_prepare,
+	.wait_finish = vb2_ops_wait_finish,
+	.buf_prepare = nuvoton_video_buf_prepare,
+	.buf_finish = nuvoton_video_buf_finish,
+	.start_streaming = nuvoton_video_start_streaming,
+	.stop_streaming = nuvoton_video_stop_streaming,
+	.buf_queue =  nuvoton_video_buf_queue,
+};
+
+static int nuvoton_video_setup_video(struct nuvoton_video *video)
+{
+	struct v4l2_device *v4l2_dev = &video->v4l2_dev;
+	struct video_device *vdev = &video->vdev;
+	struct vb2_queue *vbq = &video->queue;
+	int rc;
+
+	video->pix_fmt.pixelformat = V4L2_PIX_FMT_RGB565;
+	video->pix_fmt.field = V4L2_FIELD_NONE;
+	video->pix_fmt.colorspace = V4L2_COLORSPACE_SRGB;
+	video->v4l2_input_status = V4L2_IN_ST_NO_SIGNAL;
+
+	rc = v4l2_device_register(video->dev, v4l2_dev);
+	if (rc) {
+		dev_err(video->dev, "Failed to register v4l2 device\n");
+		return rc;
+	}
+
+	v4l2_ctrl_handler_init(&video->ctrl_handler, 10);
+
+	v4l2_ctrl_new_std_menu(&video->ctrl_handler, &nuvoton_video_ctrl_ops,
+			       V4L2_CID_DETECT_MD_MODE,
+			       V4L2_DETECT_MD_MODE_REGION_GRID, 0,
+			       V4L2_DETECT_MD_MODE_GLOBAL);
+
+	if (video->ctrl_handler.error) {
+		dev_err(video->dev, "Failed to init controls: %d\n",
+			video->ctrl_handler.error);
+
+		rc = video->ctrl_handler.error;
+
+		v4l2_ctrl_handler_free(&video->ctrl_handler);
+		v4l2_device_unregister(v4l2_dev);
+
+		return rc;
+	}
+
+	v4l2_dev->ctrl_handler = &video->ctrl_handler;
+
+	vbq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+	vbq->io_modes = VB2_MMAP | VB2_READ | VB2_DMABUF;
+	vbq->dev = v4l2_dev->dev;
+	vbq->lock = &video->video_lock;
+	vbq->ops = &nuvoton_video_vb2_ops;
+	vbq->mem_ops = &vb2_dma_contig_memops;
+	vbq->drv_priv = video;
+	vbq->buf_struct_size = sizeof(struct nuvoton_video_buffer);
+	vbq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
+	vbq->min_buffers_needed = 3;
+
+	rc = vb2_queue_init(vbq);
+	if (rc) {
+		v4l2_ctrl_handler_free(&video->ctrl_handler);
+		v4l2_device_unregister(v4l2_dev);
+
+		dev_err(video->dev, "Failed to init vb2 queue\n");
+		return rc;
+	}
+
+	vdev->queue = vbq;
+	vdev->fops = &nuvoton_video_v4l2_fops;
+	vdev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_READWRITE |
+			    V4L2_CAP_STREAMING;
+	vdev->v4l2_dev = v4l2_dev;
+	strscpy(vdev->name, DEVICE_NAME, sizeof(vdev->name));
+	vdev->vfl_type = VFL_TYPE_VIDEO;
+	vdev->vfl_dir = VFL_DIR_RX;
+	vdev->release = video_device_release_empty;
+	vdev->ioctl_ops = &nuvoton_video_ioctls;
+	vdev->lock = &video->video_lock;
+
+	video_set_drvdata(vdev, video);
+	rc = video_register_device(vdev, VFL_TYPE_VIDEO, 0);
+	if (rc) {
+		vb2_queue_release(vbq);
+		v4l2_ctrl_handler_free(&video->ctrl_handler);
+		v4l2_device_unregister(v4l2_dev);
+
+		dev_err(video->dev, "Failed to register video device\n");
+		return rc;
+	}
+
+	return 0;
+}
+
+static int nuvoton_video_init(struct nuvoton_video *video)
+{
+	int irq;
+	int rc;
+	struct device *dev = video->dev;
+
+	irq = irq_of_parse_and_map(dev->of_node, 0);
+	if (!irq) {
+		dev_err(dev, "Unable to find VCD IRQ\n");
+		return -ENODEV;
+	}
+
+	rc = devm_request_threaded_irq(dev, irq, NULL, nuvoton_video_irq,
+				       IRQF_ONESHOT, DEVICE_NAME, video);
+	if (rc < 0) {
+		dev_err(dev, "Unable to request IRQ %d\n", irq);
+		return rc;
+	}
+
+	of_reserved_mem_device_init(dev);
+
+	rc = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
+	if (rc) {
+		dev_err(dev, "Failed to set DMA mask\n");
+		of_reserved_mem_device_release(dev);
+	}
+
+	return rc;
+}
+
+static const struct regmap_config nuvoton_video_regmap_cfg = {
+	.reg_bits	= 32,
+	.reg_stride	= 4,
+	.val_bits	= 32,
+	.max_register	= VCD_FIFO,
+};
+
+static const struct regmap_config nuvoton_video_ece_regmap_cfg = {
+	.reg_bits	= 32,
+	.reg_stride	= 4,
+	.val_bits	= 32,
+	.max_register	= ECE_HEX_RECT_OFFSET,
+};
+
+static int nuvoton_video_probe(struct platform_device *pdev)
+{
+	int rc;
+	void __iomem *regs;
+	struct nuvoton_video *video = kzalloc(sizeof(*video), GFP_KERNEL);
+
+	if (!video)
+		return -ENOMEM;
+
+	video->frame_rate = MAX_FRAME_RATE;
+	video->dev = &pdev->dev;
+	spin_lock_init(&video->lock);
+	mutex_init(&video->video_lock);
+	INIT_LIST_HEAD(&video->buffers);
+
+	regs = devm_platform_ioremap_resource_byname(pdev, VCD_MODULE_NAME);
+	if (IS_ERR(regs)) {
+		dev_err(&pdev->dev, "Failed to get VCD regmap resource!\n");
+		return PTR_ERR(regs);
+	}
+
+	video->vcd_regmap = devm_regmap_init_mmio(&pdev->dev, regs,
+						  &nuvoton_video_regmap_cfg);
+	if (IS_ERR(video->vcd_regmap)) {
+		dev_err(&pdev->dev, "Failed to init VCD regmap!\n");
+		return PTR_ERR(video->vcd_regmap);
+	}
+
+	regs = devm_platform_ioremap_resource_byname(pdev, ECE_MODULE_NAME);
+	if (IS_ERR(regs)) {
+		dev_err(&pdev->dev, "Failed to get ECE regmap resource!\n");
+		return PTR_ERR(regs);
+	}
+
+	video->ece.regmap = devm_regmap_init_mmio(&pdev->dev, regs,
+						  &nuvoton_video_ece_regmap_cfg);
+	if (IS_ERR(video->ece.regmap)) {
+		dev_err(&pdev->dev, "Failed to init ECE regmap!\n");
+		return PTR_ERR(video->ece.regmap);
+	}
+
+	video->reset = devm_reset_control_get(&pdev->dev, VCD_MODULE_NAME);
+	if (IS_ERR(video->reset)) {
+		dev_err(&pdev->dev, "Failed to get VCD reset control!\n");
+		return PTR_ERR(video->reset);
+	}
+
+	video->ece.reset = devm_reset_control_get(&pdev->dev, ECE_MODULE_NAME);
+	if (IS_ERR(video->ece.reset)) {
+		dev_err(&pdev->dev, "Failed to get ECE reset control!\n");
+		return PTR_ERR(video->ece.reset);
+	}
+
+	video->gcr_regmap =
+		syscon_regmap_lookup_by_compatible("nuvoton,npcm750-gcr");
+	if (IS_ERR(video->gcr_regmap))
+		return PTR_ERR(video->gcr_regmap);
+
+	video->gfx_regmap =
+		syscon_regmap_lookup_by_compatible("nuvoton,npcm750-gfxi");
+	if (IS_ERR(video->gfx_regmap))
+		return PTR_ERR(video->gfx_regmap);
+
+	rc = nuvoton_video_init(video);
+	if (rc)
+		return rc;
+
+	rc = nuvoton_video_setup_video(video);
+	if (rc)
+		return rc;
+
+	dev_info(video->dev, "NPCM video driver probed\n");
+
+	return 0;
+}
+
+static int nuvoton_video_remove(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct v4l2_device *v4l2_dev = dev_get_drvdata(dev);
+	struct nuvoton_video *video = to_nuvoton_video(v4l2_dev);
+
+	video_unregister_device(&video->vdev);
+
+	vb2_queue_release(&video->queue);
+
+	v4l2_ctrl_handler_free(&video->ctrl_handler);
+
+	v4l2_device_unregister(v4l2_dev);
+
+	nuvoton_video_ece_stop(video);
+
+	of_reserved_mem_device_release(dev);
+
+	return 0;
+}
+
+static const struct of_device_id nuvoton_video_match[] = {
+	{
+		.compatible = "nuvoton,npcm750-video",
+	},
+	{},
+};
+
+MODULE_DEVICE_TABLE(of, nuvoton_video_match)
+
+static struct platform_driver nuvoton_video_driver = {
+	.driver = {
+		.name = DEVICE_NAME,
+		.of_match_table = nuvoton_video_match,
+	},
+	.probe = nuvoton_video_probe,
+	.remove = nuvoton_video_remove,
+};
+
+module_platform_driver(nuvoton_video_driver);
+
+MODULE_AUTHOR("Joseph Liu<kwliu@nuvoton.com>");
+MODULE_AUTHOR("Marvin Lin<kflin@nuvoton.com>");
+MODULE_DESCRIPTION("Driver for Nuvoton NPCM Video Capture/Encode Engine");
+MODULE_LICENSE("GPL v2");
-- 
2.17.1


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

* Re: [PATCH v2 1/5] arm: dts: Add node for NPCM Video Capture/Encode Engine
  2022-05-13  3:34 ` [PATCH v2 1/5] arm: dts: Add node for " Marvin Lin
@ 2022-05-13  5:22   ` Paul Menzel
  2022-05-17  1:55     ` Kun-Fa Lin
  0 siblings, 1 reply; 17+ messages in thread
From: Paul Menzel @ 2022-05-13  5:22 UTC (permalink / raw)
  To: Marvin Lin
  Cc: mchehab, linux-media, linux-kernel, kwliu, tmaimon77,
	avifishman70, openbmc, tali.perry1, kflin

Dear Marvin,


Thank you for the patch.

Am 13.05.22 um 05:34 schrieb Marvin Lin:
> Add node for Video Capture/Differentiation Engine (VCD) and Encoding
> Compression Engine (ECE) present on Nuvoton NPCM SoCs.

Could you please add the datasheet name and revision to the commit message?

What board did you test this on?

> Signed-off-by: Marvin Lin <kflin@nuvoton.com>

As you sent this from your Google Mail account, that address is going to 
be used in the author field. Is your Nuvoton address configured for git?

     git config user.email

Then I would have thought, that that address is also in the files 
generated by `git format-patch`.


Kind regards,

Paul


> ---
>   arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi | 19 +++++++++++++++++++
>   1 file changed, 19 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi b/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi
> index 3696980a3da1..0d2df74974bf 100644
> --- a/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi
> +++ b/arch/arm/boot/dts/nuvoton-common-npcm7xx.dtsi
> @@ -178,6 +178,19 @@
>   			status = "disabled";
>   		};
>   
> +		video: video@f0810000 {
> +			compatible = "nuvoton,npcm750-video";
> +			reg = <0xf0810000 0x10000>, <0xf0820000 0x2000>;
> +			reg-names = "vcd", "ece";
> +			interrupts = <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>;
> +			resets = <&rstc NPCM7XX_RESET_IPSRST2 NPCM7XX_RESET_VCD>,
> +				 <&rstc NPCM7XX_RESET_IPSRST2 NPCM7XX_RESET_ECE>;
> +			reset-names = "vcd", "ece";
> +			nuvoton,syscon-gcr = <&gcr>;
> +			nuvoton,syscon-gfxi = <&gfxi>;
> +			status = "disabled";
> +		};
> +
>   		apb {
>   			#address-cells = <1>;
>   			#size-cells = <1>;
> @@ -553,6 +566,12 @@
>   				pinctrl-0 = <&smb15_pins>;
>   				status = "disabled";
>   			};
> +
> +			gfxi: gfxi@e000 {
> +				compatible = "nuvoton,npcm750-gfxi", "syscon",
> +					     "simple-mfd";
> +				reg = <0xe000 0x100>;
> +			};
>   		};
>   	};
>   

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

* Re: [PATCH v2 5/5] drivers: media: platform: Add NPCM Video Capture/Encode Engine driver
  2022-05-13  3:34 ` [PATCH v2 5/5] drivers: media: platform: Add NPCM Video Capture/Encode Engine driver Marvin Lin
@ 2022-05-13  5:46   ` Paul Menzel
  2022-05-17  2:59     ` Kun-Fa Lin
  2022-05-13  9:09   ` kernel test robot
                     ` (3 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: Paul Menzel @ 2022-05-13  5:46 UTC (permalink / raw)
  To: Marvin Lin
  Cc: mchehab, linux-media, linux-kernel, kwliu, tmaimon77,
	avifishman70, openbmc, tali.perry1, kflin

Dear Marvin,


Thank you for the patch.

Am 13.05.22 um 05:34 schrieb Marvin Lin:
> Add driver for Video Capture/Differentiation Engine (VCD) and Encoding
> Compression Engine (ECE) present on Nuvoton NPCM SoCs.

Please mention the datasheet name and revision used to implement this? 
How can your patch be tested?

For a over 2000 line patch, I would expect a longer commit message with 
a summary of the hardware features, and implementation.

As the module author should you also be added to the file `MAINTAINERS`? 
(Maybe even with a functional address <linux-npcm-video@nuvoton.com>?

> Signed-off-by: Marvin Lin <kflin@nuvoton.com>

Same comment as in 1/5 regarding the author email address.

> ---
>   drivers/media/platform/Kconfig              |    1 +
>   drivers/media/platform/Makefile             |    1 +
>   drivers/media/platform/nuvoton/Kconfig      |   12 +
>   drivers/media/platform/nuvoton/Makefile     |    2 +
>   drivers/media/platform/nuvoton/npcm-video.c | 2074 +++++++++++++++++++
>   5 files changed, 2090 insertions(+)
>   create mode 100644 drivers/media/platform/nuvoton/Kconfig
>   create mode 100644 drivers/media/platform/nuvoton/Makefile
>   create mode 100644 drivers/media/platform/nuvoton/npcm-video.c
> 
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index f1056ceaf5a8..21daeedb47c9 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -72,6 +72,7 @@ source "drivers/media/platform/chips-media/Kconfig"
>   source "drivers/media/platform/intel/Kconfig"
>   source "drivers/media/platform/marvell/Kconfig"
>   source "drivers/media/platform/mediatek/Kconfig"
> +source "drivers/media/platform/nuvoton/Kconfig"
>   source "drivers/media/platform/nvidia/Kconfig"
>   source "drivers/media/platform/nxp/Kconfig"
>   source "drivers/media/platform/qcom/Kconfig"
> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
> index a881e97bae95..4a1da16c4f8a 100644
> --- a/drivers/media/platform/Makefile
> +++ b/drivers/media/platform/Makefile
> @@ -15,6 +15,7 @@ obj-y += chips-media/
>   obj-y += intel/
>   obj-y += marvell/
>   obj-y += mediatek/
> +obj-y += nuvoton/
>   obj-y += nvidia/
>   obj-y += nxp/
>   obj-y += qcom/
> diff --git a/drivers/media/platform/nuvoton/Kconfig b/drivers/media/platform/nuvoton/Kconfig
> new file mode 100644
> index 000000000000..8718631b0424
> --- /dev/null
> +++ b/drivers/media/platform/nuvoton/Kconfig
> @@ -0,0 +1,12 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +
> +comment "Nuvoton media platform drivers"
> +
> +config VIDEO_NUVOTON

Is that driver going to support all Nuvoton devices or just NPCM? If 
only NPCM, that should be part of the Kconfig config name?

> +	tristate "Nuvoton NPCM Video Capture/Encode Engine driver"
> +	depends on V4L_PLATFORM_DRIVERS
> +	depends on VIDEO_DEV
> +	select VIDEOBUF2_DMA_CONTIG
> +	help
> +	  Support for the Video Capture/Differentiation Engine (VCD) and
> +	  Encoding Compression Engine (ECE) present on Nuvoton NPCM SoCs.

Mention the module name?

 > To compile this driver as a module, choose M here: the module will be 
called XXX.

> diff --git a/drivers/media/platform/nuvoton/Makefile b/drivers/media/platform/nuvoton/Makefile
> new file mode 100644
> index 000000000000..63041bb526a9
> --- /dev/null
> +++ b/drivers/media/platform/nuvoton/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +obj-$(CONFIG_VIDEO_NUVOTON) += npcm-video.o
> diff --git a/drivers/media/platform/nuvoton/npcm-video.c b/drivers/media/platform/nuvoton/npcm-video.c
> new file mode 100644
> index 000000000000..4ed5290f4c5d
> --- /dev/null
> +++ b/drivers/media/platform/nuvoton/npcm-video.c
> @@ -0,0 +1,2074 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for Video Capture/Differentiation Engine (VCD) and Encoding
> + * Compression Engine (ECE) present on Nuvoton NPCM SoCs.
> + *
> + * Copyright (C) 2022 Nuvoton Technologies
> + */
> +
> +#include <linux/atomic.h>
> +#include <linux/bitfield.h>
> +#include <linux/bitmap.h>
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/interrupt.h>
> +#include <linux/jiffies.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_reserved_mem.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>
> +#include <linux/sched.h>
> +#include <linux/spinlock.h>
> +#include <linux/string.h>
> +#include <linux/v4l2-controls.h>
> +#include <linux/videodev2.h>
> +#include <linux/wait.h>
> +#include <linux/workqueue.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-dev.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-dv-timings.h>
> +#include <media/v4l2-event.h>
> +#include <media/v4l2-ioctl.h>
> +#include <media/videobuf2-dma-contig.h>
> +
> +#define DEVICE_NAME			"nuvoton-video"
> +
> +#define MAX_FRAME_RATE			60
> +#define MAX_WIDTH			1920
> +#define MAX_HEIGHT			1200
> +#define MIN_WIDTH			320
> +#define MIN_HEIGHT			240
> +#define MIN_LP				512
> +#define MAX_LP				4096
> +#define RECT_W				16
> +#define RECT_H				16
> +#define BITMAP_SIZE			32
> +
> +#define VCD_MODULE_NAME			"vcd"
> +#define ECE_MODULE_NAME			"ece"
> +
> +/* VCD Registers */
> +#define VCD_DIFF_TBL			0x0000
> +#define VCD_FBA_ADR			0x8000
> +#define VCD_FBB_ADR			0x8004
> +
> +#define VCD_FB_LP			0x8008
> +#define  VCD_FBA_LP			GENMASK(15, 0)
> +#define  VCD_FBB_LP			GENMASK(31, 16)
> +
> +#define VCD_CAP_RES			0x800c
> +#define  VCD_CAP_RES_VERT_RES		GENMASK(10, 0)
> +#define  VCD_CAP_RES_HOR_RES		GENMASK(26, 16)
> +
> +#define VCD_MODE			0x8014
> +#define  VCD_MODE_VCDE			BIT(0)
> +#define  VCD_MODE_CM565			BIT(1)
> +#define  VCD_MODE_IDBC			BIT(3)
> +#define  VCD_MODE_COLOR_CNVRT		GENMASK(5, 4)
> +#define   VCD_MODE_COLOR_CNVRT_NO_CNVRT	0
> +#define   VCD_MODE_COLOR_CNVRT_RGB_222	1
> +#define   VCD_MODE_COLOR_CNVRT_666_MODE	2
> +#define   VCD_MODE_COLOR_CNVRT_RGB_888	3
> +#define  VCD_MODE_KVM_BW_SET		BIT(16)
> +
> +#define VCD_CMD				0x8018
> +#define  VCD_CMD_GO			BIT(0)
> +#define  VCD_CMD_RST			BIT(1)
> +#define  VCD_CMD_OPERATION		GENMASK(6, 4)
> +#define   VCD_CMD_OPERATION_CAPTURE	0
> +#define   VCD_CMD_OPERATION_COMPARE	2
> +
> +#define	VCD_STAT			0x801c
> +#define	 VCD_STAT_DONE			BIT(0)
> +#define	 VCD_STAT_IFOT			BIT(2)
> +#define	 VCD_STAT_IFOR			BIT(3)
> +#define	 VCD_STAT_BUSY			BIT(30)
> +#define	VCD_STAT_CLEAR			0x3fff
> +
> +#define VCD_INTE			0x8020
> +#define  VCD_INTE_DONE_IE		BIT(0)
> +#define  VCD_INTE_IFOT_IE		BIT(2)
> +#define  VCD_INTE_IFOR_IE		BIT(3)
> +
> +#define VCD_RCHG			0x8028
> +#define VCD_RCHG_TIM_PRSCL		GENMASK(12, 9)
> +
> +#define VCD_FIFO			0x805c
> +#define  VCD_FIFO_TH			0x100350ff
> +
> +#define VCD_MAX_SRC_BUFFER_SIZE		0x500000 /* 1920 x 1200 x 2 bpp */
> +#define VCD_KVM_BW_PCLK			120000000UL
> +#define VCD_BUSY_TIMEOUT_US		50000
> +
> +/* ECE Registers */
> +#define ECE_DDA_CTRL			0x0000
> +#define  ECE_DDA_CTRL_ECEEN		BIT(0)
> +#define  ECE_DDA_CTRL_INTEN		BIT(8)
> +
> +#define ECE_DDA_STS			0x0004
> +#define  ECE_DDA_STS_CDREADY		BIT(8)
> +#define  ECE_DDA_STS_ACDRDY		BIT(10)
> +
> +#define ECE_FBR_BA			0x0008
> +#define ECE_ED_BA			0x000c
> +#define ECE_RECT_XY			0x0010
> +
> +#define ECE_RECT_DIMEN			0x0014
> +#define  ECE_RECT_DIMEN_WR		GENMASK(10, 0)
> +#define  ECE_RECT_DIMEN_WLTR		GENMASK(14, 11)
> +#define  ECE_RECT_DIMEN_HR		GENMASK(26, 16)
> +#define  ECE_RECT_DIMEN_HLTR		GENMASK(30, 27)
> +
> +#define ECE_RESOL			0x001c
> +#define  ECE_RESOL_FB_LP_512		0
> +#define  ECE_RESOL_FB_LP_1024		1
> +#define  ECE_RESOL_FB_LP_2048		2
> +#define  ECE_RESOL_FB_LP_2560		3
> +#define  ECE_RESOL_FB_LP_4096		4
> +
> +#define ECE_HEX_CTRL			0x0040
> +#define  ECE_HEX_CTRL_ENCDIS		BIT(0)
> +#define  ECE_HEX_CTRL_ENC_GAP		GENMASK(12, 8)
> +
> +#define ECE_HEX_RECT_OFFSET		0x0048
> +#define  ECE_HEX_RECT_OFFSET_MASK	GENMASK(22, 0)
> +
> +#define ECE_TILE_W			16
> +#define ECE_TILE_H			16
> +#define ECE_POLL_TIMEOUT_US		50000
> +
> +/* GCR Registers */
> +#define INTCR				0x3c
> +#define  INTCR_GFXIFDIS			GENMASK(9, 8)
> +#define  INTCR_DEHS			BIT(27)
> +
> +#define INTCR2				0x60
> +#define  INTCR2_GIRST2			BIT(2)
> +#define  INTCR2_GIHCRST			BIT(5)
> +#define  INTCR2_GIVCRST			BIT(6)
> +
> +#define INTCR3				0x9c
> +#define  INTCR3_GMMAP			GENMASK(10, 8)
> +#define   INTCR3_GMMAP_128MB		0
> +#define   INTCR3_GMMAP_256MB		1
> +#define   INTCR3_GMMAP_512MB		2
> +#define   INTCR3_GMMAP_1GB		3
> +#define   INTCR3_GMMAP_2GB		4
> +
> +#define ADDR_GMMAP_128MB		0x07000000
> +#define ADDR_GMMAP_256MB		0x0f000000
> +#define ADDR_GMMAP_512MB		0x1f000000
> +#define ADDR_GMMAP_1GB			0x3f000000
> +#define ADDR_GMMAP_2GB			0x7f000000
> +
> +#define GMMAP_LENGTH			0xc00000 /* Total 16MB, but 4MB preserved*/
> +
> +#define MFSEL1				0x0c
> +#define  MFSEL1_DVH1SEL			BIT(27)
> +
> +/* GFXI Register */
> +#define DISPST				0x00
> +#define  DISPST_HSCROFF			BIT(1)
> +#define  DISPST_MGAMODE			BIT(7)
> +
> +#define HVCNTL				0x10
> +#define  HVCNTL_MASK			GENMASK(7, 0)
> +
> +#define HVCNTH				0x14
> +#define  HVCNTH_MASK			GENMASK(2, 0)
> +
> +#define VVCNTL				0x20
> +#define  VVCNTL_MASK			GENMASK(7, 0)
> +
> +#define VVCNTH				0x24
> +#define  VVCNTH_MASK			GENMASK(2, 0)
> +
> +#define GPLLINDIV			0x40
> +#define  GPLLINDIV_MASK			GENMASK(5, 0)
> +#define  GPLLINDIV_GPLLFBDV8		BIT(7)
> +
> +#define GPLLFBDIV			0x44
> +#define  GPLLFBDIV_MASK			GENMASK(7, 0)
> +
> +#define GPLLST				0x48
> +#define  GPLLST_PLLOTDIV1		GENMASK(2, 0)
> +#define  GPLLST_PLLOTDIV2		GENMASK(5, 3)
> +#define  GPLLST_GPLLFBDV109		GENMASK(7, 6)
> +
> +struct nuvoton_video_addr {
> +	unsigned int size;

size_t?

> +	dma_addr_t dma;
> +	void *virt;
> +};
> +
> +struct nuvoton_video_buffer {
> +	struct vb2_v4l2_buffer vb;
> +	struct list_head link;
> +};
> +
> +#define to_nuvoton_video_buffer(x) \
> +	container_of((x), struct nuvoton_video_buffer, vb)
> +
> +enum {
> +	VIDEO_STREAMING,
> +	VIDEO_FRAME_INPRG,
> +	VIDEO_STOPPED,
> +};
> +
> +struct rect_list {
> +	struct v4l2_clip clip;
> +	struct list_head list;
> +};
> +
> +struct rect_list_info {
> +	struct rect_list *list;
> +	struct rect_list *first;
> +	struct list_head *head;
> +	int index;
> +	int tile_perline;
> +	int tile_perrow;
> +	int offset_perline;
> +	int tile_size;
> +	int tile_cnt;

Can all of these be unsigned?

> +};
> +
> +struct nuvoton_ece {
> +	struct regmap *regmap;
> +	atomic_t clients;
> +	struct reset_control *reset;
> +};
> +
> +struct nuvoton_video {
> +	struct regmap *gcr_regmap;
> +	struct regmap *gfx_regmap;
> +	struct regmap *vcd_regmap;
> +
> +	struct device *dev;
> +	struct v4l2_ctrl_handler ctrl_handler;
> +	struct v4l2_device v4l2_dev;
> +	struct v4l2_pix_format pix_fmt;
> +	struct v4l2_bt_timings active_timings;
> +	struct v4l2_bt_timings detected_timings;
> +	u32 v4l2_input_status;
> +	struct vb2_queue queue;
> +	struct video_device vdev;
> +	struct mutex video_lock;	/* v4l2 and videobuf2 lock */
> +
> +	struct list_head buffers;
> +	spinlock_t lock;		/* buffer list lock */
> +	unsigned long flags;
> +	unsigned int sequence;
> +
> +	unsigned int max_buffer_size;
> +	struct nuvoton_video_addr src;
> +	struct reset_control *reset;
> +	struct nuvoton_ece ece;
> +
> +	int frame_rate;
> +	int vb_index;

Unsigned?

> +	u32 bytesperline;
> +	u8 bytesperpixel;
> +	u32 rect_cnt;
> +	u8 num_buffers;
> +	struct list_head *list;
> +	u32 *rect;

I would not limit the size?

> +	int ctrl_cmd;
> +	int op_cmd;
> +};
> +
> +#define to_nuvoton_video(x) container_of((x), struct nuvoton_video, v4l2_dev)
> +
> +static const struct v4l2_dv_timings_cap nuvoton_video_timings_cap = {
> +	.type = V4L2_DV_BT_656_1120,
> +	.bt = {
> +		.min_width = MIN_WIDTH,
> +		.max_width = MAX_WIDTH,
> +		.min_height = MIN_HEIGHT,
> +		.max_height = MAX_HEIGHT,
> +		.min_pixelclock = 6574080, /* 640 x 480 x 24Hz */
> +		.max_pixelclock = 138240000, /* 1920 x 1200 x 60Hz */
> +		.standards = V4L2_DV_BT_STD_CEA861 | V4L2_DV_BT_STD_DMT |
> +			     V4L2_DV_BT_STD_CVT | V4L2_DV_BT_STD_GTF,
> +		.capabilities = V4L2_DV_BT_CAP_PROGRESSIVE |
> +				V4L2_DV_BT_CAP_REDUCED_BLANKING |
> +				V4L2_DV_BT_CAP_CUSTOM,
> +	},
> +};
> +
> +static DECLARE_BITMAP(bitmap, BITMAP_SIZE);
> +
> +static void nuvoton_video_ece_prepend_rect_header(u8 *addr, u16 x, u16 y, u16 w,
> +						  u16 h)
> +{
> +	__be16 x_pos = cpu_to_be16(x);
> +	__be16 y_pos = cpu_to_be16(y);
> +	__be16 width = cpu_to_be16(w);
> +	__be16 height = cpu_to_be16(h);
> +	__be32 encoding = cpu_to_be32(5); /* Hextile encoding */
> +
> +	memcpy(addr, &x_pos, 2);
> +	memcpy(addr + 2, &y_pos, 2);
> +	memcpy(addr + 4, &width, 2);
> +	memcpy(addr + 6, &height, 2);
> +	memcpy(addr + 8, &encoding, 4);
> +}
> +
> +static u32 nuvoton_video_ece_get_ed_size(struct nuvoton_video *video,
> +					 u32 offset, void *addr)

Use unsigned int as return value?

> +{
> +	struct regmap *ece = video->ece.regmap;
> +	u32 size, gap, val;
> +	int ret;
> +
> +	ret = regmap_read_poll_timeout(ece, ECE_DDA_STS, val,
> +				       (val & ECE_DDA_STS_CDREADY),
> +				       0, ECE_POLL_TIMEOUT_US);
> +
> +	if (ret) {
> +		dev_warn(video->dev, "Wait for ECE_DDA_STS_CDREADY timeout\n");
> +		return 0;
> +	}
> +
> +	size = readl(addr + offset);
> +
> +	regmap_read(ece, ECE_HEX_CTRL, &val);
> +	gap = FIELD_GET(ECE_HEX_CTRL_ENC_GAP, val);
> +
> +	dev_dbg(video->dev, "offset = %u, ed_size = %u, gap = %u\n", offset,
> +		size, gap);
> +
> +	return size + gap;
> +}
> +
> +static void nuvoton_video_ece_enc_rect(struct nuvoton_video *video, u32 r_off_x,
> +				       u32 r_off_y, u32 r_w, u32 r_h)
> +{
> +	struct regmap *ece = video->ece.regmap;
> +	u32 rect_offset = (r_off_y * video->bytesperline) + (r_off_x * 2);
> +	u32 temp;
> +	u32 w_tile;
> +	u32 h_tile;
> +	u32 w_size = ECE_TILE_W;
> +	u32 h_size = ECE_TILE_H;

Any reason to fix the sizes?

> +
> +	regmap_update_bits(ece, ECE_DDA_CTRL, ECE_DDA_CTRL_ECEEN, 0);
> +	regmap_update_bits(ece, ECE_DDA_CTRL, ECE_DDA_CTRL_ECEEN,
> +			   ECE_DDA_CTRL_ECEEN);
> +	regmap_write(ece, ECE_DDA_STS, ECE_DDA_STS_CDREADY |
> +		     ECE_DDA_STS_ACDRDY);
> +	regmap_write(ece, ECE_RECT_XY, rect_offset);
> +
> +	w_tile = r_w / ECE_TILE_W;
> +	h_tile = r_h / ECE_TILE_H;
> +
> +	if (r_w % ECE_TILE_W) {
> +		w_tile += 1;
> +		w_size = r_w % ECE_TILE_W;
> +	}
> +
> +	if (r_h % ECE_TILE_H || !h_tile) {
> +		h_tile += 1;
> +		h_size = r_h % ECE_TILE_H;
> +	}
> +
> +	temp = FIELD_PREP(ECE_RECT_DIMEN_WLTR, w_size - 1) |
> +	       FIELD_PREP(ECE_RECT_DIMEN_HLTR, h_size - 1) |
> +	       FIELD_PREP(ECE_RECT_DIMEN_WR, w_tile - 1) |
> +	       FIELD_PREP(ECE_RECT_DIMEN_HR, h_tile - 1);
> +
> +	regmap_write(ece, ECE_RECT_DIMEN, temp);
> +}
> +
> +static u32 nuvoton_video_ece_read_rect_offset(struct nuvoton_video *video)
> +{
> +	struct regmap *ece = video->ece.regmap;
> +	u32 offset;
> +
> +	regmap_read(ece, ECE_HEX_RECT_OFFSET, &offset);
> +	return FIELD_GET(ECE_HEX_RECT_OFFSET_MASK, offset);
> +}
> +
> +/*
> + * Set the line pitch (in bytes) for the frame buffers.
> + * Can be on of those values: 512, 1024, 2048, 2560 or 4096 bytes.
> + */
> +static void nuvoton_video_ece_set_lp(struct nuvoton_video *video, u32 pitch)
> +{
> +	u32 lp;
> +	struct regmap *ece = video->ece.regmap;
> +
> +	switch (pitch) {
> +	case 512:
> +		lp = ECE_RESOL_FB_LP_512;
> +		break;
> +	case 1024:
> +		lp = ECE_RESOL_FB_LP_1024;
> +		break;
> +	case 2048:
> +		lp = ECE_RESOL_FB_LP_2048;
> +		break;
> +	case 2560:
> +		lp = ECE_RESOL_FB_LP_2560;
> +		break;
> +	case 4096:
> +		lp = ECE_RESOL_FB_LP_4096;
> +		break;
> +	default:
> +		return;
> +	}
> +
> +	regmap_write(ece, ECE_RESOL, lp);
> +}
> +
> +static void nuvoton_video_ece_set_fb_addr(struct nuvoton_video *video,
> +					  u32 buffer)
> +{
> +	struct regmap *ece = video->ece.regmap;
> +
> +	regmap_write(ece, ECE_FBR_BA, buffer);
> +}
> +
> +static void nuvoton_video_ece_set_enc_dba(struct nuvoton_video *video, u32 addr)
> +{
> +	struct regmap *ece = video->ece.regmap;
> +
> +	regmap_write(ece, ECE_ED_BA, addr);
> +}
> +
> +static void nuvoton_video_ece_clear_rect_offset(struct nuvoton_video *video)
> +{
> +	struct regmap *ece = video->ece.regmap;
> +
> +	regmap_write(ece, ECE_HEX_RECT_OFFSET, 0);
> +}
> +
> +static void nuvoton_video_ece_ctrl_reset(struct nuvoton_video *video)
> +{
> +	struct regmap *ece = video->ece.regmap;
> +
> +	regmap_update_bits(ece, ECE_DDA_CTRL, ECE_DDA_CTRL_ECEEN, 0);
> +	regmap_update_bits(ece, ECE_HEX_CTRL, ECE_HEX_CTRL_ENCDIS,
> +			   ECE_HEX_CTRL_ENCDIS);
> +	regmap_update_bits(ece, ECE_DDA_CTRL, ECE_DDA_CTRL_ECEEN,
> +			   ECE_DDA_CTRL_ECEEN);
> +	regmap_update_bits(ece, ECE_HEX_CTRL, ECE_HEX_CTRL_ENCDIS, 0);
> +
> +	nuvoton_video_ece_clear_rect_offset(video);
> +}
> +
> +static void nuvoton_video_ece_ip_reset(struct nuvoton_video *video)
> +{
> +	reset_control_assert(video->ece.reset);
> +	msleep(100);
> +	reset_control_deassert(video->ece.reset);
> +	msleep(100);

100 ms is quite long. Please add a comment, where that is documented. Is 
there a way to poll, if the device is done?

> +}
> +
> +static int nuvoton_video_ece_init(struct nuvoton_video *video)
> +{
> +	nuvoton_video_ece_ip_reset(video);
> +	nuvoton_video_ece_ctrl_reset(video);
> +
> +	return 0;
> +}
> +
> +static int nuvoton_video_ece_stop(struct nuvoton_video *video)
> +{
> +	struct regmap *ece = video->ece.regmap;
> +
> +	regmap_update_bits(ece, ECE_DDA_CTRL, ECE_DDA_CTRL_ECEEN, 0);
> +	regmap_update_bits(ece, ECE_DDA_CTRL, ECE_DDA_CTRL_INTEN, 0);
> +	regmap_update_bits(ece, ECE_HEX_CTRL, ECE_HEX_CTRL_ENCDIS,
> +			   ECE_HEX_CTRL_ENCDIS);
> +	nuvoton_video_ece_clear_rect_offset(video);
> +
> +	return 0;
> +}
> +
> +static bool nuvoton_video_alloc_buf(struct nuvoton_video *video,
> +				    struct nuvoton_video_addr *addr,
> +				    unsigned int size)
> +{
> +	if (size > VCD_MAX_SRC_BUFFER_SIZE)
> +		size = VCD_MAX_SRC_BUFFER_SIZE;
> +
> +	addr->virt = dma_alloc_coherent(video->dev, size, &addr->dma,
> +					GFP_KERNEL);
> +
> +	if (!addr->virt)
> +		return false;
> +
> +	addr->size = size;
> +	return true;
> +}
> +
> +static void nuvoton_video_free_buf(struct nuvoton_video *video,
> +				   struct nuvoton_video_addr *addr)
> +{
> +	dma_free_coherent(video->dev, addr->size, addr->virt, addr->dma);
> +	addr->size = 0;
> +	addr->dma = 0ULL;
> +	addr->virt = NULL;
> +}
> +
> +static void nuvoton_video_free_diff_table(struct nuvoton_video *video)
> +{
> +	struct list_head *head, *pos, *nx;
> +	struct rect_list *tmp;
> +	int i;

unsigned?

> +
> +	for (i = 0; i < video->num_buffers; i++) {
> +		head = &video->list[i];
> +		list_for_each_safe(pos, nx, head) {
> +			tmp = list_entry(pos, struct rect_list, list);
> +			if (tmp) {
> +				list_del(&tmp->list);
> +				kfree(tmp);
> +			}
> +		}
> +	}
> +}
> +
> +static int nuvoton_video_add_rect(struct nuvoton_video *video, int index,
> +				  u32 x, u32 y, u32 w, u32 h)
> +{
> +	struct list_head *head = &video->list[index];
> +	struct rect_list *list = NULL;
> +	struct v4l2_rect *r;
> +
> +	list = kzalloc(sizeof(*list), GFP_KERNEL);
> +	if (!list)
> +		return 0;
> +
> +	r = &list->clip.c;
> +	r->left = x;
> +	r->top = y;
> +	r->width = w;
> +	r->height = h;
> +
> +	list_add_tail(&list->list, head);
> +	return 1;
> +}
> +
> +static void nuvoton_video_merge_rect(struct nuvoton_video *video,
> +				     struct rect_list_info *info)
> +{
> +	struct list_head *head = info->head;
> +	struct rect_list *list = info->list;
> +	struct rect_list *first = info->first;
> +	struct v4l2_rect *r = &list->clip.c;
> +	struct v4l2_rect *f = &first->clip.c;
> +
> +	if (!first) {
> +		first = list;
> +		info->first = first;
> +		list_add_tail(&list->list, head);
> +		video->rect_cnt++;
> +	} else {
> +		if ((r->left == (f->left + f->width)) && r->top == f->top) {
> +			f->width += r->width;
> +			kfree(list);
> +		} else if ((r->top == (f->top + f->height)) &&
> +			   (r->left == f->left)) {
> +			f->height += r->height;
> +			kfree(list);
> +		} else if (((r->top > f->top) &&
> +			   (r->top < (f->top + f->height))) &&
> +			   ((r->left > f->left) &&
> +			   (r->left < (f->left + f->width)))) {
> +			kfree(list);
> +		} else {
> +			list_add_tail(&list->list, head);
> +			video->rect_cnt++;
> +			info->first = list;
> +		}
> +	}
> +}
> +
> +static struct rect_list *nuvoton_video_new_rect(struct nuvoton_video *video,
> +						int offset, int index)
> +{
> +	struct v4l2_bt_timings *act = &video->active_timings;
> +	struct rect_list *list = NULL;
> +	struct v4l2_rect *r;
> +
> +	list = kzalloc(sizeof(*list), GFP_KERNEL);
> +	if (!list)
> +		return NULL;
> +
> +	r = &list->clip.c;
> +
> +	r->left = (offset << 4);
> +	r->top = (index >> 2);
> +	r->width = RECT_W;
> +	r->height = RECT_H;
> +	if ((r->left + RECT_W) > act->width)
> +		r->width = act->width - r->left;
> +	if ((r->top + RECT_H) > act->height)
> +		r->height = act->height - r->top;
> +
> +	return list;
> +}
> +
> +static int nuvoton_video_find_rect(struct nuvoton_video *video,
> +				   struct rect_list_info *info, u32 offset)
> +{
> +	int i = info->index;
> +
> +	if (offset < info->tile_perline) {
> +		info->list = nuvoton_video_new_rect(video, offset, i);

`i` is only used here, so use `info->index`?

> +		if (!info->list)
> +			return -ENOMEM;
> +
> +		nuvoton_video_merge_rect(video, info);
> +	}
> +	return 0;
> +}
> +
> +static int nuvoton_video_build_table(struct nuvoton_video *video,
> +				     struct rect_list_info *info)
> +{
> +	int i = info->index;
> +	int j, ret, bit;

Make `j` unsigned?

> +	u32 value;
> +	struct regmap *vcd = video->vcd_regmap;
> +
> +	for (j = 0; j < info->offset_perline; j += 4) {
> +		regmap_read(vcd, VCD_DIFF_TBL + (j + i), &value);

`i` is only used here, so use `info->index`?

> +
> +		bitmap_from_arr32(bitmap, &value, BITMAP_SIZE);
> +
> +		for_each_set_bit(bit, bitmap, BITMAP_SIZE) {
> +			ret = nuvoton_video_find_rect(video, info,
> +						      bit + (j << 3));
> +			if (ret < 0)
> +				return ret;
> +		}
> +	}
> +	info->index += 64;
> +	return info->tile_perline;
> +}
> +
> +static int nuvoton_video_get_rect_list(struct nuvoton_video *video, int index)
> +{
> +	struct v4l2_bt_timings *act = &video->active_timings;
> +	struct rect_list_info info;
> +	int ret = 0;
> +	u32 mod, tile_cnt = 0;
> +
> +	memset(&info, 0, sizeof(struct rect_list_info));
> +	info.head = &video->list[index];
> +
> +	info.tile_perline = act->width >> 4;
> +	mod = act->width % RECT_W;
> +	if (mod != 0)
> +		info.tile_perline += 1;
> +
> +	info.tile_perrow = act->height >> 4;
> +	mod = act->height % RECT_H;
> +	if (mod != 0)
> +		info.tile_perrow += 1;
> +
> +	info.tile_size = info.tile_perrow * info.tile_perline;
> +
> +	info.offset_perline = info.tile_perline >> 5;
> +	mod = info.tile_perline % 32;
> +	if (mod != 0)
> +		info.offset_perline += 1;
> +
> +	info.offset_perline *= 4;
> +
> +	do {
> +		ret = nuvoton_video_build_table(video, &info);
> +		if (ret < 0)
> +			return ret;
> +		tile_cnt += ret;
> +	} while (tile_cnt < info.tile_size);
> +
> +	return ret;
> +}
> +
> +static u8 nuvoton_video_is_mga(struct nuvoton_video *video)
> +{
> +	struct regmap *gfxi = video->gfx_regmap;
> +	u32 dispst;
> +
> +	regmap_read(gfxi, DISPST, &dispst);
> +	return ((dispst & DISPST_MGAMODE) == DISPST_MGAMODE);
> +}
> +
> +static u32 nuvoton_video_hres(struct nuvoton_video *video)
> +{
> +	struct regmap *gfxi = video->gfx_regmap;
> +	u32 hvcnth, hvcntl, apb_hor_res;
> +
> +	regmap_read(gfxi, HVCNTH, &hvcnth);
> +	regmap_read(gfxi, HVCNTL, &hvcntl);
> +	apb_hor_res = (((hvcnth & HVCNTH_MASK) << 8) +
> +		       (hvcntl & HVCNTL_MASK) + 1);
> +
> +	return apb_hor_res;
> +}
> +
> +static u32 nuvoton_video_vres(struct nuvoton_video *video)
> +{
> +	struct regmap *gfxi = video->gfx_regmap;
> +	u32 vvcnth, vvcntl, apb_ver_res;
> +
> +	regmap_read(gfxi, VVCNTH, &vvcnth);
> +	regmap_read(gfxi, VVCNTL, &vvcntl);
> +
> +	apb_ver_res = (((vvcnth & VVCNTH_MASK) << 8) + (vvcntl & VVCNTL_MASK));
> +
> +	return apb_ver_res;
> +}
> +
> +static int nuvoton_video_capres(struct nuvoton_video *video, u32 hor_res,
> +				u32 vert_res)
> +{
> +	struct regmap *vcd = video->vcd_regmap;
> +	u32 res, cap_res;
> +
> +	if (hor_res > MAX_WIDTH || vert_res > MAX_HEIGHT)
> +		return -EINVAL;
> +
> +	res = FIELD_PREP(VCD_CAP_RES_VERT_RES, vert_res) |
> +	      FIELD_PREP(VCD_CAP_RES_HOR_RES, hor_res);
> +
> +	regmap_write(vcd, VCD_CAP_RES, res);
> +	regmap_read(vcd, VCD_CAP_RES, &cap_res);
> +
> +	if (cap_res != res)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static void nuvoton_video_vcd_ip_reset(struct nuvoton_video *video)
> +{
> +	reset_control_assert(video->reset);
> +	msleep(100);
> +	reset_control_deassert(video->reset);
> +	msleep(100);

100 ms is quite long. Please add a comment, where that is documented. Is 
there a way to poll, if the device is done?

> +}
> +
> +static void nuvoton_video_vcd_state_machine_reset(struct nuvoton_video *video)
> +{
> +	struct regmap *vcd = video->vcd_regmap;
> +	u32 stat;
> +	int ret;
> +
> +	regmap_update_bits(vcd, VCD_MODE, VCD_MODE_VCDE, 0);
> +
> +	regmap_update_bits(vcd, VCD_MODE, VCD_MODE_IDBC, 0);
> +
> +	regmap_update_bits(vcd, VCD_CMD, VCD_CMD_RST, VCD_CMD_RST);
> +
> +	ret = regmap_read_poll_timeout(vcd, VCD_STAT, stat,
> +				       (stat & VCD_STAT_DONE), 0,
> +				       ECE_POLL_TIMEOUT_US);
> +
> +	if (ret) {
> +		dev_warn(video->dev, "Wait for VCD_STAT_DONE timeout\n");
> +		return;
> +	}
> +
> +	regmap_write(vcd, VCD_STAT, VCD_STAT_CLEAR);
> +
> +	regmap_update_bits(vcd, VCD_MODE, VCD_MODE_VCDE, VCD_MODE_VCDE);
> +
> +	regmap_update_bits(vcd, VCD_MODE, VCD_MODE_IDBC, VCD_MODE_IDBC);
> +}
> +
> +static int nuvoton_video_gfx_reset(struct nuvoton_video *video)
> +{
> +	struct regmap *gcr = video->gcr_regmap;
> +
> +	regmap_update_bits(gcr, INTCR2, INTCR2_GIRST2, INTCR2_GIRST2);
> +
> +	nuvoton_video_vcd_state_machine_reset(video);
> +
> +	regmap_update_bits(gcr, INTCR2, INTCR2_GIRST2, 0);
> +
> +	return 0;
> +}
> +
> +static void nuvoton_video_kvm_bw(struct nuvoton_video *video, u8 bandwidth)
> +{
> +	struct regmap *vcd = video->vcd_regmap;
> +
> +	if (!nuvoton_video_is_mga(video))
> +		bandwidth = 1;
> +
> +	if (bandwidth)
> +		regmap_update_bits(vcd, VCD_MODE, VCD_MODE_KVM_BW_SET,
> +				   VCD_MODE_KVM_BW_SET);
> +	else
> +		regmap_update_bits(vcd, VCD_MODE, VCD_MODE_KVM_BW_SET, 0);
> +}
> +
> +static u32 nuvoton_video_pclk(struct nuvoton_video *video)
> +{
> +	struct regmap *gfxi = video->gfx_regmap;
> +	u32 tmp, pllfbdiv, pllinotdiv, gpllfbdiv;
> +	u8 gpllfbdv109, gpllfbdv8, gpllindiv;
> +	u8 gpllst_pllotdiv1, gpllst_pllotdiv2;
> +
> +	regmap_read(gfxi, GPLLST, &tmp);
> +	gpllfbdv109 = FIELD_GET(GPLLST_GPLLFBDV109, tmp);
> +	gpllst_pllotdiv1 = FIELD_GET(GPLLST_PLLOTDIV1, tmp);
> +	gpllst_pllotdiv2 = FIELD_GET(GPLLST_PLLOTDIV2, tmp);
> +
> +	regmap_read(gfxi, GPLLINDIV, &tmp);
> +	gpllfbdv8 = FIELD_GET(GPLLINDIV_GPLLFBDV8, tmp);
> +	gpllindiv = FIELD_GET(GPLLINDIV_MASK, tmp);
> +
> +	regmap_read(gfxi, GPLLFBDIV, &tmp);
> +	gpllfbdiv = FIELD_GET(GPLLFBDIV_MASK, tmp);
> +
> +	pllfbdiv = (512 * gpllfbdv109 + 256 * gpllfbdv8 + gpllfbdiv);
> +	pllinotdiv = (gpllindiv * gpllst_pllotdiv1 * gpllst_pllotdiv2);
> +	if (pllfbdiv == 0 || pllinotdiv == 0)
> +		return 0;
> +
> +	return ((pllfbdiv * 25000) / pllinotdiv) * 1000;
> +}
> +
> +static int nuvoton_video_get_bpp(struct nuvoton_video *video)
> +{
> +	struct regmap *vcd = video->vcd_regmap;
> +	u32 mode, color_cnvr;
> +
> +	regmap_read(vcd, VCD_MODE, &mode);
> +
> +	color_cnvr = FIELD_GET(VCD_MODE_COLOR_CNVRT, mode);
> +
> +	switch (color_cnvr) {
> +	case VCD_MODE_COLOR_CNVRT_NO_CNVRT:
> +		return 2;
> +	case VCD_MODE_COLOR_CNVRT_RGB_222:
> +	case VCD_MODE_COLOR_CNVRT_666_MODE:
> +		return 1;
> +	case VCD_MODE_COLOR_CNVRT_RGB_888:
> +		return 4;
> +	}
> +	return 0;
> +}
> +
> +/*
> + * Pitch must be a power of 2, >= linebytes,
> + * at least 512, and no more than 4096.
> + */
> +static void nuvoton_video_set_linepitch(struct nuvoton_video *video,
> +					u32 linebytes)
> +{
> +	struct regmap *vcd = video->vcd_regmap;
> +	u32 pitch = MIN_LP;
> +
> +	while ((pitch < linebytes) && (pitch < MAX_LP))
> +		pitch *= 2;
> +
> +	regmap_write(vcd, VCD_FB_LP, FIELD_PREP(VCD_FBA_LP, pitch) |
> +		     FIELD_PREP(VCD_FBB_LP, pitch));
> +}
> +
> +static u32 nuvoton_video_get_linepitch(struct nuvoton_video *video)
> +{
> +	struct regmap *vcd = video->vcd_regmap;
> +	u32 linepitch;
> +
> +	regmap_read(vcd, VCD_FB_LP, &linepitch);
> +
> +	return FIELD_GET(VCD_FBA_LP, linepitch);
> +}
> +
> +static int nuvoton_video_command(struct nuvoton_video *video, u32 value)
> +{
> +	struct regmap *vcd = video->vcd_regmap;
> +	u32 cmd;
> +
> +	regmap_write(vcd, VCD_STAT, VCD_STAT_CLEAR);
> +
> +	regmap_read(vcd, VCD_CMD, &cmd);
> +	cmd |= FIELD_PREP(VCD_CMD_OPERATION, value);
> +
> +	regmap_write(vcd, VCD_CMD, cmd);
> +	regmap_update_bits(vcd, VCD_CMD, VCD_CMD_GO, VCD_CMD_GO);
> +	video->op_cmd = value;
> +
> +	return 0;
> +}
> +
> +static int nuvoton_video_init_reg(struct nuvoton_video *video)
> +{
> +	struct regmap *gcr = video->gcr_regmap;
> +	struct regmap *vcd = video->vcd_regmap;
> +
> +	/* Selects Data Enable */
> +	regmap_update_bits(gcr, INTCR, INTCR_DEHS, 0);
> +
> +	/* Enable display of KVM GFX and access to memory */
> +	regmap_update_bits(gcr, INTCR, INTCR_GFXIFDIS, 0);
> +
> +	/* Active Vertical/Horizontal Counters Reset */
> +	regmap_update_bits(gcr, INTCR2, INTCR2_GIHCRST | INTCR2_GIVCRST,
> +			   INTCR2_GIHCRST | INTCR2_GIVCRST);
> +
> +	/* Select KVM GFX input */
> +	regmap_update_bits(gcr, MFSEL1, MFSEL1_DVH1SEL, 0);
> +
> +	/* Reset video modules */
> +	nuvoton_video_vcd_ip_reset(video);
> +	nuvoton_video_gfx_reset(video);
> +
> +	/* Set the FIFO thresholds */
> +	regmap_write(vcd, VCD_FIFO, VCD_FIFO_TH);
> +
> +	/* Set video mode */
> +	regmap_update_bits(vcd, VCD_MODE, 0xffffffff,
> +			   VCD_MODE_VCDE | VCD_MODE_CM565 |
> +			   VCD_MODE_IDBC | VCD_MODE_KVM_BW_SET);
> +
> +	regmap_write(vcd, VCD_RCHG, FIELD_PREP(VCD_RCHG_TIM_PRSCL, 0xf));
> +
> +	return 0;
> +}
> +
> +static int nuvoton_video_start_frame(struct nuvoton_video *video)
> +{
> +	unsigned long flags;
> +	struct nuvoton_video_buffer *buf;
> +	struct regmap *vcd = video->vcd_regmap;
> +	u32 val;
> +	int ret;
> +
> +	dev_dbg(video->dev, "%s\n", __func__);
> +
> +	if (video->v4l2_input_status) {
> +		dev_dbg(video->dev, "No video signal; skip capture frame\n");
> +		return 0;
> +	}
> +
> +	ret = regmap_read_poll_timeout(vcd, VCD_STAT, val,
> +				       !(val & VCD_STAT_BUSY), 1000,
> +				       VCD_BUSY_TIMEOUT_US);
> +
> +	if (ret) {
> +		dev_err(video->dev, "Wait for VCD_STAT_BUSY timeout\n");
> +		return -EBUSY;
> +	}
> +
> +	spin_lock_irqsave(&video->lock, flags);
> +	buf = list_first_entry_or_null(&video->buffers,
> +				       struct nuvoton_video_buffer, link);
> +	if (!buf) {
> +		spin_unlock_irqrestore(&video->lock, flags);
> +		dev_dbg(video->dev, "No empty buffers; skip capture frame\n");
> +		return 0;
> +	}
> +
> +	set_bit(VIDEO_FRAME_INPRG, &video->flags);
> +	spin_unlock_irqrestore(&video->lock, flags);
> +
> +	nuvoton_video_vcd_state_machine_reset(video);
> +
> +	regmap_update_bits(vcd, VCD_INTE, VCD_INTE_DONE_IE | VCD_INTE_IFOT_IE |
> +			   VCD_INTE_IFOR_IE, VCD_INTE_DONE_IE |
> +			   VCD_INTE_IFOT_IE | VCD_INTE_IFOR_IE);
> +
> +	nuvoton_video_command(video, video->ctrl_cmd);
> +
> +	return 0;
> +}
> +
> +static void nuvoton_video_bufs_done(struct nuvoton_video *video,
> +				    enum vb2_buffer_state state)
> +{
> +	unsigned long flags;
> +	struct nuvoton_video_buffer *buf;
> +
> +	spin_lock_irqsave(&video->lock, flags);
> +	list_for_each_entry(buf, &video->buffers, link)
> +		vb2_buffer_done(&buf->vb.vb2_buf, state);
> +	INIT_LIST_HEAD(&video->buffers);
> +	spin_unlock_irqrestore(&video->lock, flags);
> +}
> +
> +static void nuvoton_video_get_diff_rect(struct nuvoton_video *video, int index)
> +{
> +	u32 width = video->active_timings.width;
> +	u32 height = video->active_timings.height;
> +
> +	if (video->op_cmd != VCD_CMD_OPERATION_CAPTURE) {
> +		video->rect_cnt = 0;
> +		nuvoton_video_get_rect_list(video, index);
> +		video->rect[index] = video->rect_cnt;
> +	} else {
> +		video->rect[index] = nuvoton_video_add_rect(video, index, 0, 0,
> +							    width, height);
> +	}
> +}
> +
> +static irqreturn_t nuvoton_video_irq(int irq, void *arg)
> +{
> +	struct nuvoton_video *video = arg;
> +	struct regmap *vcd = video->vcd_regmap;
> +	struct nuvoton_video_buffer *buf;
> +	struct rect_list *rect_list;
> +	struct v4l2_rect *rect;
> +	u32 status, ed_offset, ed_size, total_size;
> +	void *addr;
> +	dma_addr_t vb_dma_addr;
> +	int index;
> +
> +	regmap_read(vcd, VCD_STAT, &status);
> +	dev_dbg(video->dev, "VCD irq status 0x%x\n", status);
> +
> +	regmap_write(vcd, VCD_INTE, 0);
> +	regmap_write(vcd, VCD_STAT, VCD_STAT_CLEAR);
> +
> +	if (test_bit(VIDEO_STOPPED, &video->flags) ||
> +	    !test_bit(VIDEO_STREAMING, &video->flags)) {
> +		clear_bit(VIDEO_FRAME_INPRG, &video->flags);
> +		return IRQ_NONE;
> +	}
> +
> +	if (status & VCD_STAT_DONE) {
> +		spin_lock(&video->lock);
> +		buf = list_first_entry_or_null(&video->buffers,
> +					       struct nuvoton_video_buffer,
> +					       link);
> +
> +		if (!buf) {
> +			spin_unlock(&video->lock);
> +			clear_bit(VIDEO_FRAME_INPRG, &video->flags);
> +			return IRQ_NONE;
> +		}
> +
> +		addr = vb2_plane_vaddr(&buf->vb.vb2_buf, 0);
> +		vb_dma_addr = vb2_dma_contig_plane_dma_addr(&buf->vb.vb2_buf, 0);
> +		index = buf->vb.vb2_buf.index;
> +
> +		nuvoton_video_ece_ctrl_reset(video);
> +		nuvoton_video_ece_clear_rect_offset(video);
> +
> +		nuvoton_video_ece_set_fb_addr(video, video->src.dma);
> +
> +		/* Set base address of encoded data to video buffer */
> +		nuvoton_video_ece_set_enc_dba(video, vb_dma_addr);
> +
> +		nuvoton_video_ece_set_lp(video, video->bytesperline);
> +		nuvoton_video_get_diff_rect(video, index);
> +
> +		total_size = 0;
> +
> +		list_for_each_entry(rect_list, &video->list[index], list) {
> +			rect = &rect_list->clip.c;
> +			ed_offset = nuvoton_video_ece_read_rect_offset(video);
> +
> +			nuvoton_video_ece_enc_rect(video, rect->left,
> +						   rect->top, rect->width,
> +						   rect->height);
> +			ed_size = nuvoton_video_ece_get_ed_size(video,
> +								ed_offset,
> +								addr);
> +
> +			nuvoton_video_ece_prepend_rect_header(addr + ed_offset,
> +							      rect->left,
> +							      rect->top,
> +							      rect->width,
> +							      rect->height);
> +
> +			total_size += ed_size;
> +		}
> +
> +		vb2_set_plane_payload(&buf->vb.vb2_buf, 0, total_size);
> +		buf->vb.vb2_buf.timestamp = ktime_get_ns();
> +		buf->vb.sequence = video->sequence++;
> +		buf->vb.field = V4L2_FIELD_NONE;
> +		vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_DONE);
> +		list_del(&buf->link);
> +
> +		spin_unlock(&video->lock);
> +
> +		clear_bit(VIDEO_FRAME_INPRG, &video->flags);
> +	}
> +
> +	if (status & VCD_STAT_IFOR || status & VCD_STAT_IFOT) {
> +		dev_warn(video->dev, "VCD FIFO overrun or over thresholds\n");
> +		nuvoton_video_vcd_ip_reset(video);
> +		nuvoton_video_gfx_reset(video);
> +		nuvoton_video_start_frame(video);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static void nuvoton_video_clear_gmmap(struct nuvoton_video *video)
> +{
> +	struct regmap *gcr = video->gcr_regmap;
> +	u32 intcr3, gmmap;
> +	void __iomem *baseptr;
> +
> +	regmap_read(gcr, INTCR3, &intcr3);
> +	gmmap = FIELD_GET(INTCR3_GMMAP, intcr3);
> +
> +	switch (gmmap) {
> +	case INTCR3_GMMAP_128MB:
> +		baseptr = ioremap(ADDR_GMMAP_128MB, GMMAP_LENGTH);
> +		break;
> +	case INTCR3_GMMAP_256MB:
> +		baseptr = ioremap(ADDR_GMMAP_256MB, GMMAP_LENGTH);
> +		break;
> +	case INTCR3_GMMAP_512MB:
> +		baseptr = ioremap(ADDR_GMMAP_512MB, GMMAP_LENGTH);
> +		break;
> +	case INTCR3_GMMAP_1GB:
> +		baseptr = ioremap(ADDR_GMMAP_1GB, GMMAP_LENGTH);
> +		break;
> +	case INTCR3_GMMAP_2GB:
> +		baseptr = ioremap(ADDR_GMMAP_2GB, GMMAP_LENGTH);
> +		break;
> +	}
> +
> +	memset(baseptr, 0, GMMAP_LENGTH);
> +	iounmap(baseptr);
> +}
> +
> +static void nuvoton_video_get_resolution(struct nuvoton_video *video)
> +{
> +	struct v4l2_bt_timings *act = &video->active_timings;
> +	struct v4l2_bt_timings *det = &video->detected_timings;
> +	struct regmap *gfxi;
> +	u32 dispst;
> +
> +	video->v4l2_input_status = 0;
> +
> +	det->width = nuvoton_video_hres(video);
> +	det->height = nuvoton_video_vres(video);
> +
> +	if (act->width != det->width || act->height != det->height) {
> +		dev_dbg(video->dev, "Resolution changed\n");
> +
> +		nuvoton_video_bufs_done(video, VB2_BUF_STATE_ERROR);
> +
> +		if (nuvoton_video_hres(video) > 0 &&
> +		    nuvoton_video_vres(video) > 0) {
> +			//struct regmap *gfxi = video->gfx_regmap;
> +			gfxi = video->gfx_regmap;
> +			//u32 dispst;
> +
> +			if (test_bit(VIDEO_STREAMING, &video->flags)) {
> +				/*
> +				 * Wait for resolution is available,
> +				 * and it is also captured by host.
> +				 */
> +				do {
> +					mdelay(100);
> +					regmap_read(gfxi, DISPST, &dispst);
> +				} while (nuvoton_video_vres(video) < 100 ||
> +					 nuvoton_video_pclk(video) == 0 ||
> +					 (dispst & DISPST_HSCROFF));
> +			}
> +
> +			det->width = nuvoton_video_hres(video);
> +			det->height = nuvoton_video_vres(video);
> +			det->pixelclock = nuvoton_video_pclk(video);
> +		}
> +	}
> +
> +	if (det->width == 0 || det->height == 0) {
> +		det->width = MIN_WIDTH;
> +		det->height = MIN_HEIGHT;
> +		nuvoton_video_clear_gmmap(video);
> +		video->v4l2_input_status = V4L2_IN_ST_NO_SIGNAL;
> +	}
> +
> +	dev_dbg(video->dev, "Got resolution[%dx%d] -> [%dx%d], status %d\n",
> +		act->width, act->height, det->width, det->height,
> +		video->v4l2_input_status);
> +}
> +
> +static void nuvoton_video_set_resolution(struct nuvoton_video *video)
> +{
> +	struct v4l2_bt_timings *act = &video->active_timings;
> +	struct regmap *vcd = video->vcd_regmap;
> +	u32 mode;
> +
> +	/* Set video frame physical address */
> +	regmap_write(vcd, VCD_FBA_ADR, video->src.dma);
> +	regmap_write(vcd, VCD_FBB_ADR, video->src.dma);
> +
> +	nuvoton_video_capres(video, act->width, act->height);
> +
> +	video->bytesperpixel = nuvoton_video_get_bpp(video);
> +	nuvoton_video_set_linepitch(video, act->width * video->bytesperpixel);
> +
> +	video->bytesperline = nuvoton_video_get_linepitch(video);
> +
> +	nuvoton_video_kvm_bw(video, act->pixelclock > VCD_KVM_BW_PCLK);
> +
> +	nuvoton_video_gfx_reset(video);
> +
> +	regmap_read(vcd, VCD_MODE, &mode);
> +
> +	clear_bit(VIDEO_FRAME_INPRG, &video->flags);
> +
> +	dev_dbg(video->dev, "VCD mode = 0x%x, %s mode\n", mode,
> +		nuvoton_video_is_mga(video) ? "Hi Res" : "VGA");
> +
> +	dev_dbg(video->dev,
> +		"Digital mode: %d x %d x %d, pixelclock %lld, bytesperline %d\n",
> +		act->width, act->height, video->bytesperpixel, act->pixelclock,
> +		video->bytesperline);
> +}
> +
> +static int nuvoton_video_start(struct nuvoton_video *video)
> +{
> +	int rc;
> +
> +	dev_dbg(video->dev, "%s\n", __func__);
> +
> +	rc = nuvoton_video_init_reg(video);
> +	if (rc)
> +		return rc;
> +
> +	nuvoton_video_get_resolution(video);
> +
> +	video->active_timings = video->detected_timings;
> +
> +	video->max_buffer_size = VCD_MAX_SRC_BUFFER_SIZE;
> +	if (!nuvoton_video_alloc_buf(video, &video->src,
> +				     video->max_buffer_size))
> +		return -ENOMEM;
> +
> +	nuvoton_video_set_resolution(video);
> +
> +	video->pix_fmt.width = video->active_timings.width;
> +	video->pix_fmt.height = video->active_timings.height;
> +	video->pix_fmt.sizeimage = video->max_buffer_size;
> +	video->pix_fmt.bytesperline = video->bytesperline;
> +
> +	if (atomic_inc_return(&video->ece.clients) == 1) {
> +		nuvoton_video_ece_init(video);
> +		nuvoton_video_ece_set_fb_addr(video, video->src.dma);
> +		nuvoton_video_ece_set_lp(video, video->bytesperline);
> +
> +		dev_dbg(video->dev, "ECE open: client %d\n",
> +			atomic_read(&video->ece.clients));
> +	}
> +
> +	return 0;
> +}
> +
> +static void nuvoton_video_stop(struct nuvoton_video *video)
> +{
> +	unsigned long flags;
> +	struct regmap *vcd = video->vcd_regmap;
> +
> +	dev_dbg(video->dev, "%s\n", __func__);
> +
> +	spin_lock_irqsave(&video->lock, flags);
> +	set_bit(VIDEO_STOPPED, &video->flags);
> +	spin_unlock_irqrestore(&video->lock, flags);
> +
> +	regmap_write(vcd, VCD_INTE, 0);
> +	regmap_write(vcd, VCD_MODE, 0);
> +	regmap_write(vcd, VCD_RCHG, 0);
> +	regmap_write(vcd, VCD_STAT, VCD_STAT_CLEAR);
> +
> +	if (video->src.size)
> +		nuvoton_video_free_buf(video, &video->src);
> +
> +	if (video->list)
> +		nuvoton_video_free_diff_table(video);
> +
> +	kfree(video->list);
> +	video->list = NULL;
> +
> +	kfree(video->rect);
> +	video->rect = NULL;
> +
> +	video->v4l2_input_status = V4L2_IN_ST_NO_SIGNAL;
> +	video->flags = 0;
> +	video->ctrl_cmd = VCD_CMD_OPERATION_CAPTURE;
> +
> +	if (atomic_dec_return(&video->ece.clients) == 0) {
> +		nuvoton_video_ece_stop(video);
> +		dev_dbg(video->dev, "ECE close: client %d\n",
> +			atomic_read(&video->ece.clients));
> +	}
> +}
> +
> +static int nuvoton_video_querycap(struct file *file, void *fh,
> +				  struct v4l2_capability *cap)
> +{
> +	strscpy(cap->driver, DEVICE_NAME, sizeof(cap->driver));
> +	strscpy(cap->card, "Nuvoton Video Engine", sizeof(cap->card));
> +	snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s",
> +		 DEVICE_NAME);
> +
> +	return 0;
> +}
> +
> +static int nuvoton_video_enum_format(struct file *file, void *fh,
> +				     struct v4l2_fmtdesc *f)
> +{
> +	if (f->index)
> +		return -EINVAL;
> +
> +	f->pixelformat = V4L2_PIX_FMT_RGB565;
> +
> +	return 0;
> +}
> +
> +static int nuvoton_video_get_format(struct file *file, void *fh,
> +				    struct v4l2_format *f)
> +{
> +	struct nuvoton_video *video = video_drvdata(file);
> +
> +	f->fmt.pix = video->pix_fmt;
> +
> +	return 0;
> +}
> +
> +static int nuvoton_video_enum_input(struct file *file, void *fh,
> +				    struct v4l2_input *inp)
> +{
> +	struct nuvoton_video *video = video_drvdata(file);
> +
> +	if (inp->index)
> +		return -EINVAL;
> +
> +	strlcpy(inp->name, "Host VGA capture", sizeof(inp->name));
> +	inp->type = V4L2_INPUT_TYPE_CAMERA;
> +	inp->capabilities = V4L2_IN_CAP_DV_TIMINGS;
> +	inp->status = video->v4l2_input_status;
> +
> +	return 0;
> +}
> +
> +static int nuvoton_video_get_input(struct file *file, void *fh, unsigned int *i)
> +{
> +	*i = 0;
> +
> +	return 0;
> +}
> +
> +static int nuvoton_video_set_input(struct file *file, void *fh, unsigned int i)
> +{
> +	if (i)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static int nuvoton_video_get_parm(struct file *file, void *fh,
> +				  struct v4l2_streamparm *a)
> +{
> +	struct nuvoton_video *video = video_drvdata(file);
> +
> +	a->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
> +	a->parm.capture.readbuffers = 3;
> +	a->parm.capture.timeperframe.numerator = 1;
> +	if (!video->frame_rate)
> +		a->parm.capture.timeperframe.denominator = MAX_FRAME_RATE;
> +	else
> +		a->parm.capture.timeperframe.denominator = video->frame_rate;
> +
> +	return 0;
> +}
> +
> +static int nuvoton_video_set_parm(struct file *file, void *fh,
> +				  struct v4l2_streamparm *a)
> +{
> +	unsigned int frame_rate = 0;
> +
> +	a->parm.capture.capability = V4L2_CAP_TIMEPERFRAME;
> +	a->parm.capture.readbuffers = 3;
> +
> +	if (a->parm.capture.timeperframe.numerator)
> +		frame_rate = a->parm.capture.timeperframe.denominator /
> +			     a->parm.capture.timeperframe.numerator;
> +
> +	if (!frame_rate || frame_rate > MAX_FRAME_RATE) {
> +		frame_rate = 0;
> +		a->parm.capture.timeperframe.denominator = MAX_FRAME_RATE;
> +		a->parm.capture.timeperframe.numerator = 1;
> +	}
> +
> +	return 0;
> +}
> +
> +static int nuvoton_video_enum_framesizes(struct file *file, void *fh,
> +					 struct v4l2_frmsizeenum *fsize)
> +{
> +	struct nuvoton_video *video = video_drvdata(file);
> +
> +	if (fsize->index)
> +		return -EINVAL;
> +
> +	if (fsize->pixel_format != V4L2_PIX_FMT_RGB565)
> +		return -EINVAL;
> +
> +	fsize->discrete.width = video->pix_fmt.width;
> +	fsize->discrete.height = video->pix_fmt.height;
> +	fsize->type = V4L2_FRMSIZE_TYPE_DISCRETE;
> +
> +	return 0;
> +}
> +
> +static int nuvoton_video_enum_frameintervals(struct file *file, void *fh,
> +					     struct v4l2_frmivalenum *fival)
> +{
> +	struct nuvoton_video *video = video_drvdata(file);
> +
> +	if (fival->index)
> +		return -EINVAL;
> +
> +	if (fival->width != video->detected_timings.width ||
> +	    fival->height != video->detected_timings.height)
> +		return -EINVAL;
> +
> +	if (fival->pixel_format != V4L2_PIX_FMT_RGB565)
> +		return -EINVAL;
> +
> +	fival->type = V4L2_FRMIVAL_TYPE_CONTINUOUS;
> +
> +	fival->stepwise.min.denominator = MAX_FRAME_RATE;
> +	fival->stepwise.min.numerator = 1;
> +	fival->stepwise.max.denominator = 1;
> +	fival->stepwise.max.numerator = 1;
> +	fival->stepwise.step = fival->stepwise.max;
> +
> +	return 0;
> +}
> +
> +static int nuvoton_video_get_vid_overlay(struct file *file, void *fh,
> +					 struct v4l2_format *fmt)
> +{
> +	struct nuvoton_video *video = video_drvdata(file);
> +	struct v4l2_window *win = &fmt->fmt.win;
> +	struct list_head *head, *pos, *nx;
> +	struct rect_list *entry, *tmp;
> +	struct v4l2_rect *rect;
> +
> +	if (video->list && video->rect) {
> +		win->clipcount = video->rect[video->vb_index];
> +		head = &video->list[video->vb_index];
> +
> +		entry = list_first_entry_or_null(head, struct rect_list, list);
> +		if (entry) {
> +			//struct v4l2_rect *r = &entry->clip.c;
> +			rect = &entry->clip.c;
> +
> +			win->w.top = rect->top;
> +			win->w.left = rect->left;
> +			win->w.width = rect->width;
> +			win->w.height = rect->height;
> +
> +			list_del(&entry->list);
> +			kfree(entry);
> +			if (video->rect[video->vb_index])
> +				video->rect[video->vb_index]--;
> +		}
> +
> +		list_for_each_safe(pos, nx, head) {
> +			tmp = list_entry(pos, struct rect_list, list);
> +			if (tmp) {
> +				list_del(&tmp->list);
> +				kfree(tmp);
> +			}
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int nuvoton_video_set_dv_timings(struct file *file, void *fh,
> +					struct v4l2_dv_timings *timings)
> +{
> +	struct nuvoton_video *video = video_drvdata(file);
> +
> +	if (timings->bt.width == video->active_timings.width &&
> +	    timings->bt.height == video->active_timings.height)
> +		return 0;
> +
> +	if (vb2_is_busy(&video->queue)) {
> +		dev_err(video->dev, "%s device busy\n", __func__);
> +		return -EBUSY;
> +	}
> +
> +	video->active_timings = timings->bt;
> +
> +	nuvoton_video_set_resolution(video);
> +
> +	video->pix_fmt.width = timings->bt.width;
> +	video->pix_fmt.height = timings->bt.height;
> +	video->pix_fmt.sizeimage = video->max_buffer_size;
> +	video->pix_fmt.bytesperline = video->bytesperline;
> +
> +	timings->type = V4L2_DV_BT_656_1120;
> +
> +	return 0;
> +}
> +
> +static int nuvoton_video_get_dv_timings(struct file *file, void *fh,
> +					struct v4l2_dv_timings *timings)
> +{
> +	struct nuvoton_video *video = video_drvdata(file);
> +
> +	timings->type = V4L2_DV_BT_656_1120;
> +	timings->bt = video->active_timings;
> +
> +	return 0;
> +}
> +
> +static int nuvoton_video_query_dv_timings(struct file *file, void *fh,
> +					  struct v4l2_dv_timings *timings)
> +{
> +	struct nuvoton_video *video = video_drvdata(file);
> +
> +	nuvoton_video_get_resolution(video);
> +
> +	timings->type = V4L2_DV_BT_656_1120;
> +	timings->bt = video->detected_timings;
> +
> +	return video->v4l2_input_status ? -ENOLINK : 0;
> +}
> +
> +static int nuvoton_video_enum_dv_timings(struct file *file, void *fh,
> +					 struct v4l2_enum_dv_timings *timings)
> +{
> +	return v4l2_enum_dv_timings_cap(timings, &nuvoton_video_timings_cap,
> +					NULL, NULL);
> +}
> +
> +static int nuvoton_video_dv_timings_cap(struct file *file, void *fh,
> +					struct v4l2_dv_timings_cap *cap)
> +{
> +	*cap = nuvoton_video_timings_cap;
> +
> +	return 0;
> +}
> +
> +static int nuvoton_video_sub_event(struct v4l2_fh *fh,
> +				   const struct v4l2_event_subscription *sub)
> +{
> +	switch (sub->type) {
> +	case V4L2_EVENT_SOURCE_CHANGE:
> +		return v4l2_src_change_event_subscribe(fh, sub);
> +	}
> +
> +	return v4l2_ctrl_subscribe_event(fh, sub);
> +}
> +
> +static const struct v4l2_ioctl_ops nuvoton_video_ioctls = {
> +	.vidioc_querycap = nuvoton_video_querycap,
> +
> +	.vidioc_enum_fmt_vid_cap = nuvoton_video_enum_format,
> +	.vidioc_g_fmt_vid_cap = nuvoton_video_get_format,
> +	.vidioc_s_fmt_vid_cap = nuvoton_video_get_format,
> +	.vidioc_try_fmt_vid_cap = nuvoton_video_get_format,
> +
> +	.vidioc_reqbufs = vb2_ioctl_reqbufs,
> +	.vidioc_querybuf = vb2_ioctl_querybuf,
> +	.vidioc_qbuf = vb2_ioctl_qbuf,
> +	.vidioc_expbuf = vb2_ioctl_expbuf,
> +	.vidioc_dqbuf = vb2_ioctl_dqbuf,
> +	.vidioc_create_bufs = vb2_ioctl_create_bufs,
> +	.vidioc_prepare_buf = vb2_ioctl_prepare_buf,
> +	.vidioc_streamon = vb2_ioctl_streamon,
> +	.vidioc_streamoff = vb2_ioctl_streamoff,
> +
> +	.vidioc_enum_input = nuvoton_video_enum_input,
> +	.vidioc_g_input = nuvoton_video_get_input,
> +	.vidioc_s_input = nuvoton_video_set_input,
> +
> +	.vidioc_g_parm = nuvoton_video_get_parm,
> +	.vidioc_s_parm = nuvoton_video_set_parm,
> +	.vidioc_g_fmt_vid_overlay = nuvoton_video_get_vid_overlay,
> +	.vidioc_enum_framesizes = nuvoton_video_enum_framesizes,
> +	.vidioc_enum_frameintervals = nuvoton_video_enum_frameintervals,
> +
> +	.vidioc_s_dv_timings = nuvoton_video_set_dv_timings,
> +	.vidioc_g_dv_timings = nuvoton_video_get_dv_timings,
> +	.vidioc_query_dv_timings = nuvoton_video_query_dv_timings,
> +	.vidioc_enum_dv_timings = nuvoton_video_enum_dv_timings,
> +	.vidioc_dv_timings_cap = nuvoton_video_dv_timings_cap,
> +
> +	.vidioc_subscribe_event = nuvoton_video_sub_event,
> +	.vidioc_unsubscribe_event = v4l2_event_unsubscribe,
> +};
> +
> +static int nuvoton_video_set_ctrl(struct v4l2_ctrl *ctrl)
> +{
> +	struct nuvoton_video *video = container_of(ctrl->handler,
> +						   struct nuvoton_video,
> +						   ctrl_handler);
> +
> +	switch (ctrl->id) {
> +	case V4L2_CID_DETECT_MD_MODE:
> +		if (ctrl->val == V4L2_DETECT_MD_MODE_GLOBAL)
> +			video->ctrl_cmd = VCD_CMD_OPERATION_CAPTURE;
> +		else
> +			video->ctrl_cmd = VCD_CMD_OPERATION_COMPARE;
> +	break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct v4l2_ctrl_ops nuvoton_video_ctrl_ops = {
> +	.s_ctrl = nuvoton_video_set_ctrl,
> +};
> +
> +static int nuvoton_video_open(struct file *file)
> +{
> +	int rc;
> +	struct nuvoton_video *video = video_drvdata(file);
> +
> +	mutex_lock(&video->video_lock);
> +
> +	rc = v4l2_fh_open(file);
> +	if (rc) {
> +		mutex_unlock(&video->video_lock);
> +		return rc;
> +	}
> +
> +	if (v4l2_fh_is_singular_file(file))
> +		nuvoton_video_start(video);
> +
> +	mutex_unlock(&video->video_lock);
> +
> +	return 0;
> +}
> +
> +static int nuvoton_video_release(struct file *file)
> +{
> +	int rc;
> +	struct nuvoton_video *video = video_drvdata(file);
> +
> +	mutex_lock(&video->video_lock);
> +
> +	if (v4l2_fh_is_singular_file(file))
> +		nuvoton_video_stop(video);
> +
> +	rc = _vb2_fop_release(file, NULL);
> +
> +	mutex_unlock(&video->video_lock);
> +
> +	return rc;
> +}
> +
> +static const struct v4l2_file_operations nuvoton_video_v4l2_fops = {
> +	.owner = THIS_MODULE,
> +	.read = vb2_fop_read,
> +	.poll = vb2_fop_poll,
> +	.unlocked_ioctl = video_ioctl2,
> +	.mmap = vb2_fop_mmap,
> +	.open = nuvoton_video_open,
> +	.release = nuvoton_video_release,
> +};
> +
> +static int nuvoton_video_queue_setup(struct vb2_queue *q,
> +				     unsigned int *num_buffers,
> +				     unsigned int *num_planes,
> +				     unsigned int sizes[],
> +				     struct device *alloc_devs[])
> +{
> +	struct nuvoton_video *video = vb2_get_drv_priv(q);
> +	int i;

unsigned?

> +
> +	dev_dbg(video->dev, "%s\n", __func__);
> +
> +	if (*num_planes) {
> +		if (sizes[0] < video->max_buffer_size)
> +			return -EINVAL;
> +
> +		return 0;
> +	}
> +
> +	*num_planes = 1;
> +	sizes[0] = video->max_buffer_size;
> +
> +	kfree(video->rect);
> +	video->rect = NULL;
> +
> +	video->rect = kcalloc(*num_buffers, sizeof(*video->rect), GFP_KERNEL);
> +
> +	if (video->list) {
> +		nuvoton_video_free_diff_table(video);
> +		kfree(video->list);
> +		video->list = NULL;
> +	}
> +
> +	video->list = kzalloc(sizeof(*video->list) * *num_buffers, GFP_KERNEL);
> +
> +	for (i = 0; i < *num_buffers; i++)
> +		INIT_LIST_HEAD(&video->list[i]);
> +
> +	video->num_buffers = *num_buffers;
> +
> +	return 0;
> +}
> +
> +static int nuvoton_video_buf_prepare(struct vb2_buffer *vb)
> +{
> +	struct nuvoton_video *video = vb2_get_drv_priv(vb->vb2_queue);
> +
> +	if (vb2_plane_size(vb, 0) < video->max_buffer_size)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static int nuvoton_video_start_streaming(struct vb2_queue *q,
> +					 unsigned int count)
> +{
> +	int rc;
> +	struct nuvoton_video *video = vb2_get_drv_priv(q);
> +
> +	dev_dbg(video->dev, "%s\n", __func__);
> +
> +	video->sequence = 0;
> +
> +	rc = nuvoton_video_start_frame(video);
> +	if (rc) {
> +		nuvoton_video_bufs_done(video, VB2_BUF_STATE_QUEUED);
> +		return rc;
> +	}
> +
> +	set_bit(VIDEO_STREAMING, &video->flags);
> +	return 0;
> +}
> +
> +static void nuvoton_video_stop_streaming(struct vb2_queue *q)
> +{
> +	struct nuvoton_video *video = vb2_get_drv_priv(q);
> +	struct regmap *vcd = video->vcd_regmap;
> +
> +	dev_dbg(video->dev, "%s\n", __func__);
> +
> +	clear_bit(VIDEO_STREAMING, &video->flags);
> +
> +	regmap_write(vcd, VCD_INTE, 0);
> +	regmap_write(vcd, VCD_STAT, VCD_STAT_CLEAR);
> +	nuvoton_video_gfx_reset(video);
> +
> +	nuvoton_video_bufs_done(video, VB2_BUF_STATE_ERROR);
> +
> +	video->ctrl_cmd = VCD_CMD_OPERATION_CAPTURE;
> +}
> +
> +static void nuvoton_video_buf_queue(struct vb2_buffer *vb)
> +{
> +	int empty;
> +	struct nuvoton_video *video = vb2_get_drv_priv(vb->vb2_queue);
> +	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> +	struct nuvoton_video_buffer *nvb = to_nuvoton_video_buffer(vbuf);
> +	unsigned long flags;
> +
> +	dev_dbg(video->dev, "%s\n", __func__);
> +
> +	spin_lock_irqsave(&video->lock, flags);
> +	empty = list_empty(&video->buffers);

Where is empty read later?

> +	list_add_tail(&nvb->link, &video->buffers);
> +	spin_unlock_irqrestore(&video->lock, flags);
> +}
> +
> +static void nuvoton_video_buf_finish(struct vb2_buffer *vb)
> +{
> +	struct nuvoton_video *video = vb2_get_drv_priv(vb->vb2_queue);
> +	struct regmap *vcd = video->vcd_regmap;
> +	u32 val;
> +	int ret;
> +
> +	dev_dbg(video->dev, "%s\n", __func__);

I think ftrace can be used to trace functions (though this is static).

> +
> +	ret = regmap_read_poll_timeout(vcd, VCD_STAT, val,
> +				       !(val & VCD_STAT_BUSY), 1000,
> +				       VCD_BUSY_TIMEOUT_US);
> +
> +	if (ret) {
> +		dev_warn(video->dev, "Wait for VCD_STAT_BUSY timeout\n");
> +		return;
> +	}
> +
> +	/* Capture next frame when a video buffer is dequeued */
> +	nuvoton_video_start_frame(video);
> +
> +	video->vb_index = vb->index;
> +}
> +
> +static const struct vb2_ops nuvoton_video_vb2_ops = {
> +	.queue_setup = nuvoton_video_queue_setup,
> +	.wait_prepare = vb2_ops_wait_prepare,
> +	.wait_finish = vb2_ops_wait_finish,
> +	.buf_prepare = nuvoton_video_buf_prepare,
> +	.buf_finish = nuvoton_video_buf_finish,
> +	.start_streaming = nuvoton_video_start_streaming,
> +	.stop_streaming = nuvoton_video_stop_streaming,
> +	.buf_queue =  nuvoton_video_buf_queue,
> +};
> +
> +static int nuvoton_video_setup_video(struct nuvoton_video *video)
> +{
> +	struct v4l2_device *v4l2_dev = &video->v4l2_dev;
> +	struct video_device *vdev = &video->vdev;
> +	struct vb2_queue *vbq = &video->queue;
> +	int rc;
> +
> +	video->pix_fmt.pixelformat = V4L2_PIX_FMT_RGB565;
> +	video->pix_fmt.field = V4L2_FIELD_NONE;
> +	video->pix_fmt.colorspace = V4L2_COLORSPACE_SRGB;
> +	video->v4l2_input_status = V4L2_IN_ST_NO_SIGNAL;
> +
> +	rc = v4l2_device_register(video->dev, v4l2_dev);
> +	if (rc) {
> +		dev_err(video->dev, "Failed to register v4l2 device\n");
> +		return rc;
> +	}
> +
> +	v4l2_ctrl_handler_init(&video->ctrl_handler, 10);
> +
> +	v4l2_ctrl_new_std_menu(&video->ctrl_handler, &nuvoton_video_ctrl_ops,
> +			       V4L2_CID_DETECT_MD_MODE,
> +			       V4L2_DETECT_MD_MODE_REGION_GRID, 0,
> +			       V4L2_DETECT_MD_MODE_GLOBAL);
> +
> +	if (video->ctrl_handler.error) {
> +		dev_err(video->dev, "Failed to init controls: %d\n",
> +			video->ctrl_handler.error);
> +
> +		rc = video->ctrl_handler.error;
> +
> +		v4l2_ctrl_handler_free(&video->ctrl_handler);
> +		v4l2_device_unregister(v4l2_dev);
> +
> +		return rc;
> +	}
> +
> +	v4l2_dev->ctrl_handler = &video->ctrl_handler;
> +
> +	vbq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> +	vbq->io_modes = VB2_MMAP | VB2_READ | VB2_DMABUF;
> +	vbq->dev = v4l2_dev->dev;
> +	vbq->lock = &video->video_lock;
> +	vbq->ops = &nuvoton_video_vb2_ops;
> +	vbq->mem_ops = &vb2_dma_contig_memops;
> +	vbq->drv_priv = video;
> +	vbq->buf_struct_size = sizeof(struct nuvoton_video_buffer);
> +	vbq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> +	vbq->min_buffers_needed = 3;
> +
> +	rc = vb2_queue_init(vbq);
> +	if (rc) {
> +		v4l2_ctrl_handler_free(&video->ctrl_handler);
> +		v4l2_device_unregister(v4l2_dev);
> +
> +		dev_err(video->dev, "Failed to init vb2 queue\n");
> +		return rc;
> +	}
> +
> +	vdev->queue = vbq;
> +	vdev->fops = &nuvoton_video_v4l2_fops;
> +	vdev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_READWRITE |
> +			    V4L2_CAP_STREAMING;
> +	vdev->v4l2_dev = v4l2_dev;
> +	strscpy(vdev->name, DEVICE_NAME, sizeof(vdev->name));
> +	vdev->vfl_type = VFL_TYPE_VIDEO;
> +	vdev->vfl_dir = VFL_DIR_RX;
> +	vdev->release = video_device_release_empty;
> +	vdev->ioctl_ops = &nuvoton_video_ioctls;
> +	vdev->lock = &video->video_lock;
> +
> +	video_set_drvdata(vdev, video);
> +	rc = video_register_device(vdev, VFL_TYPE_VIDEO, 0);
> +	if (rc) {
> +		vb2_queue_release(vbq);
> +		v4l2_ctrl_handler_free(&video->ctrl_handler);
> +		v4l2_device_unregister(v4l2_dev);
> +
> +		dev_err(video->dev, "Failed to register video device\n");
> +		return rc;
> +	}
> +
> +	return 0;
> +}
> +
> +static int nuvoton_video_init(struct nuvoton_video *video)
> +{
> +	int irq;
> +	int rc;
> +	struct device *dev = video->dev;
> +
> +	irq = irq_of_parse_and_map(dev->of_node, 0);
> +	if (!irq) {
> +		dev_err(dev, "Unable to find VCD IRQ\n");
> +		return -ENODEV;
> +	}
> +
> +	rc = devm_request_threaded_irq(dev, irq, NULL, nuvoton_video_irq,
> +				       IRQF_ONESHOT, DEVICE_NAME, video);
> +	if (rc < 0) {
> +		dev_err(dev, "Unable to request IRQ %d\n", irq);
> +		return rc;
> +	}
> +
> +	of_reserved_mem_device_init(dev);
> +
> +	rc = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> +	if (rc) {
> +		dev_err(dev, "Failed to set DMA mask\n");
> +		of_reserved_mem_device_release(dev);
> +	}
> +
> +	return rc;
> +}
> +
> +static const struct regmap_config nuvoton_video_regmap_cfg = {
> +	.reg_bits	= 32,
> +	.reg_stride	= 4,
> +	.val_bits	= 32,
> +	.max_register	= VCD_FIFO,
> +};
> +
> +static const struct regmap_config nuvoton_video_ece_regmap_cfg = {
> +	.reg_bits	= 32,
> +	.reg_stride	= 4,
> +	.val_bits	= 32,
> +	.max_register	= ECE_HEX_RECT_OFFSET,
> +};
> +
> +static int nuvoton_video_probe(struct platform_device *pdev)
> +{
> +	int rc;
> +	void __iomem *regs;
> +	struct nuvoton_video *video = kzalloc(sizeof(*video), GFP_KERNEL);
> +
> +	if (!video)
> +		return -ENOMEM;
> +
> +	video->frame_rate = MAX_FRAME_RATE;
> +	video->dev = &pdev->dev;
> +	spin_lock_init(&video->lock);
> +	mutex_init(&video->video_lock);
> +	INIT_LIST_HEAD(&video->buffers);
> +
> +	regs = devm_platform_ioremap_resource_byname(pdev, VCD_MODULE_NAME);
> +	if (IS_ERR(regs)) {
> +		dev_err(&pdev->dev, "Failed to get VCD regmap resource!\n");

Can you help the user more by saying what to fix like check devicetree 
or so?

> +		return PTR_ERR(regs);
> +	}
> +
> +	video->vcd_regmap = devm_regmap_init_mmio(&pdev->dev, regs,
> +						  &nuvoton_video_regmap_cfg);
> +	if (IS_ERR(video->vcd_regmap)) {
> +		dev_err(&pdev->dev, "Failed to init VCD regmap!\n");
> +		return PTR_ERR(video->vcd_regmap);
> +	}
> +
> +	regs = devm_platform_ioremap_resource_byname(pdev, ECE_MODULE_NAME);
> +	if (IS_ERR(regs)) {
> +		dev_err(&pdev->dev, "Failed to get ECE regmap resource!\n");
> +		return PTR_ERR(regs);
> +	}
> +
> +	video->ece.regmap = devm_regmap_init_mmio(&pdev->dev, regs,
> +						  &nuvoton_video_ece_regmap_cfg);
> +	if (IS_ERR(video->ece.regmap)) {
> +		dev_err(&pdev->dev, "Failed to init ECE regmap!\n");
> +		return PTR_ERR(video->ece.regmap);
> +	}
> +
> +	video->reset = devm_reset_control_get(&pdev->dev, VCD_MODULE_NAME);
> +	if (IS_ERR(video->reset)) {
> +		dev_err(&pdev->dev, "Failed to get VCD reset control!\n");
> +		return PTR_ERR(video->reset);
> +	}
> +
> +	video->ece.reset = devm_reset_control_get(&pdev->dev, ECE_MODULE_NAME);
> +	if (IS_ERR(video->ece.reset)) {
> +		dev_err(&pdev->dev, "Failed to get ECE reset control!\n");
> +		return PTR_ERR(video->ece.reset);
> +	}
> +
> +	video->gcr_regmap =
> +		syscon_regmap_lookup_by_compatible("nuvoton,npcm750-gcr");
> +	if (IS_ERR(video->gcr_regmap))
> +		return PTR_ERR(video->gcr_regmap);
> +
> +	video->gfx_regmap =
> +		syscon_regmap_lookup_by_compatible("nuvoton,npcm750-gfxi");
> +	if (IS_ERR(video->gfx_regmap))
> +		return PTR_ERR(video->gfx_regmap);
> +
> +	rc = nuvoton_video_init(video);
> +	if (rc)
> +		return rc;
> +
> +	rc = nuvoton_video_setup_video(video);
> +	if (rc)
> +		return rc;
> +
> +	dev_info(video->dev, "NPCM video driver probed\n");
> +
> +	return 0;
> +}
> +
> +static int nuvoton_video_remove(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct v4l2_device *v4l2_dev = dev_get_drvdata(dev);
> +	struct nuvoton_video *video = to_nuvoton_video(v4l2_dev);
> +
> +	video_unregister_device(&video->vdev);
> +
> +	vb2_queue_release(&video->queue);
> +
> +	v4l2_ctrl_handler_free(&video->ctrl_handler);
> +
> +	v4l2_device_unregister(v4l2_dev);
> +
> +	nuvoton_video_ece_stop(video);
> +
> +	of_reserved_mem_device_release(dev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id nuvoton_video_match[] = {
> +	{
> +		.compatible = "nuvoton,npcm750-video",
> +	},
> +	{},
> +};
> +
> +MODULE_DEVICE_TABLE(of, nuvoton_video_match)
> +
> +static struct platform_driver nuvoton_video_driver = {
> +	.driver = {
> +		.name = DEVICE_NAME,
> +		.of_match_table = nuvoton_video_match,
> +	},
> +	.probe = nuvoton_video_probe,
> +	.remove = nuvoton_video_remove,
> +};
> +
> +module_platform_driver(nuvoton_video_driver);
> +
> +MODULE_AUTHOR("Joseph Liu<kwliu@nuvoton.com>");
> +MODULE_AUTHOR("Marvin Lin<kflin@nuvoton.com>");
> +MODULE_DESCRIPTION("Driver for Nuvoton NPCM Video Capture/Encode Engine");
> +MODULE_LICENSE("GPL v2");


Kind regards,

Paul


[1]: https://notabs.org/coding/smallIntsBigPenalty.htm

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

* Re: [PATCH v2 5/5] drivers: media: platform: Add NPCM Video Capture/Encode Engine driver
  2022-05-13  3:34 ` [PATCH v2 5/5] drivers: media: platform: Add NPCM Video Capture/Encode Engine driver Marvin Lin
  2022-05-13  5:46   ` Paul Menzel
@ 2022-05-13  9:09   ` kernel test robot
  2022-05-13 10:30   ` kernel test robot
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 17+ messages in thread
From: kernel test robot @ 2022-05-13  9:09 UTC (permalink / raw)
  To: Marvin Lin, mchehab, linux-media, linux-kernel
  Cc: kbuild-all, openbmc, avifishman70, tmaimon77, tali.perry1, kwliu, kflin

Hi Marvin,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on robh/for-next]
[also build test WARNING on linux/master linus/master v5.18-rc6]
[cannot apply to media-tree/master next-20220512]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Marvin-Lin/Support-Nuvoton-NPCM-Video-Capture-Encode-Engine/20220513-113806
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20220513/202205131733.OM8Kr2te-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/7fa1f7750e7317479ce2c2c043b44fa19c46b1a8
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Marvin-Lin/Support-Nuvoton-NPCM-Video-Capture-Encode-Engine/20220513-113806
        git checkout 7fa1f7750e7317479ce2c2c043b44fa19c46b1a8
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=alpha SHELL=/bin/bash drivers/media/platform/nuvoton/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/media/platform/nuvoton/npcm-video.c: In function 'nuvoton_video_buf_queue':
>> drivers/media/platform/nuvoton/npcm-video.c:1776:13: warning: variable 'empty' set but not used [-Wunused-but-set-variable]
    1776 |         int empty;
         |             ^~~~~


vim +/empty +1776 drivers/media/platform/nuvoton/npcm-video.c

  1773	
  1774	static void nuvoton_video_buf_queue(struct vb2_buffer *vb)
  1775	{
> 1776		int empty;
  1777		struct nuvoton_video *video = vb2_get_drv_priv(vb->vb2_queue);
  1778		struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
  1779		struct nuvoton_video_buffer *nvb = to_nuvoton_video_buffer(vbuf);
  1780		unsigned long flags;
  1781	
  1782		dev_dbg(video->dev, "%s\n", __func__);
  1783	
  1784		spin_lock_irqsave(&video->lock, flags);
  1785		empty = list_empty(&video->buffers);
  1786		list_add_tail(&nvb->link, &video->buffers);
  1787		spin_unlock_irqrestore(&video->lock, flags);
  1788	}
  1789	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

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

* Re: [PATCH v2 5/5] drivers: media: platform: Add NPCM Video Capture/Encode Engine driver
  2022-05-13  3:34 ` [PATCH v2 5/5] drivers: media: platform: Add NPCM Video Capture/Encode Engine driver Marvin Lin
  2022-05-13  5:46   ` Paul Menzel
  2022-05-13  9:09   ` kernel test robot
@ 2022-05-13 10:30   ` kernel test robot
  2022-05-14 10:41   ` kernel test robot
  2022-05-17 13:30   ` Jonathan Neuschäfer
  4 siblings, 0 replies; 17+ messages in thread
From: kernel test robot @ 2022-05-13 10:30 UTC (permalink / raw)
  To: Marvin Lin, mchehab, linux-media, linux-kernel
  Cc: kbuild-all, openbmc, avifishman70, tmaimon77, tali.perry1, kwliu, kflin

Hi Marvin,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on robh/for-next]
[also build test ERROR on linux/master linus/master v5.18-rc6]
[cannot apply to media-tree/master next-20220512]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Marvin-Lin/Support-Nuvoton-NPCM-Video-Capture-Encode-Engine/20220513-113806
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: powerpc-allmodconfig (https://download.01.org/0day-ci/archive/20220513/202205131804.AMALhZXJ-lkp@intel.com/config)
compiler: powerpc-linux-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/7fa1f7750e7317479ce2c2c043b44fa19c46b1a8
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Marvin-Lin/Support-Nuvoton-NPCM-Video-Capture-Encode-Engine/20220513-113806
        git checkout 7fa1f7750e7317479ce2c2c043b44fa19c46b1a8
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=powerpc SHELL=/bin/bash drivers/media/platform/nuvoton/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/media/platform/nuvoton/npcm-video.c: In function 'nuvoton_video_buf_queue':
   drivers/media/platform/nuvoton/npcm-video.c:1776:13: warning: variable 'empty' set but not used [-Wunused-but-set-variable]
    1776 |         int empty;
         |             ^~~~~
   drivers/media/platform/nuvoton/npcm-video.c: At top level:
>> drivers/media/platform/nuvoton/npcm-video.c:2060:1: error: expected ',' or ';' before 'static'
    2060 | static struct platform_driver nuvoton_video_driver = {
         | ^~~~~~
   In file included from drivers/media/platform/nuvoton/npcm-video.c:24:
   drivers/media/platform/nuvoton/npcm-video.c: In function 'nuvoton_video_driver_init':
>> drivers/media/platform/nuvoton/npcm-video.c:2069:24: error: 'nuvoton_video_driver' undeclared (first use in this function); did you mean 'nuvoton_video_probe'?
    2069 | module_platform_driver(nuvoton_video_driver);
         |                        ^~~~~~~~~~~~~~~~~~~~
   include/linux/platform_device.h:222:36: note: in definition of macro 'platform_driver_register'
     222 |         __platform_driver_register(drv, THIS_MODULE)
         |                                    ^~~
   include/linux/platform_device.h:252:9: note: in expansion of macro 'module_driver'
     252 |         module_driver(__platform_driver, platform_driver_register, \
         |         ^~~~~~~~~~~~~
   drivers/media/platform/nuvoton/npcm-video.c:2069:1: note: in expansion of macro 'module_platform_driver'
    2069 | module_platform_driver(nuvoton_video_driver);
         | ^~~~~~~~~~~~~~~~~~~~~~
   drivers/media/platform/nuvoton/npcm-video.c:2069:24: note: each undeclared identifier is reported only once for each function it appears in
    2069 | module_platform_driver(nuvoton_video_driver);
         |                        ^~~~~~~~~~~~~~~~~~~~
   include/linux/platform_device.h:222:36: note: in definition of macro 'platform_driver_register'
     222 |         __platform_driver_register(drv, THIS_MODULE)
         |                                    ^~~
   include/linux/platform_device.h:252:9: note: in expansion of macro 'module_driver'
     252 |         module_driver(__platform_driver, platform_driver_register, \
         |         ^~~~~~~~~~~~~
   drivers/media/platform/nuvoton/npcm-video.c:2069:1: note: in expansion of macro 'module_platform_driver'
    2069 | module_platform_driver(nuvoton_video_driver);
         | ^~~~~~~~~~~~~~~~~~~~~~
   In file included from include/linux/device.h:32,
                    from drivers/media/platform/nuvoton/npcm-video.c:14:
   drivers/media/platform/nuvoton/npcm-video.c: In function 'nuvoton_video_driver_exit':
>> drivers/media/platform/nuvoton/npcm-video.c:2069:24: error: 'nuvoton_video_driver' undeclared (first use in this function); did you mean 'nuvoton_video_probe'?
    2069 | module_platform_driver(nuvoton_video_driver);
         |                        ^~~~~~~~~~~~~~~~~~~~
   include/linux/device/driver.h:267:24: note: in definition of macro 'module_driver'
     267 |         __unregister(&(__driver) , ##__VA_ARGS__); \
         |                        ^~~~~~~~
   drivers/media/platform/nuvoton/npcm-video.c:2069:1: note: in expansion of macro 'module_platform_driver'
    2069 | module_platform_driver(nuvoton_video_driver);
         | ^~~~~~~~~~~~~~~~~~~~~~
   drivers/media/platform/nuvoton/npcm-video.c: In function 'nuvoton_video_driver_init':
   include/linux/device/driver.h:263:1: error: control reaches end of non-void function [-Werror=return-type]
     263 | } \
         | ^
   include/linux/platform_device.h:252:9: note: in expansion of macro 'module_driver'
     252 |         module_driver(__platform_driver, platform_driver_register, \
         |         ^~~~~~~~~~~~~
   drivers/media/platform/nuvoton/npcm-video.c:2069:1: note: in expansion of macro 'module_platform_driver'
    2069 | module_platform_driver(nuvoton_video_driver);
         | ^~~~~~~~~~~~~~~~~~~~~~
   At top level:
   drivers/media/platform/nuvoton/npcm-video.c:2030:12: warning: 'nuvoton_video_remove' defined but not used [-Wunused-function]
    2030 | static int nuvoton_video_remove(struct platform_device *pdev)
         |            ^~~~~~~~~~~~~~~~~~~~
   drivers/media/platform/nuvoton/npcm-video.c:1954:12: warning: 'nuvoton_video_probe' defined but not used [-Wunused-function]
    1954 | static int nuvoton_video_probe(struct platform_device *pdev)
         |            ^~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +2060 drivers/media/platform/nuvoton/npcm-video.c

  2059	
> 2060	static struct platform_driver nuvoton_video_driver = {
  2061		.driver = {
  2062			.name = DEVICE_NAME,
  2063			.of_match_table = nuvoton_video_match,
  2064		},
  2065		.probe = nuvoton_video_probe,
  2066		.remove = nuvoton_video_remove,
  2067	};
  2068	
> 2069	module_platform_driver(nuvoton_video_driver);
  2070	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

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

* Re: [PATCH v2 5/5] drivers: media: platform: Add NPCM Video Capture/Encode Engine driver
  2022-05-13  3:34 ` [PATCH v2 5/5] drivers: media: platform: Add NPCM Video Capture/Encode Engine driver Marvin Lin
                     ` (2 preceding siblings ...)
  2022-05-13 10:30   ` kernel test robot
@ 2022-05-14 10:41   ` kernel test robot
  2022-05-17 13:30   ` Jonathan Neuschäfer
  4 siblings, 0 replies; 17+ messages in thread
From: kernel test robot @ 2022-05-14 10:41 UTC (permalink / raw)
  To: Marvin Lin, mchehab, linux-media, linux-kernel
  Cc: llvm, kbuild-all, openbmc, avifishman70, tmaimon77, tali.perry1,
	kwliu, kflin

Hi Marvin,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on robh/for-next]
[also build test WARNING on linux/master linus/master v5.18-rc6]
[cannot apply to media-tree/master next-20220513]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Marvin-Lin/Support-Nuvoton-NPCM-Video-Capture-Encode-Engine/20220513-113806
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20220514/202205141815.QVBWWaqM-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 38189438b69ca27b4c6ce707c52dbd217583d046)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/7fa1f7750e7317479ce2c2c043b44fa19c46b1a8
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Marvin-Lin/Support-Nuvoton-NPCM-Video-Capture-Encode-Engine/20220513-113806
        git checkout 7fa1f7750e7317479ce2c2c043b44fa19c46b1a8
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/media/platform/nuvoton/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/media/platform/nuvoton/npcm-video.c:1776:6: warning: variable 'empty' set but not used [-Wunused-but-set-variable]
           int empty;
               ^
   1 warning generated.


vim +/empty +1776 drivers/media/platform/nuvoton/npcm-video.c

  1773	
  1774	static void nuvoton_video_buf_queue(struct vb2_buffer *vb)
  1775	{
> 1776		int empty;
  1777		struct nuvoton_video *video = vb2_get_drv_priv(vb->vb2_queue);
  1778		struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
  1779		struct nuvoton_video_buffer *nvb = to_nuvoton_video_buffer(vbuf);
  1780		unsigned long flags;
  1781	
  1782		dev_dbg(video->dev, "%s\n", __func__);
  1783	
  1784		spin_lock_irqsave(&video->lock, flags);
  1785		empty = list_empty(&video->buffers);
  1786		list_add_tail(&nvb->link, &video->buffers);
  1787		spin_unlock_irqrestore(&video->lock, flags);
  1788	}
  1789	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

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

* Re: [PATCH v2 1/5] arm: dts: Add node for NPCM Video Capture/Encode Engine
  2022-05-13  5:22   ` Paul Menzel
@ 2022-05-17  1:55     ` Kun-Fa Lin
  0 siblings, 0 replies; 17+ messages in thread
From: Kun-Fa Lin @ 2022-05-17  1:55 UTC (permalink / raw)
  To: Paul Menzel
  Cc: mchehab, linux-media, linux-kernel, CS20 KWLiu, tmaimon77,
	avifishman70, openbmc, tali.perry1, Marvin Lin

Dear Paul,

Thanks for your review and comments.

> Could you please add the datasheet name and revision to the commit message?

It may not be appropriate since the datasheet is not public.

> What board did you test this on?

I tested on Nuvoton NPCM750 evaluation board.

>
> > Signed-off-by: Marvin Lin <kflin@nuvoton.com>
>
> As you sent this from your Google Mail account, that address is going to
> be used in the author field. Is your Nuvoton address configured for git?
>
>      git config user.email
>
> Then I would have thought, that that address is also in the files
> generated by `git format-patch`.

Thanks for the reminder. I'll change the address configuration.

Regards,
Marvin

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

* Re: [PATCH v2 5/5] drivers: media: platform: Add NPCM Video Capture/Encode Engine driver
  2022-05-13  5:46   ` Paul Menzel
@ 2022-05-17  2:59     ` Kun-Fa Lin
  2022-06-06  2:42       ` Kun-Fa Lin
  0 siblings, 1 reply; 17+ messages in thread
From: Kun-Fa Lin @ 2022-05-17  2:59 UTC (permalink / raw)
  To: Paul Menzel
  Cc: mchehab, linux-media, linux-kernel, CS20 KWLiu, tmaimon77,
	avifishman70, openbmc, tali.perry1, Marvin Lin

Dear Paul,

Thanks for your review and comments.

> Please mention the datasheet name and revision used to implement this?
> How can your patch be tested?
>
> For a over 2000 line patch, I would expect a longer commit message with
> a summary of the hardware features, and implementation.

Okay, I'll add more information to the commit message, but it may not
be appropriate to add the datasheet name since it is not public.
And I tested with openbmc/obmc-ikvm (with patches to support Hextile
encoding that our driver used) and used VNC Viewer to verify the video
result.

>
> As the module author should you also be added to the file `MAINTAINERS`?
> (Maybe even with a functional address <linux-npcm-video@nuvoton.com>?
>
> > Signed-off-by: Marvin Lin <kflin@nuvoton.com>
>
> Same comment as in 1/5 regarding the author email address.

I'll add a new entry in MAINTAINERS.

> > +++ b/drivers/media/platform/nuvoton/Kconfig
> > @@ -0,0 +1,12 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +
> > +comment "Nuvoton media platform drivers"
> > +
> > +config VIDEO_NUVOTON
>
> Is that driver going to support all Nuvoton devices or just NPCM? If
> only NPCM, that should be part of the Kconfig config name?
>
> > +     tristate "Nuvoton NPCM Video Capture/Encode Engine driver"
> > +     depends on V4L_PLATFORM_DRIVERS
> > +     depends on VIDEO_DEV
> > +     select VIDEOBUF2_DMA_CONTIG
> > +     help
> > +       Support for the Video Capture/Differentiation Engine (VCD) and
> > +       Encoding Compression Engine (ECE) present on Nuvoton NPCM SoCs.
>
> Mention the module name?
>
>  > To compile this driver as a module, choose M here: the module will be
> called XXX.

The driver just supports NPCM. I'll change the config to
VIDEO_NUVOTON_NPCM_VCD_ECE.

> > +struct nuvoton_video_addr {
> > +     unsigned int size;
>
> size_t?

> > +struct rect_list_info {
> > +     struct rect_list *list;
> > +     struct rect_list *first;
> > +     struct list_head *head;
> > +     int index;
> > +     int tile_perline;
> > +     int tile_perrow;
> > +     int offset_perline;
> > +     int tile_size;
> > +     int tile_cnt;
>
> Can all of these be unsigned?

> > +     int frame_rate;
> > +     int vb_index;
>
> Unsigned?
>

They will be addressed in the next patch.

> > +     u32 bytesperline;
> > +     u8 bytesperpixel;
> > +     u32 rect_cnt;
> > +     u8 num_buffers;
> > +     struct list_head *list;
> > +     u32 *rect;
>
> I would not limit the size?

It's clearer to know that it stores u32 exactly.

> > +static u32 nuvoton_video_ece_get_ed_size(struct nuvoton_video *video,
> > +                                      u32 offset, void *addr)
>
> Use unsigned int as return value?

Okay.

> > +static void nuvoton_video_ece_enc_rect(struct nuvoton_video *video, u32 r_off_x,
> > +                                    u32 r_off_y, u32 r_w, u32 r_h)
> > +{
> > +     struct regmap *ece = video->ece.regmap;
> > +     u32 rect_offset = (r_off_y * video->bytesperline) + (r_off_x * 2);
> > +     u32 temp;
> > +     u32 w_tile;
> > +     u32 h_tile;
> > +     u32 w_size = ECE_TILE_W;
> > +     u32 h_size = ECE_TILE_H;
>
> Any reason to fix the sizes?

A "Hextile" is fixed to 16x16 pixels size, which is defined in Remote
Framebuffer Protocol (RFC 6143, chapter 7.7.4 Hextile Encoding).

> > +static void nuvoton_video_ece_ip_reset(struct nuvoton_video *video)
> > +{
> > +     reset_control_assert(video->ece.reset);
> > +     msleep(100);
> > +     reset_control_deassert(video->ece.reset);
> > +     msleep(100);
>
> 100 ms is quite long. Please add a comment, where that is documented. Is
> there a way to poll, if the device is done?

I'll add a comment. It should be reduced to ~10 us (suggested in
spec.) and there's no way to poll.

> > +
> > +static void nuvoton_video_free_diff_table(struct nuvoton_video *video)
> > +{
> > +     struct list_head *head, *pos, *nx;
> > +     struct rect_list *tmp;
> > +     int i;
>
> unsigned?
>

> > +static int nuvoton_video_find_rect(struct nuvoton_video *video,
> > +                                struct rect_list_info *info, u32 offset)
> > +{
> > +     int i = info->index;
> > +
> > +     if (offset < info->tile_perline) {
> > +             info->list = nuvoton_video_new_rect(video, offset, i);
>
> `i` is only used here, so use `info->index`?
>

> > +static int nuvoton_video_build_table(struct nuvoton_video *video,
> > +                                  struct rect_list_info *info)
> > +{
> > +     int i = info->index;
> > +     int j, ret, bit;
>
> Make `j` unsigned?
>
> > +     u32 value;
> > +     struct regmap *vcd = video->vcd_regmap;
> > +
> > +     for (j = 0; j < info->offset_perline; j += 4) {
> > +             regmap_read(vcd, VCD_DIFF_TBL + (j + i), &value);
>
> `i` is only used here, so use `info->index`?
>

> > +static void nuvoton_video_vcd_ip_reset(struct nuvoton_video *video)
> > +{
> > +     reset_control_assert(video->reset);
> > +     msleep(100);
> > +     reset_control_deassert(video->reset);
> > +     msleep(100);
>
> 100 ms is quite long. Please add a comment, where that is documented. Is
> there a way to poll, if the device is done?
>

> > +static int nuvoton_video_queue_setup(struct vb2_queue *q,
> > +                                  unsigned int *num_buffers,
> > +                                  unsigned int *num_planes,
> > +                                  unsigned int sizes[],
> > +                                  struct device *alloc_devs[])
> > +{
> > +     struct nuvoton_video *video = vb2_get_drv_priv(q);
> > +     int i;
>
> unsigned?
>

> > +static void nuvoton_video_buf_queue(struct vb2_buffer *vb)
> > +{
> > +     int empty;
> > +     struct nuvoton_video *video = vb2_get_drv_priv(vb->vb2_queue);
> > +     struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> > +     struct nuvoton_video_buffer *nvb = to_nuvoton_video_buffer(vbuf);
> > +     unsigned long flags;
> > +
> > +     dev_dbg(video->dev, "%s\n", __func__);
> > +
> > +     spin_lock_irqsave(&video->lock, flags);
> > +     empty = list_empty(&video->buffers);
>
> Where is empty read later?
>

> > +     regs = devm_platform_ioremap_resource_byname(pdev, VCD_MODULE_NAME);
> > +     if (IS_ERR(regs)) {
> > +             dev_err(&pdev->dev, "Failed to get VCD regmap resource!\n");
>
> Can you help the user more by saying what to fix like check devicetree
> or so?
>

Okay. All of them will be addressed in the next patch.

Regards,
Marvin

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

* Re: [PATCH v2 5/5] drivers: media: platform: Add NPCM Video Capture/Encode Engine driver
  2022-05-13  3:34 ` [PATCH v2 5/5] drivers: media: platform: Add NPCM Video Capture/Encode Engine driver Marvin Lin
                     ` (3 preceding siblings ...)
  2022-05-14 10:41   ` kernel test robot
@ 2022-05-17 13:30   ` Jonathan Neuschäfer
  2022-05-19  5:39     ` Kun-Fa Lin
  4 siblings, 1 reply; 17+ messages in thread
From: Jonathan Neuschäfer @ 2022-05-17 13:30 UTC (permalink / raw)
  To: Marvin Lin
  Cc: mchehab, linux-media, linux-kernel, kwliu, tmaimon77,
	avifishman70, openbmc, tali.perry1, kflin

[-- Attachment #1: Type: text/plain, Size: 863 bytes --]

Hello,

first of all, thank you for bringing this driver upstream.

On Fri, May 13, 2022 at 11:34:50AM +0800, Marvin Lin wrote:
> Add driver for Video Capture/Differentiation Engine (VCD) and Encoding
> Compression Engine (ECE) present on Nuvoton NPCM SoCs.
> 
> Signed-off-by: Marvin Lin <kflin@nuvoton.com>
> ---
>  drivers/media/platform/Kconfig              |    1 +
>  drivers/media/platform/Makefile             |    1 +
>  drivers/media/platform/nuvoton/Kconfig      |   12 +
>  drivers/media/platform/nuvoton/Makefile     |    2 +
>  drivers/media/platform/nuvoton/npcm-video.c | 2074 +++++++++++++++++++

Similar to what I said in reply to the bindings patch, I would prefer to
have the VCD code and the ECE code in two separate files, if possible,
to make reuse of the VCD code alone (on WPCM450) easier.


Best regards,
Jonathan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH v2 5/5] drivers: media: platform: Add NPCM Video Capture/Encode Engine driver
  2022-05-17 13:30   ` Jonathan Neuschäfer
@ 2022-05-19  5:39     ` Kun-Fa Lin
  2022-05-19 10:12       ` Jonathan Neuschäfer
  0 siblings, 1 reply; 17+ messages in thread
From: Kun-Fa Lin @ 2022-05-19  5:39 UTC (permalink / raw)
  To: Jonathan Neuschäfer
  Cc: mchehab, linux-media, linux-kernel, CS20 KWLiu, tmaimon77,
	avifishman70, openbmc, tali.perry1, Marvin Lin

Hi Jonathan,

Thanks for your comment.

> Similar to what I said in reply to the bindings patch, I would prefer to
> have the VCD code and the ECE code in two separate files, if possible,
> to make reuse of the VCD code alone (on WPCM450) easier.

This video driver is bassed on V4L2 framework to implement the
required interfaces,
and the V4L2 application in user-space (e.g., openbmc/obmc-ikvm) will
interact with
these interfaces to get compressed video data for supporting KVM
features. In this
architecture, for the case of WPCM450 (though it's in EOL for several
years), I'd prefer
to try to let ECE be an optional node instead of separating ECE code
from this V4L2
video driver. However, this patch series could be the base version and
keep reviewing,
and afterwards we could make another patch for optional ECE, is it OK for you?

Regards,
Marvin

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

* Re: [PATCH v2 5/5] drivers: media: platform: Add NPCM Video Capture/Encode Engine driver
  2022-05-19  5:39     ` Kun-Fa Lin
@ 2022-05-19 10:12       ` Jonathan Neuschäfer
  0 siblings, 0 replies; 17+ messages in thread
From: Jonathan Neuschäfer @ 2022-05-19 10:12 UTC (permalink / raw)
  To: Kun-Fa Lin
  Cc: Jonathan Neuschäfer, mchehab, linux-media, linux-kernel,
	CS20 KWLiu, tmaimon77, avifishman70, openbmc, tali.perry1,
	Marvin Lin

[-- Attachment #1: Type: text/plain, Size: 815 bytes --]

Hello Marvin,

On Thu, May 19, 2022 at 01:39:01PM +0800, Kun-Fa Lin wrote:
[...]
> In this architecture, for the case of WPCM450 (though it's in EOL for
> several years), I'd prefer to try to let ECE be an optional node
> instead of separating ECE code from this V4L2 video driver. However,
> this patch series could be the base version and keep reviewing, and
> afterwards we could make another patch for optional ECE, is it OK for
> you?

That's OK for me.

I will wait until the driver is ready and merged, and then start working
on WPCM450 integration and making ECE optional.

Regarding WPCM450 being EOL, I'm aware, and I don't want to hold up your
team's work on newer SoCs. But I personally find this old SoC interesting
and I want to share driver code with newer SoCs where possible.


Thank you,
Jonathan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH v2 5/5] drivers: media: platform: Add NPCM Video Capture/Encode Engine driver
  2022-05-17  2:59     ` Kun-Fa Lin
@ 2022-06-06  2:42       ` Kun-Fa Lin
  0 siblings, 0 replies; 17+ messages in thread
From: Kun-Fa Lin @ 2022-06-06  2:42 UTC (permalink / raw)
  To: Paul Menzel
  Cc: mchehab, linux-media, linux-kernel, CS20 KWLiu, tmaimon77,
	avifishman70, openbmc, tali.perry1, Marvin Lin

Dear Paul,

These problems have been addressed in the new patch.
Could you please help to review the new patch v4? Thanks.

Regards,
Marvin

Kun-Fa Lin <milkfafa@gmail.com> 於 2022年5月17日 週二 上午10:59寫道:
>
> Dear Paul,
>
> Thanks for your review and comments.
>
> > Please mention the datasheet name and revision used to implement this?
> > How can your patch be tested?
> >
> > For a over 2000 line patch, I would expect a longer commit message with
> > a summary of the hardware features, and implementation.
>
> Okay, I'll add more information to the commit message, but it may not
> be appropriate to add the datasheet name since it is not public.
> And I tested with openbmc/obmc-ikvm (with patches to support Hextile
> encoding that our driver used) and used VNC Viewer to verify the video
> result.
>
> >
> > As the module author should you also be added to the file `MAINTAINERS`?
> > (Maybe even with a functional address <linux-npcm-video@nuvoton.com>?
> >
> > > Signed-off-by: Marvin Lin <kflin@nuvoton.com>
> >
> > Same comment as in 1/5 regarding the author email address.
>
> I'll add a new entry in MAINTAINERS.
>
> > > +++ b/drivers/media/platform/nuvoton/Kconfig
> > > @@ -0,0 +1,12 @@
> > > +# SPDX-License-Identifier: GPL-2.0-only
> > > +
> > > +comment "Nuvoton media platform drivers"
> > > +
> > > +config VIDEO_NUVOTON
> >
> > Is that driver going to support all Nuvoton devices or just NPCM? If
> > only NPCM, that should be part of the Kconfig config name?
> >
> > > +     tristate "Nuvoton NPCM Video Capture/Encode Engine driver"
> > > +     depends on V4L_PLATFORM_DRIVERS
> > > +     depends on VIDEO_DEV
> > > +     select VIDEOBUF2_DMA_CONTIG
> > > +     help
> > > +       Support for the Video Capture/Differentiation Engine (VCD) and
> > > +       Encoding Compression Engine (ECE) present on Nuvoton NPCM SoCs.
> >
> > Mention the module name?
> >
> >  > To compile this driver as a module, choose M here: the module will be
> > called XXX.
>
> The driver just supports NPCM. I'll change the config to
> VIDEO_NUVOTON_NPCM_VCD_ECE.
>
> > > +struct nuvoton_video_addr {
> > > +     unsigned int size;
> >
> > size_t?
>
> > > +struct rect_list_info {
> > > +     struct rect_list *list;
> > > +     struct rect_list *first;
> > > +     struct list_head *head;
> > > +     int index;
> > > +     int tile_perline;
> > > +     int tile_perrow;
> > > +     int offset_perline;
> > > +     int tile_size;
> > > +     int tile_cnt;
> >
> > Can all of these be unsigned?
>
> > > +     int frame_rate;
> > > +     int vb_index;
> >
> > Unsigned?
> >
>
> They will be addressed in the next patch.
>
> > > +     u32 bytesperline;
> > > +     u8 bytesperpixel;
> > > +     u32 rect_cnt;
> > > +     u8 num_buffers;
> > > +     struct list_head *list;
> > > +     u32 *rect;
> >
> > I would not limit the size?
>
> It's clearer to know that it stores u32 exactly.
>
> > > +static u32 nuvoton_video_ece_get_ed_size(struct nuvoton_video *video,
> > > +                                      u32 offset, void *addr)
> >
> > Use unsigned int as return value?
>
> Okay.
>
> > > +static void nuvoton_video_ece_enc_rect(struct nuvoton_video *video, u32 r_off_x,
> > > +                                    u32 r_off_y, u32 r_w, u32 r_h)
> > > +{
> > > +     struct regmap *ece = video->ece.regmap;
> > > +     u32 rect_offset = (r_off_y * video->bytesperline) + (r_off_x * 2);
> > > +     u32 temp;
> > > +     u32 w_tile;
> > > +     u32 h_tile;
> > > +     u32 w_size = ECE_TILE_W;
> > > +     u32 h_size = ECE_TILE_H;
> >
> > Any reason to fix the sizes?
>
> A "Hextile" is fixed to 16x16 pixels size, which is defined in Remote
> Framebuffer Protocol (RFC 6143, chapter 7.7.4 Hextile Encoding).
>
> > > +static void nuvoton_video_ece_ip_reset(struct nuvoton_video *video)
> > > +{
> > > +     reset_control_assert(video->ece.reset);
> > > +     msleep(100);
> > > +     reset_control_deassert(video->ece.reset);
> > > +     msleep(100);
> >
> > 100 ms is quite long. Please add a comment, where that is documented. Is
> > there a way to poll, if the device is done?
>
> I'll add a comment. It should be reduced to ~10 us (suggested in
> spec.) and there's no way to poll.
>
> > > +
> > > +static void nuvoton_video_free_diff_table(struct nuvoton_video *video)
> > > +{
> > > +     struct list_head *head, *pos, *nx;
> > > +     struct rect_list *tmp;
> > > +     int i;
> >
> > unsigned?
> >
>
> > > +static int nuvoton_video_find_rect(struct nuvoton_video *video,
> > > +                                struct rect_list_info *info, u32 offset)
> > > +{
> > > +     int i = info->index;
> > > +
> > > +     if (offset < info->tile_perline) {
> > > +             info->list = nuvoton_video_new_rect(video, offset, i);
> >
> > `i` is only used here, so use `info->index`?
> >
>
> > > +static int nuvoton_video_build_table(struct nuvoton_video *video,
> > > +                                  struct rect_list_info *info)
> > > +{
> > > +     int i = info->index;
> > > +     int j, ret, bit;
> >
> > Make `j` unsigned?
> >
> > > +     u32 value;
> > > +     struct regmap *vcd = video->vcd_regmap;
> > > +
> > > +     for (j = 0; j < info->offset_perline; j += 4) {
> > > +             regmap_read(vcd, VCD_DIFF_TBL + (j + i), &value);
> >
> > `i` is only used here, so use `info->index`?
> >
>
> > > +static void nuvoton_video_vcd_ip_reset(struct nuvoton_video *video)
> > > +{
> > > +     reset_control_assert(video->reset);
> > > +     msleep(100);
> > > +     reset_control_deassert(video->reset);
> > > +     msleep(100);
> >
> > 100 ms is quite long. Please add a comment, where that is documented. Is
> > there a way to poll, if the device is done?
> >
>
> > > +static int nuvoton_video_queue_setup(struct vb2_queue *q,
> > > +                                  unsigned int *num_buffers,
> > > +                                  unsigned int *num_planes,
> > > +                                  unsigned int sizes[],
> > > +                                  struct device *alloc_devs[])
> > > +{
> > > +     struct nuvoton_video *video = vb2_get_drv_priv(q);
> > > +     int i;
> >
> > unsigned?
> >
>
> > > +static void nuvoton_video_buf_queue(struct vb2_buffer *vb)
> > > +{
> > > +     int empty;
> > > +     struct nuvoton_video *video = vb2_get_drv_priv(vb->vb2_queue);
> > > +     struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> > > +     struct nuvoton_video_buffer *nvb = to_nuvoton_video_buffer(vbuf);
> > > +     unsigned long flags;
> > > +
> > > +     dev_dbg(video->dev, "%s\n", __func__);
> > > +
> > > +     spin_lock_irqsave(&video->lock, flags);
> > > +     empty = list_empty(&video->buffers);
> >
> > Where is empty read later?
> >
>
> > > +     regs = devm_platform_ioremap_resource_byname(pdev, VCD_MODULE_NAME);
> > > +     if (IS_ERR(regs)) {
> > > +             dev_err(&pdev->dev, "Failed to get VCD regmap resource!\n");
> >
> > Can you help the user more by saying what to fix like check devicetree
> > or so?
> >
>
> Okay. All of them will be addressed in the next patch.
>
> Regards,
> Marvin

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

end of thread, other threads:[~2022-06-06  2:42 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-13  3:34 [PATCH v2 0/5] Support Nuvoton NPCM Video Capture/Encode Engine Marvin Lin
2022-05-13  3:34 ` [PATCH v2 1/5] arm: dts: Add node for " Marvin Lin
2022-05-13  5:22   ` Paul Menzel
2022-05-17  1:55     ` Kun-Fa Lin
2022-05-13  3:34 ` [PATCH v2 2/5] dt-bindings: media: Add dt-bindings " Marvin Lin
2022-05-13  3:34 ` [PATCH v2 3/5] dt-bindings: arm/npcm: Add dt-bindings for Graphics Core Information Marvin Lin
2022-05-13  3:34 ` [PATCH v2 4/5] media: Add HEXTILE compressed format Marvin Lin
2022-05-13  3:34 ` [PATCH v2 5/5] drivers: media: platform: Add NPCM Video Capture/Encode Engine driver Marvin Lin
2022-05-13  5:46   ` Paul Menzel
2022-05-17  2:59     ` Kun-Fa Lin
2022-06-06  2:42       ` Kun-Fa Lin
2022-05-13  9:09   ` kernel test robot
2022-05-13 10:30   ` kernel test robot
2022-05-14 10:41   ` kernel test robot
2022-05-17 13:30   ` Jonathan Neuschäfer
2022-05-19  5:39     ` Kun-Fa Lin
2022-05-19 10:12       ` Jonathan Neuschäfer

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