[PATCH 1/3] drm/atomic: Take the atomic toys away from X

[PATCH 1/3] drm/atomic: Take the atomic toys away from X

[Daniel Vetter]

The -modesetting ddx has a totally broken idea of how atomic works:
- doesn't disable old connectors, assuming they get auto-disable like
  with the legacy setcrtc
- assumes ASYNC_FLIP is wired through for the atomic ioctl
- not a single call to TEST_ONLY

Iow the implementation is a 1:1 translation of legacy ioctls to
atomic, which is a) broken b) pointless.

We already have bugs in both i915 and amdgpu-DC where this prevents us
from enabling neat features.

If anyone ever cares about atomic in X we can easily add a new atomic
level (req->value == 2) for X to get back the shiny toys.

Since these broken versions of -modesetting have been shipping,
there's really no other way to get out of this bind.

References: https://gitlab.freedesktop.org/xorg/xserver/issues/629
References: https://gitlab.freedesktop.org/xorg/xserver/merge_requests/180
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Michel Dänzer <michel@daenzer.net>
Cc: Alex Deucher <alexdeucher@gmail.com>
Cc: Adam Jackson <ajax@redhat.com>
Cc: Sean Paul <sean@poorly.run>
Cc: David Airlie <airlied@linux.ie>
Cc: stable@vger.kernel.org
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_ioctl.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 2c120c58f72d..1cb7b4c3c87c 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -334,6 +334,9 @@ drm_setclientcap(struct drm_device *dev, void *data, struct drm_file *file_priv)
 		file_priv->universal_planes = req->value;
 		break;
 	case DRM_CLIENT_CAP_ATOMIC:
+		/* The modesetting DDX has a totally broken idea of atomic. */
+		if (strstr(current->comm, "X"))
+			return -EOPNOTSUPP;
 		if (!drm_core_check_feature(dev, DRIVER_ATOMIC))
 			return -EOPNOTSUPP;
 		if (req->value > 1)
-- 
2.23.0

Re: [PATCH 1/3] drm/atomic: Take the atomic toys away from X

[Maarten Lankhorst]

Op 03-09-2019 om 21:06 schreef Daniel Vetter:
> The -modesetting ddx has a totally broken idea of how atomic works:
> - doesn't disable old connectors, assuming they get auto-disable like
>   with the legacy setcrtc
> - assumes ASYNC_FLIP is wired through for the atomic ioctl
> - not a single call to TEST_ONLY
>
> Iow the implementation is a 1:1 translation of legacy ioctls to
> atomic, which is a) broken b) pointless.
>
> We already have bugs in both i915 and amdgpu-DC where this prevents us
> from enabling neat features.
>
> If anyone ever cares about atomic in X we can easily add a new atomic
> level (req->value == 2) for X to get back the shiny toys.
>
> Since these broken versions of -modesetting have been shipping,
> there's really no other way to get out of this bind.
>
> References: https://gitlab.freedesktop.org/xorg/xserver/issues/629
> References: https://gitlab.freedesktop.org/xorg/xserver/merge_requests/180
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Michel Dänzer <michel@daenzer.net>
> Cc: Alex Deucher <alexdeucher@gmail.com>
> Cc: Adam Jackson <ajax@redhat.com>
> Cc: Sean Paul <sean@poorly.run>
> Cc: David Airlie <airlied@linux.ie>
> Cc: stable@vger.kernel.org
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/drm_ioctl.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index 2c120c58f72d..1cb7b4c3c87c 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -334,6 +334,9 @@ drm_setclientcap(struct drm_device *dev, void *data, struct drm_file *file_priv)
>  		file_priv->universal_planes = req->value;
>  		break;
>  	case DRM_CLIENT_CAP_ATOMIC:
> +		/* The modesetting DDX has a totally broken idea of atomic. */
> +		if (strstr(current->comm, "X"))
> +			return -EOPNOTSUPP;
>  		if (!drm_core_check_feature(dev, DRIVER_ATOMIC))
>  			return -EOPNOTSUPP;
>  		if (req->value > 1)

Good riddance!

Missing one more:
commit abbc0697d5fbf53f74ce0bcbe936670199764cfa
Author: Dave Airlie <airlied@redhat.com>
Date:   Wed Apr 24 16:33:29 2019 +1000

    drm/fb: revert the i915 Actually configure untiled displays from master
   
    This code moved in here in master, so revert it the same way.
   
    This is the same revert as 9fa246256e09 ("Revert "drm/i915/fbdev:
    Actually configure untiled displays"") in drm-fixes.
   
    Signed-off-by: Dave Airlie <airlied@redhat.com>

