Re: [PATCH v29 0/7] Add MediaTek SoC DRM (vdosys1) support for mt8195

Re: [PATCH v29 0/7] Add MediaTek SoC DRM (vdosys1) support for mt8195

[AngeloGioacchino Del Regno]

Il 27/12/22 09:10, Nancy.Lin ha scritto:
> The hardware path of vdosys1 with DPTx output need to go through by several modules, such as, OVL_ADAPTOR and MERGE.
> 
> Add DRM and these modules support by the patches below:
> 

Hello Chun-Kuang,

This series reached version 29 and was tested for a long time.
Is there anything else to fix in here, or can it be finally picked?

Its soc/mediatek counterpart was already picked by Matthias.

Thanks,
Angelo

> Changes in v29:
> - rebase to next-20221226
> - fix reviewer comment in v28
>    - keep original flow if comp node not found in mtk_drm_crtc_create()
> 
> Changes in v28:
> - rebase to next-20221107
> - fix reviewer comment in v27
>    - extra new line at the end mtk_ethdr.h
> 
> Changes in v27:
> - rebase to next-20221102
> - change mmsys compatible for mt8195 vdosys1
>    - base on jason's series[ref 1]
> - fix reviewer comment
>    - add error return code if no ovl_adaptor's comp found
> 
> Changes in v26:
> - rebase to next-20220819
> - resend for patch corrupted in v25
> 
> Changes in v25:
> - rebase to next-20220803
> 
> Changes in v24:
> - fix ovl_adaptor binding issue (mtk_disp_ovl_adaptor.c)
>    - Since ovl_adaptor is an aggregated component, it should be bounded after
>      all its child components are bounded.
> - rebase to next-20220708
> 
> Changes in v23:
> - separate[7] mmsys/mutex and drm patches into two series
> 
> Changes in v22:
> - rebase to next-20220525
> - rebase to vdosys0 series v22
> - separate dts to a new patch
> 
> Changes in v21:
> - fix reviewer comment
>    - fix rdma and ethdr binding doc and dts
> 
> Changes in v20:
> - fix reviewer comment
>    - update mmsys update bit api name
>    - add mtk_mmsys_update_bits error message if lose gce property
>    - list all mt8195 vdosys1 reset bits
> 
> Changes in v19:
> - fix reviewer comment
>    - separate mt8195 mmsys component to a new patch
>    - separate mt8195 vdo0 and vdo1 routing table
>    - separate mmsys_write_reg api to a new patch and simplify write reg code
>    - separate mmsys 64 bit reset to a new patch
>    - separate mtk-mutex dp_intf1 component to a new patch
> 
> Changes in v18:
> - fix reviewer comment
>    - fix rdma binding doc
>    - fix ethdr binding doc
>    - refine mmsys config cmdq support
>    - refine merge reset control flow, get reset control in probe function
>    - add ethdr reset control error handling and remove dbg log
> - rebase to vdosys0 series v20 (ref [5])
> 
> Changes in v17:
> - fix reviewer comment in v16
>    - separate ovl adaptor comp in mtk-mmsys and mtk-mutex
>    - separate mmsys config API
>    - move mdp_rdma binding yaml
> - fix ovl adaptor pm runtime get sync timing issue
> - rebase to vdosys0 series v19 (ref [5])
> - rebase to [7] for modify vblank register change
> 
> Changes in v16:
> - fix reviewer comment in v 15
>    - fix mtk_drm_ddp_comp.c alignment
>    - fix vdosys0 mmsys num before adding vdosys1 patch
> 
> Changes in v15:
> - fix ethdr uppercase hex number in dts
> 
> Changes in v14:
> - remove MTK_MMSYS 64 bit dependency
> - add ethdr.yaml back and fix dt_schema check fail
> 
> Resend v13
> - add related maintainer in maillist
> 
> Changes in v13:
> - fix reviewer comment in v12
>    - fix rdma dt-binding format
>    - fix dts node naming
> - fix 32 bit build error
>    - modify 64bit dependency for mtk-mmsys
> - rebase to vdosys0 series v16. (ref [5])
> 
> Changes in v12:
> - fix reviewer comment in v11
>    - modify mbox index
>    - refine dma dev for ovl_adaptor sub driver
> 
> Changes in v11:
> - remove ethdr vblank spin lock
> - refine ovl_adaptor print message
> 
> Changes in v10:
> - refine ethdr reset control using devm_reset_control_array_get_optional_exclusive
> - fix ovl_adaptor mtk_ovl_adaptor_clk_enable error handle issue
> 
> Changes in v9:
> - rebase on kernel-5.16-rc1
> - rebase on vdosys0 series v13. (ref [5])
> - fix ovl_adaptor sub driver is brought up unintentionally
> - fix clang build test fail- duplicate ethdr/mdp_rdma init_module/cleanup_module symbol issue
> 
> Changes in v8:
> - separate merge async reset to new patch.
> - separate drm ovl_adaptor sub driver to new patch.
> - fix reviewer comment in v7.
> 
> Changes in v7:
> - rebase on vdosys0 series v12 (ref[5])
> - add dma description in ethdr binding document.
> - refine vdosys1 bit definition of mmsys routing table.
> - separate merge modification into 3 pathces.
> - separate mutex modification into 2 patches.
> - add plane color coding for mdp_rdma csc.
> - move mdp_rdma pm control to ovl_adaptor.
> - fix reviewer comment in v6.
> 
> Changes in v6:
> - rebase on kernel-5.15-rc1.
> - change mbox label to gce0 for dts node of vdosys1.
> - modify mmsys reset num for mt8195.
> - rebase on vdosys0 series v10. (ref [5])
> - use drm to bring up ovl_adaptor driver.
> - move drm iommu/mutex check from kms init to drm bind.
> - modify rdma binding doc location. (Documentation/devicetree/bindings/arm/)
> - modify for reviewer's comment in v5.
> 
> Changes in v5:
> - add mmsys reset controller reference.
> 
> Changes in v4:
> - use merge common driver for merge1~4.
> - refine ovl_adaptor rdma driver.
> - use ovl_adaptor ddp_comp function instead of ethdr.
> - modify for reviewer's comment in v3.
> 
> Changes in v3:
> - modify for reviewer's comment in v2.
> - add vdosys1 2 pixels align limit.
> - add mixer odd offset support.
> 
> Changes in v2:
> - Merge PSEUDO_OVL and ETHDR into one DRM component.
> - Add mmsys config API for vdosys1 hardware setting.
> - Add mmsys reset control using linux reset framework.
> 
> Signed-off-by: Nancy.Lin <nancy.lin@mediatek.com>
> 
> This series are based on the following patch:
> [1] Change mmsys compatible for mt8195 mediatek-drm
>      20221126101220.18179-1-jason-jh.lin@mediatek.com
> [2] Add MediaTek SoC(vdosys1) support for mt8195
>      20221103032512.9144-1-nancy.lin@mediatek.com
> 
> Nancy.Lin (7):
>    dt-bindings: mediatek: add ethdr definition for mt8195
>    drm/mediatek: add ETHDR support for MT8195
>    drm/mediatek: add ovl_adaptor support for MT8195
>    drm/mediatek: add dma dev get function
>    drm/mediatek: modify mediatek-drm for mt8195 multi mmsys support
>    drm/mediatek: add drm ovl_adaptor sub driver for MT8195
>    drm/mediatek: add mediatek-drm of vdosys1 support for MT8195
> 
>   .../display/mediatek/mediatek,ethdr.yaml      | 188 ++++++
>   drivers/gpu/drm/mediatek/Makefile             |   2 +
>   drivers/gpu/drm/mediatek/mtk_disp_drv.h       |  26 +
>   .../gpu/drm/mediatek/mtk_disp_ovl_adaptor.c   | 533 ++++++++++++++++++
>   drivers/gpu/drm/mediatek/mtk_drm_crtc.c       |  85 ++-
>   drivers/gpu/drm/mediatek/mtk_drm_crtc.h       |   6 +-
>   drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c   | 129 +++--
>   drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h   |  58 +-
>   drivers/gpu/drm/mediatek/mtk_drm_drv.c        | 366 ++++++++----
>   drivers/gpu/drm/mediatek/mtk_drm_drv.h        |  24 +-
>   drivers/gpu/drm/mediatek/mtk_ethdr.c          | 370 ++++++++++++
>   drivers/gpu/drm/mediatek/mtk_ethdr.h          |  25 +
>   12 files changed, 1624 insertions(+), 188 deletions(-)
>   create mode 100644 Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.yaml
>   create mode 100644 drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
>   create mode 100644 drivers/gpu/drm/mediatek/mtk_ethdr.c
>   create mode 100644 drivers/gpu/drm/mediatek/mtk_ethdr.h
> 

Re: [PATCH v29 3/7] drm/mediatek: add ovl_adaptor support for MT8195

[Guillaume Ranquet]

On Tue, 27 Dec 2022 09:10, "" wrote:

Hi Nancy.

I've been using your patches lately to test out the HDMI series on
mt8195 and I have hit a scheduling bug.

