On Thu, Nov 6, 2014 at 6:29 PM, Daniel Vetter <daniel@xxxxxxxx> wrote: >>> Proposal. Add a new boolean ->active to the crtc state, which will track >>> the dpms state of the crtc. Add a helper to the atomic core functions >>> which will compute ->active from the state update according to the >>> proposals for issue 1&2. Require that all callers of >>> ->atomich_check/commit update ->active properly and require >>> implementations to obey it. That means the atomic helpers will switch from >>> looking at ->enable to looking at ->active to decided whether a crtc an >>> all its outputs should be enabled or not. >> >> hmm.. then I'm starting to get a bit fuzzy on the distinction between >> enabled and active.. I guess I should go back and look more closely >> at the differentiation between 'enabled' and 'active' in current i915 >> helpers.. >> >> Seems like we should be able to just repurpose enabled in the new >> helpers, to not automatically be true when a mode is set, but instead >> be true iff: >> 1) >=1 attached connector is ON >> 2) valid mode is set >> 3) >=1 attached plane has an fb >> >> seems like the existing problem w/ enabled is just to do with how >> current helpers use it.. but maybe I'm overlooking something.. > > Well 3) is imo wrong, since black screen with no fbs is imo a valid > state (hopefully only transitional). And the core/helper will ensure > that 1&2 never get out of sync (you won't get an -EINVAL though since > existing userspace does stupid stuff that violates this, the core > setCrtc ioctl cleans that up though). (hmm, I wonder whether that is even possible on all hw..) > > That aside I guess I need to elaborate on what makes dpms special in > i915, and why there's a real difference between crtc->enable == true > && ->active == false and crtc->enable == false in i915. For complex > configs we do resource checking (shared dplls) and that's done in the > modeset. For a pipe which has been disabled just with dpms we then > guarantee that we'll keep these resources reserve and so will always > be able to enable the pipe again. If you disable the pipe completely > (i.e. set crtc->enable to false) we'll release these resources. E.g. > in the i915 share dpll code we have both an active refcount (does the > ppl need to be running) and a reference mask (which crtc is > referencing this pll, even when the crtc is disabled with dpms). ahh, ok, "reserved but not enabled" makes a lot more sense.. that was the distinction that I was missing. That probably deserves to be in headerdoc somewhere.. BR, -R _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel