On Thu, Nov 6, 2014 at 6:29 PM, Daniel Vetter daniel@ffwll.ch 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:
- valid mode is set
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