>Add ovl_adaptor driver for MT8195.
>Ovl_adaptor is an encapsulated module and designed for simplified
>DRM control flow. This module is composed of 8 RDMAs, 4 MERGEs and
>an ETHDR. Two RDMAs merge into one layer, so this module support 4
>layers.
>
>Signed-off-by: Nancy.Lin <nancy.lin@mediatek.com>
>Reviewed-by: Chun-Kuang Hu <chunkuang.hu@kernel.org>
>Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>Tested-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>Tested-by: Bo-Chen Chen <rex-bc.chen@mediatek.com>
>---
> drivers/gpu/drm/mediatek/Makefile             |   1 +
> drivers/gpu/drm/mediatek/mtk_disp_drv.h       |  26 +
> .../gpu/drm/mediatek/mtk_disp_ovl_adaptor.c   | 533 ++++++++++++++++++
> drivers/gpu/drm/mediatek/mtk_drm_drv.h        |   1 +
> 4 files changed, 561 insertions(+)
> create mode 100644 drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
>
>diff --git a/drivers/gpu/drm/mediatek/Makefile b/drivers/gpu/drm/mediatek/Makefile
>index 840f14436d3c..d4d193f60271 100644
>--- a/drivers/gpu/drm/mediatek/Makefile
>+++ b/drivers/gpu/drm/mediatek/Makefile
>@@ -6,6 +6,7 @@ mediatek-drm-y := mtk_disp_aal.o \
> 		  mtk_disp_gamma.o \
> 		  mtk_disp_merge.o \
> 		  mtk_disp_ovl.o \
>+		  mtk_disp_ovl_adaptor.o \
> 		  mtk_disp_rdma.o \
> 		  mtk_drm_crtc.o \
> 		  mtk_drm_ddp_comp.o \
>diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
>index 33e61a136bbc..654f8e257984 100644
>--- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
>+++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
>@@ -7,6 +7,8 @@
> #define _MTK_DISP_DRV_H_
>
> #include <linux/soc/mediatek/mtk-cmdq.h>
>+#include <linux/soc/mediatek/mtk-mmsys.h>
>+#include <linux/soc/mediatek/mtk-mutex.h>
> #include "mtk_drm_plane.h"
> #include "mtk_mdp_rdma.h"
>
>@@ -116,6 +118,30 @@ void mtk_rdma_unregister_vblank_cb(struct device *dev);
> void mtk_rdma_enable_vblank(struct device *dev);
> void mtk_rdma_disable_vblank(struct device *dev);
>
>+void mtk_ovl_adaptor_add_comp(struct device *dev, struct mtk_mutex *mutex);
>+void mtk_ovl_adaptor_remove_comp(struct device *dev, struct mtk_mutex *mutex);
>+void mtk_ovl_adaptor_connect(struct device *dev, struct device *mmsys_dev,
>+			     unsigned int next);
>+void mtk_ovl_adaptor_disconnect(struct device *dev, struct device *mmsys_dev,
>+				unsigned int next);
>+int mtk_ovl_adaptor_clk_enable(struct device *dev);
>+void mtk_ovl_adaptor_clk_disable(struct device *dev);
>+void mtk_ovl_adaptor_config(struct device *dev, unsigned int w,
>+			    unsigned int h, unsigned int vrefresh,
>+			    unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
>+void mtk_ovl_adaptor_layer_config(struct device *dev, unsigned int idx,
>+				  struct mtk_plane_state *state,
>+				  struct cmdq_pkt *cmdq_pkt);
>+void mtk_ovl_adaptor_register_vblank_cb(struct device *dev, void (*vblank_cb)(void *),
>+					void *vblank_cb_data);
>+void mtk_ovl_adaptor_unregister_vblank_cb(struct device *dev);
>+void mtk_ovl_adaptor_enable_vblank(struct device *dev);
>+void mtk_ovl_adaptor_disable_vblank(struct device *dev);
>+void mtk_ovl_adaptor_start(struct device *dev);
>+void mtk_ovl_adaptor_stop(struct device *dev);
>+unsigned int mtk_ovl_adaptor_layer_nr(struct device *dev);
>+struct device *mtk_ovl_adaptor_dma_dev_get(struct device *dev);
>+
> int mtk_mdp_rdma_clk_enable(struct device *dev);
> void mtk_mdp_rdma_clk_disable(struct device *dev);
> void mtk_mdp_rdma_start(struct device *dev, struct cmdq_pkt *cmdq_pkt);
>diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
>new file mode 100644
>index 000000000000..046217828ab3
>--- /dev/null
>+++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
>@@ -0,0 +1,533 @@
>+// SPDX-License-Identifier: GPL-2.0-only
>+/*
>+ * Copyright (c) 2021 MediaTek Inc.
>+ */
>+
>+#include <drm/drm_fourcc.h>
>+#include <drm/drm_of.h>
>+#include <linux/clk.h>
>+#include <linux/component.h>
>+#include <linux/of_device.h>
>+#include <linux/of_address.h>
>+#include <linux/platform_device.h>
>+#include <linux/pm_runtime.h>
>+#include <linux/reset.h>
>+#include <linux/soc/mediatek/mtk-cmdq.h>
>+#include <linux/soc/mediatek/mtk-mmsys.h>
>+#include <linux/soc/mediatek/mtk-mutex.h>
>+
>+#include "mtk_disp_drv.h"
>+#include "mtk_drm_crtc.h"
>+#include "mtk_drm_ddp_comp.h"
>+#include "mtk_drm_drv.h"
>+#include "mtk_ethdr.h"
>+
>+#define MTK_OVL_ADAPTOR_RDMA_MAX_WIDTH 1920
>+#define MTK_OVL_ADAPTOR_LAYER_NUM 4
>+
>+enum mtk_ovl_adaptor_comp_type {
>+	OVL_ADAPTOR_TYPE_RDMA = 0,
>+	OVL_ADAPTOR_TYPE_MERGE,
>+	OVL_ADAPTOR_TYPE_ETHDR,
>+	OVL_ADAPTOR_TYPE_NUM,
>+};
>+
>+enum mtk_ovl_adaptor_comp_id {
>+	OVL_ADAPTOR_MDP_RDMA0,
>+	OVL_ADAPTOR_MDP_RDMA1,
>+	OVL_ADAPTOR_MDP_RDMA2,
>+	OVL_ADAPTOR_MDP_RDMA3,
>+	OVL_ADAPTOR_MDP_RDMA4,
>+	OVL_ADAPTOR_MDP_RDMA5,
>+	OVL_ADAPTOR_MDP_RDMA6,
>+	OVL_ADAPTOR_MDP_RDMA7,
>+	OVL_ADAPTOR_MERGE0,
>+	OVL_ADAPTOR_MERGE1,
>+	OVL_ADAPTOR_MERGE2,
>+	OVL_ADAPTOR_MERGE3,
>+	OVL_ADAPTOR_ETHDR0,
>+	OVL_ADAPTOR_ID_MAX
>+};
>+
>+struct ovl_adaptor_comp_match {
>+	enum mtk_ovl_adaptor_comp_type type;
>+	int alias_id;
>+};
>+
>+struct mtk_disp_ovl_adaptor {
>+	struct device *ovl_adaptor_comp[OVL_ADAPTOR_ID_MAX];
>+	struct device *mmsys_dev;
>+	bool children_bound;
>+};
>+
>+static const char * const private_comp_stem[OVL_ADAPTOR_TYPE_NUM] = {
>+	[OVL_ADAPTOR_TYPE_RDMA]		= "vdo1-rdma",
>+	[OVL_ADAPTOR_TYPE_MERGE]	= "merge",
>+	[OVL_ADAPTOR_TYPE_ETHDR]	= "ethdr",
>+};
>+
>+static const struct ovl_adaptor_comp_match comp_matches[OVL_ADAPTOR_ID_MAX] = {
>+	[OVL_ADAPTOR_MDP_RDMA0]	= { OVL_ADAPTOR_TYPE_RDMA, 0 },
>+	[OVL_ADAPTOR_MDP_RDMA1]	= { OVL_ADAPTOR_TYPE_RDMA, 1 },
>+	[OVL_ADAPTOR_MDP_RDMA2]	= { OVL_ADAPTOR_TYPE_RDMA, 2 },
>+	[OVL_ADAPTOR_MDP_RDMA3]	= { OVL_ADAPTOR_TYPE_RDMA, 3 },
>+	[OVL_ADAPTOR_MDP_RDMA4]	= { OVL_ADAPTOR_TYPE_RDMA, 4 },
>+	[OVL_ADAPTOR_MDP_RDMA5]	= { OVL_ADAPTOR_TYPE_RDMA, 5 },
>+	[OVL_ADAPTOR_MDP_RDMA6]	= { OVL_ADAPTOR_TYPE_RDMA, 6 },
>+	[OVL_ADAPTOR_MDP_RDMA7]	= { OVL_ADAPTOR_TYPE_RDMA, 7 },
>+	[OVL_ADAPTOR_MERGE0]	= { OVL_ADAPTOR_TYPE_MERGE, 1 },
>+	[OVL_ADAPTOR_MERGE1]	= { OVL_ADAPTOR_TYPE_MERGE, 2 },
>+	[OVL_ADAPTOR_MERGE2]	= { OVL_ADAPTOR_TYPE_MERGE, 3 },
>+	[OVL_ADAPTOR_MERGE3]	= { OVL_ADAPTOR_TYPE_MERGE, 4 },
>+	[OVL_ADAPTOR_ETHDR0]	= { OVL_ADAPTOR_TYPE_ETHDR, 0 },
>+};
>+
>+void mtk_ovl_adaptor_layer_config(struct device *dev, unsigned int idx,
>+				  struct mtk_plane_state *state,
>+				  struct cmdq_pkt *cmdq_pkt)
>+{
>+	struct mtk_disp_ovl_adaptor *ovl_adaptor = dev_get_drvdata(dev);
>+	struct mtk_plane_pending_state *pending = &state->pending;
>+	struct mtk_mdp_rdma_cfg rdma_config = {0};
>+	struct device *rdma_l;
>+	struct device *rdma_r;
>+	struct device *merge;
>+	struct device *ethdr;
>+	const struct drm_format_info *fmt_info = drm_format_info(pending->format);
>+	bool use_dual_pipe = false;
>+	unsigned int align_width;
>+	unsigned int l_w = 0;
>+	unsigned int r_w = 0;
>+
>+	dev_dbg(dev, "%s+ idx:%d, enable:%d, fmt:0x%x\n", __func__, idx,
>+		pending->enable, pending->format);
>+	dev_dbg(dev, "addr 0x%pad, fb w:%d, {%d,%d,%d,%d}\n",
>+		&pending->addr, (pending->pitch / fmt_info->cpp[0]),
>+		pending->x, pending->y, pending->width, pending->height);
>+
>+	rdma_l = ovl_adaptor->ovl_adaptor_comp[OVL_ADAPTOR_MDP_RDMA0 + 2 * idx];
>+	rdma_r = ovl_adaptor->ovl_adaptor_comp[OVL_ADAPTOR_MDP_RDMA0 + 2 * idx + 1];
>+	merge = ovl_adaptor->ovl_adaptor_comp[OVL_ADAPTOR_MERGE0 + idx];
>+	ethdr = ovl_adaptor->ovl_adaptor_comp[OVL_ADAPTOR_ETHDR0];
>+
>+	if (!pending->enable) {
>+		mtk_merge_stop_cmdq(merge, cmdq_pkt);

It doesn't seem to be safe here to call mtk_merge_stop_cmdq() as shown
with this stack trace on i1200-demo:

 BUG: scheduling while atomic: swapper/0/0/0x00010002
 Modules linked in:
 CPU: 0 PID: 0 Comm: swapper/0 Not tainted
6.2.0-rc7-next-20230213-mtk-00029-g9bf9f23daebe-dirty #118
 Hardware name: MediaTek MT8195 demo board (DT)
 Call trace:
  dump_backtrace+0x98/0xf4
  show_stack+0x18/0x24
  dump_stack_lvl+0x48/0x60
  dump_stack+0x18/0x24
  __schedule_bug+0x50/0x68
  __schedule+0x74c/0x80c
  schedule+0x5c/0xc4
  schedule_hrtimeout_range_clock+0xb0/0x150
  schedule_hrtimeout_range+0x14/0x20
  usleep_range_state+0x74/0xa8
  mtk_mmsys_reset+0x7c/0xd0
  reset_control_reset+0x50/0x154
  mtk_merge_stop_cmdq+0x50/0x80
  mtk_ovl_adaptor_layer_config+0x1ec/0x20c
  mtk_crtc_ddp_config+0xa4/0x1cc
  mtk_crtc_ddp_irq+0xd0/0xd4
  mtk_ethdr_irq_handler+0x2c/0x44
  __handle_irq_event_percpu+0x60/0x14c
  handle_irq_event+0x4c/0xac
  handle_fasteoi_irq+0xa4/0x198
  generic_handle_domain_irq+0x2c/0x44
  gic_handle_irq+0x50/0x124
  call_on_irq_stack+0x24/0x4c
  do_interrupt_handler+0x80/0x84
  el1_interrupt+0x34/0x68
  el1h_64_irq_handler+0x18/0x24
  el1h_64_irq+0x64/0x68
  cpuidle_enter_state+0x1d8/0x2e8
  cpuidle_enter+0x38/0x50
  do_idle+0x1fc/0x27c
  cpu_startup_entry+0x28/0x2c
  kernel_init+0x0/0x1dc
  arch_post_acpi_subsys_init+0x0/0x8
  start_kernel+0x3dc/0x6e8
  __primary_switched+0xbc/0xc4
 bad: scheduling from the idle thread!

HTH,
Guillaume.

>+		mtk_mdp_rdma_stop(rdma_l, cmdq_pkt);
>+		mtk_mdp_rdma_stop(rdma_r, cmdq_pkt);
>+		mtk_ethdr_layer_config(ethdr, idx, state, cmdq_pkt);
>+		return;
>+	}
>+
>+	/* ETHDR is in 1T2P domain, width needs to be 2 pixels align */
>+	align_width = ALIGN_DOWN(pending->width, 2);
>+
>+	if (align_width > MTK_OVL_ADAPTOR_RDMA_MAX_WIDTH)
>+		use_dual_pipe = true;
>+
>+	if (use_dual_pipe) {
>+		l_w = (align_width / 2) + ((pending->width / 2) % 2);
>+		r_w = align_width - l_w;
>+	} else {
>+		l_w = align_width;
>+	}
>+	mtk_merge_advance_config(merge, l_w, r_w, pending->height, 0, 0, cmdq_pkt);
>+	mtk_mmsys_merge_async_config(ovl_adaptor->mmsys_dev, idx, align_width / 2,
>+				     pending->height, cmdq_pkt);
>+
>+	rdma_config.width = l_w;
>+	rdma_config.height = pending->height;
>+	rdma_config.addr0 = pending->addr;
>+	rdma_config.pitch = pending->pitch;
>+	rdma_config.fmt = pending->format;
>+	rdma_config.color_encoding = pending->color_encoding;
>+	mtk_mdp_rdma_config(rdma_l, &rdma_config, cmdq_pkt);
>+
>+	if (use_dual_pipe) {
>+		rdma_config.x_left = l_w;
>+		rdma_config.width = r_w;
>+		mtk_mdp_rdma_config(rdma_r, &rdma_config, cmdq_pkt);
>+	}
>+
>+	mtk_merge_start_cmdq(merge, cmdq_pkt);
>+
>+	mtk_mdp_rdma_start(rdma_l, cmdq_pkt);
>+	if (use_dual_pipe)
>+		mtk_mdp_rdma_start(rdma_r, cmdq_pkt);
>+	else
>+		mtk_mdp_rdma_stop(rdma_r, cmdq_pkt);
>+
>+	mtk_ethdr_layer_config(ethdr, idx, state, cmdq_pkt);
>+}
>+
>+void mtk_ovl_adaptor_config(struct device *dev, unsigned int w,
>+			    unsigned int h, unsigned int vrefresh,
>+			    unsigned int bpc, struct cmdq_pkt *cmdq_pkt)
>+{
>+	struct mtk_disp_ovl_adaptor *ovl_adaptor = dev_get_drvdata(dev);
>+
>+	mtk_ethdr_config(ovl_adaptor->ovl_adaptor_comp[OVL_ADAPTOR_ETHDR0], w, h,
>+			 vrefresh, bpc, cmdq_pkt);
>+}
>+
>+void mtk_ovl_adaptor_start(struct device *dev)
>+{
>+	struct mtk_disp_ovl_adaptor *ovl_adaptor = dev_get_drvdata(dev);
>+
>+	mtk_ethdr_start(ovl_adaptor->ovl_adaptor_comp[OVL_ADAPTOR_ETHDR0]);
>+}
>+
>+void mtk_ovl_adaptor_stop(struct device *dev)
>+{
>+	struct mtk_disp_ovl_adaptor *ovl_adaptor = dev_get_drvdata(dev);
>+
>+	mtk_ethdr_stop(ovl_adaptor->ovl_adaptor_comp[OVL_ADAPTOR_ETHDR0]);
>+}
>+
>+int mtk_ovl_adaptor_clk_enable(struct device *dev)
>+{
>+	struct mtk_disp_ovl_adaptor *ovl_adaptor = dev_get_drvdata(dev);
>+	struct device *comp;
>+	int ret;
>+	int i;
>+
>+	for (i = 0; i < OVL_ADAPTOR_MERGE0; i++) {
>+		comp = ovl_adaptor->ovl_adaptor_comp[i];
>+		ret = pm_runtime_get_sync(comp);
>+		if (ret < 0) {
>+			dev_err(dev, "Failed to enable power domain %d, err %d\n", i, ret);
>+			goto pwr_err;
>+		}
>+	}
>+
>+	for (i = 0; i < OVL_ADAPTOR_ID_MAX; i++) {
>+		comp = ovl_adaptor->ovl_adaptor_comp[i];
>+
>+		if (i < OVL_ADAPTOR_MERGE0)
>+			ret = mtk_mdp_rdma_clk_enable(comp);
>+		else if (i < OVL_ADAPTOR_ETHDR0)
>+			ret = mtk_merge_clk_enable(comp);
>+		else
>+			ret = mtk_ethdr_clk_enable(comp);
>+		if (ret) {
>+			dev_err(dev, "Failed to enable clock %d, err %d\n", i, ret);
>+			goto clk_err;
>+		}
>+	}
>+
>+	return ret;
>+
>+clk_err:
>+	while (--i >= 0) {
>+		comp = ovl_adaptor->ovl_adaptor_comp[i];
>+		if (i < OVL_ADAPTOR_MERGE0)
>+			mtk_mdp_rdma_clk_disable(comp);
>+		else if (i < OVL_ADAPTOR_ETHDR0)
>+			mtk_merge_clk_disable(comp);
>+		else
>+			mtk_ethdr_clk_disable(comp);
>+	}
>+	i = OVL_ADAPTOR_MERGE0;
>+
>+pwr_err:
>+	while (--i >= 0)
>+		pm_runtime_put(ovl_adaptor->ovl_adaptor_comp[i]);
>+
>+	return ret;
>+}
>+
>+void mtk_ovl_adaptor_clk_disable(struct device *dev)
>+{
>+	struct mtk_disp_ovl_adaptor *ovl_adaptor = dev_get_drvdata(dev);
>+	struct device *comp;
>+	int i;
>+
>+	for (i = 0; i < OVL_ADAPTOR_ID_MAX; i++) {
>+		comp = ovl_adaptor->ovl_adaptor_comp[i];
>+
>+		if (i < OVL_ADAPTOR_MERGE0) {
>+			mtk_mdp_rdma_clk_disable(comp);
>+			pm_runtime_put(comp);
>+		} else if (i < OVL_ADAPTOR_ETHDR0) {
>+			mtk_merge_clk_disable(comp);
>+		} else {
>+			mtk_ethdr_clk_disable(comp);
>+		}
>+	}
>+}
>+
>+unsigned int mtk_ovl_adaptor_layer_nr(struct device *dev)
>+{
>+	return MTK_OVL_ADAPTOR_LAYER_NUM;
>+}
>+
>+struct device *mtk_ovl_adaptor_dma_dev_get(struct device *dev)
>+{
>+	struct mtk_disp_ovl_adaptor *ovl_adaptor = dev_get_drvdata(dev);
>+
>+	return ovl_adaptor->ovl_adaptor_comp[OVL_ADAPTOR_MDP_RDMA0];
>+}
>+
>+void mtk_ovl_adaptor_register_vblank_cb(struct device *dev, void (*vblank_cb)(void *),
>+					void *vblank_cb_data)
>+{
>+	struct mtk_disp_ovl_adaptor *ovl_adaptor = dev_get_drvdata(dev);
>+
>+	mtk_ethdr_register_vblank_cb(ovl_adaptor->ovl_adaptor_comp[OVL_ADAPTOR_ETHDR0],
>+				     vblank_cb, vblank_cb_data);
>+}
>+
>+void mtk_ovl_adaptor_unregister_vblank_cb(struct device *dev)
>+{
>+	struct mtk_disp_ovl_adaptor *ovl_adaptor = dev_get_drvdata(dev);
>+
>+	mtk_ethdr_unregister_vblank_cb(ovl_adaptor->ovl_adaptor_comp[OVL_ADAPTOR_ETHDR0]);
>+}
>+
>+void mtk_ovl_adaptor_enable_vblank(struct device *dev)
>+{
>+	struct mtk_disp_ovl_adaptor *ovl_adaptor = dev_get_drvdata(dev);
>+
>+	mtk_ethdr_enable_vblank(ovl_adaptor->ovl_adaptor_comp[OVL_ADAPTOR_ETHDR0]);
>+}
>+
>+void mtk_ovl_adaptor_disable_vblank(struct device *dev)
>+{
>+	struct mtk_disp_ovl_adaptor *ovl_adaptor = dev_get_drvdata(dev);
>+
>+	mtk_ethdr_disable_vblank(ovl_adaptor->ovl_adaptor_comp[OVL_ADAPTOR_ETHDR0]);
>+}
>+
>+void mtk_ovl_adaptor_add_comp(struct device *dev, struct mtk_mutex *mutex)
>+{
>+	mtk_mutex_add_comp(mutex, DDP_COMPONENT_MDP_RDMA0);
>+	mtk_mutex_add_comp(mutex, DDP_COMPONENT_MDP_RDMA1);
>+	mtk_mutex_add_comp(mutex, DDP_COMPONENT_MDP_RDMA2);
>+	mtk_mutex_add_comp(mutex, DDP_COMPONENT_MDP_RDMA3);
>+	mtk_mutex_add_comp(mutex, DDP_COMPONENT_MDP_RDMA4);
>+	mtk_mutex_add_comp(mutex, DDP_COMPONENT_MDP_RDMA5);
>+	mtk_mutex_add_comp(mutex, DDP_COMPONENT_MDP_RDMA6);
>+	mtk_mutex_add_comp(mutex, DDP_COMPONENT_MDP_RDMA7);
>+	mtk_mutex_add_comp(mutex, DDP_COMPONENT_MERGE1);
>+	mtk_mutex_add_comp(mutex, DDP_COMPONENT_MERGE2);
>+	mtk_mutex_add_comp(mutex, DDP_COMPONENT_MERGE3);
>+	mtk_mutex_add_comp(mutex, DDP_COMPONENT_MERGE4);
>+	mtk_mutex_add_comp(mutex, DDP_COMPONENT_ETHDR_MIXER);
>+}
>+
>+void mtk_ovl_adaptor_remove_comp(struct device *dev, struct mtk_mutex *mutex)
>+{
>+	mtk_mutex_remove_comp(mutex, DDP_COMPONENT_MDP_RDMA0);
>+	mtk_mutex_remove_comp(mutex, DDP_COMPONENT_MDP_RDMA1);
>+	mtk_mutex_remove_comp(mutex, DDP_COMPONENT_MDP_RDMA2);
>+	mtk_mutex_remove_comp(mutex, DDP_COMPONENT_MDP_RDMA3);
>+	mtk_mutex_remove_comp(mutex, DDP_COMPONENT_MDP_RDMA4);
>+	mtk_mutex_remove_comp(mutex, DDP_COMPONENT_MDP_RDMA5);
>+	mtk_mutex_remove_comp(mutex, DDP_COMPONENT_MDP_RDMA6);
>+	mtk_mutex_remove_comp(mutex, DDP_COMPONENT_MDP_RDMA7);
>+	mtk_mutex_remove_comp(mutex, DDP_COMPONENT_MERGE1);
>+	mtk_mutex_remove_comp(mutex, DDP_COMPONENT_MERGE2);
>+	mtk_mutex_remove_comp(mutex, DDP_COMPONENT_MERGE3);
>+	mtk_mutex_remove_comp(mutex, DDP_COMPONENT_MERGE4);
>+	mtk_mutex_remove_comp(mutex, DDP_COMPONENT_ETHDR_MIXER);
>+}
>+
>+void mtk_ovl_adaptor_connect(struct device *dev, struct device *mmsys_dev, unsigned int next)
>+{
>+	mtk_mmsys_ddp_connect(mmsys_dev, DDP_COMPONENT_MDP_RDMA0, DDP_COMPONENT_MERGE1);
>+	mtk_mmsys_ddp_connect(mmsys_dev, DDP_COMPONENT_MDP_RDMA1, DDP_COMPONENT_MERGE1);
>+	mtk_mmsys_ddp_connect(mmsys_dev, DDP_COMPONENT_MDP_RDMA2, DDP_COMPONENT_MERGE2);
>+	mtk_mmsys_ddp_connect(mmsys_dev, DDP_COMPONENT_MERGE1, DDP_COMPONENT_ETHDR_MIXER);
>+	mtk_mmsys_ddp_connect(mmsys_dev, DDP_COMPONENT_MERGE2, DDP_COMPONENT_ETHDR_MIXER);
>+	mtk_mmsys_ddp_connect(mmsys_dev, DDP_COMPONENT_MERGE3, DDP_COMPONENT_ETHDR_MIXER);
>+	mtk_mmsys_ddp_connect(mmsys_dev, DDP_COMPONENT_MERGE4, DDP_COMPONENT_ETHDR_MIXER);
>+	mtk_mmsys_ddp_connect(mmsys_dev, DDP_COMPONENT_ETHDR_MIXER, next);
>+}
>+
>+void mtk_ovl_adaptor_disconnect(struct device *dev, struct device *mmsys_dev, unsigned int next)
>+{
>+	mtk_mmsys_ddp_disconnect(mmsys_dev, DDP_COMPONENT_MDP_RDMA0, DDP_COMPONENT_MERGE1);
>+	mtk_mmsys_ddp_disconnect(mmsys_dev, DDP_COMPONENT_MDP_RDMA1, DDP_COMPONENT_MERGE1);
>+	mtk_mmsys_ddp_disconnect(mmsys_dev, DDP_COMPONENT_MDP_RDMA2, DDP_COMPONENT_MERGE2);
>+	mtk_mmsys_ddp_disconnect(mmsys_dev, DDP_COMPONENT_MERGE1, DDP_COMPONENT_ETHDR_MIXER);
>+	mtk_mmsys_ddp_disconnect(mmsys_dev, DDP_COMPONENT_MERGE2, DDP_COMPONENT_ETHDR_MIXER);
>+	mtk_mmsys_ddp_disconnect(mmsys_dev, DDP_COMPONENT_MERGE3, DDP_COMPONENT_ETHDR_MIXER);
>+	mtk_mmsys_ddp_disconnect(mmsys_dev, DDP_COMPONENT_MERGE4, DDP_COMPONENT_ETHDR_MIXER);
>+	mtk_mmsys_ddp_disconnect(mmsys_dev, DDP_COMPONENT_ETHDR_MIXER, next);
>+}
>+
>+static int ovl_adaptor_comp_get_id(struct device *dev, struct device_node *node,
>+				   enum mtk_ovl_adaptor_comp_type type)
>+{
>+	int alias_id = of_alias_get_id(node, private_comp_stem[type]);
>+	int i;
>+
>+	for (i = 0; i < ARRAY_SIZE(comp_matches); i++)
>+		if (comp_matches[i].type == type &&
>+		    comp_matches[i].alias_id == alias_id)
>+			return i;
>+
>+	dev_warn(dev, "Failed to get id. type: %d, alias: %d\n", type, alias_id);
>+	return -EINVAL;
>+}
>+
>+static const struct of_device_id mtk_ovl_adaptor_comp_dt_ids[] = {
>+	{
>+		.compatible = "mediatek,mt8195-vdo1-rdma",
>+		.data = (void *)OVL_ADAPTOR_TYPE_RDMA,
>+	}, {
>+		.compatible = "mediatek,mt8195-disp-merge",
>+		.data = (void *)OVL_ADAPTOR_TYPE_MERGE,
>+	}, {
>+		.compatible = "mediatek,mt8195-disp-ethdr",
>+		.data = (void *)OVL_ADAPTOR_TYPE_ETHDR,
>+	},
>+	{},
>+};
>+
>+static int compare_of(struct device *dev, void *data)
>+{
>+	return dev->of_node == data;
>+}
>+
>+static int ovl_adaptor_comp_init(struct device *dev, struct component_match **match)
>+{
>+	struct mtk_disp_ovl_adaptor *priv = dev_get_drvdata(dev);
>+	struct device_node *node, *parent;
>+	struct platform_device *comp_pdev;
>+
>+	parent = dev->parent->parent->of_node->parent;
>+
>+	for_each_child_of_node(parent, node) {
>+		const struct of_device_id *of_id;
>+		enum mtk_ovl_adaptor_comp_type type;
>+		int id;
>+
>+		of_id = of_match_node(mtk_ovl_adaptor_comp_dt_ids, node);
>+		if (!of_id)
>+			continue;
>+
>+		if (!of_device_is_available(node)) {
>+			dev_dbg(dev, "Skipping disabled component %pOF\n",
>+				node);
>+			continue;
>+		}
>+
>+		type = (enum mtk_ovl_adaptor_comp_type)of_id->data;
>+		id = ovl_adaptor_comp_get_id(dev, node, type);
>+		if (id < 0) {
>+			dev_warn(dev, "Skipping unknown component %pOF\n",
>+				 node);
>+			continue;
>+		}
>+
>+		comp_pdev = of_find_device_by_node(node);
>+		if (!comp_pdev)
>+			return -EPROBE_DEFER;
>+
>+		priv->ovl_adaptor_comp[id] = &comp_pdev->dev;
>+
>+		drm_of_component_match_add(dev, match, compare_of, node);
>+		dev_dbg(dev, "Adding component match for %pOF\n", node);
>+	}
>+
>+	if (!*match) {
>+		dev_err(dev, "No match device for ovl_adaptor\n");
>+		return -ENODEV;
>+	}
>+
>+	return 0;
>+}
>+
>+static int mtk_disp_ovl_adaptor_comp_bind(struct device *dev, struct device *master,
>+					  void *data)
>+{
>+	struct mtk_disp_ovl_adaptor *priv = dev_get_drvdata(dev);
>+
>+	if (!priv->children_bound)
>+		return -EPROBE_DEFER;
>+
>+	return 0;
>+}
>+
>+static void mtk_disp_ovl_adaptor_comp_unbind(struct device *dev, struct device *master,
>+					     void *data)
>+{
>+}
>+
>+static const struct component_ops mtk_disp_ovl_adaptor_comp_ops = {
>+	.bind	= mtk_disp_ovl_adaptor_comp_bind,
>+	.unbind = mtk_disp_ovl_adaptor_comp_unbind,
>+};
>+
>+static int mtk_disp_ovl_adaptor_master_bind(struct device *dev)
>+{
>+	struct mtk_disp_ovl_adaptor *priv = dev_get_drvdata(dev);
>+	int ret;
>+
>+	ret = component_bind_all(dev, priv->mmsys_dev);
>+	if (ret)
>+		return dev_err_probe(dev, ret, "component_bind_all failed!\n");
>+
>+	priv->children_bound = true;
>+	return 0;
>+}
>+
>+static void mtk_disp_ovl_adaptor_master_unbind(struct device *dev)
>+{
>+	struct mtk_disp_ovl_adaptor *priv = dev_get_drvdata(dev);
>+
>+	priv->children_bound = false;
>+}
>+
>+static const struct component_master_ops mtk_disp_ovl_adaptor_master_ops = {
>+	.bind		= mtk_disp_ovl_adaptor_master_bind,
>+	.unbind		= mtk_disp_ovl_adaptor_master_unbind,
>+};
>+
>+static int mtk_disp_ovl_adaptor_probe(struct platform_device *pdev)
>+{
>+	struct mtk_disp_ovl_adaptor *priv;
>+	struct device *dev = &pdev->dev;
>+	struct component_match *match = NULL;
>+	int ret;
>+
>+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>+	if (!priv)
>+		return -ENOMEM;
>+
>+	platform_set_drvdata(pdev, priv);
>+
>+	ret = ovl_adaptor_comp_init(dev, &match);
>+	if (ret < 0)
>+		return ret;
>+
>+	priv->mmsys_dev = pdev->dev.platform_data;
>+
>+	component_master_add_with_match(dev, &mtk_disp_ovl_adaptor_master_ops, match);
>+
>+	pm_runtime_enable(dev);
>+
>+	ret = component_add(dev, &mtk_disp_ovl_adaptor_comp_ops);
>+	if (ret != 0) {
>+		pm_runtime_disable(dev);
>+		dev_err(dev, "Failed to add component: %d\n", ret);
>+	}
>+
>+	return ret;
>+}
>+
>+static int mtk_disp_ovl_adaptor_remove(struct platform_device *pdev)
>+{
>+	component_master_del(&pdev->dev, &mtk_disp_ovl_adaptor_master_ops);
>+	pm_runtime_disable(&pdev->dev);
>+	return 0;
>+}
>+
>+struct platform_driver mtk_disp_ovl_adaptor_driver = {
>+	.probe		= mtk_disp_ovl_adaptor_probe,
>+	.remove		= mtk_disp_ovl_adaptor_remove,
>+	.driver		= {
>+		.name	= "mediatek-disp-ovl-adaptor",
>+		.owner	= THIS_MODULE,
>+	},
>+};
>diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
>index 3fb85776b8b3..d27561e5e274 100644
>--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
>+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
>@@ -51,6 +51,7 @@ extern struct platform_driver mtk_disp_ccorr_driver;
> extern struct platform_driver mtk_disp_color_driver;
> extern struct platform_driver mtk_disp_gamma_driver;
> extern struct platform_driver mtk_disp_merge_driver;
>+extern struct platform_driver mtk_disp_ovl_adaptor_driver;
> extern struct platform_driver mtk_disp_ovl_driver;
> extern struct platform_driver mtk_disp_rdma_driver;
> extern struct platform_driver mtk_dpi_driver;
>--
>2.18.0
>
>

Re: [PATCH v29 4/7] drm/mediatek: add dma dev get function

[Guillaume Ranquet]

On Tue, 27 Dec 2022 09:10, "" wrote:
>This is a preparation for adding support for the ovl_adaptor sub driver
>Ovl_adaptor is a DRM sub driver, which doesn't have dma dev. Add
>dma_dev_get function for getting representative dma dev in ovl_adaptor.
>
>Signed-off-by: Nancy.Lin <nancy.lin@mediatek.com>
>Reviewed-by: AngeloGioachino Del Regno <angelogioacchino.delregno@collabora.com>
>Reviewed-by: CK Hu <ck.hu@mediatek.com>
>Tested-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>Tested-by: Bo-Chen Chen <rex-bc.chen@mediatek.com>
>---
> drivers/gpu/drm/mediatek/mtk_drm_crtc.c     | 15 +++++++++++++++
> drivers/gpu/drm/mediatek/mtk_drm_crtc.h     |  1 +
> drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |  8 ++++++++
> 3 files changed, 24 insertions(+)
>
>diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
>index 112615817dcb..78e20f604158 100644
>--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
>+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
>@@ -58,6 +58,7 @@ struct mtk_drm_crtc {
> #endif
>
> 	struct device			*mmsys_dev;
>+	struct device			*dma_dev;
> 	struct mtk_mutex		*mutex;
> 	unsigned int			ddp_comp_nr;
> 	struct mtk_ddp_comp		**ddp_comp;
>@@ -865,6 +866,13 @@ static int mtk_drm_crtc_init_comp_planes(struct drm_device *drm_dev,
> 	return 0;
> }
>
>+struct device *mtk_drm_crtc_dma_dev_get(struct drm_crtc *crtc)
>+{
>+	struct mtk_drm_crtc *mtk_crtc = to_mtk_crtc(crtc);
>+
>+	return mtk_crtc->dma_dev;
>+}

While testing out the HDMI patchset for i1200, I've ended up with a
panic here with crtc being NULL.

I've fixed the issue on my side by testing crtc prior doing anything
in that function.

Not sure this is the proper fix.

HTH,
Guillaume.

>+
> int mtk_drm_crtc_create(struct drm_device *drm_dev,
> 			const enum mtk_ddp_comp_id *path, unsigned int path_len)
> {
>@@ -953,6 +961,13 @@ int mtk_drm_crtc_create(struct drm_device *drm_dev,
> 			return ret;
> 	}
>
>+	/*
>+	 * Default to use the first component as the dma dev.
>+	 * In the case of ovl_adaptor sub driver, it needs to use the
>+	 * dma_dev_get function to get representative dma dev.
>+	 */
>+	mtk_crtc->dma_dev = mtk_ddp_comp_dma_dev_get(&priv->ddp_comp[path[0]]);
>+
> 	ret = mtk_drm_crtc_init(drm_dev, mtk_crtc, pipe);
> 	if (ret < 0)
> 		return ret;
>diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.h b/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
>index cb9a36c48d4f..f5a6e80c5265 100644
>--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
>+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.h
>@@ -22,5 +22,6 @@ int mtk_drm_crtc_plane_check(struct drm_crtc *crtc, struct drm_plane *plane,
> 			     struct mtk_plane_state *state);
> void mtk_drm_crtc_async_update(struct drm_crtc *crtc, struct drm_plane *plane,
> 			       struct drm_atomic_state *plane_state);
>+struct device *mtk_drm_crtc_dma_dev_get(struct drm_crtc *crtc);
>
> #endif /* MTK_DRM_CRTC_H */
>diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
>index 2d0052c23dcb..364f3f7f59fa 100644
>--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
>+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h
>@@ -71,6 +71,7 @@ struct mtk_ddp_comp_funcs {
> 	void (*bgclr_in_off)(struct device *dev);
> 	void (*ctm_set)(struct device *dev,
> 			struct drm_crtc_state *state);
>+	struct device * (*dma_dev_get)(struct device *dev);
> };
>
> struct mtk_ddp_comp {
>@@ -203,6 +204,13 @@ static inline void mtk_ddp_ctm_set(struct mtk_ddp_comp *comp,
> 		comp->funcs->ctm_set(comp->dev, state);
> }
>
>+static inline struct device *mtk_ddp_comp_dma_dev_get(struct mtk_ddp_comp *comp)
>+{
>+	if (comp->funcs && comp->funcs->dma_dev_get)
>+		return comp->funcs->dma_dev_get(comp->dev);
>+	return comp->dev;
>+}
>+
> int mtk_ddp_comp_get_id(struct device_node *node,
> 			enum mtk_ddp_comp_type comp_type);
> unsigned int mtk_drm_find_possible_crtc_by_comp(struct drm_device *drm,
>--
>2.18.0
>
>

Re: [PATCH v29 1/7] dt-bindings: mediatek: add ethdr definition for mt8195

[Nancy Lin (林欣螢)]

On Tue, 2022-12-27 at 16:10 +0800, Nancy.Lin wrote:
> Add vdosys1 ETHDR definition.
> 
> Signed-off-by: Nancy.Lin <nancy.lin@mediatek.com>
> Reviewed-by: Chun-Kuang Hu <chunkuang.hu@kernel.org>
> Reviewed-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@collabora.com>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Tested-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@collabora.com>
> ---
>  .../display/mediatek/mediatek,ethdr.yaml      | 188
> ++++++++++++++++++
>  1 file changed, 188 insertions(+)
>  create mode 100644
> Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.yam
> l
> 
> diff --git
> a/Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.y
> aml
> b/Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.y
> aml
> new file mode 100644
> index 000000000000..3b11e47a8834
> --- /dev/null
> +++
> b/Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.y
> aml
> @@ -0,0 +1,188 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: 
> http://devicetree.org/schemas/display/mediatek/mediatek,ethdr.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek Ethdr Device
> +
> +maintainers:
> +  - Chun-Kuang Hu <chunkuang.hu@kernel.org>
> +  - Philipp Zabel <p.zabel@pengutronix.de>
> +
> +description:
> +  ETHDR (ET High Dynamic Range) is a MediaTek internal HDR engine
> and is
> +  designed for HDR video and graphics conversion in the external
> display path.
> +  It handles multiple HDR input types and performs tone mapping,
> color
> +  space/color format conversion, and then combine different layers,
> +  output the required HDR or SDR signal to the subsequent display
> path.
> +  This engine is composed of two video frontends, two graphic
> frontends,
> +  one video backend and a mixer. ETHDR has two DMA function blocks,
> DS and ADL.
> +  These two function blocks read the pre-programmed registers from
> DRAM and
> +  set them to HW in the v-blanking period.
> +
> +properties:
> +  compatible:
> +    const: mediatek,mt8195-disp-ethdr
> +
> +  reg:
> +    maxItems: 7
> +
> +  reg-names:
> +    items:
> +      - const: mixer
> +      - const: vdo_fe0
> +      - const: vdo_fe1
> +      - const: gfx_fe0
> +      - const: gfx_fe1
> +      - const: vdo_be
> +      - const: adl_ds
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  iommus:
> +    minItems: 1
> +    maxItems: 2
> +
> +  clocks:
> +    items:
> +      - description: mixer clock
> +      - description: video frontend 0 clock
> +      - description: video frontend 1 clock
> +      - description: graphic frontend 0 clock
> +      - description: graphic frontend 1 clock
> +      - description: video backend clock
> +      - description: autodownload and menuload clock
> +      - description: video frontend 0 async clock
> +      - description: video frontend 1 async clock
> +      - description: graphic frontend 0 async clock
> +      - description: graphic frontend 1 async clock
> +      - description: video backend async clock
> +      - description: ethdr top clock
> +
> +  clock-names:
> +    items:
> +      - const: mixer
> +      - const: vdo_fe0
> +      - const: vdo_fe1
> +      - const: gfx_fe0
> +      - const: gfx_fe1
> +      - const: vdo_be
> +      - const: adl_ds
> +      - const: vdo_fe0_async
> +      - const: vdo_fe1_async
> +      - const: gfx_fe0_async
> +      - const: gfx_fe1_async
> +      - const: vdo_be_async
> +      - const: ethdr_top
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  resets:
> +    items:
> +      - description: video frontend 0 async reset
> +      - description: video frontend 1 async reset
> +      - description: graphic frontend 0 async reset
> +      - description: graphic frontend 1 async reset
> +      - description: video backend async reset
> +
> +  reset-names:
> +    items:
> +      - const: vdo_fe0_async
> +      - const: vdo_fe1_async
> +      - const: gfx_fe0_async
> +      - const: gfx_fe1_async
> +      - const: vdo_be_async
> +
> +  mediatek,gce-client-reg:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    description: The register of display function block to be set by
> gce.
> +      There are 4 arguments in this property, gce node, subsys id,
> offset and
> +      register size. The subsys id is defined in the gce header of
> each chips
> +      include/dt-bindings/gce/<chip>-gce.h, mapping to the register
> of display
> +      function block.
> +    items:
> +      items:
> +        - description: phandle of GCE
> +        - description: GCE subsys id
> +        - description: register offset
> +        - description: register size
> +    minItems: 7
> +    maxItems: 7
> +

Hi Rob and krzysztof,

I got the two messages when running dt_binding_check [1]. This binding
patch was sent previously in [2]. 

If I remove the following items/minItems/maxItems in the mediatek,gce-
client property, the two message disappear. I don't know what's wrong
with the original syntax. Do you have any suggestions for this?

-    items:
-      items:
-        - description: phandle of GCE
-        - description: GCE subsys id
-        - description: register offset
-        - description: register size
-    minItems: 7
-    maxItems: 7


[1].
Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.examp
le.dtb
/proj/mtk19347/cros/src/third_party/kernel/v5.10/Documentation/devicetr
ee/bindings/display/mediatek/mediatek,ethdr.example.dtb: 
hdr-engine@1c114000: mediatek,gce-client-reg:0: [4294967295, 7, 16384,
4096, 4294967295, 7, 20480, 4096, 4294967295, 7, 28672, 4096,
4294967295, 7, 36864, 4096, 4294967295, 7, 40960, 4096, 4294967295, 7,
45056, 4096, 4294967295, 7, 49152, 4096] is too long
        From schema:
/proj/mtk19347/cros/src/third_party/kernel/v5.10/Documentation/devicetr
ee/bindings/display/mediatek/mediatek,ethdr.yaml
/proj/mtk19347/cros/src/third_party/kernel/v5.10/Documentation/devicetr
ee/bindings/display/mediatek/mediatek,ethdr.example.dtb: 
hdr-engine@1c114000: mediatek,gce-client-reg: [[4294967295, 7, 16384,
4096, 4294967295, 7, 20480, 4096, 4294967295, 7, 28672, 4096,
4294967295, 7, 36864, 4096, 4294967295, 7, 40960, 4096, 4294967295, 7,
45056, 4096, 4294967295, 7, 49152, 4096]] is too short
        From schema:
/proj/mtk19347/cros/src/third_party/kernel/v5.10/Documentation/devicetr
ee/bindings/display/mediatek/mediatek,ethdr.yaml

[2] [RESEND,v5,3/3] dt-bindings: mediatek: add ethdr definition for
mt8195

https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20220608043852.4980-4-rex-bc.chen@mediatek.com/


Regards,
Nancy















> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - interrupts
> +  - power-domains
> +  - resets
> +  - mediatek,gce-client-reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/mt8195-clk.h>
> +    #include <dt-bindings/gce/mt8195-gce.h>
> +    #include <dt-bindings/memory/mt8195-memory-port.h>
> +    #include <dt-bindings/power/mt8195-power.h>
> +    #include <dt-bindings/reset/mt8195-resets.h>
> +
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        hdr-engine@1c114000 {
> +                compatible = "mediatek,mt8195-disp-ethdr";
> +                reg = <0 0x1c114000 0 0x1000>,
> +                      <0 0x1c115000 0 0x1000>,
> +                      <0 0x1c117000 0 0x1000>,
> +                      <0 0x1c119000 0 0x1000>,
> +                      <0 0x1c11a000 0 0x1000>,
> +                      <0 0x1c11b000 0 0x1000>,
> +                      <0 0x1c11c000 0 0x1000>;
> +                reg-names = "mixer", "vdo_fe0", "vdo_fe1",
> "gfx_fe0", "gfx_fe1",
> +                            "vdo_be", "adl_ds";
> +                mediatek,gce-client-reg = <&gce0 SUBSYS_1c11XXXX
> 0x4000 0x1000>,
> +                                          <&gce0 SUBSYS_1c11XXXX
> 0x5000 0x1000>,
> +                                          <&gce0 SUBSYS_1c11XXXX
> 0x7000 0x1000>,
> +                                          <&gce0 SUBSYS_1c11XXXX
> 0x9000 0x1000>,
> +                                          <&gce0 SUBSYS_1c11XXXX
> 0xa000 0x1000>,
> +                                          <&gce0 SUBSYS_1c11XXXX
> 0xb000 0x1000>,
> +                                          <&gce0 SUBSYS_1c11XXXX
> 0xc000 0x1000>;
> +                clocks = <&vdosys1 CLK_VDO1_DISP_MIXER>,
> +                         <&vdosys1 CLK_VDO1_HDR_VDO_FE0>,
> +                         <&vdosys1 CLK_VDO1_HDR_VDO_FE1>,
> +                         <&vdosys1 CLK_VDO1_HDR_GFX_FE0>,
> +                         <&vdosys1 CLK_VDO1_HDR_GFX_FE1>,
> +                         <&vdosys1 CLK_VDO1_HDR_VDO_BE>,
> +                         <&vdosys1 CLK_VDO1_26M_SLOW>,
> +                         <&vdosys1 CLK_VDO1_HDR_VDO_FE0_DL_ASYNC>,
> +                         <&vdosys1 CLK_VDO1_HDR_VDO_FE1_DL_ASYNC>,
> +                         <&vdosys1 CLK_VDO1_HDR_GFX_FE0_DL_ASYNC>,
> +                         <&vdosys1 CLK_VDO1_HDR_GFX_FE1_DL_ASYNC>,
> +                         <&vdosys1 CLK_VDO1_HDR_VDO_BE_DL_ASYNC>,
> +                         <&topckgen CLK_TOP_ETHDR>;
> +                clock-names = "mixer", "vdo_fe0", "vdo_fe1",
> "gfx_fe0", "gfx_fe1",
> +                              "vdo_be", "adl_ds", "vdo_fe0_async",
> "vdo_fe1_async",
> +                              "gfx_fe0_async",
> "gfx_fe1_async","vdo_be_async",
> +                              "ethdr_top";
> +                power-domains = <&spm MT8195_POWER_DOMAIN_VDOSYS1>;
> +                iommus = <&iommu_vpp M4U_PORT_L3_HDR_DS>,
> +                         <&iommu_vpp M4U_PORT_L3_HDR_ADL>;
> +                interrupts = <GIC_SPI 517 IRQ_TYPE_LEVEL_HIGH 0>; /*
> disp mixer */
> +                resets = <&vdosys1
> MT8195_VDOSYS1_SW1_RST_B_HDR_VDO_FE0_DL_ASYNC>,
> +                         <&vdosys1
> MT8195_VDOSYS1_SW1_RST_B_HDR_VDO_FE1_DL_ASYNC>,
> +                         <&vdosys1
> MT8195_VDOSYS1_SW1_RST_B_HDR_GFX_FE0_DL_ASYNC>,
> +                         <&vdosys1
> MT8195_VDOSYS1_SW1_RST_B_HDR_GFX_FE1_DL_ASYNC>,
> +                         <&vdosys1
> MT8195_VDOSYS1_SW1_RST_B_HDR_VDO_BE_DL_ASYNC>;
> +                reset-names = "vdo_fe0_async", "vdo_fe1_async",
> "gfx_fe0_async",
> +                              "gfx_fe1_async", "vdo_be_async";
> +        };
> +    };
> +...

Re: [PATCH v29 1/7] dt-bindings: mediatek: add ethdr definition for mt8195

[Krzysztof Kozlowski]

On 15/03/2023 04:45, Nancy Lin (林欣螢) wrote:

Trim the replies and remove unneeded context. You want to get the
attention of other people, not force them to read entire email.

>> +  mediatek,gce-client-reg:>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>> +    description: The register of display function block to be set by
>> gce.
>> +      There are 4 arguments in this property, gce node, subsys id,
>> offset and
>> +      register size. The subsys id is defined in the gce header of
>> each chips
>> +      include/dt-bindings/gce/<chip>-gce.h, mapping to the register
>> of display
>> +      function block.
>> +    items:
>> +      items:
>> +        - description: phandle of GCE
>> +        - description: GCE subsys id
>> +        - description: register offset
>> +        - description: register size
>> +    minItems: 7
>> +    maxItems: 7
>> +
> 
> Hi Rob and krzysztof,
> 
> I got the two messages when running dt_binding_check [1]. This binding
> patch was sent previously in [2]. 
> 
> If I remove the following items/minItems/maxItems in the mediatek,gce-
> client property, the two message disappear. I don't know what's wrong
> with the original syntax. Do you have any suggestions for this?
> 
> -    items:
> -      items:
> -        - description: phandle of GCE
> -        - description: GCE subsys id
> -        - description: register offset
> -        - description: register size
> -    minItems: 7
> -    maxItems: 7
> 
> 
> [1].
> Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.examp
> le.dtb
> /proj/mtk19347/cros/src/third_party/kernel/v5.10/Documentation/devicetr
> ee/bindings/display/mediatek/mediatek,ethdr.example.dtb: 
> hdr-engine@1c114000: mediatek,gce-client-reg:0: [4294967295, 7, 16384,
> 4096, 4294967295, 7, 20480, 4096, 4294967295, 7, 28672, 4096,
> 4294967295, 7, 36864, 4096, 4294967295, 7, 40960, 4096, 4294967295, 7,
> 45056, 4096, 4294967295, 7, 49152, 4096] is too long
>         From schema:

This looks like known issue with phandles with variable number of
arguments. Either we add it to the exceptions or just define it in
reduced way like in other cases - only maxItems: 1 without describing items.


Best regards,
Krzysztof

Re: [PATCH v29 1/7] dt-bindings: mediatek: add ethdr definition for mt8195

[Nancy Lin (林欣螢)]

On Wed, 2023-03-15 at 08:16 +0100, Krzysztof Kozlowski wrote:
> On 15/03/2023 04:45, Nancy Lin (林欣螢) wrote:
> 
> Trim the replies and remove unneeded context. You want to get the
> attention of other people, not force them to read entire email.
> 
> > > +  mediatek,gce-client-reg:>> +    $ref:
> > > /schemas/types.yaml#/definitions/phandle-array
> > > +    description: The register of display function block to be
> > > set by
> > > gce.
> > > +      There are 4 arguments in this property, gce node, subsys
> > > id,
> > > offset and
> > > +      register size. The subsys id is defined in the gce header
> > > of
> > > each chips
> > > +      include/dt-bindings/gce/<chip>-gce.h, mapping to the
> > > register
> > > of display
> > > +      function block.
> > > +    items:
> > > +      items:
> > > +        - description: phandle of GCE
> > > +        - description: GCE subsys id
> > > +        - description: register offset
> > > +        - description: register size
> > > +    minItems: 7
> > > +    maxItems: 7
> > > +
> > 
> > Hi Rob and krzysztof,
> > 
> > I got the two messages when running dt_binding_check [1]. This
> > binding
> > patch was sent previously in [2]. 
> > 
> > If I remove the following items/minItems/maxItems in the
> > mediatek,gce-
> > client property, the two message disappear. I don't know what's
> > wrong
> > with the original syntax. Do you have any suggestions for this?
> > 
> > -    items:
> > -      items:
> > -        - description: phandle of GCE
> > -        - description: GCE subsys id
> > -        - description: register offset
> > -        - description: register size
> > -    minItems: 7
> > -    maxItems: 7
> > 
> > 
> > [1].
> > Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.e
> > xamp
> > le.dtb
> > /proj/mtk19347/cros/src/third_party/kernel/v5.10/Documentation/devi
> > cetr
> > ee/bindings/display/mediatek/mediatek,ethdr.example.dtb: 
> > hdr-engine@1c114000: mediatek,gce-client-reg:0: [4294967295, 7,
> > 16384,
> > 4096, 4294967295, 7, 20480, 4096, 4294967295, 7, 28672, 4096,
> > 4294967295, 7, 36864, 4096, 4294967295, 7, 40960, 4096, 4294967295,
> > 7,
> > 45056, 4096, 4294967295, 7, 49152, 4096] is too long
> >         From schema:
> 
> This looks like known issue with phandles with variable number of
> arguments. Either we add it to the exceptions or just define it in
> reduced way like in other cases - only maxItems: 1 without describing
> items.
> 
> 
> Best regards,
> Krzysztof


Hi Krzysztof,

Thanks for the comment.

But I have several items for this vendor property in the binding
example.
Can I remove maxItems? Change the mediatek,gce-client-reg as [1].

[1]
  mediatek,gce-client-reg:
    $ref: /schemas/types.yaml#/definitions/phandle-array
    description: The register of display function block to be set by
gce.
      There are 4 arguments in this property, gce node, subsys id,
offset and
      register size. The subsys id is defined in the gce header of each
chips
      include/dt-bindings/gce/<chip>-gce.h, mapping to the register of
display
      function block.

Regards,
Nancy

> 

Re: [PATCH v29 1/7] dt-bindings: mediatek: add ethdr definition for mt8195

[Krzysztof Kozlowski]

On 16/03/2023 07:19, Nancy Lin (林欣螢) wrote:
> On Wed, 2023-03-15 at 08:16 +0100, Krzysztof Kozlowski wrote:
>> On 15/03/2023 04:45, Nancy Lin (林欣螢) wrote:
>>
>> Trim the replies and remove unneeded context. You want to get the
>> attention of other people, not force them to read entire email.
>>
>>>> +  mediatek,gce-client-reg:>> +    $ref:
>>>> /schemas/types.yaml#/definitions/phandle-array
>>>> +    description: The register of display function block to be
>>>> set by
>>>> gce.
>>>> +      There are 4 arguments in this property, gce node, subsys
>>>> id,
>>>> offset and
>>>> +      register size. The subsys id is defined in the gce header
>>>> of
>>>> each chips
>>>> +      include/dt-bindings/gce/<chip>-gce.h, mapping to the
>>>> register
>>>> of display
>>>> +      function block.
>>>> +    items:
>>>> +      items:
>>>> +        - description: phandle of GCE
>>>> +        - description: GCE subsys id
>>>> +        - description: register offset
>>>> +        - description: register size
>>>> +    minItems: 7
>>>> +    maxItems: 7
>>>> +
>>>
>>> Hi Rob and krzysztof,
>>>
>>> I got the two messages when running dt_binding_check [1]. This
>>> binding
>>> patch was sent previously in [2]. 
>>>
>>> If I remove the following items/minItems/maxItems in the
>>> mediatek,gce-
>>> client property, the two message disappear. I don't know what's
>>> wrong
>>> with the original syntax. Do you have any suggestions for this?
>>>
>>> -    items:
>>> -      items:
>>> -        - description: phandle of GCE
>>> -        - description: GCE subsys id
>>> -        - description: register offset
>>> -        - description: register size
>>> -    minItems: 7
>>> -    maxItems: 7
>>>
>>>
>>> [1].
>>> Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.e
>>> xamp
>>> le.dtb
>>> /proj/mtk19347/cros/src/third_party/kernel/v5.10/Documentation/devi
>>> cetr
>>> ee/bindings/display/mediatek/mediatek,ethdr.example.dtb: 
>>> hdr-engine@1c114000: mediatek,gce-client-reg:0: [4294967295, 7,
>>> 16384,
>>> 4096, 4294967295, 7, 20480, 4096, 4294967295, 7, 28672, 4096,
>>> 4294967295, 7, 36864, 4096, 4294967295, 7, 40960, 4096, 4294967295,
>>> 7,
>>> 45056, 4096, 4294967295, 7, 49152, 4096] is too long
>>>         From schema:
>>
>> This looks like known issue with phandles with variable number of
>> arguments. Either we add it to the exceptions or just define it in
>> reduced way like in other cases - only maxItems: 1 without describing
>> items.
>>
>>
>> Best regards,
>> Krzysztof
> 
> 
> Hi Krzysztof,
> 
> Thanks for the comment.
> 
> But I have several items for this vendor property in the binding
> example.

Do you? I thought you have one phandle?

> Can I remove maxItems? Change the mediatek,gce-client-reg as [1].
> 
> [1]
>   mediatek,gce-client-reg:
>     $ref: /schemas/types.yaml#/definitions/phandle-array
>     description: The register of display function block to be set by
> gce.
>       There are 4 arguments in this property, gce node, subsys id,
> offset and
>       register size. The subsys id is defined in the gce header of each
> chips
>       include/dt-bindings/gce/<chip>-gce.h, mapping to the register of
> display
>       function block.

No, this needs some constraints.

Best regards,
Krzysztof

Re: [PATCH v29 1/7] dt-bindings: mediatek: add ethdr definition for mt8195

[AngeloGioacchino Del Regno]

Il 16/03/23 07:31, Krzysztof Kozlowski ha scritto:
> On 16/03/2023 07:19, Nancy Lin (林欣螢) wrote:
>> On Wed, 2023-03-15 at 08:16 +0100, Krzysztof Kozlowski wrote:
>>> On 15/03/2023 04:45, Nancy Lin (林欣螢) wrote:
>>>

..snip..

>>>>
>>>>
>>>> [1].
>>>> Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.e
>>>> xamp
>>>> le.dtb
>>>> /proj/mtk19347/cros/src/third_party/kernel/v5.10/Documentation/devi
>>>> cetr
>>>> ee/bindings/display/mediatek/mediatek,ethdr.example.dtb:
>>>> hdr-engine@1c114000: mediatek,gce-client-reg:0: [4294967295, 7,
>>>> 16384,
>>>> 4096, 4294967295, 7, 20480, 4096, 4294967295, 7, 28672, 4096,
>>>> 4294967295, 7, 36864, 4096, 4294967295, 7, 40960, 4096, 4294967295,
>>>> 7,
>>>> 45056, 4096, 4294967295, 7, 49152, 4096] is too long
>>>>          From schema:
>>>
>>> This looks like known issue with phandles with variable number of
>>> arguments. Either we add it to the exceptions or just define it in
>>> reduced way like in other cases - only maxItems: 1 without describing
>>> items.
>>>

...

>>
>> But I have several items for this vendor property in the binding
>> example.
> 
> Do you? I thought you have one phandle?
> 
>> Can I remove maxItems? Change the mediatek,gce-client-reg as [1].
>>
>> [1]
>>    mediatek,gce-client-reg:
>>      $ref: /schemas/types.yaml#/definitions/phandle-array
>>      description: The register of display function block to be set by
>> gce.
>>        There are 4 arguments in this property, gce node, subsys id,
>> offset and
>>        register size. The subsys id is defined in the gce header of each
>> chips
>>        include/dt-bindings/gce/<chip>-gce.h, mapping to the register of
>> display
>>        function block.
> 
> No, this needs some constraints.

Hello Krzysztof, Nancy,

Since this series has reached v29, can we please reach an agreement on the bindings
to use here, so that we can get this finally upstreamed?

I will put some examples to try to get this issue resolved.

### Example 1: Constrain the number of GCE entries to *seven* array elements (7x4!)

   mediatek,gce-client-reg:
     $ref: /schemas/types.yaml#/definitions/phandle-array
     maxItems: 1
     description: The register of display function block to be set by gce.
       There are 4 arguments in this property, gce node, subsys id, offset and
       register size. The subsys id is defined in the gce header of each chips
       include/dt-bindings/gce/<chip>-gce.h, mapping to the register of display
       function block.
     items:
       minItems: 28
       maxItems: 28
       items:                     <----- this block doesn't seem to get checked :\
         - description: phandle of GCE
         - description: GCE subsys id
         - description: register offset
         - description: register size


### Example 2: Don't care about constraining the number of arguments

   mediatek,gce-client-reg:
     $ref: /schemas/types.yaml#/definitions/phandle-array
     maxItems: 1
     description: The register of display function block to be set by gce.
       There are 4 arguments in this property, gce node, subsys id, offset and
       register size. The subsys id is defined in the gce header of each chips
       include/dt-bindings/gce/<chip>-gce.h, mapping to the register of display
       function block.


Regards,
Angelo

Re: [PATCH v29 1/7] dt-bindings: mediatek: add ethdr definition for mt8195

[Krzysztof Kozlowski]

On 16/03/2023 10:53, AngeloGioacchino Del Regno wrote:

> Hello Krzysztof, Nancy,
> 
> Since this series has reached v29, can we please reach an agreement on the bindings
> to use here, so that we can get this finally upstreamed?
> 
> I will put some examples to try to get this issue resolved.
> 
> ### Example 1: Constrain the number of GCE entries to *seven* array elements (7x4!)
> 
>    mediatek,gce-client-reg:
>      $ref: /schemas/types.yaml#/definitions/phandle-array
>      maxItems: 1
>      description: The register of display function block to be set by gce.
>        There are 4 arguments in this property, gce node, subsys id, offset and
>        register size. The subsys id is defined in the gce header of each chips
>        include/dt-bindings/gce/<chip>-gce.h, mapping to the register of display
>        function block.
>      items:
>        minItems: 28
>        maxItems: 28
>        items:                     <----- this block doesn't seem to get checked :\
>          - description: phandle of GCE
>          - description: GCE subsys id
>          - description: register offset
>          - description: register size

This is what we would like to have but it requires exception in
dtschema. Thus:

> 
> 
> ### Example 2: Don't care about constraining the number of arguments
> 
>    mediatek,gce-client-reg:
>      $ref: /schemas/types.yaml#/definitions/phandle-array
>      maxItems: 1
>      description: The register of display function block to be set by gce.
>        There are 4 arguments in this property, gce node, subsys id, offset and
>        register size. The subsys id is defined in the gce header of each chips
>        include/dt-bindings/gce/<chip>-gce.h, mapping to the register of display
>        function block.

use this.

Best regards,
Krzysztof

Re: [PATCH v29 1/7] dt-bindings: mediatek: add ethdr definition for mt8195

[Nancy Lin (林欣螢)]

On Thu, 2023-03-16 at 12:36 +0100, Krzysztof Kozlowski wrote:
> On 16/03/2023 10:53, AngeloGioacchino Del Regno wrote:
> 
> > Hello Krzysztof, Nancy,
> > 
> > Since this series has reached v29, can we please reach an agreement
> > on the bindings
> > to use here, so that we can get this finally upstreamed?
> > 
> > I will put some examples to try to get this issue resolved.
> > 
> > ### Example 1: Constrain the number of GCE entries to *seven* array
> > elements (7x4!)
> > 
> >    mediatek,gce-client-reg:
> >      $ref: /schemas/types.yaml#/definitions/phandle-array
> >      maxItems: 1
> >      description: The register of display function block to be set
> > by gce.
> >        There are 4 arguments in this property, gce node, subsys id,
> > offset and
> >        register size. The subsys id is defined in the gce header of
> > each chips
> >        include/dt-bindings/gce/<chip>-gce.h, mapping to the
> > register of display
> >        function block.
> >      items:
> >        minItems: 28
> >        maxItems: 28
> >        items:                     <----- this block doesn't seem to
> > get checked :\
> >          - description: phandle of GCE
> >          - description: GCE subsys id
> >          - description: register offset
> >          - description: register size
> 
> This is what we would like to have but it requires exception in
> dtschema. Thus:
> 
> > 
> > 
> > ### Example 2: Don't care about constraining the number of
> > arguments
> > 
> >    mediatek,gce-client-reg:
> >      $ref: /schemas/types.yaml#/definitions/phandle-array
> >      maxItems: 1
> >      description: The register of display function block to be set
> > by gce.
> >        There are 4 arguments in this property, gce node, subsys id,
> > offset and
> >        register size. The subsys id is defined in the gce header of
> > each chips
> >        include/dt-bindings/gce/<chip>-gce.h, mapping to the
> > register of display
> >        function block.
> 
> use this.
> 
> Best regards,
> Krzysztof


Hi Krzysztof, Angelo,

Thanks for the comment.
The Example 2 can pass dt_binding_check. 

But the example in the binding has 7 items [1] and dts [2]. Does the
"maxItems: 1" affect any other schema or dts check? 

[1]
+    mediatek,gce-client-reg = <&gce0 SUBSYS_1c11XXXX 0x4000 0x1000>,
+                              <&gce0 SUBSYS_1c11XXXX 0x5000 0x1000>,
+                              <&gce0 SUBSYS_1c11XXXX 0x7000 0x1000>,
+                              <&gce0 SUBSYS_1c11XXXX 0x9000 0x1000>,
+                              <&gce0 SUBSYS_1c11XXXX 0xa000 0x1000>,
+                              <&gce0 SUBSYS_1c11XXXX 0xb000 0x1000>,
+                              <&gce0 SUBSYS_1c11XXXX 0xc000 0x1000>;
> 

[2] [v21,25/25] arm64: dts: mt8195: add display node for vdosys1

https://patchwork.kernel.org/project/linux-mediatek/patch/20220504091440.2052-26-nancy.lin@mediatek.com/

Regards,
Nancy

Re: [PATCH v29 1/7] dt-bindings: mediatek: add ethdr definition for mt8195

[Krzysztof Kozlowski]

On 17/03/2023 08:55, Nancy Lin (林欣螢) wrote:
> On Thu, 2023-03-16 at 12:36 +0100, Krzysztof Kozlowski wrote:
>> On 16/03/2023 10:53, AngeloGioacchino Del Regno wrote:
>>
>>> Hello Krzysztof, Nancy,
>>>
>>> Since this series has reached v29, can we please reach an agreement
>>> on the bindings
>>> to use here, so that we can get this finally upstreamed?
>>>
>>> I will put some examples to try to get this issue resolved.
>>>
>>> ### Example 1: Constrain the number of GCE entries to *seven* array
>>> elements (7x4!)
>>>
>>>    mediatek,gce-client-reg:
>>>      $ref: /schemas/types.yaml#/definitions/phandle-array
>>>      maxItems: 1
>>>      description: The register of display function block to be set
>>> by gce.
>>>        There are 4 arguments in this property, gce node, subsys id,
>>> offset and
>>>        register size. The subsys id is defined in the gce header of
>>> each chips
>>>        include/dt-bindings/gce/<chip>-gce.h, mapping to the
>>> register of display
>>>        function block.
>>>      items:
>>>        minItems: 28
>>>        maxItems: 28
>>>        items:                     <----- this block doesn't seem to
>>> get checked :\
>>>          - description: phandle of GCE
>>>          - description: GCE subsys id
>>>          - description: register offset
>>>          - description: register size
>>
>> This is what we would like to have but it requires exception in
>> dtschema. Thus:
>>
>>>
>>>
>>> ### Example 2: Don't care about constraining the number of
>>> arguments
>>>
>>>    mediatek,gce-client-reg:
>>>      $ref: /schemas/types.yaml#/definitions/phandle-array
>>>      maxItems: 1
>>>      description: The register of display function block to be set
>>> by gce.
>>>        There are 4 arguments in this property, gce node, subsys id,
>>> offset and
>>>        register size. The subsys id is defined in the gce header of
>>> each chips
>>>        include/dt-bindings/gce/<chip>-gce.h, mapping to the
>>> register of display
>>>        function block.
>>
>> use this.
>>
>> Best regards,
>> Krzysztof
> 
> 
> Hi Krzysztof, Angelo,
> 
> Thanks for the comment.
> The Example 2 can pass dt_binding_check. 
> 
> But the example in the binding has 7 items [1] and dts [2]. Does the
> "maxItems: 1" affect any other schema or dts check? 

Ah, then it should be maxItems: 7, not 1.

Best regards,
Krzysztof

Re: [PATCH v29 1/7] dt-bindings: mediatek: add ethdr definition for mt8195

[AngeloGioacchino Del Regno]

Il 17/03/23 10:03, Krzysztof Kozlowski ha scritto:
> On 17/03/2023 08:55, Nancy Lin (林欣螢) wrote:
>> On Thu, 2023-03-16 at 12:36 +0100, Krzysztof Kozlowski wrote:
>>> On 16/03/2023 10:53, AngeloGioacchino Del Regno wrote:
>>>
>>>> Hello Krzysztof, Nancy,
>>>>
>>>> Since this series has reached v29, can we please reach an agreement
>>>> on the bindings
>>>> to use here, so that we can get this finally upstreamed?
>>>>
>>>> I will put some examples to try to get this issue resolved.
>>>>
>>>> ### Example 1: Constrain the number of GCE entries to *seven* array
>>>> elements (7x4!)
>>>>
>>>>     mediatek,gce-client-reg:
>>>>       $ref: /schemas/types.yaml#/definitions/phandle-array
>>>>       maxItems: 1
>>>>       description: The register of display function block to be set
>>>> by gce.
>>>>         There are 4 arguments in this property, gce node, subsys id,
>>>> offset and
>>>>         register size. The subsys id is defined in the gce header of
>>>> each chips
>>>>         include/dt-bindings/gce/<chip>-gce.h, mapping to the
>>>> register of display
>>>>         function block.
>>>>       items:
>>>>         minItems: 28
>>>>         maxItems: 28
>>>>         items:                     <----- this block doesn't seem to
>>>> get checked :\
>>>>           - description: phandle of GCE
>>>>           - description: GCE subsys id
>>>>           - description: register offset
>>>>           - description: register size
>>>
>>> This is what we would like to have but it requires exception in
>>> dtschema. Thus:
>>>
>>>>
>>>>
>>>> ### Example 2: Don't care about constraining the number of
>>>> arguments
>>>>
>>>>     mediatek,gce-client-reg:
>>>>       $ref: /schemas/types.yaml#/definitions/phandle-array
>>>>       maxItems: 1
>>>>       description: The register of display function block to be set
>>>> by gce.
>>>>         There are 4 arguments in this property, gce node, subsys id,
>>>> offset and
>>>>         register size. The subsys id is defined in the gce header of
>>>> each chips
>>>>         include/dt-bindings/gce/<chip>-gce.h, mapping to the
>>>> register of display
>>>>         function block.
>>>
>>> use this.
>>>
>>> Best regards,
>>> Krzysztof
>>
>>
>> Hi Krzysztof, Angelo,
>>
>> Thanks for the comment.
>> The Example 2 can pass dt_binding_check.
>>
>> But the example in the binding has 7 items [1] and dts [2]. Does the
>> "maxItems: 1" affect any other schema or dts check?
> 
> Ah, then it should be maxItems: 7, not 1.
> 

Keep in mind for your v30:

maxItems: 7 will pass - but only if minItems is *not* 7 :-)

-> (so, do not declare minItems, as default is 1) <-

Regards,
Angelo

> Best regards,
> Krzysztof
> 

Re: [PATCH v29 1/7] dt-bindings: mediatek: add ethdr definition for mt8195

[Nancy Lin (林欣螢)]

On Fri, 2023-03-17 at 10:37 +0100, AngeloGioacchino Del Regno wrote:
> Il 17/03/23 10:03, Krzysztof Kozlowski ha scritto:
> > On 17/03/2023 08:55, Nancy Lin (林欣螢) wrote:
> > > On Thu, 2023-03-16 at 12:36 +0100, Krzysztof Kozlowski wrote:
> > > > On 16/03/2023 10:53, AngeloGioacchino Del Regno wrote:
> > > > 
> > > > > Hello Krzysztof, Nancy,
> > > > > 
> > > > > Since this series has reached v29, can we please reach an
> > > > > agreement
> > > > > on the bindings
> > > > > to use here, so that we can get this finally upstreamed?
> > > > > 
> > > > > I will put some examples to try to get this issue resolved.
> > > > > 
> > > > > ### Example 1: Constrain the number of GCE entries to *seven*
> > > > > array
> > > > > elements (7x4!)
> > > > > 
> > > > >     mediatek,gce-client-reg:
> > > > >       $ref: /schemas/types.yaml#/definitions/phandle-array
> > > > >       maxItems: 1
> > > > >       description: The register of display function block to
> > > > > be set
> > > > > by gce.
> > > > >         There are 4 arguments in this property, gce node,
> > > > > subsys id,
> > > > > offset and
> > > > >         register size. The subsys id is defined in the gce
> > > > > header of
> > > > > each chips
> > > > >         include/dt-bindings/gce/<chip>-gce.h, mapping to the
> > > > > register of display
> > > > >         function block.
> > > > >       items:
> > > > >         minItems: 28
> > > > >         maxItems: 28
> > > > >         items:                     <----- this block doesn't
> > > > > seem to
> > > > > get checked :\
> > > > >           - description: phandle of GCE
> > > > >           - description: GCE subsys id
> > > > >           - description: register offset
> > > > >           - description: register size
> > > > 
> > > > This is what we would like to have but it requires exception in
> > > > dtschema. Thus:
> > > > 
> > > > > 
> > > > > 
> > > > > ### Example 2: Don't care about constraining the number of
> > > > > arguments
> > > > > 
> > > > >     mediatek,gce-client-reg:
> > > > >       $ref: /schemas/types.yaml#/definitions/phandle-array
> > > > >       maxItems: 1
> > > > >       description: The register of display function block to
> > > > > be set
> > > > > by gce.
> > > > >         There are 4 arguments in this property, gce node,
> > > > > subsys id,
> > > > > offset and
> > > > >         register size. The subsys id is defined in the gce
> > > > > header of
> > > > > each chips
> > > > >         include/dt-bindings/gce/<chip>-gce.h, mapping to the
> > > > > register of display
> > > > >         function block.
> > > > 
> > > > use this.
> > > > 
> > > > Best regards,
> > > > Krzysztof
> > > 
> > > 
> > > Hi Krzysztof, Angelo,
> > > 
> > > Thanks for the comment.
> > > The Example 2 can pass dt_binding_check.
> > > 
> > > But the example in the binding has 7 items [1] and dts [2]. Does
> > > the
> > > "maxItems: 1" affect any other schema or dts check?
> > 
> > Ah, then it should be maxItems: 7, not 1.
> > 
> 
> Keep in mind for your v30:
> 
> maxItems: 7 will pass - but only if minItems is *not* 7 :-)
> 
> -> (so, do not declare minItems, as default is 1) <-
> 
> Regards,
> Angelo
> 
Hi Angelo,

I still have one message [1] when runing dt_binding_check for "example
2 + maxItems: 7" [2].

[1]
/proj/mtk19347/cros/src/third_party/kernel/v5.10/Documentation/devicetr
ee/bindings/display/mediatek/mediatek,ethdr.example.dtb: 
hdr-engine@1c114000: mediatek,gce-client-reg: [[4294967295, 7, 16384,
4096, 4294967295, 7, 20480, 4096, 4294967295, 7, 28672, 4096,
4294967295, 7, 36864, 4096, 4294967295, 7, 40960, 4096, 4294967295, 7,
45056, 4096, 4294967295, 7, 49152, 4096]] is too short


[2]
   mediatek,gce-client-reg:
     $ref: /schemas/types.yaml#/definitions/phandle-array
     maxItems: 7
     description: The register of display function block to be set by
gce.
       There are 4 arguments in this property, gce node, subsys id,
offset and
       register size. The subsys id is defined in the gce header of
each chips
       include/dt-bindings/gce/<chip>-gce.h, mapping to the register of
display
       function block.

Regards,
Nancy


> > Best regards,
> > Krzysztof
> > 

Re: [PATCH v29 1/7] dt-bindings: mediatek: add ethdr definition for mt8195

[AngeloGioacchino Del Regno]

Il 17/03/23 10:52, Nancy Lin (林欣螢) ha scritto:
> On Fri, 2023-03-17 at 10:37 +0100, AngeloGioacchino Del Regno wrote:
>> Il 17/03/23 10:03, Krzysztof Kozlowski ha scritto:
>>> On 17/03/2023 08:55, Nancy Lin (林欣螢) wrote:
>>>> On Thu, 2023-03-16 at 12:36 +0100, Krzysztof Kozlowski wrote:
>>>>> On 16/03/2023 10:53, AngeloGioacchino Del Regno wrote:
>>>>>
>>>>>> Hello Krzysztof, Nancy,
>>>>>>
>>>>>> Since this series has reached v29, can we please reach an
>>>>>> agreement
>>>>>> on the bindings
>>>>>> to use here, so that we can get this finally upstreamed?
>>>>>>
>>>>>> I will put some examples to try to get this issue resolved.
>>>>>>
>>>>>> ### Example 1: Constrain the number of GCE entries to *seven*
>>>>>> array
>>>>>> elements (7x4!)
>>>>>>
>>>>>>      mediatek,gce-client-reg:
>>>>>>        $ref: /schemas/types.yaml#/definitions/phandle-array
>>>>>>        maxItems: 1
>>>>>>        description: The register of display function block to
>>>>>> be set
>>>>>> by gce.
>>>>>>          There are 4 arguments in this property, gce node,
>>>>>> subsys id,
>>>>>> offset and
>>>>>>          register size. The subsys id is defined in the gce
>>>>>> header of
>>>>>> each chips
>>>>>>          include/dt-bindings/gce/<chip>-gce.h, mapping to the
>>>>>> register of display
>>>>>>          function block.
>>>>>>        items:
>>>>>>          minItems: 28
>>>>>>          maxItems: 28
>>>>>>          items:                     <----- this block doesn't
>>>>>> seem to
>>>>>> get checked :\
>>>>>>            - description: phandle of GCE
>>>>>>            - description: GCE subsys id
>>>>>>            - description: register offset
>>>>>>            - description: register size
>>>>>
>>>>> This is what we would like to have but it requires exception in
>>>>> dtschema. Thus:
>>>>>
>>>>>>
>>>>>>
>>>>>> ### Example 2: Don't care about constraining the number of
>>>>>> arguments
>>>>>>
>>>>>>      mediatek,gce-client-reg:
>>>>>>        $ref: /schemas/types.yaml#/definitions/phandle-array
>>>>>>        maxItems: 1
>>>>>>        description: The register of display function block to
>>>>>> be set
>>>>>> by gce.
>>>>>>          There are 4 arguments in this property, gce node,
>>>>>> subsys id,
>>>>>> offset and
>>>>>>          register size. The subsys id is defined in the gce
>>>>>> header of
>>>>>> each chips
>>>>>>          include/dt-bindings/gce/<chip>-gce.h, mapping to the
>>>>>> register of display
>>>>>>          function block.
>>>>>
>>>>> use this.
>>>>>
>>>>> Best regards,
>>>>> Krzysztof
>>>>
>>>>
>>>> Hi Krzysztof, Angelo,
>>>>
>>>> Thanks for the comment.
>>>> The Example 2 can pass dt_binding_check.
>>>>
>>>> But the example in the binding has 7 items [1] and dts [2]. Does
>>>> the
>>>> "maxItems: 1" affect any other schema or dts check?
>>>
>>> Ah, then it should be maxItems: 7, not 1.
>>>
>>
>> Keep in mind for your v30:
>>
>> maxItems: 7 will pass - but only if minItems is *not* 7 :-)
>>
>> -> (so, do not declare minItems, as default is 1) <-
>>
>> Regards,
>> Angelo
>>
> Hi Angelo,
> 
> I still have one message [1] when runing dt_binding_check for "example
> 2 + maxItems: 7" [2].
> 
> [1]
> /proj/mtk19347/cros/src/third_party/kernel/v5.10/Documentation/devicetr
> ee/bindings/display/mediatek/mediatek,ethdr.example.dtb:
> hdr-engine@1c114000: mediatek,gce-client-reg: [[4294967295, 7, 16384,
> 4096, 4294967295, 7, 20480, 4096, 4294967295, 7, 28672, 4096,
> 4294967295, 7, 36864, 4096, 4294967295, 7, 40960, 4096, 4294967295, 7,
> 45056, 4096, 4294967295, 7, 49152, 4096]] is too short
> 
> 
> [2]
>     mediatek,gce-client-reg:
>       $ref: /schemas/types.yaml#/definitions/phandle-array
>       maxItems: 7
>       description: The register of display function block to be set by
> gce.
>         There are 4 arguments in this property, gce node, subsys id,
> offset and
>         register size. The subsys id is defined in the gce header of
> each chips
>         include/dt-bindings/gce/<chip>-gce.h, mapping to the register of
> display
>         function block.
> 

Maybe I'm wrong about the "do not declare minItems"... try with

minItems: 1
maxItems: 7


...does it work now?

> Regards,
> Nancy
> 
> 
>>> Best regards,
>>> Krzysztof
>>>

Re: [PATCH v29 1/7] dt-bindings: mediatek: add ethdr definition for mt8195

[Nancy Lin (林欣螢)]

Dear Angelo,

Sorry for late reply.

On Fri, 2023-03-17 at 10:58 +0100, AngeloGioacchino Del Regno wrote:
> Il 17/03/23 10:52, Nancy Lin (林欣螢) ha scritto:
> > On Fri, 2023-03-17 at 10:37 +0100, AngeloGioacchino Del Regno
> > wrote:
> > > Il 17/03/23 10:03, Krzysztof Kozlowski ha scritto:
> > > > On 17/03/2023 08:55, Nancy Lin (林欣螢) wrote:
> > > > > On Thu, 2023-03-16 at 12:36 +0100, Krzysztof Kozlowski wrote:
> > > > > > On 16/03/2023 10:53, AngeloGioacchino Del Regno wrote:
> > > > > > 
> > > > > > > Hello Krzysztof, Nancy,
> > > > > > > 
> > > > > > > Since this series has reached v29, can we please reach an
> > > > > > > agreement
> > > > > > > on the bindings
> > > > > > > to use here, so that we can get this finally upstreamed?
> > > > > > > 
> > > > > > > I will put some examples to try to get this issue
> > > > > > > resolved.
> > > > > > > 
> > > > > > > ### Example 1: Constrain the number of GCE entries to
> > > > > > > *seven*
> > > > > > > array
> > > > > > > elements (7x4!)
> > > > > > > 
> > > > > > >      mediatek,gce-client-reg:
> > > > > > >        $ref: /schemas/types.yaml#/definitions/phandle-
> > > > > > > array
> > > > > > >        maxItems: 1
> > > > > > >        description: The register of display function
> > > > > > > block to
> > > > > > > be set
> > > > > > > by gce.
> > > > > > >          There are 4 arguments in this property, gce
> > > > > > > node,
> > > > > > > subsys id,
> > > > > > > offset and
> > > > > > >          register size. The subsys id is defined in the
> > > > > > > gce
> > > > > > > header of
> > > > > > > each chips
> > > > > > >          include/dt-bindings/gce/<chip>-gce.h, mapping to
> > > > > > > the
> > > > > > > register of display
> > > > > > >          function block.
> > > > > > >        items:
> > > > > > >          minItems: 28
> > > > > > >          maxItems: 28
> > > > > > >          items:                     <----- this block
> > > > > > > doesn't
> > > > > > > seem to
> > > > > > > get checked :\
> > > > > > >            - description: phandle of GCE
> > > > > > >            - description: GCE subsys id
> > > > > > >            - description: register offset
> > > > > > >            - description: register size
> > > > > > 
> > > > > > This is what we would like to have but it requires
> > > > > > exception in
> > > > > > dtschema. Thus:
> > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > ### Example 2: Don't care about constraining the number
> > > > > > > of
> > > > > > > arguments
> > > > > > > 
> > > > > > >      mediatek,gce-client-reg:
> > > > > > >        $ref: /schemas/types.yaml#/definitions/phandle-
> > > > > > > array
> > > > > > >        maxItems: 1
> > > > > > >        description: The register of display function
> > > > > > > block to
> > > > > > > be set
> > > > > > > by gce.
> > > > > > >          There are 4 arguments in this property, gce
> > > > > > > node,
> > > > > > > subsys id,
> > > > > > > offset and
> > > > > > >          register size. The subsys id is defined in the
> > > > > > > gce
> > > > > > > header of
> > > > > > > each chips
> > > > > > >          include/dt-bindings/gce/<chip>-gce.h, mapping to
> > > > > > > the
> > > > > > > register of display
> > > > > > >          function block.
> > > > > > 
> > > > > > use this.
> > > > > > 
> > > > > > Best regards,
> > > > > > Krzysztof
> > > > > 
> > > > > 
> > > > > Hi Krzysztof, Angelo,
> > > > > 
> > > > > Thanks for the comment.
> > > > > The Example 2 can pass dt_binding_check.
> > > > > 
> > > > > But the example in the binding has 7 items [1] and dts [2].
> > > > > Does
> > > > > the
> > > > > "maxItems: 1" affect any other schema or dts check?
> > > > 
> > > > Ah, then it should be maxItems: 7, not 1.
> > > > 
> > > 
> > > Keep in mind for your v30:
> > > 
> > > maxItems: 7 will pass - but only if minItems is *not* 7 :-)
> > > 
> > > -> (so, do not declare minItems, as default is 1) <-
> > > 
> > > Regards,
> > > Angelo
> > > 
> > 
> > Hi Angelo,
> > 
> > I still have one message [1] when runing dt_binding_check for
> > "example
> > 2 + maxItems: 7" [2].
> > 
> > [1]
> > /proj/mtk19347/cros/src/third_party/kernel/v5.10/Documentation/devi
> > cetr
> > ee/bindings/display/mediatek/mediatek,ethdr.example.dtb:
> > hdr-engine@1c114000: mediatek,gce-client-reg: [[4294967295, 7,
> > 16384,
> > 4096, 4294967295, 7, 20480, 4096, 4294967295, 7, 28672, 4096,
> > 4294967295, 7, 36864, 4096, 4294967295, 7, 40960, 4096, 4294967295,
> > 7,
> > 45056, 4096, 4294967295, 7, 49152, 4096]] is too short
> > 
> > 
> > [2]
> >     mediatek,gce-client-reg:
> >       $ref: /schemas/types.yaml#/definitions/phandle-array
> >       maxItems: 7
> >       description: The register of display function block to be set
> > by
> > gce.
> >         There are 4 arguments in this property, gce node, subsys
> > id,
> > offset and
> >         register size. The subsys id is defined in the gce header
> > of
> > each chips
> >         include/dt-bindings/gce/<chip>-gce.h, mapping to the
> > register of
> > display
> >         function block.
> > 
> 
> Maybe I'm wrong about the "do not declare minItems"... try with
> 
> minItems: 1
> maxItems: 7
> 
> 
> ...does it work now?
> 

