Re: [PATCH v3 5/5] drm/panthor: Fix the fast-reset logic

[Boris Brezillon <boris.brezillon@collabora.com>]

On Wed, 11 Dec 2024 09:57:07 +0000
Liviu Dudau <liviu.dudau@arm.com> wrote:

> On Wed, Dec 11, 2024 at 08:54:19AM +0100, Boris Brezillon wrote:
> > If we do a GPU soft-reset, that's no longer fast reset. This also means
> > the slow reset fallback doesn't work because the MCU state is only reset
> > after a GPU soft-reset.
> > 
> > Let's move the retry logic to panthor_device_resume() to issue a
> > soft-reset between the fast and slow attempts, and patch
> > panthor_gpu_suspend() to only power-off the L2 when a fast reset is
> > requested.
> > 
> > v3:
> > - No changes
> > 
> > v2:
> > - Add R-b
> > 
> > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> > Reviewed-by: Steven Price <steven.price@arm.com>  
> 
> For reasons that are not clear yet to me my v2 R-bs emails seem to have not
> reached you or the dri-devel mailing lists.
> 
> 
> > ---
> >  drivers/gpu/drm/panthor/panthor_device.c | 32 ++++++++++----
> >  drivers/gpu/drm/panthor/panthor_device.h | 11 +++++
> >  drivers/gpu/drm/panthor/panthor_fw.c     | 54 ++++++------------------
> >  drivers/gpu/drm/panthor/panthor_gpu.c    | 11 ++---
> >  4 files changed, 53 insertions(+), 55 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/panthor/panthor_device.c b/drivers/gpu/drm/panthor/panthor_device.c
> > index 0362101ea896..2c817e65e6be 100644
> > --- a/drivers/gpu/drm/panthor/panthor_device.c
> > +++ b/drivers/gpu/drm/panthor/panthor_device.c
> > @@ -431,6 +431,22 @@ int panthor_device_mmap_io(struct panthor_device *ptdev, struct vm_area_struct *
> >  	return 0;
> >  }
> >  
> > +static int panthor_device_resume_hw_components(struct panthor_device *ptdev)
> > +{
> > +	int ret;
> > +
> > +	panthor_gpu_resume(ptdev);
> > +	panthor_mmu_resume(ptdev);
> > +
> > +	ret = panthor_fw_resume(ptdev);
> > +	if (!ret)
> > +		return 0;
> > +
> > +	panthor_mmu_suspend(ptdev);
> > +	panthor_gpu_suspend(ptdev);
> > +	return ret;  
> 
> My only comment was a nit here where I prefer the construct:
> 
> 	if (ret) {
> 		panthor_mmu_suspend(ptdev);
> 		panthor_gpu_suspend(ptdev);
> 	}
> 
> 	return ret;
> 
> but feel free to ignore it.
> 
> For the whole series: Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>

I applied the series before seeing your replies. Sorry about that :-/.