[PATCH v21 0/4] media: mediatek: support mdp3 on mt8183 platform

[PATCH v21 0/4] media: mediatek: support mdp3 on mt8183 platform

[Moudy Ho]

Change since v20:
- Rebase on linux-next.
- Move the MDP3 GCE events to the corresponding node and adjust the
  relevant driver settings.

Change since v19:
- Rebase on linux-next.
- Export the function "mdp_cmdq_send" suggected by CK.
- Fix "Macro argument reuse" reported by checkpatch.pl

Change since v18:
- Rebase on linux-next.
- Adjust copyright date of MDP3 driver.
- Functions renaming as follows:
  [1] is_output_disable() => is_output_disabled()
  [2] mdp_component_init() => mdp_comp_config()
  [3] mdp_component_deinit() => mdp_comp_destroy()
  [4] mdp_comp_ctx_init() => mdp_comp_ctx_config()
  [5] mdp_sub_comps_create() => mdp_comp_sub_create()
- Document MDP3 10-bit format descriptions in "mtk-mdp3-regs.c".
- Add error control for functions mdp_comp_clocks_on and mdp_comp_clock_on.
- Moved function "mtk_mutex_put" from function
  "mdp_comp_destroy"(renamed from mdp_component_deinit) to avoid semantic ambiguity.
- For some allocated parameters, assign a value of NULL after freeing
  to avoid the possibility of repeated use.
- Removed unnecessary timestamp pass flow.
- About parameters passed by the user in function "mdp_try_fmt_mplane", add relevant checks to
  clamp them in a reasonable range to avoid the possibility of overflow

Change since v17:
- Depend on:
  [1] https://patchwork.kernel.org/project/linux-mediatek/list/?series=649104
- In response to future CMDQ api changes listed below:
  https://patchwork.kernel.org/project/linux-mediatek/patch/20220608144055.27562-1-chunkuang.hu@kernel.org/
  adjust CMDQ flush and callback flow in MDP3.

Change since v16:
- Rebased on v5.19-rc1
- Depend on:
  [1] https://patchwork.kernel.org/project/linux-mediatek/list/?series=646131
- In response to MUTEX changes, adjust API naming and parameters when
  used in function "mdp_path_subfrm_require".
- Remove unnecessary MDP3 phandle in 8183 dts.

Change since v15:
- Depend on:
  [1] https://patchwork.kernel.org/project/linux-mediatek/list/?series=640926
- Split the bindings under ./soc/mediatek into a separate patch.
- Fix data abort in "mdp_auto_release_work"
- Adjust the steps in the function "mdp_cmdq_send" to make the error handling
  more reasonable

Change since v14:
- Rebase on v5.18-rc6
- Depend on:
  [1] https://patchwork.kernel.org/project/linux-mediatek/list/?series=640926
- In response to CMDQ API change, replace the function "cmdq_pkt_flush_async"
  with the standard APIs of mbox
- Fix the description of "mediatek,gce-client-reg" property in MDP3-related
  bindings

Change since v13:
- Rebase on v5.18-rc4
- Depend on:
  [1] https://patchwork.kernel.org/project/linux-mediatek/list/?series=636041
- Remove advanced functionality about ISP settings for direct link cases.
- Remove the software designation in the mt8183 dts and
  revise corresponding bindings.

Change since v12:
- Rebase on linux-next
- Depend on:
  [1] https://patchwork.kernel.org/project/linux-mediatek/list/?series=630948
- Remove messages related to routing information in MDP3, and leave the related
  settings in MMSYS.
- Remove unnecessary phandle and redundant property in RDMA dt-binding and
  adjust the corresponding driver.
- Revise MDP3 node name in dts. 
- Removed unnecessary functions, mutex and work queue in MDP3 driver
- Fixed format mapping error for V4L2_PIX_FMT_RGB565X

Change since v11:
- Rebase on linux-next tag:next-20220316
- Depend on:
  [1] https://patchwork.kernel.org/project/linux-mediatek/list/?series=624281
- Remove redundant hardware index in data-binding suggested by Rob Herring.
- Referring to Rob Herring's suggestion to improve some descriptions in the
  RDMA dt-binding
- Move MDP3 file folder from "./drive/media/platform/mtk-mdp3" to
  "./driver/media/platform/mediatek/mdp3"
- Fixed the V4L2 and MDP color format mapping error in RGB565 which
  checked by Benjamin Gaignard

Change since v10:
- The routing table needs to be discarded, and the calculation result
  on the SCP side is used to write a suitable mux setting for
  1 input port and 2 output ports.
- Adjust dts parsing flow to remove redundant HW IDs.
- Fix memory leak caused by no free path information in function "mdp_cmdq_send".

Change since v9:
- Keep only the MDP3 driver patches and split the remaining mmsys and
  mutex patches into another mail.
- Move mutex mod settings to corresponding driver and make relevant adjustments
  for this in MDP3 driver.
- Fix compile warning reported by kernel test robot.

Change since v8:
- Rebase on v5.16-rc2.
- Refer to Angelo's suggestion, adjust the register writing format to increase
  readability and significance.
- Refer to Angelo's suggestion, adjust or reduce inappropriate debugging
  messages.
- Refer to Rob Herring's suggestion to correct the the binding file
  to make it with the specification.
- Fix compile warning reported by kernel test robot.

Change since v7:
- Rebase on v5.15-rc6.
- Revise several V4L2 M2M settings to pass v4l2-compliance test.
- Integrate those same component dt-binding documents of DRM and MDP, and
  move them under the MMSYS domain.
- Split MMSYS and MUTEX into two different files according to
  their functional properties.

Changes since v6:
- Refactor GCE event to corresponding node.
- Fix dt_binding_check fail.
- Fix compilation errors.

Changes since v5:
- Rebase on v5.14-rc6.
- Move MMSYS/Mutex settings to corresponding driver.
- Revise the software license description and copyright.
- Remove unnecessary enum. or definitions.
- Optimize platform/chip definition conditions.
- Use general printing functions instead of MDP3 private ones.
- Fix compile warning.

Changes since v4:
- Rebase on v5.13-rc1.
- Remove the CMDQ flush flow to match the CMDQ API change.
- Integrate four of MDP's direct-link subcomponents into MDP controller node
  from syscon node to avoid illegal clock usage.
- Rewrite dt-binding in a JSON compatible subset of YAML
- Fix a bit of macro argument precedence.

Changes since v3:
- Rebase on v5.9-rc1.
- modify code for review comment from Rob Herring, cancel multiple nodes using
  same register base situation.
- control IOMMU port through pm runtime get/put to DMA components' device.
- SCP(VPU) driver revision.
- stop queuing jobs(remove flush_workqueue()) after mdp_m2m_release().
- add computation of plane address with data_offset.
- fix scale ratio check issue.
- add default v4l2_format setting.

Changes since v2:
- modify code for review comment from Tomasz Figa & Alexandre Courbot
- review comment from Rob Herring will offer code revision in v4, due to
  it's related to device node modification, will need to modify code
  architecture

Changes since v1:
- modify code for CMDQ v3 API support
- EC ipi cmd migration
- fix compliance test fail item (m2m cmd with -f) due to there is two problem in
  runing all format(-f) cmd:
1. out of memory before test complete
        Due to capture buffer mmap (refcount + 1) after reqbuf but seems
        no corresponding munmap called before device close.
        There are total 12XX items(formats) in format test and each format
        alloc 8 capture/output buffers.
2. unceasingly captureBufs() (randomly)
        Seems the break statement didn't catch the count == 0 situation:
        In v4l2-test-buffers.cpp, function: captureBufs()
                        ...
                        count--;
                        if (!node->is_m2m && !count)
                                break;
        Log is as attachment

Hi,

This patch is used to present Media Data Path 3 (MDP3)
which provided scaling and color format conversion.
support using GCE to write register in critical time limitation.
support V4L2 m2m device control.

Moudy Ho (4):
  dt-binding: mediatek: add bindings for MediaTek MDP3 components
  dt-binding: mediatek: add bindings for MediaTek CCORR and WDMA
  arm64: dts: mt8183: add Mediatek MDP3 nodes
  media: platform: mtk-mdp3: add Mediatek MDP3 driver

 .../bindings/media/mediatek,mdp3-rdma.yaml    |   95 ++
 .../bindings/media/mediatek,mdp3-rsz.yaml     |   77 ++
 .../bindings/media/mediatek,mdp3-wrot.yaml    |   80 ++
 .../bindings/soc/mediatek/mediatek,ccorr.yaml |   68 ++
 .../bindings/soc/mediatek/mediatek,wdma.yaml  |   81 ++
 arch/arm64/boot/dts/mediatek/mt8183.dtsi      |   63 +
 drivers/media/platform/mediatek/Kconfig       |    1 +
 drivers/media/platform/mediatek/Makefile      |    1 +
 drivers/media/platform/mediatek/mdp3/Kconfig  |   20 +
 drivers/media/platform/mediatek/mdp3/Makefile |    6 +
 .../platform/mediatek/mdp3/mdp_reg_ccorr.h    |   19 +
 .../platform/mediatek/mdp3/mdp_reg_rdma.h     |   65 ++
 .../platform/mediatek/mdp3/mdp_reg_rsz.h      |   39 +
 .../platform/mediatek/mdp3/mdp_reg_wdma.h     |   47 +
 .../platform/mediatek/mdp3/mdp_reg_wrot.h     |   55 +
 .../platform/mediatek/mdp3/mtk-img-ipi.h      |  290 +++++
 .../platform/mediatek/mdp3/mtk-mdp3-cmdq.c    |  465 ++++++++
 .../platform/mediatek/mdp3/mtk-mdp3-cmdq.h    |   43 +
 .../platform/mediatek/mdp3/mtk-mdp3-comp.c    | 1030 +++++++++++++++++
 .../platform/mediatek/mdp3/mtk-mdp3-comp.h    |  186 +++
 .../platform/mediatek/mdp3/mtk-mdp3-core.c    |  357 ++++++
 .../platform/mediatek/mdp3/mtk-mdp3-core.h    |   93 ++
 .../platform/mediatek/mdp3/mtk-mdp3-m2m.c     |  769 ++++++++++++
 .../platform/mediatek/mdp3/mtk-mdp3-m2m.h     |   48 +
 .../platform/mediatek/mdp3/mtk-mdp3-regs.c    |  742 ++++++++++++
 .../platform/mediatek/mdp3/mtk-mdp3-regs.h    |  373 ++++++
 .../platform/mediatek/mdp3/mtk-mdp3-vpu.c     |  312 +++++
 .../platform/mediatek/mdp3/mtk-mdp3-vpu.h     |   78 ++
 28 files changed, 5503 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/mediatek,mdp3-rdma.yaml
 create mode 100644 Documentation/devicetree/bindings/media/mediatek,mdp3-rsz.yaml
 create mode 100644 Documentation/devicetree/bindings/media/mediatek,mdp3-wrot.yaml
 create mode 100644 Documentation/devicetree/bindings/soc/mediatek/mediatek,ccorr.yaml
 create mode 100644 Documentation/devicetree/bindings/soc/mediatek/mediatek,wdma.yaml
 create mode 100644 drivers/media/platform/mediatek/mdp3/Kconfig
 create mode 100644 drivers/media/platform/mediatek/mdp3/Makefile
 create mode 100644 drivers/media/platform/mediatek/mdp3/mdp_reg_ccorr.h
 create mode 100644 drivers/media/platform/mediatek/mdp3/mdp_reg_rdma.h
 create mode 100644 drivers/media/platform/mediatek/mdp3/mdp_reg_rsz.h
 create mode 100644 drivers/media/platform/mediatek/mdp3/mdp_reg_wdma.h
 create mode 100644 drivers/media/platform/mediatek/mdp3/mdp_reg_wrot.h
 create mode 100644 drivers/media/platform/mediatek/mdp3/mtk-img-ipi.h
 create mode 100644 drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c
 create mode 100644 drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.h
 create mode 100644 drivers/media/platform/mediatek/mdp3/mtk-mdp3-comp.c
 create mode 100644 drivers/media/platform/mediatek/mdp3/mtk-mdp3-comp.h
 create mode 100644 drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.c
 create mode 100644 drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.h
 create mode 100644 drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c
 create mode 100644 drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.h
 create mode 100644 drivers/media/platform/mediatek/mdp3/mtk-mdp3-regs.c
 create mode 100644 drivers/media/platform/mediatek/mdp3/mtk-mdp3-regs.h
 create mode 100644 drivers/media/platform/mediatek/mdp3/mtk-mdp3-vpu.c
 create mode 100644 drivers/media/platform/mediatek/mdp3/mtk-mdp3-vpu.h

-- 
2.18.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v21 1/4] dt-binding: mediatek: add bindings for MediaTek MDP3 components

[Moudy Ho]

This patch adds DT binding documents for Media Data Path 3 (MDP3)
a unit in multimedia system combined with several components and
used for scaling and color format convert.

Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>
Reviewed-by: Rob Herring <robh@kernel.org>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 .../bindings/media/mediatek,mdp3-rdma.yaml    | 95 +++++++++++++++++++
 .../bindings/media/mediatek,mdp3-rsz.yaml     | 77 +++++++++++++++
 .../bindings/media/mediatek,mdp3-wrot.yaml    | 80 ++++++++++++++++
 3 files changed, 252 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/mediatek,mdp3-rdma.yaml
 create mode 100644 Documentation/devicetree/bindings/media/mediatek,mdp3-rsz.yaml
 create mode 100644 Documentation/devicetree/bindings/media/mediatek,mdp3-wrot.yaml

diff --git a/Documentation/devicetree/bindings/media/mediatek,mdp3-rdma.yaml b/Documentation/devicetree/bindings/media/mediatek,mdp3-rdma.yaml
new file mode 100644
index 000000000000..6bbc4dde0393
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/mediatek,mdp3-rdma.yaml
@@ -0,0 +1,95 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/mediatek,mdp3-rdma.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mediatek Read Direct Memory Access
+
+maintainers:
+  - Matthias Brugger <matthias.bgg@gmail.com>
+  - Ping-Hsun Wu <ping-hsun.wu@mediatek.com>
+
+description: |
+  Mediatek Read Direct Memory Access(RDMA) component used to do read DMA.
+  It contains one line buffer to store the sufficient pixel data, and
+  must be siblings to the central MMSYS_CONFIG node.
+  For a description of the MMSYS_CONFIG binding, see
+  Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
+  for details.
+
+properties:
+  compatible:
+    items:
+      - const: mediatek,mt8183-mdp3-rdma
+
+  reg:
+    maxItems: 1
+
+  mediatek,gce-client-reg:
+    $ref: '/schemas/types.yaml#/definitions/phandle-array'
+    items:
+      items:
+        - description: phandle of GCE
+        - description: GCE subsys id
+        - description: register offset
+        - description: register size
+    description: The register of client driver can be configured by gce with
+      4 arguments defined in this property. Each GCE subsys id is mapping to
+      a client defined in the header include/dt-bindings/gce/<chip>-gce.h.
+
+  mediatek,gce-events:
+    description:
+      The event id which is mapping to the specific hardware event signal
+      to gce. The event id is defined in the gce header
+      include/dt-bindings/gce/<chip>-gce.h of each chips.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+
+  power-domains:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: RDMA clock
+      - description: RSZ clock
+
+  iommus:
+    maxItems: 1
+
+  mboxes:
+    items:
+      - description: used for 1st data pipe from RDMA
+      - description: used for 2nd data pipe from RDMA
+
+required:
+  - compatible
+  - reg
+  - mediatek,gce-client-reg
+  - mediatek,gce-events
+  - power-domains
+  - clocks
+  - iommus
+  - mboxes
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/mt8183-clk.h>
+    #include <dt-bindings/gce/mt8183-gce.h>
+    #include <dt-bindings/power/mt8183-power.h>
+    #include <dt-bindings/memory/mt8183-larb-port.h>
+
+    mdp3_rdma0: mdp3-rdma0@14001000 {
+      compatible = "mediatek,mt8183-mdp3-rdma";
+      reg = <0x14001000 0x1000>;
+      mediatek,gce-client-reg = <&gce SUBSYS_1400XXXX 0x1000 0x1000>;
+      mediatek,gce-events = <CMDQ_EVENT_MDP_RDMA0_SOF>,
+                            <CMDQ_EVENT_MDP_RDMA0_EOF>;
+      power-domains = <&spm MT8183_POWER_DOMAIN_DISP>;
+      clocks = <&mmsys CLK_MM_MDP_RDMA0>,
+               <&mmsys CLK_MM_MDP_RSZ1>;
+      iommus = <&iommu>;
+      mboxes = <&gce 20 CMDQ_THR_PRIO_LOWEST>,
+               <&gce 21 CMDQ_THR_PRIO_LOWEST>;
+    };
diff --git a/Documentation/devicetree/bindings/media/mediatek,mdp3-rsz.yaml b/Documentation/devicetree/bindings/media/mediatek,mdp3-rsz.yaml
new file mode 100644
index 000000000000..b136b3845036
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/mediatek,mdp3-rsz.yaml
@@ -0,0 +1,77 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/mediatek,mdp3-rsz.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mediatek Resizer
+
+maintainers:
+  - Matthias Brugger <matthias.bgg@gmail.com>
+  - Ping-Hsun Wu <ping-hsun.wu@mediatek.com>
+
+description: |
+  One of Media Data Path 3 (MDP3) components used to do frame resizing.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - mediatek,mt8183-mdp3-rsz
+
+  reg:
+    maxItems: 1
+
+  mediatek,gce-client-reg:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    items:
+      items:
+        - description: phandle of GCE
+        - description: GCE subsys id
+        - description: register offset
+        - description: register size
+    description: The register of client driver can be configured by gce with
+      4 arguments defined in this property. Each GCE subsys id is mapping to
+      a client defined in the header include/dt-bindings/gce/<chip>-gce.h.
+
+  mediatek,gce-events:
+    description:
+      The event id which is mapping to the specific hardware event signal
+      to gce. The event id is defined in the gce header
+      include/dt-bindings/gce/<chip>-gce.h of each chips.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+
+  clocks:
+    minItems: 1
+
+required:
+  - compatible
+  - reg
+  - mediatek,gce-client-reg
+  - mediatek,gce-events
+  - clocks
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/mt8183-clk.h>
+    #include <dt-bindings/gce/mt8183-gce.h>
+
+    mdp3_rsz0: mdp3-rsz0@14003000 {
+      compatible = "mediatek,mt8183-mdp3-rsz";
+      reg = <0x14003000 0x1000>;
+      mediatek,gce-client-reg = <&gce SUBSYS_1400XXXX 0x3000 0x1000>;
+      mediatek,gce-events = <CMDQ_EVENT_MDP_RSZ0_SOF>,
+                            <CMDQ_EVENT_MDP_RSZ0_EOF>;
+      clocks = <&mmsys CLK_MM_MDP_RSZ0>;
+    };
+
+    mdp3_rsz1: mdp3-rsz1@14004000 {
+      compatible = "mediatek,mt8183-mdp3-rsz";
+      reg = <0x14004000 0x1000>;
+      mediatek,gce-client-reg = <&gce SUBSYS_1400XXXX 0x4000 0x1000>;
+      mediatek,gce-events = <CMDQ_EVENT_MDP_RSZ1_SOF>,
+                            <CMDQ_EVENT_MDP_RSZ1_EOF>;
+      clocks = <&mmsys CLK_MM_MDP_RSZ1>;
+    };
diff --git a/Documentation/devicetree/bindings/media/mediatek,mdp3-wrot.yaml b/Documentation/devicetree/bindings/media/mediatek,mdp3-wrot.yaml
new file mode 100644
index 000000000000..0069b350ee64
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/mediatek,mdp3-wrot.yaml
@@ -0,0 +1,80 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/mediatek,mdp3-wrot.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mediatek Write DMA with Rotation
+
+maintainers:
+  - Matthias Brugger <matthias.bgg@gmail.com>
+  - Ping-Hsun Wu <ping-hsun.wu@mediatek.com>
+
+description: |
+  One of Media Data Path 3 (MDP3) components used to write DMA with frame rotation.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - mediatek,mt8183-mdp3-wrot
+
+  reg:
+    maxItems: 1
+
+  mediatek,gce-client-reg:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    items:
+      items:
+        - description: phandle of GCE
+        - description: GCE subsys id
+        - description: register offset
+        - description: register size
+    description: The register of client driver can be configured by gce with
+      4 arguments defined in this property. Each GCE subsys id is mapping to
+      a client defined in the header include/dt-bindings/gce/<chip>-gce.h.
+
+  mediatek,gce-events:
+    description:
+      The event id which is mapping to the specific hardware event signal
+      to gce. The event id is defined in the gce header
+      include/dt-bindings/gce/<chip>-gce.h of each chips.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+
+  power-domains:
+    maxItems: 1
+
+  clocks:
+    minItems: 1
+
+  iommus:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - mediatek,gce-client-reg
+  - mediatek,gce-events
+  - power-domains
+  - clocks
+  - iommus
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/mt8183-clk.h>
+    #include <dt-bindings/gce/mt8183-gce.h>
+    #include <dt-bindings/power/mt8183-power.h>
+    #include <dt-bindings/memory/mt8183-larb-port.h>
+
+    mdp3_wrot0: mdp3-wrot0@14005000 {
+      compatible = "mediatek,mt8183-mdp3-wrot";
+      reg = <0x14005000 0x1000>;
+      mediatek,gce-client-reg = <&gce SUBSYS_1400XXXX 0x5000 0x1000>;
+      mediatek,gce-events = <CMDQ_EVENT_MDP_WROT0_SOF>,
+                            <CMDQ_EVENT_MDP_WROT0_EOF>;
+      power-domains = <&spm MT8183_POWER_DOMAIN_DISP>;
+      clocks = <&mmsys CLK_MM_MDP_WROT0>;
+      iommus = <&iommu>;
+    };
-- 
2.18.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v21 2/4] dt-binding: mediatek: add bindings for MediaTek CCORR and WDMA

[Moudy Ho]

This patch adds DT binding documentation for MediaTek's CCORR and
WDMA components.
These components exist in both MediaTek's Media Data Path 3(MDP3) and DRM,
and the bindings are placed under the folder "./soc/mediatek" to prevent
duplicate builds.

Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>
Reviewed-by: Rob Herring <robh@kernel.org>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 .../bindings/soc/mediatek/mediatek,ccorr.yaml | 68 ++++++++++++++++
 .../bindings/soc/mediatek/mediatek,wdma.yaml  | 81 +++++++++++++++++++
 2 files changed, 149 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/mediatek/mediatek,ccorr.yaml
 create mode 100644 Documentation/devicetree/bindings/soc/mediatek/mediatek,wdma.yaml

