On Mon, 2021-10-18 at 09:13 +0200, Dafna Hirschfeld wrote:

On 16.10.21 04:23, Yong Wu wrote:

quoted

On Mon, 2021-10-11 at 14:36 +0200, Dafna Hirschfeld wrote:

quoted

On 29.09.21 03:37, Yong Wu wrote:

quoted

MediaTek IOMMU-SMI diagram is like below. all the consumer
connect
with
smi-larb, then connect with smi-common.

          M4U
           |
      smi-common
           |
    -------------
    |         |    ...
    |         |
larb1     larb2
    |         |
vdec       venc

When the consumer works, it should enable the smi-larb's power
which
also need enable the smi-common's power firstly.

Thus, First of all, use the device link connect the consumer
and
the
smi-larbs. then add device link between the smi-larb and smi-
common.

This patch adds device_link between the consumer and the larbs.

When device_link_add, I add the flag DL_FLAG_STATELESS to avoid
calling
pm_runtime_xx to keep the original status of clocks. It can
avoid
two
issues:
1) Display HW show fastlogo abnormally reported in [1]. At the
beggining,
all the clocks are enabled before entering kernel, but the
clocks
for
display HW(always in larb0) will be gated after clk_enable and
clk_disable
called from device_link_add(->pm_runtime_resume) and rpm_idle.
The
clock
operation happened before display driver probe. At that time,
the
display
HW will be abnormal.

2) A deadlock issue reported in [2]. Use DL_FLAG_STATELESS to
skip
pm_runtime_xx to avoid the deadlock.

Corresponding, DL_FLAG_AUTOREMOVE_CONSUMER can't be added, then
device_link_removed should be added explicitly.

[1]

https://lore.kernel.org/linux-mediatek/1564213888.22908.4.camel@mhfsdcap03/ (local)

quoted

quoted

quoted

[2] https://lore.kernel.org/patchwork/patch/1086569/

Suggested-by: Tomasz Figa <tfiga@chromium.org>
Signed-off-by: Yong Wu <yong.wu@mediatek.com>
Tested-by: Frank Wunderlich <redacted> # BPI-
R2/MT7623
---
   drivers/iommu/mtk_iommu.c    | 22 ++++++++++++++++++++++
   drivers/iommu/mtk_iommu_v1.c | 20 +++++++++++++++++++-
   2 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/mtk_iommu.c

b/drivers/iommu/mtk_iommu.c
index d5848f78a677..a2fa55899434 100644

--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c

@@ -560,22 +560,44 @@ static struct iommu_device

*mtk_iommu_probe_device(struct device *dev)
   {
   	struct iommu_fwspec *fwspec =
dev_iommu_fwspec_get(dev);
   	struct mtk_iommu_data *data;
+	struct device_link *link;
+	struct device *larbdev;
+	unsigned int larbid;
   
   	if (!fwspec || fwspec->ops != &mtk_iommu_ops)
   		return ERR_PTR(-ENODEV); /* Not a iommu client
device
*/
   
   	data = dev_iommu_priv_get(dev);
   
+	/*
+	 * Link the consumer device with the smi-larb
device(supplier)
+	 * The device in each a larb is a independent HW. thus
only
link
+	 * one larb here.
+	 */
+	larbid = MTK_M4U_TO_LARB(fwspec->ids[0]);

so larbid is always the same for all the ids of a device?

Yes. For me, each a dtsi node should represent a HW unit which can
only
connect one larb.

quoted

If so maybe it worth testing it and return error if this is not
the
case.

Thanks for the suggestion. This is very helpful. I did see someone
put
the different larbs in one node. I will check this, and add return

I am working on bugs found on media drivers, could you please point
me to
that wrong node?
Will you send a fix to that node in the dtsi?

sorry. I mean it happened in the internal branch and it has already
been fixed internally,  all the upstream nodes are ok for this.

Thanks

Thanks,
Dafna

quoted

EINVAL for this case.

quoted

quoted

Thanks,
Dafna

quoted

quoted

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek