From: Amit Nischal <anischal@codeaurora.org>
To: Stephen Boyd <sboyd@kernel.org>
Cc: Michael Turquette <mturquette@baylibre.com>,
Andy Gross <andy.gross@linaro.org>,
David Brown <david.brown@linaro.org>,
Rajendra Nayak <rnayak@codeaurora.org>,
Odelu Kukatla <okukatla@codeaurora.org>,
Taniya Das <tdas@codeaurora.org>,
linux-arm-msm@vger.kernel.org, linux-soc@vger.kernel.org,
linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-clk-owner@vger.kernel.org
Subject: Re: [PATCH 2/4] clk: qcom: Add clk_rcg2_gfx3d_ops for SDM845
Date: Mon, 30 Jul 2018 16:58:56 +0530 [thread overview]
Message-ID: <07e0321116993d27d6585bd1a186328d@codeaurora.org> (raw)
In-Reply-To: <153250192252.48062.9210075387954345932@swboyd.mtv.corp.google.com>
On 2018-07-25 12:28, Stephen Boyd wrote:
> Quoting Amit Nischal (2018-07-12 05:30:21)
>> On 2018-07-09 11:45, Stephen Boyd wrote:
>> > Quoting Amit Nischal (2018-06-06 04:41:46)
>> >> To turn on the gpu_gx_gdsc, there is a hardware requirement to
>> >> turn on the root clock (GFX3D RCG) first which would be the turn
>> >> on signal for the gdsc along with the SW_COLLAPSE. As per the
>> >> current implementation of clk_rcg2_shared_ops, it clears the
>> >> root_enable bit in the enable() and set_rate() clock ops. But due
>> >> to the above said requirement for GFX3D shared RCG, root_enable bit
>> >> would be already set by gdsc driver and rcg2_shared_ops should not
>> >> clear
>> >> the root unless the disable is called.
>> >>
>> >
>> > It sounds like the GDSC enable is ANDed with the RCG's root enable
>> > bit?
>>
>> Here, the root clock (GFX3D RCG) needs to be enabled first before
>> writing to SW_COLLAPSE bit of the GDSC. RCG's CLK_ON signal would
>> power up the GDSC.
>>
>> > Does the RCG need to be clocking for the GDSC to actually turn on?
>> > Or is it purely that the enable bit is logically combined that way so
>> > that if the RCG is parented to a PLL that's off the GDSC will still
>> > turn
>> > on?
>> >
>>
>> Yes, the RCG needs to be clocking for the GPU_GX GDSC to actually turn
>> on.
>
> Cool, please add these details to the commit text.
Thanks. I will add these details in the commit text in the next patch
series.
>
>>
>> >> Add support for the same by reusing the existing clk_rcg2_shared_ops
>> >> and deriving "clk_rcg2_gfx3d_ops" clk_ops for GFX3D clock to
>> >> take care of the root set/clear requirement.
>> >
>> > Anyway, this patch will probably significantly change given that the
>> > RCG
>> > is a glorified div-2 that muxes between a safe CXO speed and a
>> > configurable PLL frequency. A lot of the logic can probably just be
>> > hardcoded then.
>> >
>>
>> Thanks for the suggestion.
>> We are planning to introduce the "clk_rcg2_gfx3d_determine_rate" op
>> which will
>> always return the best parent as ‘GPU_CC_PLL0_OUT_EVEN’ and
>> best_parent
>> rate equal to twice of the requested rate. With this approach
>> frequency
>> table
>> for rcg2 structure would not be required as all the supported
>> frequencies would
>> be derived from the GPU_CC_PLL0_OUT_EVEN src with a divider of 2.
>>
>> Also, we will add support to check the requested rate if falls within
>> allowed
>> set_rate range. This will make sure that the source PLL would not go
>> out
>> of
>> the supported range. set_rate_range would be set by the GPUCC driver
>> with min/max
>> value by calling below API.
>>
>> clk_hw_set_rate_range(&gpu_cc_gx_gfx3d_clk_src.clkr.hw, 180000000,
>> 710000000)
>
> Ok. Sounds good! Is the rate range call really needed? It can't be
> determined in the PLL code with some table or avoided by making sure
> GPU
> uses OPP table with only approved frequencies?
>
Currently fabia PLL code does not have any table to check this and
intention
was to avoid relying on the client to call set_rate with only approved
frequencies so we have added the set_rate_range() call in the GPUCC
driver
in order to set the rate range.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-clk" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2018-07-30 11:29 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-06-06 11:41 [PATCH 0/4] Add QCOM graphics clock controller driver for SDM845 Amit Nischal
2018-06-06 11:41 ` [PATCH 1/4] clk: qcom: gdsc: Add support to enable/disable the clocks with GDSC Amit Nischal
2018-07-09 5:34 ` Stephen Boyd
2018-07-12 12:23 ` Amit Nischal
2018-07-25 6:52 ` Stephen Boyd
2018-07-30 11:14 ` Amit Nischal
2018-06-06 11:41 ` [PATCH 2/4] clk: qcom: Add clk_rcg2_gfx3d_ops for SDM845 Amit Nischal
2018-07-09 6:15 ` Stephen Boyd
2018-07-12 12:30 ` Amit Nischal
2018-07-25 6:58 ` Stephen Boyd
2018-07-30 11:28 ` Amit Nischal [this message]
2018-08-02 22:44 ` Stephen Boyd
2018-08-06 9:07 ` Amit Nischal
2018-08-06 15:04 ` Jordan Crouse
2018-08-08 5:58 ` Stephen Boyd
2018-08-08 14:51 ` Jordan Crouse
2018-08-13 6:30 ` Amit Nischal
2018-06-06 11:41 ` [PATCH 3/4] dt-bindings: clock: Introduce QCOM Graphics clock bindings Amit Nischal
2018-07-09 5:38 ` Stephen Boyd
2018-07-12 12:32 ` Amit Nischal
2018-06-06 11:41 ` [PATCH 4/4] clk: qcom: Add graphics clock controller driver for SDM845 Amit Nischal
2018-07-09 6:07 ` Stephen Boyd
2018-07-12 12:38 ` Amit Nischal
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=07e0321116993d27d6585bd1a186328d@codeaurora.org \
--to=anischal@codeaurora.org \
--cc=andy.gross@linaro.org \
--cc=david.brown@linaro.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-clk-owner@vger.kernel.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-soc@vger.kernel.org \
--cc=mturquette@baylibre.com \
--cc=okukatla@codeaurora.org \
--cc=rnayak@codeaurora.org \
--cc=sboyd@kernel.org \
--cc=tdas@codeaurora.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).