On Mon, Sep 30, 2013 at 03:20:44PM -0700, Jesse Barnes wrote: > On Tue, 1 Oct 2013 00:19:55 +0300 > Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote: > > > On Sat, Sep 28, 2013 at 08:05:14AM -0700, Jesse Barnes wrote: > > > On Sat, 28 Sep 2013 11:54:21 +0200 > > > Daniel Vetter <daniel@xxxxxxxx> wrote: > > > > > > > On Fri, Sep 27, 2013 at 04:02:29PM -0700, Jesse Barnes wrote: > > > > > The global integrated clock source bit resides in DPLL B on VLV, but we > > > > > were treating it as a per-pipe resource. It needs to be set whenever > > > > > any PLL is active, so pull setting the bit out of vlv_update_pll and > > > > > into vlv_enable_pll. Also add a vlv_disable_pll to prevent disabling it > > > > > when pipe B shuts down. > > > > > > > > > > I'm guessing on the references here, I expect this to bite any config > > > > > where multiple displays are active or displays are moved from pipe to > > > > > pipe. > > > > > > > > > > v2: re-add bits in vlv_update_pll to keep from confusing the state checker > > > > > v3: use enum pipe checks (Daniel) > > > > > set CRI clock source early (Ville) > > > > > consistently set CRI clock source everywhere (Ville) > > > > > > > > Btw do we care about the additional power consumption of having that clock > > > > running all the time? My long-term plan/idea for these display refclocks > > > > was to enable/disable them in the ->modeset_global_resources callback so > > > > that we only ever enable what we actually need. Haswell has this somewhat > > > > implemented already implicitly through the pc8+ power refcounting. > > > > > > > > Just a "have you thought about this" comment. > > > > > > Yes I have. But I don't think this actually enables a clock, just > > > allows it to flow through to the PLLs and DPIO. Shutting down the > > > display power well will probably take care of it though, so I'm not too > > > worried about it now (plus like Ville said, this may be required to > > > access some display reg anyway). > > > > Yeah I think this doesn't actually enable the clock. IIRC there's some > > DPIO refclock stuff in ccu, so we may be able to turn it off there if > > needed. > > > > But what happened to the idea to set this bit in init_hw or somesuch > > place and just preserving it for pipe B in the PLL funcs? > > It's being set early and preserved in the v3 version (well overwritten, > but that's the same number of lines). Ah so it is. But you're also still setting it in vlv_enable_pll(), which is no longer necessary. > > Daniel and I briefly discussed having a global settings mask for the > PLL checker and global_resources too, but that would be even more lines > than this simple approach. > > Jesse -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
