dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel.vetter@ffwll.ch>
To: James Jones <jajones@nvidia.com>
Cc: "Kirill A. Shutemov" <kirill@shutemov.name>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	LKML <linux-kernel@vger.kernel.org>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	Ben Skeggs <bskeggs@redhat.com>
Subject: Re: [git pull] drm for 5.8-rc1
Date: Thu, 2 Jul 2020 09:36:29 +0200	[thread overview]
Message-ID: <CAKMK7uGJW7S6msS6MzxMAzNoUvfcZbajhc8rhD39d4TUk4Vx8w@mail.gmail.com> (raw)
In-Reply-To: <77e744b9-b5e2-9e9b-44c1-98584d2ae2f3@nvidia.com>

On Wed, Jul 1, 2020 at 9:45 PM James Jones <jajones@nvidia.com> wrote:
>
> OK, I think I see what's going on.  In the Xorg modesetting driver, the
> logic is basically:
>
> if (gbm_has_modifiers && DRM_CAP_ADDFB2_MODIFIERS != 0) {
>    drmModeAddFB2WithModifiers(..., gbm_bo_get_modifier(bo->gbm));
> } else {
>    drmModeAddFB(...);
> }
>
> There's no attempt to verify the DRM-KMS device supports the modifier,
> but then, why would there be?  GBM presumably chose a supported modifier
> at buffer creation time, and we don't know which plane the FB is going
> to be used with yet.  GBM doesn't actually ask the kernel which
> modifiers it supports here either though.  It just goes into Mesa via
> DRI and reports the modifier (unpatched) Mesa chose on its own.  Mesa
> just hard-codes the modifiers in its driver backends since its thinking
> in terms of a device's 3D engine, not display.  In theory, Mesa's DRI
> drivers could query KMS for supported modifiers if allocating from GBM
> using the non-modifiers path and the SCANOUT flag is set (perhaps some
> drivers do this or its equivalent?  Haven't checked.), but that seems
> pretty gnarly and doesn't fix the modifier-based GBM allocation path
> AFAIK.  Bit of a mess.
>
> For a quick userspace fix that could probably be pushed out everywhere
> (Only affects Xorg server 1.20+ AFAIK), just retrying
> drmModeAddFB2WithModifiers() without the DRM_MODE_FB_MODIFIERS flag on
> failure should be sufficient.  Still need to verify as I'm having
> trouble wrangling my Xorg build at the moment and I'm pressed for time.
> A more complete fix would be quite involved, as modesetting isn't really
> properly plumbed to validate GBM's modifiers against KMS planes, and it
> doesn't seem like GBM/Mesa/DRI should be responsible for this as noted
> above given the general modifier workflow/design.
>
> Most importantly, options I've considered for fixing from the kernel side:
>
> -Accept "legacy" modifiers in nouveau in addition to the new modifiers,
> though avoid reporting them to userspace as supported to avoid further
> proliferation.  This is pretty straightforward.  I'll need to modify
> both the AddFB2 handler (nouveau_validate_decode_mod) and the mode set
> plane validation logic (nv50_plane_format_mod_supported), but it should
> end up just being a few lines of code.
>
> -Don't validate modifiers in AddFB.  This doesn't really gain anything
> because it just pushes the failure down to mode set time, so it's not
> that useful, so I don't plan on pursuing this.
>
> As noted, need to run just now, but I should have a kernel patch to test
> out either tonight or tomorrow.
>
> If anyone's curious, the reason my testing missed this was I did most of
> my verification of "old" code against the Xorg 1.19 build included with
> my distro.  I did hack up a Xorg 1.20-ish build to test as well that
> would have included this path, but I must not have properly configured
> it with GBM modifier support somehow.  I was pretty focused on just
> testing the forcibly-disabled atomic path in the modesetting driver in
> this build, so I didn't look too closely at things beyond that.

Yeah, so modifier support in -modesetting is totally broken. It should
be disabled by default because the stuff just doesn't really work.

If that doesn't help then I guess we need to do the same thing as what
we've done for atomic already in:

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

For added insult for the atomic stuff: Xorg master branch is actually
fixed, but no one has done a release of that in over 2 years, so the
fixes never got anywhere. I have little hope the Xorg will get back
into shape, seems abandoned unfortunately.
-Daniel

>
> Thanks,
> -James
>
> On 7/1/20 12:59 AM, Kirill A. Shutemov wrote:
> > On Wed, Jul 01, 2020 at 10:57:19AM +0300, Kirill A. Shutemov wrote:
> >> On Tue, Jun 30, 2020 at 09:40:19PM -0700, James Jones wrote:
> >>> This implies something is trying to use one of the old
> >>> DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK format modifiers with DRM-KMS without
> >>> first checking whether it is supported by the kernel.  I had tried to force
> >>> an Xorg+Mesa stack without my userspace patches to hit this error when
> >>> testing, but must have missed some permutation.  If the stalled Mesa patches
> >>> go in, this would stop happening of course, but those were held up for a
> >>> long time in review, and are now waiting on me to make some modifications.
> >>>
> >>> Are you using the modesetting driver in X? If so, with glamor I presume?
> >>
> >> Yes and yes. I attached Xorg.log.
> >
> > Attached now.
> >
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2020-07-02  7:36 UTC|newest]

