2021-01-26 02:50:16

by Nicolas Boichat

[permalink] [raw]
Subject: [PATCH v11 0/4] drm/panfrost: Add support for mt8183 GPU

Hi!

Follow-up on the v5 [1], things have gotten significantly
better in the last 9 months, thanks to the efforts on Bifrost
support by the Collabora team (and probably others I'm not
aware of).

I've been testing this series on a MT8183/kukui device, with a
chromeos-5.10 kernel [2], and got basic Chromium OS UI up with
mesa 20.3.2 (lots of artifacts though).

devfreq is currently not supported, as we'll need:
- Clock core support for switching the GPU core clock (see 2/4).
- Platform-specific handling of the 2-regulator (see 3/4).

Since the latter is easy to detect, patch 3/4 just disables
devfreq if the more than one regulator is specified in the
compatible matching table.

[1] https://patchwork.kernel.org/project/linux-mediatek/cover/[email protected]/
[2] https://crrev.com/c/2608070

Changes in v11:
- binding: power-domain-names not power-domainS-names
- mt8183*.dts: remove incorrect supply-names

Changes in v10:
- Fix the binding to make sure sram-supply property can be provided.

Changes in v9:
- Explain why devfreq needs to be disabled for GPUs with >1
regulators.

Changes in v8:
- Use DRM_DEV_INFO instead of ERROR

Changes in v7:
- Fix GPU ID in commit message
- Fix GPU ID in commit message

Changes in v6:
- Rebased, actually tested with recent mesa driver.
- Add gpu regulators to kukui dtsi as well.
- Power domains are now attached to spm, not scpsys
- Drop R-B.
- devfreq: New change
- Context conflicts, reflow the code.
- Use ARRAY_SIZE for power domains too.

Changes in v5:
- Rename "2d" power domain to "core2"
- Rename "2d" power domain to "core2" (keep R-B again).
- Change power domain name from 2d to core2.

