Quoting spanda@xxxxxxxxxxxxxx (2018-07-13 01:25:49) > 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. If the clk is disabled in hardware it doesn't mean clk_get_rate() should return 0 Hz. The frequency of the clk is set to something, so we should return what that frequency is by reading the hardware. > 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. This is super wrong. If the PLL is disabled it still has some frequency configured. -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