diff --git a/Documentation/devicetree/bindings/soc/mediatek/mediatek,ccorr.yaml b/Documentation/devicetree/bindings/soc/mediatek/mediatek,ccorr.yaml
new file mode 100644
index 000000000000..8677a3f58487
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/mediatek/mediatek,ccorr.yaml
@@ -0,0 +1,68 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/soc/mediatek/mediatek,ccorr.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mediatek color correction
+
+maintainers:
+  - Matthias Brugger <matthias.bgg@gmail.com>
+  - Ping-Hsun Wu <ping-hsun.wu@mediatek.com>
+
+description: |
+  Mediatek color correction with 3X3 matrix.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - mediatek,mt8183-mdp3-ccorr
+
+  reg:
+    maxItems: 1
+
+  mediatek,gce-client-reg:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    items:
+      items:
+        - description: phandle of GCE
+        - description: GCE subsys id
+        - description: register offset
+        - description: register size
+    description: The register of client driver can be configured by gce with
+      4 arguments defined in this property. Each GCE subsys id is mapping to
+      a client defined in the header include/dt-bindings/gce/<chip>-gce.h.
+
+  mediatek,gce-events:
+    description:
+      The event id which is mapping to the specific hardware event signal
+      to gce. The event id is defined in the gce header
+      include/dt-bindings/gce/<chip>-gce.h of each chips.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+
+  clocks:
+    minItems: 1
+
+required:
+  - compatible
+  - reg
+  - mediatek,gce-client-reg
+  - mediatek,gce-events
+  - clocks
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/mt8183-clk.h>
+    #include <dt-bindings/gce/mt8183-gce.h>
+
+    mdp3_ccorr: mdp3-ccorr@1401c000 {
+      compatible = "mediatek,mt8183-mdp3-ccorr";
+      reg = <0x1401c000 0x1000>;
+      mediatek,gce-client-reg = <&gce SUBSYS_1401XXXX 0xc000 0x1000>;
+      mediatek,gce-events = <CMDQ_EVENT_MDP_CCORR_SOF>,
+                            <CMDQ_EVENT_MDP_CCORR_EOF>;
+      clocks = <&mmsys CLK_MM_MDP_CCORR>;
+    };
diff --git a/Documentation/devicetree/bindings/soc/mediatek/mediatek,wdma.yaml b/Documentation/devicetree/bindings/soc/mediatek/mediatek,wdma.yaml
new file mode 100644
index 000000000000..7b01724f21c3
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/mediatek/mediatek,wdma.yaml
@@ -0,0 +1,81 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/soc/mediatek/mediatek,wdma.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mediatek Write Direct Memory Access
+
+maintainers:
+  - Matthias Brugger <matthias.bgg@gmail.com>
+  - Ping-Hsun Wu <ping-hsun.wu@mediatek.com>
+
+description: |
+  Mediatek Write Direct Memory Access(WDMA) component used to write
+  the data into DMA.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - mediatek,mt8183-mdp3-wdma
+
+  reg:
+    maxItems: 1
+
+  mediatek,gce-client-reg:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    items:
+      items:
+        - description: phandle of GCE
+        - description: GCE subsys id
+        - description: register offset
+        - description: register size
+    description: The register of client driver can be configured by gce with
+      4 arguments defined in this property. Each GCE subsys id is mapping to
+      a client defined in the header include/dt-bindings/gce/<chip>-gce.h.
+
+  mediatek,gce-events:
+    description:
+      The event id which is mapping to the specific hardware event signal
+      to gce. The event id is defined in the gce header
+      include/dt-bindings/gce/<chip>-gce.h of each chips.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+
+  power-domains:
+    maxItems: 1
+
+  clocks:
+    minItems: 1
+
+  iommus:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - mediatek,gce-client-reg
+  - mediatek,gce-events
+  - power-domains
+  - clocks
+  - iommus
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/mt8183-clk.h>
+    #include <dt-bindings/gce/mt8183-gce.h>
+    #include <dt-bindings/power/mt8183-power.h>
+    #include <dt-bindings/memory/mt8183-larb-port.h>
+
+    mdp3_wdma: mdp3-wdma@14006000 {
+      compatible = "mediatek,mt8183-mdp3-wdma";
+      reg = <0x14006000 0x1000>;
+      mediatek,gce-client-reg = <&gce SUBSYS_1400XXXX 0x6000 0x1000>;
+      mediatek,gce-events = <CMDQ_EVENT_MDP_WDMA0_SOF>,
+                            <CMDQ_EVENT_MDP_WDMA0_EOF>;
+      power-domains = <&spm MT8183_POWER_DOMAIN_DISP>;
+      clocks = <&mmsys CLK_MM_MDP_WDMA0>;
+      iommus = <&iommu>;
+    };
-- 
2.18.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v21 3/4] arm64: dts: mt8183: add Mediatek MDP3 nodes

[Moudy Ho]

Add device nodes for Media Data Path 3 (MDP3) modules.

Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 arch/arm64/boot/dts/mediatek/mt8183.dtsi | 63 ++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index 9485c1efc87c..daad94a60fb8 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -1691,6 +1691,60 @@
 			mediatek,gce-client-reg = <&gce SUBSYS_1400XXXX 0 0x1000>;
 		};
 
+		mdp3-rdma0@14001000 {
+			compatible = "mediatek,mt8183-mdp3-rdma";
+			reg = <0 0x14001000 0 0x1000>;
+			mediatek,gce-client-reg = <&gce SUBSYS_1400XXXX 0x1000 0x1000>;
+			mediatek,gce-events = <CMDQ_EVENT_MDP_RDMA0_SOF>,
+					      <CMDQ_EVENT_MDP_RDMA0_EOF>;
+			power-domains = <&spm MT8183_POWER_DOMAIN_DISP>;
+			clocks = <&mmsys CLK_MM_MDP_RDMA0>,
+				 <&mmsys CLK_MM_MDP_RSZ1>;
+			iommus = <&iommu M4U_PORT_MDP_RDMA0>;
+			mboxes = <&gce 20 CMDQ_THR_PRIO_LOWEST 0>,
+				 <&gce 21 CMDQ_THR_PRIO_LOWEST 0>;
+		};
+
+		mdp3-rsz0@14003000 {
+			compatible = "mediatek,mt8183-mdp3-rsz";
+			reg = <0 0x14003000 0 0x1000>;
+			mediatek,gce-client-reg = <&gce SUBSYS_1400XXXX 0x3000 0x1000>;
+			mediatek,gce-events = <CMDQ_EVENT_MDP_RSZ0_SOF>,
+					      <CMDQ_EVENT_MDP_RSZ0_EOF>;
+			clocks = <&mmsys CLK_MM_MDP_RSZ0>;
+		};
+
+		mdp3-rsz1@14004000 {
+			compatible = "mediatek,mt8183-mdp3-rsz";
+			reg = <0 0x14004000 0 0x1000>;
+			mediatek,gce-client-reg = <&gce SUBSYS_1400XXXX 0x4000 0x1000>;
+			mediatek,gce-events = <CMDQ_EVENT_MDP_RSZ1_SOF>,
+					      <CMDQ_EVENT_MDP_RSZ1_EOF>;
+			clocks = <&mmsys CLK_MM_MDP_RSZ1>;
+		};
+
+		mdp3-wrot0@14005000 {
+			compatible = "mediatek,mt8183-mdp3-wrot";
+			reg = <0 0x14005000 0 0x1000>;
+			mediatek,gce-client-reg = <&gce SUBSYS_1400XXXX 0x5000 0x1000>;
+			mediatek,gce-events = <CMDQ_EVENT_MDP_WROT0_SOF>,
+					      <CMDQ_EVENT_MDP_WROT0_EOF>;
+			power-domains = <&spm MT8183_POWER_DOMAIN_DISP>;
+			clocks = <&mmsys CLK_MM_MDP_WROT0>;
+			iommus = <&iommu M4U_PORT_MDP_WROT0>;
+		};
+
+		mdp3-wdma@14006000 {
+			compatible = "mediatek,mt8183-mdp3-wdma";
+			reg = <0 0x14006000 0 0x1000>;
+			mediatek,gce-client-reg = <&gce SUBSYS_1400XXXX 0x6000 0x1000>;
+			mediatek,gce-events = <CMDQ_EVENT_MDP_WDMA0_SOF>,
+					      <CMDQ_EVENT_MDP_WDMA0_EOF>;
+			power-domains = <&spm MT8183_POWER_DOMAIN_DISP>;
+			clocks = <&mmsys CLK_MM_MDP_WDMA0>;
+			iommus = <&iommu M4U_PORT_MDP_WDMA0>;
+		};
+
 		ovl0: ovl@14008000 {
 			compatible = "mediatek,mt8183-disp-ovl";
 			reg = <0 0x14008000 0 0x1000>;
@@ -1834,6 +1888,15 @@
 			power-domains = <&spm MT8183_POWER_DOMAIN_DISP>;
 		};
 
