RE: Re: Re: [PATCH 1/2] dt-bindings: gpu: mali-valhall-csf: Document i.MX952 support

[Guangliu Ding]

Hi Liviu

> On Wed, Apr 01, 2026 at 09:43:12AM +0000, Guangliu Ding wrote:
> > Hi Daniel
> >
> > > On 4/1/26 11:48, Guangliu Ding wrote:
> > > > [You don't often get email from guangliu.ding@xxxxxxx. Learn why
> > > > this is important at https://aka.ms/LearnAboutSenderIdentification
> > > > ]
> > > >
> > > > Hi Liviu
> > > >
> > > > Thanks for your review. Please refer to my comments below:
> > > >
> > > >> On Tue, Mar 31, 2026 at 06:12:38PM +0800, Guangliu Ding wrote:
> > > >>> Add compatible string of Mali G310 GPU on i.MX952 board.
> > > >>>
> > > >>> Signed-off-by: Guangliu Ding <guangliu.ding@xxxxxxx>
> > > >>> Reviewed-by: Jiyu Yang <jiyu.yang@xxxxxxx>
> > > >>> ---
> > > >>>  Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> > > >>> | 1
> > > >>> +
> > > >>>  1 file changed, 1 insertion(+)
> > > >>>
> > > >>> diff --git
> > > >>> a/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yam
> > > >>> l
> > > >> b/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yaml
> > > >>> index 8eccd4338a2b..6a10843a26e2 100644
> > > >>> ---
> > > >>> a/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf.yam
> > > >>> l
> > > >>> +++ b/Documentation/devicetree/bindings/gpu/arm,mali-valhall-csf
> > > >>> +++ .yam
> > > >>> +++ l
> > > >>> @@ -20,6 +20,7 @@ properties:
> > > >>>            - enum:
> > > >>>                - mediatek,mt8196-mali
> > > >>>                - nxp,imx95-mali            # G310
> > > >>> +              - nxp,imx952-mali           # G310
> > > >> Can you explain why this is needed? Can it not be covered by the
> > > >> existing compatible?
> > > > There are functional differences in GPU module (GPUMIX) between
> > > > i.MX95 and i.MX952. So they cannot be fully covered by a single existing
> compatible.
> > > > On i.MX952, The GPU clock is controlled by hardware GPU auto
> > > > clock-gating mechanism, while the GPU clock is managed explicitly
> > > > by the
> > > driver on i.MX95.
> > > > Because of these behavioral differences, separate compatible
> > > > strings "nxp,imx95-mali" and "nxp,imx952-mali" are needed to allow
> > > > the driver to handle the two variants independently and to keep
> > > > room for future
> > > divergence.
> > >
> > >
> > > This information should be added in the commit message explaining
> > > why
> > >
> > > the change is needed.
> > >
> > >
> > > But then where is the driver code taking care of these diferences?
> > >
> >
> > Yes. Currently the driver does not require "nxp,imx952-mali" string.
> > However, when GPU ipa_counters are enabled to calculate the GPU
> > busy_time/idle_time for GPU DVFS feature, they will conflict with the
> > hardware GPU auto clock‑gating mechanism, causing GPU clock to remain
> always on.
> > In such cases, ipa_counters need to be disabled so that the GPU auto
> > clock‑gating mechanism can operate normally, using "nxp,imx952-mali"
> string.
> 
> OK, I understand that you're following guidance from some other senior
> people on how to upstream patches so you've tried to create the smallest
> patchset to ensure that it gets reviewed and accepted, but in this case we need
> to see the other patches as well to decide if your approach is the right one and
> we do need a separate compatible string.
> 
> If enabling GPU ipa_counters causes the clocks to get stuck active, that feels
> like a hardware bug, so figuring out how to handle that is more important than
> adding a compatible string.
> 
> Either add the patch(es) that use the compatible to this series in v2, or put a
> comment in the commit message on where we can see the driver changes.
> 

According to discussions with the GPU vendor, this is a hardware limitation 
of Mali-G310 rather than a hardware bug, and it has been addressed in newer
Mali GPU families.

In addition, ipa_counters are not enabled in the current Panthor driver. We observed
this issue with the private Mali DDK where ipa_counters were enabled.
Therefore, keeping the compatible string is necessary to allow for future divergence.

> Best regards,
> Liviu
> 
> --
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     ¯\_(ツ)_/¯