Re: [PATCH v2 7/8] drm/panfrost: Rework page table flushing and runtime PM interaction

From: Steven Price <steven.price@arm.com>
To: Robin Murphy <robin.murphy@arm.com>,
	Rob Herring <robh@kernel.org>,
	dri-devel@lists.freedesktop.org
Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com>,
	Maxime Ripard <maxime.ripard@bootlin.com>,
	David Airlie <airlied@linux.ie>,
	Boris Brezillon <boris.brezillon@collabora.com>,
	Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>,
	Sean Paul <sean@poorly.run>
Subject: Re: [PATCH v2 7/8] drm/panfrost: Rework page table flushing and runtime PM interaction
Date: Fri, 23 Aug 2019 16:05:07 +0100	[thread overview]
Message-ID: <749c3430-9f35-84b4-9856-e43addf35f87@arm.com> (raw)
In-Reply-To: <0dc17539-99d0-d0ea-9b24-506f4dafef71@arm.com>

On 23/08/2019 12:11, Robin Murphy wrote:
> On 23/08/2019 03:12, Rob Herring wrote:
>> There is no point in resuming the h/w just to do flush operations and
>> doing so takes several locks which cause lockdep issues with the 
>> shrinker.
>> Rework the flush operations to only happen when the h/w is already awake.
>> This avoids taking any locks associated with resuming.
>>
>> Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>> Cc: Steven Price <steven.price@arm.com>
>> Cc: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
>> Cc: David Airlie <airlied@linux.ie>
>> Cc: Daniel Vetter <daniel@ffwll.ch>
>> Signed-off-by: Rob Herring <robh@kernel.org>
>> ---
>> v2: new patch
>>
>>   drivers/gpu/drm/panfrost/panfrost_mmu.c | 41 ++++++++++++-------------
>>   1 file changed, 20 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c 
>> b/drivers/gpu/drm/panfrost/panfrost_mmu.c
>> index 842bdd7cf6be..ccf671a9c3fb 100644
>> --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
>> +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
>> @@ -220,6 +220,23 @@ static size_t get_pgsize(u64 addr, size_t size)
>>       return SZ_2M;
>>   }
>> +void panfrost_mmu_flush_range(struct panfrost_device *pfdev,
>> +                  struct panfrost_mmu *mmu,
>> +                  u64 iova, size_t size)
>> +{
>> +    if (mmu->as < 0)
>> +        return;
>> +
>> +    /* Flush the PTs only if we're already awake */
>> +    if (!pm_runtime_get_if_in_use(pfdev->dev))
>> +        return;
> 
> Is the MMU guaranteed to be reset on resume such that the TLBs will 
> always come up clean? Otherwise there are potentially corners where 
> stale entries that we skip here might hang around if the hardware lies 
> about powering things down.

Assuming runtime PM has actually committed to the power off, then on 
power on panfrost_device_resume() will be called which performs a reset 
of the GPU which will clear the L2/TLBs so there shouldn't be any 
problem there.

As far as I can see from the code that is how pm_runtime_get_if_in_use() 
works - it will return true unless the suspend() callback has been called.

Steve
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel