Messages in this thread |  | | Date | Mon, 13 Aug 2018 12:00:38 +0530 | From | Amit Nischal <> | Subject | Re: [PATCH 2/4] clk: qcom: Add clk_rcg2_gfx3d_ops for SDM845 |
| |
On 2018-08-08 11:28, Stephen Boyd wrote: > Quoting Jordan Crouse (2018-08-06 08:04:37) >> On Mon, Aug 06, 2018 at 02:37:18PM +0530, Amit Nischal wrote: >> > On 2018-08-03 04:14, Stephen Boyd wrote: >> > >Quoting Amit Nischal (2018-07-30 04:28:56) >> > >>On 2018-07-25 12:28, Stephen Boyd wrote: >> > >>> >> > >>> 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. >> > >> >> > > >> > >But GPU will use OPP so it doesn't seem like it really buys us anything >> > >here. And it really doesn't matter when the clk driver implementation >> > >doesn't use the min/max to clamp the values of the round_rate() >> > >call. Is >> > >that being done here? I need to double check. I would be more convinced >> > >if the implementation was looking at min/max to constrain the rate >> > >requested. >> > > >> > >> > So our understanding is that GPU(client) driver will always call the >> > set_rate with approved frequencies only and we can completely rely >> > on the >> > client. Is our understanding is correct? >> >> >> First: on sdm845 the software doesn't set the GPU clocks - we rely on >> the GMU >> firmware to do that on our behalf but for the GPU at least this is an >> academic >> exercise. > > So what is this GPU clk driver for then? > >> >> But that said: traditionally we've expected that the clock driver >> correctly >> clamp the requested rate to the correct values. In the past we have >> taken >> advantage of this and we may in the future. I don't think it is >> reasonable >> to require the leaf driver to only pass "approved" frequencies >> especially >> since we depend on our own OPP table that may or may not be similar to >> the >> one used by the clock driver. >> > > Ok. Sounds like things can't be kept in sync between the clk driver and > the OPP tables. Why is that hard to do? > > Either way, I'd be fine if the code actually used the frequency limits > to round the rate to something within range, but I don't recall seeing > that being done here. So if the min/max limits stay, the clk driver > should round to within that range. >
Thanks Stephen for your suggestion. I have modified the existing determine_rate() op to use the min/max limits and round the requested rate so that it stays withing the set_rate range. I will submit the same in the next patch series.
> -- > 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
|  |