Can we unrevert that now?

With that fixed, on the whole series:

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Re: [PATCH 1/3] drm/atomic: Take the atomic toys away from X

[Daniel Vetter]

On Thu, Sep 5, 2019 at 4:19 PM Maarten Lankhorst
<maarten.lankhorst@linux.intel.com> wrote:
>
> Op 03-09-2019 om 21:06 schreef Daniel Vetter:
> > The -modesetting ddx has a totally broken idea of how atomic works:
> > - doesn't disable old connectors, assuming they get auto-disable like
> >   with the legacy setcrtc
> > - assumes ASYNC_FLIP is wired through for the atomic ioctl
> > - not a single call to TEST_ONLY
> >
> > Iow the implementation is a 1:1 translation of legacy ioctls to
> > atomic, which is a) broken b) pointless.
> >
> > We already have bugs in both i915 and amdgpu-DC where this prevents us
> > from enabling neat features.
> >
> > If anyone ever cares about atomic in X we can easily add a new atomic
> > level (req->value == 2) for X to get back the shiny toys.
> >
> > Since these broken versions of -modesetting have been shipping,
> > there's really no other way to get out of this bind.
> >
> > References: https://gitlab.freedesktop.org/xorg/xserver/issues/629
> > References: https://gitlab.freedesktop.org/xorg/xserver/merge_requests/180
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Michel Dänzer <michel@daenzer.net>
> > Cc: Alex Deucher <alexdeucher@gmail.com>
> > Cc: Adam Jackson <ajax@redhat.com>
> > Cc: Sean Paul <sean@poorly.run>
> > Cc: David Airlie <airlied@linux.ie>
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> >  drivers/gpu/drm/drm_ioctl.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> > index 2c120c58f72d..1cb7b4c3c87c 100644
> > --- a/drivers/gpu/drm/drm_ioctl.c
> > +++ b/drivers/gpu/drm/drm_ioctl.c
> > @@ -334,6 +334,9 @@ drm_setclientcap(struct drm_device *dev, void *data, struct drm_file *file_priv)
> >               file_priv->universal_planes = req->value;
> >               break;
> >       case DRM_CLIENT_CAP_ATOMIC:
> > +             /* The modesetting DDX has a totally broken idea of atomic. */
> > +             if (strstr(current->comm, "X"))
> > +                     return -EOPNOTSUPP;
> >               if (!drm_core_check_feature(dev, DRIVER_ATOMIC))
> >                       return -EOPNOTSUPP;
> >               if (req->value > 1)
>
> Good riddance!
>
> Missing one more:
> commit abbc0697d5fbf53f74ce0bcbe936670199764cfa
> Author: Dave Airlie <airlied@redhat.com>
> Date:   Wed Apr 24 16:33:29 2019 +1000
>
>     drm/fb: revert the i915 Actually configure untiled displays from master
>
>     This code moved in here in master, so revert it the same way.
>
>     This is the same revert as 9fa246256e09 ("Revert "drm/i915/fbdev:
>     Actually configure untiled displays"") in drm-fixes.
>
>     Signed-off-by: Dave Airlie <airlied@redhat.com>
>
> Can we unrevert that now?

My idea is to land this in drm-misc-fixes first (or maybe
drm-misc-next-fixes). And then we can land the revert of the revert
once that's backmerged into drm-intel. -fixes since this one here is
cc: stable.

And yes I'll add a reference to that one in the commit message when merging.

> With that fixed, on the whole series:
>
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [Intel-gfx] [PATCH 1/3] drm/atomic: Take the atomic toys away from X

[Rob Clark]

