On Mon, 2021-10-18 at 09:13 +0200, Dafna Hirschfeld wrote:
On 16.10.21 04:23, Yong Wu wrote:
On Mon, 2021-10-11 at 14:36 +0200, Dafna Hirschfeld wrote:
On 29.09.21 03:37, Yong Wu wrote:
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:
- 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.
- 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/
[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 frank-w@public-files.de # 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-larbdevice(supplier)
* The device in each a larb is a independent HW. thusonly 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.
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
EINVAL for this case.
Thanks, Dafna