Re: [PATCH 3/4] drm: Add helper for simple display pipeline

[Daniel Vetter]

On Tue, May 10, 2016 at 8:59 AM, Daniel Vetter <daniel@xxxxxxxx> wrote:
>>     if (ret)
>>         return ret;
>>
>>     /* How to handle !visible, is it even possible? */
>
>         if (!visible)
>                 return -EINVAL;
>
> You can't, so need to reject it.

Ok, on further thought I think we need a bit more here. We not just
need to make sure the plane is visible and not scaled or positioned,
but also that the plane is only enabled iff the crtc is.

So even before you call anything else you need to have this check:

if (crtc_state->enable != !!plane_state->crtc)
        return -EINVAL; /* plane must match crtc enable state */

if (!crtc_state->enable)
        return 0; /* nothing to check when disabling or disabled
already, calling check helpers and driver callbacks might only result
in confusion in such a case. */

Then continue with the remaining check logic that you have already,
with my coments in the earlier mail addressed.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch