Re: [PATCH 3/3] arm64: dts: mediatek: mt8188: Add the MFGCFG phandle to the GPU

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/panfrost: missing GPU timestamp quirk on MT8370
--

commit ad326344199c9c294e6415c92318bcef828d8630
Author: Michał Kopeć <michal@nozomi.space>

arm64: dts: mediatek: mt8188: Add the MFGCFG phandle to the GPU

This commit adds the MFGCFG phandle to the GPU node in the MT8188 device
tree. The GPU driver requires this configuration block to enable the GPU
timestamp counter, which otherwise reads zero.

> diff --git a/arch/arm64/boot/dts/mediatek/mt8188.dtsi b/arch/arm64/boot/dts/mediatek/mt8188.dtsi
> index 14a320f0c70cb..b4be89bf55bf7 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8188.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8188.dtsi
> @@ -2266,6 +2266,7 @@ gpu: gpu@13000000 {
>  				     <GIC_SPI 382 IRQ_TYPE_LEVEL_HIGH 0>,
>  				     <GIC_SPI 381 IRQ_TYPE_LEVEL_HIGH 0>;
>  			interrupt-names = "job", "mmu", "gpu";
> +			mediatek,mfgcfg = <&mfgcfg>;

[Severity: Medium]
Since we are enabling the GPU timestamp counter via the MFG configuration
block for the MT8188, does the MT8370 SoC also need its corresponding quirk
updated in the panfrost driver to avoid breaking userspace timestamp queries?

Looking at drivers/gpu/drm/panfrost/panfrost_drv.c, the mediatek_mt8370_data
struct is defined as:

static const struct panfrost_compatible mediatek_mt8370_data = {
	.num_supplies = ARRAY_SIZE(default_supplies) - 1,
	.supply_names = default_supplies,
	.num_pm_domains = 2,
	.pm_domain_names = mediatek_pm_domains,
	.pm_features = BIT(GPU_PM_CLK_DIS) | BIT(GPU_PM_VREG_OFF),
	.gpu_quirks = BIT(GPU_QUIRK_FORCE_AARCH64_PGTABLE),
};

This struct lacks .vendor_quirk = panfrost_gpu_mt8188_quirk.

When the driver probes on MT8370 using the mediatek,mt8370-mali compatible,
the missing quirk means the MFG configuration block is never enabled. As a
result, the GPU timestamp counter will read zero on the MT8370 SoC.

Could this be an omission, or is it intentional to skip the vendor quirk
for MT8370?

>  			nvmem-cells = <&gpu_speedbin>;
>  			nvmem-cell-names = "speed-bin";
>  			operating-points-v2 = <&gpu_opp_table>;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261003200132.866240-1-michal@nozomi.space?part=3