Messages in this thread | | | Date | Thu, 1 Jun 2023 18:09:24 +0800 | Subject | Re: [PATCH v6 2/6] drm/etnaviv: add a dedicated function to get various clocks | From | Sui Jingfeng <> |
| |
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@loongson.cn> >> --- >> 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
| |