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
