Re: [PATCH v3 2/2] arm64: dts: imx95: Describe Mali G310 GPU

[Marek Vasut <marek.vasut@mailbox.org>]

On 9/29/25 6:20 PM, Rain Yang wrote:
> On Mon, Sep 29, 2025 at 03:09:01PM +0200, Marek Vasut wrote:
>> On 9/29/25 11:57 AM, Rain Yang wrote:
>>> On Mon, Sep 29, 2025 at 02:23:01AM +0200, Marek Vasut wrote:
>>>> On 9/26/25 7:57 AM, Peng Fan wrote:
>>>>
>>>> Hello Peng,
>>>>
>>>>> On Thu, Sep 25, 2025 at 10:38:31PM +0200, Marek Vasut wrote:
>>>>>> The instance of the GPU populated in i.MX95 is the G310, describe this
>>>>>> GPU in the DT. Include dummy GPU voltage regulator and OPP tables.
>>>>>>
>>>>>>
>>>>>> +		gpu: gpu@4d900000 {
>>>>>> +			compatible = "nxp,imx95-mali", "arm,mali-valhall-csf";
>>>>>> +			reg = <0 0x4d900000 0 0x480000>;
>>>>>> +			clocks = <&scmi_clk IMX95_CLK_GPU>, <&scmi_clk IMX95_CLK_GPUAPB>;
>>>>>> +			clock-names = "core", "coregroup";
>>>>>> +			interrupts = <GIC_SPI 289 IRQ_TYPE_LEVEL_HIGH>,
>>>>>> +				     <GIC_SPI 290 IRQ_TYPE_LEVEL_HIGH>,
>>>>>> +				     <GIC_SPI 288 IRQ_TYPE_LEVEL_HIGH>;
>>>>>> +			interrupt-names = "job", "mmu", "gpu";
>>>>>> +			mali-supply = <&gpu_fixed_reg>;
>>>>>> +			operating-points-v2 = <&gpu_opp_table>;
>>>>>> +			power-domains = <&scmi_devpd IMX95_PD_GPU>;
>>>>>> +			#cooling-cells = <2>;
>>>>>> +			dynamic-power-coefficient = <1013>;
>>>>>
>>>>> Sorry for my ignorance, would you please share how to get the value?
>>>> Copy-pasted from NXP downstream kernel fork DT bindings, see:
>>>>
>>>> https://github.com/nxp-imx/linux-imx.git
>>>>
>>>> 11495de7c24a ("MGS-7621-4 dts: gpu: update devfreq para")
>>> Hi Marek,
>>>
>>> 1. this "mali: gpu@4d900000" label can be found in this commit you showed.
>>> please correct this to be compatible with the downstream
>>
>> No, sorry, that's not how it works. Upstream is not being adjusted to match
>> decisions made by downstream kernel forks unless there is a good rationale
>> for such a change. "Downstream does this" is not a good one. (*)
>>
>>> and upstream kernel
>>
>> All of imx*.dts* use gpu: or gpu2d:/gpu3d:/vpuvg: for the GPU label.
>> Also, variants of gpu: label seems more popular:
>>
>> linux$ grep -hro '[a-z0-9_]\+: gpu@' arch/ | sort | uniq -c
>>       3 adreno_gpu: gpu@
>>       1 bb2d: gpu@
>>       1 gpu2d: gpu@
>>       1 gpu3d: gpu@
>>      80 gpu: gpu@
>>       4 gpu_2d: gpu@
>>       1 gpu_3d0: gpu@
>>       4 gpu_3d: gpu@
>>       6 gpu_mem: gpu@
>>       1 gpu_reserved: gpu@
>>       2 gpu_vg: gpu@
>>      17 mali: gpu@
>>       1 v3d: gpu@
>>       2 zap_shader_region: gpu@
>>
> 
> Existence does not necessarily imply validity. Since a single SoC may contain
> multiple GPUs, it's generally better to use the specific GPU name as a label
> rather than simply using 'gpu', to avoid potential conflicts.

Does the MX95 contain multiple GPUs ?

If no, then the statement above does not apply.

If yes, then there is the gpu/gpu2d/gpu3d option.

>>> 2. the compatible string is different from our downstream kernel,
>>
>> See above (*)
> Additionally, there are still some performance differences between the upstream
> driver and the Mali property driver. If compatibility with both can be achieved,
> it would allow users to choose the driver that best suits their needs.
> Personally, I find it convenient to switch between the two drivers using insmod
> and rmmod, which allows for quick testing and comparison.

Update the downstream fork to match upstream bindings, not the other way 
around.

>>> also you dropped the "nxp,imx95-mali" compatible patch in the panthor
>>> driver, why?
>>
>> Because it is unnecessary, the generic compatible string is sufficient for
>> the in-tree kernel driver.
>>
>>> this will impact the mali property driver too, which
>>> has already been used in many customer project.
>>
>> See above (*)
>>
>> All the more reason to focus on upstream and avoid deployment of various
>> downstream components, blobs and so on. They cannot be maintained in the long
>> run, they break, and cause all kinds of maintenance problems.
>>
>> Upstream cannot be hindered by downstream blobs and their issues, sorry.
>>
>>> 3. the number of frequency in opp-table is only one, but there are two clocks
>>> in clocks property, this really make people confused.
>>> CLK/CLK_COREGROUP/CLK_STACK in i.MX95 are from the same source
>>> <&scmi_clk IMX95_CLK_GPU>, the other clock <&scmi_clk IMX95_CLK_GPUAPB>
>>> is always-on APB clock, which can't be changed by A-cores, and has been removed
>>> from clocks property in the latest release.
>>
>> Can the APB clock be enabled/disabled from Linux, e.g. to save power ?
> 
> please note that the APB clock can't be turned off on the A-core.

I think if the clock are connected to the GPU, then they should be 
described in the DT.