On Tue, Sep 3, 2019 at 12:07 PM Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>
> The -modesetting ddx has a totally broken idea of how atomic works:
> - doesn't disable old connectors, assuming they get auto-disable like
>   with the legacy setcrtc
> - assumes ASYNC_FLIP is wired through for the atomic ioctl
> - not a single call to TEST_ONLY
>
> Iow the implementation is a 1:1 translation of legacy ioctls to
> atomic, which is a) broken b) pointless.
>
> We already have bugs in both i915 and amdgpu-DC where this prevents us
> from enabling neat features.
>
> If anyone ever cares about atomic in X we can easily add a new atomic
> level (req->value == 2) for X to get back the shiny toys.
>
> Since these broken versions of -modesetting have been shipping,
> there's really no other way to get out of this bind.
>
> References: https://gitlab.freedesktop.org/xorg/xserver/issues/629
> References: https://gitlab.freedesktop.org/xorg/xserver/merge_requests/180
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Michel Dänzer <michel@daenzer.net>
> Cc: Alex Deucher <alexdeucher@gmail.com>
> Cc: Adam Jackson <ajax@redhat.com>
> Cc: Sean Paul <sean@poorly.run>
> Cc: David Airlie <airlied@linux.ie>
> Cc: stable@vger.kernel.org
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/drm_ioctl.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index 2c120c58f72d..1cb7b4c3c87c 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -334,6 +334,9 @@ drm_setclientcap(struct drm_device *dev, void *data, struct drm_file *file_priv)
>                 file_priv->universal_planes = req->value;
>                 break;
>         case DRM_CLIENT_CAP_ATOMIC:
> +               /* The modesetting DDX has a totally broken idea of atomic. */
> +               if (strstr(current->comm, "X"))
> +                       return -EOPNOTSUPP;

Seems like we can be a bit more targeted than "anything that has 'X'
in the name".. at a minimum restrict things to "starts with 'X'" seems
saner.  But I guess we could probably somehow look at the processes
memory map and look for modesetting_drv.so.

BR,
-R

>                 if (!drm_core_check_feature(dev, DRIVER_ATOMIC))
>                         return -EOPNOTSUPP;
>                 if (req->value > 1)
> --
> 2.23.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/atomic: Take the atomic toys away from X

[Daniel Vetter]

The -modesetting ddx has a totally broken idea of how atomic works:
- doesn't disable old connectors, assuming they get auto-disable like
  with the legacy setcrtc
- assumes ASYNC_FLIP is wired through for the atomic ioctl
- not a single call to TEST_ONLY

Iow the implementation is a 1:1 translation of legacy ioctls to
atomic, which is a) broken b) pointless.

We already have bugs in both i915 and amdgpu-DC where this prevents us
from enabling neat features.

If anyone ever cares about atomic in X we can easily add a new atomic
level (req->value == 2) for X to get back the shiny toys.

Since these broken versions of -modesetting have been shipping,
there's really no other way to get out of this bind.

v2:
- add an informational dmesg output (Rob, Ajax)
- reorder after the DRIVER_ATOMIC check to avoid useless noise (Ilia)
- allow req->value > 2 so that X can do another attempt at atomic in
  the future

Cc: Ilia Mirkin <imirkin@alum.mit.edu>
References: https://gitlab.freedesktop.org/xorg/xserver/issues/629
References: https://gitlab.freedesktop.org/xorg/xserver/merge_requests/180
References: abbc0697d5fb ("drm/fb: revert the i915 Actually configure untiled displays from master")
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> (v1)
Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com> (v1)
Cc: Michel Dänzer <michel@daenzer.net>
Cc: Alex Deucher <alexdeucher@gmail.com>
Cc: Adam Jackson <ajax@redhat.com>
Acked-by: Adam Jackson <ajax@redhat.com>
Cc: Sean Paul <sean@poorly.run>
Cc: David Airlie <airlied@linux.ie>
Cc: Rob Clark <robdclark@gmail.com>
Acked-by: Rob Clark <robdclark@gmail.com>
Cc: stable@vger.kernel.org
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_ioctl.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 2c120c58f72d..56aa8bbb3a8c 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -336,7 +336,12 @@ drm_setclientcap(struct drm_device *dev, void *data, struct drm_file *file_priv)
 	case DRM_CLIENT_CAP_ATOMIC:
 		if (!drm_core_check_feature(dev, DRIVER_ATOMIC))
 			return -EOPNOTSUPP;
-		if (req->value > 1)
+		/* The modesetting DDX has a totally broken idea of atomic. */
+		if (strstr(current->comm, "X") && req->value == 1) {
+			pr_info("broken atomic modeset userspace detected, disabling atomic\n");
+			return -EOPNOTSUPP;
+		}
+		if (req->value > 2)
 			return -EINVAL;
 		file_priv->atomic = req->value;
 		file_priv->universal_planes = req->value;
-- 
2.23.0

[PATCH] drm/atomic: Take the atomic toys away from X

