[PATCH v14 0/6] Add mutex support for MDP

[PATCH v14 0/6] Add mutex support for MDP

[Moudy Ho]

Change since V13:
- Rebase on linux-next tag:next-20220316
- Adjust the MUTEX MOD table structure and corresponding functions.
- Adjust the definition style about 8183 MDP MOD.
- Remove redundant definitions and enumerations.
- Adjust the CMDQ operation in MUTEX to be backward compatible

Change since V12:
- Rebase on linux-next
- Remove ISP related settings in MMSYS
- Removed CMDQ operations previously used by MDP in MMSYS
- Move mediatek MUTEX dt-binding path
- Add additional property in MUTEX for CMDQ operations

Change since V11:
- Rebase on v5.17-rc6.

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/matthias.bgg/linux.git/commit/?h=v5.17-next/soc&id=5f9b5b757e44de47ebdc116c14b90e3cc8bc7acb
[2]: https://git.kernel.org/pub/scm/linux/kernel/git/matthias.bgg/linux.git/commit/?h=v5.17-next/soc&id=831785f0e5b919c29e1bc5f9a74e9ebd38289e24
[3]: https://git.kernel.org/pub/scm/linux/kernel/git/matthias.bgg/linux.git/commit/?h=v5.17-next/soc&id=15f1768365aed810826a61fef4a744437aa5b426

Change since v10:
- For some ISP application scenarios, such as preview and recording
  at the same time.
  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 and 2 output.
- Adjust the GCE client register parsing method to avoid redundant warning logs.

Change since v9:
- Add API for MDP getting mutex mod.

Hi,

This patch splits mmsys and mutex settings from Media Data Path 3 (MDP3),
and original mailling list list below:
https://patchwork.kernel.org/project/linux-mediatek/cover/20211201095031.31606-1-moudy.ho@mediatek.com/
Corresponding settings and interfaces are added for MDP operation in the
mutex drivers, which increases the independence of the modules

Moudy Ho (6):
  soc: mediatek: mutex: add common interface to accommodate multiple
    modules operationg MUTEX
  soc: mediatek: mutex: add 8183 MUTEX MOD settings for MDP
  dt-bindings: soc: mediatek: move out common module from display folder
  dt-bindings: soc: mediatek: add gce-client-reg for MUTEX
  dts: arm64: mt8183: add GCE client property for Mediatek MUTEX
  soc: mediatek: mutex: add functions that operate registers by CMDQ

 .../mediatek/mediatek,mutex.yaml              |  10 +-
 arch/arm64/boot/dts/mediatek/mt8183.dtsi      |   1 +
 drivers/soc/mediatek/mtk-mutex.c              | 187 +++++++++++++++++-
 include/linux/soc/mediatek/mtk-mutex.h        |  23 +++
 4 files changed, 219 insertions(+), 2 deletions(-)
 rename Documentation/devicetree/bindings/{display => soc}/mediatek/mediatek,mutex.yaml (83%)

-- 
2.18.0


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

[PATCH v14 1/6] soc: mediatek: mutex: add common interface to accommodate multiple modules operationg MUTEX

[Moudy Ho]

In order to allow multiple modules to operate MUTEX hardware through
a common interfrace, a flexible index "mtk_mutex_table_index" needs to
be added to replace original component ID so that like DDP and MDP
can add their own MUTEX table settings independently.

In addition, 4 generic interface "mtk_mutex_set_mod", "mtk_mutex_set_sof",
"mtk_mutex_clear_mod" and "mtk_mutex_clear_sof" have been added, which is
expected to replace the "mtk_mutex_add_comp" and "mtk_mutex_remove_comp"
pair originally dedicated to DDP in the future.

Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>
---
 drivers/soc/mediatek/mtk-mutex.c       | 105 +++++++++++++++++++++++++
 include/linux/soc/mediatek/mtk-mutex.h |  21 +++++
 2 files changed, 126 insertions(+)

diff --git a/drivers/soc/mediatek/mtk-mutex.c b/drivers/soc/mediatek/mtk-mutex.c
index aaf8fc1abb43..f8c33186685a 100644
--- a/drivers/soc/mediatek/mtk-mutex.c
+++ b/drivers/soc/mediatek/mtk-mutex.c
@@ -151,11 +151,17 @@ enum mtk_mutex_sof_id {
 	MUTEX_SOF_DSI3,
 };
 
+struct mtk_mutex_mod {
+	u8 tab;
+	u32 value;
+};
+
 struct mtk_mutex_data {
 	const unsigned int *mutex_mod;
 	const unsigned int *mutex_sof;
 	const unsigned int mutex_mod_reg;
 	const unsigned int mutex_sof_reg;
+	const struct mtk_mutex_mod *mutex_table_mod;
 	const bool no_clk;
 };
 
@@ -445,6 +451,65 @@ void mtk_mutex_add_comp(struct mtk_mutex *mutex,
 }
 EXPORT_SYMBOL_GPL(mtk_mutex_add_comp);
 
