Re: [git pull] drm for 5.8-rc1
On Fri, Jul 3, 2020 at 8:01 AM James Jones wrote:
>
> On 7/2/20 2:14 PM, James Jones wrote:
> > On 7/2/20 1:22 AM, Daniel Stone wrote:
> >> Hi,
> >>
> >> On Wed, 1 Jul 2020 at 20:45, James Jones 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(...);
> >>> }
> >>
> >> I read this thread expecting to explain the correct behaviour we
> >> implement in Weston and how modesetting needs to be fixed, but ...
> >> that seems OK to me? As long as `gbm_has_modifiers` is a proxy for 'we
> >> used gbm_(bo|surface)_create_with_modifiers to allocate the buffer'.
> >
> > Yes, the hazards of reporting findings before verifying. I now see
> > modesetting does query the DRM-KMS modifiers and attempt to allocate
> > with them if it found any. However, I still see a lot of ways things
> > can go wrong, but I'm not going to share my speculation again until I've
> > actually verified it, which is taking a frustratingly long time. The
> > modesetting driver is not my friend right now.
>
> OK, several hours of dumb build+config mistakes later, I was actually
> able to reproduce the failure and walk through things. There is a
> trivial fix for the issues in the X modesetting driver, working off
> Daniel Stone's claim that gbm_bo_get_modifier() should only be called
> when the allocation was made with gbm_bo_create_with_modifiers().
> modeset doesn't respect that requirement now in the case that the atomic
> modesetting path is disabled, which is always the case currently because
> that path is broken. Respecting that requirement is a half-liner and
> allows X to start properly.
>
> If I force modeset to use the atomic path, X still fails to start with
> the above fix, validating the second theory I'd had:
>
> -Current Mesa nouveau code basically ignores the modifier list passed in
> unless it is a single modifier requesting linear layout, and goes about
> allocating whatever layout it sees fit, and succeeds the allocation
> despite being passed a list of modifiers it knows nothing about. Not
> great, fixed in my pending patches, obviously doesn't help existing
> deployed userspace.
>
> -Current Mesa nouveau code, when asked what modifier it used for the
> above allocation, returns one of the "legacy" modifiers nouveau DRM-KMS
> knows nothing about.
>
> -When the modeset driver tries to create an FB for that BO with the
> returned modifier, the nouveau kernel driver of course refuses.
>
> I think it's probably worth fixing the modesetting driver for the
> reasons Daniel Vetter mentioned. Then if I get my Mesa patches in
> before a new modesetting driver with working Atomic support is released,
> there'll be no need for long-term workarounds in the kernel.
So atm there's 0 working -modesetting with atomic because all of them
are caught up by the kernel's check to refuse to hand out atomic
support to X. I think we'd also need a patch to pump the atomic setcap
to 2 (which should bypass the kernel's hack), plus of course someone
needs to release 1.21.
> Down to the real question of what to do in the kernel to support current
> userspace code: I still think the best fix is to accept the old
> modifiers but not advertise them. However, Daniel Stone and others, if
> you think this will actually break userspace in other ways (Could you
> describe in a bit more detail or point me to test cases if so?), I
> suppose the only option would be to advertise & accept the old modifiers
> for now, and I suppose at a config option at some point to phase the old
> ones out, eventually drop them entirely. This would be unfortunate,
> because as I mentioned, it could sometimes result in situations where
> apps think they can share a buffer between two devices but will get
> garbled data in practice.
>
> I've included an initial version of the kernel patch inline below.
> Needs more testing, but I wanted to share it in case anyone has feedback
> on the idea, wants to see the general workflow, or wants to help test.
I think the only risk I'm seeing if nouveau currently also allocates
formats (sometimes, maybe) that you then can't always scan out. If
that's not the case, then silently allowing old modifiers to keep
things working sounds like the best option.
Otherwise sounds all reasonable, maybe more comments and hints to what
exactly is broken in userspace for the real patch (once tested and
all).
-Daniel
> >>> 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.
> >>
> >> Right, it doesn't ask, because userspace
Re: [git pull] drm for 5.8-rc1
On 7/2/20 2:14 PM, James Jones wrote:
On 7/2/20 1:22 AM, Daniel Stone wrote:
Hi,
On Wed, 1 Jul 2020 at 20:45, James Jones 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(...);
}
I read this thread expecting to explain the correct behaviour we
implement in Weston and how modesetting needs to be fixed, but ...
that seems OK to me? As long as `gbm_has_modifiers` is a proxy for 'we
used gbm_(bo|surface)_create_with_modifiers to allocate the buffer'.
Yes, the hazards of reporting findings before verifying. I now see
modesetting does query the DRM-KMS modifiers and attempt to allocate
with them if it found any. However, I still see a lot of ways things
can go wrong, but I'm not going to share my speculation again until I've
actually verified it, which is taking a frustratingly long time. The
modesetting driver is not my friend right now.
OK, several hours of dumb build+config mistakes later, I was actually
able to reproduce the failure and walk through things. There is a
trivial fix for the issues in the X modesetting driver, working off
Daniel Stone's claim that gbm_bo_get_modifier() should only be called
when the allocation was made with gbm_bo_create_with_modifiers().
modeset doesn't respect that requirement now in the case that the atomic
modesetting path is disabled, which is always the case currently because
that path is broken. Respecting that requirement is a half-liner and
allows X to start properly.
If I force modeset to use the atomic path, X still fails to start with
the above fix, validating the second theory I'd had:
-Current Mesa nouveau code basically ignores the modifier list passed in
unless it is a single modifier requesting linear layout, and goes about
allocating whatever layout it sees fit, and succeeds the allocation
despite being passed a list of modifiers it knows nothing about. Not
great, fixed in my pending patches, obviously doesn't help existing
deployed userspace.
-Current Mesa nouveau code, when asked what modifier it used for the
above allocation, returns one of the "legacy" modifiers nouveau DRM-KMS
knows nothing about.
-When the modeset driver tries to create an FB for that BO with the
returned modifier, the nouveau kernel driver of course refuses.
I think it's probably worth fixing the modesetting driver for the
reasons Daniel Vetter mentioned. Then if I get my Mesa patches in
before a new modesetting driver with working Atomic support is released,
there'll be no need for long-term workarounds in the kernel.
Down to the real question of what to do in the kernel to support current
userspace code: I still think the best fix is to accept the old
modifiers but not advertise them. However, Daniel Stone and others, if
you think this will actually break userspace in other ways (Could you
describe in a bit more detail or point me to test cases if so?), I
suppose the only option would be to advertise & accept the old modifiers
for now, and I suppose at a config option at some point to phase the old
ones out, eventually drop them entirely. This would be unfortunate,
because as I mentioned, it could sometimes result in situations where
apps think they can share a buffer between two devices but will get
garbled data in practice.
I've included an initial version of the kernel patch inline below.
Needs more testing, but I wanted to share it in case anyone has feedback
on the idea, wants to see the general workflow, or wants to help test.
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.
Right, it doesn't ask, because userspace tells it which modifiers to
use. The correct behaviour is to take the list from the KMS
`IN_FORMATS` property and then pass that to
`gbm_(bo|surface)_create_with_modifiers`; GBM must then select from
that list and only that list. If that call does not succeed and Xorg
falls back to `gbm_surface_create`, then it must not call
`gbm_bo_get_modifier` - so that would be a modesetting bug. If that
call does succeed and `gbm_bo_get_modifier` subsequently reports a
modifier which was not in the list, that's a Mesa driver bug.
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?
Re: [git pull] drm for 5.8-rc1
On 7/2/20 1:22 AM, Daniel Stone wrote:
Hi,
On Wed, 1 Jul 2020 at 20:45, James Jones 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(...);
}
I read this thread expecting to explain the correct behaviour we
implement in Weston and how modesetting needs to be fixed, but ...
that seems OK to me? As long as `gbm_has_modifiers` is a proxy for 'we
used gbm_(bo|surface)_create_with_modifiers to allocate the buffer'.
Yes, the hazards of reporting findings before verifying. I now see
modesetting does query the DRM-KMS modifiers and attempt to allocate
with them if it found any. However, I still see a lot of ways things
can go wrong, but I'm not going to share my speculation again until I've
actually verified it, which is taking a frustratingly long time. The
modesetting driver is not my friend right now.
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.
Right, it doesn't ask, because userspace tells it which modifiers to
use. The correct behaviour is to take the list from the KMS
`IN_FORMATS` property and then pass that to
`gbm_(bo|surface)_create_with_modifiers`; GBM must then select from
that list and only that list. If that call does not succeed and Xorg
falls back to `gbm_surface_create`, then it must not call
`gbm_bo_get_modifier` - so that would be a modesetting bug. If that
call does succeed and `gbm_bo_get_modifier` subsequently reports a
modifier which was not in the list, that's a Mesa driver bug.
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.
Two options for GBM users:
* call gbm_*_create_with_modifiers, it succeeds, call
gbm_bo_get_modifier, pass modifier into AddFB
* call gbm_*_create (without modifiers), it succeeds, do not call
gbm_bo_get_modifier, do not pass a modifier into AddFB
Anything else is a bug in the user. Note that falling back from 1 to 2
is fine: if `gbm_*_create_with_modifiers()` fails, you can fall back
to the non-modifier path, provided you don't later try to get a
modifier back out.
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.
This would break other drivers.
I think this could be done in a way that wouldn't, though it wouldn't be
quite as simple. Let's see what the true root cause is first though.
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.
I do think that they should also be reported to userspace if they are
accepted. Other users can and do look at the modifier list to see if
the buffer is acceptable for a given plane, so the consistency is good
here. Of course, in Mesa you would want to prioritise the new
modifiers over the legacy ones, and not allocate or return the legacy
ones unless that was all you were asked for. This would involve
tracking the used modifier explicitly through Mesa, rather than
throwing it away at alloc time and then later divining it from the
tiling mode.
Reporting them as supported is equivalent to reporting support for a
memory layout the chips don't actually support (It corresponds to a
valid layout on Tegra chips, but not on discrete NV chips). This is
what the new modifiers are trying to avoid in
Re: [git pull] drm for 5.8-rc1
Hi,
On Wed, 1 Jul 2020 at 20:45, James Jones 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(...);
> }
I read this thread expecting to explain the correct behaviour we
implement in Weston and how modesetting needs to be fixed, but ...
that seems OK to me? As long as `gbm_has_modifiers` is a proxy for 'we
used gbm_(bo|surface)_create_with_modifiers to allocate the buffer'.
> 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.
Right, it doesn't ask, because userspace tells it which modifiers to
use. The correct behaviour is to take the list from the KMS
`IN_FORMATS` property and then pass that to
`gbm_(bo|surface)_create_with_modifiers`; GBM must then select from
that list and only that list. If that call does not succeed and Xorg
falls back to `gbm_surface_create`, then it must not call
`gbm_bo_get_modifier` - so that would be a modesetting bug. If that
call does succeed and `gbm_bo_get_modifier` subsequently reports a
modifier which was not in the list, that's a Mesa driver bug.
> 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.
Two options for GBM users:
* call gbm_*_create_with_modifiers, it succeeds, call
gbm_bo_get_modifier, pass modifier into AddFB
* call gbm_*_create (without modifiers), it succeeds, do not call
gbm_bo_get_modifier, do not pass a modifier into AddFB
Anything else is a bug in the user. Note that falling back from 1 to 2
is fine: if `gbm_*_create_with_modifiers()` fails, you can fall back
to the non-modifier path, provided you don't later try to get a
modifier back out.
> 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.
This would break other drivers.
> 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.
I do think that they should also be reported to userspace if they are
accepted. Other users can and do look at the modifier list to see if
the buffer is acceptable for a given plane, so the consistency is good
here. Of course, in Mesa you would want to prioritise the new
modifiers over the legacy ones, and not allocate or return the legacy
ones unless that was all you were asked for. This would involve
tracking the used modifier explicitly through Mesa, rather than
throwing it away at alloc time and then later divining it from the
tiling mode.
Cheers,
Daniel
Re: [git pull] drm for 5.8-rc1
On Wed, 1 Jul 2020 12:45:48 -0700
James Jones 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.
Hi,
the way I believe it is supposed to work is that modesetting DDX first
asks KMS what modifiers it supports, and then passes that list to GBM
when it is attempting to create a gbm_bo or a gbm_surface. If
modesetting does not do that and still uses modifiers API for creating
bos or surfaces, modesetting is broken I believe.
gbm_bo_create_with_modifiers()
gbm_surface_create_with_modifiers()
Of course, this doesn't affect the need of a kernel workaround.
How is modesetting creating the bo or surface currently?
Why would the KMS driver starting to support modifiers API suddenly
make the buffers fail, does it change how the buffers get allocated in
modesetting as well?
Or maybe you're talking about something else than what I assumed, I'm
not really up-to-date here.
Thanks,
pq
pgpro6QKHEnfN.pgp
Description: OpenPGP digital signature
Re: [git pull] drm for 5.8-rc1
On Wed, Jul 1, 2020 at 9:45 PM James Jones 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
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-de...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: [git pull] drm for 5.8-rc1
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.
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-de...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [git pull] drm for 5.8-rc1
On Wed, Jul 1, 2020 at 7:37 PM James Jones wrote: > > On 7/1/20 10:04 AM, Karol Herbst wrote: > > On Wed, Jul 1, 2020 at 6:01 PM Daniel Vetter wrote: > >> > >> On Wed, Jul 1, 2020 at 5:51 PM James Jones wrote: > >>> > >>> On 7/1/20 4:24 AM, Karol Herbst wrote: > On Wed, Jul 1, 2020 at 6:45 AM 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. > > > > that's completely irrelevant. If a kernel change breaks userspace, > it's a kernel bug. > >>> > >>> Agreed it is unacceptable to break userspace, but I don't think it's > >>> irrelevant. Perhaps the musings on pending userspace patches are. > >>> > >>> My intent here was to point out it appears at first glance that > >>> something isn't behaving as expected in userspace, so fixing this would > >>> likely require some sort of work-around for broken userspace rather than > >>> straight-forward fixing of a bug in the kernel logic. My intent was not > >>> to shift blame to something besides my code & testing for the > >>> regression, though I certainly see how it could be interpreted that way. > >>> > >>> Regardless, I'm looking in to it. > >> > > > > I assume the MR you were talking about is > > https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/3724 ? > > Correct. > > > I am > > also aware of the tegra driver being broken on my jetson nano and I am > > now curious if this MR could fix this bug as well... and sorry for the > > harsh reply, I was just a annoyed by the fact that "everything > > modifier related is just breaking things", first tegra and that nobody > > is looking into fixing it and then apparently the userspace code being > > quite broken as well :/ > > > > Anyway, yeah I trust you guys on figuring out the keeping "broken" > > userspace happy from a kernel side and maybe I can help out with > > reviewing the mesa bits. I am just wondering if it could help with the > > tegra situation giving me more reasons to look into it as this would > > solve other issues I should be working on :) > > Not sure if you're claiming this, but if there's Tegra breakage > attributable to this patch series, I'd love to hear more details there > as well. The Tegra patches did have backwards-compat code to handle the > old modifiers, since Tegra was the only working use case I could find > for them within the kernel itself. However, the Tegra kernel patches > are independent (and haven't even been reviewed yet to my knowledge), so > Tegra shouldn't be affected at all given it uses TegraDRM rather than > Nouveau's modesetting driver. > > If there are just general existing issues with modifier support on > Tegra, let's take that to a smaller venue. I probably won't be as much > help there, but I can at least try to help get some eyes on it. > I am sure that your patches here have nothing to do with it, just inside mesa since https://gitlab.freedesktop.org/mesa/mesa/commit/c56fe4118a2dd991ff1b2a532c0f234eddd435e9 it's broken on the jetson nano and because it's so old I am not able to tell if this is because of some kernel changes or because of the modifier code inside mesa being slightly broken. Maybe you have an idea, but Thierry knows about the issue, but I think he never was able to reproduce it on any system. > Thanks, > -James > > >> If we do need to have a kernel workaround I'm happy to help out, I've > >> done a bunch of these and occasionally it's good to get rather > >> creative :-) > >> > >> Ideally we'd also push a minimal fix in userspace to all stable > >> branches and make sure distros upgrade (might need releases if some > >> distro is stuck on old horrors), so that we don't have to keep the > >> hack in place for 10+ years or so. Definitely if the hack amounts to > >> disabling modifiers on nouveau, that would be kinda sad. > >> -Daniel > >> > >>> > >>> Thanks, > >>> -James > >>> > > Are you using the modesetting driver in X? If so, with glamor I > > presume? What version of Mesa? Any distro patches? Any non-default > > xorg.conf options that would affect modesetting, your X driver if it > > isn't modesetting, or glamour? > > > > Thanks, > > -James > > > > On 6/30/20 4:08 PM, Kirill A. Shutemov wrote: > >> On Tue, Jun 02, 2020 at 04:06:32PM +1000, Dave Airlie wrote: > >>> James Jones (4): > >> ... > >>> drm/nouveau/kms: Support NVIDIA format modifiers > >> > >> This commit is the first one that breaks Xorg startup fo
Re: [git pull] drm for 5.8-rc1
On 7/1/20 10:04 AM, Karol Herbst wrote: On Wed, Jul 1, 2020 at 6:01 PM Daniel Vetter wrote: On Wed, Jul 1, 2020 at 5:51 PM James Jones wrote: On 7/1/20 4:24 AM, Karol Herbst wrote: On Wed, Jul 1, 2020 at 6:45 AM 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. that's completely irrelevant. If a kernel change breaks userspace, it's a kernel bug. Agreed it is unacceptable to break userspace, but I don't think it's irrelevant. Perhaps the musings on pending userspace patches are. My intent here was to point out it appears at first glance that something isn't behaving as expected in userspace, so fixing this would likely require some sort of work-around for broken userspace rather than straight-forward fixing of a bug in the kernel logic. My intent was not to shift blame to something besides my code & testing for the regression, though I certainly see how it could be interpreted that way. Regardless, I'm looking in to it. I assume the MR you were talking about is https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/3724 ? Correct. I am also aware of the tegra driver being broken on my jetson nano and I am now curious if this MR could fix this bug as well... and sorry for the harsh reply, I was just a annoyed by the fact that "everything modifier related is just breaking things", first tegra and that nobody is looking into fixing it and then apparently the userspace code being quite broken as well :/ Anyway, yeah I trust you guys on figuring out the keeping "broken" userspace happy from a kernel side and maybe I can help out with reviewing the mesa bits. I am just wondering if it could help with the tegra situation giving me more reasons to look into it as this would solve other issues I should be working on :) Not sure if you're claiming this, but if there's Tegra breakage attributable to this patch series, I'd love to hear more details there as well. The Tegra patches did have backwards-compat code to handle the old modifiers, since Tegra was the only working use case I could find for them within the kernel itself. However, the Tegra kernel patches are independent (and haven't even been reviewed yet to my knowledge), so Tegra shouldn't be affected at all given it uses TegraDRM rather than Nouveau's modesetting driver. If there are just general existing issues with modifier support on Tegra, let's take that to a smaller venue. I probably won't be as much help there, but I can at least try to help get some eyes on it. Thanks, -James If we do need to have a kernel workaround I'm happy to help out, I've done a bunch of these and occasionally it's good to get rather creative :-) Ideally we'd also push a minimal fix in userspace to all stable branches and make sure distros upgrade (might need releases if some distro is stuck on old horrors), so that we don't have to keep the hack in place for 10+ years or so. Definitely if the hack amounts to disabling modifiers on nouveau, that would be kinda sad. -Daniel Thanks, -James Are you using the modesetting driver in X? If so, with glamor I presume? What version of Mesa? Any distro patches? Any non-default xorg.conf options that would affect modesetting, your X driver if it isn't modesetting, or glamour? Thanks, -James On 6/30/20 4:08 PM, Kirill A. Shutemov wrote: On Tue, Jun 02, 2020 at 04:06:32PM +1000, Dave Airlie wrote: James Jones (4): ... drm/nouveau/kms: Support NVIDIA format modifiers This commit is the first one that breaks Xorg startup for my setup: GTX 1080 + Dell UP2414Q (4K DP MST monitor). I believe this is the crucial part of dmesg (full dmesg is attached): [ 29.997140] [drm:nouveau_framebuffer_new] Unsupported modifier: 0x314 [ 29.997143] [drm:drm_internal_framebuffer_create] could not create framebuffer [ 29.997145] [drm:drm_ioctl] pid=3393, ret = -22 Any suggestions? ___ dri-devel mailing list dri-de...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-de...@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-de...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [git pull] drm for 5.8-rc1
On Wed, Jul 1, 2020 at 6:01 PM Daniel Vetter wrote: > > On Wed, Jul 1, 2020 at 5:51 PM James Jones wrote: > > > > On 7/1/20 4:24 AM, Karol Herbst wrote: > > > On Wed, Jul 1, 2020 at 6:45 AM 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. > > >> > > > > > > that's completely irrelevant. If a kernel change breaks userspace, > > > it's a kernel bug. > > > > Agreed it is unacceptable to break userspace, but I don't think it's > > irrelevant. Perhaps the musings on pending userspace patches are. > > > > My intent here was to point out it appears at first glance that > > something isn't behaving as expected in userspace, so fixing this would > > likely require some sort of work-around for broken userspace rather than > > straight-forward fixing of a bug in the kernel logic. My intent was not > > to shift blame to something besides my code & testing for the > > regression, though I certainly see how it could be interpreted that way. > > > > Regardless, I'm looking in to it. > I assume the MR you were talking about is https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/3724 ? I am also aware of the tegra driver being broken on my jetson nano and I am now curious if this MR could fix this bug as well... and sorry for the harsh reply, I was just a annoyed by the fact that "everything modifier related is just breaking things", first tegra and that nobody is looking into fixing it and then apparently the userspace code being quite broken as well :/ Anyway, yeah I trust you guys on figuring out the keeping "broken" userspace happy from a kernel side and maybe I can help out with reviewing the mesa bits. I am just wondering if it could help with the tegra situation giving me more reasons to look into it as this would solve other issues I should be working on :) > If we do need to have a kernel workaround I'm happy to help out, I've > done a bunch of these and occasionally it's good to get rather > creative :-) > > Ideally we'd also push a minimal fix in userspace to all stable > branches and make sure distros upgrade (might need releases if some > distro is stuck on old horrors), so that we don't have to keep the > hack in place for 10+ years or so. Definitely if the hack amounts to > disabling modifiers on nouveau, that would be kinda sad. > -Daniel > > > > > Thanks, > > -James > > > > >> Are you using the modesetting driver in X? If so, with glamor I > > >> presume? What version of Mesa? Any distro patches? Any non-default > > >> xorg.conf options that would affect modesetting, your X driver if it > > >> isn't modesetting, or glamour? > > >> > > >> Thanks, > > >> -James > > >> > > >> On 6/30/20 4:08 PM, Kirill A. Shutemov wrote: > > >>> On Tue, Jun 02, 2020 at 04:06:32PM +1000, Dave Airlie wrote: > > James Jones (4): > > >>> ... > > drm/nouveau/kms: Support NVIDIA format modifiers > > >>> > > >>> This commit is the first one that breaks Xorg startup for my setup: > > >>> GTX 1080 + Dell UP2414Q (4K DP MST monitor). > > >>> > > >>> I believe this is the crucial part of dmesg (full dmesg is attached): > > >>> > > >>> [ 29.997140] [drm:nouveau_framebuffer_new] Unsupported modifier: > > >>> 0x314 > > >>> [ 29.997143] [drm:drm_internal_framebuffer_create] could not create > > >>> framebuffer > > >>> [ 29.997145] [drm:drm_ioctl] pid=3393, ret = -22 > > >>> > > >>> Any suggestions? > > >>> > > >> ___ > > >> dri-devel mailing list > > >> dri-de...@lists.freedesktop.org > > >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > > >> > > > > > > ___ > > > dri-devel mailing list > > > dri-de...@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch >
Re: [git pull] drm for 5.8-rc1
On Wed, Jul 1, 2020 at 5:51 PM James Jones wrote: > > On 7/1/20 4:24 AM, Karol Herbst wrote: > > On Wed, Jul 1, 2020 at 6:45 AM 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. > >> > > > > that's completely irrelevant. If a kernel change breaks userspace, > > it's a kernel bug. > > Agreed it is unacceptable to break userspace, but I don't think it's > irrelevant. Perhaps the musings on pending userspace patches are. > > My intent here was to point out it appears at first glance that > something isn't behaving as expected in userspace, so fixing this would > likely require some sort of work-around for broken userspace rather than > straight-forward fixing of a bug in the kernel logic. My intent was not > to shift blame to something besides my code & testing for the > regression, though I certainly see how it could be interpreted that way. > > Regardless, I'm looking in to it. If we do need to have a kernel workaround I'm happy to help out, I've done a bunch of these and occasionally it's good to get rather creative :-) Ideally we'd also push a minimal fix in userspace to all stable branches and make sure distros upgrade (might need releases if some distro is stuck on old horrors), so that we don't have to keep the hack in place for 10+ years or so. Definitely if the hack amounts to disabling modifiers on nouveau, that would be kinda sad. -Daniel > > Thanks, > -James > > >> Are you using the modesetting driver in X? If so, with glamor I > >> presume? What version of Mesa? Any distro patches? Any non-default > >> xorg.conf options that would affect modesetting, your X driver if it > >> isn't modesetting, or glamour? > >> > >> Thanks, > >> -James > >> > >> On 6/30/20 4:08 PM, Kirill A. Shutemov wrote: > >>> On Tue, Jun 02, 2020 at 04:06:32PM +1000, Dave Airlie wrote: > James Jones (4): > >>> ... > drm/nouveau/kms: Support NVIDIA format modifiers > >>> > >>> This commit is the first one that breaks Xorg startup for my setup: > >>> GTX 1080 + Dell UP2414Q (4K DP MST monitor). > >>> > >>> I believe this is the crucial part of dmesg (full dmesg is attached): > >>> > >>> [ 29.997140] [drm:nouveau_framebuffer_new] Unsupported modifier: > >>> 0x314 > >>> [ 29.997143] [drm:drm_internal_framebuffer_create] could not create > >>> framebuffer > >>> [ 29.997145] [drm:drm_ioctl] pid=3393, ret = -22 > >>> > >>> Any suggestions? > >>> > >> ___ > >> dri-devel mailing list > >> dri-de...@lists.freedesktop.org > >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > >> > > > > ___ > > dri-devel mailing list > > dri-de...@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [git pull] drm for 5.8-rc1
On 7/1/20 4:24 AM, Karol Herbst wrote: On Wed, Jul 1, 2020 at 6:45 AM 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. that's completely irrelevant. If a kernel change breaks userspace, it's a kernel bug. Agreed it is unacceptable to break userspace, but I don't think it's irrelevant. Perhaps the musings on pending userspace patches are. My intent here was to point out it appears at first glance that something isn't behaving as expected in userspace, so fixing this would likely require some sort of work-around for broken userspace rather than straight-forward fixing of a bug in the kernel logic. My intent was not to shift blame to something besides my code & testing for the regression, though I certainly see how it could be interpreted that way. Regardless, I'm looking in to it. Thanks, -James Are you using the modesetting driver in X? If so, with glamor I presume? What version of Mesa? Any distro patches? Any non-default xorg.conf options that would affect modesetting, your X driver if it isn't modesetting, or glamour? Thanks, -James On 6/30/20 4:08 PM, Kirill A. Shutemov wrote: On Tue, Jun 02, 2020 at 04:06:32PM +1000, Dave Airlie wrote: James Jones (4): ... drm/nouveau/kms: Support NVIDIA format modifiers This commit is the first one that breaks Xorg startup for my setup: GTX 1080 + Dell UP2414Q (4K DP MST monitor). I believe this is the crucial part of dmesg (full dmesg is attached): [ 29.997140] [drm:nouveau_framebuffer_new] Unsupported modifier: 0x314 [ 29.997143] [drm:drm_internal_framebuffer_create] could not create framebuffer [ 29.997145] [drm:drm_ioctl] pid=3393, ret = -22 Any suggestions? ___ dri-devel mailing list dri-de...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-de...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [git pull] drm for 5.8-rc1
On Wed, Jul 1, 2020 at 6:45 AM 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. > that's completely irrelevant. If a kernel change breaks userspace, it's a kernel bug. > Are you using the modesetting driver in X? If so, with glamor I > presume? What version of Mesa? Any distro patches? Any non-default > xorg.conf options that would affect modesetting, your X driver if it > isn't modesetting, or glamour? > > Thanks, > -James > > On 6/30/20 4:08 PM, Kirill A. Shutemov wrote: > > On Tue, Jun 02, 2020 at 04:06:32PM +1000, Dave Airlie wrote: > >> James Jones (4): > > ... > >>drm/nouveau/kms: Support NVIDIA format modifiers > > > > This commit is the first one that breaks Xorg startup for my setup: > > GTX 1080 + Dell UP2414Q (4K DP MST monitor). > > > > I believe this is the crucial part of dmesg (full dmesg is attached): > > > > [ 29.997140] [drm:nouveau_framebuffer_new] Unsupported modifier: > > 0x314 > > [ 29.997143] [drm:drm_internal_framebuffer_create] could not create > > framebuffer > > [ 29.997145] [drm:drm_ioctl] pid=3393, ret = -22 > > > > Any suggestions? > > > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel >
Re: [git pull] drm for 5.8-rc1
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. -- Kirill A. Shutemov [42.835] X.Org X Server 1.20.8 X Protocol Version 11, Revision 0 [42.835] Build Operating System: Linux 5.7.0-2-g7fe3a385103d x86_64 Gentoo [42.835] Current Operating System: Linux box 5.7.0-rc2-01340-gfa4f4c213f5f #49 SMP PREEMPT Wed Jul 1 10:44:16 +03 2020 x86_64 [42.835] Kernel command line: root=/dev/mapper/box-root dolvm rw rootfstype=ext4 drm.debug=0xf [42.836] Build Date: 16 June 2020 09:51:54PM [42.836] [42.836] Current version of pixman: 0.40.0 [42.836]Before reporting problems, check http://wiki.x.org to make sure that you have the latest version. [42.836] Markers: (--) probed, (**) from config file, (==) default setting, (++) from command line, (!!) notice, (II) informational, (WW) warning, (EE) error, (NI) not implemented, (??) unknown. [42.837] (==) Log file: "/var/log/Xorg.0.log", Time: Wed Jul 1 10:46:48 2020 [42.840] (==) Using config directory: "/etc/X11/xorg.conf.d" [42.840] (==) Using system config directory "/usr/share/X11/xorg.conf.d" [42.841] (==) No Layout section. Using the first Screen section. [42.841] (==) No screen section available. Using defaults. [42.841] (**) |-->Screen "Default Screen Section" (0) [42.841] (**) | |-->Monitor "" [42.842] (==) No device specified for screen "Default Screen Section". Using the first device section listed. [42.842] (**) | |-->Device "Device0" [42.842] (==) No monitor specified for screen "Default Screen Section". Using a default monitor configuration. [42.842] (==) Automatically adding devices [42.842] (==) Automatically enabling devices [42.842] (==) Automatically adding GPU devices [42.842] (==) Max clients allowed: 256, resource mask: 0x1f [42.842] (WW) The directory "/usr/share/fonts/misc/" does not exist. [42.842]Entry deleted from font path. [42.842] (WW) The directory "/usr/share/fonts/TTF/" does not exist. [42.842]Entry deleted from font path. [42.842] (WW) The directory "/usr/share/fonts/OTF/" does not exist. [42.842]Entry deleted from font path. [42.842] (WW) The directory "/usr/share/fonts/Type1/" does not exist. [42.842]Entry deleted from font path. [42.842] (WW) The directory "/usr/share/fonts/100dpi/" does not exist. [42.842]Entry deleted from font path. [42.842] (WW) The directory "/usr/share/fonts/75dpi/" does not exist. [42.842]Entry deleted from font path. [42.842] (==) FontPath set to: [42.842] (==) ModulePath set to "/usr/lib64/xorg/modules" [42.842] (II) The server relies on udev to provide the list of input devices. If no devices become available, reconfigure udev or disable AutoAddDevices. [42.842] (II) Loader magic: 0x563dfdb59c40 [42.842] (II) Module ABI versions: [42.842]X.Org ANSI C Emulation: 0.4 [42.842]X.Org Video Driver: 24.1 [42.842]X.Org XInput driver : 24.1 [42.843]X.Org Server Extension : 10.0 [42.843] (II) xfree86: Adding drm device (/dev/dri/card0) [42.899] (--) PCI:*(101@0:0:0) 10de:1b80:1458:3730 rev 161, Mem @ 0xd700/16777216, 0xc000/268435456, 0xd000/33554432, I/O @ 0xb000/128, BIOS @ 0x/131072 [42.899] (II) LoadModule: "glx" [42.903] (II) Loading /usr/lib64/xorg/modules/extensions/libglx.so [42.913] (II) Module glx: vendor="X.Org Foundation" [42.913]compiled for 1.20.8, module version = 1.0.0 [42.913]ABI class: X.Org Server Extension, version 10.0 [42.913] (II) LoadModule: "modesetting" [42.914] (II) Loading /usr/lib64/xorg/modules/drivers/modesetting_drv.so [42.915] (II) Module modesetting: vendor="X.Org Foundation" [42.915]compiled for 1.20.8, module version = 1.20.8 [42.915]Module class: X.Org Video Driver [42.915]ABI class: X.Org Video Driver, version 24.1 [42.915] (II) modesetting: Driver for Modesetting Kernel Drivers: kms [42.916] (--) using VT number 7 [42.921] (II) modeset(0): using drv /dev/dri/card0 [42.922] (II) modeset(0): Creating default Display subsection in Screen section
Re: [git pull] drm for 5.8-rc1
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. > What version of Mesa? 20.0.8 > Any distro patches? I don't see any. It's Gentoo. > Any non-default xorg.conf options that would affect modesetting, your X > driver if it isn't modesetting, or glamour? Modesetting without anything tricky. -- Kirill A. Shutemov
Re: [git pull] drm for 5.8-rc1
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? What version of Mesa? Any distro patches? Any non-default xorg.conf options that would affect modesetting, your X driver if it isn't modesetting, or glamour? Thanks, -James On 6/30/20 4:08 PM, Kirill A. Shutemov wrote: On Tue, Jun 02, 2020 at 04:06:32PM +1000, Dave Airlie wrote: James Jones (4): ... drm/nouveau/kms: Support NVIDIA format modifiers This commit is the first one that breaks Xorg startup for my setup: GTX 1080 + Dell UP2414Q (4K DP MST monitor). I believe this is the crucial part of dmesg (full dmesg is attached): [ 29.997140] [drm:nouveau_framebuffer_new] Unsupported modifier: 0x314 [ 29.997143] [drm:drm_internal_framebuffer_create] could not create framebuffer [ 29.997145] [drm:drm_ioctl] pid=3393, ret = -22 Any suggestions?
Re: [git pull] drm for 5.8-rc1
On Thu, Jun 04, 2020 at 10:10:21AM +0200, Christian König wrote: > Dave and Daniel explicitly said they want to have this and it is ok as long > as I open code it in the driver and keep it AMD internal. But it fundamentally is broken, and we've been through all the arguments why it is broken, and it is not going to be any less broken because someone said they want this. I have to say I'm really pissed that you guys just sneaked it in. I've been hoping that graphics is getting a litter better, but this attitud of we sneak hacks into the driver code that just happen to work on some platform you care about instead because you're too lazy to do the proper thing has top stop right now. drm has been creating so much more maintainaince nightmares and buggy code than any other subsysem just because of that attitude.
Re: [git pull] drm for 5.8-rc1
Am 03.06.20 um 22:13 schrieb Jason Gunthorpe:
On Tue, Jun 02, 2020 at 04:06:32PM +1000, Dave Airlie wrote:
Hi Linus,
This is the main drm pull request for 5.8-rc1.
Highlights:
Core DRM had a lot of refactoring around managed drm resources to make
drivers simpler.
Intel Tigerlake support is on by default
amdgpu now support p2p PCI buffer sharing and encrypted GPU memory
Christoph Hellwig basically NAK'd this approach, why is it getting
merged all of a sudden??
Dave and Daniel explicitly said they want to have this and it is ok as
long as I open code it in the driver and keep it AMD internal.
We have that in discussion for years now and constructing/using the sg
table is actually only the very minor piece of it. On the other hand
there is a lot of work underway to get rid of abusing the sg tables as well.
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fintel-gfx%2F20200311152838.GA24280%40infradead.org%2F&data=02%7C01%7Cchristian.koenig%40amd.com%7C55b238b9104d4a8d4feb08d807faa11c%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637268120315706063&sdata=AgVJ45%2Ft%2FVYkyIGIGgMrop69XLQReLDpF0ahL5rjEjo%3D&reserved=0
Are we now OK with this same approach open coded in a driver?
Intel is apparently doing this as well for years, see the i915 driver
internals.
This wasn't Cc'd to the usual people doing work in this PCI P2P area??
I certainly prefer a common framework for this, but when my upstream
maintainer says he wants to take this who am I to object?
Christian.
See
commit f44ffd677fb3562ac0a1ff9c8ae52672be741f00
Author: Christian König
Date: Fri Mar 23 16:56:37 2018 +0100
drm/amdgpu: add support for exporting VRAM using DMA-buf v3
We should be able to do this now after checking all the prerequisites.
v2: fix entrie count in the sgt
v3: manually construct the sg
Signed-off-by: Christian König
Acked-by: Daniel Vetter
Acked-by: Sumit Semwal
Link:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.freedesktop.org%2Fpatch%2F359295&data=02%7C01%7Cchristian.koenig%40amd.com%7C55b238b9104d4a8d4feb08d807faa11c%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637268120315706063&sdata=YzNvxBVOf5hcUm5KjOzzV%2FcHG5jdGEYmrI76PQN9v3U%3D&reserved=0
[..]
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
index 82a3299e53c042..128a667ed8fa0d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
@@ -22,6 +22,7 @@
* Authors: Christian König
*/
+#include
#include "amdgpu.h"
#include "amdgpu_vm.h"
#include "amdgpu_atomfirmware.h"
@@ -458,6 +459,104 @@ static void amdgpu_vram_mgr_del(struct
ttm_mem_type_manager *man,
mem->mm_node = NULL;
}
+/**
+ * amdgpu_vram_mgr_alloc_sgt - allocate and fill a sg table
+ *
+ * @adev: amdgpu device pointer
+ * @mem: TTM memory object
+ * @dev: the other device
+ * @dir: dma direction
+ * @sgt: resulting sg table
+ *
+ * Allocate and fill a sg table from a VRAM allocation.
+ */
+int amdgpu_vram_mgr_alloc_sgt(struct amdgpu_device *adev,
+ struct ttm_mem_reg *mem,
+ struct device *dev,
+ enum dma_data_direction dir,
+ struct sg_table **sgt)
+{
+ struct drm_mm_node *node;
+ struct scatterlist *sg;
+ int num_entries = 0;
+ unsigned int pages;
+ int i, r;
+
+ *sgt = kmalloc(sizeof(*sg), GFP_KERNEL);
+ if (!*sgt)
+ return -ENOMEM;
+
+ for (pages = mem->num_pages, node = mem->mm_node;
+pages; pages -= node->size, ++node)
+ ++num_entries;
+
+ r = sg_alloc_table(*sgt, num_entries, GFP_KERNEL);
+ if (r)
+ goto error_free;
+
+ for_each_sg((*sgt)->sgl, sg, num_entries, i)
+ sg->length = 0;
+
+ node = mem->mm_node;
+ for_each_sg((*sgt)->sgl, sg, num_entries, i) {
+ phys_addr_t phys = (node->start << PAGE_SHIFT) +
+ adev->gmc.aper_base;
+ size_t size = node->size << PAGE_SHIFT;
+ dma_addr_t addr;
+
+ ++node;
+ addr = dma_map_resource(dev, phys, size, dir,
+ DMA_ATTR_SKIP_CPU_SYNC);
+ r = dma_mapping_error(dev, addr);
+ if (r)
+ goto error_unmap;
+
+ sg_set_page(sg, NULL, size, 0);
+ sg_dma_address(sg) = addr;
+ sg_dma_len(sg) = size;
^^
Jason
Re: [git pull] drm for 5.8-rc1
On Tue, Jun 02, 2020 at 04:06:32PM +1000, Dave Airlie wrote:
> Hi Linus,
>
> This is the main drm pull request for 5.8-rc1.
>
> Highlights:
> Core DRM had a lot of refactoring around managed drm resources to make
> drivers simpler.
> Intel Tigerlake support is on by default
> amdgpu now support p2p PCI buffer sharing and encrypted GPU memory
Christoph Hellwig basically NAK'd this approach, why is it getting
merged all of a sudden??
https://lore.kernel.org/intel-gfx/20200311152838.ga24...@infradead.org/
Are we now OK with this same approach open coded in a driver?
This wasn't Cc'd to the usual people doing work in this PCI P2P area??
See
commit f44ffd677fb3562ac0a1ff9c8ae52672be741f00
Author: Christian König
Date: Fri Mar 23 16:56:37 2018 +0100
drm/amdgpu: add support for exporting VRAM using DMA-buf v3
We should be able to do this now after checking all the prerequisites.
v2: fix entrie count in the sgt
v3: manually construct the sg
Signed-off-by: Christian König
Acked-by: Daniel Vetter
Acked-by: Sumit Semwal
Link: https://patchwork.freedesktop.org/patch/359295
[..]
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
index 82a3299e53c042..128a667ed8fa0d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
@@ -22,6 +22,7 @@
* Authors: Christian König
*/
+#include
#include "amdgpu.h"
#include "amdgpu_vm.h"
#include "amdgpu_atomfirmware.h"
@@ -458,6 +459,104 @@ static void amdgpu_vram_mgr_del(struct
ttm_mem_type_manager *man,
mem->mm_node = NULL;
}
+/**
+ * amdgpu_vram_mgr_alloc_sgt - allocate and fill a sg table
+ *
+ * @adev: amdgpu device pointer
+ * @mem: TTM memory object
+ * @dev: the other device
+ * @dir: dma direction
+ * @sgt: resulting sg table
+ *
+ * Allocate and fill a sg table from a VRAM allocation.
+ */
+int amdgpu_vram_mgr_alloc_sgt(struct amdgpu_device *adev,
+ struct ttm_mem_reg *mem,
+ struct device *dev,
+ enum dma_data_direction dir,
+ struct sg_table **sgt)
+{
+ struct drm_mm_node *node;
+ struct scatterlist *sg;
+ int num_entries = 0;
+ unsigned int pages;
+ int i, r;
+
+ *sgt = kmalloc(sizeof(*sg), GFP_KERNEL);
+ if (!*sgt)
+ return -ENOMEM;
+
+ for (pages = mem->num_pages, node = mem->mm_node;
+pages; pages -= node->size, ++node)
+ ++num_entries;
+
+ r = sg_alloc_table(*sgt, num_entries, GFP_KERNEL);
+ if (r)
+ goto error_free;
+
+ for_each_sg((*sgt)->sgl, sg, num_entries, i)
+ sg->length = 0;
+
+ node = mem->mm_node;
+ for_each_sg((*sgt)->sgl, sg, num_entries, i) {
+ phys_addr_t phys = (node->start << PAGE_SHIFT) +
+ adev->gmc.aper_base;
+ size_t size = node->size << PAGE_SHIFT;
+ dma_addr_t addr;
+
+ ++node;
+ addr = dma_map_resource(dev, phys, size, dir,
+ DMA_ATTR_SKIP_CPU_SYNC);
+ r = dma_mapping_error(dev, addr);
+ if (r)
+ goto error_unmap;
+
+ sg_set_page(sg, NULL, size, 0);
+ sg_dma_address(sg) = addr;
+ sg_dma_len(sg) = size;
^^
Jason
Re: [git pull] drm for 5.8-rc1
On Wed, Jun 3, 2020 at 9:18 AM Thomas Zimmermann wrote:
>
> Hi
>
> Am 02.06.20 um 23:56 schrieb Linus Torvalds:
> > On Tue, Jun 2, 2020 at 2:21 PM Linus Torvalds
> > wrote:
> >>
> >> I'm still working through the rest of the merge, so far that was the
> >> only one that made me go "Whaa?".
> >
> > Hmm. I'm also ending up effectively reverting the drm commit
> > b28ad7deb2f2 ("drm/tidss: Use simple encoder") because commit
> > 9da67433f64e ("drm/tidss: fix crash related to accessing freed
> > memory") made the premise of that simply encoder commit no longer be
> > true.
>
> That's OK. The simple encoder is just for consolidating these
> almost-empty encoders at a single place.
>
> > If there is a better way to sort that out (ie something like "use
> > simple encoder but make it free things at destroy time"), I don't know
> > of it.
>
> There's now drmm_kmalloc() to auto-free the memory when DRM releases a
> device.
Yeah I think we discussed that tidss patch on dri-devel when it showed
up, right fix is to essentially undo it, replace with a
s/devm_kzalloc/drmm_kmalloc/ and then re-apply the simple encoder
conversion. We had (and I think still have) some details to sort out
in all this, so some back&forth is entirely expected here. Also it's
just driver unload, which at least for integrated gpu no user ever
cares about, only developers.
-Daniel
>
> Best regards
> Thomas
>
> >
> > I'll let you guys fight it out (added people involved with those
> > commits to the participants,
> >
> > Linus
> > ___
> > dri-devel mailing list
> > dri-de...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
Re: [git pull] drm for 5.8-rc1
Hi
Am 02.06.20 um 23:56 schrieb Linus Torvalds:
> On Tue, Jun 2, 2020 at 2:21 PM Linus Torvalds
> wrote:
>>
>> I'm still working through the rest of the merge, so far that was the
>> only one that made me go "Whaa?".
>
> Hmm. I'm also ending up effectively reverting the drm commit
> b28ad7deb2f2 ("drm/tidss: Use simple encoder") because commit
> 9da67433f64e ("drm/tidss: fix crash related to accessing freed
> memory") made the premise of that simply encoder commit no longer be
> true.
That's OK. The simple encoder is just for consolidating these
almost-empty encoders at a single place.
> If there is a better way to sort that out (ie something like "use
> simple encoder but make it free things at destroy time"), I don't know
> of it.
There's now drmm_kmalloc() to auto-free the memory when DRM releases a
device.
Best regards
Thomas
>
> I'll let you guys fight it out (added people involved with those
> commits to the participants,
>
> Linus
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
signature.asc
Description: OpenPGP digital signature
Re: [git pull] drm for 5.8-rc1
On Wed, 3 Jun 2020 at 08:14, Linus Torvalds wrote: > > On Mon, Jun 1, 2020 at 11:06 PM Dave Airlie wrote: > > > > I've pushed a merged by me tree here, which I think gets them all > > correct, but please let me know if you think different. > > https://cgit.freedesktop.org/~airlied/linux/log/?h=drm-5.8-merged > > Ok, I get the same result, except my resolution to the simple encoder > issue was slightly different. I removed the simple helper header > include too as part of basically undoing the whole simple encoder > conversion. Yes sounds like my experience. I spent time on the tides and it was a revert pretty much of the commit in next, I just missed the header include line. I also realised I'd likely mismerged earlier when fixing this up, I'm going to have to put more time into merge fixing up, I'm still not always happy with my methods of figuring out what the correct answer is. > But other than that we're identical, which is a good sign. Apparently > the drm mis-merge in the middle got fixed up. Cool, thanks for redoing it, since this was definitely one of the more conflicty ones I've had in a while. Dave.
Re: [git pull] drm for 5.8-rc1
The pull request you sent on Tue, 2 Jun 2020 16:06:32 +1000: > git://anongit.freedesktop.org/drm/drm tags/drm-next-2020-06-02 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/faa392181a0bd42c5478175cef601adeecdc91b6 Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker
Re: [git pull] drm for 5.8-rc1
On Mon, Jun 1, 2020 at 11:06 PM Dave Airlie wrote: > > I've pushed a merged by me tree here, which I think gets them all > correct, but please let me know if you think different. > https://cgit.freedesktop.org/~airlied/linux/log/?h=drm-5.8-merged Ok, I get the same result, except my resolution to the simple encoder issue was slightly different. I removed the simple helper header include too as part of basically undoing the whole simple encoder conversion. But other than that we're identical, which is a good sign. Apparently the drm mis-merge in the middle got fixed up. Linus
Re: [git pull] drm for 5.8-rc1
On Tue, Jun 2, 2020 at 2:21 PM Linus Torvalds
wrote:
>
> I'm still working through the rest of the merge, so far that was the
> only one that made me go "Whaa?".
Hmm. I'm also ending up effectively reverting the drm commit
b28ad7deb2f2 ("drm/tidss: Use simple encoder") because commit
9da67433f64e ("drm/tidss: fix crash related to accessing freed
memory") made the premise of that simply encoder commit no longer be
true.
If there is a better way to sort that out (ie something like "use
simple encoder but make it free things at destroy time"), I don't know
of it.
I'll let you guys fight it out (added people involved with those
commits to the participants,
Linus
Re: [git pull] drm for 5.8-rc1
On Tue, Jun 2, 2020 at 2:21 PM Linus Torvalds
wrote:
>
> Hmm. Some of them are due to your previous mis-merges.
>
> Your commit 937eea297e26 ("Merge tag 'amd-drm-next-5.8-2020-04-24' of
> git://people.freedesktop.org/~agd5f/linux into drm-next") seems to
> have mis-merged the CONFIG_DEBUG_FS thing in
> drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c.
Sorry, wrong filename. That should have been
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c, I cut-and-pasted
the wrong path from the conflict list..
Linus
Re: [git pull] drm for 5.8-rc1
On Mon, Jun 1, 2020 at 11:06 PM Dave Airlie wrote:
>
> This tree is a bit conflicty, the i915 ones are probably the hairy
> ones, but amdgpu has a bunch as well, along with smattering of others.
Hmm. Some of them are due to your previous mis-merges.
Your commit 937eea297e26 ("Merge tag 'amd-drm-next-5.8-2020-04-24' of
git://people.freedesktop.org/~agd5f/linux into drm-next") seems to
have mis-merged the CONFIG_DEBUG_FS thing in
drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c.
I'm still working through the rest of the merge, so far that was the
only one that made me go "Whaa?".
Linus
