On Thu, 8 Feb 2024 14:30:02 +0000 Liviu Dudau <liviu.du...@arm.com> wrote:
> > +#ifdef CONFIG_PM > > +int panthor_device_resume(struct device *dev) > > +{ > > + struct panthor_device *ptdev = dev_get_drvdata(dev); > > + int ret, cookie; > > + > > + if (atomic_read(&ptdev->pm.state) != PANTHOR_DEVICE_PM_STATE_SUSPENDED) > > + return -EINVAL; > > + > > + atomic_set(&ptdev->pm.state, PANTHOR_DEVICE_PM_STATE_RESUMING); > > + > > + ret = clk_prepare_enable(ptdev->clks.core); > > + if (ret) > > + goto err_set_suspended; > > + > > + ret = clk_prepare_enable(ptdev->clks.stacks); > > + if (ret) > > + goto err_disable_core_clk; > > + > > + ret = clk_prepare_enable(ptdev->clks.coregroup); > > + if (ret) > > + goto err_disable_stacks_clk; > > + > > + ret = panthor_devfreq_resume(ptdev); > > + if (ret) > > + goto err_disable_coregroup_clk; > > + > > + if (panthor_device_is_initialized(ptdev) && > > + drm_dev_enter(&ptdev->base, &cookie)) { > > + panthor_gpu_resume(ptdev); > > + panthor_mmu_resume(ptdev); > > + ret = drm_WARN_ON(&ptdev->base, panthor_fw_resume(ptdev)); > > + if (!ret) > > + panthor_sched_resume(ptdev); > > + > > + drm_dev_exit(cookie); > > + > > + if (ret) > > + goto err_devfreq_suspend; > > + } > > + > > + if (atomic_read(&ptdev->reset.pending)) > > + queue_work(ptdev->reset.wq, &ptdev->reset.work); > > + > > + /* Clear all IOMEM mappings pointing to this device after we've > > + * resumed. This way the fake mappings pointing to the dummy pages > > + * are removed and the real iomem mapping will be restored on next > > + * access. > > + */ > > + mutex_lock(&ptdev->pm.mmio_lock); > > + unmap_mapping_range(ptdev->base.anon_inode->i_mapping, > > + DRM_PANTHOR_USER_MMIO_OFFSET, 0, 1); > > + atomic_set(&ptdev->pm.state, PANTHOR_DEVICE_PM_STATE_ACTIVE); > > + mutex_unlock(&ptdev->pm.mmio_lock); > > + return 0; > > + > > +err_devfreq_suspend: > > + panthor_devfreq_suspend(ptdev); > > Should we also call panthor_mmu_suspend(ptdev) and panthor_gpu_suspend(ptdev) > before panthor_devfreq_suspend()? Otherwise those blocks will be left in an > inconsistent state. Yep, it's been fixed in panthor-next (see [1]). Note that I didn't add it to the error path, to keep the panthor_{mmu,gpu}_suspend() under the drm_dev_enter/exit() section. > > + > > +err_disable_coregroup_clk: > > + clk_disable_unprepare(ptdev->clks.coregroup); > > + > > +err_disable_stacks_clk: > > + clk_disable_unprepare(ptdev->clks.stacks); > > + > > +err_disable_core_clk: > > + clk_disable_unprepare(ptdev->clks.core); > > + > > +err_set_suspended: > > + atomic_set(&ptdev->pm.state, PANTHOR_DEVICE_PM_STATE_SUSPENDED); > > + return ret; > > +} [1]https://gitlab.freedesktop.org/panfrost/linux/-/commit/d9eccd669206ffd9383b955bffba93426ebea40a