Re: [PATCH 3/3] drm/panfrost: Stay in the threaded MMU IRQ handler until we've handled all IRQs

[Steven Price <steven.price@arm.com>]

On 03/02/2021 14:45, Rob Herring wrote:
> On Mon, Feb 1, 2021 at 2:21 AM Boris Brezillon
> <boris.brezillon@collabora.com> wrote:
>>
>> Doing a hw-irq -> threaded-irq round-trip is counter-productive, stay
>> in the threaded irq handler as long as we can.
>>
>> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
>> ---
>>   drivers/gpu/drm/panfrost/panfrost_mmu.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c
>> index 21e552d1ac71..65bc20628c4e 100644
>> --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c
>> +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c
>> @@ -580,6 +580,8 @@ static irqreturn_t panfrost_mmu_irq_handler_thread(int irq, void *data)
>>          u32 status = mmu_read(pfdev, MMU_INT_RAWSTAT);
>>          int i, ret;
>>
>> +again:
>> +
>>          for (i = 0; status; i++) {
>>                  u32 mask = BIT(i) | BIT(i + 16);
>>                  u64 addr;
>> @@ -628,6 +630,11 @@ static irqreturn_t panfrost_mmu_irq_handler_thread(int irq, void *data)
>>                  status &= ~mask;
>>          }
>>
>> +       /* If we received new MMU interrupts, process them before returning. */
>> +       status = mmu_read(pfdev, MMU_INT_RAWSTAT);
>> +       if (status)
>> +               goto again;
>> +
> 
> Can't we avoid the goto? Change the for loop like this:
> 
> i = 0;
> while (status = mmu_read(pfdev, MMU_INT_RAWSTAT)) {
>      ...
> 
>      i++;
>      if (i == 16)
>          i = 0;
> }

Well that reads from the RAWSTAT register excessively (which could be 
expensive at low GPU clock speeds), but we could do:

for(i = 0; status; i++) {
	...

	if (!status) {
		i = 0;
		status = mmu_read(pfdev, MMU_INT_RAWSTAT);
	}
}

(or similar with a while() if you prefer). Of course we could even get 
rid of the 'i' loop altogether:

status = mmu_read(pfdev, MMU_INT_RAWSTAT);
while (status) {
	int i = ffs(status | (status >> 16)) - 1;

	... existing code ...

	status &= ~mask;
	if (!status)
		status = mmu_read(pfdev, MMU_INT_RAWSTAT);
}

Steve

>>          mmu_write(pfdev, MMU_INT_MASK, ~0);
>>          return IRQ_HANDLED;
>>   };
>> --
>> 2.26.2
>>

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