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

[Greg KH <greg@kroah.com>]

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel