Re: [PATCH v2 4/8] drm/panthor: Store IRQ register base iomem pointer in panthor_irq

[Karunika Choo]

On 22/04/2026 10:34, Steven Price wrote:
> On 12/04/2026 15:29, Karunika Choo wrote:
>> Update common IRQ handling code to work from an IRQ-local iomem base
>> instead of referencing block-specific interrupt register offsets.
>>
>> Store the interrupt base address iomem pointer in struct panthor_irq and
>> switch the shared IRQ helpers to use generic INT_* offsets from that
>> local base. This removes the need for each caller to expose absolute IRQ
>> register addresses while keeping the common IRQ flow unchanged.
>>
>> No functional change intended.
>>
>> v2:
>> - Change IRQ request function to accept an iomem pointer instead of
>>   computing it from an offset argument.
>>
>> Signed-off-by: Karunika Choo <karunika.choo@xxxxxxx>
> 
> One minor comment below...
> 
>> ---
>>  drivers/gpu/drm/panthor/panthor_device.h   | 32 ++++++++++++++--------
>>  drivers/gpu/drm/panthor/panthor_fw.c       |  5 ++--
>>  drivers/gpu/drm/panthor/panthor_fw_regs.h  |  2 ++
>>  drivers/gpu/drm/panthor/panthor_gpu.c      |  6 ++--
>>  drivers/gpu/drm/panthor/panthor_gpu_regs.h |  3 ++
>>  drivers/gpu/drm/panthor/panthor_mmu.c      |  5 ++--
>>  drivers/gpu/drm/panthor/panthor_mmu_regs.h |  3 ++
>>  drivers/gpu/drm/panthor/panthor_pwr.c      |  6 ++--
>>  8 files changed, 42 insertions(+), 20 deletions(-)
>>
> [...]
>> @@ -1470,7 +1470,8 @@ int panthor_fw_init(struct panthor_device *ptdev)
>>  	if (irq <= 0)
>>  		return -ENODEV;
>>  
>> -	ret = panthor_request_job_irq(ptdev, &fw->irq, irq, 0);
>> +	ret = panthor_request_job_irq(ptdev, &fw->irq, irq, 0,
>> +				      ptdev->iomem + JOB_INT_BASE);
>>  	if (ret) {
>>  		drm_err(&ptdev->base, "failed to request job irq");
>>  		return ret;
> [..]
>> @@ -162,7 +162,9 @@ int panthor_gpu_init(struct panthor_device *ptdev)
>>  	if (irq < 0)
>>  		return irq;
>>  
>> -	ret = panthor_request_gpu_irq(ptdev, &ptdev->gpu->irq, irq, GPU_INTERRUPTS_MASK);
>> +	ret = panthor_request_gpu_irq(ptdev, &ptdev->gpu->irq, irq,
>> +				      GPU_INTERRUPTS_MASK,
>> +				      ptdev->iomem + GPU_INT_BASE);
>>  	if (ret)
>>  		return ret;
>>  
> [...]
>> @@ -3229,7 +3229,8 @@ int panthor_mmu_init(struct panthor_device *ptdev)
>>  		return -ENODEV;
>>  
>>  	ret = panthor_request_mmu_irq(ptdev, &mmu->irq, irq,
>> -				      panthor_mmu_fault_mask(ptdev, ~0));
>> +				      panthor_mmu_fault_mask(ptdev, ~0),
>> +				      ptdev->iomem + MMU_INT_BASE);
>>  	if (ret)
>>  		return ret;
>>  
> [...]
>> diff --git a/drivers/gpu/drm/panthor/panthor_pwr.c b/drivers/gpu/drm/panthor/panthor_pwr.c
>> index aafb0c5c7d23..11c43de1ddd5 100644
>> --- a/drivers/gpu/drm/panthor/panthor_pwr.c
>> +++ b/drivers/gpu/drm/panthor/panthor_pwr.c
>> @@ -70,7 +70,7 @@ static void panthor_pwr_irq_handler(struct panthor_device *ptdev, u32 status)
>>  	}
>>  	spin_unlock(&ptdev->pwr->reqs_lock);
>>  }
>> -PANTHOR_IRQ_HANDLER(pwr, PWR, panthor_pwr_irq_handler);
>> +PANTHOR_IRQ_HANDLER(pwr, panthor_pwr_irq_handler);
>>  
>>  static void panthor_pwr_write_command(struct panthor_device *ptdev, u32 command, u64 args)
>>  {
>> @@ -464,7 +464,9 @@ int panthor_pwr_init(struct panthor_device *ptdev)
>>  	if (irq < 0)
>>  		return irq;
>>  
>> -	err = panthor_request_pwr_irq(ptdev, &pwr->irq, irq, PWR_INTERRUPTS_MASK);
>> +	err = panthor_request_pwr_irq(
>> +		ptdev, &pwr->irq, irq, PWR_INTERRUPTS_MASK,
>> +		ptdev->iomem + GPU_CONTROL_BASE + PWR_CONTROL_BASE);
> 
> This one is the odd one out because it adds GPU_CONTROL_BASE put the
> other panthor_request_xxx_irq() calls don't. Sashiko also points out
> that there's an argument these should all be using ptdev->gpu->iomem in
> the final refactor.
> 

I understand that it is slightly different, it is only there to
illustrate the fact that it is a child of the GPU_CONTROL register page.
w.r.t using ptdev->gpu->iomem, that would mean that we need reach into
the panthor_gpu component to access the memory, hence why we used a
separate iomem for this one.

Kind regards,
Karunika


> Thanks,
> Steve
> 
>>  	if (err)
>>  		return err;
>>  
>