Re: [RFC PATCH 2/2] drm/panthor: add initial mt8196 support

[Florent Tomasin <florent.tomasin@arm.com>]

On 05/09/2025 00:06, Chia-I Wu wrote:
> On Wed, Sep 3, 2025 at 11:02 PM Boris Brezillon
> <boris.brezillon@collabora.com> wrote:
>>
>> On Wed,  3 Sep 2025 15:55:04 -0700
>> Chia-I Wu <olvaffe@gmail.com> wrote:
>>
>>> diff --git a/drivers/gpu/drm/panthor/Makefile b/drivers/gpu/drm/panthor/Makefile
>>> index 02db21748c125..75e92c461304b 100644
>>> --- a/drivers/gpu/drm/panthor/Makefile
>>> +++ b/drivers/gpu/drm/panthor/Makefile
>>> @@ -12,4 +12,6 @@ panthor-y := \
>>>       panthor_mmu.o \
>>>       panthor_sched.o
>>>
>>> +panthor-$(CONFIG_DRM_PANTHOR_SOC_MT8196) += panthor_soc_mt8196.o
>>
>> Based on the stuff you describe (ASN hash, core mask read from an nvmem
>> cell, extra clks/regulators?), I don't think we need per-soc source
>> files and per-soc config options. If it becomes too HW specific (no
>> abstraction to make it SoC-agnostic), we can reconsider the per-SoC
>> file approach, but I believe it can all live in panthor_drv.c for now.
> That's about right except no extra clk/regulator is needed.
> 
> gpueb on mt8196 is yet another mcu running on its own fw.  It can
> provide clk/regulator to panthor and no change is needed from panthor.
> But it can also do dvfs autonomously, in which case panthor needs to
> be modified to make clk/regulator/devfreq optional.  I think the
> latter is where Nicolas Frattaroli is going and requires more invasive
> integration.

Hi Chia-I Wu,

I beleive the changes your are proposing need to be treated as 3
different features:

1. Handling of ASN hash
2. clk/regulator registration
3. Core Mask control in Panthor

* For 1. since it is a GPU HW property, have you considered
handling it in the DTB as HW quirk?

Pass the value of the register from the DTB, then Panthor
driver applies a mask to filter the bits and then configre the HW regs

> The clk/regulator provider on MT8196 is GPUEB, whose driver[1] needs to
> be cleaned up and upstreamed separately.

* For 2. Have you considered making the GPUEB MTK driver act as
a clock/regulator supplier?

Without a clocks, the driver won't be able to support
instrumenation.

And without a regulator or clock, how will system/runtime PM
suspend/resume be handled in the driver?

* For 3. I think this is a more complex topic and other vendors may
have different needs.

At first glance, `panthor_soc_data` does not seem to be sufficient
to cope with other vendors.

What would be the protocole used to communicate wth MCU?
Can the MCU update the core mask on its own?
Will the core mask change at runtime?

If the core mask is static, what about adding a DTB entry?

Kind regards,
Florent