Yes, It works well with "example2 + minItems:1 + maxItems: 7" [1]

[1]
  mediatek,gce-client-reg:
    $ref: /schemas/types.yaml#/definitions/phandle-array
    minItems: 1
    maxItems: 7
    description: The register of display function block to be set by
gce.
      There are 4 arguments in this property, gce node, subsys id,
offset and
      register size. The subsys id is defined in the gce header of each
chips
      include/dt-bindings/gce/<chip>-gce.h, mapping to the register of
display
      function block.

Regards,
Nancy


> > Regards,
> > Nancy
> > 
> > 
> > > > Best regards,
> > > > Krzysztof
> > > > 
> 
> 
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v29 1/7] dt-bindings: mediatek: add ethdr definition for mt8195

[AngeloGioacchino Del Regno]

Il 21/03/23 06:33, Nancy Lin (林欣螢) ha scritto:
> Dear Angelo,
> 
> Sorry for late reply.
> 
> On Fri, 2023-03-17 at 10:58 +0100, AngeloGioacchino Del Regno wrote:
>> Il 17/03/23 10:52, Nancy Lin (林欣螢) ha scritto:
>>> On Fri, 2023-03-17 at 10:37 +0100, AngeloGioacchino Del Regno
>>> wrote:
>>>> Il 17/03/23 10:03, Krzysztof Kozlowski ha scritto:
>>>>> On 17/03/2023 08:55, Nancy Lin (林欣螢) wrote:
>>>>>> On Thu, 2023-03-16 at 12:36 +0100, Krzysztof Kozlowski wrote:
>>>>>>> On 16/03/2023 10:53, AngeloGioacchino Del Regno wrote:
>>>>>>>
>>>>>>>> Hello Krzysztof, Nancy,
>>>>>>>>
>>>>>>>> Since this series has reached v29, can we please reach an
>>>>>>>> agreement
>>>>>>>> on the bindings
>>>>>>>> to use here, so that we can get this finally upstreamed?
>>>>>>>>
>>>>>>>> I will put some examples to try to get this issue
>>>>>>>> resolved.
>>>>>>>>
>>>>>>>> ### Example 1: Constrain the number of GCE entries to
>>>>>>>> *seven*
>>>>>>>> array
>>>>>>>> elements (7x4!)
>>>>>>>>
>>>>>>>>       mediatek,gce-client-reg:
>>>>>>>>         $ref: /schemas/types.yaml#/definitions/phandle-
>>>>>>>> array
>>>>>>>>         maxItems: 1
>>>>>>>>         description: The register of display function
>>>>>>>> block to
>>>>>>>> be set
>>>>>>>> by gce.
>>>>>>>>           There are 4 arguments in this property, gce
>>>>>>>> node,
>>>>>>>> subsys id,
>>>>>>>> offset and
>>>>>>>>           register size. The subsys id is defined in the
>>>>>>>> gce
>>>>>>>> header of
>>>>>>>> each chips
>>>>>>>>           include/dt-bindings/gce/<chip>-gce.h, mapping to
>>>>>>>> the
>>>>>>>> register of display
>>>>>>>>           function block.
>>>>>>>>         items:
>>>>>>>>           minItems: 28
>>>>>>>>           maxItems: 28
>>>>>>>>           items:                     <----- this block
>>>>>>>> doesn't
>>>>>>>> seem to
>>>>>>>> get checked :\
>>>>>>>>             - description: phandle of GCE
>>>>>>>>             - description: GCE subsys id
>>>>>>>>             - description: register offset
>>>>>>>>             - description: register size
>>>>>>>
>>>>>>> This is what we would like to have but it requires
>>>>>>> exception in
>>>>>>> dtschema. Thus:
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> ### Example 2: Don't care about constraining the number
>>>>>>>> of
>>>>>>>> arguments
>>>>>>>>
>>>>>>>>       mediatek,gce-client-reg:
>>>>>>>>         $ref: /schemas/types.yaml#/definitions/phandle-
>>>>>>>> array
>>>>>>>>         maxItems: 1
>>>>>>>>         description: The register of display function
>>>>>>>> block to
>>>>>>>> be set
>>>>>>>> by gce.
>>>>>>>>           There are 4 arguments in this property, gce
>>>>>>>> node,
>>>>>>>> subsys id,
>>>>>>>> offset and
>>>>>>>>           register size. The subsys id is defined in the
>>>>>>>> gce
>>>>>>>> header of
>>>>>>>> each chips
>>>>>>>>           include/dt-bindings/gce/<chip>-gce.h, mapping to
>>>>>>>> the
>>>>>>>> register of display
>>>>>>>>           function block.
>>>>>>>
>>>>>>> use this.
>>>>>>>
>>>>>>> Best regards,
>>>>>>> Krzysztof
>>>>>>
>>>>>>
>>>>>> Hi Krzysztof, Angelo,
>>>>>>
>>>>>> Thanks for the comment.
>>>>>> The Example 2 can pass dt_binding_check.
>>>>>>
>>>>>> But the example in the binding has 7 items [1] and dts [2].
>>>>>> Does
>>>>>> the
>>>>>> "maxItems: 1" affect any other schema or dts check?
>>>>>
>>>>> Ah, then it should be maxItems: 7, not 1.
>>>>>
>>>>
>>>> Keep in mind for your v30:
>>>>
>>>> maxItems: 7 will pass - but only if minItems is *not* 7 :-)
>>>>
>>>> -> (so, do not declare minItems, as default is 1) <-
>>>>
>>>> Regards,
>>>> Angelo
>>>>
>>>
>>> Hi Angelo,
>>>
>>> I still have one message [1] when runing dt_binding_check for
>>> "example
>>> 2 + maxItems: 7" [2].
>>>
>>> [1]
>>> /proj/mtk19347/cros/src/third_party/kernel/v5.10/Documentation/devi
>>> cetr
>>> ee/bindings/display/mediatek/mediatek,ethdr.example.dtb:
>>> hdr-engine@1c114000: mediatek,gce-client-reg: [[4294967295, 7,
>>> 16384,
>>> 4096, 4294967295, 7, 20480, 4096, 4294967295, 7, 28672, 4096,
>>> 4294967295, 7, 36864, 4096, 4294967295, 7, 40960, 4096, 4294967295,
>>> 7,
>>> 45056, 4096, 4294967295, 7, 49152, 4096]] is too short
>>>
>>>
>>> [2]
>>>      mediatek,gce-client-reg:
>>>        $ref: /schemas/types.yaml#/definitions/phandle-array
>>>        maxItems: 7
>>>        description: The register of display function block to be set
>>> by
>>> gce.
>>>          There are 4 arguments in this property, gce node, subsys
>>> id,
>>> offset and
>>>          register size. The subsys id is defined in the gce header
>>> of
>>> each chips
>>>          include/dt-bindings/gce/<chip>-gce.h, mapping to the
>>> register of
>>> display
>>>          function block.
>>>
>>
>> Maybe I'm wrong about the "do not declare minItems"... try with
>>
>> minItems: 1
>> maxItems: 7
>>
>>
>> ...does it work now?
>>
> 
> Yes, It works well with "example2 + minItems:1 + maxItems: 7" [1]
> 
> [1]
>    mediatek,gce-client-reg:
>      $ref: /schemas/types.yaml#/definitions/phandle-array
>      minItems: 1
>      maxItems: 7
>      description: The register of display function block to be set by
> gce.
>        There are 4 arguments in this property, gce node, subsys id,
> offset and
>        register size. The subsys id is defined in the gce header of each
> chips
>        include/dt-bindings/gce/<chip>-gce.h, mapping to the register of
> display
>        function block.
> 

Please send a v30 with that solution ASAP then, so that we may perhaps *finally*
get it in for v6.4.

Regards,
Angelo