Changes in v4:
- Add power-domain-names description
(kept Alyssa's reviewed-by as the change is minor)
- Add power-domain-names to describe the 3 domains.
(kept Alyssa's reviewed-by as the change is minor)
- Add power domain names.

Changes in v3:
- Match mt8183-mali instead of bifrost, as we require special
handling for the 2 regulators and 3 power domains.

Changes in v2:
- Use sram instead of mali_sram as SRAM supply name.
- Rename mali@ to gpu@.

Nicolas Boichat (4):
dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183
arm64: dts: mt8183: Add node for the Mali GPU
drm/panfrost: devfreq: Disable devfreq when num_supplies > 1
drm/panfrost: Add mt8183-mali compatible string

.../bindings/gpu/arm,mali-bifrost.yaml | 28 +++++
arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 5 +
.../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 5 +
arch/arm64/boot/dts/mediatek/mt8183.dtsi | 105 ++++++++++++++++++
drivers/gpu/drm/panfrost/panfrost_devfreq.c | 9 ++
drivers/gpu/drm/panfrost/panfrost_drv.c | 10 ++
6 files changed, 162 insertions(+)

--
2.30.0.280.ga3ce27912f-goog


2021-01-26 10:31:32

by Nicolas Boichat

[permalink] [raw]
Subject: [PATCH v11 1/4] dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183

Define a compatible string for the Mali Bifrost GPU found in
Mediatek's MT8183 SoCs.

Signed-off-by: Nicolas Boichat <[email protected]>
---

Changes in v11:
- binding: power-domain-names not power-domainS-names

Changes in v10:
- Fix the binding to make sure sram-supply property can be provided.

Changes in v9: None
Changes in v8: None
Changes in v7: None
Changes in v6:
- Rebased, actually tested with recent mesa driver.

Changes in v5:
- Rename "2d" power domain to "core2"

Changes in v4:
- Add power-domain-names description
(kept Alyssa's reviewed-by as the change is minor)

Changes in v3: None
Changes in v2: None

.../bindings/gpu/arm,mali-bifrost.yaml | 28 +++++++++++++++++++
1 file changed, 28 insertions(+)

diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml
index 184492162e7e..3e758f88e2cd 100644
--- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml
+++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml
@@ -17,6 +17,7 @@ properties:
items:
- enum:
- amlogic,meson-g12a-mali
+ - mediatek,mt8183-mali
- realtek,rtd1619-mali
- rockchip,px30-mali
- const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully discoverable
@@ -41,6 +42,8 @@ properties:

mali-supply: true

+ sram-supply: true
+
operating-points-v2: true

power-domains:
@@ -87,6 +90,31 @@ allOf:
then:
required:
- resets
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: mediatek,mt8183-mali
+ then:
+ properties:
+ power-domains:
+ description:
+ List of phandle and PM domain specifier as documented in
+ Documentation/devicetree/bindings/power/power_domain.txt
+ minItems: 3
+ maxItems: 3
+ power-domain-names:
+ items:
+ - const: core0
+ - const: core1
+ - const: core2
+ required:
+ - sram-supply
+ - power-domains
+ - power-domain-names
+ else:
+ properties:
+ sram-supply: false

examples:
- |
--
2.30.0.280.ga3ce27912f-goog

2021-01-26 10:31:52

by Nicolas Boichat

[permalink] [raw]
Subject: [PATCH v11 4/4] drm/panfrost: Add mt8183-mali compatible string

Add support for MT8183's G72 Bifrost.

Signed-off-by: Nicolas Boichat <[email protected]>
Reviewed-by: Tomeu Vizoso <[email protected]>
Reviewed-by: Steven Price <[email protected]>
---

Changes in v11: None
Changes in v10: None
Changes in v9: None
Changes in v8: None
Changes in v7:
- Fix GPU ID in commit message

Changes in v6:
- Context conflicts, reflow the code.
- Use ARRAY_SIZE for power domains too.

Changes in v5:
- Change power domain name from 2d to core2.

Changes in v4:
- Add power domain names.

Changes in v3:
- Match mt8183-mali instead of bifrost, as we require special
handling for the 2 regulators and 3 power domains.

Changes in v2: None

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 83a461bdeea8..ca07098a6141 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -665,6 +665,15 @@ static const struct panfrost_compatible amlogic_data = {
.vendor_quirk = panfrost_gpu_amlogic_quirk,
};

+const char * const mediatek_mt8183_supplies[] = { "mali", "sram" };
+const char * const mediatek_mt8183_pm_domains[] = { "core0", "core1", "core2" };
+static const struct panfrost_compatible mediatek_mt8183_data = {
+ .num_supplies = ARRAY_SIZE(mediatek_mt8183_supplies),
+ .supply_names = mediatek_mt8183_supplies,
+ .num_pm_domains = ARRAY_SIZE(mediatek_mt8183_pm_domains),
+ .pm_domain_names = mediatek_mt8183_pm_domains,
+};
+
static const struct of_device_id dt_match[] = {
/* Set first to probe before the generic compatibles */
{ .compatible = "amlogic,meson-gxm-mali",
@@ -681,6 +690,7 @@ static const struct of_device_id dt_match[] = {
{ .compatible = "arm,mali-t860", .data = &default_data, },
{ .compatible = "arm,mali-t880", .data = &default_data, },
{ .compatible = "arm,mali-bifrost", .data = &default_data, },
+ { .compatible = "mediatek,mt8183-mali", .data = &mediatek_mt8183_data },
{}
};
MODULE_DEVICE_TABLE(of, dt_match);
--
2.30.0.280.ga3ce27912f-goog

2021-01-26 10:32:11

by Nicolas Boichat

[permalink] [raw]
Subject: [PATCH v11 3/4] drm/panfrost: devfreq: Disable devfreq when num_supplies > 1

GPUs with more than a single regulator (e.g. G72 on MT8183) will
require platform-specific handling for devfreq, for 2 reasons:
1. The opp core (drivers/opp/core.c:_generic_set_opp_regulator)
does not support multiple regulators, so we'll need custom
handlers.
2. Generally, platforms with 2 regulators have platform-specific
constraints on how the voltages should be set (e.g.
minimum/maximum voltage difference between them), so we
should not just create generic handlers that simply
change the voltages without taking care of those constraints.

Disable devfreq for now on those GPUs.

Signed-off-by: Nicolas Boichat <[email protected]>
Reviewed-by: Tomeu Vizoso <[email protected]>
Reviewed-by: Steven Price <[email protected]>
---

Changes in v11: None
Changes in v10: None
Changes in v9:
- Explain why devfreq needs to be disabled for GPUs with >1
regulators.

Changes in v8:
- Use DRM_DEV_INFO instead of ERROR

Changes in v7:
- Fix GPU ID in commit message

Changes in v6:
- devfreq: New change

Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None

drivers/gpu/drm/panfrost/panfrost_devfreq.c | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
index 7c5ffc81dce1..a544ae1e560a 100644
--- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
+++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
@@ -93,6 +93,15 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
struct thermal_cooling_device *cooling;
struct panfrost_devfreq *pfdevfreq = &pfdev->pfdevfreq;

+ if (pfdev->comp->num_supplies > 1) {
+ /*
+ * GPUs with more than 1 supply require platform-specific handling:
+ * continue without devfreq
+ */
+ DRM_DEV_INFO(dev, "More than 1 supply is not supported yet\n");
+ return 0;
+ }
+
opp_table = dev_pm_opp_set_regulators(dev, pfdev->comp->supply_names,
pfdev->comp->num_supplies);
if (IS_ERR(opp_table)) {
--
2.30.0.280.ga3ce27912f-goog

2021-01-26 10:32:44

by Nicolas Boichat

[permalink] [raw]
Subject: [PATCH v11 2/4] arm64: dts: mt8183: Add node for the Mali GPU

Add a basic GPU node for mt8183.

Signed-off-by: Nicolas Boichat <[email protected]>
---
The binding we use with out-of-tree Mali drivers includes more
clocks, this is used for devfreq: the out-of-tree driver switches
clk_mux to clk_sub_parent (26Mhz), adjusts clk_main_parent, then
switches clk_mux back to clk_main_parent:
(see https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-4.19/drivers/gpu/arm/midgard/platform/mediatek/mali_kbase_runtime_pm.c#423)
clocks =
<&topckgen CLK_TOP_MFGPLL_CK>,
<&topckgen CLK_TOP_MUX_MFG>,
<&clk26m>,
<&mfgcfg CLK_MFG_BG3D>;
clock-names =
"clk_main_parent",
"clk_mux",
"clk_sub_parent",
"subsys_mfg_cg";
(based on discussions, this probably belongs in the clock core)

This only matters for devfreq, that is disabled anyway as we don't
have platform-specific code to handle >1 supplies.

Changes in v11:
- mt8183*.dts: remove incorrect supply-names

Changes in v10: None
Changes in v9: None
Changes in v8: None
Changes in v7: None
Changes in v6:
- Add gpu regulators to kukui dtsi as well.
- Power domains are now attached to spm, not scpsys
- Drop R-B.

Changes in v5:
- Rename "2d" power domain to "core2" (keep R-B again).

Changes in v4:
- Add power-domain-names to describe the 3 domains.
(kept Alyssa's reviewed-by as the change is minor)

Changes in v3: None
Changes in v2:
- Use sram instead of mali_sram as SRAM supply name.
- Rename mali@ to gpu@.

arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 5 +
.../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 5 +
arch/arm64/boot/dts/mediatek/mt8183.dtsi | 105 ++++++++++++++++++
3 files changed, 115 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
index cba2d8933e79..1cfbea5a0101 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
@@ -42,6 +42,11 @@ &auxadc {
status = "okay";
};

+&gpu {
+ mali-supply = <&mt6358_vgpu_reg>;
+ sram-supply = <&mt6358_vsram_gpu_reg>;
+};
+
&i2c0 {
pinctrl-names = "default";
pinctrl-0 = <&i2c_pins_0>;
diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
index bf2ad1294dd3..a38315b604df 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
@@ -249,6 +249,11 @@ &cpu7 {
proc-supply = <&mt6358_vproc11_reg>;
};

+&gpu {
+ mali-supply = <&mt6358_vgpu_reg>;
+ sram-supply = <&mt6358_vsram_gpu_reg>;
+};
+
&i2c0 {
pinctrl-names = "default";
pinctrl-0 = <&i2c0_pins>;
diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index 5b782a4769e7..5430e05e18a0 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -964,6 +964,111 @@ mfgcfg: syscon@13000000 {
#clock-cells = <1>;
};

+ gpu: gpu@13040000 {
+ compatible = "mediatek,mt8183-mali", "arm,mali-bifrost";
+ reg = <0 0x13040000 0 0x4000>;
+ interrupts =
+ <GIC_SPI 280 IRQ_TYPE_LEVEL_LOW>,
+ <GIC_SPI 279 IRQ_TYPE_LEVEL_LOW>,
+ <GIC_SPI 278 IRQ_TYPE_LEVEL_LOW>;
+ interrupt-names = "job", "mmu", "gpu";
+
+ clocks = <&topckgen CLK_TOP_MFGPLL_CK>;
+
+ power-domains =
+ <&spm MT8183_POWER_DOMAIN_MFG_CORE0>,
+ <&spm MT8183_POWER_DOMAIN_MFG_CORE1>,
+ <&spm MT8183_POWER_DOMAIN_MFG_2D>;
+ power-domain-names = "core0", "core1", "core2";
+
+ operating-points-v2 = <&gpu_opp_table>;
+ };
+
+ gpu_opp_table: opp_table0 {
+ compatible = "operating-points-v2";
+ opp-shared;
+
+ opp-300000000 {
+ opp-hz = /bits/ 64 <300000000>;
+ opp-microvolt = <625000>, <850000>;
+ };
+
+ opp-320000000 {
+ opp-hz = /bits/ 64 <320000000>;
+ opp-microvolt = <631250>, <850000>;
+ };
+
+ opp-340000000 {
+ opp-hz = /bits/ 64 <340000000>;
+ opp-microvolt = <637500>, <850000>;
+ };
+
+ opp-360000000 {
+ opp-hz = /bits/ 64 <360000000>;
+ opp-microvolt = <643750>, <850000>;
+ };
+
+ opp-380000000 {
+ opp-hz = /bits/ 64 <380000000>;
+ opp-microvolt = <650000>, <850000>;
+ };
+
+ opp-400000000 {
+ opp-hz = /bits/ 64 <400000000>;
+ opp-microvolt = <656250>, <850000>;
+ };
+
+ opp-420000000 {
+ opp-hz = /bits/ 64 <420000000>;
+ opp-microvolt = <662500>, <850000>;
+ };
+
+ opp-460000000 {
+ opp-hz = /bits/ 64 <460000000>;
+ opp-microvolt = <675000>, <850000>;
+ };
+
+ opp-500000000 {
+ opp-hz = /bits/ 64 <500000000>;
+ opp-microvolt = <687500>, <850000>;
+ };
+
+ opp-540000000 {
+ opp-hz = /bits/ 64 <540000000>;
+ opp-microvolt = <700000>, <850000>;
+ };
+
+ opp-580000000 {
+ opp-hz = /bits/ 64 <580000000>;
+ opp-microvolt = <712500>, <850000>;
+ };
+
+ opp-620000000 {
+ opp-hz = /bits/ 64 <620000000>;
+ opp-microvolt = <725000>, <850000>;
+ };
+
+ opp-653000000 {
+ opp-hz = /bits/ 64 <653000000>;
+ opp-microvolt = <743750>, <850000>;
+ };
+
+ opp-698000000 {
+ opp-hz = /bits/ 64 <698000000>;
+ opp-microvolt = <768750>, <868750>;
+ };
+
+ opp-743000000 {
+ opp-hz = /bits/ 64 <743000000>;
+ opp-microvolt = <793750>, <893750>;
+ };
+
+ opp-800000000 {
+ opp-hz = /bits/ 64 <800000000>;
+ opp-microvolt = <825000>, <925000>;
+ };
+ };
+
mmsys: syscon@14000000 {
compatible = "mediatek,mt8183-mmsys", "syscon";
reg = <0 0x14000000 0 0x1000>;
--
2.30.0.280.ga3ce27912f-goog

2021-02-05 18:01:10

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v11 1/4] dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183

On Tue, 26 Jan 2021 09:17:56 +0800, Nicolas Boichat wrote:
> Define a compatible string for the Mali Bifrost GPU found in
> Mediatek's MT8183 SoCs.
>
> Signed-off-by: Nicolas Boichat <[email protected]>
> ---
>
> Changes in v11:
> - binding: power-domain-names not power-domainS-names
>
> Changes in v10:
> - Fix the binding to make sure sram-supply property can be provided.
>
> Changes in v9: None
> Changes in v8: None
> Changes in v7: None
> Changes in v6:
> - Rebased, actually tested with recent mesa driver.
>
> Changes in v5:
> - Rename "2d" power domain to "core2"
>
> Changes in v4:
> - Add power-domain-names description
> (kept Alyssa's reviewed-by as the change is minor)
>
> Changes in v3: None
> Changes in v2: None
>
> .../bindings/gpu/arm,mali-bifrost.yaml | 28 +++++++++++++++++++
> 1 file changed, 28 insertions(+)
>


Please add Acked-by/Reviewed-by tags when posting new versions. However,
there's no need to repost patches *only* to add the tags. The upstream
maintainer will do that for acks received on the version they apply.

If a tag was not added on purpose, please state why and what changed.

2021-02-06 04:05:15

by Nicolas Boichat

[permalink] [raw]
Subject: Re: [PATCH v11 1/4] dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183

On Sat, Feb 6, 2021 at 1:55 AM Rob Herring <[email protected]> wrote:
>
> On Tue, 26 Jan 2021 09:17:56 +0800, Nicolas Boichat wrote:
> > Define a compatible string for the Mali Bifrost GPU found in
> > Mediatek's MT8183 SoCs.
> >
> > Signed-off-by: Nicolas Boichat <[email protected]>
> > ---
> >
> > Changes in v11:
> > - binding: power-domain-names not power-domainS-names
> >
> > Changes in v10:
> > - Fix the binding to make sure sram-supply property can be provided.
> >
> > Changes in v9: None
> > Changes in v8: None
> > Changes in v7: None
> > Changes in v6:
> > - Rebased, actually tested with recent mesa driver.
> >
> > Changes in v5:
> > - Rename "2d" power domain to "core2"
> >
> > Changes in v4:
> > - Add power-domain-names description
> > (kept Alyssa's reviewed-by as the change is minor)
> >
> > Changes in v3: None
> > Changes in v2: None
> >
> > .../bindings/gpu/arm,mali-bifrost.yaml | 28 +++++++++++++++++++
> > 1 file changed, 28 insertions(+)
> >
>
>
> Please add Acked-by/Reviewed-by tags when posting new versions. However,
> there's no need to repost patches *only* to add the tags. The upstream
> maintainer will do that for acks received on the version they apply.
>
> If a tag was not added on purpose, please state why and what changed.

There were changes in v11, I thought you'd want to review again?

Anyway, I can resend a v12 with all the Rb/Ab if that works better for you.

>

2021-04-20 13:00:03

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v11 1/4] dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183

On Fri, Feb 5, 2021 at 9:02 PM Nicolas Boichat <[email protected]> wrote:
>
> On Sat, Feb 6, 2021 at 1:55 AM Rob Herring <[email protected]> wrote:
> >
> > On Tue, 26 Jan 2021 09:17:56 +0800, Nicolas Boichat wrote:
> > > Define a compatible string for the Mali Bifrost GPU found in
> > > Mediatek's MT8183 SoCs.
> > >
> > > Signed-off-by: Nicolas Boichat <[email protected]>
> > > ---
> > >
> > > Changes in v11:
> > > - binding: power-domain-names not power-domainS-names
> > >
> > > Changes in v10:
> > > - Fix the binding to make sure sram-supply property can be provided.
> > >
> > > Changes in v9: None
> > > Changes in v8: None
> > > Changes in v7: None
> > > Changes in v6:
> > > - Rebased, actually tested with recent mesa driver.
> > >
> > > Changes in v5:
> > > - Rename "2d" power domain to "core2"
> > >
> > > Changes in v4:
> > > - Add power-domain-names description
> > > (kept Alyssa's reviewed-by as the change is minor)
> > >
> > > Changes in v3: None
> > > Changes in v2: None
> > >
> > > .../bindings/gpu/arm,mali-bifrost.yaml | 28 +++++++++++++++++++
> > > 1 file changed, 28 insertions(+)
> > >
> >
> >
> > Please add Acked-by/Reviewed-by tags when posting new versions. However,
> > there's no need to repost patches *only* to add the tags. The upstream
> > maintainer will do that for acks received on the version they apply.
> >
> > If a tag was not added on purpose, please state why and what changed.
>
> There were changes in v11, I thought you'd want to review again?

Looked like a minor change from the changelog, so it would have been
appropriate to keep. However, I see another issue.

Rob

2021-04-20 13:03:58

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v11 1/4] dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183

On Mon, Jan 25, 2021 at 7:18 PM Nicolas Boichat <[email protected]> wrote:
>
> Define a compatible string for the Mali Bifrost GPU found in
> Mediatek's MT8183 SoCs.
>
> Signed-off-by: Nicolas Boichat <[email protected]>
> ---
>
> Changes in v11:
> - binding: power-domain-names not power-domainS-names
>
> Changes in v10:
> - Fix the binding to make sure sram-supply property can be provided.
>
> Changes in v9: None
> Changes in v8: None
> Changes in v7: None
> Changes in v6:
> - Rebased, actually tested with recent mesa driver.
>
> Changes in v5:
> - Rename "2d" power domain to "core2"
>
> Changes in v4:
> - Add power-domain-names description
> (kept Alyssa's reviewed-by as the change is minor)
>
> Changes in v3: None
> Changes in v2: None
>
> .../bindings/gpu/arm,mali-bifrost.yaml | 28 +++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml
> index 184492162e7e..3e758f88e2cd 100644
> --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml
> +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml
> @@ -17,6 +17,7 @@ properties:
> items:
> - enum:
> - amlogic,meson-g12a-mali
> + - mediatek,mt8183-mali
> - realtek,rtd1619-mali
> - rockchip,px30-mali
> - const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully discoverable
> @@ -41,6 +42,8 @@ properties:
>
> mali-supply: true
>
> + sram-supply: true
> +
> operating-points-v2: true
>
> power-domains:
> @@ -87,6 +90,31 @@ allOf:
> then:
> required:
> - resets
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: mediatek,mt8183-mali
> + then:
> + properties:
> + power-domains:
> + description:
> + List of phandle and PM domain specifier as documented in
> + Documentation/devicetree/bindings/power/power_domain.txt
> + minItems: 3
> + maxItems: 3

This won't work because the top level schema restricts this to 1. The
top level needs to say:

power-domains:
minItems: 1
maxItems: 3

And you need just 'minItems: 3' here and 'maxItems: 1' in the else clause.

And drop the description. That's every 'power-domains' property.

> + power-domain-names:
> + items:
> + - const: core0
> + - const: core1
> + - const: core2

Blank line

> + required:
> + - sram-supply
> + - power-domains
> + - power-domain-names
> + else:
> + properties:
> + sram-supply: false
>
> examples:
> - |
> --
> 2.30.0.280.ga3ce27912f-goog
>

2021-04-20 15:37:36

by Neil Armstrong

[permalink] [raw]
Subject: Re: [PATCH v11 2/4] arm64: dts: mt8183: Add node for the Mali GPU

On 26/01/2021 02:17, Nicolas Boichat wrote:
> Add a basic GPU node for mt8183.
>
> Signed-off-by: Nicolas Boichat <[email protected]>
> ---
> The binding we use with out-of-tree Mali drivers includes more
> clocks, this is used for devfreq: the out-of-tree driver switches
> clk_mux to clk_sub_parent (26Mhz), adjusts clk_main_parent, then
> switches clk_mux back to clk_main_parent:
> (see https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-4.19/drivers/gpu/arm/midgard/platform/mediatek/mali_kbase_runtime_pm.c#423)
> clocks =
> <&topckgen CLK_TOP_MFGPLL_CK>,
> <&topckgen CLK_TOP_MUX_MFG>,
> <&clk26m>,
> <&mfgcfg CLK_MFG_BG3D>;
> clock-names =
> "clk_main_parent",
> "clk_mux",
> "clk_sub_parent",
> "subsys_mfg_cg";
> (based on discussions, this probably belongs in the clock core)
>
> This only matters for devfreq, that is disabled anyway as we don't
> have platform-specific code to handle >1 supplies.
>
> Changes in v11:
> - mt8183*.dts: remove incorrect supply-names
>
> Changes in v10: None
> Changes in v9: None
> Changes in v8: None
> Changes in v7: None
> Changes in v6:
> - Add gpu regulators to kukui dtsi as well.
> - Power domains are now attached to spm, not scpsys
> - Drop R-B.
>
> Changes in v5:
> - Rename "2d" power domain to "core2" (keep R-B again).
>
> Changes in v4:
> - Add power-domain-names to describe the 3 domains.
> (kept Alyssa's reviewed-by as the change is minor)
>
> Changes in v3: None
> Changes in v2:
> - Use sram instead of mali_sram as SRAM supply name.
> - Rename mali@ to gpu@.
>
> arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 5 +
> .../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 5 +
> arch/arm64/boot/dts/mediatek/mt8183.dtsi | 105 ++++++++++++++++++
> 3 files changed, 115 insertions(+)
>

If you re-spin, you can also add the same changes to mt8183-pumpkin.dts :
diff --git a/arch/arm64/boot/dts/mediatek/mt8183-pumpkin.dts b/arch/arm64/boot/dts/mediatek/mt8183-pumpkin.dts
index eb6e595c2975..cc23e5df391e 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-pumpkin.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8183-pumpkin.dts
@@ -68,6 +68,11 @@ &auxadc {
status = "okay";
};

+&gpu {
+ mali-supply = <&mt6358_vgpu_reg>;
+ sram-supply = <&mt6358_vsram_gpu_reg>;
+};
+
&i2c0 {
pinctrl-names = "default";
pinctrl-0 = <&i2c_pins_0>;

I did a boot-test of the platform with panfrost and drm-misc-next and it worked fine.

Thanks,
Neil

> diff --git a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
> index cba2d8933e79..1cfbea5a0101 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
> +++ b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
> @@ -42,6 +42,11 @@ &auxadc {
> status = "okay";
> };
>
> +&gpu {
> + mali-supply = <&mt6358_vgpu_reg>;
> + sram-supply = <&mt6358_vsram_gpu_reg>;
> +};
> +
> &i2c0 {
> pinctrl-names = "default";
> pinctrl-0 = <&i2c_pins_0>;
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> index bf2ad1294dd3..a38315b604df 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> @@ -249,6 +249,11 @@ &cpu7 {
> proc-supply = <&mt6358_vproc11_reg>;
> };
>
> +&gpu {
> + mali-supply = <&mt6358_vgpu_reg>;
> + sram-supply = <&mt6358_vsram_gpu_reg>;
> +};
> +
> &i2c0 {
> pinctrl-names = "default";
> pinctrl-0 = <&i2c0_pins>;
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> index 5b782a4769e7..5430e05e18a0 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> @@ -964,6 +964,111 @@ mfgcfg: syscon@13000000 {
> #clock-cells = <1>;
> };
>
> + gpu: gpu@13040000 {
> + compatible = "mediatek,mt8183-mali", "arm,mali-bifrost";
> + reg = <0 0x13040000 0 0x4000>;
> + interrupts =
> + <GIC_SPI 280 IRQ_TYPE_LEVEL_LOW>,
> + <GIC_SPI 279 IRQ_TYPE_LEVEL_LOW>,
> + <GIC_SPI 278 IRQ_TYPE_LEVEL_LOW>;
> + interrupt-names = "job", "mmu", "gpu";
> +
> + clocks = <&topckgen CLK_TOP_MFGPLL_CK>;
> +
> + power-domains =
> + <&spm MT8183_POWER_DOMAIN_MFG_CORE0>,
> + <&spm MT8183_POWER_DOMAIN_MFG_CORE1>,
> + <&spm MT8183_POWER_DOMAIN_MFG_2D>;
> + power-domain-names = "core0", "core1", "core2";
> +
> + operating-points-v2 = <&gpu_opp_table>;
> + };
> +
> + gpu_opp_table: opp_table0 {
> + compatible = "operating-points-v2";
> + opp-shared;
> +
> + opp-300000000 {
> + opp-hz = /bits/ 64 <300000000>;
> + opp-microvolt = <625000>, <850000>;
> + };
> +
> + opp-320000000 {
> + opp-hz = /bits/ 64 <320000000>;
> + opp-microvolt = <631250>, <850000>;
> + };
> +
> + opp-340000000 {
> + opp-hz = /bits/ 64 <340000000>;
> + opp-microvolt = <637500>, <850000>;
> + };
> +
> + opp-360000000 {
> + opp-hz = /bits/ 64 <360000000>;
> + opp-microvolt = <643750>, <850000>;
> + };
> +
> + opp-380000000 {
> + opp-hz = /bits/ 64 <380000000>;
> + opp-microvolt = <650000>, <850000>;
> + };
> +
> + opp-400000000 {
> + opp-hz = /bits/ 64 <400000000>;
> + opp-microvolt = <656250>, <850000>;
> + };
> +
> + opp-420000000 {
> + opp-hz = /bits/ 64 <420000000>;
> + opp-microvolt = <662500>, <850000>;
> + };
> +
> + opp-460000000 {
> + opp-hz = /bits/ 64 <460000000>;
> + opp-microvolt = <675000>, <850000>;
> + };
> +
> + opp-500000000 {
> + opp-hz = /bits/ 64 <500000000>;
> + opp-microvolt = <687500>, <850000>;
> + };
> +
> + opp-540000000 {
> + opp-hz = /bits/ 64 <540000000>;
> + opp-microvolt = <700000>, <850000>;
> + };
> +
> + opp-580000000 {
> + opp-hz = /bits/ 64 <580000000>;
> + opp-microvolt = <712500>, <850000>;
> + };
> +
> + opp-620000000 {
> + opp-hz = /bits/ 64 <620000000>;
> + opp-microvolt = <725000>, <850000>;
> + };
> +
> + opp-653000000 {
> + opp-hz = /bits/ 64 <653000000>;
> + opp-microvolt = <743750>, <850000>;
> + };
> +
> + opp-698000000 {
> + opp-hz = /bits/ 64 <698000000>;
> + opp-microvolt = <768750>, <868750>;
> + };
> +
> + opp-743000000 {
> + opp-hz = /bits/ 64 <743000000>;
> + opp-microvolt = <793750>, <893750>;
> + };
> +
> + opp-800000000 {
> + opp-hz = /bits/ 64 <800000000>;
> + opp-microvolt = <825000>, <925000>;
> + };
> + };
> +
> mmsys: syscon@14000000 {
> compatible = "mediatek,mt8183-mmsys", "syscon";
> reg = <0 0x14000000 0 0x1000>;
>

2021-04-21 00:07:46

by Nicolas Boichat

[permalink] [raw]
Subject: Re: [PATCH v11 2/4] arm64: dts: mt8183: Add node for the Mali GPU

On Tue, Apr 20, 2021 at 11:33 PM Neil Armstrong <[email protected]> wrote:
>
> On 26/01/2021 02:17, Nicolas Boichat wrote:
> > Add a basic GPU node for mt8183.
> >
> > Signed-off-by: Nicolas Boichat <[email protected]>
> > ---
> > The binding we use with out-of-tree Mali drivers includes more
> > clocks, this is used for devfreq: the out-of-tree driver switches
> > clk_mux to clk_sub_parent (26Mhz), adjusts clk_main_parent, then
> > switches clk_mux back to clk_main_parent:
> > (see https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-4.19/drivers/gpu/arm/midgard/platform/mediatek/mali_kbase_runtime_pm.c#423)
> > clocks =
> > <&topckgen CLK_TOP_MFGPLL_CK>,
> > <&topckgen CLK_TOP_MUX_MFG>,
> > <&clk26m>,
> > <&mfgcfg CLK_MFG_BG3D>;
> > clock-names =
> > "clk_main_parent",
> > "clk_mux",
> > "clk_sub_parent",
> > "subsys_mfg_cg";
> > (based on discussions, this probably belongs in the clock core)
> >
> > This only matters for devfreq, that is disabled anyway as we don't
> > have platform-specific code to handle >1 supplies.
> >
> > Changes in v11:
> > - mt8183*.dts: remove incorrect supply-names
> >
> > Changes in v10: None
> > Changes in v9: None
> > Changes in v8: None
> > Changes in v7: None
> > Changes in v6:
> > - Add gpu regulators to kukui dtsi as well.
> > - Power domains are now attached to spm, not scpsys
> > - Drop R-B.
> >
> > Changes in v5:
> > - Rename "2d" power domain to "core2" (keep R-B again).
> >
> > Changes in v4:
> > - Add power-domain-names to describe the 3 domains.
> > (kept Alyssa's reviewed-by as the change is minor)
> >
> > Changes in v3: None
> > Changes in v2:
> > - Use sram instead of mali_sram as SRAM supply name.
> > - Rename mali@ to gpu@.
> >
> > arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 5 +
> > .../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 5 +
> > arch/arm64/boot/dts/mediatek/mt8183.dtsi | 105 ++++++++++++++++++
> > 3 files changed, 115 insertions(+)
> >
>
> If you re-spin, you can also add the same changes to mt8183-pumpkin.dts :

Will do in v12.

> diff --git a/arch/arm64/boot/dts/mediatek/mt8183-pumpkin.dts b/arch/arm64/boot/dts/mediatek/mt8183-pumpkin.dts
> index eb6e595c2975..cc23e5df391e 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183-pumpkin.dts
> +++ b/arch/arm64/boot/dts/mediatek/mt8183-pumpkin.dts
> @@ -68,6 +68,11 @@ &auxadc {
> status = "okay";
> };
>
> +&gpu {
> + mali-supply = <&mt6358_vgpu_reg>;
> + sram-supply = <&mt6358_vsram_gpu_reg>;
> +};
> +
> &i2c0 {
> pinctrl-names = "default";
> pinctrl-0 = <&i2c_pins_0>;
>
> I did a boot-test of the platform with panfrost and drm-misc-next and it worked fine.

Great news thanks!

>
> Thanks,
> Neil
>
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
> > index cba2d8933e79..1cfbea5a0101 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
> > +++ b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
> > @@ -42,6 +42,11 @@ &auxadc {
> > status = "okay";
> > };
> >
> > +&gpu {
> > + mali-supply = <&mt6358_vgpu_reg>;
> > + sram-supply = <&mt6358_vsram_gpu_reg>;
> > +};
> > +
> > &i2c0 {
> > pinctrl-names = "default";
> > pinctrl-0 = <&i2c_pins_0>;
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> > index bf2ad1294dd3..a38315b604df 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt8183-kukui.dtsi
> > @@ -249,6 +249,11 @@ &cpu7 {
> > proc-supply = <&mt6358_vproc11_reg>;
> > };
> >
> > +&gpu {
> > + mali-supply = <&mt6358_vgpu_reg>;
> > + sram-supply = <&mt6358_vsram_gpu_reg>;
> > +};
> > +
> > &i2c0 {
> > pinctrl-names = "default";
> > pinctrl-0 = <&i2c0_pins>;
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > index 5b782a4769e7..5430e05e18a0 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> > @@ -964,6 +964,111 @@ mfgcfg: syscon@13000000 {
> > #clock-cells = <1>;
> > };
> >
> > + gpu: gpu@13040000 {
> > + compatible = "mediatek,mt8183-mali", "arm,mali-bifrost";
> > + reg = <0 0x13040000 0 0x4000>;
> > + interrupts =
> > + <GIC_SPI 280 IRQ_TYPE_LEVEL_LOW>,
> > + <GIC_SPI 279 IRQ_TYPE_LEVEL_LOW>,
> > + <GIC_SPI 278 IRQ_TYPE_LEVEL_LOW>;
> > + interrupt-names = "job", "mmu", "gpu";
> > +
> > + clocks = <&topckgen CLK_TOP_MFGPLL_CK>;
> > +
> > + power-domains =
> > + <&spm MT8183_POWER_DOMAIN_MFG_CORE0>,
> > + <&spm MT8183_POWER_DOMAIN_MFG_CORE1>,
> > + <&spm MT8183_POWER_DOMAIN_MFG_2D>;
> > + power-domain-names = "core0", "core1", "core2";
> > +
> > + operating-points-v2 = <&gpu_opp_table>;
> > + };
> > +
> > + gpu_opp_table: opp_table0 {
> > + compatible = "operating-points-v2";
> > + opp-shared;
> > +
> > + opp-300000000 {
> > + opp-hz = /bits/ 64 <300000000>;
> > + opp-microvolt = <625000>, <850000>;
> > + };
> > +
> > + opp-320000000 {
> > + opp-hz = /bits/ 64 <320000000>;
> > + opp-microvolt = <631250>, <850000>;
> > + };
> > +
> > + opp-340000000 {
> > + opp-hz = /bits/ 64 <340000000>;
> > + opp-microvolt = <637500>, <850000>;
> > + };
> > +
> > + opp-360000000 {
> > + opp-hz = /bits/ 64 <360000000>;
> > + opp-microvolt = <643750>, <850000>;
> > + };
> > +
> > + opp-380000000 {
> > + opp-hz = /bits/ 64 <380000000>;
> > + opp-microvolt = <650000>, <850000>;
> > + };
> > +
> > + opp-400000000 {
> > + opp-hz = /bits/ 64 <400000000>;
> > + opp-microvolt = <656250>, <850000>;
> > + };
> > +
> > + opp-420000000 {
> > + opp-hz = /bits/ 64 <420000000>;
> > + opp-microvolt = <662500>, <850000>;
> > + };
> > +
> > + opp-460000000 {
> > + opp-hz = /bits/ 64 <460000000>;
> > + opp-microvolt = <675000>, <850000>;
> > + };
> > +
> > + opp-500000000 {
> > + opp-hz = /bits/ 64 <500000000>;
> > + opp-microvolt = <687500>, <850000>;
> > + };
> > +
> > + opp-540000000 {
> > + opp-hz = /bits/ 64 <540000000>;
> > + opp-microvolt = <700000>, <850000>;
> > + };
> > +
> > + opp-580000000 {
> > + opp-hz = /bits/ 64 <580000000>;
> > + opp-microvolt = <712500>, <850000>;
> > + };
> > +
> > + opp-620000000 {
> > + opp-hz = /bits/ 64 <620000000>;
> > + opp-microvolt = <725000>, <850000>;
> > + };
> > +
> > + opp-653000000 {
> > + opp-hz = /bits/ 64 <653000000>;
> > + opp-microvolt = <743750>, <850000>;
> > + };
> > +
> > + opp-698000000 {
> > + opp-hz = /bits/ 64 <698000000>;
> > + opp-microvolt = <768750>, <868750>;
> > + };
> > +
> > + opp-743000000 {
> > + opp-hz = /bits/ 64 <743000000>;
> > + opp-microvolt = <793750>, <893750>;
> > + };
> > +
> > + opp-800000000 {
> > + opp-hz = /bits/ 64 <800000000>;
> > + opp-microvolt = <825000>, <925000>;
> > + };
> > + };
> > +
> > mmsys: syscon@14000000 {
> > compatible = "mediatek,mt8183-mmsys", "syscon";
> > reg = <0 0x14000000 0 0x1000>;
> >
>

2021-04-21 00:13:42

by Nicolas Boichat

[permalink] [raw]
Subject: Re: [PATCH v11 1/4] dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183

On Tue, Apr 20, 2021 at 9:01 PM Rob Herring <[email protected]> wrote:
>
> On Mon, Jan 25, 2021 at 7:18 PM Nicolas Boichat <[email protected]> wrote:
> >
> > Define a compatible string for the Mali Bifrost GPU found in
> > Mediatek's MT8183 SoCs.
> >
> > Signed-off-by: Nicolas Boichat <[email protected]>
> > ---
> >
> > Changes in v11:
> > - binding: power-domain-names not power-domainS-names
> >
> > Changes in v10:
> > - Fix the binding to make sure sram-supply property can be provided.
> >
> > Changes in v9: None
> > Changes in v8: None
> > Changes in v7: None
> > Changes in v6:
> > - Rebased, actually tested with recent mesa driver.
> >
> > Changes in v5:
> > - Rename "2d" power domain to "core2"
> >
> > Changes in v4:
> > - Add power-domain-names description
> > (kept Alyssa's reviewed-by as the change is minor)
> >
> > Changes in v3: None
> > Changes in v2: None
> >
> > .../bindings/gpu/arm,mali-bifrost.yaml | 28 +++++++++++++++++++
> > 1 file changed, 28 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml
> > index 184492162e7e..3e758f88e2cd 100644
> > --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml
> > +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml
> > @@ -17,6 +17,7 @@ properties:
> > items:
> > - enum:
> > - amlogic,meson-g12a-mali
> > + - mediatek,mt8183-mali
> > - realtek,rtd1619-mali
> > - rockchip,px30-mali
> > - const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully discoverable
> > @@ -41,6 +42,8 @@ properties:
> >
> > mali-supply: true
> >
> > + sram-supply: true
> > +
> > operating-points-v2: true
> >
> > power-domains:
> > @@ -87,6 +90,31 @@ allOf:
> > then:
> > required:
> > - resets
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + const: mediatek,mt8183-mali
> > + then:
> > + properties:
> > + power-domains:
> > + description:
> > + List of phandle and PM domain specifier as documented in
> > + Documentation/devicetree/bindings/power/power_domain.txt
> > + minItems: 3
> > + maxItems: 3
>
> This won't work because the top level schema restricts this to 1. The
> top level needs to say:
>
> power-domains:
> minItems: 1
> maxItems: 3
>
> And you need just 'minItems: 3' here and 'maxItems: 1' in the else clause.
>
> And drop the description. That's every 'power-domains' property.
>
> > + power-domain-names:
> > + items:
> > + - const: core0
> > + - const: core1
> > + - const: core2
>
> Blank line

Thanks, hopefully all fixed in v12.

> > + required:
> > + - sram-supply
> > + - power-domains
> > + - power-domain-names
> > + else:
> > + properties:
> > + sram-supply: false
> >
> > examples:
> > - |
> > --
> > 2.30.0.280.ga3ce27912f-goog
> >