Re: [PATCH v6 2/6] drm/etnaviv: add a dedicated function to get various clocks

From: Sui Jingfeng
Date: Thu Jun 01 2023 - 06:11:11 EST


Hi,

On 2023/6/1 02:07, Lucas Stach wrote:
Am Mittwoch, dem 31.05.2023 um 00:06 +0800 schrieb Sui Jingfeng:
Because it is also platform-dependent, there are environments where don't
have CLK subsystem support, for example, discreted PCI gpu. So don't rage
quit if there is no CLK subsystem.

For the GPU in LS7a1000 and LS2k2000, the working frequency of the GPU is
tuned by configuring the PLL register directly.

Is this PLL under control of system firmware and invisible to Linux?
Yes, it is registers, both system firmware and kernel space driver can access it.
Signed-off-by: Sui Jingfeng <suijingfeng@xxxxxxxxxxx>
---
drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 62 ++++++++++++++++++---------
drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 1 +
2 files changed, 42 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index 636d3f39ddcb..4937580551a5 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -1565,10 +1565,45 @@ static irqreturn_t irq_handler(int irq, void *data)
return ret;
}
+static int etnaviv_gpu_clk_get(struct etnaviv_gpu *gpu)
+{
+ struct device *dev = gpu->dev;
+
+ if (gpu->no_clk)
+ return 0;
+
+ gpu->clk_reg = devm_clk_get_optional(dev, "reg");
+ DBG("clk_reg: %p", gpu->clk_reg);
+ if (IS_ERR(gpu->clk_reg))
+ return PTR_ERR(gpu->clk_reg);
+
+ gpu->clk_bus = devm_clk_get_optional(dev, "bus");
+ DBG("clk_bus: %p", gpu->clk_bus);
+ if (IS_ERR(gpu->clk_bus))
+ return PTR_ERR(gpu->clk_bus);
+
+ gpu->clk_core = devm_clk_get(dev, "core");
+ DBG("clk_core: %p", gpu->clk_core);
+ if (IS_ERR(gpu->clk_core))
+ return PTR_ERR(gpu->clk_core);
+ gpu->base_rate_core = clk_get_rate(gpu->clk_core);
+
+ gpu->clk_shader = devm_clk_get_optional(dev, "shader");
+ DBG("clk_shader: %p", gpu->clk_shader);
+ if (IS_ERR(gpu->clk_shader))
+ return PTR_ERR(gpu->clk_shader);
+ gpu->base_rate_shader = clk_get_rate(gpu->clk_shader);
+
+ return 0;
+}
+
static int etnaviv_gpu_clk_enable(struct etnaviv_gpu *gpu)
{
int ret;
+ if (gpu->no_clk)
+ return 0;
+
I don't see why this would be needed? If your platform doesn't provide
CONFIG_HAVE_CLK all those functions should be successful no-ops, so
there is no need to special case this in the driver.

Or does your platform in fact provide a clk subsystem, just the GPU
clocks are managed by it?

Also all those functions are fine with being called on a NULL clk,
right
so
shouldn't it be enough to simply avoid calling etnaviv_gpu_clk_get() in
the PCI device case?

Yes, I just tried, your are right.

There also no need to add the 'no_clk' member into struct etnaviv_gpu

Regards,
Lucas

ret = clk_prepare_enable(gpu->clk_reg);
if (ret)
return ret;
@@ -1599,6 +1634,9 @@ static int etnaviv_gpu_clk_enable(struct etnaviv_gpu *gpu)
static int etnaviv_gpu_clk_disable(struct etnaviv_gpu *gpu)
{
+ if (gpu->no_clk)
+ return 0;
+
clk_disable_unprepare(gpu->clk_shader);
clk_disable_unprepare(gpu->clk_core);
clk_disable_unprepare(gpu->clk_bus);
@@ -1865,27 +1903,9 @@ static int etnaviv_gpu_platform_probe(struct platform_device *pdev)
return err;
/* Get Clocks: */
- gpu->clk_reg = devm_clk_get_optional(&pdev->dev, "reg");
- DBG("clk_reg: %p", gpu->clk_reg);
- if (IS_ERR(gpu->clk_reg))
- return PTR_ERR(gpu->clk_reg);
-
- gpu->clk_bus = devm_clk_get_optional(&pdev->dev, "bus");
- DBG("clk_bus: %p", gpu->clk_bus);
- if (IS_ERR(gpu->clk_bus))
- return PTR_ERR(gpu->clk_bus);
-
- gpu->clk_core = devm_clk_get(&pdev->dev, "core");
- DBG("clk_core: %p", gpu->clk_core);
- if (IS_ERR(gpu->clk_core))
- return PTR_ERR(gpu->clk_core);
- gpu->base_rate_core = clk_get_rate(gpu->clk_core);
-
- gpu->clk_shader = devm_clk_get_optional(&pdev->dev, "shader");
- DBG("clk_shader: %p", gpu->clk_shader);
- if (IS_ERR(gpu->clk_shader))
- return PTR_ERR(gpu->clk_shader);
- gpu->base_rate_shader = clk_get_rate(gpu->clk_shader);
+ err = etnaviv_gpu_clk_get(gpu);
+ if (err)
+ return err;
/* TODO: figure out max mapped size */
dev_set_drvdata(dev, gpu);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
index 98c6f9c320fc..6da5209a7d64 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
@@ -148,6 +148,7 @@ struct etnaviv_gpu {
struct clk *clk_reg;
struct clk *clk_core;
struct clk *clk_shader;
+ bool no_clk;
unsigned int freq_scale;
unsigned long base_rate_core;

--
Jingfeng