Messages in this thread | | | Subject | Re: [PATCH v3 3/3] clk: qcom: Add display clock controller driver for SDM845 | From | Taniya Das <> | Date | Mon, 16 Jul 2018 11:27:48 +0530 |
| |
Hello Stephen,
On 7/13/2018 1:55 PM, spanda@codeaurora.org wrote: > On 2018-07-13 01:11, Stephen Boyd wrote: >> Quoting Taniya Das (2018-07-12 10:21:33) >>> ++ Display driver team, >>> >>> On 7/9/2018 8:36 PM, Stephen Boyd wrote: >>> > Quoting Taniya Das (2018-07-09 02:34:07) >>> >> >>> >> >>> >> On 7/9/2018 1:07 PM, Stephen Boyd wrote: >>> >>> Quoting Taniya Das (2018-07-09 00:07:21) >>> >>>> >>> >>>> >>> >>>> On 7/9/2018 11:46 AM, Stephen Boyd wrote: >>> >>>>>> >>> >>>>>> > Why is the nocache flag needed? Applies to all clks in >>> this file. >>> >>>>>> > >>> >>>>>> >>> >>>>>> This flag is required for all RCGs whose PLLs are controlled >>> outside the >>> >>>>>> clock controller. The display code would require the >>> recalculated rate >>> >>>>>> always. >>> >>>>> >>> >>>>> Right. Why is the PLL controlled outside of the clock >>> controller? The >>> >>>>> rate should propagate upward to the PLL from here, so who's going >>> >>>>> outside of that? >>> >>>>> >>> >>>> The DSI0/1 PLL are not part of the display clock controller, but >>> in the >>> >>>> display subsystem which are managed by the DRM drivers. When DRM >>> drivers >>> >>>> query for the rate clock driver should always return the non >>> cached rates. >>> >>> >>> >>> Why? Is the DSI PLL changing rate all the time, randomly, without >>> going >>> >>> through the clk APIs to do so? >>> >>> >>> >> >>> >> Hmm, I am afraid I do not have an answer for this, but this was the >>> >> requirement to always return the non cached rates from the clock >>> driver. >>> >> >>> > >>> > Ok. Who knows about this requirement? Can we add someone from the >>> > display driver to understand more? >>> > >>> As per my discussions offline with the display teams, >>> >>> There is a use-case where the clock framework is unaware of the PLL VCO >>> frequency change and thus the drivers would query to get the actual HW >>> frequency rather than the cached one. >>> >>> Do you think keeping these flags would have any impact other than always >>> getting the non-cached rates? >>> >> >> The flag will make it so clk_get_rate() works in spite of something >> changing the frequency behind the framework's back, but I want to >> understand what and why it's changing without framework involvement. We >> shouldn't need the flag here, because this flag is typically for clks >> that are controlled by some other entity that the kernel doesn't have >> control over. In this case, it seems like we have full control of the >> clk tree for the display PLL down to this clk, so it should be perfectly >> fine to not have this flag. The presence of the flag means that the >> display driver is doing something wrong. > > These clocks are sourced from DSI PLL. In dsi command mode there is an > idle use case when there > is no activity on display, we switch of the source DSI PLL to save power > by calling clk_disable_unprepare(). > In this scenario if some client queries the clk_get_rate(), then if > NO_CACHE flag is used the clk > driver will return the cached rate which is some non-zero value set when > we last called clk_set_rate(), > before enabling the clock, whereas actually in HW this clk is disabled. > So we used the NO_CACHE flag > for the call to land in clk_recalc_rate and we return zero if the PLL is > disabled. > I remember some customer had scripts that runs through all the clocks > during idle screen scenario > and call clk_get_rate(), where they complained display clk_get_rate > returns none zero value. > >
Do you think the clock driver needs any update for flag for the next series?
> -- > To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
-- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation.
--
| |