| Message ID | 20250116-mt8370-enable-gpu-v1-2-0a6b78e925c8@collabora.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add Mali GPU support for Mediatek MT8370 SoC | expand |
On 16/01/2025 14:25, Louis-Alexis Eyraud wrote: > This commit adds a compatible for the MediaTek MT8370 SoC, with an > integrated ARM Mali G57 MC2 GPU (Valhall-JM, dual core), and adds > platform data using the same supplies and the same power domain lists > as MT8186 (one regulator, two power domains). > > Signed-off-by: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com> Reviewed-by: Steven Price <steven.price@arm.com> > --- > drivers/gpu/drm/panfrost/panfrost_drv.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c > index 0f3935556ac761adcd80197d87e8e478df436fd5..1d51b64ed0f0660cc95263a289d5dad204540cfd 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c > @@ -837,6 +837,15 @@ static const struct panfrost_compatible mediatek_mt8192_data = { > .pm_features = BIT(GPU_PM_CLK_DIS) | BIT(GPU_PM_VREG_OFF), > }; > > +/* MT8370 uses the same power domains and power supplies as MT8186 */ > +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), > +}; > + > static const struct of_device_id dt_match[] = { > /* Set first to probe before the generic compatibles */ > { .compatible = "amlogic,meson-gxm-mali", > @@ -859,6 +868,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 }, > {} > }; > MODULE_DEVICE_TABLE(of, dt_match); >
On Thu, Jan 16, 2025 at 03:25:58PM +0100, Louis-Alexis Eyraud wrote: > This commit adds a compatible for the MediaTek MT8370 SoC, with an > integrated ARM Mali G57 MC2 GPU (Valhall-JM, dual core), and adds > platform data using the same supplies and the same power domain lists > as MT8186 (one regulator, two power domains). > > Signed-off-by: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com> > --- > drivers/gpu/drm/panfrost/panfrost_drv.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c > index 0f3935556ac761adcd80197d87e8e478df436fd5..1d51b64ed0f0660cc95263a289d5dad204540cfd 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c > @@ -837,6 +837,15 @@ static const struct panfrost_compatible mediatek_mt8192_data = { > .pm_features = BIT(GPU_PM_CLK_DIS) | BIT(GPU_PM_VREG_OFF), > }; > > +/* MT8370 uses the same power domains and power supplies as MT8186 */ > +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), > +}; No, people, stop this nonsense. This is exactly the same as previous. Don't duplicate entries just because you want a commit. > + > static const struct of_device_id dt_match[] = { > /* Set first to probe before the generic compatibles */ > { .compatible = "amlogic,meson-gxm-mali", > @@ -859,6 +868,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 }, No, express properly compatibility or say in bindings commit msg why devices are not compatible. Best regards, Krzysztof
Hello, sorry for the delay, ---- On Sat, 18 Jan 2025 17:08:10 +0100 Krzysztof Kozlowski wrote --- > On Thu, Jan 16, 2025 at 03:25:58PM +0100, Louis-Alexis Eyraud wrote: > > This commit adds a compatible for the MediaTek MT8370 SoC, with an > > integrated ARM Mali G57 MC2 GPU (Valhall-JM, dual core), and adds > > platform data using the same supplies and the same power domain lists > > as MT8186 (one regulator, two power domains). > > > > Signed-off-by: Louis-Alexis Eyraud louisalexis.eyraud@collabora.com> > > --- > > drivers/gpu/drm/panfrost/panfrost_drv.c | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c > > index 0f3935556ac761adcd80197d87e8e478df436fd5..1d51b64ed0f0660cc95263a289d5dad204540cfd 100644 > > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c > > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c > > @@ -837,6 +837,15 @@ static const struct panfrost_compatible mediatek_mt8192_data = { > > .pm_features = BIT(GPU_PM_CLK_DIS) | BIT(GPU_PM_VREG_OFF), > > }; > > > > +/* MT8370 uses the same power domains and power supplies as MT8186 */ > > +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), > > +}; > > No, people, stop this nonsense. This is exactly the same as previous. > Don't duplicate entries just because you want a commit. > I added this new compatible in bindings and panfrost driver because there were no other matching compatible Using another mali-vallhal-jm compatible would make the driver probe fail because of power domains number difference. Using mt8186-mali compatible would work without modifications but as it is not the same architecture (mali-bifrost), it would be incorrect. I've also misguessed on the dt_match array modifications, sorry. I'll amend this patch in order to reuse the mt8186 platform data instead. > > + > > static const struct of_device_id dt_match[] = { > > /* Set first to probe before the generic compatibles */ > > { .compatible = "amlogic,meson-gxm-mali", > > @@ -859,6 +868,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 }, > > No, express properly compatibility or say in bindings commit msg why > devices are not compatible. > I'll reword in V2 the commit messages to make the compatible need more explicit. > Best regards, > Krzysztof > > Regards, Louis-Alexis Eyraud
On 30/01/2025 13:15, Louis-Alexis Eyraud wrote: > Hello, > > sorry for the delay, > > ---- On Sat, 18 Jan 2025 17:08:10 +0100 Krzysztof Kozlowski wrote --- > > On Thu, Jan 16, 2025 at 03:25:58PM +0100, Louis-Alexis Eyraud wrote: > > > This commit adds a compatible for the MediaTek MT8370 SoC, with an > > > integrated ARM Mali G57 MC2 GPU (Valhall-JM, dual core), and adds > > > platform data using the same supplies and the same power domain lists > > > as MT8186 (one regulator, two power domains). > > > > > > Signed-off-by: Louis-Alexis Eyraud louisalexis.eyraud@collabora.com> > > > --- > > > drivers/gpu/drm/panfrost/panfrost_drv.c | 10 ++++++++++ > > > 1 file changed, 10 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c > > > index 0f3935556ac761adcd80197d87e8e478df436fd5..1d51b64ed0f0660cc95263a289d5dad204540cfd 100644 > > > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c > > > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c > > > @@ -837,6 +837,15 @@ static const struct panfrost_compatible mediatek_mt8192_data = { > > > .pm_features = BIT(GPU_PM_CLK_DIS) | BIT(GPU_PM_VREG_OFF), > > > }; > > > > > > +/* MT8370 uses the same power domains and power supplies as MT8186 */ > > > +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), > > > +}; > > > > No, people, stop this nonsense. This is exactly the same as previous. > > Don't duplicate entries just because you want a commit. > > > I added this new compatible in bindings and panfrost driver because there were no other matching compatible > Using another mali-vallhal-jm compatible would make the driver probe fail because of power domains number difference. > Using mt8186-mali compatible would work without modifications but as it is not the same architecture (mali-bifrost), it would be incorrect. Fix your email app, so it won't add spaces before quote and will wrap the text properly. > > I've also misguessed on the dt_match array modifications, sorry. > I'll amend this patch in order to reuse the mt8186 platform data instead. > > > > + > > > static const struct of_device_id dt_match[] = { > > > /* Set first to probe before the generic compatibles */ > > > { .compatible = "amlogic,meson-gxm-mali", > > > @@ -859,6 +868,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 }, > > > > No, express properly compatibility or say in bindings commit msg why > > devices are not compatible. > > > I'll reword in V2 the commit messages to make the compatible need more explicit. Your commit msg should then explain that this is not compatible with mt8186 because programming model or architecture is different. Number of power domains rarely matters for actual compatibility and as easily visible in panfrost driver: does not matter here, either. Best regards, Krzysztof
On 30/01/2025 13:15, Louis-Alexis Eyraud wrote: > Hello, > > sorry for the delay, You gave yourself 12 days to respond, which is fine. But to me, you gave 15 minutes and immediately sent v2. So now your v2 will be still rejected. Best regards, Krzysztof
diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c index 0f3935556ac761adcd80197d87e8e478df436fd5..1d51b64ed0f0660cc95263a289d5dad204540cfd 100644 --- a/drivers/gpu/drm/panfrost/panfrost_drv.c +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c @@ -837,6 +837,15 @@ static const struct panfrost_compatible mediatek_mt8192_data = { .pm_features = BIT(GPU_PM_CLK_DIS) | BIT(GPU_PM_VREG_OFF), }; +/* MT8370 uses the same power domains and power supplies as MT8186 */ +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), +}; + static const struct of_device_id dt_match[] = { /* Set first to probe before the generic compatibles */ { .compatible = "amlogic,meson-gxm-mali", @@ -859,6 +868,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 }, {} }; MODULE_DEVICE_TABLE(of, dt_match);
This commit adds a compatible for the MediaTek MT8370 SoC, with an integrated ARM Mali G57 MC2 GPU (Valhall-JM, dual core), and adds platform data using the same supplies and the same power domain lists as MT8186 (one regulator, two power domains). Signed-off-by: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com> --- drivers/gpu/drm/panfrost/panfrost_drv.c | 10 ++++++++++ 1 file changed, 10 insertions(+)