On 16.10.21 04:23, Yong Wu wrote:

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)
[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?


Thanks,
Dafna

EINVAL for this case.

quoted

Thanks,
Dafna

quoted

quoted