On Mon, May 26, 2014 at 07:35:45AM -0400, Rob Clark wrote:
On Mon, May 26, 2014 at 5:31 AM, Daniel Vetter daniel@ffwll.ch wrote:
+struct drm_crtc_state * +drm_atomic_get_crtc_state(struct drm_crtc *crtc, struct drm_atomic_state *a) +{
struct drm_crtc_state *cstate;int ret;cstate = a->cstates[crtc->id];if (!cstate) {ret = drm_modeset_lock(&crtc->mutex, &a->acquire_ctx);if (ret)return ERR_PTR(ret);cstate = drm_crtc_create_state(crtc);if (!cstate)return ERR_PTR(-ENOMEM);init_crtc_state(crtc, cstate, a);a->crtcs[crtc->id] = crtc;a->cstates[crtc->id] = cstate;/* we'll need it later, so make sure we have state* for primary plane too:*/drm_atomic_get_plane_state(crtc->primary, a);I haven't figured out why. With primary planes I don't really see a need for this. If we need it to implement the legacy setcrtc interface, then that should be done there, not here.
well, if you sort out how to disable primary helper plane, then yes, you are right :-)
see commit_crtc_state()
Imo bail when we have a crtc with NULL primary plane in crtc_commit (and wont disable the entire crtc ofc).
Also I think your current commit_crtc should be pushed down into the crtc helpers - it's not going to do any good for i915. But I guess until we have a real user of the atomic interface (i.e. updates more than 1 crtc) like the fb helper code we don't need this yet.
But as soon as we update more than one crtc we _must_ push it down into the crtc helpers or the i915 machinery - if you try to enable crtcs which need resources from crtcs which aren't yet off this is simply going to fail. -Daniel