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

[Rob Clark]

On Mon, May 26, 2014 at 10:56 AM, Daniel Vetter <daniel at ffwll.ch> wrote:
> 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 at 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.

it is in helpers (if we rename drm_atomic_funcs ->
drm_atomic_helper_funcs, which is basically what it is).

I suspect what you actually want is to split up atomic_commit() so you
can do the loop over all the planes and crtcs internally.

BR,
-R

> 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