+void mtk_mutex_set_mod(struct mtk_mutex *mutex,
+		       enum mtk_mutex_table_index idx)
+{
+	struct mtk_mutex_ctx *mtx = container_of(mutex, struct mtk_mutex_ctx,
+						 mutex[mutex->id]);
+	unsigned int reg;
+	unsigned int offset;
+
+	WARN_ON(&mtx->mutex[mutex->id] != mutex);
+
+	if (idx < MUTEX_TABLE_IDX_MDP_RDMA0 ||
+	    idx >= MUTEX_TABLE_IDX_MAX)
+		return;
+
+	if (mtx->data->mutex_table_mod[idx].tab)
+		offset = DISP_REG_MUTEX_MOD2(mutex->id);
+	else
+		offset = DISP_REG_MUTEX_MOD(mtx->data->mutex_mod_reg,
+					    mutex->id);
+
+	reg = readl_relaxed(mtx->regs + offset);
+	reg |= 1 << mtx->data->mutex_table_mod[idx].value;
+	writel_relaxed(reg, mtx->regs + offset);
+}
+EXPORT_SYMBOL_GPL(mtk_mutex_set_mod);
+
+void mtk_mutex_set_sof(struct mtk_mutex *mutex,
+		       enum mtk_mutex_table_index idx)
+{
+	struct mtk_mutex_ctx *mtx = container_of(mutex, struct mtk_mutex_ctx,
+						 mutex[mutex->id]);
+	unsigned int sof_id;
+
+	WARN_ON(&mtx->mutex[mutex->id] != mutex);
+
+	if (idx < MUTEX_TABLE_IDX_MDP_RDMA0 ||
+	    idx >= MUTEX_TABLE_IDX_MAX)
+		return;
+
+	switch (idx) {
+	case MUTEX_TABLE_IDX_MDP_RDMA0:
+	case MUTEX_TABLE_IDX_MDP_RSZ0:
+	case MUTEX_TABLE_IDX_MDP_RSZ1:
+	case MUTEX_TABLE_IDX_MDP_TDSHP0:
+	case MUTEX_TABLE_IDX_MDP_WROT0:
+	case MUTEX_TABLE_IDX_MDP_WDMA:
+	case MUTEX_TABLE_IDX_MDP_AAL0:
+	case MUTEX_TABLE_IDX_MDP_CCORR0:
+	default:
+		sof_id = MUTEX_SOF_SINGLE_MODE;
+		break;
+	}
+
+	writel_relaxed(mtx->data->mutex_sof[sof_id],
+		       mtx->regs +
+		       DISP_REG_MUTEX_SOF(mtx->data->mutex_sof_reg, mutex->id));
+}
+EXPORT_SYMBOL_GPL(mtk_mutex_set_sof);
+
 void mtk_mutex_remove_comp(struct mtk_mutex *mutex,
 			   enum mtk_ddp_comp_id id)
 {
@@ -485,6 +550,46 @@ void mtk_mutex_remove_comp(struct mtk_mutex *mutex,
 }
 EXPORT_SYMBOL_GPL(mtk_mutex_remove_comp);
 
+void mtk_mutex_clear_mod(struct mtk_mutex *mutex,
+			 enum mtk_mutex_table_index idx)
+{
+	struct mtk_mutex_ctx *mtx = container_of(mutex, struct mtk_mutex_ctx,
+						 mutex[mutex->id]);
+	unsigned int reg;
+	unsigned int offset;
+
+	WARN_ON(&mtx->mutex[mutex->id] != mutex);
+
+	if (idx < MUTEX_TABLE_IDX_MDP_RDMA0 ||
+	    idx >= MUTEX_TABLE_IDX_MAX)
+		return;
+
+	if (mtx->data->mutex_table_mod[idx].tab)
+		offset = DISP_REG_MUTEX_MOD2(mutex->id);
+	else
+		offset = DISP_REG_MUTEX_MOD(mtx->data->mutex_mod_reg,
+					    mutex->id);
+
+	reg = readl_relaxed(mtx->regs + offset);
+	reg &= ~(1 << mtx->data->mutex_table_mod[idx].value);
+	writel_relaxed(reg, mtx->regs + offset);
+}
+EXPORT_SYMBOL_GPL(mtk_mutex_clear_mod);
+
+void mtk_mutex_clear_sof(struct mtk_mutex *mutex)
+{
+	struct mtk_mutex_ctx *mtx = container_of(mutex, struct mtk_mutex_ctx,
+						 mutex[mutex->id]);
+
+	WARN_ON(&mtx->mutex[mutex->id] != mutex);
+
+	writel_relaxed(MUTEX_SOF_SINGLE_MODE,
+		       mtx->regs +
+		       DISP_REG_MUTEX_SOF(mtx->data->mutex_sof_reg,
+					  mutex->id));
+}
+EXPORT_SYMBOL_GPL(mtk_mutex_clear_sof);
+
 void mtk_mutex_enable(struct mtk_mutex *mutex)
 {
 	struct mtk_mutex_ctx *mtx = container_of(mutex, struct mtk_mutex_ctx,
diff --git a/include/linux/soc/mediatek/mtk-mutex.h b/include/linux/soc/mediatek/mtk-mutex.h
index 6fe4ffbde290..200f4365c950 100644
--- a/include/linux/soc/mediatek/mtk-mutex.h
+++ b/include/linux/soc/mediatek/mtk-mutex.h
@@ -10,14 +10,35 @@ struct regmap;
 struct device;
 struct mtk_mutex;
 
+enum mtk_mutex_table_index {
+	/* MDP table index */
+	MUTEX_TABLE_IDX_MDP_RDMA0,
+	MUTEX_TABLE_IDX_MDP_RSZ0,
+	MUTEX_TABLE_IDX_MDP_RSZ1,
+	MUTEX_TABLE_IDX_MDP_TDSHP0,
+	MUTEX_TABLE_IDX_MDP_WROT0,
+	MUTEX_TABLE_IDX_MDP_WDMA,
+	MUTEX_TABLE_IDX_MDP_AAL0,
+	MUTEX_TABLE_IDX_MDP_CCORR0,
+
+	MUTEX_TABLE_IDX_MAX		/* ALWAYS keep at the end */
+};
+
 struct mtk_mutex *mtk_mutex_get(struct device *dev);
 int mtk_mutex_prepare(struct mtk_mutex *mutex);
 void mtk_mutex_add_comp(struct mtk_mutex *mutex,
 			enum mtk_ddp_comp_id id);
+void mtk_mutex_set_mod(struct mtk_mutex *mutex,
+		       enum mtk_mutex_table_index idx);
+void mtk_mutex_set_sof(struct mtk_mutex *mutex,
+		       enum mtk_mutex_table_index idx);
 void mtk_mutex_enable(struct mtk_mutex *mutex);
 void mtk_mutex_disable(struct mtk_mutex *mutex);
 void mtk_mutex_remove_comp(struct mtk_mutex *mutex,
 			   enum mtk_ddp_comp_id id);
+void mtk_mutex_clear_mod(struct mtk_mutex *mutex,
+			 enum mtk_mutex_table_index idx);
+void mtk_mutex_clear_sof(struct mtk_mutex *mutex);
 void mtk_mutex_unprepare(struct mtk_mutex *mutex);
 void mtk_mutex_put(struct mtk_mutex *mutex);
 void mtk_mutex_acquire(struct mtk_mutex *mutex);
-- 
2.18.0


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

[PATCH v14 2/6] soc: mediatek: mutex: add 8183 MUTEX MOD settings for MDP

[Moudy Ho]

For the purpose of module independence, related settings should be moved
from MDP to the corresponding driver.
This patch adds 8183 MUTEX MOD settings for MDP.

Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>
---
 drivers/soc/mediatek/mtk-mutex.c | 37 ++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/drivers/soc/mediatek/mtk-mutex.c b/drivers/soc/mediatek/mtk-mutex.c
index f8c33186685a..c72e9f6ee4cc 100644
--- a/drivers/soc/mediatek/mtk-mutex.c
+++ b/drivers/soc/mediatek/mtk-mutex.c
@@ -80,6 +80,15 @@
 #define MT8183_MUTEX_MOD_DISP_GAMMA0		16
 #define MT8183_MUTEX_MOD_DISP_DITHER0		17
 
+#define MT8183_MUTEX_MOD_MDP_RDMA0		2
+#define MT8183_MUTEX_MOD_MDP_RSZ0		4
+#define MT8183_MUTEX_MOD_MDP_RSZ1		5
+#define MT8183_MUTEX_MOD_MDP_TDSHP0		6
+#define MT8183_MUTEX_MOD_MDP_WROT0		7
+#define MT8183_MUTEX_MOD_MDP_WDMA		8
+#define MT8183_MUTEX_MOD_MDP_AAL0		23
+#define MT8183_MUTEX_MOD_MDP_CCORR0		24
+
 #define MT8173_MUTEX_MOD_DISP_OVL0		11
 #define MT8173_MUTEX_MOD_DISP_OVL1		12
 #define MT8173_MUTEX_MOD_DISP_RDMA0		13
@@ -249,6 +258,33 @@ static const unsigned int mt8183_mutex_mod[DDP_COMPONENT_ID_MAX] = {
 	[DDP_COMPONENT_WDMA0] = MT8183_MUTEX_MOD_DISP_WDMA0,
 };
 
+static const struct mtk_mutex_mod mt8183_mutex_table_mod[MUTEX_TABLE_IDX_MAX] = {
+	[MUTEX_TABLE_IDX_MDP_RDMA0] = {
+		0, MT8183_MUTEX_MOD_MDP_RDMA0
+	},
+	[MUTEX_TABLE_IDX_MDP_RSZ0] = {
+		0, MT8183_MUTEX_MOD_MDP_RSZ0
+	},
+	[MUTEX_TABLE_IDX_MDP_RSZ1] = {
+		0, MT8183_MUTEX_MOD_MDP_RSZ1
+	},
+	[MUTEX_TABLE_IDX_MDP_TDSHP0] = {
+		0, MT8183_MUTEX_MOD_MDP_TDSHP0
+	},
+	[MUTEX_TABLE_IDX_MDP_WROT0] = {
+		0, MT8183_MUTEX_MOD_MDP_WROT0
+	},
+	[MUTEX_TABLE_IDX_MDP_WDMA] = {
+		0, MT8183_MUTEX_MOD_MDP_WDMA
+	},
+	[MUTEX_TABLE_IDX_MDP_AAL0] = {
+		0, MT8183_MUTEX_MOD_MDP_AAL0
+	},
+	[MUTEX_TABLE_IDX_MDP_CCORR0] = {
+		0, MT8183_MUTEX_MOD_MDP_CCORR0
+	},
+};
+
 static const unsigned int mt8186_mutex_mod[DDP_COMPONENT_ID_MAX] = {
 	[DDP_COMPONENT_AAL0] = MT8186_MUTEX_MOD_DISP_AAL0,
 	[DDP_COMPONENT_CCORR] = MT8186_MUTEX_MOD_DISP_CCORR0,
@@ -340,6 +376,7 @@ static const struct mtk_mutex_data mt8183_mutex_driver_data = {
 	.mutex_sof = mt8183_mutex_sof,
 	.mutex_mod_reg = MT8183_MUTEX0_MOD0,
 	.mutex_sof_reg = MT8183_MUTEX0_SOF0,
+	.mutex_table_mod = mt8183_mutex_table_mod,
 	.no_clk = true,
 };
 
-- 
2.18.0


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

[PATCH v14 3/6] dt-bindings: soc: mediatek: move out common module from display folder

[Moudy Ho]

In order to share the same hardware information with MDP3,
change the MUTEX dt-binding to the path "soc/mediatek".

Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>
---
 .../bindings/{display => soc}/mediatek/mediatek,mutex.yaml      | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
 rename Documentation/devicetree/bindings/{display => soc}/mediatek/mediatek,mutex.yaml (97%)

diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,mutex.yaml b/Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yaml
similarity index 97%
rename from Documentation/devicetree/bindings/display/mediatek/mediatek,mutex.yaml
rename to Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yaml
index 6eca525eced0..f825af49f820 100644
--- a/Documentation/devicetree/bindings/display/mediatek/mediatek,mutex.yaml
+++ b/Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yaml
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
 %YAML 1.2
 ---
-$id: http://devicetree.org/schemas/display/mediatek/mediatek,mutex.yaml#
+$id: http://devicetree.org/schemas/soc/mediatek/mediatek,mutex.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
 title: Mediatek mutex
-- 
2.18.0


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

[PATCH v14 4/6] dt-bindings: soc: mediatek: add gce-client-reg for MUTEX

[Moudy Ho]

In order to allow modules with latency requirements such as MDP3
to set registers through CMDQ, add the relevant GCE property.

Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Reviewed-by: CK Hu <ck.hu@mediatek.com>
---
 .../devicetree/bindings/soc/mediatek/mediatek,mutex.yaml  | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yaml b/Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yaml
index f825af49f820..a4892979480c 100644
--- a/Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yaml
+++ b/Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yaml
@@ -60,6 +60,14 @@ properties:
       include/dt-bindings/gce/<chip>-gce.h of each chips.
     $ref: /schemas/types.yaml#/definitions/phandle-array
 
+  mediatek,gce-client-reg:
+    description: The register of client driver can be configured by gce with
+      4 arguments defined in this property, such as phandle of gce, subsys id,
+      register offset and size. Each GCE subsys id is mapping to a client
+      defined in the header include/dt-bindings/gce/<chip>-gce.h.
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    maxItems: 1
+
 required:
   - compatible
   - reg
-- 
2.18.0


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

[PATCH v14 5/6] dts: arm64: mt8183: add GCE client property for Mediatek MUTEX

[Moudy Ho]

In order to allow modules with latency requirements such as MDP3
to set registers through CMDQ, add the relevant dts property.

Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8183.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index 4b08691ed39e..fc6ac2a46324 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -1514,6 +1514,7 @@
 			power-domains = <&spm MT8183_POWER_DOMAIN_DISP>;
 			mediatek,gce-events = <CMDQ_EVENT_MUTEX_STREAM_DONE0>,
 					      <CMDQ_EVENT_MUTEX_STREAM_DONE1>;
+			mediatek,gce-client-reg = <&gce SUBSYS_1401XXXX 0x6000 0x1000>;
 		};
 
 		larb0: larb@14017000 {
-- 
2.18.0


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

[PATCH v14 6/6] soc: mediatek: mutex: add functions that operate registers by CMDQ

[Moudy Ho]

Due to HW limitations, MDP3 is necessary to enable MUTEX in each frame
for SOF triggering and cooperate with CMDQ control to reduce the amount
of interrupts generated(also, reduce frame latency).

In response to the above situation, a new interface
"mtk_mutex_enable_by_cmdq" has been added to achieve the purpose.

Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>
---
 drivers/soc/mediatek/mtk-mutex.c       | 45 +++++++++++++++++++++++++-
 include/linux/soc/mediatek/mtk-mutex.h |  2 ++
 2 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/mediatek/mtk-mutex.c b/drivers/soc/mediatek/mtk-mutex.c
index c72e9f6ee4cc..32c87b8ed077 100644
--- a/drivers/soc/mediatek/mtk-mutex.c
+++ b/drivers/soc/mediatek/mtk-mutex.c
@@ -7,10 +7,14 @@
 #include <linux/iopoll.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
+#include <linux/of_address.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
 #include <linux/soc/mediatek/mtk-mmsys.h>
 #include <linux/soc/mediatek/mtk-mutex.h>
+#include <linux/soc/mediatek/mtk-cmdq.h>
+
+#define MTK_MUTEX_ENABLE			BIT(0)
 
 #define MT2701_MUTEX0_MOD0			0x2c
 #define MT2701_MUTEX0_SOF0			0x30
@@ -180,6 +184,9 @@ struct mtk_mutex_ctx {
 	void __iomem			*regs;
 	struct mtk_mutex		mutex[10];
 	const struct mtk_mutex_data	*data;
+	phys_addr_t			addr;
+	struct cmdq_client_reg		cmdq_reg;
+	bool				has_gce_client_reg;
 };
 
 static const unsigned int mt2701_mutex_mod[DDP_COMPONENT_ID_MAX] = {
@@ -638,6 +645,29 @@ void mtk_mutex_enable(struct mtk_mutex *mutex)
 }
 EXPORT_SYMBOL_GPL(mtk_mutex_enable);
 
+void mtk_mutex_enable_by_cmdq(struct mtk_mutex *mutex, void *pkt)
+{
+#if IS_REACHABLE(CONFIG_MTK_CMDQ)
+	struct mtk_mutex_ctx *mtx = container_of(mutex, struct mtk_mutex_ctx,
+						 mutex[mutex->id]);
+	struct cmdq_pkt *cmdq_pkt = (struct cmdq_pkt *)pkt;
+
+	WARN_ON(&mtx->mutex[mutex->id] != mutex);
+
+	if (!mtx->has_gce_client_reg) {
+		dev_dbg(mtx->dev, "mediatek,gce-client-reg hasn't been set in dts");
+		return;
+	}
+
+	cmdq_pkt_write_mask(cmdq_pkt, mtx->cmdq_reg.subsys,
+			    mtx->addr + DISP_REG_MUTEX_EN(mutex->id),
+			    MTK_MUTEX_ENABLE, MTK_MUTEX_ENABLE);
+#else
+	dev_dbg(mtx->dev, "Not support for enable MUTEX by CMDQ");
+#endif
+}
+EXPORT_SYMBOL_GPL(mtk_mutex_enable_by_cmdq);
+
 void mtk_mutex_disable(struct mtk_mutex *mutex)
 {
 	struct mtk_mutex_ctx *mtx = container_of(mutex, struct mtk_mutex_ctx,
@@ -676,7 +706,7 @@ static int mtk_mutex_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct mtk_mutex_ctx *mtx;
-	struct resource *regs;
+	struct resource *regs, addr;
 	int i;
 
 	mtx = devm_kzalloc(dev, sizeof(*mtx), GFP_KERNEL);
@@ -697,6 +727,19 @@ static int mtk_mutex_probe(struct platform_device *pdev)
 		}
 	}
 
+	if (of_address_to_resource(dev->of_node, 0, &addr) < 0)
+		mtx->addr = 0L;
+	else
+		mtx->addr = addr.start;
+
+#if IS_REACHABLE(CONFIG_MTK_CMDQ)
+	ret = cmdq_dev_get_client_reg(dev, &mtx->cmdq_reg, 0);
+	if (ret)
+		dev_dbg(dev, "No mediatek,gce-client-reg!\n");
+	else
+		mtx->has_gce_client_reg = true;
+#endif
+
 	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	mtx->regs = devm_ioremap_resource(dev, regs);
 	if (IS_ERR(mtx->regs)) {
diff --git a/include/linux/soc/mediatek/mtk-mutex.h b/include/linux/soc/mediatek/mtk-mutex.h
index 200f4365c950..17eea55b6809 100644
--- a/include/linux/soc/mediatek/mtk-mutex.h
+++ b/include/linux/soc/mediatek/mtk-mutex.h
@@ -33,6 +33,8 @@ void mtk_mutex_set_mod(struct mtk_mutex *mutex,
 void mtk_mutex_set_sof(struct mtk_mutex *mutex,
 		       enum mtk_mutex_table_index idx);
 void mtk_mutex_enable(struct mtk_mutex *mutex);
+void mtk_mutex_enable_by_cmdq(struct mtk_mutex *mutex,
+			      void *pkt);
 void mtk_mutex_disable(struct mtk_mutex *mutex);
 void mtk_mutex_remove_comp(struct mtk_mutex *mutex,
 			   enum mtk_ddp_comp_id id);
-- 
2.18.0


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

Re: [PATCH v14 6/6] soc: mediatek: mutex: add functions that operate registers by CMDQ

[Miles Chen]

Hi Moudy,

>  #define MT2701_MUTEX0_MOD0			0x2c
>  #define MT2701_MUTEX0_SOF0			0x30
> @@ -180,6 +184,9 @@ struct mtk_mutex_ctx {
>  	void __iomem			*regs;
>  	struct mtk_mutex		mutex[10];
>  	const struct mtk_mutex_data	*data;
> +	phys_addr_t			addr;
> +	struct cmdq_client_reg		cmdq_reg;
> +	bool				has_gce_client_reg;
>  };
>

...snip...

>  static const unsigned int mt2701_mutex_mod[DDP_COMPONENT_ID_MAX] = {
> @@ -638,6 +645,29 @@ void mtk_mutex_enable(struct mtk_mutex *mutex)
>  }
>  EXPORT_SYMBOL_GPL(mtk_mutex_enable);
>  
> +void mtk_mutex_enable_by_cmdq(struct mtk_mutex *mutex, void *pkt)
> +{
> +#if IS_REACHABLE(CONFIG_MTK_CMDQ)
> +	struct mtk_mutex_ctx *mtx = container_of(mutex, struct mtk_mutex_ctx,
> +						 mutex[mutex->id]);
> +	struct cmdq_pkt *cmdq_pkt = (struct cmdq_pkt *)pkt;
> +
> +	WARN_ON(&mtx->mutex[mutex->id] != mutex);
> +
> +	if (!mtx->has_gce_client_reg) {
> +		dev_dbg(mtx->dev, "mediatek,gce-client-reg hasn't been set in dts");
> +		return;
> +	}
> +
> +	cmdq_pkt_write_mask(cmdq_pkt, mtx->cmdq_reg.subsys,
> +			    mtx->addr + DISP_REG_MUTEX_EN(mutex->id),
> +			    MTK_MUTEX_ENABLE, MTK_MUTEX_ENABLE);

We do not check if mtx->addr is valid.

> +#else
> +	dev_dbg(mtx->dev, "Not support for enable MUTEX by CMDQ");
> +#endif
> +}
> +EXPORT_SYMBOL_GPL(mtk_mutex_enable_by_cmdq);
> +
>  void mtk_mutex_disable(struct mtk_mutex *mutex)
>  {
>  	struct mtk_mutex_ctx *mtx = container_of(mutex, struct mtk_mutex_ctx,
> @@ -676,7 +706,7 @@ static int mtk_mutex_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	struct mtk_mutex_ctx *mtx;
> -	struct resource *regs;
> +	struct resource *regs, addr;
>  	int i;
>  
>  	mtx = devm_kzalloc(dev, sizeof(*mtx), GFP_KERNEL);
> @@ -697,6 +727,19 @@ static int mtk_mutex_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> +	if (of_address_to_resource(dev->of_node, 0, &addr) < 0)
> +		mtx->addr = 0L;

addr is phys_addr_t, which means that it holds a physical address.
0 may be a valid address in some platforms, so I think
we should not set mtx->addr = 0L when of_address_to_resource fails.
Maybe probe fail by returning dev_err_probe()?

Thanks,
Miles

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

Re: [PATCH v14 6/6] soc: mediatek: mutex: add functions that operate registers by CMDQ

[moudy.ho]

On Fri, 2022-03-18 at 07:31 +0800, Miles Chen wrote:
> Hi Moudy,
> 
> >  #define MT2701_MUTEX0_MOD0			0x2c
> >  #define MT2701_MUTEX0_SOF0			0x30
> > @@ -180,6 +184,9 @@ struct mtk_mutex_ctx {
> >  	void __iomem			*regs;
> >  	struct mtk_mutex		mutex[10];
> >  	const struct mtk_mutex_data	*data;
> > +	phys_addr_t			addr;
> > +	struct cmdq_client_reg		cmdq_reg;
> > +	bool				has_gce_client_reg;
> >  };
> > 
> 
> ...snip...
> 
> >  static const unsigned int mt2701_mutex_mod[DDP_COMPONENT_ID_MAX] =
> > {
> > @@ -638,6 +645,29 @@ void mtk_mutex_enable(struct mtk_mutex *mutex)
> >  }
> >  EXPORT_SYMBOL_GPL(mtk_mutex_enable);
> >  
> > +void mtk_mutex_enable_by_cmdq(struct mtk_mutex *mutex, void *pkt)
> > +{
> > +#if IS_REACHABLE(CONFIG_MTK_CMDQ)
> > +	struct mtk_mutex_ctx *mtx = container_of(mutex, struct
> > mtk_mutex_ctx,
> > +						 mutex[mutex->id]);
> > +	struct cmdq_pkt *cmdq_pkt = (struct cmdq_pkt *)pkt;
> > +
> > +	WARN_ON(&mtx->mutex[mutex->id] != mutex);
> > +
> > +	if (!mtx->has_gce_client_reg) {
> > +		dev_dbg(mtx->dev, "mediatek,gce-client-reg hasn't been
> > set in dts");
> > +		return;
> > +	}
> > +
> > +	cmdq_pkt_write_mask(cmdq_pkt, mtx->cmdq_reg.subsys,
> > +			    mtx->addr + DISP_REG_MUTEX_EN(mutex->id),
> > +			    MTK_MUTEX_ENABLE, MTK_MUTEX_ENABLE);
> 
> We do not check if mtx->addr is valid.
> 
> > +#else
> > +	dev_dbg(mtx->dev, "Not support for enable MUTEX by CMDQ");
> > +#endif
> > +}
> > +EXPORT_SYMBOL_GPL(mtk_mutex_enable_by_cmdq);
> > +
> >  void mtk_mutex_disable(struct mtk_mutex *mutex)
> >  {
> >  	struct mtk_mutex_ctx *mtx = container_of(mutex, struct
> > mtk_mutex_ctx,
> > @@ -676,7 +706,7 @@ static int mtk_mutex_probe(struct
> > platform_device *pdev)
> >  {
> >  	struct device *dev = &pdev->dev;
> >  	struct mtk_mutex_ctx *mtx;
> > -	struct resource *regs;
> > +	struct resource *regs, addr;
> >  	int i;
> >  
> >  	mtx = devm_kzalloc(dev, sizeof(*mtx), GFP_KERNEL);
> > @@ -697,6 +727,19 @@ static int mtk_mutex_probe(struct
> > platform_device *pdev)
> >  		}
> >  	}
> >  
> > +	if (of_address_to_resource(dev->of_node, 0, &addr) < 0)
> > +		mtx->addr = 0L;
> 
> addr is phys_addr_t, which means that it holds a physical address.
> 0 may be a valid address in some platforms, so I think
> we should not set mtx->addr = 0L when of_address_to_resource fails.
> Maybe probe fail by returning dev_err_probe()?
> 
> Thanks,
> Miles

Hi Miles,

This does lead to the situation you mentioned, and sorry for the
deficient dts parsing flow. Maybe we should follow "gce-client" usage
and add a check condition when needed to avoid compatibility issues?

Thanks,
Moudy


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

Re: [PATCH v14 6/6] soc: mediatek: mutex: add functions that operate registers by CMDQ

[Miles Chen]

Hi Moudy,

>> >  		}
>> >  	}
>> >  
>> > +	if (of_address_to_resource(dev->of_node, 0, &addr) < 0)
>> > +		mtx->addr = 0L;
>> 
>> addr is phys_addr_t, which means that it holds a physical address.
>> 0 may be a valid address in some platforms, so I think
>> we should not set mtx->addr = 0L when of_address_to_resource fails.
>> Maybe probe fail by returning dev_err_probe()?
>> 
>> Thanks,
>> Miles
>
>Hi Miles,
>
>This does lead to the situation you mentioned, and sorry for the
>deficient dts parsing flow. Maybe we should follow "gce-client" usage
>and add a check condition when needed to avoid compatibility issues?
>
>Thanks,
>Moudy

According to Documentation/devicetree/bindings/display/mediatek/mediatek,mutex.yaml,
it is required to have a reg field in the mutex node. So I think we can
fail in this case.

Thanks,
Miles

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

Re: [PATCH v14 3/6] dt-bindings: soc: mediatek: move out common module from display folder

[Rob Herring]

On Thu, 17 Mar 2022 22:39:23 +0800, Moudy Ho wrote:
> In order to share the same hardware information with MDP3,
> change the MUTEX dt-binding to the path "soc/mediatek".
> 
> Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>
> ---
>  .../bindings/{display => soc}/mediatek/mediatek,mutex.yaml      | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>  rename Documentation/devicetree/bindings/{display => soc}/mediatek/mediatek,mutex.yaml (97%)
> 

Acked-by: Rob Herring <robh@kernel.org>

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

Re: [PATCH v14 4/6] dt-bindings: soc: mediatek: add gce-client-reg for MUTEX

[Rob Herring]

On Thu, Mar 17, 2022 at 10:39:24PM +0800, Moudy Ho wrote:
> In order to allow modules with latency requirements such as MDP3
> to set registers through CMDQ, add the relevant GCE property.
> 
> Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> Reviewed-by: CK Hu <ck.hu@mediatek.com>
> ---
>  .../devicetree/bindings/soc/mediatek/mediatek,mutex.yaml  | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yaml b/Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yaml
> index f825af49f820..a4892979480c 100644
> --- a/Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yaml
> +++ b/Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yaml
> @@ -60,6 +60,14 @@ properties:
>        include/dt-bindings/gce/<chip>-gce.h of each chips.
>      $ref: /schemas/types.yaml#/definitions/phandle-array
>  
> +  mediatek,gce-client-reg:
> +    description: The register of client driver can be configured by gce with
> +      4 arguments defined in this property, such as phandle of gce, subsys id,
> +      register offset and size. Each GCE subsys id is mapping to a client
> +      defined in the header include/dt-bindings/gce/<chip>-gce.h.
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    maxItems: 1

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

> +
>  required:
>    - compatible
>    - reg
> -- 
> 2.18.0
> 
> 

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

Re: [PATCH v14 1/6] soc: mediatek: mutex: add common interface to accommodate multiple modules operationg MUTEX

[CK Hu]

Hi, Moudy:

On Thu, 2022-03-17 at 22:39 +0800, Moudy Ho wrote:
> In order to allow multiple modules to operate MUTEX hardware through
> a common interfrace, a flexible index "mtk_mutex_table_index" needs
> to
> be added to replace original component ID so that like DDP and MDP
> can add their own MUTEX table settings independently.
> 
> In addition, 4 generic interface "mtk_mutex_set_mod",
> "mtk_mutex_set_sof",
> "mtk_mutex_clear_mod" and "mtk_mutex_clear_sof" have been added,
> which is
> expected to replace the "mtk_mutex_add_comp" and
> "mtk_mutex_remove_comp"
> pair originally dedicated to DDP in the future.
> 
> Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>
> ---
>  drivers/soc/mediatek/mtk-mutex.c       | 105
> +++++++++++++++++++++++++
>  include/linux/soc/mediatek/mtk-mutex.h |  21 +++++
>  2 files changed, 126 insertions(+)
> 
> diff --git a/drivers/soc/mediatek/mtk-mutex.c
> b/drivers/soc/mediatek/mtk-mutex.c
> index aaf8fc1abb43..f8c33186685a 100644
> --- a/drivers/soc/mediatek/mtk-mutex.c
> +++ b/drivers/soc/mediatek/mtk-mutex.c
> @@ -151,11 +151,17 @@ enum mtk_mutex_sof_id {
>  	MUTEX_SOF_DSI3,
>  };
>  
> +struct mtk_mutex_mod {
> +	u8 tab;

tab is always 0 in this series, so remove this.

> +	u32 value;

Without tab, this struct is not necessary.

> +};
> +
>  struct mtk_mutex_data {
>  	const unsigned int *mutex_mod;
>  	const unsigned int *mutex_sof;
>  	const unsigned int mutex_mod_reg;
>  	const unsigned int mutex_sof_reg;
> +	const struct mtk_mutex_mod *mutex_table_mod;
>  	const bool no_clk;
>  };
>  
> @@ -445,6 +451,65 @@ void mtk_mutex_add_comp(struct mtk_mutex *mutex,
>  }
>  EXPORT_SYMBOL_GPL(mtk_mutex_add_comp);
>  
> +void mtk_mutex_set_mod(struct mtk_mutex *mutex,
> +		       enum mtk_mutex_table_index idx)
> +{
> +	struct mtk_mutex_ctx *mtx = container_of(mutex, struct
> mtk_mutex_ctx,
> +						 mutex[mutex->id]);
> +	unsigned int reg;
> +	unsigned int offset;
> +
> +	WARN_ON(&mtx->mutex[mutex->id] != mutex);
> +
> +	if (idx < MUTEX_TABLE_IDX_MDP_RDMA0 ||
> +	    idx >= MUTEX_TABLE_IDX_MAX)

error message?

> +		return;
> +
> +	if (mtx->data->mutex_table_mod[idx].tab)
> +		offset = DISP_REG_MUTEX_MOD2(mutex->id);
> +	else
> +		offset = DISP_REG_MUTEX_MOD(mtx->data->mutex_mod_reg,
> +					    mutex->id);
> +
> +	reg = readl_relaxed(mtx->regs + offset);
> +	reg |= 1 << mtx->data->mutex_table_mod[idx].value;
> +	writel_relaxed(reg, mtx->regs + offset);
> +}
> +EXPORT_SYMBOL_GPL(mtk_mutex_set_mod);
> +
> +void mtk_mutex_set_sof(struct mtk_mutex *mutex,
> +		       enum mtk_mutex_table_index idx)
> +{
> +	struct mtk_mutex_ctx *mtx = container_of(mutex, struct
> mtk_mutex_ctx,
> +						 mutex[mutex->id]);
> +	unsigned int sof_id;
> +
> +	WARN_ON(&mtx->mutex[mutex->id] != mutex);
> +
> +	if (idx < MUTEX_TABLE_IDX_MDP_RDMA0 ||
> +	    idx >= MUTEX_TABLE_IDX_MAX)

Ditto.

> +		return;
> +
> +	switch (idx) {
> +	case MUTEX_TABLE_IDX_MDP_RDMA0:
> +	case MUTEX_TABLE_IDX_MDP_RSZ0:
> +	case MUTEX_TABLE_IDX_MDP_RSZ1:
> +	case MUTEX_TABLE_IDX_MDP_TDSHP0:
> +	case MUTEX_TABLE_IDX_MDP_WROT0:
> +	case MUTEX_TABLE_IDX_MDP_WDMA:
> +	case MUTEX_TABLE_IDX_MDP_AAL0:
> +	case MUTEX_TABLE_IDX_MDP_CCORR0:

Error & return?

Regards,
CK

> +	default:
> +		sof_id = MUTEX_SOF_SINGLE_MODE;
> +		break;
> +	}
> +
> +	writel_relaxed(mtx->data->mutex_sof[sof_id],
> +		       mtx->regs +
> +		       DISP_REG_MUTEX_SOF(mtx->data->mutex_sof_reg,
> mutex->id));
> +}
> +EXPORT_SYMBOL_GPL(mtk_mutex_set_sof);
> +
>  void mtk_mutex_remove_comp(struct mtk_mutex *mutex,
>  			   enum mtk_ddp_comp_id id)
>  {
> @@ -485,6 +550,46 @@ void mtk_mutex_remove_comp(struct mtk_mutex
> *mutex,
>  }
>  EXPORT_SYMBOL_GPL(mtk_mutex_remove_comp);
>  
> +void mtk_mutex_clear_mod(struct mtk_mutex *mutex,
> +			 enum mtk_mutex_table_index idx)
> +{
> +	struct mtk_mutex_ctx *mtx = container_of(mutex, struct
> mtk_mutex_ctx,
> +						 mutex[mutex->id]);
> +	unsigned int reg;
> +	unsigned int offset;
> +
> +	WARN_ON(&mtx->mutex[mutex->id] != mutex);
> +
> +	if (idx < MUTEX_TABLE_IDX_MDP_RDMA0 ||
> +	    idx >= MUTEX_TABLE_IDX_MAX)
> +		return;
> +
> +	if (mtx->data->mutex_table_mod[idx].tab)
> +		offset = DISP_REG_MUTEX_MOD2(mutex->id);
> +	else
> +		offset = DISP_REG_MUTEX_MOD(mtx->data->mutex_mod_reg,
> +					    mutex->id);
> +
> +	reg = readl_relaxed(mtx->regs + offset);
> +	reg &= ~(1 << mtx->data->mutex_table_mod[idx].value);
> +	writel_relaxed(reg, mtx->regs + offset);
> +}
> +EXPORT_SYMBOL_GPL(mtk_mutex_clear_mod);
> +
> +void mtk_mutex_clear_sof(struct mtk_mutex *mutex)
> +{
> +	struct mtk_mutex_ctx *mtx = container_of(mutex, struct
> mtk_mutex_ctx,
> +						 mutex[mutex->id]);
> +
> +	WARN_ON(&mtx->mutex[mutex->id] != mutex);
> +
> +	writel_relaxed(MUTEX_SOF_SINGLE_MODE,
> +		       mtx->regs +
> +		       DISP_REG_MUTEX_SOF(mtx->data->mutex_sof_reg,
> +					  mutex->id));
> +}
> +EXPORT_SYMBOL_GPL(mtk_mutex_clear_sof);
> +
>  void mtk_mutex_enable(struct mtk_mutex *mutex)
>  {
>  	struct mtk_mutex_ctx *mtx = container_of(mutex, struct
> mtk_mutex_ctx,
> diff --git a/include/linux/soc/mediatek/mtk-mutex.h
> b/include/linux/soc/mediatek/mtk-mutex.h
> index 6fe4ffbde290..200f4365c950 100644
> --- a/include/linux/soc/mediatek/mtk-mutex.h
> +++ b/include/linux/soc/mediatek/mtk-mutex.h
> @@ -10,14 +10,35 @@ struct regmap;
>  struct device;
>  struct mtk_mutex;
>  
> +enum mtk_mutex_table_index {
> +	/* MDP table index */
> +	MUTEX_TABLE_IDX_MDP_RDMA0,
> +	MUTEX_TABLE_IDX_MDP_RSZ0,
> +	MUTEX_TABLE_IDX_MDP_RSZ1,
> +	MUTEX_TABLE_IDX_MDP_TDSHP0,
> +	MUTEX_TABLE_IDX_MDP_WROT0,
> +	MUTEX_TABLE_IDX_MDP_WDMA,
> +	MUTEX_TABLE_IDX_MDP_AAL0,
> +	MUTEX_TABLE_IDX_MDP_CCORR0,
> +
> +	MUTEX_TABLE_IDX_MAX		/* ALWAYS keep at the end */
> +};
> +
>  struct mtk_mutex *mtk_mutex_get(struct device *dev);
>  int mtk_mutex_prepare(struct mtk_mutex *mutex);
>  void mtk_mutex_add_comp(struct mtk_mutex *mutex,
>  			enum mtk_ddp_comp_id id);
> +void mtk_mutex_set_mod(struct mtk_mutex *mutex,
> +		       enum mtk_mutex_table_index idx);
> +void mtk_mutex_set_sof(struct mtk_mutex *mutex,
> +		       enum mtk_mutex_table_index idx);
>  void mtk_mutex_enable(struct mtk_mutex *mutex);
>  void mtk_mutex_disable(struct mtk_mutex *mutex);
>  void mtk_mutex_remove_comp(struct mtk_mutex *mutex,
>  			   enum mtk_ddp_comp_id id);
> +void mtk_mutex_clear_mod(struct mtk_mutex *mutex,
> +			 enum mtk_mutex_table_index idx);
> +void mtk_mutex_clear_sof(struct mtk_mutex *mutex);
>  void mtk_mutex_unprepare(struct mtk_mutex *mutex);
>  void mtk_mutex_put(struct mtk_mutex *mutex);
>  void mtk_mutex_acquire(struct mtk_mutex *mutex);


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

Re: [PATCH v14 6/6] soc: mediatek: mutex: add functions that operate registers by CMDQ

[CK Hu]

Hi, Moudy:

On Thu, 2022-03-17 at 22:39 +0800, Moudy Ho wrote:
> Due to HW limitations, MDP3 is necessary to enable MUTEX in each
> frame
> for SOF triggering and cooperate with CMDQ control to reduce the
> amount
> of interrupts generated(also, reduce frame latency).
> 
> In response to the above situation, a new interface
> "mtk_mutex_enable_by_cmdq" has been added to achieve the purpose.
> 
> Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>
> ---
>  drivers/soc/mediatek/mtk-mutex.c       | 45
> +++++++++++++++++++++++++-
>  include/linux/soc/mediatek/mtk-mutex.h |  2 ++
>  2 files changed, 46 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/soc/mediatek/mtk-mutex.c
> b/drivers/soc/mediatek/mtk-mutex.c
> index c72e9f6ee4cc..32c87b8ed077 100644
> --- a/drivers/soc/mediatek/mtk-mutex.c
> +++ b/drivers/soc/mediatek/mtk-mutex.c
> @@ -7,10 +7,14 @@
>  #include <linux/iopoll.h>
>  #include <linux/module.h>
>  #include <linux/of_device.h>
> +#include <linux/of_address.h>
>  #include <linux/platform_device.h>
>  #include <linux/regmap.h>
>  #include <linux/soc/mediatek/mtk-mmsys.h>
>  #include <linux/soc/mediatek/mtk-mutex.h>
> +#include <linux/soc/mediatek/mtk-cmdq.h>
> +
> +#define MTK_MUTEX_ENABLE			BIT(0)

If this is necessary, separate this to another patch and apply to
mtk_mutex_enable() also.

>  
>  #define MT2701_MUTEX0_MOD0			0x2c
>  #define MT2701_MUTEX0_SOF0			0x30
> @@ -180,6 +184,9 @@ struct mtk_mutex_ctx {
>  	void __iomem			*regs;
>  	struct mtk_mutex		mutex[10];
>  	const struct mtk_mutex_data	*data;
> +	phys_addr_t			addr;
> +	struct cmdq_client_reg		cmdq_reg;
> +	bool				has_gce_client_reg;
>  };
>  
>  static const unsigned int mt2701_mutex_mod[DDP_COMPONENT_ID_MAX] = {
> @@ -638,6 +645,29 @@ void mtk_mutex_enable(struct mtk_mutex *mutex)
>  }
>  EXPORT_SYMBOL_GPL(mtk_mutex_enable);
>  
> +void mtk_mutex_enable_by_cmdq(struct mtk_mutex *mutex, void *pkt)
> +{
> +#if IS_REACHABLE(CONFIG_MTK_CMDQ)
> +	struct mtk_mutex_ctx *mtx = container_of(mutex, struct
> mtk_mutex_ctx,
> +						 mutex[mutex->id]);
> +	struct cmdq_pkt *cmdq_pkt = (struct cmdq_pkt *)pkt;
> +
> +	WARN_ON(&mtx->mutex[mutex->id] != mutex);
> +
> +	if (!mtx->has_gce_client_reg) {
> +		dev_dbg(mtx->dev, "mediatek,gce-client-reg hasn't been
> set in dts");

dev_err()

> +		return;
> +	}
> +
> +	cmdq_pkt_write_mask(cmdq_pkt, mtx->cmdq_reg.subsys,
> +			    mtx->addr + DISP_REG_MUTEX_EN(mutex->id),
+			    MTK_MUTEX_ENABLE, MTK_MUTEX_ENABLE);

According to mtk_mutex_enable(),

	cmdq_pkt_write(cmdq_pkt, mtx->cmdq_reg.subsys,
		       mtx->addr + DISP_REG_MUTEX_EN(mutex->id),
		       1);

> +#else
> +	dev_dbg(mtx->dev, "Not support for enable MUTEX by CMDQ");

dev_err()
Regards,
CK

> +#endif
> +}
> +EXPORT_SYMBOL_GPL(mtk_mutex_enable_by_cmdq);
> +
>  void mtk_mutex_disable(struct mtk_mutex *mutex)
>  {
>  	struct mtk_mutex_ctx *mtx = container_of(mutex, struct
> mtk_mutex_ctx,
> @@ -676,7 +706,7 @@ static int mtk_mutex_probe(struct platform_device
> *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	struct mtk_mutex_ctx *mtx;
> -	struct resource *regs;
> +	struct resource *regs, addr;
>  	int i;
>  
>  	mtx = devm_kzalloc(dev, sizeof(*mtx), GFP_KERNEL);
> @@ -697,6 +727,19 @@ static int mtk_mutex_probe(struct
> platform_device *pdev)
>  		}
>  	}
>  
> +	if (of_address_to_resource(dev->of_node, 0, &addr) < 0)
> +		mtx->addr = 0L;
> +	else
> +		mtx->addr = addr.start;
> +
> +#if IS_REACHABLE(CONFIG_MTK_CMDQ)
> +	ret = cmdq_dev_get_client_reg(dev, &mtx->cmdq_reg, 0);
> +	if (ret)
> +		dev_dbg(dev, "No mediatek,gce-client-reg!\n");
> +	else
> +		mtx->has_gce_client_reg = true;
> +#endif
> +
>  	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	mtx->regs = devm_ioremap_resource(dev, regs);
>  	if (IS_ERR(mtx->regs)) {
> diff --git a/include/linux/soc/mediatek/mtk-mutex.h
> b/include/linux/soc/mediatek/mtk-mutex.h
> index 200f4365c950..17eea55b6809 100644
> --- a/include/linux/soc/mediatek/mtk-mutex.h
> +++ b/include/linux/soc/mediatek/mtk-mutex.h
> @@ -33,6 +33,8 @@ void mtk_mutex_set_mod(struct mtk_mutex *mutex,
>  void mtk_mutex_set_sof(struct mtk_mutex *mutex,
>  		       enum mtk_mutex_table_index idx);
>  void mtk_mutex_enable(struct mtk_mutex *mutex);
> +void mtk_mutex_enable_by_cmdq(struct mtk_mutex *mutex,
> +			      void *pkt);
>  void mtk_mutex_disable(struct mtk_mutex *mutex);
>  void mtk_mutex_remove_comp(struct mtk_mutex *mutex,
>  			   enum mtk_ddp_comp_id id);


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

Re: [PATCH v14 3/6] dt-bindings: soc: mediatek: move out common module from display folder

[Chun-Kuang Hu]

Rob Herring <robh@kernel.org> 於 2022年3月22日 週二 上午6:28寫道：
>
> On Thu, 17 Mar 2022 22:39:23 +0800, Moudy Ho wrote:
> > In order to share the same hardware information with MDP3,
> > change the MUTEX dt-binding to the path "soc/mediatek".

Acked-by: Chun-Kuang Hu <chunkuang.hu@kernel.org>

> >
> > Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>
> > ---
> >  .../bindings/{display => soc}/mediatek/mediatek,mutex.yaml      | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >  rename Documentation/devicetree/bindings/{display => soc}/mediatek/mediatek,mutex.yaml (97%)
> >
>
> Acked-by: Rob Herring <robh@kernel.org>

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

Re: [PATCH v14 4/6] dt-bindings: soc: mediatek: add gce-client-reg for MUTEX

[moudy.ho]

On Mon, 2022-03-21 at 17:30 -0500, Rob Herring wrote:
> On Thu, Mar 17, 2022 at 10:39:24PM +0800, Moudy Ho wrote:
> > In order to allow modules with latency requirements such as MDP3
> > to set registers through CMDQ, add the relevant GCE property.
> > 
> > Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>
> > Reviewed-by: AngeloGioacchino Del Regno <
> > angelogioacchino.delregno@collabora.com>
> > Reviewed-by: CK Hu <ck.hu@mediatek.com>
> > ---
> >  .../devicetree/bindings/soc/mediatek/mediatek,mutex.yaml  | 8
> > ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yam
> > l
> > b/Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yam
> > l
> > index f825af49f820..a4892979480c 100644
> > ---
> > a/Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yam
> > l
> > +++
> > b/Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yam
> > l
> > @@ -60,6 +60,14 @@ properties:
> >        include/dt-bindings/gce/<chip>-gce.h of each chips.
> >      $ref: /schemas/types.yaml#/definitions/phandle-array
> >  
> > +  mediatek,gce-client-reg:
> > +    description: The register of client driver can be configured
> > by gce with
> > +      4 arguments defined in this property, such as phandle of
> > gce, subsys id,
> > +      register offset and size. Each GCE subsys id is mapping to a
> > client
> > +      defined in the header include/dt-bindings/gce/<chip>-gce.h.
> > +    $ref: /schemas/types.yaml#/definitions/phandle-array
> > +    maxItems: 1
> 
> items:
>   items:
>     - description: phandle of GCE
>     - description: GCE subsys id
>     - description: register offset
>     - description: register size
> 
Hi Rob,

Thanks for the review, I will follow the suggestions to make
corresponding corrections in the next version. Also, I'm sorry that I
left out the information about the dependent series that may cause YAML
checking fail.
ref: 
https://patchwork.kernel.org/project/linux-mediatek/list/?series=621899

Thanks,
Moudy
> > +
> >  required:
> >    - compatible
> >    - reg
> > -- 
> > 2.18.0
> > 
> > 


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

Re: [PATCH v14 1/6] soc: mediatek: mutex: add common interface to accommodate multiple modules operationg MUTEX

[moudy.ho]

On Tue, 2022-03-22 at 17:37 +0800, CK Hu wrote:
> Hi, Moudy:
> 

Hi CK,
Thanks for the comments.

> On Thu, 2022-03-17 at 22:39 +0800, Moudy Ho wrote:
> > In order to allow multiple modules to operate MUTEX hardware
> > through
> > a common interfrace, a flexible index "mtk_mutex_table_index" needs
> > to
> > be added to replace original component ID so that like DDP and MDP
> > can add their own MUTEX table settings independently.
> > 
> > In addition, 4 generic interface "mtk_mutex_set_mod",
> > "mtk_mutex_set_sof",
> > "mtk_mutex_clear_mod" and "mtk_mutex_clear_sof" have been added,
> > which is
> > expected to replace the "mtk_mutex_add_comp" and
> > "mtk_mutex_remove_comp"
> > pair originally dedicated to DDP in the future.
> > 
> > Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>
> > ---
> >  drivers/soc/mediatek/mtk-mutex.c       | 105
> > +++++++++++++++++++++++++
> >  include/linux/soc/mediatek/mtk-mutex.h |  21 +++++
> >  2 files changed, 126 insertions(+)
> > 
> > diff --git a/drivers/soc/mediatek/mtk-mutex.c
> > b/drivers/soc/mediatek/mtk-mutex.c
> > index aaf8fc1abb43..f8c33186685a 100644
> > --- a/drivers/soc/mediatek/mtk-mutex.c
> > +++ b/drivers/soc/mediatek/mtk-mutex.c
> > @@ -151,11 +151,17 @@ enum mtk_mutex_sof_id {
> >  	MUTEX_SOF_DSI3,
> >  };
> >  
> > +struct mtk_mutex_mod {
> > +	u8 tab;
> 
> tab is always 0 in this series, so remove this.
> 
> > +	u32 value;
> 
> Without tab, this struct is not necessary.
> 

Both will be adjusted in next version.

> > +};
> > +
> >  struct mtk_mutex_data {
> >  	const unsigned int *mutex_mod;
> >  	const unsigned int *mutex_sof;
> >  	const unsigned int mutex_mod_reg;
> >  	const unsigned int mutex_sof_reg;
> > +	const struct mtk_mutex_mod *mutex_table_mod;
> >  	const bool no_clk;
> >  };
> >  
> > @@ -445,6 +451,65 @@ void mtk_mutex_add_comp(struct mtk_mutex
> > *mutex,
> >  }
> >  EXPORT_SYMBOL_GPL(mtk_mutex_add_comp);
> >  
> > +void mtk_mutex_set_mod(struct mtk_mutex *mutex,
> > +		       enum mtk_mutex_table_index idx)
> > +{
> > +	struct mtk_mutex_ctx *mtx = container_of(mutex, struct
> > mtk_mutex_ctx,
> > +						 mutex[mutex->id]);
> > +	unsigned int reg;
> > +	unsigned int offset;
> > +
> > +	WARN_ON(&mtx->mutex[mutex->id] != mutex);
> > +
> > +	if (idx < MUTEX_TABLE_IDX_MDP_RDMA0 ||
> > +	    idx >= MUTEX_TABLE_IDX_MAX)
> 
> error message?
> 

It will be added in next version.

> > +		return;
> > +
> > +	if (mtx->data->mutex_table_mod[idx].tab)
> > +		offset = DISP_REG_MUTEX_MOD2(mutex->id);
> > +	else
> > +		offset = DISP_REG_MUTEX_MOD(mtx->data->mutex_mod_reg,
> > +					    mutex->id);
> > +
> > +	reg = readl_relaxed(mtx->regs + offset);
> > +	reg |= 1 << mtx->data->mutex_table_mod[idx].value;
> > +	writel_relaxed(reg, mtx->regs + offset);
> > +}
> > +EXPORT_SYMBOL_GPL(mtk_mutex_set_mod);
> > +
> > +void mtk_mutex_set_sof(struct mtk_mutex *mutex,
> > +		       enum mtk_mutex_table_index idx)
> > +{
> > +	struct mtk_mutex_ctx *mtx = container_of(mutex, struct
> > mtk_mutex_ctx,
> > +						 mutex[mutex->id]);
> > +	unsigned int sof_id;
> > +
> > +	WARN_ON(&mtx->mutex[mutex->id] != mutex);
> > +
> > +	if (idx < MUTEX_TABLE_IDX_MDP_RDMA0 ||
> > +	    idx >= MUTEX_TABLE_IDX_MAX)
> 
> Ditto.
> 
> > +		return;
> > +
> > +	switch (idx) {
> > +	case MUTEX_TABLE_IDX_MDP_RDMA0:
> > +	case MUTEX_TABLE_IDX_MDP_RSZ0:
> > +	case MUTEX_TABLE_IDX_MDP_RSZ1:
> > +	case MUTEX_TABLE_IDX_MDP_TDSHP0:
> > +	case MUTEX_TABLE_IDX_MDP_WROT0:
> > +	case MUTEX_TABLE_IDX_MDP_WDMA:
> > +	case MUTEX_TABLE_IDX_MDP_AAL0:
> > +	case MUTEX_TABLE_IDX_MDP_CCORR0:
> 
> Error & return?
> 
> Regards,
> CK
> 

Since single mode SOF is the only type used in the 8183 MDP3, the
switch case should be removed and represented in another way. Perhaps
we can import a new SOF tables to integrate the requirement of
different modules.

Thanks,
Moudy

> > +	default:
> > +		sof_id = MUTEX_SOF_SINGLE_MODE;
> > +		break;
> > +	}
> > +
> > +	writel_relaxed(mtx->data->mutex_sof[sof_id],
> > +		       mtx->regs +
> > +		       DISP_REG_MUTEX_SOF(mtx->data->mutex_sof_reg,
> > mutex->id));
> > +}
> > +EXPORT_SYMBOL_GPL(mtk_mutex_set_sof);
> > +
> >  void mtk_mutex_remove_comp(struct mtk_mutex *mutex,
> >  			   enum mtk_ddp_comp_id id)
> >  {
> > @@ -485,6 +550,46 @@ void mtk_mutex_remove_comp(struct mtk_mutex
> > *mutex,
> >  }
> >  EXPORT_SYMBOL_GPL(mtk_mutex_remove_comp);
> >  
> > +void mtk_mutex_clear_mod(struct mtk_mutex *mutex,
> > +			 enum mtk_mutex_table_index idx)
> > +{
> > +	struct mtk_mutex_ctx *mtx = container_of(mutex, struct
> > mtk_mutex_ctx,
> > +						 mutex[mutex->id]);
> > +	unsigned int reg;
> > +	unsigned int offset;
> > +
> > +	WARN_ON(&mtx->mutex[mutex->id] != mutex);
> > +
> > +	if (idx < MUTEX_TABLE_IDX_MDP_RDMA0 ||
> > +	    idx >= MUTEX_TABLE_IDX_MAX)
> > +		return;
> > +
> > +	if (mtx->data->mutex_table_mod[idx].tab)
> > +		offset = DISP_REG_MUTEX_MOD2(mutex->id);
> > +	else
> > +		offset = DISP_REG_MUTEX_MOD(mtx->data->mutex_mod_reg,
> > +					    mutex->id);
> > +
> > +	reg = readl_relaxed(mtx->regs + offset);
> > +	reg &= ~(1 << mtx->data->mutex_table_mod[idx].value);
> > +	writel_relaxed(reg, mtx->regs + offset);
> > +}
> > +EXPORT_SYMBOL_GPL(mtk_mutex_clear_mod);
> > +
> > +void mtk_mutex_clear_sof(struct mtk_mutex *mutex)
> > +{
> > +	struct mtk_mutex_ctx *mtx = container_of(mutex, struct
> > mtk_mutex_ctx,
> > +						 mutex[mutex->id]);
> > +
> > +	WARN_ON(&mtx->mutex[mutex->id] != mutex);
> > +
> > +	writel_relaxed(MUTEX_SOF_SINGLE_MODE,
> > +		       mtx->regs +
> > +		       DISP_REG_MUTEX_SOF(mtx->data->mutex_sof_reg,
> > +					  mutex->id));
> > +}
> > +EXPORT_SYMBOL_GPL(mtk_mutex_clear_sof);
> > +
> >  void mtk_mutex_enable(struct mtk_mutex *mutex)
> >  {
> >  	struct mtk_mutex_ctx *mtx = container_of(mutex, struct
> > mtk_mutex_ctx,
> > diff --git a/include/linux/soc/mediatek/mtk-mutex.h
> > b/include/linux/soc/mediatek/mtk-mutex.h
> > index 6fe4ffbde290..200f4365c950 100644
> > --- a/include/linux/soc/mediatek/mtk-mutex.h
> > +++ b/include/linux/soc/mediatek/mtk-mutex.h
> > @@ -10,14 +10,35 @@ struct regmap;
> >  struct device;
> >  struct mtk_mutex;
> >  
> > +enum mtk_mutex_table_index {
> > +	/* MDP table index */
> > +	MUTEX_TABLE_IDX_MDP_RDMA0,
> > +	MUTEX_TABLE_IDX_MDP_RSZ0,
> > +	MUTEX_TABLE_IDX_MDP_RSZ1,
> > +	MUTEX_TABLE_IDX_MDP_TDSHP0,
> > +	MUTEX_TABLE_IDX_MDP_WROT0,
> > +	MUTEX_TABLE_IDX_MDP_WDMA,
> > +	MUTEX_TABLE_IDX_MDP_AAL0,
> > +	MUTEX_TABLE_IDX_MDP_CCORR0,
> > +
> > +	MUTEX_TABLE_IDX_MAX		/* ALWAYS keep at the end */
> > +};
> > +
> >  struct mtk_mutex *mtk_mutex_get(struct device *dev);
> >  int mtk_mutex_prepare(struct mtk_mutex *mutex);
> >  void mtk_mutex_add_comp(struct mtk_mutex *mutex,
> >  			enum mtk_ddp_comp_id id);
> > +void mtk_mutex_set_mod(struct mtk_mutex *mutex,
> > +		       enum mtk_mutex_table_index idx);
> > +void mtk_mutex_set_sof(struct mtk_mutex *mutex,
> > +		       enum mtk_mutex_table_index idx);
> >  void mtk_mutex_enable(struct mtk_mutex *mutex);
> >  void mtk_mutex_disable(struct mtk_mutex *mutex);
> >  void mtk_mutex_remove_comp(struct mtk_mutex *mutex,
> >  			   enum mtk_ddp_comp_id id);
> > +void mtk_mutex_clear_mod(struct mtk_mutex *mutex,
> > +			 enum mtk_mutex_table_index idx);
> > +void mtk_mutex_clear_sof(struct mtk_mutex *mutex);
> >  void mtk_mutex_unprepare(struct mtk_mutex *mutex);
> >  void mtk_mutex_put(struct mtk_mutex *mutex);
> >  void mtk_mutex_acquire(struct mtk_mutex *mutex);
> 
> 


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

Re: [PATCH v14 6/6] soc: mediatek: mutex: add functions that operate registers by CMDQ

[moudy.ho]

On Wed, 2022-04-13 at 10:41 +0200, AngeloGioacchino Del Regno wrote:
> Il 11/04/22 09:24, Moudy Ho ha scritto:
> > Due to HW limitations, MDP3 is necessary to enable MUTEX in each
> > frame
> > for SOF triggering and cooperate with CMDQ control to reduce the
> > amount
> > of interrupts generated(also, reduce frame latency).
> > 
> > In response to the above situation, a new interface
> > "mtk_mutex_enable_by_cmdq" has been added to achieve the purpose.
> > 
> > Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>
> > ---
> >   drivers/soc/mediatek/mtk-mutex.c       | 42
> > +++++++++++++++++++++++++-
> >   include/linux/soc/mediatek/mtk-mutex.h |  2 ++
> >   2 files changed, 43 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/soc/mediatek/mtk-mutex.c
> > b/drivers/soc/mediatek/mtk-mutex.c
> > index fc9ba2749946..1811beaf399d 100644
> > --- a/drivers/soc/mediatek/mtk-mutex.c
> > +++ b/drivers/soc/mediatek/mtk-mutex.c
> > @@ -7,10 +7,12 @@
> >   #include <linux/iopoll.h>
> >   #include <linux/module.h>
> >   #include <linux/of_device.h>
> > +#include <linux/of_address.h>
> >   #include <linux/platform_device.h>
> >   #include <linux/regmap.h>
> >   #include <linux/soc/mediatek/mtk-mmsys.h>
> >   #include <linux/soc/mediatek/mtk-mutex.h>
> > +#include <linux/soc/mediatek/mtk-cmdq.h>
> >   
> >   #define MT2701_MUTEX0_MOD0			0x2c
> >   #define MT2701_MUTEX0_SOF0			0x30
> > @@ -176,6 +178,9 @@ struct mtk_mutex_ctx {
> >   	void __iomem			*regs;
> >   	struct mtk_mutex		mutex[10];
> >   	const struct mtk_mutex_data	*data;
> > +	phys_addr_t			addr;
> > +	struct cmdq_client_reg		cmdq_reg;
> > +	bool				has_gce_client_reg;
> >   };
> >   
> >   static const unsigned int mt2701_mutex_mod[DDP_COMPONENT_ID_MAX]
> > = {
> > @@ -618,6 +623,28 @@ void mtk_mutex_enable(struct mtk_mutex *mutex)
> >   }
> >   EXPORT_SYMBOL_GPL(mtk_mutex_enable);
> >   
> > +void mtk_mutex_enable_by_cmdq(struct mtk_mutex *mutex, void *pkt)
> > +{
> > +#if IS_REACHABLE(CONFIG_MTK_CMDQ)
> > +	struct mtk_mutex_ctx *mtx = container_of(mutex, struct
> > mtk_mutex_ctx,
> > +						 mutex[mutex->id]);
> > +	struct cmdq_pkt *cmdq_pkt = (struct cmdq_pkt *)pkt;
> > +
> > +	WARN_ON(&mtx->mutex[mutex->id] != mutex);
> > +
> > +	if (!mtx->has_gce_client_reg) {
> > +		dev_dbg(mtx->dev, "mediatek,gce-client-reg hasn't been
> > set in dts");
> > +		return;
> 
> Is this really supposed to happen?
> 
> Reiterating: when the gce-client-reg is not set, this function should
> either never
> be called from the principle, or it should actually fail.
> If a driver relies on mtk_mutex_enable_by_cmdq() and does *not* fall
> back to
> register write from cpu, then no change will occur at all, leading to
> a random
> breakage with no apparent reason.
> 
> This means that - for safety - this function should return -EINVAL
> when it gets
> called while no gce client reg is declared.
> Of course, this would also mean that the dev_dbg() should be a
> dev_err(), and
> that efforts should be done to avoid triggering this error by adding
> fallbacks
> in the drivers that will call this.
> 
> Another option would be to keep this function void, keep the print a
> dev_dbg(),
> but automatically fallback to mtk_mutex_enable(), so that here:
> 
> if (!mtx->has_gce_client_reg) {
> 	dev_dbg(mtx->dev,
> 		"No GCE client register found: falling back to cpu
> write.\n");
> 	mtk_mutex_enable(mutex);
> 	return;
> }
> 
> ...you're free to choose whichever of the two options, but this has
> to be fixed
> to remove this fragility.
> 
> > +	}
> > +
> > +	cmdq_pkt_write(cmdq_pkt, mtx->cmdq_reg.subsys,
> > +		       mtx->addr + DISP_REG_MUTEX_EN(mutex->id), 1);
> > +#else
> > +	dev_err(mtx->dev, "Not support for enable MUTEX by CMDQ");
> 
> ...obviously, if mtk_cmdq not reachable, instead of simply letting
> drivers break,
> you should, also here, fall back to the less performant and very
> suboptimal (for
> the specific case of mdp3 and some others) mtk_mutex_enable().
> 
> 
> Thanks,
> Angelo

Hi Angelo,

Thanks for your review.
Considering that the CPU writing flow already exists, the new CMDQ
method should be simplified to avoid confusion.
I will use the second method to directly return the function and print
an error message.
Users (currently only MDP3 needs to use CMDQ because it is single-mode
totally) should pay attention to this function.

Thanks,
Moudy


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

Re: [PATCH v14 6/6] soc: mediatek: mutex: add functions that operate registers by CMDQ

[AngeloGioacchino Del Regno]

Il 11/04/22 09:24, Moudy Ho ha scritto:
> Due to HW limitations, MDP3 is necessary to enable MUTEX in each frame
> for SOF triggering and cooperate with CMDQ control to reduce the amount
> of interrupts generated(also, reduce frame latency).
> 
> In response to the above situation, a new interface
> "mtk_mutex_enable_by_cmdq" has been added to achieve the purpose.
> 
> Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>
> ---
>   drivers/soc/mediatek/mtk-mutex.c       | 42 +++++++++++++++++++++++++-
>   include/linux/soc/mediatek/mtk-mutex.h |  2 ++
>   2 files changed, 43 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/soc/mediatek/mtk-mutex.c b/drivers/soc/mediatek/mtk-mutex.c
> index fc9ba2749946..1811beaf399d 100644
> --- a/drivers/soc/mediatek/mtk-mutex.c
> +++ b/drivers/soc/mediatek/mtk-mutex.c
> @@ -7,10 +7,12 @@
>   #include <linux/iopoll.h>
>   #include <linux/module.h>
>   #include <linux/of_device.h>
> +#include <linux/of_address.h>
>   #include <linux/platform_device.h>
>   #include <linux/regmap.h>
>   #include <linux/soc/mediatek/mtk-mmsys.h>
>   #include <linux/soc/mediatek/mtk-mutex.h>
> +#include <linux/soc/mediatek/mtk-cmdq.h>
>   
>   #define MT2701_MUTEX0_MOD0			0x2c
>   #define MT2701_MUTEX0_SOF0			0x30
> @@ -176,6 +178,9 @@ struct mtk_mutex_ctx {
>   	void __iomem			*regs;
>   	struct mtk_mutex		mutex[10];
>   	const struct mtk_mutex_data	*data;
> +	phys_addr_t			addr;
> +	struct cmdq_client_reg		cmdq_reg;
> +	bool				has_gce_client_reg;
>   };
>   
>   static const unsigned int mt2701_mutex_mod[DDP_COMPONENT_ID_MAX] = {
> @@ -618,6 +623,28 @@ void mtk_mutex_enable(struct mtk_mutex *mutex)
>   }
>   EXPORT_SYMBOL_GPL(mtk_mutex_enable);
>   
> +void mtk_mutex_enable_by_cmdq(struct mtk_mutex *mutex, void *pkt)
> +{
> +#if IS_REACHABLE(CONFIG_MTK_CMDQ)
> +	struct mtk_mutex_ctx *mtx = container_of(mutex, struct mtk_mutex_ctx,
> +						 mutex[mutex->id]);
> +	struct cmdq_pkt *cmdq_pkt = (struct cmdq_pkt *)pkt;
> +
> +	WARN_ON(&mtx->mutex[mutex->id] != mutex);
> +
> +	if (!mtx->has_gce_client_reg) {
> +		dev_dbg(mtx->dev, "mediatek,gce-client-reg hasn't been set in dts");
> +		return;

Is this really supposed to happen?

Reiterating: when the gce-client-reg is not set, this function should either never
be called from the principle, or it should actually fail.
If a driver relies on mtk_mutex_enable_by_cmdq() and does *not* fall back to
register write from cpu, then no change will occur at all, leading to a random
breakage with no apparent reason.

This means that - for safety - this function should return -EINVAL when it gets
called while no gce client reg is declared.
Of course, this would also mean that the dev_dbg() should be a dev_err(), and
that efforts should be done to avoid triggering this error by adding fallbacks
in the drivers that will call this.

Another option would be to keep this function void, keep the print a dev_dbg(),
but automatically fallback to mtk_mutex_enable(), so that here:

if (!mtx->has_gce_client_reg) {
	dev_dbg(mtx->dev,
		"No GCE client register found: falling back to cpu write.\n");
	mtk_mutex_enable(mutex);
	return;
}

...you're free to choose whichever of the two options, but this has to be fixed
to remove this fragility.

> +	}
> +
> +	cmdq_pkt_write(cmdq_pkt, mtx->cmdq_reg.subsys,
> +		       mtx->addr + DISP_REG_MUTEX_EN(mutex->id), 1);
> +#else
> +	dev_err(mtx->dev, "Not support for enable MUTEX by CMDQ");

...obviously, if mtk_cmdq not reachable, instead of simply letting drivers break,
you should, also here, fall back to the less performant and very suboptimal (for
the specific case of mdp3 and some others) mtk_mutex_enable().


Thanks,
Angelo

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

[PATCH v14 6/6] soc: mediatek: mutex: add functions that operate registers by CMDQ

[Moudy Ho]

Due to HW limitations, MDP3 is necessary to enable MUTEX in each frame
for SOF triggering and cooperate with CMDQ control to reduce the amount
of interrupts generated(also, reduce frame latency).

In response to the above situation, a new interface
"mtk_mutex_enable_by_cmdq" has been added to achieve the purpose.

Signed-off-by: Moudy Ho <moudy.ho@mediatek.com>
---
 drivers/soc/mediatek/mtk-mutex.c       | 42 +++++++++++++++++++++++++-
 include/linux/soc/mediatek/mtk-mutex.h |  2 ++
 2 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/mediatek/mtk-mutex.c b/drivers/soc/mediatek/mtk-mutex.c
index fc9ba2749946..1811beaf399d 100644
--- a/drivers/soc/mediatek/mtk-mutex.c
+++ b/drivers/soc/mediatek/mtk-mutex.c
@@ -7,10 +7,12 @@
 #include <linux/iopoll.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
+#include <linux/of_address.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
 #include <linux/soc/mediatek/mtk-mmsys.h>
 #include <linux/soc/mediatek/mtk-mutex.h>
+#include <linux/soc/mediatek/mtk-cmdq.h>
 
 #define MT2701_MUTEX0_MOD0			0x2c
 #define MT2701_MUTEX0_SOF0			0x30
@@ -176,6 +178,9 @@ struct mtk_mutex_ctx {
 	void __iomem			*regs;
 	struct mtk_mutex		mutex[10];
 	const struct mtk_mutex_data	*data;
+	phys_addr_t			addr;
+	struct cmdq_client_reg		cmdq_reg;
+	bool				has_gce_client_reg;
 };
 
 static const unsigned int mt2701_mutex_mod[DDP_COMPONENT_ID_MAX] = {
@@ -618,6 +623,28 @@ void mtk_mutex_enable(struct mtk_mutex *mutex)
 }
 EXPORT_SYMBOL_GPL(mtk_mutex_enable);
 
+void mtk_mutex_enable_by_cmdq(struct mtk_mutex *mutex, void *pkt)
+{
+#if IS_REACHABLE(CONFIG_MTK_CMDQ)
+	struct mtk_mutex_ctx *mtx = container_of(mutex, struct mtk_mutex_ctx,
+						 mutex[mutex->id]);
+	struct cmdq_pkt *cmdq_pkt = (struct cmdq_pkt *)pkt;
+
+	WARN_ON(&mtx->mutex[mutex->id] != mutex);
+
+	if (!mtx->has_gce_client_reg) {
+		dev_dbg(mtx->dev, "mediatek,gce-client-reg hasn't been set in dts");
+		return;
+	}
+
+	cmdq_pkt_write(cmdq_pkt, mtx->cmdq_reg.subsys,
+		       mtx->addr + DISP_REG_MUTEX_EN(mutex->id), 1);
+#else
+	dev_err(mtx->dev, "Not support for enable MUTEX by CMDQ");
+#endif
+}
+EXPORT_SYMBOL_GPL(mtk_mutex_enable_by_cmdq);
+
 void mtk_mutex_disable(struct mtk_mutex *mutex)
 {
 	struct mtk_mutex_ctx *mtx = container_of(mutex, struct mtk_mutex_ctx,
@@ -656,7 +683,7 @@ static int mtk_mutex_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct mtk_mutex_ctx *mtx;
-	struct resource *regs;
+	struct resource *regs, addr;
 	int i;
 
 	mtx = devm_kzalloc(dev, sizeof(*mtx), GFP_KERNEL);
@@ -677,6 +704,19 @@ static int mtk_mutex_probe(struct platform_device *pdev)
 		}
 	}
 
+	if (of_address_to_resource(dev->of_node, 0, &addr) < 0)
+		mtx->addr = 0L;
+	else
+		mtx->addr = addr.start;
+
+#if IS_REACHABLE(CONFIG_MTK_CMDQ)
+	ret = cmdq_dev_get_client_reg(dev, &mtx->cmdq_reg, 0);
+	if (ret)
+		dev_dbg(dev, "No mediatek,gce-client-reg!\n");
+	else
+		mtx->has_gce_client_reg = true;
+#endif
+
 	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	mtx->regs = devm_ioremap_resource(dev, regs);
 	if (IS_ERR(mtx->regs)) {
diff --git a/include/linux/soc/mediatek/mtk-mutex.h b/include/linux/soc/mediatek/mtk-mutex.h
index 200f4365c950..17eea55b6809 100644
--- a/include/linux/soc/mediatek/mtk-mutex.h
+++ b/include/linux/soc/mediatek/mtk-mutex.h
@@ -33,6 +33,8 @@ void mtk_mutex_set_mod(struct mtk_mutex *mutex,
 void mtk_mutex_set_sof(struct mtk_mutex *mutex,
 		       enum mtk_mutex_table_index idx);
 void mtk_mutex_enable(struct mtk_mutex *mutex);
+void mtk_mutex_enable_by_cmdq(struct mtk_mutex *mutex,
+			      void *pkt);
 void mtk_mutex_disable(struct mtk_mutex *mutex);
 void mtk_mutex_remove_comp(struct mtk_mutex *mutex,
 			   enum mtk_ddp_comp_id id);
-- 
2.18.0


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