[Daniel Vetter]

The -modesetting ddx has a totally broken idea of how atomic works:
- doesn't disable old connectors, assuming they get auto-disable like
  with the legacy setcrtc
- assumes ASYNC_FLIP is wired through for the atomic ioctl
- not a single call to TEST_ONLY

Iow the implementation is a 1:1 translation of legacy ioctls to
atomic, which is a) broken b) pointless.

We already have bugs in both i915 and amdgpu-DC where this prevents us
from enabling neat features.

If anyone ever cares about atomic in X we can easily add a new atomic
level (req->value == 2) for X to get back the shiny toys.

Since these broken versions of -modesetting have been shipping,
there's really no other way to get out of this bind.

v2:
- add an informational dmesg output (Rob, Ajax)
- reorder after the DRIVER_ATOMIC check to avoid useless noise (Ilia)
- allow req->value > 2 so that X can do another attempt at atomic in
  the future

v3: Go with paranoid, insist that the X should be first (suggested by
Rob)

Cc: Ilia Mirkin <imirkin@alum.mit.edu>
References: https://gitlab.freedesktop.org/xorg/xserver/issues/629
References: https://gitlab.freedesktop.org/xorg/xserver/merge_requests/180
References: abbc0697d5fb ("drm/fb: revert the i915 Actually configure untiled displays from master")
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> (v1)
Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com> (v1)
Cc: Michel Dänzer <michel@daenzer.net>
Cc: Alex Deucher <alexdeucher@gmail.com>
Cc: Adam Jackson <ajax@redhat.com>
Acked-by: Adam Jackson <ajax@redhat.com>
Cc: Sean Paul <sean@poorly.run>
Cc: David Airlie <airlied@linux.ie>
Cc: Rob Clark <robdclark@gmail.com>
Acked-by: Rob Clark <robdclark@gmail.com>
Cc: stable@vger.kernel.org
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_ioctl.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 2c120c58f72d..1cd5cc492df1 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -336,7 +336,12 @@ drm_setclientcap(struct drm_device *dev, void *data, struct drm_file *file_priv)
 	case DRM_CLIENT_CAP_ATOMIC:
 		if (!drm_core_check_feature(dev, DRIVER_ATOMIC))
 			return -EOPNOTSUPP;
-		if (req->value > 1)
+		/* The modesetting DDX has a totally broken idea of atomic. */
+		if (current->comm[0] == 'X' && req->value == 1) {
+			pr_info("broken atomic modeset userspace detected, disabling atomic\n");
+			return -EOPNOTSUPP;
+		}
+		if (req->value > 2)
 			return -EINVAL;
 		file_priv->atomic = req->value;
 		file_priv->universal_planes = req->value;
-- 
2.23.0

Re: [PATCH] drm/atomic: Take the atomic toys away from X

[Yves-Alexis Perez]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On Thu, 2019-09-05 at 20:53 +0200, Daniel Vetter wrote:
> The -modesetting ddx has a totally broken idea of how atomic works:
> - doesn't disable old connectors, assuming they get auto-disable like
>   with the legacy setcrtc
> - assumes ASYNC_FLIP is wired through for the atomic ioctl
> - not a single call to TEST_ONLY
> 
> Iow the implementation is a 1:1 translation of legacy ioctls to
> atomic, which is a) broken b) pointless.
> 
> We already have bugs in both i915 and amdgpu-DC where this prevents us
> from enabling neat features.
> 
> If anyone ever cares about atomic in X we can easily add a new atomic
> level (req->value == 2) for X to get back the shiny toys.
> 
> Since these broken versions of -modesetting have been shipping,
> there's really no other way to get out of this bind.

Hi Daniel and Greg (especially). It seems that this patch was never applied to
stable, maybe it fell through the cracks?

It doesn't apply as-is in 4.19 branch but a small change in the context makes
it apply. I'm experiencing issues with lightdm and vt-switch in Debian Buster
(which has a 4.19 kernel) so I'd appreciate if the patch was included in at
least that release.

