[Lima] [PATCH] drm/lima: Convert to clk_bulk API

[Qiang Yu]

On Sat, Jul 17, 2021 at 9:08 PM Marek Vasut <marex at denx.de> wrote:
>
> On 7/17/21 2:34 PM, Qiang Yu wrote:
> > On Sat, Jul 17, 2021 at 2:20 AM Marek Vasut <marex at denx.de> 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@denx.de/
>
> > 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 at denx.de>
> >> Cc: Qiang Yu <yuq825 at gmail.com>
> >> Cc: lima at lists.freedesktop.org
> >> ---
> >>   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".