+		mdp3-ccorr@1401c000 {
+			compatible = "mediatek,mt8183-mdp3-ccorr";
+			reg = <0 0x1401c000 0 0x1000>;
+			mediatek,gce-client-reg = <&gce SUBSYS_1401XXXX 0xc000 0x1000>;
+			mediatek,gce-events = <CMDQ_EVENT_MDP_CCORR_SOF>,
+					      <CMDQ_EVENT_MDP_CCORR_EOF>;
+			clocks = <&mmsys CLK_MM_MDP_CCORR>;
+		};
+
 		imgsys: syscon@15020000 {
 			compatible = "mediatek,mt8183-imgsys", "syscon";
 			reg = <0 0x15020000 0 0x1000>;
-- 
2.18.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [DKIM] [PATCH v21 4/4] media: platform: mtk-mdp3: add Mediatek MDP3 driver

[Hans Verkuil]

Hi Moudy,

Some comments below:

On 7/6/22 09:54, Moudy Ho wrote:
> This patch adds driver for Mediatek's Media Data Path ver.3 (MDP3).
> It provides the following functions:
>   color transform, format conversion, resize, crop, rotate, flip
>   and additional image quality enhancement.
> 
> The MDP3 driver is mainly used for Google Chromebook products to
> import the new architecture to set the HW settings as shown below:
>   User -> V4L2 framework
>     -> MDP3 driver -> SCP (setting calculations)
>       -> MDP3 driver -> CMDQ (GCE driver) -> HW
> 
> Each modules' related operation control is sited in mtk-mdp3-comp.c
> Each modules' register table is defined in file with "mdp_reg_" prefix
> GCE related API, operation control  sited in mtk-mdp3-cmdq.c
> V4L2 m2m device functions are implemented in mtk-mdp3-m2m.c
> Probe, power, suspend/resume, system level functions are defined in
> mtk-mdp3-core.c
> 
> v4l2-compliance 1.22.1, 32 bits, 32-bit time_t
> 
> Compliance test for mtk-mdp3 device /dev/video2:
> 
> Driver Info:
> 	Driver name      : mtk-mdp3
> 	Card type        : 14001000.mdp3-rdma0

Card type is expected to be a human readable name, and that's not the case
here.

> 	Bus info         : platform:mtk-mdp3

<snip>

> diff --git a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c
> new file mode 100644
> index 000000000000..9d2b8acc303f
> --- /dev/null
> +++ b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c
> @@ -0,0 +1,769 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2022 MediaTek Inc.
> + * Author: Ping-Hsun Wu <ping-hsun.wu@mediatek.com>
> + */
> +
> +#include <linux/platform_device.h>
> +#include <media/v4l2-ioctl.h>
> +#include <media/v4l2-event.h>
> +#include <media/videobuf2-dma-contig.h>
> +#include "mtk-mdp3-m2m.h"
> +
> +static inline struct mdp_m2m_ctx *fh_to_ctx(struct v4l2_fh *fh)
> +{
> +	return container_of(fh, struct mdp_m2m_ctx, fh);
> +}
> +
> +static inline struct mdp_m2m_ctx *ctrl_to_ctx(struct v4l2_ctrl *ctrl)
> +{
> +	return container_of(ctrl->handler, struct mdp_m2m_ctx, ctrl_handler);
> +}
> +
> +static inline struct mdp_frame *ctx_get_frame(struct mdp_m2m_ctx *ctx,
> +					      enum v4l2_buf_type type)
> +{
> +	if (V4L2_TYPE_IS_OUTPUT(type))
> +		return &ctx->curr_param.output;
> +	else
> +		return &ctx->curr_param.captures[0];
> +}
> +
> +static void mdp_m2m_ctx_set_state(struct mdp_m2m_ctx *ctx, u32 state)
> +{
> +	atomic_or(state, &ctx->curr_param.state);
> +}
> +
> +static bool mdp_m2m_ctx_is_state_set(struct mdp_m2m_ctx *ctx, u32 mask)
> +{
> +	bool ret;
> +
> +	ret = ((atomic_read(&ctx->curr_param.state) & mask) == mask);

This can just do 'return' so you can drop the 'ret' variable.

> +
> +	return ret;
> +}
> +
> +static void mdp_m2m_process_done(void *priv, int vb_state)
> +{
> +	struct mdp_m2m_ctx *ctx = priv;
> +	struct vb2_v4l2_buffer *src_vbuf, *dst_vbuf;
> +
> +	src_vbuf = (struct vb2_v4l2_buffer *)
> +			v4l2_m2m_src_buf_remove(ctx->m2m_ctx);
> +	dst_vbuf = (struct vb2_v4l2_buffer *)
> +			v4l2_m2m_dst_buf_remove(ctx->m2m_ctx);
> +	ctx->curr_param.frame_no = ctx->frame_count[MDP_M2M_SRC];
> +	src_vbuf->sequence = ctx->frame_count[MDP_M2M_SRC]++;
> +	dst_vbuf->sequence = ctx->frame_count[MDP_M2M_DST]++;
> +	v4l2_m2m_buf_copy_metadata(src_vbuf, dst_vbuf, true);
> +
> +	v4l2_m2m_buf_done(src_vbuf, vb_state);
> +	v4l2_m2m_buf_done(dst_vbuf, vb_state);
> +	v4l2_m2m_job_finish(ctx->mdp_dev->m2m_dev, ctx->m2m_ctx);
> +}
> +
> +static void mdp_m2m_device_run(void *priv)
> +{
> +	struct mdp_m2m_ctx *ctx = priv;
> +	struct mdp_frame *frame;
> +	struct vb2_v4l2_buffer *src_vb, *dst_vb;
> +	struct img_ipi_frameparam param = {0};
> +	struct mdp_cmdq_param task = {0};

Just use '= {}', it's cleaner.

> +	enum vb2_buffer_state vb_state = VB2_BUF_STATE_ERROR;
> +	int ret;
> +
> +	if (mdp_m2m_ctx_is_state_set(ctx, MDP_M2M_CTX_ERROR)) {
> +		dev_err(&ctx->mdp_dev->pdev->dev,
> +			"mdp_m2m_ctx is in error state\n");
> +		goto worker_end;
> +	}
> +
> +	param.frame_no = ctx->curr_param.frame_no;
> +	param.type = ctx->curr_param.type;
> +	param.num_inputs = 1;
> +	param.num_outputs = 1;
> +
> +	frame = ctx_get_frame(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
> +	src_vb = v4l2_m2m_next_src_buf(ctx->m2m_ctx);
> +	mdp_set_src_config(&param.inputs[0], frame, &src_vb->vb2_buf);
> +
> +	frame = ctx_get_frame(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
> +	dst_vb = v4l2_m2m_next_dst_buf(ctx->m2m_ctx);
> +	mdp_set_dst_config(&param.outputs[0], frame, &dst_vb->vb2_buf);
> +
> +	ret = mdp_vpu_process(&ctx->vpu, &param);
> +	if (ret) {
> +		dev_err(&ctx->mdp_dev->pdev->dev,
> +			"VPU MDP process failed: %d\n", ret);
> +		goto worker_end;
> +	}
> +
> +	task.config = ctx->vpu.config;
> +	task.param = &param;
> +	task.composes[0] = &frame->compose;
> +	task.cmdq_cb = NULL;
> +	task.cb_data = NULL;
> +	task.mdp_ctx = ctx;
> +
> +	ret = mdp_cmdq_send(ctx->mdp_dev, &task);
> +	if (ret) {
> +		dev_err(&ctx->mdp_dev->pdev->dev,
> +			"CMDQ sendtask failed: %d\n", ret);
> +		goto worker_end;
> +	}
> +
> +	return;
> +
> +worker_end:
> +	mdp_m2m_process_done(ctx, vb_state);
> +}
> +
> +static int mdp_m2m_start_streaming(struct vb2_queue *q, unsigned int count)
> +{
> +	struct mdp_m2m_ctx *ctx = vb2_get_drv_priv(q);
> +
> +	ctx->frame_count[MDP_M2M_SRC] = 0;
> +	ctx->frame_count[MDP_M2M_DST] = 0;

Don't set both, check which queue it is first.

> +
> +	return 0;
> +}
> +
> +static struct vb2_v4l2_buffer *mdp_m2m_buf_remove(struct mdp_m2m_ctx *ctx,
> +						  unsigned int type)
> +{
> +	if (V4L2_TYPE_IS_OUTPUT(type))
> +		return (struct vb2_v4l2_buffer *)
> +			v4l2_m2m_src_buf_remove(ctx->m2m_ctx);
> +	else
> +		return (struct vb2_v4l2_buffer *)
> +			v4l2_m2m_dst_buf_remove(ctx->m2m_ctx);
> +}
> +
> +static void mdp_m2m_stop_streaming(struct vb2_queue *q)
> +{
> +	struct mdp_m2m_ctx *ctx = vb2_get_drv_priv(q);
> +	struct vb2_v4l2_buffer *vb;
> +
> +	vb = mdp_m2m_buf_remove(ctx, q->type);
> +	while (vb) {
> +		v4l2_m2m_buf_done(vb, VB2_BUF_STATE_ERROR);
> +		vb = mdp_m2m_buf_remove(ctx, q->type);
> +	}
> +}
> +
> +static int mdp_m2m_queue_setup(struct vb2_queue *q,
> +			       unsigned int *num_buffers,
> +			       unsigned int *num_planes, unsigned int sizes[],
> +			       struct device *alloc_devs[])
> +{
> +	struct mdp_m2m_ctx *ctx = vb2_get_drv_priv(q);
> +	struct v4l2_pix_format_mplane *pix_mp;
> +	struct device *dev = &ctx->mdp_dev->pdev->dev;
> +	u32 i;
> +
> +	pix_mp = &ctx_get_frame(ctx, q->type)->format.fmt.pix_mp;
> +
> +	/* from VIDIOC_CREATE_BUFS */
> +	if (*num_planes) {
> +		if (*num_planes != pix_mp->num_planes)
> +			return -EINVAL;
> +		for (i = 0; i < pix_mp->num_planes; ++i)
> +			if (sizes[i] < pix_mp->plane_fmt[i].sizeimage)
> +				return -EINVAL;
> +	} else {/* from VIDIOC_REQBUFS */
> +		*num_planes = pix_mp->num_planes;
> +		for (i = 0; i < pix_mp->num_planes; ++i)
> +			sizes[i] = pix_mp->plane_fmt[i].sizeimage;
> +	}
> +
> +	dev_info(dev, "[%d] type:%d, planes:%u, buffers:%u, size:%u,%u,%u",

This should be dropped or changed to dev_dbg. You don't want logging
when doing normal things, that will just spam the kernel log.

> +		 ctx->id, q->type, *num_planes, *num_buffers,
> +		 sizes[0], sizes[1], sizes[2]);
> +	return 0;
> +}
> +
> +static int mdp_m2m_buf_prepare(struct vb2_buffer *vb)
> +{
> +	struct mdp_m2m_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
> +	struct v4l2_pix_format_mplane *pix_mp;
> +	struct vb2_v4l2_buffer *v4l2_buf = to_vb2_v4l2_buffer(vb);
> +	u32 i;
> +
> +	v4l2_buf->field = V4L2_FIELD_NONE;
> +
> +	if (!V4L2_TYPE_IS_OUTPUT(vb->type)) {
> +		pix_mp = &ctx_get_frame(ctx, vb->type)->format.fmt.pix_mp;
> +		for (i = 0; i < pix_mp->num_planes; ++i) {
> +			vb2_set_plane_payload(vb, i,
> +					      pix_mp->plane_fmt[i].sizeimage);
> +		}
> +	}
> +	return 0;
> +}
> +
> +static int mdp_m2m_buf_out_validate(struct vb2_buffer *vb)
> +{
> +	struct vb2_v4l2_buffer *v4l2_buf = to_vb2_v4l2_buffer(vb);
> +
> +	v4l2_buf->field = V4L2_FIELD_NONE;
> +
> +	return 0;
> +}
> +
> +static void mdp_m2m_buf_queue(struct vb2_buffer *vb)
> +{
> +	struct mdp_m2m_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
> +	struct vb2_v4l2_buffer *v4l2_buf = to_vb2_v4l2_buffer(vb);
> +
> +	v4l2_buf->field = V4L2_FIELD_NONE;
> +
> +	v4l2_m2m_buf_queue(ctx->m2m_ctx, to_vb2_v4l2_buffer(vb));
> +}
> +
> +static const struct vb2_ops mdp_m2m_qops = {
> +	.queue_setup	= mdp_m2m_queue_setup,
> +	.wait_prepare	= vb2_ops_wait_prepare,
> +	.wait_finish	= vb2_ops_wait_finish,
> +	.buf_prepare	= mdp_m2m_buf_prepare,
> +	.start_streaming = mdp_m2m_start_streaming,
> +	.stop_streaming	= mdp_m2m_stop_streaming,
> +	.buf_queue	= mdp_m2m_buf_queue,
> +	.buf_out_validate = mdp_m2m_buf_out_validate,
> +};
> +
> +static int mdp_m2m_querycap(struct file *file, void *fh,
> +			    struct v4l2_capability *cap)
> +{
> +	struct mdp_m2m_ctx *ctx = fh_to_ctx(fh);
> +
> +	strscpy(cap->driver, MDP_MODULE_NAME, sizeof(cap->driver));
> +	strscpy(cap->card, ctx->mdp_dev->pdev->name, sizeof(cap->card));

As mentioned at the top, this is not a suitable name for cap->card.

> +	strscpy(cap->bus_info, "platform:mtk-mdp3", sizeof(cap->bus_info));
> +
> +	return 0;
> +}
> +
> +static int mdp_m2m_enum_fmt_mplane(struct file *file, void *fh,
> +				   struct v4l2_fmtdesc *f)
> +{
> +	return mdp_enum_fmt_mplane(f);
> +}
> +
> +static int mdp_m2m_g_fmt_mplane(struct file *file, void *fh,
> +				struct v4l2_format *f)
> +{
> +	struct mdp_m2m_ctx *ctx = fh_to_ctx(fh);
> +	struct mdp_frame *frame;
> +	struct v4l2_pix_format_mplane *pix_mp;
> +	struct device *dev = &ctx->mdp_dev->pdev->dev;
> +
> +	frame = ctx_get_frame(ctx, f->type);
> +	*f = frame->format;
> +	pix_mp = &f->fmt.pix_mp;
> +	pix_mp->colorspace = ctx->curr_param.colorspace;
> +	pix_mp->xfer_func = ctx->curr_param.xfer_func;
> +	pix_mp->ycbcr_enc = ctx->curr_param.ycbcr_enc;
> +	pix_mp->quantization = ctx->curr_param.quant;
> +
> +	dev_info(dev, "[%d] type:%d, frame:%ux%u colorspace=%d", ctx->id, f->type,
> +		 f->fmt.pix_mp.width, f->fmt.pix_mp.height,
> +		 f->fmt.pix_mp.colorspace);

Drop this, you're returning this info to userspace anyway, so why spam the
kernel log?

> +	return 0;
> +}
> +
> +static int mdp_m2m_s_fmt_mplane(struct file *file, void *fh,
> +				struct v4l2_format *f)
> +{
> +	struct mdp_m2m_ctx *ctx = fh_to_ctx(fh);
> +	struct mdp_frame *frame = ctx_get_frame(ctx, f->type);
> +	struct mdp_frame *capture;
> +	const struct mdp_format *fmt;
> +	struct vb2_queue *vq;
> +	struct device *dev = &ctx->mdp_dev->pdev->dev;
> +
> +	dev_info(dev, "[%d] type:%d", ctx->id, f->type);

dev_dbg. If dev_info is used elsewhere in similar situations, then replace that
with dev_dbg as well. Regular usage of this driver must not produce kernel logging.

> +
> +	fmt = mdp_try_fmt_mplane(f, &ctx->curr_param, ctx->id);
> +	if (!fmt) {
> +		dev_info(dev, "[%d] try_fmt failed, type:%d", ctx->id, f->type);

Drop this, the error code says enough.

> +		return -EINVAL;
> +	}
> +
> +	vq = v4l2_m2m_get_vq(ctx->m2m_ctx, f->type);
> +	if (vb2_is_streaming(vq)) {

This must be vb2_is_busy(): changing formats is prohibited once buffers are
allocated (vb2_is_busy() is true in that case).

> +		dev_info(&ctx->mdp_dev->pdev->dev, "Queue %d busy\n", f->type);

Drop this, the error code says enough.

> +		return -EBUSY;
> +	}
> +
> +	frame->format = *f;
> +	frame->mdp_fmt = fmt;
> +	frame->ycbcr_prof = mdp_map_ycbcr_prof_mplane(f, fmt->mdp_color);
> +	frame->usage = V4L2_TYPE_IS_OUTPUT(f->type) ?
> +		MDP_BUFFER_USAGE_HW_READ : MDP_BUFFER_USAGE_MDP;
> +
> +	capture = ctx_get_frame(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
> +	if (V4L2_TYPE_IS_OUTPUT(f->type)) {
> +		capture->crop.c.left = 0;
> +		capture->crop.c.top = 0;
> +		capture->crop.c.width = f->fmt.pix_mp.width;
> +		capture->crop.c.height = f->fmt.pix_mp.height;
> +		ctx->curr_param.colorspace = f->fmt.pix_mp.colorspace;
> +		ctx->curr_param.ycbcr_enc = f->fmt.pix_mp.ycbcr_enc;
> +		ctx->curr_param.quant = f->fmt.pix_mp.quantization;
> +		ctx->curr_param.xfer_func = f->fmt.pix_mp.xfer_func;
> +	} else {
> +		capture->compose.left = 0;
> +		capture->compose.top = 0;
> +		capture->compose.width = f->fmt.pix_mp.width;
> +		capture->compose.height = f->fmt.pix_mp.height;
> +	}
> +
> +	ctx->frame_count[MDP_M2M_SRC] = 0;
> +	ctx->frame_count[MDP_M2M_DST] = 0;

Changing the format doesn't mean you need to zero this. Just drop these two
line.

> +
> +	dev_info(dev, "[%d] type:%d, frame:%ux%u", ctx->id, f->type,
> +		 f->fmt.pix_mp.width, f->fmt.pix_mp.height);

Drop this.

> +	return 0;
> +}
> +
> +static int mdp_m2m_try_fmt_mplane(struct file *file, void *fh,
> +				  struct v4l2_format *f)
> +{
> +	struct mdp_m2m_ctx *ctx = fh_to_ctx(fh);
> +
> +	if (!mdp_try_fmt_mplane(f, &ctx->curr_param, ctx->id))
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static int mdp_m2m_streamon(struct file *file, void *fh,
> +			    enum v4l2_buf_type type)
> +{
> +	struct mdp_m2m_ctx *ctx = fh_to_ctx(fh);
> +	struct mdp_frame *capture;
> +	int ret;
> +	bool out_streaming, cap_streaming;
> +
> +	capture = ctx_get_frame(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
> +	out_streaming = ctx->m2m_ctx->out_q_ctx.q.streaming;
> +	cap_streaming = ctx->m2m_ctx->cap_q_ctx.q.streaming;

Use vb2_is_streaming() rather than accessing q.streaming directly.

> +
> +	/* Check to see if scaling ratio is within supported range */
> +	if ((V4L2_TYPE_IS_OUTPUT(type) && cap_streaming) ||
> +	    (!V4L2_TYPE_IS_OUTPUT(type) && out_streaming)) {
> +		ret = mdp_check_scaling_ratio(&capture->crop.c,
> +					      &capture->compose,
> +					      capture->rotation,
> +					      ctx->curr_param.limit);
> +		if (ret) {
> +			dev_info(&ctx->mdp_dev->pdev->dev,
> +				 "Out of scaling range\n");
> +			return ret;
> +		}
> +	}
> +
> +	if (!mdp_m2m_ctx_is_state_set(ctx, MDP_VPU_INIT)) {
> +		ret = mdp_vpu_get_locked(ctx->mdp_dev);
> +		if (ret)
> +			return ret;
> +
> +		ret = mdp_vpu_ctx_init(&ctx->vpu, &ctx->mdp_dev->vpu,
> +				       MDP_DEV_M2M);
> +		if (ret) {
> +			dev_err(&ctx->mdp_dev->pdev->dev,
> +				"VPU init failed %d\n", ret);
> +			return -EINVAL;
> +		}
> +		mdp_m2m_ctx_set_state(ctx, MDP_VPU_INIT);
> +	}
> +
> +	return v4l2_m2m_streamon(file, ctx->m2m_ctx, type);
> +}

There really is no reason to override streamon. You can also do this in
start_streaming(). I recommend moving these tests there. Internally
streamon() will call start_streaming(), so any error you return in that
callback function will be the error return of the streamon as well.

> +
> +static int mdp_m2m_g_selection(struct file *file, void *fh,
> +			       struct v4l2_selection *s)
> +{
> +	struct mdp_m2m_ctx *ctx = fh_to_ctx(fh);
> +	struct mdp_frame *frame;
> +	struct device *dev = &ctx->mdp_dev->pdev->dev;
> +	bool valid = false;
> +
> +	if (s->type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
> +		valid = mdp_target_is_crop(s->target);
> +	else if (s->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
> +		valid = mdp_target_is_compose(s->target);
> +
> +	if (!valid) {
> +		dev_err(dev, "[%s:%d] invalid type:%u target:%u", __func__, ctx->id,
> +			s->type, s->target);

Drop this. User errors should never cause spamming in the kernel log.

Please check your code: you can use dev_dbg to help debugging, but dev_info
should be used very sparingly (typically only in probe() and remove()), and
dev_warn/err only for hardware/driver warnings/errors, and never due to
userspace inputs.

> +		return -EINVAL;
> +	}
> +
> +	switch (s->target) {
> +	case V4L2_SEL_TGT_CROP:
> +		if (s->type == V4L2_BUF_TYPE_VIDEO_OUTPUT) {
> +			frame = ctx_get_frame(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
> +			s->r = frame->crop.c;
> +			return 0;
> +		}

It's easier to read if you swap the tests:

		if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
			return -EINVAL;
		frame = ctx_get_frame(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
		s->r = frame->crop.c;
		return 0;

Same below.

> +		break;
> +	case V4L2_SEL_TGT_COMPOSE:
> +		if (s->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
> +			frame = ctx_get_frame(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
> +			s->r = frame->compose;
> +			return 0;
> +		}
> +		break;
> +	case V4L2_SEL_TGT_CROP_DEFAULT:
> +	case V4L2_SEL_TGT_CROP_BOUNDS:
> +		if (s->type == V4L2_BUF_TYPE_VIDEO_OUTPUT) {
> +			frame = ctx_get_frame(ctx, s->type);
> +			s->r.left = 0;
> +			s->r.top = 0;
> +			s->r.width = frame->format.fmt.pix_mp.width;
> +			s->r.height = frame->format.fmt.pix_mp.height;
> +			return 0;
> +		}
> +		break;
> +	case V4L2_SEL_TGT_COMPOSE_DEFAULT:
> +	case V4L2_SEL_TGT_COMPOSE_BOUNDS:
> +		if (s->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
> +			frame = ctx_get_frame(ctx, s->type);
> +			s->r.left = 0;
> +			s->r.top = 0;
> +			s->r.width = frame->format.fmt.pix_mp.width;
> +			s->r.height = frame->format.fmt.pix_mp.height;
> +			return 0;
> +		}
> +		break;
> +	}
> +	return -EINVAL;
> +}
> +
> +static int mdp_m2m_s_selection(struct file *file, void *fh,
> +			       struct v4l2_selection *s)
> +{
> +	struct mdp_m2m_ctx *ctx = fh_to_ctx(fh);
> +	struct mdp_frame *frame = ctx_get_frame(ctx, s->type);
> +	struct mdp_frame *capture;
> +	struct v4l2_rect r;
> +	struct device *dev = &ctx->mdp_dev->pdev->dev;
> +	bool valid = false;
> +	int ret;
> +
> +	if (s->type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
> +		valid = (s->target == V4L2_SEL_TGT_CROP);
> +	else if (s->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
> +		valid = (s->target == V4L2_SEL_TGT_COMPOSE);
> +
> +	if (!valid) {
> +		dev_err(dev, "[%s:%d] invalid type:%u target:%u", __func__, ctx->id,
> +			s->type, s->target);
> +		return -EINVAL;
> +	}
> +
> +	ret = mdp_try_crop(ctx, &r, s, frame);
> +	if (ret)
> +		return ret;
> +	capture = ctx_get_frame(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
> +
> +	if (mdp_target_is_crop(s->target))
> +		capture->crop.c = r;
> +	else
> +		capture->compose = r;
> +
> +	s->r = r;
> +	memset(s->reserved, 0, sizeof(s->reserved));

No need for this memset, the v4l2 core will clear this for you.

> +
> +	ctx->frame_count[MDP_M2M_SRC] = 0;
> +	ctx->frame_count[MDP_M2M_DST] = 0;

Drop this!

> +	return 0;
> +}
> +
> +static const struct v4l2_ioctl_ops mdp_m2m_ioctl_ops = {
> +	.vidioc_querycap		= mdp_m2m_querycap,
> +	.vidioc_enum_fmt_vid_cap	= mdp_m2m_enum_fmt_mplane,
> +	.vidioc_enum_fmt_vid_out	= mdp_m2m_enum_fmt_mplane,
> +	.vidioc_g_fmt_vid_cap_mplane	= mdp_m2m_g_fmt_mplane,
> +	.vidioc_g_fmt_vid_out_mplane	= mdp_m2m_g_fmt_mplane,
> +	.vidioc_s_fmt_vid_cap_mplane	= mdp_m2m_s_fmt_mplane,
> +	.vidioc_s_fmt_vid_out_mplane	= mdp_m2m_s_fmt_mplane,
> +	.vidioc_try_fmt_vid_cap_mplane	= mdp_m2m_try_fmt_mplane,
> +	.vidioc_try_fmt_vid_out_mplane	= mdp_m2m_try_fmt_mplane,
> +	.vidioc_reqbufs			= v4l2_m2m_ioctl_reqbufs,
> +	.vidioc_querybuf		= v4l2_m2m_ioctl_querybuf,
> +	.vidioc_qbuf			= v4l2_m2m_ioctl_qbuf,
> +	.vidioc_expbuf			= v4l2_m2m_ioctl_expbuf,
> +	.vidioc_dqbuf			= v4l2_m2m_ioctl_dqbuf,
> +	.vidioc_create_bufs		= v4l2_m2m_ioctl_create_bufs,
> +	.vidioc_streamon		= mdp_m2m_streamon,
> +	.vidioc_streamoff		= v4l2_m2m_ioctl_streamoff,
> +	.vidioc_g_selection		= mdp_m2m_g_selection,
> +	.vidioc_s_selection		= mdp_m2m_s_selection,
> +	.vidioc_subscribe_event		= v4l2_ctrl_subscribe_event,
> +	.vidioc_unsubscribe_event	= v4l2_event_unsubscribe,
> +};
> +
> +static int mdp_m2m_queue_init(void *priv,
> +			      struct vb2_queue *src_vq,
> +			      struct vb2_queue *dst_vq)
> +{
> +	struct mdp_m2m_ctx *ctx = priv;
> +	int ret;
> +
> +	src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
> +	src_vq->io_modes = VB2_MMAP | VB2_DMABUF;
> +	src_vq->ops = &mdp_m2m_qops;
> +	src_vq->mem_ops = &vb2_dma_contig_memops;
> +	src_vq->drv_priv = ctx;
> +	src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
> +	src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> +	src_vq->dev = &ctx->mdp_dev->pdev->dev;
> +	src_vq->lock = &ctx->ctx_lock;
> +
> +	ret = vb2_queue_init(src_vq);
> +	if (ret)
> +		return ret;
> +
> +	dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
> +	dst_vq->io_modes = VB2_MMAP | VB2_DMABUF;
> +	dst_vq->ops = &mdp_m2m_qops;
> +	dst_vq->mem_ops = &vb2_dma_contig_memops;
> +	dst_vq->drv_priv = ctx;
> +	dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
> +	dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> +	dst_vq->dev = &ctx->mdp_dev->pdev->dev;
> +	dst_vq->lock = &ctx->ctx_lock;
> +
> +	return vb2_queue_init(dst_vq);
> +}
> +
> +static int mdp_m2m_s_ctrl(struct v4l2_ctrl *ctrl)
> +{
> +	struct mdp_m2m_ctx *ctx = ctrl_to_ctx(ctrl);
> +	struct mdp_frame *capture;
> +
> +	if (ctrl->flags & V4L2_CTRL_FLAG_INACTIVE)
> +		return 0;

Why this test? As far as I can tell this flag is never set, so there
is no need to check against it.

> +
> +	capture = ctx_get_frame(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
> +	switch (ctrl->id) {
> +	case V4L2_CID_HFLIP:
> +		capture->hflip = ctrl->val;
> +		break;
> +	case V4L2_CID_VFLIP:
> +		capture->vflip = ctrl->val;
> +		break;
> +	case V4L2_CID_ROTATE:
> +		capture->rotation = ctrl->val;
> +		break;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct v4l2_ctrl_ops mdp_m2m_ctrl_ops = {
> +	.s_ctrl	= mdp_m2m_s_ctrl,
> +};
> +
> +static int mdp_m2m_ctrls_create(struct mdp_m2m_ctx *ctx)
> +{
> +	v4l2_ctrl_handler_init(&ctx->ctrl_handler, MDP_MAX_CTRLS);
> +	ctx->ctrls.hflip = v4l2_ctrl_new_std(&ctx->ctrl_handler,
> +					     &mdp_m2m_ctrl_ops, V4L2_CID_HFLIP,
> +					     0, 1, 1, 0);
> +	ctx->ctrls.vflip = v4l2_ctrl_new_std(&ctx->ctrl_handler,
> +					     &mdp_m2m_ctrl_ops, V4L2_CID_VFLIP,
> +					     0, 1, 1, 0);
> +	ctx->ctrls.rotate = v4l2_ctrl_new_std(&ctx->ctrl_handler,
> +					      &mdp_m2m_ctrl_ops,
> +					      V4L2_CID_ROTATE, 0, 270, 90, 0);
> +
> +	if (ctx->ctrl_handler.error) {
> +		int err = ctx->ctrl_handler.error;
> +
> +		v4l2_ctrl_handler_free(&ctx->ctrl_handler);
> +		dev_err(&ctx->mdp_dev->pdev->dev,
> +			"Failed to register controls\n");
> +		return err;
> +	}
> +	return 0;
> +}
> +
> +static int mdp_m2m_open(struct file *file)
> +{
> +	struct video_device *vdev = video_devdata(file);
> +	struct mdp_dev *mdp = video_get_drvdata(vdev);
> +	struct mdp_m2m_ctx *ctx;
> +	struct device *dev = &mdp->pdev->dev;
> +	int ret;
> +	struct v4l2_format default_format;

Just add '= {}' to avoid the memset later on.

> +
> +	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
> +	if (!ctx)
> +		return -ENOMEM;
> +
> +	if (mutex_lock_interruptible(&mdp->m2m_lock)) {
> +		ret = -ERESTARTSYS;
> +		goto err_free_ctx;
> +	}
> +
> +	ctx->id = ida_alloc(&mdp->mdp_ida, GFP_KERNEL);
> +	ctx->mdp_dev = mdp;
> +
> +	v4l2_fh_init(&ctx->fh, vdev);
> +	vdev->device_caps = V4L2_CAP_VIDEO_M2M_MPLANE | V4L2_CAP_STREAMING;

This doesn't belong in open(). It is already done when the video device is registered.

> +	file->private_data = &ctx->fh;
> +	ret = mdp_m2m_ctrls_create(ctx);
> +	if (ret)
> +		goto err_exit_fh;
> +
> +	/* Use separate control handler per file handle */
> +	ctx->fh.ctrl_handler = &ctx->ctrl_handler;
> +	v4l2_fh_add(&ctx->fh);
> +
> +	mutex_init(&ctx->ctx_lock);
> +	ctx->m2m_ctx = v4l2_m2m_ctx_init(mdp->m2m_dev, ctx, mdp_m2m_queue_init);
> +	if (IS_ERR(ctx->m2m_ctx)) {
> +		dev_err(dev, "Failed to initialize m2m context\n");
> +		ret = PTR_ERR(ctx->m2m_ctx);
> +		goto err_release_handler;
> +	}
> +	ctx->fh.m2m_ctx = ctx->m2m_ctx;
> +
> +	ctx->curr_param.ctx = ctx;
> +	ret = mdp_frameparam_init(&ctx->curr_param);
> +	if (ret) {
> +		dev_err(dev, "Failed to initialize mdp parameter\n");
> +		goto err_release_m2m_ctx;
> +	}
> +
> +	mutex_unlock(&mdp->m2m_lock);
> +
> +	/* Default format */
> +	memset(&default_format, 0, sizeof(default_format));

This can be dropped if default_format is inited with = {}.

> +	default_format.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
> +	default_format.fmt.pix_mp.width = 32;
> +	default_format.fmt.pix_mp.height = 32;
> +	default_format.fmt.pix_mp.pixelformat = V4L2_PIX_FMT_YUV420M;
> +	mdp_m2m_s_fmt_mplane(file, &ctx->fh, &default_format);
> +	default_format.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
> +	mdp_m2m_s_fmt_mplane(file, &ctx->fh, &default_format);
> +
> +	dev_dbg(dev, "%s:[%d]", __func__, ctx->id);
> +
> +	return 0;
> +
> +err_release_m2m_ctx:
> +	v4l2_m2m_ctx_release(ctx->m2m_ctx);
> +err_release_handler:
> +	v4l2_ctrl_handler_free(&ctx->ctrl_handler);
> +	v4l2_fh_del(&ctx->fh);
> +err_exit_fh:
> +	v4l2_fh_exit(&ctx->fh);
> +	mutex_unlock(&mdp->m2m_lock);
> +err_free_ctx:
> +	kfree(ctx);
> +
> +	return ret;
> +}
> +
> +static int mdp_m2m_release(struct file *file)
> +{
> +	struct mdp_m2m_ctx *ctx = fh_to_ctx(file->private_data);
> +	struct mdp_dev *mdp = video_drvdata(file);
> +	struct device *dev = &mdp->pdev->dev;
> +
> +	mutex_lock(&mdp->m2m_lock);
> +	v4l2_m2m_ctx_release(ctx->m2m_ctx);
> +	if (mdp_m2m_ctx_is_state_set(ctx, MDP_VPU_INIT)) {
> +		mdp_vpu_ctx_deinit(&ctx->vpu);
> +		mdp_vpu_put_locked(mdp);
> +	}
> +
> +	v4l2_ctrl_handler_free(&ctx->ctrl_handler);
> +	v4l2_fh_del(&ctx->fh);
> +	v4l2_fh_exit(&ctx->fh);
> +	ida_free(&mdp->mdp_ida, ctx->id);
> +	mutex_unlock(&mdp->m2m_lock);
> +
> +	dev_dbg(dev, "%s:[%d]", __func__, ctx->id);
> +	kfree(ctx);
> +
> +	return 0;
> +}
> +
> +static const struct v4l2_file_operations mdp_m2m_fops = {
> +	.owner		= THIS_MODULE,
> +	.poll		= v4l2_m2m_fop_poll,
> +	.unlocked_ioctl	= video_ioctl2,
> +	.mmap		= v4l2_m2m_fop_mmap,
> +	.open		= mdp_m2m_open,
> +	.release	= mdp_m2m_release,
> +};
> +
> +static const struct v4l2_m2m_ops mdp_m2m_ops = {
> +	.device_run	= mdp_m2m_device_run,
> +};
> +
> +int mdp_m2m_device_register(struct mdp_dev *mdp)
> +{
> +	struct device *dev = &mdp->pdev->dev;
> +	int ret = 0;
> +
> +	mdp->m2m_vdev = video_device_alloc();
> +	if (!mdp->m2m_vdev) {
> +		dev_err(dev, "Failed to allocate video device\n");
> +		ret = -ENOMEM;
> +		goto err_video_alloc;
> +	}
> +	mdp->m2m_vdev->device_caps = V4L2_CAP_VIDEO_M2M_MPLANE |
> +		V4L2_CAP_STREAMING;
> +	mdp->m2m_vdev->fops = &mdp_m2m_fops;
> +	mdp->m2m_vdev->ioctl_ops = &mdp_m2m_ioctl_ops;
> +	mdp->m2m_vdev->release = mdp_video_device_release;
> +	mdp->m2m_vdev->lock = &mdp->m2m_lock;
> +	mdp->m2m_vdev->vfl_dir = VFL_DIR_M2M;
> +	mdp->m2m_vdev->v4l2_dev = &mdp->v4l2_dev;
> +	snprintf(mdp->m2m_vdev->name, sizeof(mdp->m2m_vdev->name), "%s:m2m",
> +		 MDP_MODULE_NAME);
> +	video_set_drvdata(mdp->m2m_vdev, mdp);
> +
> +	mdp->m2m_dev = v4l2_m2m_init(&mdp_m2m_ops);
> +	if (IS_ERR(mdp->m2m_dev)) {
> +		dev_err(dev, "Failed to initialize v4l2-m2m device\n");
> +		ret = PTR_ERR(mdp->m2m_dev);
> +		goto err_m2m_init;
> +	}
> +
> +	ret = video_register_device(mdp->m2m_vdev, VFL_TYPE_VIDEO, 2);

Why 2? Just use -1 to pick the first free device number.

> +	if (ret) {
> +		dev_err(dev, "Failed to register video device\n");
> +		goto err_video_register;
> +	}
> +
> +	v4l2_info(&mdp->v4l2_dev, "Driver registered as /dev/video%d",
> +		  mdp->m2m_vdev->num);
> +	return 0;
> +
> +err_video_register:
> +	v4l2_m2m_release(mdp->m2m_dev);
> +err_m2m_init:
> +	video_device_release(mdp->m2m_vdev);
> +err_video_alloc:
> +
> +	return ret;
> +}
> +
> +void mdp_m2m_device_unregister(struct mdp_dev *mdp)
> +{
> +	video_unregister_device(mdp->m2m_vdev);
> +}
> +
> +void mdp_m2m_job_finish(struct mdp_m2m_ctx *ctx)
> +{
> +	enum vb2_buffer_state vb_state = VB2_BUF_STATE_DONE;
> +
> +	mdp_m2m_process_done(ctx, vb_state);
> +}
> diff --git a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.h b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.h
> new file mode 100644
> index 000000000000..61ddbaf1bf13
> --- /dev/null
> +++ b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.h
> @@ -0,0 +1,48 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) 2022 MediaTek Inc.
> + * Author: Ping-Hsun Wu <ping-hsun.wu@mediatek.com>
> + */
> +
> +#ifndef __MTK_MDP3_M2M_H__
> +#define __MTK_MDP3_M2M_H__
> +
> +#include <media/v4l2-ctrls.h>
> +#include "mtk-mdp3-core.h"
> +#include "mtk-mdp3-vpu.h"
> +#include "mtk-mdp3-regs.h"
> +
> +#define MDP_MAX_CTRLS	10
> +
> +enum {
> +	MDP_M2M_SRC = 0,
> +	MDP_M2M_DST = 1,
> +	MDP_M2M_MAX,
> +};
> +
> +struct mdp_m2m_ctrls {
> +	struct v4l2_ctrl	*hflip;
> +	struct v4l2_ctrl	*vflip;
> +	struct v4l2_ctrl	*rotate;
> +};
> +
> +struct mdp_m2m_ctx {
> +	u32				id;
> +	struct mdp_dev			*mdp_dev;
> +	struct v4l2_fh			fh;
> +	struct v4l2_ctrl_handler	ctrl_handler;
> +	struct mdp_m2m_ctrls		ctrls;
> +	struct v4l2_m2m_ctx		*m2m_ctx;
> +	struct mdp_vpu_ctx		vpu;
> +	u32				frame_count[MDP_M2M_MAX];
> +
> +	struct mdp_frameparam		curr_param;
> +	/* synchronization protect for mdp m2m context */
> +	struct mutex			ctx_lock;
> +};
> +
> +int mdp_m2m_device_register(struct mdp_dev *mdp);
> +void mdp_m2m_device_unregister(struct mdp_dev *mdp);
> +void mdp_m2m_job_finish(struct mdp_m2m_ctx *ctx);
> +
> +#endif  /* __MTK_MDP3_M2M_H__ */
> diff --git a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-regs.c b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-regs.c
> new file mode 100644
> index 000000000000..18874eb7851c
> --- /dev/null
> +++ b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-regs.c
> @@ -0,0 +1,742 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2022 MediaTek Inc.
> + * Author: Ping-Hsun Wu <ping-hsun.wu@mediatek.com>
> + */
> +
> +#include <media/v4l2-common.h>
> +#include <media/videobuf2-v4l2.h>
> +#include <media/videobuf2-dma-contig.h>
> +#include "mtk-mdp3-core.h"
> +#include "mtk-mdp3-regs.h"
> +#include "mtk-mdp3-m2m.h"
> +
> +/*
> + * All 10-bit related formats are not added in the basic format list,
> + * please add the corresponding format settings before use.
> + */
> +static const struct mdp_format mdp_formats[] = {
> +	{
> +		.pixelformat	= V4L2_PIX_FMT_GREY,
> +		.mdp_color	= MDP_COLOR_GREY,
> +		.depth		= { 8 },
> +		.row_depth	= { 8 },
> +		.num_planes	= 1,
> +		.flags		= MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE,
> +	}, {
> +		.pixelformat	= V4L2_PIX_FMT_RGB565X,
> +		.mdp_color	= MDP_COLOR_BGR565,
> +		.depth		= { 16 },
> +		.row_depth	= { 16 },
> +		.num_planes	= 1,
> +		.flags		= MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE,
> +	}, {
> +		.pixelformat	= V4L2_PIX_FMT_RGB565,
> +		.mdp_color	= MDP_COLOR_RGB565,
> +		.depth		= { 16 },
> +		.row_depth	= { 16 },
> +		.num_planes	= 1,
> +		.flags		= MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE,
> +	}, {
> +		.pixelformat	= V4L2_PIX_FMT_RGB24,
> +		.mdp_color	= MDP_COLOR_RGB888,
> +		.depth		= { 24 },
> +		.row_depth	= { 24 },
> +		.num_planes	= 1,
> +		.flags		= MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE,
> +	}, {
> +		.pixelformat	= V4L2_PIX_FMT_BGR24,
> +		.mdp_color	= MDP_COLOR_BGR888,
> +		.depth		= { 24 },
> +		.row_depth	= { 24 },
> +		.num_planes	= 1,
> +		.flags		= MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE,
> +	}, {
> +		.pixelformat	= V4L2_PIX_FMT_ABGR32,
> +		.mdp_color	= MDP_COLOR_BGRA8888,
> +		.depth		= { 32 },
> +		.row_depth	= { 32 },
> +		.num_planes	= 1,
> +		.flags		= MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE,
> +	}, {
> +		.pixelformat	= V4L2_PIX_FMT_ARGB32,
> +		.mdp_color	= MDP_COLOR_ARGB8888,
> +		.depth		= { 32 },
> +		.row_depth	= { 32 },
> +		.num_planes	= 1,
> +		.flags		= MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE,
> +	}, {
> +		.pixelformat	= V4L2_PIX_FMT_UYVY,
> +		.mdp_color	= MDP_COLOR_UYVY,
> +		.depth		= { 16 },
> +		.row_depth	= { 16 },
> +		.num_planes	= 1,
> +		.walign		= 1,
> +		.flags		= MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE,
> +	}, {
> +		.pixelformat	= V4L2_PIX_FMT_VYUY,
> +		.mdp_color	= MDP_COLOR_VYUY,
> +		.depth		= { 16 },
> +		.row_depth	= { 16 },
> +		.num_planes	= 1,
> +		.walign		= 1,
> +		.flags		= MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE,
> +	}, {
> +		.pixelformat	= V4L2_PIX_FMT_YUYV,
> +		.mdp_color	= MDP_COLOR_YUYV,
> +		.depth		= { 16 },
> +		.row_depth	= { 16 },
> +		.num_planes	= 1,
> +		.walign		= 1,
> +		.flags		= MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE,
> +	}, {
> +		.pixelformat	= V4L2_PIX_FMT_YVYU,
> +		.mdp_color	= MDP_COLOR_YVYU,
> +		.depth		= { 16 },
> +		.row_depth	= { 16 },
> +		.num_planes	= 1,
> +		.walign		= 1,
> +		.flags		= MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE,
> +	}, {
> +		.pixelformat	= V4L2_PIX_FMT_YUV420,
> +		.mdp_color	= MDP_COLOR_I420,
> +		.depth		= { 12 },
> +		.row_depth	= { 8 },
> +		.num_planes	= 1,
> +		.walign		= 1,
> +		.halign		= 1,
> +		.flags		= MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE,
> +	}, {
> +		.pixelformat	= V4L2_PIX_FMT_YVU420,
> +		.mdp_color	= MDP_COLOR_YV12,
> +		.depth		= { 12 },
> +		.row_depth	= { 8 },
> +		.num_planes	= 1,
> +		.walign		= 1,
> +		.halign		= 1,
> +		.flags		= MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE,
> +	}, {
> +		.pixelformat	= V4L2_PIX_FMT_NV12,
> +		.mdp_color	= MDP_COLOR_NV12,
> +		.depth		= { 12 },
> +		.row_depth	= { 8 },
> +		.num_planes	= 1,
> +		.walign		= 1,
> +		.halign		= 1,
> +		.flags		= MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE,
> +	}, {
> +		.pixelformat	= V4L2_PIX_FMT_NV21,
> +		.mdp_color	= MDP_COLOR_NV21,
> +		.depth		= { 12 },
> +		.row_depth	= { 8 },
> +		.num_planes	= 1,
> +		.walign		= 1,
> +		.halign		= 1,
> +		.flags		= MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE,
> +	}, {
> +		.pixelformat	= V4L2_PIX_FMT_NV16,
> +		.mdp_color	= MDP_COLOR_NV16,
> +		.depth		= { 16 },
> +		.row_depth	= { 8 },
> +		.num_planes	= 1,
> +		.walign		= 1,
> +		.flags		= MDP_FMT_FLAG_OUTPUT,
> +	}, {
> +		.pixelformat	= V4L2_PIX_FMT_NV61,
> +		.mdp_color	= MDP_COLOR_NV61,
> +		.depth		= { 16 },
> +		.row_depth	= { 8 },
> +		.num_planes	= 1,
> +		.walign		= 1,
> +		.flags		= MDP_FMT_FLAG_OUTPUT,
> +	}, {
> +		.pixelformat	= V4L2_PIX_FMT_NV24,
> +		.mdp_color	= MDP_COLOR_NV24,
> +		.depth		= { 24 },
> +		.row_depth	= { 8 },
> +		.num_planes	= 1,
> +		.flags		= MDP_FMT_FLAG_OUTPUT,
> +	}, {
> +		.pixelformat	= V4L2_PIX_FMT_NV42,
> +		.mdp_color	= MDP_COLOR_NV42,
> +		.depth		= { 24 },
> +		.row_depth	= { 8 },
> +		.num_planes	= 1,
> +		.flags		= MDP_FMT_FLAG_OUTPUT,
> +	}, {
> +		.pixelformat	= V4L2_PIX_FMT_MT21C,
> +		.mdp_color	= MDP_COLOR_420_BLK_UFO,
> +		.depth		= { 8, 4 },
> +		.row_depth	= { 8, 8 },
> +		.num_planes	= 2,
> +		.walign		= 4,
> +		.halign		= 5,
> +		.flags		= MDP_FMT_FLAG_OUTPUT,
> +	}, {
> +		.pixelformat	= V4L2_PIX_FMT_MM21,
> +		.mdp_color	= MDP_COLOR_420_BLK,
> +		.depth		= { 8, 4 },
> +		.row_depth	= { 8, 8 },
> +		.num_planes	= 2,
> +		.walign		= 4,
> +		.halign		= 5,
> +		.flags		= MDP_FMT_FLAG_OUTPUT,
> +	}, {
> +		.pixelformat	= V4L2_PIX_FMT_NV12M,
> +		.mdp_color	= MDP_COLOR_NV12,
> +		.depth		= { 8, 4 },
> +		.row_depth	= { 8, 8 },
> +		.num_planes	= 2,
> +		.walign		= 1,
> +		.halign		= 1,
> +		.flags		= MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE,
> +	}, {
> +		.pixelformat	= V4L2_PIX_FMT_NV21M,
> +		.mdp_color	= MDP_COLOR_NV21,
> +		.depth		= { 8, 4 },
> +		.row_depth	= { 8, 8 },
> +		.num_planes	= 2,
> +		.walign		= 1,
> +		.halign		= 1,
> +		.flags		= MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE,
> +	}, {
> +		.pixelformat	= V4L2_PIX_FMT_NV16M,
> +		.mdp_color	= MDP_COLOR_NV16,
> +		.depth		= { 8, 8 },
> +		.row_depth	= { 8, 8 },
> +		.num_planes	= 2,
> +		.walign		= 1,
> +		.flags		= MDP_FMT_FLAG_OUTPUT,
> +	}, {
> +		.pixelformat	= V4L2_PIX_FMT_NV61M,
> +		.mdp_color	= MDP_COLOR_NV61,
> +		.depth		= { 8, 8 },
> +		.row_depth	= { 8, 8 },
> +		.num_planes	= 2,
> +		.walign		= 1,
> +		.flags		= MDP_FMT_FLAG_OUTPUT,
> +	}, {
> +		.pixelformat	= V4L2_PIX_FMT_YUV420M,
> +		.mdp_color	= MDP_COLOR_I420,
> +		.depth		= { 8, 2, 2 },
> +		.row_depth	= { 8, 4, 4 },
> +		.num_planes	= 3,
> +		.walign		= 1,
> +		.halign		= 1,
> +		.flags		= MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE,
> +	}, {
> +		.pixelformat	= V4L2_PIX_FMT_YVU420M,
> +		.mdp_color	= MDP_COLOR_YV12,
> +		.depth		= { 8, 2, 2 },
> +		.row_depth	= { 8, 4, 4 },
> +		.num_planes	= 3,
> +		.walign		= 1,
> +		.halign		= 1,
> +		.flags		= MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE,
> +	}
> +};
> +
> +static const struct mdp_limit mdp_def_limit = {
> +	.out_limit = {
> +		.wmin	= 16,
> +		.hmin	= 16,
> +		.wmax	= 8176,
> +		.hmax	= 8176,
> +	},
> +	.cap_limit = {
> +		.wmin	= 2,
> +		.hmin	= 2,
> +		.wmax	= 8176,
> +		.hmax	= 8176,
> +	},
> +	.h_scale_up_max = 32,
> +	.v_scale_up_max = 32,
> +	.h_scale_down_max = 20,
> +	.v_scale_down_max = 128,
> +};
> +
> +static const struct mdp_format *mdp_find_fmt(u32 pixelformat, u32 type)
> +{
> +	u32 i, flag;
> +
> +	flag = V4L2_TYPE_IS_OUTPUT(type) ? MDP_FMT_FLAG_OUTPUT :
> +					MDP_FMT_FLAG_CAPTURE;
> +	for (i = 0; i < ARRAY_SIZE(mdp_formats); ++i) {
> +		if (!(mdp_formats[i].flags & flag))
> +			continue;
> +		if (mdp_formats[i].pixelformat == pixelformat)
> +			return &mdp_formats[i];
> +	}
> +	return NULL;
> +}
> +
> +static const struct mdp_format *mdp_find_fmt_by_index(u32 index, u32 type)
> +{
> +	u32 i, flag, num = 0;
> +
> +	flag = V4L2_TYPE_IS_OUTPUT(type) ? MDP_FMT_FLAG_OUTPUT :
> +					MDP_FMT_FLAG_CAPTURE;
> +	for (i = 0; i < ARRAY_SIZE(mdp_formats); ++i) {
> +		if (!(mdp_formats[i].flags & flag))
> +			continue;
> +		if (index == num)
> +			return &mdp_formats[i];
> +		num++;
> +	}
> +	return NULL;
> +}
> +
> +enum mdp_ycbcr_profile mdp_map_ycbcr_prof_mplane(struct v4l2_format *f,
> +						 u32 mdp_color)
> +{
> +	struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
> +
> +	if (MDP_COLOR_IS_RGB(mdp_color))
> +		return MDP_YCBCR_PROFILE_FULL_BT601;
> +
> +	switch (pix_mp->colorspace) {
> +	case V4L2_COLORSPACE_JPEG:
> +		return MDP_YCBCR_PROFILE_JPEG;
> +	case V4L2_COLORSPACE_REC709:
> +	case V4L2_COLORSPACE_DCI_P3:
> +		if (pix_mp->quantization == V4L2_QUANTIZATION_FULL_RANGE)
> +			return MDP_YCBCR_PROFILE_FULL_BT709;
> +		return MDP_YCBCR_PROFILE_BT709;
> +	case V4L2_COLORSPACE_BT2020:
> +		if (pix_mp->quantization == V4L2_QUANTIZATION_FULL_RANGE)
> +			return MDP_YCBCR_PROFILE_FULL_BT2020;
> +		return MDP_YCBCR_PROFILE_BT2020;
> +	default:
> +		if (pix_mp->quantization == V4L2_QUANTIZATION_FULL_RANGE)
> +			return MDP_YCBCR_PROFILE_FULL_BT601;
> +		return MDP_YCBCR_PROFILE_BT601;
> +	}
> +}
> +
> +static void mdp_bound_align_image(u32 *w, u32 *h,
> +				  struct v4l2_frmsize_stepwise *s,
> +				  unsigned int salign)
> +{
> +	unsigned int org_w, org_h;
> +
> +	org_w = *w;
> +	org_h = *h;
> +	v4l_bound_align_image(w, s->min_width, s->max_width, s->step_width,
> +			      h, s->min_height, s->max_height, s->step_height,
> +			      salign);
> +
> +	s->min_width = org_w;
> +	s->min_height = org_h;
> +	v4l2_apply_frmsize_constraints(w, h, s);
> +}
> +
> +static int mdp_clamp_align(s32 *x, int min, int max, unsigned int align)
> +{
> +	unsigned int mask;
> +
> +	if (min < 0 || max < 0)
> +		return -ERANGE;
> +
> +	/* Bits that must be zero to be aligned */
> +	mask = ~((1 << align) - 1);
> +
> +	min = 0 ? 0 : ((min + ~mask) & mask);
> +	max = max & mask;
> +	if ((unsigned int)min > (unsigned int)max)
> +		return -ERANGE;
> +
> +	/* Clamp to aligned min and max */
> +	*x = clamp(*x, min, max);
> +
> +	/* Round to nearest aligned value */
> +	if (align)
> +		*x = (*x + (1 << (align - 1))) & mask;
> +	return 0;
> +}
> +
> +int mdp_enum_fmt_mplane(struct v4l2_fmtdesc *f)
> +{
> +	const struct mdp_format *fmt;
> +
> +	fmt = mdp_find_fmt_by_index(f->index, f->type);
> +	if (!fmt)
> +		return -EINVAL;
> +
> +	f->pixelformat = fmt->pixelformat;
> +	return 0;
> +}
> +
> +const struct mdp_format *mdp_try_fmt_mplane(struct v4l2_format *f,
> +					    struct mdp_frameparam *param,
> +					    u32 ctx_id)
> +{
> +	struct device *dev = &param->ctx->mdp_dev->pdev->dev;
> +	struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
> +	const struct mdp_format *fmt;
> +	const struct mdp_pix_limit *pix_limit;
> +	struct v4l2_frmsize_stepwise s;
> +	u32 org_w, org_h;
> +	unsigned int i;
> +
> +	fmt = mdp_find_fmt(pix_mp->pixelformat, f->type);
> +	if (!fmt) {
> +		fmt = mdp_find_fmt_by_index(0, f->type);
> +		if (!fmt) {
> +			dev_dbg(dev, "%d: pixelformat %c%c%c%c invalid", ctx_id,
> +				(pix_mp->pixelformat & 0xff),
> +				(pix_mp->pixelformat >>  8) & 0xff,
> +				(pix_mp->pixelformat >> 16) & 0xff,
> +				(pix_mp->pixelformat >> 24) & 0xff);
> +			return NULL;
> +		}
> +	}
> +
> +	pix_mp->field = V4L2_FIELD_NONE;
> +	pix_mp->flags = 0;
> +	pix_mp->pixelformat = fmt->pixelformat;
> +	if (!V4L2_TYPE_IS_OUTPUT(f->type)) {
> +		pix_mp->colorspace = param->colorspace;
> +		pix_mp->xfer_func = param->xfer_func;
> +		pix_mp->ycbcr_enc = param->ycbcr_enc;
> +		pix_mp->quantization = param->quant;
> +	}
> +
> +	pix_limit = V4L2_TYPE_IS_OUTPUT(f->type) ? &param->limit->out_limit :
> +						&param->limit->cap_limit;
> +	s.min_width = pix_limit->wmin;
> +	s.max_width = pix_limit->wmax;
> +	s.step_width = fmt->walign;
> +	s.min_height = pix_limit->hmin;
> +	s.max_height = pix_limit->hmax;
> +	s.step_height = fmt->halign;
> +	org_w = pix_mp->width;
> +	org_h = pix_mp->height;
> +
> +	mdp_bound_align_image(&pix_mp->width, &pix_mp->height, &s, fmt->salign);
> +	if (org_w != pix_mp->width || org_h != pix_mp->height)
> +		dev_dbg(dev, "%d: size change: %ux%u to %ux%u", ctx_id,
> +			org_w, org_h, pix_mp->width, pix_mp->height);
> +
> +	if (pix_mp->num_planes && pix_mp->num_planes != fmt->num_planes)
> +		dev_dbg(dev, "%d num of planes change: %u to %u", ctx_id,
> +			pix_mp->num_planes, fmt->num_planes);
> +	pix_mp->num_planes = fmt->num_planes;
> +
> +	for (i = 0; i < pix_mp->num_planes; ++i) {
> +		u32 min_bpl = (pix_mp->width * fmt->row_depth[i]) >> 3;
> +		u32 max_bpl = (pix_limit->wmax * fmt->row_depth[i]) >> 3;
> +		u32 bpl = pix_mp->plane_fmt[i].bytesperline;
> +		u32 min_si, max_si;
> +		u32 si = pix_mp->plane_fmt[i].sizeimage;
> +
> +		bpl = clamp(bpl, min_bpl, max_bpl);
> +		pix_mp->plane_fmt[i].bytesperline = bpl;
> +
> +		min_si = (bpl * pix_mp->height * fmt->depth[i]) / fmt->row_depth[i];
> +		max_si = (bpl * s.max_height * fmt->depth[i]) / fmt->row_depth[i];
> +
> +		si = clamp(si, min_si, max_si);
> +		pix_mp->plane_fmt[i].sizeimage = si;
> +
> +		dev_dbg(dev, "%d: p%u, bpl:%u [%u, %u], sizeimage:%u [%u, %u]", ctx_id,
> +			i, bpl, min_bpl, max_bpl, si, min_si, max_si);
> +	}
> +
> +	return fmt;
> +}
> +
> +static int mdp_clamp_start(s32 *x, int min, int max, unsigned int align,
> +			   u32 flags)
> +{
> +	if (flags & V4L2_SEL_FLAG_GE)
> +		max = *x;
> +	if (flags & V4L2_SEL_FLAG_LE)
> +		min = *x;
> +	return mdp_clamp_align(x, min, max, align);
> +}
> +
> +static int mdp_clamp_end(s32 *x, int min, int max, unsigned int align,
> +			 u32 flags)
> +{
> +	if (flags & V4L2_SEL_FLAG_GE)
> +		min = *x;
> +	if (flags & V4L2_SEL_FLAG_LE)
> +		max = *x;
> +	return mdp_clamp_align(x, min, max, align);
> +}
> +
> +int mdp_try_crop(struct mdp_m2m_ctx *ctx, struct v4l2_rect *r,
> +		 const struct v4l2_selection *s, struct mdp_frame *frame)
> +{
> +	struct device *dev = &ctx->mdp_dev->pdev->dev;
> +	s32 left, top, right, bottom;
> +	u32 framew, frameh, walign, halign;
> +	int ret;
> +
> +	dev_dbg(dev, "%d target:%d, set:(%d,%d) %ux%u", ctx->id,
> +		s->target, s->r.left, s->r.top, s->r.width, s->r.height);
> +
> +	left = s->r.left;
> +	top = s->r.top;
> +	right = s->r.left + s->r.width;
> +	bottom = s->r.top + s->r.height;
> +	framew = frame->format.fmt.pix_mp.width;
> +	frameh = frame->format.fmt.pix_mp.height;
> +
> +	if (mdp_target_is_crop(s->target)) {
> +		walign = 1;
> +		halign = 1;
> +	} else {
> +		walign = frame->mdp_fmt->walign;
> +		halign = frame->mdp_fmt->halign;
> +	}
> +
> +	dev_dbg(dev, "%d align:%u,%u, bound:%ux%u", ctx->id,
> +		walign, halign, framew, frameh);
> +
> +	ret = mdp_clamp_start(&left, 0, right, walign, s->flags);
> +	if (ret)
> +		return ret;
> +	ret = mdp_clamp_start(&top, 0, bottom, halign, s->flags);
> +	if (ret)
> +		return ret;
> +	ret = mdp_clamp_end(&right, left, framew, walign, s->flags);
> +	if (ret)
> +		return ret;
> +	ret = mdp_clamp_end(&bottom, top, frameh, halign, s->flags);
> +	if (ret)
> +		return ret;
> +
> +	r->left = left;
> +	r->top = top;
> +	r->width = right - left;
> +	r->height = bottom - top;
> +
> +	dev_dbg(dev, "%d crop:(%d,%d) %ux%u", ctx->id,
> +		r->left, r->top, r->width, r->height);
> +	return 0;
> +}
> +
> +int mdp_check_scaling_ratio(const struct v4l2_rect *crop,
> +			    const struct v4l2_rect *compose, s32 rotation,
> +	const struct mdp_limit *limit)
> +{
> +	u32 crop_w, crop_h, comp_w, comp_h;
> +
> +	crop_w = crop->width;
> +	crop_h = crop->height;
> +	if (90 == rotation || 270 == rotation) {
> +		comp_w = compose->height;
> +		comp_h = compose->width;
> +	} else {
> +		comp_w = compose->width;
> +		comp_h = compose->height;
> +	}
> +
> +	if ((crop_w / comp_w) > limit->h_scale_down_max ||
> +	    (crop_h / comp_h) > limit->v_scale_down_max ||
> +	    (comp_w / crop_w) > limit->h_scale_up_max ||
> +	    (comp_h / crop_h) > limit->v_scale_up_max)
> +		return -ERANGE;
> +	return 0;
> +}
> +
> +/* Stride that is accepted by MDP HW */
> +static u32 mdp_fmt_get_stride(const struct mdp_format *fmt,
> +			      u32 bytesperline, unsigned int plane)
> +{
> +	enum mdp_color c = fmt->mdp_color;
> +	u32 stride;
> +
> +	stride = (bytesperline * MDP_COLOR_BITS_PER_PIXEL(c))
> +		/ fmt->row_depth[0];
> +	if (plane == 0)
> +		return stride;
> +	if (plane < MDP_COLOR_GET_PLANE_COUNT(c)) {
> +		if (MDP_COLOR_IS_BLOCK_MODE(c))
> +			stride = stride / 2;
> +		return stride;
> +	}
> +	return 0;
> +}
> +
> +/* Stride that is accepted by MDP HW of format with contiguous planes */
> +static u32 mdp_fmt_get_stride_contig(const struct mdp_format *fmt,
> +				     u32 pix_stride, unsigned int plane)
> +{
> +	enum mdp_color c = fmt->mdp_color;
> +	u32 stride = pix_stride;
> +
> +	if (plane == 0)
> +		return stride;
> +	if (plane < MDP_COLOR_GET_PLANE_COUNT(c)) {
> +		stride = stride >> MDP_COLOR_GET_H_SUBSAMPLE(c);
> +		if (MDP_COLOR_IS_UV_COPLANE(c) && !MDP_COLOR_IS_BLOCK_MODE(c))
> +			stride = stride * 2;
> +		return stride;
> +	}
> +	return 0;
> +}
> +
> +/* Plane size that is accepted by MDP HW */
> +static u32 mdp_fmt_get_plane_size(const struct mdp_format *fmt,
> +				  u32 stride, u32 height, unsigned int plane)
> +{
> +	enum mdp_color c = fmt->mdp_color;
> +	u32 bytesperline;
> +
> +	bytesperline = (stride * fmt->row_depth[0])
> +		/ MDP_COLOR_BITS_PER_PIXEL(c);
> +	if (plane == 0)
> +		return bytesperline * height;
> +	if (plane < MDP_COLOR_GET_PLANE_COUNT(c)) {
> +		height = height >> MDP_COLOR_GET_V_SUBSAMPLE(c);
> +		if (MDP_COLOR_IS_BLOCK_MODE(c))
> +			bytesperline = bytesperline * 2;
> +		return bytesperline * height;
> +	}
> +	return 0;
> +}
> +
> +static void mdp_prepare_buffer(struct img_image_buffer *b,
> +			       struct mdp_frame *frame, struct vb2_buffer *vb)
> +{
> +	struct v4l2_pix_format_mplane *pix_mp = &frame->format.fmt.pix_mp;
> +	unsigned int i;
> +
> +	b->format.colorformat = frame->mdp_fmt->mdp_color;
> +	b->format.ycbcr_prof = frame->ycbcr_prof;
> +	for (i = 0; i < pix_mp->num_planes; ++i) {
> +		u32 stride = mdp_fmt_get_stride(frame->mdp_fmt,
> +			pix_mp->plane_fmt[i].bytesperline, i);
> +
> +		b->format.plane_fmt[i].stride = stride;
> +		/*
> +		 * TODO : The way to pass an offset within a DMA-buf
> +		 * is not defined in V4L2 specification, so we abuse
> +		 * data_offset for now. Fix it when we have the right interface,
> +		 * including any necessary validation and potential alignment
> +		 * issues.
> +		 */
> +		b->format.plane_fmt[i].size =
> +			mdp_fmt_get_plane_size(frame->mdp_fmt, stride,
> +					       pix_mp->height, i) -
> +					       vb->planes[i].data_offset;
> +		b->iova[i] = vb2_dma_contig_plane_dma_addr(vb, i) +
> +			     vb->planes[i].data_offset;

Why do you need data_offset? What is the use case? Obviously I can't
merge a driver that abuses an API.

> +	}
> +	for (; i < MDP_COLOR_GET_PLANE_COUNT(b->format.colorformat); ++i) {
> +		u32 stride = mdp_fmt_get_stride_contig(frame->mdp_fmt,
> +			b->format.plane_fmt[0].stride, i);
> +
> +		b->format.plane_fmt[i].stride = stride;
> +		b->format.plane_fmt[i].size =
> +			mdp_fmt_get_plane_size(frame->mdp_fmt, stride,
> +					       pix_mp->height, i);
> +		b->iova[i] = b->iova[i - 1] + b->format.plane_fmt[i - 1].size;
> +	}
> +	b->usage = frame->usage;
> +}
> +
> +void mdp_set_src_config(struct img_input *in,
> +			struct mdp_frame *frame, struct vb2_buffer *vb)
> +{
> +	in->buffer.format.width = frame->format.fmt.pix_mp.width;
> +	in->buffer.format.height = frame->format.fmt.pix_mp.height;
> +	mdp_prepare_buffer(&in->buffer, frame, vb);
> +}
> +
> +static u32 mdp_to_fixed(u32 *r, struct v4l2_fract *f)
> +{
> +	u32 q;
> +
> +	if (f->denominator == 0) {
> +		*r = 0;
> +		return 0;
> +	}
> +
> +	q = f->numerator / f->denominator;
> +	*r = div_u64(((u64)f->numerator - q * f->denominator) <<
> +		     IMG_SUBPIXEL_SHIFT, f->denominator);
> +	return q;
> +}
> +
> +static void mdp_set_src_crop(struct img_crop *c, struct mdp_crop *crop)
> +{
> +	c->left = crop->c.left
> +		+ mdp_to_fixed(&c->left_subpix, &crop->left_subpix);
> +	c->top = crop->c.top
> +		+ mdp_to_fixed(&c->top_subpix, &crop->top_subpix);
> +	c->width = crop->c.width
> +		+ mdp_to_fixed(&c->width_subpix, &crop->width_subpix);
> +	c->height = crop->c.height
> +		+ mdp_to_fixed(&c->height_subpix, &crop->height_subpix);
> +}
> +
> +static void mdp_set_orientation(struct img_output *out,
> +				s32 rotation, bool hflip, bool vflip)
> +{
> +	u8 flip = 0;
> +
> +	if (hflip)
> +		flip ^= 1;
> +	if (vflip) {
> +		/*
> +		 * A vertical flip is equivalent to
> +		 * a 180-degree rotation with a horizontal flip
> +		 */
> +		rotation += 180;
> +		flip ^= 1;
> +	}
> +
> +	out->rotation = rotation % 360;
> +	if (flip != 0)
> +		out->flags |= IMG_CTRL_FLAG_HFLIP;
> +	else
> +		out->flags &= ~IMG_CTRL_FLAG_HFLIP;
> +}
> +
> +void mdp_set_dst_config(struct img_output *out,
> +			struct mdp_frame *frame, struct vb2_buffer *vb)
> +{
> +	out->buffer.format.width = frame->compose.width;
> +	out->buffer.format.height = frame->compose.height;
> +	mdp_prepare_buffer(&out->buffer, frame, vb);
> +	mdp_set_src_crop(&out->crop, &frame->crop);
> +	mdp_set_orientation(out, frame->rotation, frame->hflip, frame->vflip);
> +}
> +
> +int mdp_frameparam_init(struct mdp_frameparam *param)
> +{
> +	struct mdp_frame *frame;
> +
> +	if (!param)
> +		return -EINVAL;
> +
> +	INIT_LIST_HEAD(&param->list);
> +	param->limit = &mdp_def_limit;
> +	param->type = MDP_STREAM_TYPE_BITBLT;
> +
> +	frame = &param->output;
> +	frame->format.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
> +	frame->mdp_fmt = mdp_try_fmt_mplane(&frame->format, param, 0);
> +	frame->ycbcr_prof =
> +		mdp_map_ycbcr_prof_mplane(&frame->format,
> +					  frame->mdp_fmt->mdp_color);
> +	frame->usage = MDP_BUFFER_USAGE_HW_READ;
> +
> +	param->num_captures = 1;
> +	frame = &param->captures[0];
> +	frame->format.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
> +	frame->mdp_fmt = mdp_try_fmt_mplane(&frame->format, param, 0);
> +	frame->ycbcr_prof =
> +		mdp_map_ycbcr_prof_mplane(&frame->format,
> +					  frame->mdp_fmt->mdp_color);
> +	frame->usage = MDP_BUFFER_USAGE_MDP;
> +	frame->crop.c.width = param->output.format.fmt.pix_mp.width;
> +	frame->crop.c.height = param->output.format.fmt.pix_mp.height;
> +	frame->compose.width = frame->format.fmt.pix_mp.width;
> +	frame->compose.height = frame->format.fmt.pix_mp.height;
> +
> +	return 0;
> +}

Regards,

	Hans

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [DKIM] [PATCH v21 4/4] media: platform: mtk-mdp3: add Mediatek MDP3 driver

[Laurent Pinchart]

On Fri, Jul 08, 2022 at 10:08:44AM +0200, Hans Verkuil wrote:
> Hi Moudy,
> 
> Some comments below:

And one more

> On 7/6/22 09:54, Moudy Ho wrote:
> > This patch adds driver for Mediatek's Media Data Path ver.3 (MDP3).
> > It provides the following functions:
> >   color transform, format conversion, resize, crop, rotate, flip
> >   and additional image quality enhancement.
> > 
> > The MDP3 driver is mainly used for Google Chromebook products to
> > import the new architecture to set the HW settings as shown below:
> >   User -> V4L2 framework
> >     -> MDP3 driver -> SCP (setting calculations)
> >       -> MDP3 driver -> CMDQ (GCE driver) -> HW
> > 
> > Each modules' related operation control is sited in mtk-mdp3-comp.c
> > Each modules' register table is defined in file with "mdp_reg_" prefix
> > GCE related API, operation control  sited in mtk-mdp3-cmdq.c
> > V4L2 m2m device functions are implemented in mtk-mdp3-m2m.c
> > Probe, power, suspend/resume, system level functions are defined in
> > mtk-mdp3-core.c
> > 
> > v4l2-compliance 1.22.1, 32 bits, 32-bit time_t
> > 
> > Compliance test for mtk-mdp3 device /dev/video2:
> > 
> > Driver Info:
> > 	Driver name      : mtk-mdp3
> > 	Card type        : 14001000.mdp3-rdma0
> 
> Card type is expected to be a human readable name, and that's not the case
> here.
> 
> > 	Bus info         : platform:mtk-mdp3
> 
> <snip>
> 
> > diff --git a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c
> > new file mode 100644
> > index 000000000000..9d2b8acc303f
> > --- /dev/null
> > +++ b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c
> > @@ -0,0 +1,769 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (c) 2022 MediaTek Inc.
> > + * Author: Ping-Hsun Wu <ping-hsun.wu@mediatek.com>
> > + */
> > +
> > +#include <linux/platform_device.h>
> > +#include <media/v4l2-ioctl.h>
> > +#include <media/v4l2-event.h>
> > +#include <media/videobuf2-dma-contig.h>
> > +#include "mtk-mdp3-m2m.h"
> > +
> > +static inline struct mdp_m2m_ctx *fh_to_ctx(struct v4l2_fh *fh)
> > +{
> > +	return container_of(fh, struct mdp_m2m_ctx, fh);
> > +}
> > +
> > +static inline struct mdp_m2m_ctx *ctrl_to_ctx(struct v4l2_ctrl *ctrl)
> > +{
> > +	return container_of(ctrl->handler, struct mdp_m2m_ctx, ctrl_handler);
> > +}
> > +
> > +static inline struct mdp_frame *ctx_get_frame(struct mdp_m2m_ctx *ctx,
> > +					      enum v4l2_buf_type type)
> > +{
> > +	if (V4L2_TYPE_IS_OUTPUT(type))
> > +		return &ctx->curr_param.output;
> > +	else
> > +		return &ctx->curr_param.captures[0];
> > +}
> > +
> > +static void mdp_m2m_ctx_set_state(struct mdp_m2m_ctx *ctx, u32 state)
> > +{
> > +	atomic_or(state, &ctx->curr_param.state);
> > +}
> > +
> > +static bool mdp_m2m_ctx_is_state_set(struct mdp_m2m_ctx *ctx, u32 mask)
> > +{
> > +	bool ret;
> > +
> > +	ret = ((atomic_read(&ctx->curr_param.state) & mask) == mask);
> 
> This can just do 'return' so you can drop the 'ret' variable.
> 
> > +
> > +	return ret;
> > +}
> > +
> > +static void mdp_m2m_process_done(void *priv, int vb_state)
> > +{
> > +	struct mdp_m2m_ctx *ctx = priv;
> > +	struct vb2_v4l2_buffer *src_vbuf, *dst_vbuf;
> > +
> > +	src_vbuf = (struct vb2_v4l2_buffer *)
> > +			v4l2_m2m_src_buf_remove(ctx->m2m_ctx);
> > +	dst_vbuf = (struct vb2_v4l2_buffer *)
> > +			v4l2_m2m_dst_buf_remove(ctx->m2m_ctx);
> > +	ctx->curr_param.frame_no = ctx->frame_count[MDP_M2M_SRC];
> > +	src_vbuf->sequence = ctx->frame_count[MDP_M2M_SRC]++;
> > +	dst_vbuf->sequence = ctx->frame_count[MDP_M2M_DST]++;
> > +	v4l2_m2m_buf_copy_metadata(src_vbuf, dst_vbuf, true);
> > +
> > +	v4l2_m2m_buf_done(src_vbuf, vb_state);
> > +	v4l2_m2m_buf_done(dst_vbuf, vb_state);
> > +	v4l2_m2m_job_finish(ctx->mdp_dev->m2m_dev, ctx->m2m_ctx);
> > +}
> > +
> > +static void mdp_m2m_device_run(void *priv)
> > +{
> > +	struct mdp_m2m_ctx *ctx = priv;
> > +	struct mdp_frame *frame;
> > +	struct vb2_v4l2_buffer *src_vb, *dst_vb;
> > +	struct img_ipi_frameparam param = {0};
> > +	struct mdp_cmdq_param task = {0};
> 
> Just use '= {}', it's cleaner.
> 
> > +	enum vb2_buffer_state vb_state = VB2_BUF_STATE_ERROR;
> > +	int ret;
> > +
> > +	if (mdp_m2m_ctx_is_state_set(ctx, MDP_M2M_CTX_ERROR)) {
> > +		dev_err(&ctx->mdp_dev->pdev->dev,
> > +			"mdp_m2m_ctx is in error state\n");
> > +		goto worker_end;
> > +	}
> > +
> > +	param.frame_no = ctx->curr_param.frame_no;
> > +	param.type = ctx->curr_param.type;
> > +	param.num_inputs = 1;
> > +	param.num_outputs = 1;
> > +
> > +	frame = ctx_get_frame(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
> > +	src_vb = v4l2_m2m_next_src_buf(ctx->m2m_ctx);
> > +	mdp_set_src_config(&param.inputs[0], frame, &src_vb->vb2_buf);
> > +
> > +	frame = ctx_get_frame(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
> > +	dst_vb = v4l2_m2m_next_dst_buf(ctx->m2m_ctx);
> > +	mdp_set_dst_config(&param.outputs[0], frame, &dst_vb->vb2_buf);
> > +
> > +	ret = mdp_vpu_process(&ctx->vpu, &param);
> > +	if (ret) {
> > +		dev_err(&ctx->mdp_dev->pdev->dev,
> > +			"VPU MDP process failed: %d\n", ret);
> > +		goto worker_end;
> > +	}
> > +
> > +	task.config = ctx->vpu.config;
> > +	task.param = &param;
> > +	task.composes[0] = &frame->compose;
> > +	task.cmdq_cb = NULL;
> > +	task.cb_data = NULL;
> > +	task.mdp_ctx = ctx;
> > +
> > +	ret = mdp_cmdq_send(ctx->mdp_dev, &task);
> > +	if (ret) {
> > +		dev_err(&ctx->mdp_dev->pdev->dev,
> > +			"CMDQ sendtask failed: %d\n", ret);
> > +		goto worker_end;
> > +	}
> > +
> > +	return;
> > +
> > +worker_end:
> > +	mdp_m2m_process_done(ctx, vb_state);
> > +}
> > +
> > +static int mdp_m2m_start_streaming(struct vb2_queue *q, unsigned int count)
> > +{
> > +	struct mdp_m2m_ctx *ctx = vb2_get_drv_priv(q);
> > +
> > +	ctx->frame_count[MDP_M2M_SRC] = 0;
> > +	ctx->frame_count[MDP_M2M_DST] = 0;
> 
> Don't set both, check which queue it is first.
> 
> > +
> > +	return 0;
> > +}
> > +
> > +static struct vb2_v4l2_buffer *mdp_m2m_buf_remove(struct mdp_m2m_ctx *ctx,
> > +						  unsigned int type)
> > +{
> > +	if (V4L2_TYPE_IS_OUTPUT(type))
> > +		return (struct vb2_v4l2_buffer *)
> > +			v4l2_m2m_src_buf_remove(ctx->m2m_ctx);
> > +	else
> > +		return (struct vb2_v4l2_buffer *)
> > +			v4l2_m2m_dst_buf_remove(ctx->m2m_ctx);
> > +}
> > +
> > +static void mdp_m2m_stop_streaming(struct vb2_queue *q)
> > +{
> > +	struct mdp_m2m_ctx *ctx = vb2_get_drv_priv(q);
> > +	struct vb2_v4l2_buffer *vb;
> > +
> > +	vb = mdp_m2m_buf_remove(ctx, q->type);
> > +	while (vb) {
> > +		v4l2_m2m_buf_done(vb, VB2_BUF_STATE_ERROR);
> > +		vb = mdp_m2m_buf_remove(ctx, q->type);
> > +	}
> > +}
> > +
> > +static int mdp_m2m_queue_setup(struct vb2_queue *q,
> > +			       unsigned int *num_buffers,
> > +			       unsigned int *num_planes, unsigned int sizes[],
> > +			       struct device *alloc_devs[])
> > +{
> > +	struct mdp_m2m_ctx *ctx = vb2_get_drv_priv(q);
> > +	struct v4l2_pix_format_mplane *pix_mp;
> > +	struct device *dev = &ctx->mdp_dev->pdev->dev;
> > +	u32 i;
> > +
> > +	pix_mp = &ctx_get_frame(ctx, q->type)->format.fmt.pix_mp;
> > +
> > +	/* from VIDIOC_CREATE_BUFS */
> > +	if (*num_planes) {
> > +		if (*num_planes != pix_mp->num_planes)
> > +			return -EINVAL;
> > +		for (i = 0; i < pix_mp->num_planes; ++i)
> > +			if (sizes[i] < pix_mp->plane_fmt[i].sizeimage)
> > +				return -EINVAL;
> > +	} else {/* from VIDIOC_REQBUFS */
> > +		*num_planes = pix_mp->num_planes;
> > +		for (i = 0; i < pix_mp->num_planes; ++i)
> > +			sizes[i] = pix_mp->plane_fmt[i].sizeimage;
> > +	}
> > +
> > +	dev_info(dev, "[%d] type:%d, planes:%u, buffers:%u, size:%u,%u,%u",
> 
> This should be dropped or changed to dev_dbg. You don't want logging
> when doing normal things, that will just spam the kernel log.
> 
> > +		 ctx->id, q->type, *num_planes, *num_buffers,
> > +		 sizes[0], sizes[1], sizes[2]);
> > +	return 0;
> > +}
> > +
> > +static int mdp_m2m_buf_prepare(struct vb2_buffer *vb)
> > +{
> > +	struct mdp_m2m_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
> > +	struct v4l2_pix_format_mplane *pix_mp;
> > +	struct vb2_v4l2_buffer *v4l2_buf = to_vb2_v4l2_buffer(vb);
> > +	u32 i;
> > +
> > +	v4l2_buf->field = V4L2_FIELD_NONE;
> > +
> > +	if (!V4L2_TYPE_IS_OUTPUT(vb->type)) {
> > +		pix_mp = &ctx_get_frame(ctx, vb->type)->format.fmt.pix_mp;
> > +		for (i = 0; i < pix_mp->num_planes; ++i) {
> > +			vb2_set_plane_payload(vb, i,
> > +					      pix_mp->plane_fmt[i].sizeimage);
> > +		}
> > +	}
> > +	return 0;
> > +}
> > +
> > +static int mdp_m2m_buf_out_validate(struct vb2_buffer *vb)
> > +{
> > +	struct vb2_v4l2_buffer *v4l2_buf = to_vb2_v4l2_buffer(vb);
> > +
> > +	v4l2_buf->field = V4L2_FIELD_NONE;
> > +
> > +	return 0;
> > +}
> > +
> > +static void mdp_m2m_buf_queue(struct vb2_buffer *vb)
> > +{
> > +	struct mdp_m2m_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
> > +	struct vb2_v4l2_buffer *v4l2_buf = to_vb2_v4l2_buffer(vb);
> > +
> > +	v4l2_buf->field = V4L2_FIELD_NONE;
> > +
> > +	v4l2_m2m_buf_queue(ctx->m2m_ctx, to_vb2_v4l2_buffer(vb));
> > +}
> > +
> > +static const struct vb2_ops mdp_m2m_qops = {
> > +	.queue_setup	= mdp_m2m_queue_setup,
> > +	.wait_prepare	= vb2_ops_wait_prepare,
> > +	.wait_finish	= vb2_ops_wait_finish,
> > +	.buf_prepare	= mdp_m2m_buf_prepare,
> > +	.start_streaming = mdp_m2m_start_streaming,
> > +	.stop_streaming	= mdp_m2m_stop_streaming,
> > +	.buf_queue	= mdp_m2m_buf_queue,
> > +	.buf_out_validate = mdp_m2m_buf_out_validate,
> > +};
> > +
> > +static int mdp_m2m_querycap(struct file *file, void *fh,
> > +			    struct v4l2_capability *cap)
> > +{
> > +	struct mdp_m2m_ctx *ctx = fh_to_ctx(fh);
> > +
> > +	strscpy(cap->driver, MDP_MODULE_NAME, sizeof(cap->driver));
> > +	strscpy(cap->card, ctx->mdp_dev->pdev->name, sizeof(cap->card));
> 
> As mentioned at the top, this is not a suitable name for cap->card.
> 
> > +	strscpy(cap->bus_info, "platform:mtk-mdp3", sizeof(cap->bus_info));

And don't override bus_info, the value isn't right. The V4L2 core should
do the right thing for platform devices now.

> > +	return 0;
> > +}
> > +
> > +static int mdp_m2m_enum_fmt_mplane(struct file *file, void *fh,
> > +				   struct v4l2_fmtdesc *f)
> > +{
> > +	return mdp_enum_fmt_mplane(f);
> > +}
> > +
> > +static int mdp_m2m_g_fmt_mplane(struct file *file, void *fh,
> > +				struct v4l2_format *f)
> > +{
> > +	struct mdp_m2m_ctx *ctx = fh_to_ctx(fh);
> > +	struct mdp_frame *frame;
> > +	struct v4l2_pix_format_mplane *pix_mp;
> > +	struct device *dev = &ctx->mdp_dev->pdev->dev;
> > +
> > +	frame = ctx_get_frame(ctx, f->type);
> > +	*f = frame->format;
> > +	pix_mp = &f->fmt.pix_mp;
> > +	pix_mp->colorspace = ctx->curr_param.colorspace;
> > +	pix_mp->xfer_func = ctx->curr_param.xfer_func;
> > +	pix_mp->ycbcr_enc = ctx->curr_param.ycbcr_enc;
> > +	pix_mp->quantization = ctx->curr_param.quant;
> > +
> > +	dev_info(dev, "[%d] type:%d, frame:%ux%u colorspace=%d", ctx->id, f->type,
> > +		 f->fmt.pix_mp.width, f->fmt.pix_mp.height,
> > +		 f->fmt.pix_mp.colorspace);
> 
> Drop this, you're returning this info to userspace anyway, so why spam the
> kernel log?
> 
> > +	return 0;
> > +}
> > +
> > +static int mdp_m2m_s_fmt_mplane(struct file *file, void *fh,
> > +				struct v4l2_format *f)
> > +{
> > +	struct mdp_m2m_ctx *ctx = fh_to_ctx(fh);
> > +	struct mdp_frame *frame = ctx_get_frame(ctx, f->type);
> > +	struct mdp_frame *capture;
> > +	const struct mdp_format *fmt;
> > +	struct vb2_queue *vq;
> > +	struct device *dev = &ctx->mdp_dev->pdev->dev;
> > +
> > +	dev_info(dev, "[%d] type:%d", ctx->id, f->type);
> 
> dev_dbg. If dev_info is used elsewhere in similar situations, then replace that
> with dev_dbg as well. Regular usage of this driver must not produce kernel logging.
> 
> > +
> > +	fmt = mdp_try_fmt_mplane(f, &ctx->curr_param, ctx->id);
> > +	if (!fmt) {
> > +		dev_info(dev, "[%d] try_fmt failed, type:%d", ctx->id, f->type);
> 
> Drop this, the error code says enough.
> 
> > +		return -EINVAL;
> > +	}
> > +
> > +	vq = v4l2_m2m_get_vq(ctx->m2m_ctx, f->type);
> > +	if (vb2_is_streaming(vq)) {
> 
> This must be vb2_is_busy(): changing formats is prohibited once buffers are
> allocated (vb2_is_busy() is true in that case).
> 
> > +		dev_info(&ctx->mdp_dev->pdev->dev, "Queue %d busy\n", f->type);
> 
> Drop this, the error code says enough.
> 
> > +		return -EBUSY;
> > +	}
> > +
> > +	frame->format = *f;
> > +	frame->mdp_fmt = fmt;
> > +	frame->ycbcr_prof = mdp_map_ycbcr_prof_mplane(f, fmt->mdp_color);
> > +	frame->usage = V4L2_TYPE_IS_OUTPUT(f->type) ?
> > +		MDP_BUFFER_USAGE_HW_READ : MDP_BUFFER_USAGE_MDP;
> > +
> > +	capture = ctx_get_frame(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
> > +	if (V4L2_TYPE_IS_OUTPUT(f->type)) {
> > +		capture->crop.c.left = 0;
> > +		capture->crop.c.top = 0;
> > +		capture->crop.c.width = f->fmt.pix_mp.width;
> > +		capture->crop.c.height = f->fmt.pix_mp.height;
> > +		ctx->curr_param.colorspace = f->fmt.pix_mp.colorspace;
> > +		ctx->curr_param.ycbcr_enc = f->fmt.pix_mp.ycbcr_enc;
> > +		ctx->curr_param.quant = f->fmt.pix_mp.quantization;
> > +		ctx->curr_param.xfer_func = f->fmt.pix_mp.xfer_func;
> > +	} else {
> > +		capture->compose.left = 0;
> > +		capture->compose.top = 0;
> > +		capture->compose.width = f->fmt.pix_mp.width;
> > +		capture->compose.height = f->fmt.pix_mp.height;
> > +	}
> > +
> > +	ctx->frame_count[MDP_M2M_SRC] = 0;
> > +	ctx->frame_count[MDP_M2M_DST] = 0;
> 
> Changing the format doesn't mean you need to zero this. Just drop these two
> line.
> 
> > +
> > +	dev_info(dev, "[%d] type:%d, frame:%ux%u", ctx->id, f->type,
> > +		 f->fmt.pix_mp.width, f->fmt.pix_mp.height);
> 
> Drop this.
> 
> > +	return 0;
> > +}
> > +
> > +static int mdp_m2m_try_fmt_mplane(struct file *file, void *fh,
> > +				  struct v4l2_format *f)
> > +{
> > +	struct mdp_m2m_ctx *ctx = fh_to_ctx(fh);
> > +
> > +	if (!mdp_try_fmt_mplane(f, &ctx->curr_param, ctx->id))
> > +		return -EINVAL;
> > +
> > +	return 0;
> > +}
> > +
> > +static int mdp_m2m_streamon(struct file *file, void *fh,
> > +			    enum v4l2_buf_type type)
> > +{
> > +	struct mdp_m2m_ctx *ctx = fh_to_ctx(fh);
> > +	struct mdp_frame *capture;
> > +	int ret;
> > +	bool out_streaming, cap_streaming;
> > +
> > +	capture = ctx_get_frame(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
> > +	out_streaming = ctx->m2m_ctx->out_q_ctx.q.streaming;
> > +	cap_streaming = ctx->m2m_ctx->cap_q_ctx.q.streaming;
> 
> Use vb2_is_streaming() rather than accessing q.streaming directly.
> 
> > +
> > +	/* Check to see if scaling ratio is within supported range */
> > +	if ((V4L2_TYPE_IS_OUTPUT(type) && cap_streaming) ||
> > +	    (!V4L2_TYPE_IS_OUTPUT(type) && out_streaming)) {
> > +		ret = mdp_check_scaling_ratio(&capture->crop.c,
> > +					      &capture->compose,
> > +					      capture->rotation,
> > +					      ctx->curr_param.limit);
> > +		if (ret) {
> > +			dev_info(&ctx->mdp_dev->pdev->dev,
> > +				 "Out of scaling range\n");
> > +			return ret;
> > +		}
> > +	}
> > +
> > +	if (!mdp_m2m_ctx_is_state_set(ctx, MDP_VPU_INIT)) {
> > +		ret = mdp_vpu_get_locked(ctx->mdp_dev);
> > +		if (ret)
> > +			return ret;
> > +
> > +		ret = mdp_vpu_ctx_init(&ctx->vpu, &ctx->mdp_dev->vpu,
> > +				       MDP_DEV_M2M);
> > +		if (ret) {
> > +			dev_err(&ctx->mdp_dev->pdev->dev,
> > +				"VPU init failed %d\n", ret);
> > +			return -EINVAL;
> > +		}
> > +		mdp_m2m_ctx_set_state(ctx, MDP_VPU_INIT);
> > +	}
> > +
> > +	return v4l2_m2m_streamon(file, ctx->m2m_ctx, type);
> > +}
> 
> There really is no reason to override streamon. You can also do this in
> start_streaming(). I recommend moving these tests there. Internally
> streamon() will call start_streaming(), so any error you return in that
> callback function will be the error return of the streamon as well.
> 
> > +
> > +static int mdp_m2m_g_selection(struct file *file, void *fh,
> > +			       struct v4l2_selection *s)
> > +{
> > +	struct mdp_m2m_ctx *ctx = fh_to_ctx(fh);
> > +	struct mdp_frame *frame;
> > +	struct device *dev = &ctx->mdp_dev->pdev->dev;
> > +	bool valid = false;
> > +
> > +	if (s->type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
> > +		valid = mdp_target_is_crop(s->target);
> > +	else if (s->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
> > +		valid = mdp_target_is_compose(s->target);
> > +
> > +	if (!valid) {
> > +		dev_err(dev, "[%s:%d] invalid type:%u target:%u", __func__, ctx->id,
> > +			s->type, s->target);
> 
> Drop this. User errors should never cause spamming in the kernel log.
> 
> Please check your code: you can use dev_dbg to help debugging, but dev_info
> should be used very sparingly (typically only in probe() and remove()), and
> dev_warn/err only for hardware/driver warnings/errors, and never due to
> userspace inputs.
> 
> > +		return -EINVAL;
> > +	}
> > +
> > +	switch (s->target) {
> > +	case V4L2_SEL_TGT_CROP:
> > +		if (s->type == V4L2_BUF_TYPE_VIDEO_OUTPUT) {
> > +			frame = ctx_get_frame(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
> > +			s->r = frame->crop.c;
> > +			return 0;
> > +		}
> 
> It's easier to read if you swap the tests:
> 
> 		if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
> 			return -EINVAL;
> 		frame = ctx_get_frame(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
> 		s->r = frame->crop.c;
> 		return 0;
> 
> Same below.
> 
> > +		break;
> > +	case V4L2_SEL_TGT_COMPOSE:
> > +		if (s->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
> > +			frame = ctx_get_frame(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
> > +			s->r = frame->compose;
> > +			return 0;
> > +		}
> > +		break;
> > +	case V4L2_SEL_TGT_CROP_DEFAULT:
> > +	case V4L2_SEL_TGT_CROP_BOUNDS:
> > +		if (s->type == V4L2_BUF_TYPE_VIDEO_OUTPUT) {
> > +			frame = ctx_get_frame(ctx, s->type);
> > +			s->r.left = 0;
> > +			s->r.top = 0;
> > +			s->r.width = frame->format.fmt.pix_mp.width;
> > +			s->r.height = frame->format.fmt.pix_mp.height;
> > +			return 0;
> > +		}
> > +		break;
> > +	case V4L2_SEL_TGT_COMPOSE_DEFAULT:
> > +	case V4L2_SEL_TGT_COMPOSE_BOUNDS:
> > +		if (s->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
> > +			frame = ctx_get_frame(ctx, s->type);
> > +			s->r.left = 0;
> > +			s->r.top = 0;
> > +			s->r.width = frame->format.fmt.pix_mp.width;
> > +			s->r.height = frame->format.fmt.pix_mp.height;
> > +			return 0;
> > +		}
> > +		break;
> > +	}
> > +	return -EINVAL;
> > +}
> > +
> > +static int mdp_m2m_s_selection(struct file *file, void *fh,
> > +			       struct v4l2_selection *s)
> > +{
> > +	struct mdp_m2m_ctx *ctx = fh_to_ctx(fh);
> > +	struct mdp_frame *frame = ctx_get_frame(ctx, s->type);
> > +	struct mdp_frame *capture;
> > +	struct v4l2_rect r;
> > +	struct device *dev = &ctx->mdp_dev->pdev->dev;
> > +	bool valid = false;
> > +	int ret;
> > +
> > +	if (s->type == V4L2_BUF_TYPE_VIDEO_OUTPUT)
> > +		valid = (s->target == V4L2_SEL_TGT_CROP);
> > +	else if (s->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
> > +		valid = (s->target == V4L2_SEL_TGT_COMPOSE);
> > +
> > +	if (!valid) {
> > +		dev_err(dev, "[%s:%d] invalid type:%u target:%u", __func__, ctx->id,
> > +			s->type, s->target);
> > +		return -EINVAL;
> > +	}
> > +
> > +	ret = mdp_try_crop(ctx, &r, s, frame);
> > +	if (ret)
> > +		return ret;
> > +	capture = ctx_get_frame(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
> > +
> > +	if (mdp_target_is_crop(s->target))
> > +		capture->crop.c = r;
> > +	else
> > +		capture->compose = r;
> > +
> > +	s->r = r;
> > +	memset(s->reserved, 0, sizeof(s->reserved));
> 
> No need for this memset, the v4l2 core will clear this for you.
> 
> > +
> > +	ctx->frame_count[MDP_M2M_SRC] = 0;
> > +	ctx->frame_count[MDP_M2M_DST] = 0;
> 
> Drop this!
> 
> > +	return 0;
> > +}
> > +
> > +static const struct v4l2_ioctl_ops mdp_m2m_ioctl_ops = {
> > +	.vidioc_querycap		= mdp_m2m_querycap,
> > +	.vidioc_enum_fmt_vid_cap	= mdp_m2m_enum_fmt_mplane,
> > +	.vidioc_enum_fmt_vid_out	= mdp_m2m_enum_fmt_mplane,
> > +	.vidioc_g_fmt_vid_cap_mplane	= mdp_m2m_g_fmt_mplane,
> > +	.vidioc_g_fmt_vid_out_mplane	= mdp_m2m_g_fmt_mplane,
> > +	.vidioc_s_fmt_vid_cap_mplane	= mdp_m2m_s_fmt_mplane,
> > +	.vidioc_s_fmt_vid_out_mplane	= mdp_m2m_s_fmt_mplane,
> > +	.vidioc_try_fmt_vid_cap_mplane	= mdp_m2m_try_fmt_mplane,
> > +	.vidioc_try_fmt_vid_out_mplane	= mdp_m2m_try_fmt_mplane,
> > +	.vidioc_reqbufs			= v4l2_m2m_ioctl_reqbufs,
> > +	.vidioc_querybuf		= v4l2_m2m_ioctl_querybuf,
> > +	.vidioc_qbuf			= v4l2_m2m_ioctl_qbuf,
> > +	.vidioc_expbuf			= v4l2_m2m_ioctl_expbuf,
> > +	.vidioc_dqbuf			= v4l2_m2m_ioctl_dqbuf,
> > +	.vidioc_create_bufs		= v4l2_m2m_ioctl_create_bufs,
> > +	.vidioc_streamon		= mdp_m2m_streamon,
> > +	.vidioc_streamoff		= v4l2_m2m_ioctl_streamoff,
> > +	.vidioc_g_selection		= mdp_m2m_g_selection,
> > +	.vidioc_s_selection		= mdp_m2m_s_selection,
> > +	.vidioc_subscribe_event		= v4l2_ctrl_subscribe_event,
> > +	.vidioc_unsubscribe_event	= v4l2_event_unsubscribe,
> > +};
> > +
> > +static int mdp_m2m_queue_init(void *priv,
> > +			      struct vb2_queue *src_vq,
> > +			      struct vb2_queue *dst_vq)
> > +{
> > +	struct mdp_m2m_ctx *ctx = priv;
> > +	int ret;
> > +
> > +	src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
> > +	src_vq->io_modes = VB2_MMAP | VB2_DMABUF;
> > +	src_vq->ops = &mdp_m2m_qops;
> > +	src_vq->mem_ops = &vb2_dma_contig_memops;
> > +	src_vq->drv_priv = ctx;
> > +	src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
> > +	src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> > +	src_vq->dev = &ctx->mdp_dev->pdev->dev;
> > +	src_vq->lock = &ctx->ctx_lock;
> > +
> > +	ret = vb2_queue_init(src_vq);
> > +	if (ret)
> > +		return ret;
> > +
> > +	dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
> > +	dst_vq->io_modes = VB2_MMAP | VB2_DMABUF;
> > +	dst_vq->ops = &mdp_m2m_qops;
> > +	dst_vq->mem_ops = &vb2_dma_contig_memops;
> > +	dst_vq->drv_priv = ctx;
> > +	dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
> > +	dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> > +	dst_vq->dev = &ctx->mdp_dev->pdev->dev;
> > +	dst_vq->lock = &ctx->ctx_lock;
> > +
> > +	return vb2_queue_init(dst_vq);
> > +}
> > +
> > +static int mdp_m2m_s_ctrl(struct v4l2_ctrl *ctrl)
> > +{
> > +	struct mdp_m2m_ctx *ctx = ctrl_to_ctx(ctrl);
> > +	struct mdp_frame *capture;
> > +
> > +	if (ctrl->flags & V4L2_CTRL_FLAG_INACTIVE)
> > +		return 0;
> 
> Why this test? As far as I can tell this flag is never set, so there
> is no need to check against it.
> 
> > +
> > +	capture = ctx_get_frame(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
> > +	switch (ctrl->id) {
> > +	case V4L2_CID_HFLIP:
> > +		capture->hflip = ctrl->val;
> > +		break;
> > +	case V4L2_CID_VFLIP:
> > +		capture->vflip = ctrl->val;
> > +		break;
> > +	case V4L2_CID_ROTATE:
> > +		capture->rotation = ctrl->val;
> > +		break;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct v4l2_ctrl_ops mdp_m2m_ctrl_ops = {
> > +	.s_ctrl	= mdp_m2m_s_ctrl,
> > +};
> > +
> > +static int mdp_m2m_ctrls_create(struct mdp_m2m_ctx *ctx)
> > +{
> > +	v4l2_ctrl_handler_init(&ctx->ctrl_handler, MDP_MAX_CTRLS);
> > +	ctx->ctrls.hflip = v4l2_ctrl_new_std(&ctx->ctrl_handler,
> > +					     &mdp_m2m_ctrl_ops, V4L2_CID_HFLIP,
> > +					     0, 1, 1, 0);
> > +	ctx->ctrls.vflip = v4l2_ctrl_new_std(&ctx->ctrl_handler,
> > +					     &mdp_m2m_ctrl_ops, V4L2_CID_VFLIP,
> > +					     0, 1, 1, 0);
> > +	ctx->ctrls.rotate = v4l2_ctrl_new_std(&ctx->ctrl_handler,
> > +					      &mdp_m2m_ctrl_ops,
> > +					      V4L2_CID_ROTATE, 0, 270, 90, 0);
> > +
> > +	if (ctx->ctrl_handler.error) {
> > +		int err = ctx->ctrl_handler.error;
> > +
> > +		v4l2_ctrl_handler_free(&ctx->ctrl_handler);
> > +		dev_err(&ctx->mdp_dev->pdev->dev,
> > +			"Failed to register controls\n");
> > +		return err;
> > +	}
> > +	return 0;
> > +}
> > +
> > +static int mdp_m2m_open(struct file *file)
> > +{
> > +	struct video_device *vdev = video_devdata(file);
> > +	struct mdp_dev *mdp = video_get_drvdata(vdev);
> > +	struct mdp_m2m_ctx *ctx;
> > +	struct device *dev = &mdp->pdev->dev;
> > +	int ret;
> > +	struct v4l2_format default_format;
> 
> Just add '= {}' to avoid the memset later on.
> 
> > +
> > +	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
> > +	if (!ctx)
> > +		return -ENOMEM;
> > +
> > +	if (mutex_lock_interruptible(&mdp->m2m_lock)) {
> > +		ret = -ERESTARTSYS;
> > +		goto err_free_ctx;
> > +	}
> > +
> > +	ctx->id = ida_alloc(&mdp->mdp_ida, GFP_KERNEL);
> > +	ctx->mdp_dev = mdp;
> > +
> > +	v4l2_fh_init(&ctx->fh, vdev);
> > +	vdev->device_caps = V4L2_CAP_VIDEO_M2M_MPLANE | V4L2_CAP_STREAMING;
> 
> This doesn't belong in open(). It is already done when the video device is registered.
> 
> > +	file->private_data = &ctx->fh;
> > +	ret = mdp_m2m_ctrls_create(ctx);
> > +	if (ret)
> > +		goto err_exit_fh;
> > +
> > +	/* Use separate control handler per file handle */
> > +	ctx->fh.ctrl_handler = &ctx->ctrl_handler;
> > +	v4l2_fh_add(&ctx->fh);
> > +
> > +	mutex_init(&ctx->ctx_lock);
> > +	ctx->m2m_ctx = v4l2_m2m_ctx_init(mdp->m2m_dev, ctx, mdp_m2m_queue_init);
> > +	if (IS_ERR(ctx->m2m_ctx)) {
> > +		dev_err(dev, "Failed to initialize m2m context\n");
> > +		ret = PTR_ERR(ctx->m2m_ctx);
> > +		goto err_release_handler;
> > +	}
> > +	ctx->fh.m2m_ctx = ctx->m2m_ctx;
> > +
> > +	ctx->curr_param.ctx = ctx;
> > +	ret = mdp_frameparam_init(&ctx->curr_param);
> > +	if (ret) {
> > +		dev_err(dev, "Failed to initialize mdp parameter\n");
> > +		goto err_release_m2m_ctx;
> > +	}
> > +
> > +	mutex_unlock(&mdp->m2m_lock);
> > +
> > +	/* Default format */
> > +	memset(&default_format, 0, sizeof(default_format));
> 
> This can be dropped if default_format is inited with = {}.
> 
> > +	default_format.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
> > +	default_format.fmt.pix_mp.width = 32;
> > +	default_format.fmt.pix_mp.height = 32;
> > +	default_format.fmt.pix_mp.pixelformat = V4L2_PIX_FMT_YUV420M;
> > +	mdp_m2m_s_fmt_mplane(file, &ctx->fh, &default_format);
> > +	default_format.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
> > +	mdp_m2m_s_fmt_mplane(file, &ctx->fh, &default_format);
> > +
> > +	dev_dbg(dev, "%s:[%d]", __func__, ctx->id);
> > +
> > +	return 0;
> > +
> > +err_release_m2m_ctx:
> > +	v4l2_m2m_ctx_release(ctx->m2m_ctx);
> > +err_release_handler:
> > +	v4l2_ctrl_handler_free(&ctx->ctrl_handler);
> > +	v4l2_fh_del(&ctx->fh);
> > +err_exit_fh:
> > +	v4l2_fh_exit(&ctx->fh);
> > +	mutex_unlock(&mdp->m2m_lock);
> > +err_free_ctx:
> > +	kfree(ctx);
> > +
> > +	return ret;
> > +}
> > +
> > +static int mdp_m2m_release(struct file *file)
> > +{
> > +	struct mdp_m2m_ctx *ctx = fh_to_ctx(file->private_data);
> > +	struct mdp_dev *mdp = video_drvdata(file);
> > +	struct device *dev = &mdp->pdev->dev;
> > +
> > +	mutex_lock(&mdp->m2m_lock);
> > +	v4l2_m2m_ctx_release(ctx->m2m_ctx);
> > +	if (mdp_m2m_ctx_is_state_set(ctx, MDP_VPU_INIT)) {
> > +		mdp_vpu_ctx_deinit(&ctx->vpu);
> > +		mdp_vpu_put_locked(mdp);
> > +	}
> > +
> > +	v4l2_ctrl_handler_free(&ctx->ctrl_handler);
> > +	v4l2_fh_del(&ctx->fh);
> > +	v4l2_fh_exit(&ctx->fh);
> > +	ida_free(&mdp->mdp_ida, ctx->id);
> > +	mutex_unlock(&mdp->m2m_lock);
> > +
> > +	dev_dbg(dev, "%s:[%d]", __func__, ctx->id);
> > +	kfree(ctx);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct v4l2_file_operations mdp_m2m_fops = {
> > +	.owner		= THIS_MODULE,
> > +	.poll		= v4l2_m2m_fop_poll,
> > +	.unlocked_ioctl	= video_ioctl2,
> > +	.mmap		= v4l2_m2m_fop_mmap,
> > +	.open		= mdp_m2m_open,
> > +	.release	= mdp_m2m_release,
> > +};
> > +
> > +static const struct v4l2_m2m_ops mdp_m2m_ops = {
> > +	.device_run	= mdp_m2m_device_run,
> > +};
> > +
> > +int mdp_m2m_device_register(struct mdp_dev *mdp)
> > +{
> > +	struct device *dev = &mdp->pdev->dev;
> > +	int ret = 0;
> > +
> > +	mdp->m2m_vdev = video_device_alloc();
> > +	if (!mdp->m2m_vdev) {
> > +		dev_err(dev, "Failed to allocate video device\n");
> > +		ret = -ENOMEM;
> > +		goto err_video_alloc;
> > +	}
> > +	mdp->m2m_vdev->device_caps = V4L2_CAP_VIDEO_M2M_MPLANE |
> > +		V4L2_CAP_STREAMING;
> > +	mdp->m2m_vdev->fops = &mdp_m2m_fops;
> > +	mdp->m2m_vdev->ioctl_ops = &mdp_m2m_ioctl_ops;
> > +	mdp->m2m_vdev->release = mdp_video_device_release;
> > +	mdp->m2m_vdev->lock = &mdp->m2m_lock;
> > +	mdp->m2m_vdev->vfl_dir = VFL_DIR_M2M;
> > +	mdp->m2m_vdev->v4l2_dev = &mdp->v4l2_dev;
> > +	snprintf(mdp->m2m_vdev->name, sizeof(mdp->m2m_vdev->name), "%s:m2m",
> > +		 MDP_MODULE_NAME);
> > +	video_set_drvdata(mdp->m2m_vdev, mdp);
> > +
> > +	mdp->m2m_dev = v4l2_m2m_init(&mdp_m2m_ops);
> > +	if (IS_ERR(mdp->m2m_dev)) {
> > +		dev_err(dev, "Failed to initialize v4l2-m2m device\n");
> > +		ret = PTR_ERR(mdp->m2m_dev);
> > +		goto err_m2m_init;
> > +	}
> > +
> > +	ret = video_register_device(mdp->m2m_vdev, VFL_TYPE_VIDEO, 2);
> 
> Why 2? Just use -1 to pick the first free device number.
> 
> > +	if (ret) {
> > +		dev_err(dev, "Failed to register video device\n");
> > +		goto err_video_register;
> > +	}
> > +
> > +	v4l2_info(&mdp->v4l2_dev, "Driver registered as /dev/video%d",
> > +		  mdp->m2m_vdev->num);
> > +	return 0;
> > +
> > +err_video_register:
> > +	v4l2_m2m_release(mdp->m2m_dev);
> > +err_m2m_init:
> > +	video_device_release(mdp->m2m_vdev);
> > +err_video_alloc:
> > +
> > +	return ret;
> > +}
> > +
> > +void mdp_m2m_device_unregister(struct mdp_dev *mdp)
> > +{
> > +	video_unregister_device(mdp->m2m_vdev);
> > +}
> > +
> > +void mdp_m2m_job_finish(struct mdp_m2m_ctx *ctx)
> > +{
> > +	enum vb2_buffer_state vb_state = VB2_BUF_STATE_DONE;
> > +
> > +	mdp_m2m_process_done(ctx, vb_state);
> > +}
> > diff --git a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.h b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.h
> > new file mode 100644
> > index 000000000000..61ddbaf1bf13
> > --- /dev/null
> > +++ b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.h
> > @@ -0,0 +1,48 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Copyright (c) 2022 MediaTek Inc.
> > + * Author: Ping-Hsun Wu <ping-hsun.wu@mediatek.com>
> > + */
> > +
> > +#ifndef __MTK_MDP3_M2M_H__
> > +#define __MTK_MDP3_M2M_H__
> > +
> > +#include <media/v4l2-ctrls.h>
> > +#include "mtk-mdp3-core.h"
> > +#include "mtk-mdp3-vpu.h"
> > +#include "mtk-mdp3-regs.h"
> > +
> > +#define MDP_MAX_CTRLS	10
> > +
> > +enum {
> > +	MDP_M2M_SRC = 0,
> > +	MDP_M2M_DST = 1,
> > +	MDP_M2M_MAX,
> > +};
> > +
> > +struct mdp_m2m_ctrls {
> > +	struct v4l2_ctrl	*hflip;
> > +	struct v4l2_ctrl	*vflip;
> > +	struct v4l2_ctrl	*rotate;
> > +};
> > +
> > +struct mdp_m2m_ctx {
> > +	u32				id;
> > +	struct mdp_dev			*mdp_dev;
> > +	struct v4l2_fh			fh;
> > +	struct v4l2_ctrl_handler	ctrl_handler;
> > +	struct mdp_m2m_ctrls		ctrls;
> > +	struct v4l2_m2m_ctx		*m2m_ctx;
> > +	struct mdp_vpu_ctx		vpu;
> > +	u32				frame_count[MDP_M2M_MAX];
> > +
> > +	struct mdp_frameparam		curr_param;
> > +	/* synchronization protect for mdp m2m context */
> > +	struct mutex			ctx_lock;
> > +};
> > +
> > +int mdp_m2m_device_register(struct mdp_dev *mdp);
> > +void mdp_m2m_device_unregister(struct mdp_dev *mdp);
> > +void mdp_m2m_job_finish(struct mdp_m2m_ctx *ctx);
> > +
> > +#endif  /* __MTK_MDP3_M2M_H__ */
> > diff --git a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-regs.c b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-regs.c
> > new file mode 100644
> > index 000000000000..18874eb7851c
> > --- /dev/null
> > +++ b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-regs.c
> > @@ -0,0 +1,742 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (c) 2022 MediaTek Inc.
> > + * Author: Ping-Hsun Wu <ping-hsun.wu@mediatek.com>
> > + */
> > +
> > +#include <media/v4l2-common.h>
> > +#include <media/videobuf2-v4l2.h>
> > +#include <media/videobuf2-dma-contig.h>
> > +#include "mtk-mdp3-core.h"
> > +#include "mtk-mdp3-regs.h"
> > +#include "mtk-mdp3-m2m.h"
> > +
> > +/*
> > + * All 10-bit related formats are not added in the basic format list,
> > + * please add the corresponding format settings before use.
> > + */
> > +static const struct mdp_format mdp_formats[] = {
> > +	{
> > +		.pixelformat	= V4L2_PIX_FMT_GREY,
> > +		.mdp_color	= MDP_COLOR_GREY,
> > +		.depth		= { 8 },
> > +		.row_depth	= { 8 },
> > +		.num_planes	= 1,
> > +		.flags		= MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE,
> > +	}, {
> > +		.pixelformat	= V4L2_PIX_FMT_RGB565X,
> > +		.mdp_color	= MDP_COLOR_BGR565,
> > +		.depth		= { 16 },
> > +		.row_depth	= { 16 },
> > +		.num_planes	= 1,
> > +		.flags		= MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE,
> > +	}, {
> > +		.pixelformat	= V4L2_PIX_FMT_RGB565,
> > +		.mdp_color	= MDP_COLOR_RGB565,
> > +		.depth		= { 16 },
> > +		.row_depth	= { 16 },
> > +		.num_planes	= 1,
> > +		.flags		= MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE,
> > +	}, {
> > +		.pixelformat	= V4L2_PIX_FMT_RGB24,
> > +		.mdp_color	= MDP_COLOR_RGB888,
> > +		.depth		= { 24 },
> > +		.row_depth	= { 24 },
> > +		.num_planes	= 1,
> > +		.flags		= MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE,
> > +	}, {
> > +		.pixelformat	= V4L2_PIX_FMT_BGR24,
> > +		.mdp_color	= MDP_COLOR_BGR888,
> > +		.depth		= { 24 },
> > +		.row_depth	= { 24 },
> > +		.num_planes	= 1,
> > +		.flags		= MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE,
> > +	}, {
> > +		.pixelformat	= V4L2_PIX_FMT_ABGR32,
> > +		.mdp_color	= MDP_COLOR_BGRA8888,
> > +		.depth		= { 32 },
> > +		.row_depth	= { 32 },
> > +		.num_planes	= 1,
> > +		.flags		= MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE,
> > +	}, {
> > +		.pixelformat	= V4L2_PIX_FMT_ARGB32,
> > +		.mdp_color	= MDP_COLOR_ARGB8888,
> > +		.depth		= { 32 },
> > +		.row_depth	= { 32 },
> > +		.num_planes	= 1,
> > +		.flags		= MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE,
> > +	}, {
> > +		.pixelformat	= V4L2_PIX_FMT_UYVY,
> > +		.mdp_color	= MDP_COLOR_UYVY,
> > +		.depth		= { 16 },
> > +		.row_depth	= { 16 },
> > +		.num_planes	= 1,
> > +		.walign		= 1,
> > +		.flags		= MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE,
> > +	}, {
> > +		.pixelformat	= V4L2_PIX_FMT_VYUY,
> > +		.mdp_color	= MDP_COLOR_VYUY,
> > +		.depth		= { 16 },
> > +		.row_depth	= { 16 },
> > +		.num_planes	= 1,
> > +		.walign		= 1,
> > +		.flags		= MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE,
> > +	}, {
> > +		.pixelformat	= V4L2_PIX_FMT_YUYV,
> > +		.mdp_color	= MDP_COLOR_YUYV,
> > +		.depth		= { 16 },
> > +		.row_depth	= { 16 },
> > +		.num_planes	= 1,
> > +		.walign		= 1,
> > +		.flags		= MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE,
> > +	}, {
> > +		.pixelformat	= V4L2_PIX_FMT_YVYU,
> > +		.mdp_color	= MDP_COLOR_YVYU,
> > +		.depth		= { 16 },
> > +		.row_depth	= { 16 },
> > +		.num_planes	= 1,
> > +		.walign		= 1,
> > +		.flags		= MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE,
> > +	}, {
> > +		.pixelformat	= V4L2_PIX_FMT_YUV420,
> > +		.mdp_color	= MDP_COLOR_I420,
> > +		.depth		= { 12 },
> > +		.row_depth	= { 8 },
> > +		.num_planes	= 1,
> > +		.walign		= 1,
> > +		.halign		= 1,
> > +		.flags		= MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE,
> > +	}, {
> > +		.pixelformat	= V4L2_PIX_FMT_YVU420,
> > +		.mdp_color	= MDP_COLOR_YV12,
> > +		.depth		= { 12 },
> > +		.row_depth	= { 8 },
> > +		.num_planes	= 1,
> > +		.walign		= 1,
> > +		.halign		= 1,
> > +		.flags		= MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE,
> > +	}, {
> > +		.pixelformat	= V4L2_PIX_FMT_NV12,
> > +		.mdp_color	= MDP_COLOR_NV12,
> > +		.depth		= { 12 },
> > +		.row_depth	= { 8 },
> > +		.num_planes	= 1,
> > +		.walign		= 1,
> > +		.halign		= 1,
> > +		.flags		= MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE,
> > +	}, {
> > +		.pixelformat	= V4L2_PIX_FMT_NV21,
> > +		.mdp_color	= MDP_COLOR_NV21,
> > +		.depth		= { 12 },
> > +		.row_depth	= { 8 },
> > +		.num_planes	= 1,
> > +		.walign		= 1,
> > +		.halign		= 1,
> > +		.flags		= MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE,
> > +	}, {
> > +		.pixelformat	= V4L2_PIX_FMT_NV16,
> > +		.mdp_color	= MDP_COLOR_NV16,
> > +		.depth		= { 16 },
> > +		.row_depth	= { 8 },
> > +		.num_planes	= 1,
> > +		.walign		= 1,
> > +		.flags		= MDP_FMT_FLAG_OUTPUT,
> > +	}, {
> > +		.pixelformat	= V4L2_PIX_FMT_NV61,
> > +		.mdp_color	= MDP_COLOR_NV61,
> > +		.depth		= { 16 },
> > +		.row_depth	= { 8 },
> > +		.num_planes	= 1,
> > +		.walign		= 1,
> > +		.flags		= MDP_FMT_FLAG_OUTPUT,
> > +	}, {
> > +		.pixelformat	= V4L2_PIX_FMT_NV24,
> > +		.mdp_color	= MDP_COLOR_NV24,
> > +		.depth		= { 24 },
> > +		.row_depth	= { 8 },
> > +		.num_planes	= 1,
> > +		.flags		= MDP_FMT_FLAG_OUTPUT,
> > +	}, {
> > +		.pixelformat	= V4L2_PIX_FMT_NV42,
> > +		.mdp_color	= MDP_COLOR_NV42,
> > +		.depth		= { 24 },
> > +		.row_depth	= { 8 },
> > +		.num_planes	= 1,
> > +		.flags		= MDP_FMT_FLAG_OUTPUT,
> > +	}, {
> > +		.pixelformat	= V4L2_PIX_FMT_MT21C,
> > +		.mdp_color	= MDP_COLOR_420_BLK_UFO,
> > +		.depth		= { 8, 4 },
> > +		.row_depth	= { 8, 8 },
> > +		.num_planes	= 2,
> > +		.walign		= 4,
> > +		.halign		= 5,
> > +		.flags		= MDP_FMT_FLAG_OUTPUT,
> > +	}, {
> > +		.pixelformat	= V4L2_PIX_FMT_MM21,
> > +		.mdp_color	= MDP_COLOR_420_BLK,
> > +		.depth		= { 8, 4 },
> > +		.row_depth	= { 8, 8 },
> > +		.num_planes	= 2,
> > +		.walign		= 4,
> > +		.halign		= 5,
> > +		.flags		= MDP_FMT_FLAG_OUTPUT,
> > +	}, {
> > +		.pixelformat	= V4L2_PIX_FMT_NV12M,
> > +		.mdp_color	= MDP_COLOR_NV12,
> > +		.depth		= { 8, 4 },
> > +		.row_depth	= { 8, 8 },
> > +		.num_planes	= 2,
> > +		.walign		= 1,
> > +		.halign		= 1,
> > +		.flags		= MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE,
> > +	}, {
> > +		.pixelformat	= V4L2_PIX_FMT_NV21M,
> > +		.mdp_color	= MDP_COLOR_NV21,
> > +		.depth		= { 8, 4 },
> > +		.row_depth	= { 8, 8 },
> > +		.num_planes	= 2,
> > +		.walign		= 1,
> > +		.halign		= 1,
> > +		.flags		= MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE,
> > +	}, {
> > +		.pixelformat	= V4L2_PIX_FMT_NV16M,
> > +		.mdp_color	= MDP_COLOR_NV16,
> > +		.depth		= { 8, 8 },
> > +		.row_depth	= { 8, 8 },
> > +		.num_planes	= 2,
> > +		.walign		= 1,
> > +		.flags		= MDP_FMT_FLAG_OUTPUT,
> > +	}, {
> > +		.pixelformat	= V4L2_PIX_FMT_NV61M,
> > +		.mdp_color	= MDP_COLOR_NV61,
> > +		.depth		= { 8, 8 },
> > +		.row_depth	= { 8, 8 },
> > +		.num_planes	= 2,
> > +		.walign		= 1,
> > +		.flags		= MDP_FMT_FLAG_OUTPUT,
> > +	}, {
> > +		.pixelformat	= V4L2_PIX_FMT_YUV420M,
> > +		.mdp_color	= MDP_COLOR_I420,
> > +		.depth		= { 8, 2, 2 },
> > +		.row_depth	= { 8, 4, 4 },
> > +		.num_planes	= 3,
> > +		.walign		= 1,
> > +		.halign		= 1,
> > +		.flags		= MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE,
> > +	}, {
> > +		.pixelformat	= V4L2_PIX_FMT_YVU420M,
> > +		.mdp_color	= MDP_COLOR_YV12,
> > +		.depth		= { 8, 2, 2 },
> > +		.row_depth	= { 8, 4, 4 },
> > +		.num_planes	= 3,
> > +		.walign		= 1,
> > +		.halign		= 1,
> > +		.flags		= MDP_FMT_FLAG_OUTPUT | MDP_FMT_FLAG_CAPTURE,
> > +	}
> > +};
> > +
> > +static const struct mdp_limit mdp_def_limit = {
> > +	.out_limit = {
> > +		.wmin	= 16,
> > +		.hmin	= 16,
> > +		.wmax	= 8176,
> > +		.hmax	= 8176,
> > +	},
> > +	.cap_limit = {
> > +		.wmin	= 2,
> > +		.hmin	= 2,
> > +		.wmax	= 8176,
> > +		.hmax	= 8176,
> > +	},
> > +	.h_scale_up_max = 32,
> > +	.v_scale_up_max = 32,
> > +	.h_scale_down_max = 20,
> > +	.v_scale_down_max = 128,
> > +};
> > +
> > +static const struct mdp_format *mdp_find_fmt(u32 pixelformat, u32 type)
> > +{
> > +	u32 i, flag;
> > +
> > +	flag = V4L2_TYPE_IS_OUTPUT(type) ? MDP_FMT_FLAG_OUTPUT :
> > +					MDP_FMT_FLAG_CAPTURE;
> > +	for (i = 0; i < ARRAY_SIZE(mdp_formats); ++i) {
> > +		if (!(mdp_formats[i].flags & flag))
> > +			continue;
> > +		if (mdp_formats[i].pixelformat == pixelformat)
> > +			return &mdp_formats[i];
> > +	}
> > +	return NULL;
> > +}
> > +
> > +static const struct mdp_format *mdp_find_fmt_by_index(u32 index, u32 type)
> > +{
> > +	u32 i, flag, num = 0;
> > +
> > +	flag = V4L2_TYPE_IS_OUTPUT(type) ? MDP_FMT_FLAG_OUTPUT :
> > +					MDP_FMT_FLAG_CAPTURE;
> > +	for (i = 0; i < ARRAY_SIZE(mdp_formats); ++i) {
> > +		if (!(mdp_formats[i].flags & flag))
> > +			continue;
> > +		if (index == num)
> > +			return &mdp_formats[i];
> > +		num++;
> > +	}
> > +	return NULL;
> > +}
> > +
> > +enum mdp_ycbcr_profile mdp_map_ycbcr_prof_mplane(struct v4l2_format *f,
> > +						 u32 mdp_color)
> > +{
> > +	struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
> > +
> > +	if (MDP_COLOR_IS_RGB(mdp_color))
> > +		return MDP_YCBCR_PROFILE_FULL_BT601;
> > +
> > +	switch (pix_mp->colorspace) {
> > +	case V4L2_COLORSPACE_JPEG:
> > +		return MDP_YCBCR_PROFILE_JPEG;
> > +	case V4L2_COLORSPACE_REC709:
> > +	case V4L2_COLORSPACE_DCI_P3:
> > +		if (pix_mp->quantization == V4L2_QUANTIZATION_FULL_RANGE)
> > +			return MDP_YCBCR_PROFILE_FULL_BT709;
> > +		return MDP_YCBCR_PROFILE_BT709;
> > +	case V4L2_COLORSPACE_BT2020:
> > +		if (pix_mp->quantization == V4L2_QUANTIZATION_FULL_RANGE)
> > +			return MDP_YCBCR_PROFILE_FULL_BT2020;
> > +		return MDP_YCBCR_PROFILE_BT2020;
> > +	default:
> > +		if (pix_mp->quantization == V4L2_QUANTIZATION_FULL_RANGE)
> > +			return MDP_YCBCR_PROFILE_FULL_BT601;
> > +		return MDP_YCBCR_PROFILE_BT601;
> > +	}
> > +}
> > +
> > +static void mdp_bound_align_image(u32 *w, u32 *h,
> > +				  struct v4l2_frmsize_stepwise *s,
> > +				  unsigned int salign)
> > +{
> > +	unsigned int org_w, org_h;
> > +
> > +	org_w = *w;
> > +	org_h = *h;
> > +	v4l_bound_align_image(w, s->min_width, s->max_width, s->step_width,
> > +			      h, s->min_height, s->max_height, s->step_height,
> > +			      salign);
> > +
> > +	s->min_width = org_w;
> > +	s->min_height = org_h;
> > +	v4l2_apply_frmsize_constraints(w, h, s);
> > +}
> > +
> > +static int mdp_clamp_align(s32 *x, int min, int max, unsigned int align)
> > +{
> > +	unsigned int mask;
> > +
> > +	if (min < 0 || max < 0)
> > +		return -ERANGE;
> > +
> > +	/* Bits that must be zero to be aligned */
> > +	mask = ~((1 << align) - 1);
> > +
> > +	min = 0 ? 0 : ((min + ~mask) & mask);
> > +	max = max & mask;
> > +	if ((unsigned int)min > (unsigned int)max)
> > +		return -ERANGE;
> > +
> > +	/* Clamp to aligned min and max */
> > +	*x = clamp(*x, min, max);
> > +
> > +	/* Round to nearest aligned value */
> > +	if (align)
> > +		*x = (*x + (1 << (align - 1))) & mask;
> > +	return 0;
> > +}
> > +
> > +int mdp_enum_fmt_mplane(struct v4l2_fmtdesc *f)
> > +{
> > +	const struct mdp_format *fmt;
> > +
> > +	fmt = mdp_find_fmt_by_index(f->index, f->type);
> > +	if (!fmt)
> > +		return -EINVAL;
> > +
> > +	f->pixelformat = fmt->pixelformat;
> > +	return 0;
> > +}
> > +
> > +const struct mdp_format *mdp_try_fmt_mplane(struct v4l2_format *f,
> > +					    struct mdp_frameparam *param,
> > +					    u32 ctx_id)
> > +{
> > +	struct device *dev = &param->ctx->mdp_dev->pdev->dev;
> > +	struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
> > +	const struct mdp_format *fmt;
> > +	const struct mdp_pix_limit *pix_limit;
> > +	struct v4l2_frmsize_stepwise s;
> > +	u32 org_w, org_h;
> > +	unsigned int i;
> > +
> > +	fmt = mdp_find_fmt(pix_mp->pixelformat, f->type);
> > +	if (!fmt) {
> > +		fmt = mdp_find_fmt_by_index(0, f->type);
> > +		if (!fmt) {
> > +			dev_dbg(dev, "%d: pixelformat %c%c%c%c invalid", ctx_id,
> > +				(pix_mp->pixelformat & 0xff),
> > +				(pix_mp->pixelformat >>  8) & 0xff,
> > +				(pix_mp->pixelformat >> 16) & 0xff,
> > +				(pix_mp->pixelformat >> 24) & 0xff);
> > +			return NULL;
> > +		}
> > +	}
> > +
> > +	pix_mp->field = V4L2_FIELD_NONE;
> > +	pix_mp->flags = 0;
> > +	pix_mp->pixelformat = fmt->pixelformat;
> > +	if (!V4L2_TYPE_IS_OUTPUT(f->type)) {
> > +		pix_mp->colorspace = param->colorspace;
> > +		pix_mp->xfer_func = param->xfer_func;
> > +		pix_mp->ycbcr_enc = param->ycbcr_enc;
> > +		pix_mp->quantization = param->quant;
> > +	}
> > +
> > +	pix_limit = V4L2_TYPE_IS_OUTPUT(f->type) ? &param->limit->out_limit :
> > +						&param->limit->cap_limit;
> > +	s.min_width = pix_limit->wmin;
> > +	s.max_width = pix_limit->wmax;
> > +	s.step_width = fmt->walign;
> > +	s.min_height = pix_limit->hmin;
> > +	s.max_height = pix_limit->hmax;
> > +	s.step_height = fmt->halign;
> > +	org_w = pix_mp->width;
> > +	org_h = pix_mp->height;
> > +
> > +	mdp_bound_align_image(&pix_mp->width, &pix_mp->height, &s, fmt->salign);
> > +	if (org_w != pix_mp->width || org_h != pix_mp->height)
> > +		dev_dbg(dev, "%d: size change: %ux%u to %ux%u", ctx_id,
> > +			org_w, org_h, pix_mp->width, pix_mp->height);
> > +
> > +	if (pix_mp->num_planes && pix_mp->num_planes != fmt->num_planes)
> > +		dev_dbg(dev, "%d num of planes change: %u to %u", ctx_id,
> > +			pix_mp->num_planes, fmt->num_planes);
> > +	pix_mp->num_planes = fmt->num_planes;
> > +
> > +	for (i = 0; i < pix_mp->num_planes; ++i) {
> > +		u32 min_bpl = (pix_mp->width * fmt->row_depth[i]) >> 3;
> > +		u32 max_bpl = (pix_limit->wmax * fmt->row_depth[i]) >> 3;
> > +		u32 bpl = pix_mp->plane_fmt[i].bytesperline;
> > +		u32 min_si, max_si;
> > +		u32 si = pix_mp->plane_fmt[i].sizeimage;
> > +
> > +		bpl = clamp(bpl, min_bpl, max_bpl);
> > +		pix_mp->plane_fmt[i].bytesperline = bpl;
> > +
> > +		min_si = (bpl * pix_mp->height * fmt->depth[i]) / fmt->row_depth[i];
> > +		max_si = (bpl * s.max_height * fmt->depth[i]) / fmt->row_depth[i];
> > +
> > +		si = clamp(si, min_si, max_si);
> > +		pix_mp->plane_fmt[i].sizeimage = si;
> > +
> > +		dev_dbg(dev, "%d: p%u, bpl:%u [%u, %u], sizeimage:%u [%u, %u]", ctx_id,
> > +			i, bpl, min_bpl, max_bpl, si, min_si, max_si);
> > +	}
> > +
> > +	return fmt;
> > +}
> > +
> > +static int mdp_clamp_start(s32 *x, int min, int max, unsigned int align,
> > +			   u32 flags)
> > +{
> > +	if (flags & V4L2_SEL_FLAG_GE)
> > +		max = *x;
> > +	if (flags & V4L2_SEL_FLAG_LE)
> > +		min = *x;
> > +	return mdp_clamp_align(x, min, max, align);
> > +}
> > +
> > +static int mdp_clamp_end(s32 *x, int min, int max, unsigned int align,
> > +			 u32 flags)
> > +{
> > +	if (flags & V4L2_SEL_FLAG_GE)
> > +		min = *x;
> > +	if (flags & V4L2_SEL_FLAG_LE)
> > +		max = *x;
> > +	return mdp_clamp_align(x, min, max, align);
> > +}
> > +
> > +int mdp_try_crop(struct mdp_m2m_ctx *ctx, struct v4l2_rect *r,
> > +		 const struct v4l2_selection *s, struct mdp_frame *frame)
> > +{
> > +	struct device *dev = &ctx->mdp_dev->pdev->dev;
> > +	s32 left, top, right, bottom;
> > +	u32 framew, frameh, walign, halign;
> > +	int ret;
> > +
> > +	dev_dbg(dev, "%d target:%d, set:(%d,%d) %ux%u", ctx->id,
> > +		s->target, s->r.left, s->r.top, s->r.width, s->r.height);
> > +
> > +	left = s->r.left;
> > +	top = s->r.top;
> > +	right = s->r.left + s->r.width;
> > +	bottom = s->r.top + s->r.height;
> > +	framew = frame->format.fmt.pix_mp.width;
> > +	frameh = frame->format.fmt.pix_mp.height;
> > +
> > +	if (mdp_target_is_crop(s->target)) {
> > +		walign = 1;
> > +		halign = 1;
> > +	} else {
> > +		walign = frame->mdp_fmt->walign;
> > +		halign = frame->mdp_fmt->halign;
> > +	}
> > +
> > +	dev_dbg(dev, "%d align:%u,%u, bound:%ux%u", ctx->id,
> > +		walign, halign, framew, frameh);
> > +
> > +	ret = mdp_clamp_start(&left, 0, right, walign, s->flags);
> > +	if (ret)
> > +		return ret;
> > +	ret = mdp_clamp_start(&top, 0, bottom, halign, s->flags);
> > +	if (ret)
> > +		return ret;
> > +	ret = mdp_clamp_end(&right, left, framew, walign, s->flags);
> > +	if (ret)
> > +		return ret;
> > +	ret = mdp_clamp_end(&bottom, top, frameh, halign, s->flags);
> > +	if (ret)
> > +		return ret;
> > +
> > +	r->left = left;
> > +	r->top = top;
> > +	r->width = right - left;
> > +	r->height = bottom - top;
> > +
> > +	dev_dbg(dev, "%d crop:(%d,%d) %ux%u", ctx->id,
> > +		r->left, r->top, r->width, r->height);
> > +	return 0;
> > +}
> > +
> > +int mdp_check_scaling_ratio(const struct v4l2_rect *crop,
> > +			    const struct v4l2_rect *compose, s32 rotation,
> > +	const struct mdp_limit *limit)
> > +{
> > +	u32 crop_w, crop_h, comp_w, comp_h;
> > +
> > +	crop_w = crop->width;
> > +	crop_h = crop->height;
> > +	if (90 == rotation || 270 == rotation) {
> > +		comp_w = compose->height;
> > +		comp_h = compose->width;
> > +	} else {
> > +		comp_w = compose->width;
> > +		comp_h = compose->height;
> > +	}
> > +
> > +	if ((crop_w / comp_w) > limit->h_scale_down_max ||
> > +	    (crop_h / comp_h) > limit->v_scale_down_max ||
> > +	    (comp_w / crop_w) > limit->h_scale_up_max ||
> > +	    (comp_h / crop_h) > limit->v_scale_up_max)
> > +		return -ERANGE;
> > +	return 0;
> > +}
> > +
> > +/* Stride that is accepted by MDP HW */
> > +static u32 mdp_fmt_get_stride(const struct mdp_format *fmt,
> > +			      u32 bytesperline, unsigned int plane)
> > +{
> > +	enum mdp_color c = fmt->mdp_color;
> > +	u32 stride;
> > +
> > +	stride = (bytesperline * MDP_COLOR_BITS_PER_PIXEL(c))
> > +		/ fmt->row_depth[0];
> > +	if (plane == 0)
> > +		return stride;
> > +	if (plane < MDP_COLOR_GET_PLANE_COUNT(c)) {
> > +		if (MDP_COLOR_IS_BLOCK_MODE(c))
> > +			stride = stride / 2;
> > +		return stride;
> > +	}
> > +	return 0;
> > +}
> > +
> > +/* Stride that is accepted by MDP HW of format with contiguous planes */
> > +static u32 mdp_fmt_get_stride_contig(const struct mdp_format *fmt,
> > +				     u32 pix_stride, unsigned int plane)
> > +{
> > +	enum mdp_color c = fmt->mdp_color;
> > +	u32 stride = pix_stride;
> > +
> > +	if (plane == 0)
> > +		return stride;
> > +	if (plane < MDP_COLOR_GET_PLANE_COUNT(c)) {
> > +		stride = stride >> MDP_COLOR_GET_H_SUBSAMPLE(c);
> > +		if (MDP_COLOR_IS_UV_COPLANE(c) && !MDP_COLOR_IS_BLOCK_MODE(c))
> > +			stride = stride * 2;
> > +		return stride;
> > +	}
> > +	return 0;
> > +}
> > +
> > +/* Plane size that is accepted by MDP HW */
> > +static u32 mdp_fmt_get_plane_size(const struct mdp_format *fmt,
> > +				  u32 stride, u32 height, unsigned int plane)
> > +{
> > +	enum mdp_color c = fmt->mdp_color;
> > +	u32 bytesperline;
> > +
> > +	bytesperline = (stride * fmt->row_depth[0])
> > +		/ MDP_COLOR_BITS_PER_PIXEL(c);
> > +	if (plane == 0)
> > +		return bytesperline * height;
> > +	if (plane < MDP_COLOR_GET_PLANE_COUNT(c)) {
> > +		height = height >> MDP_COLOR_GET_V_SUBSAMPLE(c);
> > +		if (MDP_COLOR_IS_BLOCK_MODE(c))
> > +			bytesperline = bytesperline * 2;
> > +		return bytesperline * height;
> > +	}
> > +	return 0;
> > +}
> > +
> > +static void mdp_prepare_buffer(struct img_image_buffer *b,
> > +			       struct mdp_frame *frame, struct vb2_buffer *vb)
> > +{
> > +	struct v4l2_pix_format_mplane *pix_mp = &frame->format.fmt.pix_mp;
> > +	unsigned int i;
> > +
> > +	b->format.colorformat = frame->mdp_fmt->mdp_color;
> > +	b->format.ycbcr_prof = frame->ycbcr_prof;
> > +	for (i = 0; i < pix_mp->num_planes; ++i) {
> > +		u32 stride = mdp_fmt_get_stride(frame->mdp_fmt,
> > +			pix_mp->plane_fmt[i].bytesperline, i);
> > +
> > +		b->format.plane_fmt[i].stride = stride;
> > +		/*
> > +		 * TODO : The way to pass an offset within a DMA-buf
> > +		 * is not defined in V4L2 specification, so we abuse
> > +		 * data_offset for now. Fix it when we have the right interface,
> > +		 * including any necessary validation and potential alignment
> > +		 * issues.
> > +		 */
> > +		b->format.plane_fmt[i].size =
> > +			mdp_fmt_get_plane_size(frame->mdp_fmt, stride,
> > +					       pix_mp->height, i) -
> > +					       vb->planes[i].data_offset;
> > +		b->iova[i] = vb2_dma_contig_plane_dma_addr(vb, i) +
> > +			     vb->planes[i].data_offset;
> 
> Why do you need data_offset? What is the use case? Obviously I can't
> merge a driver that abuses an API.
> 
> > +	}
> > +	for (; i < MDP_COLOR_GET_PLANE_COUNT(b->format.colorformat); ++i) {
> > +		u32 stride = mdp_fmt_get_stride_contig(frame->mdp_fmt,
> > +			b->format.plane_fmt[0].stride, i);
> > +
> > +		b->format.plane_fmt[i].stride = stride;
> > +		b->format.plane_fmt[i].size =
> > +			mdp_fmt_get_plane_size(frame->mdp_fmt, stride,
> > +					       pix_mp->height, i);
> > +		b->iova[i] = b->iova[i - 1] + b->format.plane_fmt[i - 1].size;
> > +	}
> > +	b->usage = frame->usage;
> > +}
> > +
> > +void mdp_set_src_config(struct img_input *in,
> > +			struct mdp_frame *frame, struct vb2_buffer *vb)
> > +{
> > +	in->buffer.format.width = frame->format.fmt.pix_mp.width;
> > +	in->buffer.format.height = frame->format.fmt.pix_mp.height;
> > +	mdp_prepare_buffer(&in->buffer, frame, vb);
> > +}
> > +
> > +static u32 mdp_to_fixed(u32 *r, struct v4l2_fract *f)
> > +{
> > +	u32 q;
> > +
> > +	if (f->denominator == 0) {
> > +		*r = 0;
> > +		return 0;
> > +	}
> > +
> > +	q = f->numerator / f->denominator;
> > +	*r = div_u64(((u64)f->numerator - q * f->denominator) <<
> > +		     IMG_SUBPIXEL_SHIFT, f->denominator);
> > +	return q;
> > +}
> > +
> > +static void mdp_set_src_crop(struct img_crop *c, struct mdp_crop *crop)
> > +{
> > +	c->left = crop->c.left
> > +		+ mdp_to_fixed(&c->left_subpix, &crop->left_subpix);
> > +	c->top = crop->c.top
> > +		+ mdp_to_fixed(&c->top_subpix, &crop->top_subpix);
> > +	c->width = crop->c.width
> > +		+ mdp_to_fixed(&c->width_subpix, &crop->width_subpix);
> > +	c->height = crop->c.height
> > +		+ mdp_to_fixed(&c->height_subpix, &crop->height_subpix);
> > +}
> > +
> > +static void mdp_set_orientation(struct img_output *out,
> > +				s32 rotation, bool hflip, bool vflip)
> > +{
> > +	u8 flip = 0;
> > +
> > +	if (hflip)
> > +		flip ^= 1;
> > +	if (vflip) {
> > +		/*
> > +		 * A vertical flip is equivalent to
> > +		 * a 180-degree rotation with a horizontal flip
> > +		 */
> > +		rotation += 180;
> > +		flip ^= 1;
> > +	}
> > +
> > +	out->rotation = rotation % 360;
> > +	if (flip != 0)
> > +		out->flags |= IMG_CTRL_FLAG_HFLIP;
> > +	else
> > +		out->flags &= ~IMG_CTRL_FLAG_HFLIP;
> > +}
> > +
> > +void mdp_set_dst_config(struct img_output *out,
> > +			struct mdp_frame *frame, struct vb2_buffer *vb)
> > +{
> > +	out->buffer.format.width = frame->compose.width;
> > +	out->buffer.format.height = frame->compose.height;
> > +	mdp_prepare_buffer(&out->buffer, frame, vb);
> > +	mdp_set_src_crop(&out->crop, &frame->crop);
> > +	mdp_set_orientation(out, frame->rotation, frame->hflip, frame->vflip);
> > +}
> > +
> > +int mdp_frameparam_init(struct mdp_frameparam *param)
> > +{
> > +	struct mdp_frame *frame;
> > +
> > +	if (!param)
> > +		return -EINVAL;
> > +
> > +	INIT_LIST_HEAD(&param->list);
> > +	param->limit = &mdp_def_limit;
> > +	param->type = MDP_STREAM_TYPE_BITBLT;
> > +
> > +	frame = &param->output;
> > +	frame->format.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
> > +	frame->mdp_fmt = mdp_try_fmt_mplane(&frame->format, param, 0);
> > +	frame->ycbcr_prof =
> > +		mdp_map_ycbcr_prof_mplane(&frame->format,
> > +					  frame->mdp_fmt->mdp_color);
> > +	frame->usage = MDP_BUFFER_USAGE_HW_READ;
> > +
> > +	param->num_captures = 1;
> > +	frame = &param->captures[0];
> > +	frame->format.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
> > +	frame->mdp_fmt = mdp_try_fmt_mplane(&frame->format, param, 0);
> > +	frame->ycbcr_prof =
> > +		mdp_map_ycbcr_prof_mplane(&frame->format,
> > +					  frame->mdp_fmt->mdp_color);
> > +	frame->usage = MDP_BUFFER_USAGE_MDP;
> > +	frame->crop.c.width = param->output.format.fmt.pix_mp.width;
> > +	frame->crop.c.height = param->output.format.fmt.pix_mp.height;
> > +	frame->compose.width = frame->format.fmt.pix_mp.width;
> > +	frame->compose.height = frame->format.fmt.pix_mp.height;
> > +
> > +	return 0;
> > +}

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [DKIM] [PATCH v21 4/4] media: platform: mtk-mdp3: add Mediatek MDP3 driver

[moudy.ho]

Hi Hans,

Thanks for your review, and appreciate for all comments and
suggestions.
I'll go through each "dev_info" one by one and replace the unnecessary
with "dev_dbg" or just remove it.
In addition, I will adjust other inappropriate settings as suggested.

On Fri, 2022-07-08 at 10:08 +0200, Hans Verkuil wrote:
> Hi Moudy,
> 
> Some comments below:
> 
> On 7/6/22 09:54, Moudy Ho wrote:
> > This patch adds driver for Mediatek's Media Data Path ver.3 (MDP3).
> > It provides the following functions:
> >   color transform, format conversion, resize, crop, rotate, flip
> >   and additional image quality enhancement.
> > 
> > The MDP3 driver is mainly used for Google Chromebook products to
> > import the new architecture to set the HW settings as shown below:
> >   User -> V4L2 framework
> >     -> MDP3 driver -> SCP (setting calculations)
> >       -> MDP3 driver -> CMDQ (GCE driver) -> HW
> > 
> > Each modules' related operation control is sited in mtk-mdp3-comp.c
> > Each modules' register table is defined in file with "mdp_reg_"
> > prefix
> > GCE related API, operation control  sited in mtk-mdp3-cmdq.c
> > V4L2 m2m device functions are implemented in mtk-mdp3-m2m.c
> > Probe, power, suspend/resume, system level functions are defined in
> > mtk-mdp3-core.c
> > 
> > v4l2-compliance 1.22.1, 32 bits, 32-bit time_t
> > 
> > Compliance test for mtk-mdp3 device /dev/video2:
> > 
> > Driver Info:
> > 	Driver name      : mtk-mdp3
> > 	Card type        : 14001000.mdp3-rdma0
> 
> Card type is expected to be a human readable name, and that's not the
> case
> here.
> 
> > 	Bus info         : platform:mtk-mdp3
> 
> <snip>

[snip]

> > diff --git a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-regs.c
> > b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-regs.c
> > new file mode 100644
> > index 000000000000..18874eb7851c
> > --- /dev/null
> > +++ b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-regs.c

[snip]

> > +/* Plane size that is accepted by MDP HW */
> > +static u32 mdp_fmt_get_plane_size(const struct mdp_format *fmt,
> > +				  u32 stride, u32 height, unsigned int
> > plane)
> > +{
> > +	enum mdp_color c = fmt->mdp_color;
> > +	u32 bytesperline;
> > +
> > +	bytesperline = (stride * fmt->row_depth[0])
> > +		/ MDP_COLOR_BITS_PER_PIXEL(c);
> > +	if (plane == 0)
> > +		return bytesperline * height;
> > +	if (plane < MDP_COLOR_GET_PLANE_COUNT(c)) {
> > +		height = height >> MDP_COLOR_GET_V_SUBSAMPLE(c);
> > +		if (MDP_COLOR_IS_BLOCK_MODE(c))
> > +			bytesperline = bytesperline * 2;
> > +		return bytesperline * height;
> > +	}
> > +	return 0;
> > +}
> > +
> > +static void mdp_prepare_buffer(struct img_image_buffer *b,
> > +			       struct mdp_frame *frame, struct
> > vb2_buffer *vb)
> > +{
> > +	struct v4l2_pix_format_mplane *pix_mp = &frame-
> > >format.fmt.pix_mp;
> > +	unsigned int i;
> > +
> > +	b->format.colorformat = frame->mdp_fmt->mdp_color;
> > +	b->format.ycbcr_prof = frame->ycbcr_prof;
> > +	for (i = 0; i < pix_mp->num_planes; ++i) {
> > +		u32 stride = mdp_fmt_get_stride(frame->mdp_fmt,
> > +			pix_mp->plane_fmt[i].bytesperline, i);
> > +
> > +		b->format.plane_fmt[i].stride = stride;
> > +		/*
> > +		 * TODO : The way to pass an offset within a DMA-buf
> > +		 * is not defined in V4L2 specification, so we abuse
> > +		 * data_offset for now. Fix it when we have the right
> > interface,
> > +		 * including any necessary validation and potential
> > alignment
> > +		 * issues.
> > +		 */
> > +		b->format.plane_fmt[i].size =
> > +			mdp_fmt_get_plane_size(frame->mdp_fmt, stride,
> > +					       pix_mp->height, i) -
> > +					       vb-
> > >planes[i].data_offset;
> > +		b->iova[i] = vb2_dma_contig_plane_dma_addr(vb, i) +
> > +			     vb->planes[i].data_offset;
> 
> Why do you need data_offset? What is the use case? Obviously I can't
> merge a driver that abuses an API.
> 

Apologize for not incorporating the previously discussed results into
this version due to my carelessness, I will correct it in the next
version.

https://patchwork.kernel.org/project/linux-mediatek/patch/20210623091457.18002-1-moudy.ho@mediatek.com/

> > +	}
> > +	for (; i < MDP_COLOR_GET_PLANE_COUNT(b->format.colorformat);
> > ++i) {
> > +		u32 stride = mdp_fmt_get_stride_contig(frame->mdp_fmt,
> > +			b->format.plane_fmt[0].stride, i);
> > +
> > +		b->format.plane_fmt[i].stride = stride;
> > +		b->format.plane_fmt[i].size =
> > +			mdp_fmt_get_plane_size(frame->mdp_fmt, stride,
> > +					       pix_mp->height, i);
> > +		b->iova[i] = b->iova[i - 1] + b->format.plane_fmt[i -
> > 1].size;
> > +	}
> > +	b->usage = frame->usage;
> > +}
> > +
> > +void mdp_set_src_config(struct img_input *in,
> > +			struct mdp_frame *frame, struct vb2_buffer *vb)
> > +{
> > +	in->buffer.format.width = frame->format.fmt.pix_mp.width;
> > +	in->buffer.format.height = frame->format.fmt.pix_mp.height;
> > +	mdp_prepare_buffer(&in->buffer, frame, vb);
> > +}

[snip]

> 
> Regards,
> 
> 	Hans

Many thanks,
Moudy


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [DKIM] [PATCH v21 4/4] media: platform: mtk-mdp3: add Mediatek MDP3 driver

[moudy.ho]

On Fri, 2022-07-08 at 11:20 +0300, Laurent Pinchart wrote:
> On Fri, Jul 08, 2022 at 10:08:44AM +0200, Hans Verkuil wrote:
> > Hi Moudy,
> > 
> > Some comments below:
> 
> And one more
> 
> > On 7/6/22 09:54, Moudy Ho wrote:
> > > This patch adds driver for Mediatek's Media Data Path ver.3
> > > (MDP3).
> > > It provides the following functions:
> > >   color transform, format conversion, resize, crop, rotate, flip
> > >   and additional image quality enhancement.
> > > 
> > > The MDP3 driver is mainly used for Google Chromebook products to
> > > import the new architecture to set the HW settings as shown
> > > below:
> > >   User -> V4L2 framework
> > >     -> MDP3 driver -> SCP (setting calculations)
> > >       -> MDP3 driver -> CMDQ (GCE driver) -> HW
> > > 
> > > Each modules' related operation control is sited in mtk-mdp3-
> > > comp.c
> > > Each modules' register table is defined in file with "mdp_reg_"
> > > prefix
> > > GCE related API, operation control  sited in mtk-mdp3-cmdq.c
> > > V4L2 m2m device functions are implemented in mtk-mdp3-m2m.c
> > > Probe, power, suspend/resume, system level functions are defined
> > > in
> > > mtk-mdp3-core.c
> > > 
> > > v4l2-compliance 1.22.1, 32 bits, 32-bit time_t
> > > 
> > > Compliance test for mtk-mdp3 device /dev/video2:
> > > 
> > > Driver Info:
> > > 	Driver name      : mtk-mdp3
> > > 	Card type        : 14001000.mdp3-rdma0
> > 
> > Card type is expected to be a human readable name, and that's not
> > the case
> > here.
> > 
> > > 	Bus info         : platform:mtk-mdp3
> > 
> > <snip>
> > 
> > > diff --git a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c
> > > b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-m2m.c
> > > new file mode 100644

[snip]

> > > +static const struct vb2_ops mdp_m2m_qops = {
> > > +	.queue_setup	= mdp_m2m_queue_setup,
> > > +	.wait_prepare	= vb2_ops_wait_prepare,
> > > +	.wait_finish	= vb2_ops_wait_finish,
> > > +	.buf_prepare	= mdp_m2m_buf_prepare,
> > > +	.start_streaming = mdp_m2m_start_streaming,
> > > +	.stop_streaming	= mdp_m2m_stop_streaming,
> > > +	.buf_queue	= mdp_m2m_buf_queue,
> > > +	.buf_out_validate = mdp_m2m_buf_out_validate,
> > > +};
> > > +
> > > +static int mdp_m2m_querycap(struct file *file, void *fh,
> > > +			    struct v4l2_capability *cap)
> > > +{
> > > +	struct mdp_m2m_ctx *ctx = fh_to_ctx(fh);
> > > +
> > > +	strscpy(cap->driver, MDP_MODULE_NAME, sizeof(cap->driver));
> > > +	strscpy(cap->card, ctx->mdp_dev->pdev->name, sizeof(cap-
> > > >card));
> > 
> > As mentioned at the top, this is not a suitable name for cap->card.
> > 
> > > +	strscpy(cap->bus_info, "platform:mtk-mdp3", sizeof(cap-
> > > >bus_info));
> 
> And don't override bus_info, the value isn't right. The V4L2 core
> should
> do the right thing for platform devices now.
> 

Hi Laurent,

Thanks for the reminder, I'll fix it in the next version along with
what Hans mentioned earlier.

Regards,
Moudy

> > > +	return 0;
> > > +}
> > > +
> > > +static int mdp_m2m_enum_fmt_mplane(struct file *file, void *fh,
> > > +				   struct v4l2_fmtdesc *f)
> > > +{
> > > +	return mdp_enum_fmt_mplane(f);
> > > +}


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel