On 25/09/2024 13:58, Thierry Reding wrote:
On Tue, Sep 24, 2024 at 07:33:05PM GMT, Jon Hunter wrote:
On 06/09/2024 09:38, Jon Hunter wrote:
Hi Mikko,
On 31/05/2024 08:07, Mikko Perttunen wrote:
From: Mikko Perttunen <mperttunen@xxxxxxxxxx>
Syncpoint IRQs are currently requested in a code path that runs
during resume. Due to this, we get multiple overlapping registered
interrupt handlers as host1x is suspended and resumed.
Rearrange interrupt code to only request IRQs during initialization.
Signed-off-by: Mikko Perttunen <mperttunen@xxxxxxxxxx>
...
This change is causing a boot regression on Tegra186 with the latest
-next. I have reverted this to confirm that this fixes the problem. I
don't see any crash log but the board appears to just hang.
I had a look at this and I was able to fix this by moving where
we initialise the interrupts to after the PM runtime enable ...
diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c
index b62e4f0e8130..ff98d4903cac 100644
--- a/drivers/gpu/host1x/dev.c
+++ b/drivers/gpu/host1x/dev.c
@@ -625,12 +625,6 @@ static int host1x_probe(struct platform_device *pdev)
goto free_contexts;
}
- err = host1x_intr_init(host);
- if (err) {
- dev_err(&pdev->dev, "failed to initialize interrupts\n");
- goto deinit_syncpt;
- }
-
pm_runtime_enable(&pdev->dev);
err = devm_tegra_core_dev_init_opp_table_common(&pdev->dev);
@@ -642,6 +636,12 @@ static int host1x_probe(struct platform_device *pdev)
if (err)
goto pm_disable;
+ err = host1x_intr_init(host);
+ if (err) {
+ dev_err(&pdev->dev, "failed to initialize interrupts\n");
+ goto pm_put;
+ }
+
I think the reason why this might fail now is because host1x_intr_init()
ends up writing some registers during the call to the
host1x_hw_intr_disable_all_syncpt_intrs() function, which would hang or
crash if the device isn't on (which in turn happens during
pm_runtime_resume_and_get()).
Yes that is exactly where it hung!
Not sure exactly why this used to work because prior to Mikko's patch
because the sequence was the same before.
Previously host1x_intr_init() did not call
host1x_hw_intr_disable_all_syncpt_intrs().
host1x_debug_init(host);
err = host1x_register(host);
@@ -658,14 +658,11 @@ static int host1x_probe(struct platform_device *pdev)
host1x_unregister(host);
deinit_debugfs:
host1x_debug_deinit(host);
-
+ host1x_intr_deinit(host);
+pm_put:
pm_runtime_put_sync_suspend(&pdev->dev);
pm_disable:
pm_runtime_disable(&pdev->dev);
-
- host1x_intr_deinit(host);
-deinit_syncpt:
- host1x_syncpt_deinit(host);
free_contexts:
host1x_memory_context_list_free(&host->context_list);
free_channels:
Thierry, do you want to me to send a fix for the above or do you
want to squash with the original (assuming that OK with Mikko)?
In any case, this looks like a good fix, so please send a proper patch.
This is merged through drm-misc, so squashing this into the original
patch is not an option any longer.
Yes will do!
Jon
--
nvpublic