Regards,
- -- 
Yves-Alexis
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCAAdFiEE8vi34Qgfo83x35gF3rYcyPpXRFsFAl61ITAACgkQ3rYcyPpX
RFvlaAf9HZ0DTX1fAkNeNFoAgn4pFztnFq0fAwGj5iVIL4q6upE1wE3E8cDgUHeT
maQQvL3YHFXjgzgDHYNIuUMipFE1Djymoy+EB4ZoOftqsJ4CPy4pCMUAh57u7BrV
T+eBtj4n0wY0SgvoPism3QdbxY7CLLgCMJKLNrCPlkDCdJyGsZX9RIgfqvbkGM36
ftwBKcyy1iW5cAv10ehiXi/1zszA8bx2gULim3abcSjjz12ckNvBPy/BDvfFx19V
8cGgG3qD9PLmxRl80H1/mX30Ddw8Md5Fu7I/ndh3EGXLu8p8zod0rQVCQjAEW4X4
ew4tajDD2l9vWzN0sZIlyjq9fNgXBw==
=lPBO
-----END PGP SIGNATURE-----

Re: [PATCH] drm/atomic: Take the atomic toys away from X

[Greg KH]

On Fri, May 08, 2020 at 11:06:56AM +0200, Yves-Alexis Perez wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
> 
> On Thu, 2019-09-05 at 20:53 +0200, Daniel Vetter wrote:
> > The -modesetting ddx has a totally broken idea of how atomic works:
> > - doesn't disable old connectors, assuming they get auto-disable like
> >   with the legacy setcrtc
> > - assumes ASYNC_FLIP is wired through for the atomic ioctl
> > - not a single call to TEST_ONLY
> > 
> > Iow the implementation is a 1:1 translation of legacy ioctls to
> > atomic, which is a) broken b) pointless.
> > 
> > We already have bugs in both i915 and amdgpu-DC where this prevents us
> > from enabling neat features.
> > 
> > If anyone ever cares about atomic in X we can easily add a new atomic
> > level (req->value == 2) for X to get back the shiny toys.
> > 
> > Since these broken versions of -modesetting have been shipping,
> > there's really no other way to get out of this bind.
> 
> Hi Daniel and Greg (especially). It seems that this patch was never applied to
> stable, maybe it fell through the cracks?

What patch is "this patch"?

> It doesn't apply as-is in 4.19 branch but a small change in the context makes
> it apply. I'm experiencing issues with lightdm and vt-switch in Debian Buster
> (which has a 4.19 kernel) so I'd appreciate if the patch was included in at
> least that release.

What is the git commit id of the patch in Linus's tree?  If you have a
working backport, that makes it much easier than hoping I can fix it
up...

thanks,

greg k-h

Re: [PATCH] drm/atomic: Take the atomic toys away from X

[Yves-Alexis Perez]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On Fri, 2020-05-08 at 11:54 +0200, Greg KH wrote:
> > Hi Daniel and Greg (especially). It seems that this patch was never
> > applied to
> > stable, maybe it fell through the cracks?
> 
> What patch is "this patch"?

Sorry, the patch was in the mail I was replying to:

commit 26b1d3b527e7bf3e24b814d617866ac5199ce68d
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Thu Sep 5 20:53:18 2019 +0200

    drm/atomic: Take the atomic toys away from X

> 
> > It doesn't apply as-is in 4.19 branch but a small change in the context
> > makes
> > it apply. I'm experiencing issues with lightdm and vt-switch in Debian
> > Buster
> > (which has a 4.19 kernel) so I'd appreciate if the patch was included in
> > at
> > least that release.
> 
> What is the git commit id of the patch in Linus's tree?  If you have a
> working backport, that makes it much easier than hoping I can fix it
> up...

The commit id is in Linus tree is 26b1d3b527e7bf3e24b814d617866ac5199ce68d. To
apply properly 69fdf4206a8ba91a277b3d50a3a05b71247635b2 would need to be
cherry-picked as well but it wasn't marked for stable so I didn't bother and
only fixed the context. Here's the backport to 4.19, compile and runtime
tested. It does fix the issue for me (like it did on mainline).

So I guess
Tested-By: Yves-Alexis Perez <corsac@debian.org>

