Re: [PATCH 12/17] drm: convert crtc to properties/state

[Daniel Vetter <daniel@ffwll.ch>]

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
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch