On Wed, Apr 12, 2017 at 12:13 AM, Eric Anholt <eric@xxxxxxxxxx> wrote: > Oh, one last thing I think we need to figure out: I'm using TIM2_CLKSEL, > which seems to be necessary on this platform. My understanding is that > this means that the pixel clock is divided from clcdclk instead of > apb_pclk. Do you agree? Yes the pixed clock is always derived from clcdclk. In most older ARM reference designs this is a VCO so that is why there is a clk_set_rate() on this in the fbdev code. (On some platforms that even has no effect I guess.) > The fbdev driver is using > clk_get(&fb->dev->dev, NULL) and not TIM2_CLKSEL, which I'm surprised by > because I would have thought that would give us the first clock from the > DT node (also clcdclk). So that thing is a 1-bit line that can select one of two clocks to be muxed into the PL111/CLCD. I guess that up until now all platforms just left that line dangling in the silicon. Congratulations, you came here first ;) Though when I look at the Nomadik it seems that it might be muxing the clock between 48 and 72 MHz, and I've been using 48MHz all along ooopsie. The current assumption in the bindings is that we have only one clock and TIM2_CLKSEL is N/A. If we want proper clcdclk handling with CLKSEL you should probably add some code to implement a real mux clock for this using <linux/clock-provider.h> and drivers/clk/clk-mux.c with select COMMON_CLK so that the driver still only sees clcdclk but that in turn is a mux that can select one of two sources and will react to the clk_set_rate() call by selecting the clock which is closest in frequency to what you want. This needs a small patch to alter the bindings too I guess. A small clock node inside the CLCD, just like PCI bridges have irqchips inside them etc: clcd@10120000 { compatible = "arm,pl110", "arm,primecell"; reg = <0x10120000 0x1000>; (...) clocks = <&clcdclk>, <&foo>; clock-names = "clcdclk", "apb_pclk"; clcdclk: clock-controller@0 { compatible = "arm,pl11x-clock-mux"; clocks = <&source_a>, <&source_b>; }; }; This can be set up easily in the OF probe path since that is what we're doing: just look for this subnode, if it is there create the clock controller. I do not think the clk maintainers would mind a small mux clock controller inside the CLCD driver to handle this mux if we need it. It would *maybe* also be possible to add a second "clcdclk2" to the block and make an educated decision on which clock to use in the driver but that is not as elegant as using the clock framework mux clock I think. Yours, Linus Walleij _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel