From: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>
To: Stephen Boyd <sboyd@kernel.org>
Cc: Michael Turquette <mturquette@baylibre.com>,
Andy Gross <agross@kernel.org>,
Bjorn Andersson <bjorn.andersson@linaro.org>,
Marc Gonzalez <marc.w.gonzalez@free.fr>,
MSM <linux-arm-msm@vger.kernel.org>,
linux-clk@vger.kernel.org, lkml <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v4 1/2] clk: qcom: Add MSM8998 GPU Clock Controller (GPUCC) driver
Date: Thu, 17 Oct 2019 17:16:48 -0600 [thread overview]
Message-ID: <CAOCk7NqgWkt6BwY75eGS2dbJ7GGk3DqH5NC0VLHUq4fc6WuYog@mail.gmail.com> (raw)
In-Reply-To: <20191017215023.2BFEC20872@mail.kernel.org>
On Thu, Oct 17, 2019 at 3:50 PM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Jeffrey Hugo (2019-10-01 18:16:40)
> > diff --git a/drivers/clk/qcom/gpucc-msm8998.c b/drivers/clk/qcom/gpucc-msm8998.c
> > new file mode 100644
> > index 000000000000..f0ccb4963885
> > --- /dev/null
> > +++ b/drivers/clk/qcom/gpucc-msm8998.c
> > @@ -0,0 +1,346 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2019, Jeffrey Hugo
> > + */
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/bitops.h>
> > +#include <linux/err.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> > +#include <linux/clk-provider.h>
> > +#include <linux/regmap.h>
> > +#include <linux/reset-controller.h>
> > +#include <linux/clk.h>
>
> Drop this include please.
Will do.
>
> > +
> > +
> > +static struct clk_rcg2 rbcpr_clk_src = {
> > + .cmd_rcgr = 0x1030,
> > + .hid_width = 5,
> > + .parent_map = gpu_xo_gpll0_map,
> > + .freq_tbl = ftbl_rbcpr_clk_src,
> > + .clkr.hw.init = &(struct clk_init_data){
> > + .name = "rbcpr_clk_src",
> > + .parent_data = gpu_xo_gpll0,
> > + .num_parents = 2,
> > + .ops = &clk_rcg2_ops,
> > + },
> > +};
> > +
> > +static const struct freq_tbl ftbl_gfx3d_clk_src[] = {
> > + F(180000000, P_GPUPLL0_OUT_EVEN, 2, 0, 0),
> > + F(257000000, P_GPUPLL0_OUT_EVEN, 2, 0, 0),
> > + F(342000000, P_GPUPLL0_OUT_EVEN, 2, 0, 0),
> > + F(414000000, P_GPUPLL0_OUT_EVEN, 2, 0, 0),
> > + F(515000000, P_GPUPLL0_OUT_EVEN, 2, 0, 0),
> > + F(596000000, P_GPUPLL0_OUT_EVEN, 2, 0, 0),
> > + F(670000000, P_GPUPLL0_OUT_EVEN, 2, 0, 0),
> > + F(710000000, P_GPUPLL0_OUT_EVEN, 2, 0, 0),
> > + { }
>
> I guess this one doesn't do PLL ping pong? Instead we just reprogram the
> PLL all the time? Can we have rcg2 clk ops that set the rate on the
> parent to be exactly twice as much as the incoming frequency? I thought
> we already had this support in the code. Indeed, it is part of
> _freq_tbl_determine_rate() in clk-rcg.c, but not yet implemented in the
> same function name in clk-rcg2.c! Can you implement it? That way we
> don't need this long frequency table, just this weird one where it looks
> like:
>
> { .src = P_GPUPLL0_OUT_EVEN, .pre_div = 3 }
> { }
>
> And then some more logic in the rcg2 ops to allow this possibility for a
> frequency table when CLK_SET_RATE_PARENT is set.
Does not do PLL ping pong. I'll look at extending the rcg2 ops like
you describe.
>
> > +};
> > +
> > +static struct clk_rcg2 gfx3d_clk_src = {
> > + .cmd_rcgr = 0x1070,
> > + .hid_width = 5,
> > + .parent_map = gpu_xo_gpupll0_map,
> > + .freq_tbl = ftbl_gfx3d_clk_src,
> > + .clkr.hw.init = &(struct clk_init_data){
> > + .name = "gfx3d_clk_src",
> > + .parent_data = gpu_xo_gpupll0,
> > + .num_parents = 2,
> > + .ops = &clk_rcg2_ops,
> > + .flags = CLK_OPS_PARENT_ENABLE,
>
> Needs CLK_SET_RATE_PARENT presumably?
Ah yeah. Thanks for catching.
>
> > + },
> > +};
> > +
> > +static const struct freq_tbl ftbl_rbbmtimer_clk_src[] = {
> > + F(19200000, P_XO, 1, 0, 0),
> > + { }
> > +};
> > +
> [...]
> > +
> > +static const struct qcom_cc_desc gpucc_msm8998_desc = {
> > + .config = &gpucc_msm8998_regmap_config,
> > + .clks = gpucc_msm8998_clocks,
> > + .num_clks = ARRAY_SIZE(gpucc_msm8998_clocks),
> > + .resets = gpucc_msm8998_resets,
> > + .num_resets = ARRAY_SIZE(gpucc_msm8998_resets),
> > + .gdscs = gpucc_msm8998_gdscs,
> > + .num_gdscs = ARRAY_SIZE(gpucc_msm8998_gdscs),
> > +};
> > +
> > +static const struct of_device_id gpucc_msm8998_match_table[] = {
> > + { .compatible = "qcom,gpucc-msm8998" },
>
> The compatible is different. In the merged binding it is
> qcom,msm8998-gpucc. Either fix this or fix the binding please.
This is wrong. Will fix.
next prev parent reply other threads:[~2019-10-17 23:17 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-02 1:15 [PATCH v4 0/2] MSM8998 GPUCC Support Jeffrey Hugo
2019-10-02 1:16 ` [PATCH v4 1/2] clk: qcom: Add MSM8998 GPU Clock Controller (GPUCC) driver Jeffrey Hugo
2019-10-17 21:50 ` Stephen Boyd
2019-10-17 23:16 ` Jeffrey Hugo [this message]
2019-10-18 21:11 ` Jeffrey Hugo
2019-10-27 21:36 ` Stephen Boyd
2019-10-28 14:17 ` Jeffrey Hugo
2019-10-18 4:11 ` Taniya Das
2019-10-18 14:24 ` Jeffrey Hugo
2019-10-02 1:17 ` [PATCH v4 2/2] arm64: dts: qcom: msm8998: Add gpucc node Jeffrey Hugo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAOCk7NqgWkt6BwY75eGS2dbJ7GGk3DqH5NC0VLHUq4fc6WuYog@mail.gmail.com \
--to=jeffrey.l.hugo@gmail.com \
--cc=agross@kernel.org \
--cc=bjorn.andersson@linaro.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=marc.w.gonzalez@free.fr \
--cc=mturquette@baylibre.com \
--cc=sboyd@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).