Thread overview: 74+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-02  6:06 [git pull] drm for 5.8-rc1 Dave Airlie
2020-06-02 21:21 ` Linus Torvalds
2020-06-02 21:22   ` Linus Torvalds
2020-06-02 21:56   ` Linus Torvalds
2020-06-03  7:18     ` Thomas Zimmermann
2020-06-03  7:43       ` Daniel Vetter
2020-06-02 22:14 ` Linus Torvalds
2020-06-02 23:03   ` Dave Airlie
2020-06-02 22:20 ` pr-tracker-bot
2020-06-03 20:13 ` Jason Gunthorpe
2020-06-04  8:10   ` Christian König
2020-06-30 23:08 ` Kirill A. Shutemov
2020-07-01  4:40   ` James Jones
2020-07-01  7:57     ` Kirill A. Shutemov
2020-07-01  7:59       ` Kirill A. Shutemov
2020-07-01 19:45         ` James Jones
2020-07-02  7:36           ` Daniel Vetter [this message]
2020-07-02  7:59           ` Pekka Paalanen
2020-07-02  8:22           ` Daniel Stone
2020-07-02 21:14             ` James Jones
2020-07-03  6:01               ` James Jones
2020-07-03  7:16                 ` Daniel Vetter
2020-07-13  1:37                   ` Dave Airlie
2020-07-14 14:31                     ` James Jones
2020-08-04  8:58                       ` Karol Herbst
2020-08-12  0:19                         ` James Jones
2020-08-12 10:27                           ` Karol Herbst
2020-08-12 10:43                             ` Karol Herbst
2020-08-12 12:24                               ` Karol Herbst
2020-08-12 12:37                                 ` Ilia Mirkin
2020-08-12 17:03                                   ` James Jones
2020-08-12 17:10                                     ` Karol Herbst
2020-08-12 17:19                                       ` James Jones
2020-08-12 17:40                                         ` Alyssa Rosenzweig
2020-08-12 18:24                                           ` James Jones
2020-08-12 18:51                                             ` Karol Herbst
2020-08-13 13:00                                               ` Karol Herbst
2020-08-13 15:39                                                 ` Karol Herbst
2020-08-13 17:19                                                   ` Karol Herbst
2020-08-13 17:45                                                     ` James Jones
2020-08-13 17:48                                                       ` Karol Herbst
2020-08-14 13:57                                                         ` Thierry Reding
2020-08-14 13:59                                                           ` Karol Herbst
2020-08-14 14:10                                                             ` Thierry Reding
2020-08-14 14:05                                                       ` Thierry Reding
2020-08-14 14:44                                                         ` Karol Herbst
2020-08-14 15:34                                                           ` Thierry Reding
2020-08-14 15:40                                                             ` Karol Herbst
2020-08-14 16:06                                                               ` Thierry Reding
2020-08-14 16:12                                                                 ` Karol Herbst
2020-08-14 16:22                                                                   ` Thierry Reding
2020-08-14 17:17                                                                     ` Daniel Stone
2020-08-14 17:25                                                                       ` Daniel Vetter
2020-08-18 14:37                                                                         ` Thierry Reding
2020-09-01  7:13                                                                           ` Daniel Vetter
2020-09-01 10:42                                                                             ` Daniel Stone
2020-09-01 10:59                                                                             ` Karol Herbst
2020-09-01 14:42                                                                               ` James Jones
2020-08-14 14:08                                                   ` Thierry Reding
2020-08-14 14:45                                                     ` Karol Herbst
2020-08-14 15:24                                                       ` Thierry Reding
2020-08-14 15:43                                                         ` Karol Herbst
2020-08-14 13:54                                                 ` Thierry Reding
2020-08-14 13:40                                     ` Thierry Reding
2020-08-14 13:56                                       ` Karol Herbst
2020-08-12 15:05                               ` Thierry Reding
2020-08-12 15:20                                 ` Karol Herbst
2020-08-12 15:49                                   ` Karol Herbst
2020-07-01 11:24     ` Karol Herbst
2020-07-01 15:51       ` James Jones
2020-07-01 16:01         ` Daniel Vetter
2020-07-01 17:04           ` Karol Herbst
2020-07-01 17:37             ` James Jones
2020-07-01 18:08               ` Karol Herbst

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAKMK7uGJW7S6msS6MzxMAzNoUvfcZbajhc8rhD39d4TUk4Vx8w@mail.gmail.com \
    --to=daniel.vetter@ffwll.ch \
    --cc=bskeggs@redhat.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=jajones@nvidia.com \
    --cc=kirill@shutemov.name \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).