[PATCH v2 6/7] drm/panfrost: Add support for GPU heap allocations

[Robin Murphy]

On 26/07/2019 10:15, Steven Price wrote:
> On 25/07/2019 22:11, Rob Herring wrote:
>> On Thu, Jul 25, 2019 at 7:08 AM Robin Murphy <robin.murphy at arm.com> 
>> wrote:
>>>
>>> Hi Rob,
>>>
>>> On 25/07/2019 02:10, Rob Herring wrote:
>>> [...]
>>>> @@ -328,6 +427,18 @@ static irqreturn_t panfrost_mmu_irq_handler(int 
>>>> irq, void *data)
>>>>                access_type = (fault_status >> 8) & 0x3;
>>>>                source_id = (fault_status >> 16);
>>>>
>>>> +             /* Page fault only */
>>>> +             if ((status & mask) == BIT(i)) {
>>>> +                     WARN_ON(exception_type < 0xC1 || 
>>>> exception_type > 0xC4);
>>>> +
>>>> +                     ret = panfrost_mmu_map_fault_addr(pfdev, i, 
>>>> addr);
>>>> +                     if (!ret) {
>>>> +                             mmu_write(pfdev, MMU_INT_CLEAR, BIT(i));
>>>> +                             status &= ~mask;
>>>> +                             continue;
>>>> +                     }
>>>> +             }
>>>> +
>>>>                /* terminal fault, print info about the fault */
>>>>                dev_err(pfdev->dev,
>>>>                        "Unhandled Page fault in AS%d at VA 0x%016llX\n"
>>>> @@ -368,8 +479,9 @@ int panfrost_mmu_init(struct panfrost_device 
>>>> *pfdev)
>>>>        if (irq <= 0)
>>>>                return -ENODEV;
>>>>
>>>> -     err = devm_request_irq(pfdev->dev, irq, panfrost_mmu_irq_handler,
>>>> -                            IRQF_SHARED, "mmu", pfdev);
>>>> +     err = devm_request_threaded_irq(pfdev->dev, irq, NULL,
>>>> +                                     panfrost_mmu_irq_handler,
>>>> +                                     IRQF_ONESHOT, "mmu", pfdev);
>>>
>>> The change of flags here breaks platforms using a single shared
>>> interrupt line.
>>
>> Do they exist? I think this was largely copy-n-paste leftover from the
> 
> Good question - of the platforms I know about they all have separate 
> IRQs, but kbase does explicitly allow shared interrupts so it's quite 
> possible there is a platform out there which does.
> 
>> lima driver where utgard has a bunch of irqs and so they get combined.
>> While it's possible certainly, I'd like to avoid having to do further
>> rework either to use a workqueue or we need a single driver handler
>> which then dispatches the sub handlers. The problem is threaded irq
>> handlers don't work with shared irqs.
> 
> It seems reasonable to me to at least wait until someone identifies a 
> platform which needs shared interrupts before embarking on the refactoring.

I don't know about real silicon, but it's certainly true of our internal 
FPGA images that I've been playing with (the Juno board only has a 
single IRQ line for the entire FPGA site). That's obviously not a major 
priority for upstream, though, so I can hack around it if it's not 
otherwise justified.

Robin.