On Sat, Jul 17, 2021 at 9:08 PM Marek Vasut <marex@xxxxxxx> wrote: > > On 7/17/21 2:34 PM, Qiang Yu wrote: > > On Sat, Jul 17, 2021 at 2:20 AM Marek Vasut <marex@xxxxxxx> wrote: > >> > >> Instead of requesting two separate clock and then handling them > >> separately in various places of the driver, use clk_bulk_*() API. > >> This permits handling devices with more than "bus"/"core" clock, > >> like ZynqMP, which has "gpu"/"gpu_pp0"/"gpu_pp1" all as separate > >> clock. > > > > I can't find the ZynqMP DTS file under arch/arm64/boot/dts/xilinx > > which has mali GPU node with an upstream kernel, where is it? > > Posted here: > https://patchwork.kernel.org/project/linux-arm-kernel/patch/20210716182544.219490-1-marex@xxxxxxx/ > > > So what's the relationship between "gpu" clk and "gpu_pp0"/"gpu_pp1" > > clk? Do they need to be controlled separately or we can just control the > > "gpu" clk? Because the devfreq code just controls a single module clk. > > Per the docs, they are separate enable bits and the zynqmp clock > controller exports them as separate clock, see bits 24..26 here: > > https://www.xilinx.com/html_docs/registers/ug1087/crf_apb___gpu_ref_ctrl.html > > >> Signed-off-by: Marek Vasut <marex@xxxxxxx> > >> Cc: Qiang Yu <yuq825@xxxxxxxxx> > >> Cc: lima@xxxxxxxxxxxxxxxxxxxxx > >> --- > >> drivers/gpu/drm/lima/lima_devfreq.c | 17 +++++++++--- > >> drivers/gpu/drm/lima/lima_devfreq.h | 1 + > >> drivers/gpu/drm/lima/lima_device.c | 42 +++++++++++------------------ > >> drivers/gpu/drm/lima/lima_device.h | 4 +-- > >> 4 files changed, 32 insertions(+), 32 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/lima/lima_devfreq.c b/drivers/gpu/drm/lima/lima_devfreq.c > >> index 8989e215dfc9..533b36932f79 100644 > >> --- a/drivers/gpu/drm/lima/lima_devfreq.c > >> +++ b/drivers/gpu/drm/lima/lima_devfreq.c > >> @@ -58,7 +58,7 @@ static int lima_devfreq_get_dev_status(struct device *dev, > >> struct lima_devfreq *devfreq = &ldev->devfreq; > >> unsigned long irqflags; > >> > >> - status->current_frequency = clk_get_rate(ldev->clk_gpu); > >> + status->current_frequency = clk_get_rate(devfreq->clk_gpu); > >> > >> spin_lock_irqsave(&devfreq->lock, irqflags); > >> > >> @@ -110,12 +110,23 @@ int lima_devfreq_init(struct lima_device *ldev) > >> struct lima_devfreq *ldevfreq = &ldev->devfreq; > >> struct dev_pm_opp *opp; > >> unsigned long cur_freq; > >> - int ret; > >> + int i, ret; > >> > >> if (!device_property_present(dev, "operating-points-v2")) > >> /* Optional, continue without devfreq */ > >> return 0; > >> > >> + /* Find first clock which are not "bus" clock */ > >> + for (i = 0; i < ldev->nr_clks; i++) { > >> + if (!strcmp(ldev->clks[i].id, "bus")) > >> + continue; > >> + ldevfreq->clk_gpu = ldev->clks[i].clk; > >> + break; > >> + } > > > > I'd prefer an explicit name for the required clk name. If some DTS has different > > name other than "core" for the module clk (ie. "gpu"), it should be changed to > > "core". > > The problem here is, the zynqmp has no core clock, it has "gpu and both > pixel pipes" super-clock-gate which controls everything, and then > per-pixel-pipe sub-clock-gates. So the "gpu" clk can gate both "gpu_pp0" and "gpu_pp1" clk, how about frequency? Can we set clock rate for "gpu" then "gpu_pp0" and "gpu_pp1" pass through the same rate? If so, "gpu" works just like "core".
