Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] Unbalanced `pm_runtime_disable()` in `nvdec_remove()` and `nvdec_exit()` leads to a permanent `disable_depth` leak across bind/unbind cycles. - [High] Unconditional `dma_unmap_single()` of unallocated firmware memory causes DMA API violations and potential IOMMU corruption. -- --- Patch [1]: [PATCH] drm/tegra: nvdec: fix autosuspend cleanup during teardown --- commit f6356a1a5e1501d81058f59a7327814564c32066 Author: Guangshuo Li <[email protected]> drm/tegra: nvdec: fix autosuspend cleanup during teardown This commit fixes a reference count leak by adding a missing call to pm_runtime_dont_use_autosuspend() in nvdec_remove(). Without this, the runtime PM core increments usage_count if autosuspend is enabled with a negative delay, preventing runtime suspend during teardown. > diff --git a/drivers/gpu/drm/tegra/nvdec.c b/drivers/gpu/drm/tegra/nvdec.c > index 7a38664e890e3..cb52459b2d79a 100644 > --- a/drivers/gpu/drm/tegra/nvdec.c > +++ b/drivers/gpu/drm/tegra/nvdec.c > @@ -546,6 +546,7 @@ static void nvdec_remove(struct platform_device *pdev) > { > struct nvdec *nvdec = platform_get_drvdata(pdev); > > + pm_runtime_dont_use_autosuspend(&pdev->dev); > pm_runtime_disable(&pdev->dev); [Severity: High] This is a pre-existing issue, but does this sequence cause a permanent disable_depth leak across bind/unbind cycles? When nvdec_remove() calls pm_runtime_disable() here, it increments the disable_depth counter. The very next line calls host1x_client_unregister(), which triggers nvdec_exit(). If nvdec_exit() calls pm_runtime_force_suspend(client->dev) without a matching pm_runtime_force_resume() later, it seems the depth is incremented a second time but only decremented once on the next probe. Will this prevent the device from powering on after a re-bind? > host1x_client_unregister(&nvdec->client.base); [Severity: High] This is also a pre-existing issue, but is it safe to unconditionally unmap the firmware memory during this teardown sequence? When host1x_client_unregister() triggers nvdec_exit(), it looks like nvdec_exit() unconditionally calls: drivers/gpu/drm/tegra/nvdec.c:nvdec_exit() { ... dma_unmap_single(nvdec->dev, nvdec->falcon.firmware.phys, nvdec->falcon.firmware.size, DMA_TO_DEVICE); ... } If the driver is unbound before the firmware is loaded (or on platforms where falcon firmware is never loaded), size and phys will be 0. Could calling dma_unmap_single() on an unmapped address violate the DMA API and cause IOMMU faults? This pattern appears to exist in the vic and nvjpg drivers as well. > falcon_exit(&nvdec->falcon); > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
