On Mon, Jul 1, 2013 at 3:48 PM, Sebastian Hesselbarth <sebastian.hesselbarth@xxxxxxxxx> wrote: > I guess "extclk0" and "extclk1" should be sufficient for clock names. > Also, they are not dedicated as you can have CRTC0 and CRTC1 use e.g. > extclk0 simultaneously. See below for .is_dedicated in general. Maybe we can find better terminology, or just document it a bit better, but having two CRTCs share the same clock falls within the scheme designed and implemented there. "Dedicated" simply means a clock that is dedicated to the display controller, it is not shared by other devices. >> +struct armada_clk_info { >> + struct clk *clk; >> + >> + /* If this clock is dedicated to us, we can change its rate >> without >> + * worrying about any other users in other parts of the system. */ >> + bool is_dedicated; >> + >> + /* However, we cannot share the same dedicated clock between two >> CRTCs >> + * if each CRTC wants a different rate. Track the number of users. >> */ >> + int usage_count; > > > You can share the same clock between two CRTCs. Just consider CRTC1 > using a mode with half pixel clock as CRTC0. Not that I think this will > ever happen, but it is possible. And my implementation already lets that happen - its just that I didn't document it in enough detail. > I prefer not to try to find the best clock (source) at all. Let the > user pass the clock name by e.g. platform_data (or DT) and just try to > get the requested pixclk or a integer multiple of it. You will never be > able to find the best clock for all use cases. > > Just figure out, if integer division is sufficient to get requested > pixclk and clk_set_rate otherwise. This may be useful for current mode > running at 148.5MHz (1080p50) and new mode at 74.25MHz (1080i50, 720p). I am not opposed to this approach, it is nice and simple, but as Russell points out we do additionally need to distinguish between clocks that are "ours" to play with, vs those that are shared with other devices. It would be a bad idea to try call clk_set_rate() on the AXI bus clock, for example, changing the clock for a whole bunch of other devices. This is what the is_dedicated flag is about. However such a flag could be easily added to the DT/platform data definition that you propose. Daniel _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel