* [PATCH v3 1/7] dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183
2020-01-14 7:15 [PATCH v3 0/7] Add dts for mt8183 GPU (and misc panfrost patches) Nicolas Boichat
@ 2020-01-14 7:15 ` Nicolas Boichat
2020-01-14 7:15 ` [PATCH v3 2/7] arm64: dts: mt8183: Add node for the Mali GPU Nicolas Boichat
` (5 subsequent siblings)
6 siblings, 0 replies; 16+ messages in thread
From: Nicolas Boichat @ 2020-01-14 7:15 UTC (permalink / raw)
To: Rob Herring
Cc: Mark Rutland, devicetree, Tomeu Vizoso, David Airlie,
linux-kernel, Liam Girdwood, dri-devel, Steven Price, Mark Brown,
linux-mediatek, Alyssa Rosenzweig, hsinyi, Matthias Brugger,
linux-arm-kernel
Define a compatible string for the Mali Bifrost GPU found in
Mediatek's MT8183 SoCs.
Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
Reviewed-by: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
---
v3:
- No change
.../bindings/gpu/arm,mali-bifrost.yaml | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml
index 4ea6a8789699709..9e095608d2d98f0 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
@@ -62,6 +63,23 @@ allOf:
minItems: 2
required:
- resets
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: mediatek,mt8183-mali
+ then:
+ properties:
+ sram-supply: true
+ power-domains:
+ description:
+ List of phandle and PM domain specifier as documented in
+ Documentation/devicetree/bindings/power/power_domain.txt
+ minItems: 3
+ maxItems: 3
+ required:
+ - sram-supply
+ - power-domains
examples:
- |
--
2.25.0.rc1.283.g88dfdc4193-goog
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v3 2/7] arm64: dts: mt8183: Add node for the Mali GPU
2020-01-14 7:15 [PATCH v3 0/7] Add dts for mt8183 GPU (and misc panfrost patches) Nicolas Boichat
2020-01-14 7:15 ` [PATCH v3 1/7] dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183 Nicolas Boichat
@ 2020-01-14 7:15 ` Nicolas Boichat
2020-01-14 7:15 ` [PATCH v3 3/7] drm/panfrost: Improve error reporting in panfrost_gpu_power_on Nicolas Boichat
` (4 subsequent siblings)
6 siblings, 0 replies; 16+ messages in thread
From: Nicolas Boichat @ 2020-01-14 7:15 UTC (permalink / raw)
To: Rob Herring
Cc: Mark Rutland, devicetree, Tomeu Vizoso, David Airlie,
linux-kernel, Liam Girdwood, dri-devel, Steven Price, Mark Brown,
linux-mediatek, Alyssa Rosenzweig, hsinyi, Matthias Brugger,
linux-arm-kernel
Add a basic GPU node for mt8183.
Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
Reviewed-by: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
---
Upstreaming what matches existing bindings from our Chromium OS tree:
https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-4.19/arch/arm64/boot/dts/mediatek/mt8183.dtsi#1348
The evb part of this change depends on this patch to add PMIC dtsi:
https://patchwork.kernel.org/patch/10928161/
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";
v3:
- No changes
v2:
- Use sram instead of mali_sram as SRAM supply name.
- Rename mali@ to gpu@.
arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 7 ++
arch/arm64/boot/dts/mediatek/mt8183.dtsi | 104 ++++++++++++++++++++
2 files changed, 111 insertions(+)
diff --git a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
index 1fb195c683c3d01..7d609e0cd9b4975 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
@@ -7,6 +7,7 @@
/dts-v1/;
#include "mt8183.dtsi"
+#include "mt6358.dtsi"
/ {
model = "MediaTek MT8183 evaluation board";
@@ -30,6 +31,12 @@ &auxadc {
status = "okay";
};
+&gpu {
+ supply-names = "mali", "sram";
+ 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.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index 1ade9153e5c265b..4da3f1ed1c15bf3 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -597,6 +597,110 @@ 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 =
+ <&scpsys MT8183_POWER_DOMAIN_MFG_CORE0>,
+ <&scpsys MT8183_POWER_DOMAIN_MFG_CORE1>,
+ <&scpsys MT8183_POWER_DOMAIN_MFG_2D>;
+
+ 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.25.0.rc1.283.g88dfdc4193-goog
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v3 3/7] drm/panfrost: Improve error reporting in panfrost_gpu_power_on
2020-01-14 7:15 [PATCH v3 0/7] Add dts for mt8183 GPU (and misc panfrost patches) Nicolas Boichat
2020-01-14 7:15 ` [PATCH v3 1/7] dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183 Nicolas Boichat
2020-01-14 7:15 ` [PATCH v3 2/7] arm64: dts: mt8183: Add node for the Mali GPU Nicolas Boichat
@ 2020-01-14 7:15 ` Nicolas Boichat
2020-01-14 7:15 ` [PATCH v3 4/7] drm/panfrost: Add support for multiple regulators Nicolas Boichat
` (3 subsequent siblings)
6 siblings, 0 replies; 16+ messages in thread
From: Nicolas Boichat @ 2020-01-14 7:15 UTC (permalink / raw)
To: Rob Herring
Cc: Mark Rutland, devicetree, Tomeu Vizoso, David Airlie,
linux-kernel, Liam Girdwood, dri-devel, Steven Price, Mark Brown,
linux-mediatek, Alyssa Rosenzweig, hsinyi, Matthias Brugger,
linux-arm-kernel
It is useful to know which component cannot be powered on.
Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
Reviewed-by: Steven Price <steven.price@arm.com>
Reviewed-by: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>
---
Was useful when trying to probe Bifrost GPU, to understand what
issue we are facing.
v3:
- Rebased on https://patchwork.kernel.org/patch/11325689/
drivers/gpu/drm/panfrost/panfrost_gpu.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c b/drivers/gpu/drm/panfrost/panfrost_gpu.c
index 460fc190de6e815..856f2fd1fa8ed27 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gpu.c
+++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c
@@ -308,17 +308,20 @@ void panfrost_gpu_power_on(struct panfrost_device *pfdev)
gpu_write(pfdev, L2_PWRON_LO, pfdev->features.l2_present);
ret = readl_relaxed_poll_timeout(pfdev->iomem + L2_READY_LO,
val, val == pfdev->features.l2_present, 100, 1000);
+ if (ret)
+ dev_err(pfdev->dev, "error powering up gpu L2");
gpu_write(pfdev, SHADER_PWRON_LO, pfdev->features.shader_present);
- ret |= readl_relaxed_poll_timeout(pfdev->iomem + SHADER_READY_LO,
+ ret = readl_relaxed_poll_timeout(pfdev->iomem + SHADER_READY_LO,
val, val == pfdev->features.shader_present, 100, 1000);
+ if (ret)
+ dev_err(pfdev->dev, "error powering up gpu shader");
gpu_write(pfdev, TILER_PWRON_LO, pfdev->features.tiler_present);
- ret |= readl_relaxed_poll_timeout(pfdev->iomem + TILER_READY_LO,
+ ret = readl_relaxed_poll_timeout(pfdev->iomem + TILER_READY_LO,
val, val == pfdev->features.tiler_present, 100, 1000);
-
if (ret)
- dev_err(pfdev->dev, "error powering up gpu");
+ dev_err(pfdev->dev, "error powering up gpu tiler");
}
void panfrost_gpu_power_off(struct panfrost_device *pfdev)
--
2.25.0.rc1.283.g88dfdc4193-goog
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v3 4/7] drm/panfrost: Add support for multiple regulators
2020-01-14 7:15 [PATCH v3 0/7] Add dts for mt8183 GPU (and misc panfrost patches) Nicolas Boichat
` (2 preceding siblings ...)
2020-01-14 7:15 ` [PATCH v3 3/7] drm/panfrost: Improve error reporting in panfrost_gpu_power_on Nicolas Boichat
@ 2020-01-14 7:15 ` Nicolas Boichat
2020-01-14 15:16 ` Mark Brown
2020-01-20 14:43 ` Steven Price
2020-01-14 7:16 ` [PATCH v3 5/7] drm/panfrost: Add support for multiple power domains Nicolas Boichat
` (2 subsequent siblings)
6 siblings, 2 replies; 16+ messages in thread
From: Nicolas Boichat @ 2020-01-14 7:15 UTC (permalink / raw)
To: Rob Herring
Cc: Mark Rutland, devicetree, Tomeu Vizoso, David Airlie,
linux-kernel, Liam Girdwood, dri-devel, Steven Price, Mark Brown,
linux-mediatek, Alyssa Rosenzweig, hsinyi, Matthias Brugger,
linux-arm-kernel
Some GPUs, namely, the bifrost/g72 part on MT8183, have a second
regulator for their SRAM, let's add support for that.
We extend the framework in a generic manner so that we could
support more than 2 regulators, if required.
Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
---
v3:
- Make this more generic, by allowing any number of regulators
(in practice we fix the maximum number of regulators to 2, but
this could be increased easily).
- We only probe the second regulator if the device tree matching
data asks for it.
- I couldn't find a way to detect the number of regulators in the
device tree, if we wanted to refuse to probe the device if there
are too many regulators, which might be required for safety, see
the thread on v2 [1].
- The discussion also included the idea of a separate device tree
entry for a "soft PDC", or at least a separate driver. I'm not
sure to understand the full picture, and how different vendors
implement this, so I'm still integrating everything in the main
driver. I'd be happy to try to make mt8183 fit into such a
framework after it's created, but I don't think I'm best placed
to implement (and again, the main purpose of this was to test
if the binding is correct).
[1] https://patchwork.kernel.org/patch/11322839/
drivers/gpu/drm/panfrost/panfrost_device.c | 25 ++++++++++++-------
drivers/gpu/drm/panfrost/panfrost_device.h | 15 +++++++++++-
drivers/gpu/drm/panfrost/panfrost_drv.c | 28 +++++++++++++++-------
3 files changed, 50 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c
index 238fb6d54df4732..c30e0a3772a4f57 100644
--- a/drivers/gpu/drm/panfrost/panfrost_device.c
+++ b/drivers/gpu/drm/panfrost/panfrost_device.c
@@ -87,18 +87,26 @@ static void panfrost_clk_fini(struct panfrost_device *pfdev)
static int panfrost_regulator_init(struct panfrost_device *pfdev)
{
- int ret;
+ int ret, i;
- pfdev->regulator = devm_regulator_get(pfdev->dev, "mali");
- if (IS_ERR(pfdev->regulator)) {
- ret = PTR_ERR(pfdev->regulator);
- dev_err(pfdev->dev, "failed to get regulator: %d\n", ret);
+ BUG_ON(pfdev->comp->num_supplies > ARRAY_SIZE(pfdev->regulators));
+
+ for (i = 0; i < pfdev->comp->num_supplies; i++) {
+ pfdev->regulators[i].supply = pfdev->comp->supply_names[i];
+ }
+
+ ret = devm_regulator_bulk_get(pfdev->dev,
+ pfdev->comp->num_supplies,
+ pfdev->regulators);
+ if (ret < 0) {
+ dev_err(pfdev->dev, "failed to get regulators: %d\n", ret);
return ret;
}
- ret = regulator_enable(pfdev->regulator);
+ ret = regulator_bulk_enable(pfdev->comp->num_supplies,
+ pfdev->regulators);
if (ret < 0) {
- dev_err(pfdev->dev, "failed to enable regulator: %d\n", ret);
+ dev_err(pfdev->dev, "failed to enable regulators: %d\n", ret);
return ret;
}
@@ -107,7 +115,8 @@ static int panfrost_regulator_init(struct panfrost_device *pfdev)
static void panfrost_regulator_fini(struct panfrost_device *pfdev)
{
- regulator_disable(pfdev->regulator);
+ regulator_bulk_disable(pfdev->comp->num_supplies,
+ pfdev->regulators);
}
int panfrost_device_init(struct panfrost_device *pfdev)
diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h
index 06713811b92cdf7..021f063ffb3747f 100644
--- a/drivers/gpu/drm/panfrost/panfrost_device.h
+++ b/drivers/gpu/drm/panfrost/panfrost_device.h
@@ -7,6 +7,7 @@
#include <linux/atomic.h>
#include <linux/io-pgtable.h>
+#include <linux/regulator/consumer.h>
#include <linux/spinlock.h>
#include <drm/drm_device.h>
#include <drm/drm_mm.h>
@@ -19,6 +20,7 @@ struct panfrost_job;
struct panfrost_perfcnt;
#define NUM_JOB_SLOTS 3
+#define MAX_REGULATORS 2
struct panfrost_features {
u16 id;
@@ -51,6 +53,16 @@ struct panfrost_features {
unsigned long hw_issues[64 / BITS_PER_LONG];
};
+/*
+ * Features that cannot be automatically detected and need matching using the
+ * compatible string, typically SoC-specific.
+ */
+struct panfrost_compatible {
+ /* Supplies count and names. */
+ int num_supplies;
+ const char * const *supply_names;
+};
+
struct panfrost_device {
struct device *dev;
struct drm_device *ddev;
@@ -59,10 +71,11 @@ struct panfrost_device {
void __iomem *iomem;
struct clk *clock;
struct clk *bus_clock;
- struct regulator *regulator;
+ struct regulator_bulk_data regulators[MAX_REGULATORS];
struct reset_control *rstc;
struct panfrost_features features;
+ const struct panfrost_compatible* comp;
spinlock_t as_lock;
unsigned long as_in_use_mask;
diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
index 48e3c4165247cea..db3563b80150c9d 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -510,6 +510,10 @@ static int panfrost_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, pfdev);
+ pfdev->comp = of_device_get_match_data(&pdev->dev);
+ if (!pfdev->comp)
+ return -ENODEV;
+
/* Allocate and initialze the DRM device. */
ddev = drm_dev_alloc(&panfrost_drm_driver, &pdev->dev);
if (IS_ERR(ddev))
@@ -581,16 +585,22 @@ static int panfrost_remove(struct platform_device *pdev)
return 0;
}
+const char * const default_supplies[] = { "mali" };
+static const struct panfrost_compatible default_data = {
+ .num_supplies = ARRAY_SIZE(default_supplies),
+ .supply_names = default_supplies,
+};
+
static const struct of_device_id dt_match[] = {
- { .compatible = "arm,mali-t604" },
- { .compatible = "arm,mali-t624" },
- { .compatible = "arm,mali-t628" },
- { .compatible = "arm,mali-t720" },
- { .compatible = "arm,mali-t760" },
- { .compatible = "arm,mali-t820" },
- { .compatible = "arm,mali-t830" },
- { .compatible = "arm,mali-t860" },
- { .compatible = "arm,mali-t880" },
+ { .compatible = "arm,mali-t604", .data = &default_data, },
+ { .compatible = "arm,mali-t624", .data = &default_data, },
+ { .compatible = "arm,mali-t628", .data = &default_data, },
+ { .compatible = "arm,mali-t720", .data = &default_data, },
+ { .compatible = "arm,mali-t760", .data = &default_data, },
+ { .compatible = "arm,mali-t820", .data = &default_data, },
+ { .compatible = "arm,mali-t830", .data = &default_data, },
+ { .compatible = "arm,mali-t860", .data = &default_data, },
+ { .compatible = "arm,mali-t880", .data = &default_data, },
{}
};
MODULE_DEVICE_TABLE(of, dt_match);
--
2.25.0.rc1.283.g88dfdc4193-goog
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v3 4/7] drm/panfrost: Add support for multiple regulators
2020-01-14 7:15 ` [PATCH v3 4/7] drm/panfrost: Add support for multiple regulators Nicolas Boichat
@ 2020-01-14 15:16 ` Mark Brown
2020-01-21 4:37 ` Nicolas Boichat
2020-01-20 14:43 ` Steven Price
1 sibling, 1 reply; 16+ messages in thread
From: Mark Brown @ 2020-01-14 15:16 UTC (permalink / raw)
To: Nicolas Boichat
Cc: Mark Rutland, devicetree, Tomeu Vizoso, David Airlie,
linux-kernel, Liam Girdwood, dri-devel, Steven Price,
Rob Herring, linux-mediatek, Alyssa Rosenzweig, hsinyi,
Matthias Brugger, linux-arm-kernel
[-- Attachment #1.1: Type: text/plain, Size: 445 bytes --]
On Tue, Jan 14, 2020 at 03:15:59PM +0800, Nicolas Boichat wrote:
> - I couldn't find a way to detect the number of regulators in the
> device tree, if we wanted to refuse to probe the device if there
> are too many regulators, which might be required for safety, see
> the thread on v2 [1].
You'd need to enumerate all the properties of the device and look
for things matching *-supply.
Reviewed-by: Mark Brown <broonie@kernel.org>
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 4/7] drm/panfrost: Add support for multiple regulators
2020-01-14 15:16 ` Mark Brown
@ 2020-01-21 4:37 ` Nicolas Boichat
2020-01-22 13:40 ` Steven Price
0 siblings, 1 reply; 16+ messages in thread
From: Nicolas Boichat @ 2020-01-21 4:37 UTC (permalink / raw)
To: Mark Brown
Cc: Mark Rutland, Devicetree List, Tomeu Vizoso, David Airlie, lkml,
Liam Girdwood, dri-devel, Steven Price, Rob Herring,
moderated list:ARM/Mediatek SoC support, Alyssa Rosenzweig,
Hsin-Yi Wang, Matthias Brugger, linux-arm Mailing List
On Tue, Jan 14, 2020 at 10:16 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Tue, Jan 14, 2020 at 03:15:59PM +0800, Nicolas Boichat wrote:
>
> > - I couldn't find a way to detect the number of regulators in the
> > device tree, if we wanted to refuse to probe the device if there
> > are too many regulators, which might be required for safety, see
> > the thread on v2 [1].
>
> You'd need to enumerate all the properties of the device and look
> for things matching *-supply.
I see ,-) I was hoping for something slightly cleaner, or maybe an
existing function in the core.
Steven: How strongly do you feel about this? If so I can add that
check in the next revision.
Also, just a heads-up, I'm out for the next 2 weeks, I'll send v4 after that.
>
> Reviewed-by: Mark Brown <broonie@kernel.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 4/7] drm/panfrost: Add support for multiple regulators
2020-01-21 4:37 ` Nicolas Boichat
@ 2020-01-22 13:40 ` Steven Price
0 siblings, 0 replies; 16+ messages in thread
From: Steven Price @ 2020-01-22 13:40 UTC (permalink / raw)
To: Nicolas Boichat, Mark Brown
Cc: Mark Rutland, Devicetree List, Tomeu Vizoso, David Airlie, lkml,
dri-devel, Liam Girdwood, Rob Herring,
moderated list:ARM/Mediatek SoC support, Alyssa Rosenzweig,
Hsin-Yi Wang, Matthias Brugger, linux-arm Mailing List
On 21/01/2020 04:37, Nicolas Boichat wrote:
> On Tue, Jan 14, 2020 at 10:16 PM Mark Brown <broonie@kernel.org> wrote:
>>
>> On Tue, Jan 14, 2020 at 03:15:59PM +0800, Nicolas Boichat wrote:
>>
>>> - I couldn't find a way to detect the number of regulators in the
>>> device tree, if we wanted to refuse to probe the device if there
>>> are too many regulators, which might be required for safety, see
>>> the thread on v2 [1].
>>
>> You'd need to enumerate all the properties of the device and look
>> for things matching *-supply.
>
> I see ,-) I was hoping for something slightly cleaner, or maybe an
> existing function in the core.
>
> Steven: How strongly do you feel about this? If so I can add that
> check in the next revision.
I'm not that strongly bothered about it - my only worry is that there
may be hardware out there that might be broken by not activating a
regulator. But I don't know how common this multi-regulator design is in
practise.
Thanks,
Steve
> Also, just a heads-up, I'm out for the next 2 weeks, I'll send v4 after that.
>
>>
>> Reviewed-by: Mark Brown <broonie@kernel.org>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 4/7] drm/panfrost: Add support for multiple regulators
2020-01-14 7:15 ` [PATCH v3 4/7] drm/panfrost: Add support for multiple regulators Nicolas Boichat
2020-01-14 15:16 ` Mark Brown
@ 2020-01-20 14:43 ` Steven Price
2020-01-20 17:03 ` Mark Brown
1 sibling, 1 reply; 16+ messages in thread
From: Steven Price @ 2020-01-20 14:43 UTC (permalink / raw)
To: Nicolas Boichat, Rob Herring
Cc: Mark Rutland, devicetree, Tomeu Vizoso, David Airlie,
linux-kernel, dri-devel, Liam Girdwood, Mark Brown,
linux-mediatek, Alyssa Rosenzweig, hsinyi, Matthias Brugger,
linux-arm-kernel
On 14/01/2020 07:15, Nicolas Boichat wrote:
> Some GPUs, namely, the bifrost/g72 part on MT8183, have a second
> regulator for their SRAM, let's add support for that.
>
> We extend the framework in a generic manner so that we could
> support more than 2 regulators, if required.
>
> Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
>
> ---
>
> v3:
> - Make this more generic, by allowing any number of regulators
> (in practice we fix the maximum number of regulators to 2, but
> this could be increased easily).
> - We only probe the second regulator if the device tree matching
> data asks for it.
> - I couldn't find a way to detect the number of regulators in the
> device tree, if we wanted to refuse to probe the device if there
> are too many regulators, which might be required for safety, see
> the thread on v2 [1].
> - The discussion also included the idea of a separate device tree
> entry for a "soft PDC", or at least a separate driver. I'm not
> sure to understand the full picture, and how different vendors
> implement this, so I'm still integrating everything in the main
> driver. I'd be happy to try to make mt8183 fit into such a
> framework after it's created, but I don't think I'm best placed
> to implement (and again, the main purpose of this was to test
> if the binding is correct).
From discussions offline, I think I've come round to the view that
having a "soft PDC" in device tree isn't the right solution. Device tree
should be describing the hardware and that isn't actually a hardware
component.
I guess we'll have to wait to see how many devices have a similar
'quirk' and whether it's worth representing this is software in a more
generic manner, or if matching on compatible strings will be sufficient
for the devices that need multiple regulators.
One (minor) comment below, but otherwise LGTM.
>
> [1] https://patchwork.kernel.org/patch/11322839/
>
> drivers/gpu/drm/panfrost/panfrost_device.c | 25 ++++++++++++-------
> drivers/gpu/drm/panfrost/panfrost_device.h | 15 +++++++++++-
> drivers/gpu/drm/panfrost/panfrost_drv.c | 28 +++++++++++++++-------
> 3 files changed, 50 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c
> index 238fb6d54df4732..c30e0a3772a4f57 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_device.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_device.c
> @@ -87,18 +87,26 @@ static void panfrost_clk_fini(struct panfrost_device *pfdev)
>
> static int panfrost_regulator_init(struct panfrost_device *pfdev)
> {
> - int ret;
> + int ret, i;
>
> - pfdev->regulator = devm_regulator_get(pfdev->dev, "mali");
> - if (IS_ERR(pfdev->regulator)) {
> - ret = PTR_ERR(pfdev->regulator);
> - dev_err(pfdev->dev, "failed to get regulator: %d\n", ret);
> + BUG_ON(pfdev->comp->num_supplies > ARRAY_SIZE(pfdev->regulators));
> +
> + for (i = 0; i < pfdev->comp->num_supplies; i++) {
> + pfdev->regulators[i].supply = pfdev->comp->supply_names[i];
> + }
> +
> + ret = devm_regulator_bulk_get(pfdev->dev,
> + pfdev->comp->num_supplies,
> + pfdev->regulators);
> + if (ret < 0) {
> + dev_err(pfdev->dev, "failed to get regulators: %d\n", ret);
> return ret;
> }
>
> - ret = regulator_enable(pfdev->regulator);
> + ret = regulator_bulk_enable(pfdev->comp->num_supplies,
> + pfdev->regulators);
> if (ret < 0) {
> - dev_err(pfdev->dev, "failed to enable regulator: %d\n", ret);
> + dev_err(pfdev->dev, "failed to enable regulators: %d\n", ret);
> return ret;
> }
>
> @@ -107,7 +115,8 @@ static int panfrost_regulator_init(struct panfrost_device *pfdev)
>
> static void panfrost_regulator_fini(struct panfrost_device *pfdev)
> {
> - regulator_disable(pfdev->regulator);
> + regulator_bulk_disable(pfdev->comp->num_supplies,
> + pfdev->regulators);
> }
>
> int panfrost_device_init(struct panfrost_device *pfdev)
> diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h
> index 06713811b92cdf7..021f063ffb3747f 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_device.h
> +++ b/drivers/gpu/drm/panfrost/panfrost_device.h
> @@ -7,6 +7,7 @@
>
> #include <linux/atomic.h>
> #include <linux/io-pgtable.h>
> +#include <linux/regulator/consumer.h>
> #include <linux/spinlock.h>
> #include <drm/drm_device.h>
> #include <drm/drm_mm.h>
> @@ -19,6 +20,7 @@ struct panfrost_job;
> struct panfrost_perfcnt;
>
> #define NUM_JOB_SLOTS 3
> +#define MAX_REGULATORS 2
>
> struct panfrost_features {
> u16 id;
> @@ -51,6 +53,16 @@ struct panfrost_features {
> unsigned long hw_issues[64 / BITS_PER_LONG];
> };
>
> +/*
> + * Features that cannot be automatically detected and need matching using the
> + * compatible string, typically SoC-specific.
> + */
> +struct panfrost_compatible {
> + /* Supplies count and names. */
> + int num_supplies;
> + const char * const *supply_names;
> +};
> +
> struct panfrost_device {
> struct device *dev;
> struct drm_device *ddev;
> @@ -59,10 +71,11 @@ struct panfrost_device {
> void __iomem *iomem;
> struct clk *clock;
> struct clk *bus_clock;
> - struct regulator *regulator;
> + struct regulator_bulk_data regulators[MAX_REGULATORS];
> struct reset_control *rstc;
>
> struct panfrost_features features;
> + const struct panfrost_compatible* comp;
>
> spinlock_t as_lock;
> unsigned long as_in_use_mask;
> diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
> index 48e3c4165247cea..db3563b80150c9d 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
> @@ -510,6 +510,10 @@ static int panfrost_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, pfdev);
>
> + pfdev->comp = of_device_get_match_data(&pdev->dev);
> + if (!pfdev->comp)
> + return -ENODEV;
> +
> /* Allocate and initialze the DRM device. */
> ddev = drm_dev_alloc(&panfrost_drm_driver, &pdev->dev);
> if (IS_ERR(ddev))
> @@ -581,16 +585,22 @@ static int panfrost_remove(struct platform_device *pdev)
> return 0;
> }
>
> +const char * const default_supplies[] = { "mali" };
This should be static.
Steve
> +static const struct panfrost_compatible default_data = {
> + .num_supplies = ARRAY_SIZE(default_supplies),
> + .supply_names = default_supplies,
> +};
> +
> static const struct of_device_id dt_match[] = {
> - { .compatible = "arm,mali-t604" },
> - { .compatible = "arm,mali-t624" },
> - { .compatible = "arm,mali-t628" },
> - { .compatible = "arm,mali-t720" },
> - { .compatible = "arm,mali-t760" },
> - { .compatible = "arm,mali-t820" },
> - { .compatible = "arm,mali-t830" },
> - { .compatible = "arm,mali-t860" },
> - { .compatible = "arm,mali-t880" },
> + { .compatible = "arm,mali-t604", .data = &default_data, },
> + { .compatible = "arm,mali-t624", .data = &default_data, },
> + { .compatible = "arm,mali-t628", .data = &default_data, },
> + { .compatible = "arm,mali-t720", .data = &default_data, },
> + { .compatible = "arm,mali-t760", .data = &default_data, },
> + { .compatible = "arm,mali-t820", .data = &default_data, },
> + { .compatible = "arm,mali-t830", .data = &default_data, },
> + { .compatible = "arm,mali-t860", .data = &default_data, },
> + { .compatible = "arm,mali-t880", .data = &default_data, },
> {}
> };
> MODULE_DEVICE_TABLE(of, dt_match);
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v3 4/7] drm/panfrost: Add support for multiple regulators
2020-01-20 14:43 ` Steven Price
@ 2020-01-20 17:03 ` Mark Brown
2020-01-20 17:09 ` Steven Price
0 siblings, 1 reply; 16+ messages in thread
From: Mark Brown @ 2020-01-20 17:03 UTC (permalink / raw)
To: Steven Price
Cc: Mark Rutland, devicetree, Nicolas Boichat, Tomeu Vizoso,
David Airlie, linux-kernel, dri-devel, Liam Girdwood,
Rob Herring, linux-mediatek, Alyssa Rosenzweig, hsinyi,
Matthias Brugger, linux-arm-kernel
[-- Attachment #1.1: Type: text/plain, Size: 584 bytes --]
On Mon, Jan 20, 2020 at 02:43:10PM +0000, Steven Price wrote:
> From discussions offline, I think I've come round to the view that
> having a "soft PDC" in device tree isn't the right solution. Device tree
> should be describing the hardware and that isn't actually a hardware
> component.
You can use an implementation like that separately to it being in the
device tree, it is perfectly possible to instantiate devices that have
no representation at all in device tree based on other things that are
there like board or SoC information, or as subdevices of things that are
there.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 4/7] drm/panfrost: Add support for multiple regulators
2020-01-20 17:03 ` Mark Brown
@ 2020-01-20 17:09 ` Steven Price
0 siblings, 0 replies; 16+ messages in thread
From: Steven Price @ 2020-01-20 17:09 UTC (permalink / raw)
To: Mark Brown
Cc: Mark Rutland, devicetree, Nicolas Boichat, Tomeu Vizoso,
David Airlie, linux-kernel, dri-devel, Liam Girdwood,
Rob Herring, linux-mediatek, Alyssa Rosenzweig, hsinyi,
Matthias Brugger, linux-arm-kernel
On 20/01/2020 17:03, Mark Brown wrote:
> On Mon, Jan 20, 2020 at 02:43:10PM +0000, Steven Price wrote:
>
>> From discussions offline, I think I've come round to the view that
>> having a "soft PDC" in device tree isn't the right solution. Device tree
>> should be describing the hardware and that isn't actually a hardware
>> component.
>
> You can use an implementation like that separately to it being in the
> device tree, it is perfectly possible to instantiate devices that have
> no representation at all in device tree based on other things that are
> there like board or SoC information, or as subdevices of things that are
> there.
Yes - and I may yet implement a "soft PDC" device if this turns out to
be more than a 'quirk' for a very small number of device. But like you
say - it doesn't need to be (and shouldn't be) in the actual device tree.
For now though I think the code Nicolas has written works well enough
and it's only really worth 'fixing' if we end up with too many 'quirky'
devices.
Steve
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 5/7] drm/panfrost: Add support for multiple power domains
2020-01-14 7:15 [PATCH v3 0/7] Add dts for mt8183 GPU (and misc panfrost patches) Nicolas Boichat
` (3 preceding siblings ...)
2020-01-14 7:15 ` [PATCH v3 4/7] drm/panfrost: Add support for multiple regulators Nicolas Boichat
@ 2020-01-14 7:16 ` Nicolas Boichat
2020-01-20 14:53 ` Steven Price
2020-01-14 7:16 ` [PATCH v3 6/7, RFC] drm/panfrost: Add mt8183-mali compatible string Nicolas Boichat
2020-01-14 7:16 ` [PATCH v3 7/7, RFC] drm/panfrost: devfreq: Add support for 2 regulators Nicolas Boichat
6 siblings, 1 reply; 16+ messages in thread
From: Nicolas Boichat @ 2020-01-14 7:16 UTC (permalink / raw)
To: Rob Herring
Cc: Mark Rutland, devicetree, Tomeu Vizoso, David Airlie,
linux-kernel, Liam Girdwood, dri-devel, Steven Price, Mark Brown,
linux-mediatek, Alyssa Rosenzweig, hsinyi, Matthias Brugger,
linux-arm-kernel
When there is a single power domain per device, the core will
ensure the power domain is switched on (so it is technically
equivalent to having not power domain specified at all).
However, when there are multiple domains, as in MT8183 Bifrost
GPU, we need to handle them in driver code.
Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
---
The downstream driver we use on chromeos-4.19 currently uses 2
additional devices in device tree to accomodate for this [1], but
I believe this solution is cleaner.
[1] https://chromium.googlesource.com/chromiumos/third_party/kernel/+/refs/heads/chromeos-4.19/drivers/gpu/arm/midgard/platform/mediatek/mali_kbase_runtime_pm.c#31
v3:
- Use the compatible matching data to specify the number of power
domains. Note that setting 0 or 1 in num_pm_domains is equivalent
as the core will handle these 2 cases in the exact same way
(automatically, without driver intervention), and there should
be no adverse consequence in this case (the concern is about
switching on only some power domains and not others).
drivers/gpu/drm/panfrost/panfrost_device.c | 95 ++++++++++++++++++++--
drivers/gpu/drm/panfrost/panfrost_device.h | 9 ++
drivers/gpu/drm/panfrost/panfrost_drv.c | 1 +
3 files changed, 97 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c
index c30e0a3772a4f57..7c9766f76cc7689 100644
--- a/drivers/gpu/drm/panfrost/panfrost_device.c
+++ b/drivers/gpu/drm/panfrost/panfrost_device.c
@@ -5,6 +5,7 @@
#include <linux/clk.h>
#include <linux/reset.h>
#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
#include <linux/regulator/consumer.h>
#include "panfrost_device.h"
@@ -119,6 +120,75 @@ static void panfrost_regulator_fini(struct panfrost_device *pfdev)
pfdev->regulators);
}
+static void panfrost_pm_domain_fini(struct panfrost_device *pfdev)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(pfdev->pm_domain_devs); i++) {
+ if (!pfdev->pm_domain_devs[i])
+ break;
+
+ if (pfdev->pm_domain_links[i])
+ device_link_del(pfdev->pm_domain_links[i]);
+
+ dev_pm_domain_detach(pfdev->pm_domain_devs[i], true);
+ }
+}
+
+static int panfrost_pm_domain_init(struct panfrost_device *pfdev)
+{
+ int err;
+ int i, num_domains;
+
+ num_domains = of_count_phandle_with_args(pfdev->dev->of_node,
+ "power-domains",
+ "#power-domain-cells");
+
+ /*
+ * Single domain is handled by the core, and, if only a single power
+ * the power domain is requested, the property is optional.
+ */
+ if (num_domains < 2 && pfdev->comp->num_pm_domains < 2)
+ return 0;
+
+ if (num_domains != pfdev->comp->num_pm_domains) {
+ dev_err(pfdev->dev,
+ "Incorrect number of power domains: %d provided, %d needed\n",
+ num_domains, pfdev->comp->num_pm_domains);
+ return -EINVAL;
+ }
+
+ BUG_ON(num_domains > ARRAY_SIZE(pfdev->pm_domain_devs));
+
+ for (i = 0; i < num_domains; i++) {
+ pfdev->pm_domain_devs[i] =
+ dev_pm_domain_attach_by_id(pfdev->dev, i);
+ if (IS_ERR(pfdev->pm_domain_devs[i])) {
+ err = PTR_ERR(pfdev->pm_domain_devs[i]);
+ pfdev->pm_domain_devs[i] = NULL;
+ dev_err(pfdev->dev,
+ "failed to get pm-domain %d: %d\n", i, err);
+ goto err;
+ }
+
+ pfdev->pm_domain_links[i] = device_link_add(pfdev->dev,
+ pfdev->pm_domain_devs[i], DL_FLAG_PM_RUNTIME |
+ DL_FLAG_STATELESS | DL_FLAG_RPM_ACTIVE);
+ if (!pfdev->pm_domain_links[i]) {
+ dev_err(pfdev->pm_domain_devs[i],
+ "adding device link failed!\n");
+ err = -ENODEV;
+ goto err;
+ }
+ }
+
+ return 0;
+
+err:
+ panfrost_pm_domain_fini(pfdev);
+ return err;
+}
+
int panfrost_device_init(struct panfrost_device *pfdev)
{
int err;
@@ -149,37 +219,45 @@ int panfrost_device_init(struct panfrost_device *pfdev)
goto err_out1;
}
+ err = panfrost_pm_domain_init(pfdev);
+ if (err) {
+ dev_err(pfdev->dev, "pm_domain init failed %d\n", err);
+ goto err_out2;
+ }
+
res = platform_get_resource(pfdev->pdev, IORESOURCE_MEM, 0);
pfdev->iomem = devm_ioremap_resource(pfdev->dev, res);
if (IS_ERR(pfdev->iomem)) {
dev_err(pfdev->dev, "failed to ioremap iomem\n");
err = PTR_ERR(pfdev->iomem);
- goto err_out2;
+ goto err_out3;
}
err = panfrost_gpu_init(pfdev);
if (err)
- goto err_out2;
+ goto err_out3;
err = panfrost_mmu_init(pfdev);
if (err)
- goto err_out3;
+ goto err_out4;
err = panfrost_job_init(pfdev);
if (err)
- goto err_out4;
+ goto err_out5;
err = panfrost_perfcnt_init(pfdev);
if (err)
- goto err_out5;
+ goto err_out6;
return 0;
-err_out5:
+err_out6:
panfrost_job_fini(pfdev);
-err_out4:
+err_out5:
panfrost_mmu_fini(pfdev);
-err_out3:
+err_out4:
panfrost_gpu_fini(pfdev);
+err_out3:
+ panfrost_pm_domain_fini(pfdev);
err_out2:
panfrost_reset_fini(pfdev);
err_out1:
@@ -196,6 +274,7 @@ void panfrost_device_fini(struct panfrost_device *pfdev)
panfrost_mmu_fini(pfdev);
panfrost_gpu_fini(pfdev);
panfrost_reset_fini(pfdev);
+ panfrost_pm_domain_fini(pfdev);
panfrost_regulator_fini(pfdev);
panfrost_clk_fini(pfdev);
}
diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h
index 021f063ffb3747f..143eab57180a2e1 100644
--- a/drivers/gpu/drm/panfrost/panfrost_device.h
+++ b/drivers/gpu/drm/panfrost/panfrost_device.h
@@ -21,6 +21,7 @@ struct panfrost_perfcnt;
#define NUM_JOB_SLOTS 3
#define MAX_REGULATORS 2
+#define MAX_PM_DOMAINS 3
struct panfrost_features {
u16 id;
@@ -61,6 +62,11 @@ struct panfrost_compatible {
/* Supplies count and names. */
int num_supplies;
const char * const *supply_names;
+ /*
+ * Number of power domains required, note that values 0 and 1 are
+ * handled identically, as only values > 1 need special handling.
+ */
+ int num_pm_domains;
};
struct panfrost_device {
@@ -73,6 +79,9 @@ struct panfrost_device {
struct clk *bus_clock;
struct regulator_bulk_data regulators[MAX_REGULATORS];
struct reset_control *rstc;
+ /* pm_domains for devices with more than one. */
+ struct device *pm_domain_devs[MAX_PM_DOMAINS];
+ struct device_link *pm_domain_links[MAX_PM_DOMAINS];
struct panfrost_features features;
const struct panfrost_compatible* comp;
diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
index db3563b80150c9d..42b87e29e605149 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -589,6 +589,7 @@ const char * const default_supplies[] = { "mali" };
static const struct panfrost_compatible default_data = {
.num_supplies = ARRAY_SIZE(default_supplies),
.supply_names = default_supplies,
+ .num_pm_domains = 1, /* optional */
};
static const struct of_device_id dt_match[] = {
--
2.25.0.rc1.283.g88dfdc4193-goog
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v3 5/7] drm/panfrost: Add support for multiple power domains
2020-01-14 7:16 ` [PATCH v3 5/7] drm/panfrost: Add support for multiple power domains Nicolas Boichat
@ 2020-01-20 14:53 ` Steven Price
2020-02-07 3:06 ` Nicolas Boichat
0 siblings, 1 reply; 16+ messages in thread
From: Steven Price @ 2020-01-20 14:53 UTC (permalink / raw)
To: Nicolas Boichat, Rob Herring
Cc: Mark Rutland, devicetree, Tomeu Vizoso, David Airlie,
linux-kernel, dri-devel, Liam Girdwood, Mark Brown,
linux-mediatek, Alyssa Rosenzweig, hsinyi, Matthias Brugger,
linux-arm-kernel
On 14/01/2020 07:16, Nicolas Boichat wrote:
> When there is a single power domain per device, the core will
> ensure the power domain is switched on (so it is technically
> equivalent to having not power domain specified at all).
>
> However, when there are multiple domains, as in MT8183 Bifrost
> GPU, we need to handle them in driver code.
>
> Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
>
> ---
>
> The downstream driver we use on chromeos-4.19 currently uses 2
> additional devices in device tree to accomodate for this [1], but
> I believe this solution is cleaner.
>
> [1] https://chromium.googlesource.com/chromiumos/third_party/kernel/+/refs/heads/chromeos-4.19/drivers/gpu/arm/midgard/platform/mediatek/mali_kbase_runtime_pm.c#31
>
> v3:
> - Use the compatible matching data to specify the number of power
> domains. Note that setting 0 or 1 in num_pm_domains is equivalent
> as the core will handle these 2 cases in the exact same way
> (automatically, without driver intervention), and there should
> be no adverse consequence in this case (the concern is about
> switching on only some power domains and not others).
>
> drivers/gpu/drm/panfrost/panfrost_device.c | 95 ++++++++++++++++++++--
> drivers/gpu/drm/panfrost/panfrost_device.h | 9 ++
> drivers/gpu/drm/panfrost/panfrost_drv.c | 1 +
> 3 files changed, 97 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c
> index c30e0a3772a4f57..7c9766f76cc7689 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_device.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_device.c
> @@ -5,6 +5,7 @@
> #include <linux/clk.h>
> #include <linux/reset.h>
> #include <linux/platform_device.h>
> +#include <linux/pm_domain.h>
> #include <linux/regulator/consumer.h>
>
> #include "panfrost_device.h"
> @@ -119,6 +120,75 @@ static void panfrost_regulator_fini(struct panfrost_device *pfdev)
> pfdev->regulators);
> }
>
> +static void panfrost_pm_domain_fini(struct panfrost_device *pfdev)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(pfdev->pm_domain_devs); i++) {
> + if (!pfdev->pm_domain_devs[i])
> + break;
> +
> + if (pfdev->pm_domain_links[i])
> + device_link_del(pfdev->pm_domain_links[i]);
> +
> + dev_pm_domain_detach(pfdev->pm_domain_devs[i], true);
> + }
> +}
> +
> +static int panfrost_pm_domain_init(struct panfrost_device *pfdev)
> +{
> + int err;
> + int i, num_domains;
> +
> + num_domains = of_count_phandle_with_args(pfdev->dev->of_node,
> + "power-domains",
> + "#power-domain-cells");
> +
> + /*
> + * Single domain is handled by the core, and, if only a single power
> + * the power domain is requested, the property is optional.
> + */
> + if (num_domains < 2 && pfdev->comp->num_pm_domains < 2)
> + return 0;
> +
> + if (num_domains != pfdev->comp->num_pm_domains) {
> + dev_err(pfdev->dev,
> + "Incorrect number of power domains: %d provided, %d needed\n",
> + num_domains, pfdev->comp->num_pm_domains);
> + return -EINVAL;
> + }
> +
> + BUG_ON(num_domains > ARRAY_SIZE(pfdev->pm_domain_devs));
> +
> + for (i = 0; i < num_domains; i++) {
> + pfdev->pm_domain_devs[i] =
> + dev_pm_domain_attach_by_id(pfdev->dev, i);
> + if (IS_ERR(pfdev->pm_domain_devs[i])) {
> + err = PTR_ERR(pfdev->pm_domain_devs[i]);
> + pfdev->pm_domain_devs[i] = NULL;
> + dev_err(pfdev->dev,
> + "failed to get pm-domain %d: %d\n", i, err);
> + goto err;
> + }
> +
> + pfdev->pm_domain_links[i] = device_link_add(pfdev->dev,
> + pfdev->pm_domain_devs[i], DL_FLAG_PM_RUNTIME |
> + DL_FLAG_STATELESS | DL_FLAG_RPM_ACTIVE);
> + if (!pfdev->pm_domain_links[i]) {
> + dev_err(pfdev->pm_domain_devs[i],
> + "adding device link failed!\n");
> + err = -ENODEV;
> + goto err;
> + }
> + }
> +
> + return 0;
> +
> +err:
> + panfrost_pm_domain_fini(pfdev);
> + return err;
> +}
> +
> int panfrost_device_init(struct panfrost_device *pfdev)
> {
> int err;
> @@ -149,37 +219,45 @@ int panfrost_device_init(struct panfrost_device *pfdev)
> goto err_out1;
> }
>
> + err = panfrost_pm_domain_init(pfdev);
> + if (err) {
> + dev_err(pfdev->dev, "pm_domain init failed %d\n", err);
No need for this print - panfrost_pm_domain_init() will output a (more
appropriate) error message on failure.
> + goto err_out2;
> + }
> +
> res = platform_get_resource(pfdev->pdev, IORESOURCE_MEM, 0);
> pfdev->iomem = devm_ioremap_resource(pfdev->dev, res);
> if (IS_ERR(pfdev->iomem)) {
> dev_err(pfdev->dev, "failed to ioremap iomem\n");
> err = PTR_ERR(pfdev->iomem);
> - goto err_out2;
> + goto err_out3;
> }
>
> err = panfrost_gpu_init(pfdev);
> if (err)
> - goto err_out2;
> + goto err_out3;
>
> err = panfrost_mmu_init(pfdev);
> if (err)
> - goto err_out3;
> + goto err_out4;
>
> err = panfrost_job_init(pfdev);
> if (err)
> - goto err_out4;
> + goto err_out5;
>
> err = panfrost_perfcnt_init(pfdev);
> if (err)
> - goto err_out5;
> + goto err_out6;
>
> return 0;
> -err_out5:
> +err_out6:
> panfrost_job_fini(pfdev);
> -err_out4:
> +err_out5:
> panfrost_mmu_fini(pfdev);
> -err_out3:
> +err_out4:
> panfrost_gpu_fini(pfdev);
> +err_out3:
> + panfrost_pm_domain_fini(pfdev);
> err_out2:
> panfrost_reset_fini(pfdev);
> err_out1:
> @@ -196,6 +274,7 @@ void panfrost_device_fini(struct panfrost_device *pfdev)
> panfrost_mmu_fini(pfdev);
> panfrost_gpu_fini(pfdev);
> panfrost_reset_fini(pfdev);
> + panfrost_pm_domain_fini(pfdev);
NIT: The reverse of the construction order would be to do this before
panfrost_reset_fini().
Otherwise this LGTM.
Thanks,
Steve
> panfrost_regulator_fini(pfdev);
> panfrost_clk_fini(pfdev);
> }
> diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h
> index 021f063ffb3747f..143eab57180a2e1 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_device.h
> +++ b/drivers/gpu/drm/panfrost/panfrost_device.h
> @@ -21,6 +21,7 @@ struct panfrost_perfcnt;
>
> #define NUM_JOB_SLOTS 3
> #define MAX_REGULATORS 2
> +#define MAX_PM_DOMAINS 3
>
> struct panfrost_features {
> u16 id;
> @@ -61,6 +62,11 @@ struct panfrost_compatible {
> /* Supplies count and names. */
> int num_supplies;
> const char * const *supply_names;
> + /*
> + * Number of power domains required, note that values 0 and 1 are
> + * handled identically, as only values > 1 need special handling.
> + */
> + int num_pm_domains;
> };
>
> struct panfrost_device {
> @@ -73,6 +79,9 @@ struct panfrost_device {
> struct clk *bus_clock;
> struct regulator_bulk_data regulators[MAX_REGULATORS];
> struct reset_control *rstc;
> + /* pm_domains for devices with more than one. */
> + struct device *pm_domain_devs[MAX_PM_DOMAINS];
> + struct device_link *pm_domain_links[MAX_PM_DOMAINS];
>
> struct panfrost_features features;
> const struct panfrost_compatible* comp;
> diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
> index db3563b80150c9d..42b87e29e605149 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_drv.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
> @@ -589,6 +589,7 @@ const char * const default_supplies[] = { "mali" };
> static const struct panfrost_compatible default_data = {
> .num_supplies = ARRAY_SIZE(default_supplies),
> .supply_names = default_supplies,
> + .num_pm_domains = 1, /* optional */
> };
>
> static const struct of_device_id dt_match[] = {
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v3 5/7] drm/panfrost: Add support for multiple power domains
2020-01-20 14:53 ` Steven Price
@ 2020-02-07 3:06 ` Nicolas Boichat
0 siblings, 0 replies; 16+ messages in thread
From: Nicolas Boichat @ 2020-02-07 3:06 UTC (permalink / raw)
To: Steven Price
Cc: Mark Rutland, Devicetree List, Tomeu Vizoso, David Airlie,
Mark Brown, lkml, dri-devel, Liam Girdwood, Rob Herring,
moderated list:ARM/Mediatek SoC support, Alyssa Rosenzweig,
Hsin-Yi Wang, Matthias Brugger, linux-arm Mailing List
On Mon, Jan 20, 2020 at 10:53 PM Steven Price <steven.price@arm.com> wrote:
>
> On 14/01/2020 07:16, Nicolas Boichat wrote:
> [snip]
> >
> > + err = panfrost_pm_domain_init(pfdev);
> > + if (err) {
> > + dev_err(pfdev->dev, "pm_domain init failed %d\n", err);
>
> No need for this print - panfrost_pm_domain_init() will output a (more
> appropriate) error message on failure.
Dropped.
> > + goto err_out2;
> > + }
> > +
> [snip]
> > @@ -196,6 +274,7 @@ void panfrost_device_fini(struct panfrost_device *pfdev)
> > panfrost_mmu_fini(pfdev);
> > panfrost_gpu_fini(pfdev);
> > panfrost_reset_fini(pfdev);
> > + panfrost_pm_domain_fini(pfdev);
>
> NIT: The reverse of the construction order would be to do this before
> panfrost_reset_fini().
Oh right, fixed.
Thanks.
> [snip]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 6/7, RFC] drm/panfrost: Add mt8183-mali compatible string
2020-01-14 7:15 [PATCH v3 0/7] Add dts for mt8183 GPU (and misc panfrost patches) Nicolas Boichat
` (4 preceding siblings ...)
2020-01-14 7:16 ` [PATCH v3 5/7] drm/panfrost: Add support for multiple power domains Nicolas Boichat
@ 2020-01-14 7:16 ` Nicolas Boichat
2020-01-14 7:16 ` [PATCH v3 7/7, RFC] drm/panfrost: devfreq: Add support for 2 regulators Nicolas Boichat
6 siblings, 0 replies; 16+ messages in thread
From: Nicolas Boichat @ 2020-01-14 7:16 UTC (permalink / raw)
To: Rob Herring
Cc: Mark Rutland, devicetree, Tomeu Vizoso, David Airlie,
linux-kernel, Liam Girdwood, dri-devel, Steven Price, Mark Brown,
linux-mediatek, Alyssa Rosenzweig, hsinyi, Matthias Brugger,
linux-arm-kernel
For testing only, the driver doesn't really work yet, AFAICT.
Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
---
v3:
- Match mt8183-mali instead of bifrost, as we require special
handling for the 2 regulators and 3 power domains.
drivers/gpu/drm/panfrost/panfrost_drv.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
index 42b87e29e605149..3379a3ea754ccde 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -592,6 +592,13 @@ static const struct panfrost_compatible default_data = {
.num_pm_domains = 1, /* optional */
};
+const char * const mediatek_mt8183_supplies[] = { "mali", "sram" };
+static const struct panfrost_compatible mediatek_mt8183_data = {
+ .num_supplies = ARRAY_SIZE(mediatek_mt8183_supplies),
+ .supply_names = mediatek_mt8183_supplies,
+ .num_pm_domains = 3,
+};
+
static const struct of_device_id dt_match[] = {
{ .compatible = "arm,mali-t604", .data = &default_data, },
{ .compatible = "arm,mali-t624", .data = &default_data, },
@@ -602,6 +609,8 @@ static const struct of_device_id dt_match[] = {
{ .compatible = "arm,mali-t830", .data = &default_data, },
{ .compatible = "arm,mali-t860", .data = &default_data, },
{ .compatible = "arm,mali-t880", .data = &default_data, },
+ { .compatible = "mediatek,mt8183-mali",
+ .data = &mediatek_mt8183_data },
{}
};
MODULE_DEVICE_TABLE(of, dt_match);
--
2.25.0.rc1.283.g88dfdc4193-goog
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v3 7/7, RFC] drm/panfrost: devfreq: Add support for 2 regulators
2020-01-14 7:15 [PATCH v3 0/7] Add dts for mt8183 GPU (and misc panfrost patches) Nicolas Boichat
` (5 preceding siblings ...)
2020-01-14 7:16 ` [PATCH v3 6/7, RFC] drm/panfrost: Add mt8183-mali compatible string Nicolas Boichat
@ 2020-01-14 7:16 ` Nicolas Boichat
6 siblings, 0 replies; 16+ messages in thread
From: Nicolas Boichat @ 2020-01-14 7:16 UTC (permalink / raw)
To: Rob Herring
Cc: Mark Rutland, devicetree, Tomeu Vizoso, David Airlie,
linux-kernel, Liam Girdwood, dri-devel, Steven Price, Mark Brown,
linux-mediatek, Alyssa Rosenzweig, hsinyi, Matthias Brugger,
linux-arm-kernel
The Bifrost GPU on MT8183 uses 2 regulators (core and SRAM) for
devfreq, and provides OPP table with 2 sets of voltages.
TODO: This is incomplete as we'll need add support for setting
a pair of voltages as well.
Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
---
drivers/gpu/drm/panfrost/panfrost_devfreq.c | 17 +++++++++++++++++
drivers/gpu/drm/panfrost/panfrost_device.h | 1 +
2 files changed, 18 insertions(+)
diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
index 413987038fbfccb..9c0987a3d71c597 100644
--- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
+++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
@@ -79,6 +79,21 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
struct devfreq *devfreq;
struct thermal_cooling_device *cooling;
+ /* If we have 2 regulator, we need an OPP table with 2 voltages. */
+ if (pfdev->comp->num_supplies > 1) {
+ pfdev->devfreq.dev_opp_table =
+ dev_pm_opp_set_regulators(dev,
+ pfdev->comp->supply_names,
+ pfdev->comp->num_supplies);
+ if (IS_ERR(pfdev->devfreq.dev_opp_table)) {
+ ret = PTR_ERR(pfdev->devfreq.dev_opp_table);
+ pfdev->devfreq.dev_opp_table = NULL;
+ dev_err(dev,
+ "Failed to init devfreq opp table: %d\n", ret);
+ return ret;
+ }
+ }
+
ret = dev_pm_opp_of_add_table(dev);
if (ret == -ENODEV) /* Optional, continue without devfreq */
return 0;
@@ -119,6 +134,8 @@ void panfrost_devfreq_fini(struct panfrost_device *pfdev)
if (pfdev->devfreq.cooling)
devfreq_cooling_unregister(pfdev->devfreq.cooling);
dev_pm_opp_of_remove_table(&pfdev->pdev->dev);
+ if (pfdev->devfreq.dev_opp_table)
+ dev_pm_opp_put_regulators(pfdev->devfreq.dev_opp_table);
}
void panfrost_devfreq_resume(struct panfrost_device *pfdev)
diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h
index 143eab57180a2e1..30ba11cbf600847 100644
--- a/drivers/gpu/drm/panfrost/panfrost_device.h
+++ b/drivers/gpu/drm/panfrost/panfrost_device.h
@@ -108,6 +108,7 @@ struct panfrost_device {
struct {
struct devfreq *devfreq;
struct thermal_cooling_device *cooling;
+ struct opp_table *dev_opp_table;
ktime_t busy_time;
ktime_t idle_time;
ktime_t time_last_update;
--
2.25.0.rc1.283.g88dfdc4193-goog
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 16+ messages in thread