[PATCH] drm: don't override possible_crtcs for primary/cursor planes

[Sean Paul]

On Thu, Oct 20, 2016 at 3:06 PM, Rob Clark <robdclark at gmail.com> wrote:
> It is kind of a pointless restriction.  If userspace does silly things
> like using crtcA's cursor plane on crtcB, and then setcursor on crtcA,
> it will end up with the overlay disabled on crtcB.  But userspace is
> allowed to shoot itself like this.
>
> Signed-off-by: Rob Clark <robdclark at gmail.com>
> ---
> Note that cursor and primary planes are slightly useless to non-atomic
> userspace, since userspace has no way to know *which* crtc a primary
> or cursor plane belongs to.  So lighting up a 2nd display may or may
> not make your overlay planes go *poof*.
>
> So basically use of cursor/primary planes plus legacy ioctls is an
> undefined behavior if we go with this patch.
>
> *Maybe* we want to restrict this to atomic userspace.  (Ie. advertise
> the more limited possible_crtcs for legacy userspace.)  Depends on
> whether we can (slightly retroactively) declare that mixing atomic
> and legacy APIs is undefined behavior.  Or if really needed, add
> DRM_CLIENT_CAP_REALLY_REALLY_ATOMIC_NO_LEGACY_PLS.  (Or just pass in
> value 2 for existing DRM_CLIENT_CAP_ATOMIC)

Being biased and selfish, I'd like to avoid adding another behavior
bit. So I'm fine with this path.

Sean

>
>  drivers/gpu/drm/drm_crtc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index c215802..1e6d37f 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -707,9 +707,9 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc,
>         crtc->primary = primary;
>         crtc->cursor = cursor;
>         if (primary)
> -               primary->possible_crtcs = 1 << drm_crtc_index(crtc);
> +               WARN_ON(!(primary->possible_crtcs & (1 << drm_crtc_index(crtc))));
>         if (cursor)
> -               cursor->possible_crtcs = 1 << drm_crtc_index(crtc);
> +               WARN_ON(!(cursor->possible_crtcs & (1 << drm_crtc_index(crtc))));
>
>         if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
>                 drm_object_attach_property(&crtc->base, config->prop_active, 0);
> --
> 2.7.4
>