Hi,
On Wed, 2025-05-07 at 15:06 +0100, Steven Price wrote:
> On 02/05/2025 13:17, Louis-Alexis Eyraud wrote:
> > Add a compatible for the MediaTek MT8370 SoC, with an integrated
> > ARM
> > Mali G57 MC2 GPU (Valhall-JM, dual core), with new platform data
> > for
> > its support in the panfrost driver.
> > It uses the same data as MT8186 for the power management features
> > to
> > describe power supplies, pm_domains and enablement (one regulator,
> > two
> > power domains) but also sets the FORCE_AARCH64_PGTABLE flag in the
> > GPU
> > configuration quirks bitfield to enable AARCH64 4K page table
> > format
> > mode.
> > As MT8186 and MT8370 SoC have different GPU architecture (Mali G52
> > 2EE
> > MC2 for MT8186), making them not compatible, and this mode is only
> > enabled for Mediatek SoC that are Mali G57 based (compatible with
> > mediatek,mali-mt8188 or mediatek,mali-8192), having specific
> > platform
> > data allows to set this flag for MT8370 without modifying MT8186
> > configuration and behaviour.
> >
> > Signed-off-by: Louis-Alexis Eyraud
> > <louisalexis.eyr...@collabora.com>
>
> With one minor comment fixed below:
>
> Reviewed-by: Steven Price <steven.pr...@arm.com>
>
> > ---
> > drivers/gpu/drm/panfrost/panfrost_drv.c | 11 +++++++++++
> > 1 file changed, 11 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c
> > b/drivers/gpu/drm/panfrost/panfrost_drv.c
> > index
> > f1ec3b02f15a0029d20c7d81046ded59854e885c..8e0a1ae6940c73b7b60233950
> > ae3abdfa843cc8e 100644
> > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
> > @@ -846,6 +846,16 @@ static const struct panfrost_compatible
> > mediatek_mt8192_data = {
> > .gpu_quirks = BIT(GPU_QUIRK_FORCE_AARCH64_PGTABLE),
> > };
> >
> > +/* MT8370 uses the same power domains and power supplies as MT8186
> > */
>
> This comment is not correct - you've got the power domains of MT8186,
> but the supplies of MT8183. The comment doesn't actually add much so
> you
> could just drop it.
>
I'll remove it.
> If you're feeling adventurous then one option here is to actually
> clean
> up the mediatek entries a little. Instead of referring to particular
> part numbers we could have:
>
> static const char * const mediatek_2_pm_domains[] = { "core0",
> "core1" };
> static const char * const mediatek_3_pm_domains[] = { "core0",
> "core1", "core2" };
> static const char * const mediatek_5_pm_domains[] = { "core0",
> "core1", "core2",
> "core3",
> "core4" };
>
> Or even just have the mediatek_5_domains[] array (dropping the '5' in
> the name) and not use ARRAY_SIZE().
>
> Equally the supplies arrays could be renamed. We have the one with
> "sram" for legacy and everything else uses {"mali", NULL} but we have
> two definitions for it (mt8183_b and mt8192).
>
> Thanks,
> Steve
I'll do the array cleaning as well in a separate patch in the v6.
Regards,
Louis-Alexis
>
> > +static const struct panfrost_compatible mediatek_mt8370_data = {
> > + .num_supplies = ARRAY_SIZE(mediatek_mt8183_b_supplies) -
> > 1,
> > + .supply_names = mediatek_mt8183_b_supplies,
> > + .num_pm_domains = ARRAY_SIZE(mediatek_mt8186_pm_domains),
> > + .pm_domain_names = mediatek_mt8186_pm_domains,
> > + .pm_features = BIT(GPU_PM_CLK_DIS) | BIT(GPU_PM_VREG_OFF),
> > + .gpu_quirks = BIT(GPU_QUIRK_FORCE_AARCH64_PGTABLE),
> > +};
> > +
> > static const struct of_device_id dt_match[] = {
> > /* Set first to probe before the generic compatibles */
> > { .compatible = "amlogic,meson-gxm-mali",
> > @@ -868,6 +878,7 @@ static const struct of_device_id dt_match[] = {
> > { .compatible = "mediatek,mt8186-mali", .data =
> > &mediatek_mt8186_data },
> > { .compatible = "mediatek,mt8188-mali", .data =
> > &mediatek_mt8188_data },
> > { .compatible = "mediatek,mt8192-mali", .data =
> > &mediatek_mt8192_data },
> > + { .compatible = "mediatek,mt8370-mali", .data =
> > &mediatek_mt8370_data },
> > { .compatible = "allwinner,sun50i-h616-mali", .data =
> > &allwinner_h616_data },
> > {}
> > };
> >
