[PATCH 2/5] drm/panthor: Be robust against runtime PM resume failures in the suspend path

[Liviu Dudau]

On Thu, Nov 14, 2024 at 12:27:55PM +0100, Boris Brezillon wrote:
> On Thu, 14 Nov 2024 11:13:29 +0000
> Liviu Dudau <liviu.dudau at arm.com> wrote:
> 
> > On Wed, Nov 13, 2024 at 04:42:54PM +0100, Boris Brezillon wrote:
> > > The runtime PM resume operation is not guaranteed to succeed, but if it
> > > fails, the device should be in a suspended state. Make sure we're robust
> > > to resume failures in the unplug path.
> > > 
> > > Signed-off-by: Boris Brezillon <boris.brezillon at collabora.com>
> > > ---
> > >  drivers/gpu/drm/panthor/panthor_device.c | 23 ++++++++---------------
> > >  drivers/gpu/drm/panthor/panthor_fw.c     | 14 +++++++++-----
> > >  drivers/gpu/drm/panthor/panthor_gpu.c    |  3 ++-
> > >  drivers/gpu/drm/panthor/panthor_mmu.c    |  3 ++-
> > >  4 files changed, 21 insertions(+), 22 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c
> > > index db7ba40f771d..8b5d54b2bbb4 100644
> > > --- a/drivers/gpu/drm/panthor/panthor_device.c
> > > +++ b/drivers/gpu/drm/panthor/panthor_device.c
> > > @@ -48,6 +48,8 @@ static int panthor_clk_init(struct panthor_device *ptdev)
> > >  
> > >  void panthor_device_unplug(struct panthor_device *ptdev)
> > >  {
> > > +	int ret;
> > > +
> > >  	/* This function can be called from two different path: the reset work
> > >  	 * and the platform device remove callback. drm_dev_unplug() doesn't
> > >  	 * deal with concurrent callers, so we have to protect drm_dev_unplug()
> > > @@ -74,7 +76,8 @@ void panthor_device_unplug(struct panthor_device *ptdev)
> > >  	 */
> > >  	mutex_unlock(&ptdev->unplug.lock);
> > >  
> > > -	drm_WARN_ON(&ptdev->base, pm_runtime_get_sync(ptdev->base.dev) < 0);
> > > +	ret = pm_runtime_get_sync(ptdev->base.dev);
> > > +	drm_WARN_ON(&ptdev->base, ret < 0);
> > >  
> > >  	/* Now, try to cleanly shutdown the GPU before the device resources
> > >  	 * get reclaimed.
> > > @@ -85,7 +88,10 @@ void panthor_device_unplug(struct panthor_device *ptdev)
> > >  	panthor_gpu_unplug(ptdev);
> > >  
> > >  	pm_runtime_dont_use_autosuspend(ptdev->base.dev);
> > > -	pm_runtime_put_sync_suspend(ptdev->base.dev);
> > > +
> > > +	/* If the resume failed, we don't need to suspend here. */
> > > +	if (!ret)
> > > +		pm_runtime_put_sync_suspend(ptdev->base.dev);
> > >  
> > >  	/* If PM is disabled, we need to call the suspend handler manually. */
> > >  	if (!IS_ENABLED(CONFIG_PM))
> > > @@ -541,17 +547,4 @@ int panthor_device_suspend(struct device *dev)
> > >  	clk_disable_unprepare(ptdev->clks.core);
> > >  	atomic_set(&ptdev->pm.state, PANTHOR_DEVICE_PM_STATE_SUSPENDED);
> > >  	return 0;
> > > -
> > > -err_set_active:  
> > 
> > Label is being deleted here, but I don't see where the `goto err_set_active` is being removed.
> 
> Yes, as mentioned in my own reply, this chunk should be in patch 3.

Sorry, I did look at patch 3 before sending the email but somehow missed the line with the goto being deleted.

With the change:

Reviewed-by: Liviu Dudau <liviu.dudau at arm.com>

Best regards,
Liviu

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