commit 8a99914f7b539542622dc571c82d6cd203bddf64
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Thu Sep 5 20:53:18 2019 +0200

    drm/atomic: Take the atomic toys away from X
    
    The -modesetting ddx has a totally broken idea of how atomic works:
    - doesn't disable old connectors, assuming they get auto-disable like
      with the legacy setcrtc
    - assumes ASYNC_FLIP is wired through for the atomic ioctl
    - not a single call to TEST_ONLY
    
    Iow the implementation is a 1:1 translation of legacy ioctls to
    atomic, which is a) broken b) pointless.
    
    We already have bugs in both i915 and amdgpu-DC where this prevents us
    from enabling neat features.
    
    If anyone ever cares about atomic in X we can easily add a new atomic
    level (req->value == 2) for X to get back the shiny toys.
    
    Since these broken versions of -modesetting have been shipping,
    there's really no other way to get out of this bind.
    
    v2:
    - add an informational dmesg output (Rob, Ajax)
    - reorder after the DRIVER_ATOMIC check to avoid useless noise (Ilia)
    - allow req->value > 2 so that X can do another attempt at atomic in
      the future
    
    v3: Go with paranoid, insist that the X should be first (suggested by
    Rob)
    
    Cc: Ilia Mirkin <imirkin@alum.mit.edu>
    References: https://gitlab.freedesktop.org/xorg/xserver/issues/629
    References: https://gitlab.freedesktop.org/xorg/xserver/merge_requests/180
    References: abbc0697d5fb ("drm/fb: revert the i915 Actually configure
untiled displays from master")
    Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
    Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> (v1)
    Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com> (v1)
    Cc: Michel Dänzer <michel@daenzer.net>
    Cc: Alex Deucher <alexdeucher@gmail.com>
    Cc: Adam Jackson <ajax@redhat.com>
    Acked-by: Adam Jackson <ajax@redhat.com>
    Cc: Sean Paul <sean@poorly.run>
    Cc: David Airlie <airlied@linux.ie>
    Cc: Rob Clark <robdclark@gmail.com>
    Acked-by: Rob Clark <robdclark@gmail.com>
    Cc: stable@vger.kernel.org
    Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
    Link: 
https://patchwork.freedesktop.org/patch/msgid/20190905185318.31363-1-daniel.vetter@ffwll.ch

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index ba129b64b61f..b92682f037b2 100644
- --- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -321,7 +321,12 @@ drm_setclientcap(struct drm_device *dev, void *data,
struct drm_file *file_priv)
 	case DRM_CLIENT_CAP_ATOMIC:
 		if (!drm_core_check_feature(dev, DRIVER_ATOMIC))
 			return -EINVAL;
- -		if (req->value > 1)
+		/* The modesetting DDX has a totally broken idea of atomic. */
+		if (current->comm[0] == 'X' && req->value == 1) {
+			pr_info("broken atomic modeset userspace detected,
disabling atomic\n");
+			return -EOPNOTSUPP;
+		}
+		if (req->value > 2)
 			return -EINVAL;
 		file_priv->atomic = req->value;
 		file_priv->universal_planes = req->value;


- -- 
Yves-Alexis
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCAAdFiEE8vi34Qgfo83x35gF3rYcyPpXRFsFAl61SZUACgkQ3rYcyPpX
RFvPfwgAzMyFqiV592RBKu4tx5Ivqa4EC/1OdR8DojyQlw4AP0bYc+4O67xYbvt5
r4JXuGbSu+jW/29V+2t8ZlLi4S7bAvAo2DEhuBdGVzdmD4gM9EC+69oqVeZWWZTm
VUldLrRO8BoPxv14lX/K/kU/o5Pv8ivRoEKs385JU2p1AxNGJI2UUmIXKGtk7Cu/
Fu/flH627RHjTRk2QYsemqHqSAONaHYuSiYm783hz4wYiPOZQdGvS+ifHwMAhUqh
scaCxv+pBOxaLuZAfUXFjDX+qAcuJCxaP9bT4soweIpYqjzcAdBSmny0+OLOnie/
HcBzKwpgitKR/cVadZgb0US1oHeo2A==
=l8z1
-----END PGP SIGNATURE-----

Re: [PATCH] drm/atomic: Take the atomic toys away from X

[Greg KH]

On Fri, May 08, 2020 at 01:59:17PM +0200, Yves-Alexis Perez wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
> 
> On Fri, 2020-05-08 at 11:54 +0200, Greg KH wrote:
> > > Hi Daniel and Greg (especially). It seems that this patch was never
> > > applied to
> > > stable, maybe it fell through the cracks?
> > 
> > What patch is "this patch"?
> 
> Sorry, the patch was in the mail I was replying to:
> 
> commit 26b1d3b527e7bf3e24b814d617866ac5199ce68d
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date:   Thu Sep 5 20:53:18 2019 +0200
> 
>     drm/atomic: Take the atomic toys away from X
> 
> > 
> > > It doesn't apply as-is in 4.19 branch but a small change in the context
> > > makes
> > > it apply. I'm experiencing issues with lightdm and vt-switch in Debian
> > > Buster
> > > (which has a 4.19 kernel) so I'd appreciate if the patch was included in
> > > at
> > > least that release.
> > 
> > What is the git commit id of the patch in Linus's tree?  If you have a
> > working backport, that makes it much easier than hoping I can fix it
> > up...
> 
> The commit id is in Linus tree is 26b1d3b527e7bf3e24b814d617866ac5199ce68d. To
> apply properly 69fdf4206a8ba91a277b3d50a3a05b71247635b2 would need to be
> cherry-picked as well but it wasn't marked for stable so I didn't bother and
> only fixed the context. Here's the backport to 4.19, compile and runtime
> tested. It does fix the issue for me (like it did on mainline).
> 
> So I guess
> Tested-By: Yves-Alexis Perez <corsac@debian.org>
> 
> commit 8a99914f7b539542622dc571c82d6cd203bddf64
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date:   Thu Sep 5 20:53:18 2019 +0200
> 
>     drm/atomic: Take the atomic toys away from X
>     
>     The -modesetting ddx has a totally broken idea of how atomic works:
>     - doesn't disable old connectors, assuming they get auto-disable like
>       with the legacy setcrtc
>     - assumes ASYNC_FLIP is wired through for the atomic ioctl
>     - not a single call to TEST_ONLY
>     
>     Iow the implementation is a 1:1 translation of legacy ioctls to
>     atomic, which is a) broken b) pointless.
>     
>     We already have bugs in both i915 and amdgpu-DC where this prevents us
>     from enabling neat features.
>     
>     If anyone ever cares about atomic in X we can easily add a new atomic
>     level (req->value == 2) for X to get back the shiny toys.
>     
>     Since these broken versions of -modesetting have been shipping,
>     there's really no other way to get out of this bind.
>     
>     v2:
>     - add an informational dmesg output (Rob, Ajax)
>     - reorder after the DRIVER_ATOMIC check to avoid useless noise (Ilia)
>     - allow req->value > 2 so that X can do another attempt at atomic in
>       the future
>     
>     v3: Go with paranoid, insist that the X should be first (suggested by
>     Rob)
>     
>     Cc: Ilia Mirkin <imirkin@alum.mit.edu>
>     References: https://gitlab.freedesktop.org/xorg/xserver/issues/629
>     References: https://gitlab.freedesktop.org/xorg/xserver/merge_requests/180
>     References: abbc0697d5fb ("drm/fb: revert the i915 Actually configure
> untiled displays from master")
>     Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>     Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> (v1)
>     Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com> (v1)
>     Cc: Michel Dänzer <michel@daenzer.net>
>     Cc: Alex Deucher <alexdeucher@gmail.com>
>     Cc: Adam Jackson <ajax@redhat.com>
>     Acked-by: Adam Jackson <ajax@redhat.com>
>     Cc: Sean Paul <sean@poorly.run>
>     Cc: David Airlie <airlied@linux.ie>
>     Cc: Rob Clark <robdclark@gmail.com>
>     Acked-by: Rob Clark <robdclark@gmail.com>
>     Cc: stable@vger.kernel.org
>     Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>     Link: 
> https://patchwork.freedesktop.org/patch/msgid/20190905185318.31363-1-daniel.vetter@ffwll.ch
> 
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index ba129b64b61f..b92682f037b2 100644
> - --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -321,7 +321,12 @@ drm_setclientcap(struct drm_device *dev, void *data,
> struct drm_file *file_priv)
>  	case DRM_CLIENT_CAP_ATOMIC:
>  		if (!drm_core_check_feature(dev, DRIVER_ATOMIC))
>  			return -EINVAL;
> - -		if (req->value > 1)
> +		/* The modesetting DDX has a totally broken idea of atomic. */
> +		if (current->comm[0] == 'X' && req->value == 1) {
> +			pr_info("broken atomic modeset userspace detected,
> disabling atomic\n");
> +			return -EOPNOTSUPP;
> +		}
> +		if (req->value > 2)
>  			return -EINVAL;
>  		file_priv->atomic = req->value;
>  		file_priv->universal_planes = req->value;
> 

This is line-wrapped and can not be applied :(

Ugh, let me see if I can do this by hand...

greg k-h