Re: [Nouveau] 4.20.0-rc3 nouveau/Quadro P2000 Mobile: runpm causing ACPI errors, lockups
On Thu, Nov 29, 2018 at 12:21:31AM +0100, Karol Herbst wrote: > this was already debugged and there is no point in searching inside > the Firmware. It's not a firmware bug or anything. > > The proper fix is to do something inside Nouveau so that we don't > upset the device and being able to runtime resume it again. > > The initial thing we do inside Nouveau to cause those issues is to run > that so called "DEVINIT" script inside the vbios to initialize the > GPU, problem is, it changes something on the PCIe configuration so > that the GPU isn't able to runtime resume anymore. I am in contact > with Nvidia about that issue and hopefully we get the proper answers. > When I was digging into that myself I was able to make the situation > more stable by setting the PCIE link speed to the boot defaults, but > that was still pretty unstable. > > Anyway, because the binary driver fails here as well (through > bumblebee and so on) there isn't much of reverse engineering we can do > besides guessing and trying it on literally every hardware until it > works. > > We also have an upstream bug for this issue: > https://bugzilla.kernel.org/show_bug.cgi?id=156341 If you like I can probably dump the pcie registers on card and/or the pcie port under windows. The card works there :) Let me know. -- MST ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH v6 3/6] drm/dp_mst: Start tracking per-port VCPI allocations
On Wed, 2018-11-28 at 09:17 +0100, Daniel Vetter wrote: > On Tue, Nov 27, 2018 at 08:44:14PM -0500, Lyude Paul wrote: > > On Tue, 2018-11-27 at 20:44 +0100, Daniel Vetter wrote: > > > > We could do this the other way around so it looks like this maybe > > > > struct kref; /* manages kfree() */ > > struct topology_kref; /* corresonds to lifetime in topology */ > > > > Then only expose functions for the normal kref to drivers, so that any > > possible confusion is still limited to drm_dp_mst_topology.c > > I like this bikeshed a lot. Since we need a bunch of the plain > kref_get/put internally, probably still good to have a wrapper. For that > the port_malloc_get/put() still sounds good to me - port_kfree_get/put > sounds confusing, since it's not the kfree we're refcounting, but the > memory allocation. > > Another option would be to add _topology to the public get/put functions, > but that makes for a fairly long function name :-/ bleh. Looked at not using a malloc() prefix in the naming but it's definitely still more confusing without one. I really do question if we really want the canonical naming prefix for the MST helpers to be drm_dp_mst_topology. It would be very nice to have this freed so we could do something like this: drm_dp_mst_topology_get_mstb() /* &topology_kref */ drm_dp_mst_topology_put_mstb() /* &topology_kref */ drm_dp_mst_get_mstb_malloc() /* &kref */ drm_dp_mst_put_mstb_malloc() /* &kref */ drm_dp_mst_topology_get_port() /* &topology_kref */ drm_dp_mst_topology_put_port() /* &topology_kref */ drm_dp_mst_get_port_malloc() /* &kref */ drm_dp_mst_put_port_malloc() /* &kref */ Additionally, I had an epiphany and figured out a seriously dead-simple rule that answers "what ref to use where" in the MST helper code: Let's start with a topology like this: |- mst_primary |- port #1 |- port #2 |- port #3 |- mstb #1 |- port #4 |- mstb #2 |- port #5 |- port #6 |- mstb #3 |- port #7 |- mstb #4 |- port #8 |- port #9 If mstb #1 was removed from the topology, ports 4-9 and mstbs 2-4 must lose their topology references. Simply put: each port holds a topology reference to it's mstb (if there is one), and each mstb holds a topology reference to it's ports. Now, let's allocate some payloads on the topology: |- mst_primary |- port #1 |- port #2 |- port #3 |- mstb #1 |- port #4 |- mstb #2 (has payload) |- port #5 |- port #6 |- mstb #3 |- port #7 |- mstb #4 (has payload) |- port #8 |- port #9 So, now if we remove mstb #1 any payloads under it need to be freed. Since mstb #4's and #2's parents no longer exist in the topology however, we must go back up the tree to find our last living relative. So: mstb #4 → port #7 → mstb #3 → port #6 → mstb #1 → port #3 for payload 1 mstb #4 → port #4 → mstb #1 → port #3 for payload 2 Going off this we can come up with the rule for malloc refs: each port holds a malloc ref to it's parent mstb, and each mstb holds a malloc ref to it's parent port (if there is one). So, it's just the reverse of the topology ref rule. Now just add refs for payloads and other stuff, and we're good. Hooray! Now I can also use this in any docs I write too :) > -Daniel > > > > > > -Daniel > > > > > > > > > > > Piles of comments below. > > > > > > > > > > > > Cheers, Daniel > > > > > > > > > > > > > - Use the small changes to drm_dp_put_port() to add even more > > > > > > > error > > > > > > >checking to make misusage of the helpers more obvious. I > > > > > > > added > > > > > > > this > > > > > > >after having to chase down various use-after-free conditions > > > > > > > that > > > > > > >started popping up from the new helpers so no one else has to > > > > > > >troubleshoot that. > > > > > > > - Move some accidental DRM_DEBUG_KMS() calls to > > > > > > > DRM_DEBUG_ATOMIC() > > > > > > > - Update documentation again, note that find/release() should > > > > > > > both > > > > > > > not > > > > > > > be > > > > > > >called on the same port in a single atomic check phase (but > > > > > > > multiple > > > > > > >calls to one or the other is OK) > > > > > > > > > > > > > > Changes since v4: > > > > > > > - Don't skip the atomic checks for VCPI allocations if no new > > > > > > > VCPI > > > > > > >allocations happen in a state. This makes the next change I'm > > > > > > > about > > > > > > >to list here a lot easier to implement. > > > > > > > - Don't ignore VCPI allocations on destroyed ports, instead > > > > > > > ensure > > > > > > > that > > > > > > >when ports are destroyed and still have VCPI allocations in > > > > > > > the > > > > > > >topology state, the only state changes allowed are
Re: [Nouveau] 4.20.0-rc3 nouveau/Quadro P2000 Mobile: runpm causing ACPI errors, lockups
this was already debugged and there is no point in searching inside
the Firmware. It's not a firmware bug or anything.
The proper fix is to do something inside Nouveau so that we don't
upset the device and being able to runtime resume it again.
The initial thing we do inside Nouveau to cause those issues is to run
that so called "DEVINIT" script inside the vbios to initialize the
GPU, problem is, it changes something on the PCIe configuration so
that the GPU isn't able to runtime resume anymore. I am in contact
with Nvidia about that issue and hopefully we get the proper answers.
When I was digging into that myself I was able to make the situation
more stable by setting the PCIE link speed to the boot defaults, but
that was still pretty unstable.
Anyway, because the binary driver fails here as well (through
bumblebee and so on) there isn't much of reverse engineering we can do
besides guessing and trying it on literally every hardware until it
works.
We also have an upstream bug for this issue:
https://bugzilla.kernel.org/show_bug.cgi?id=156341
On Wed, Nov 28, 2018 at 9:30 PM Michael S. Tsirkin wrote:
>
> On Wed, Nov 28, 2018 at 05:55:44PM +0200, Mika Westerberg wrote:
> > On Wed, Nov 28, 2018 at 10:09:22AM -0500, Michael S. Tsirkin wrote:
> > > Yea all this is weird, in particular I wonder why does everyone
> > > using dsm insists on saying Arg4
> > > when they actually mean Arg3. ACPI numbers arguments from 0.
> > >
> > > So it's a bit ugly, and maybe worth fixing but unlikely to be
> > > an actual issue simply because we end up not using DSM in the end.
> >
> > I agree.
> >
> > > Poking at the probing code in nouveau_pr3_present, I started to wonder:
> > > should I try to hack it to disable d3cold and pr3 and see what
> > > happens?
> >
> > I guess it is worth a try. You can do it from sysfs for the graphics
> > PCI device there is an attribute d3cold_allowed that controls this.
> >
> > [snip]
>
> But probably too late by time nouveau is up at boot?
>
> > > > > 00:14.3 Network controller: Intel Corporation Wireless-AC 9560
> > > > > [Jefferson Peak] (rev 10)
> > > > >
> > > > > so really shouldn't be affected, but go figure. If driver really is
> > > > > getting
> > > > > all-ones from the device, it just might try to poke at a wrong b:d.f
> > > > > by mistake
> > > > > maybe ...
> > > >
> > > > Or it the power resource is shared by wifi as well.
> > >
> > > Is there a way to find out through e.g. sysfs?
> >
> > It is not shared, I checked from the acpidump you provided. Possibly the
> > infinite loop in AML when executing NVPO method have some effect on
> > this.
> >
> > [snip]
> >
> > > > No need to send, I can read it from the bugzilla just fine. Can you
> > > > attach
> > > > acpidump there as well?
> > >
> > > Done. lspci -x too just in case.
> >
> > Looking at the dmesg:
> >
> > [ 52.917009] No Local Variables are initialized for Method [NVPO]
> > [ 52.917011] No Arguments are initialized for method [NVPO]
> > [ 52.917012] ACPI Error: Method parse/execution failed
> > \_SB.PCI0.PEG0.PEGP.NVPO, AE_AML_LOOP_TIMEOUT (20181003/psparse-516)
> > [ 52.917063] ACPI Error: Method parse/execution failed \_SB.PCI0.PGON,
> > AE_AML_LOOP_TIMEOUT (20181003/psparse-516)
> > [ 52.917084] ACPI Error: Method parse/execution failed
> > \_SB.PCI0.PEG0.PG00._ON, AE_AML_LOOP_TIMEOUT (20181003/psparse-516)
> >
> > So what happens here is that Linux turns off power resource
> > \_SB.PCI0.PEG0.PG00 by calling its _OFF method (happens when the root
> > port is runtime suspended). This ends up calling \_SB.PCI0.PGON which
> > calls \_SB.PCI0.PEG0.PEGP.NVPO.
> >
> > The last method looks like this:
> >
> >Method (NVPO, 0, NotSerialized)
> > {
> > While ((\_SB.PCI0.P0LS < 0x03))
> > {
> > Sleep (One)
> > }
> >
> > So basically it polls P0LS register infinitely if the returned value is
> > less than 3. I suspect this is the issue and it then makes the other
> > like wifi to fail to execute its methods.
> >
> > P0LS comes from this operation region:
> >
> > OperationRegion (OPG0, SystemMemory, (XBAS + 0x8000), 0x1000)
> > Field (OPG0, AnyAcc, NoLock, Preserve)
> > {
> > ...
> > Offset (0x216),
> > P0LS, 4,
> >
> > This is some host bridge register but not sure which because XBAS value
> > cannot be determined from the acpidump.
>
> Oh I think XBAS is in SSDT4:
>
> OperationRegion (SANV, SystemMemory, 0x4FBF7018, 0x01F4)
> Field (SANV, AnyAcc, Lock, Preserve)
> {
> ASLB, 32,
> IMON, 8,
> IGDS, 8,
> IBTT, 8,
> IPAT, 8,
> IPSC, 8,
> IBIA, 8,
> ISSC, 8,
> IDMS, 8,
> IF1E, 8,
> HVCO, 8,
> GSMI, 8,
> PAVP, 8,
> CADL, 8,
> CSTE, 16,
> NSTE, 16,
> NDID, 8,
> DID1, 32,
> DID2, 32,
>
Re: [Nouveau] 4.20.0-rc3 nouveau/Quadro P2000 Mobile: runpm causing ACPI errors, lockups
On Wed, Nov 28, 2018 at 05:55:44PM +0200, Mika Westerberg wrote:
> On Wed, Nov 28, 2018 at 10:09:22AM -0500, Michael S. Tsirkin wrote:
> > Yea all this is weird, in particular I wonder why does everyone
> > using dsm insists on saying Arg4
> > when they actually mean Arg3. ACPI numbers arguments from 0.
> >
> > So it's a bit ugly, and maybe worth fixing but unlikely to be
> > an actual issue simply because we end up not using DSM in the end.
>
> I agree.
>
> > Poking at the probing code in nouveau_pr3_present, I started to wonder:
> > should I try to hack it to disable d3cold and pr3 and see what
> > happens?
>
> I guess it is worth a try. You can do it from sysfs for the graphics
> PCI device there is an attribute d3cold_allowed that controls this.
>
> [snip]
But probably too late by time nouveau is up at boot?
> > > > 00:14.3 Network controller: Intel Corporation Wireless-AC 9560
> > > > [Jefferson Peak] (rev 10)
> > > >
> > > > so really shouldn't be affected, but go figure. If driver really is
> > > > getting
> > > > all-ones from the device, it just might try to poke at a wrong b:d.f by
> > > > mistake
> > > > maybe ...
> > >
> > > Or it the power resource is shared by wifi as well.
> >
> > Is there a way to find out through e.g. sysfs?
>
> It is not shared, I checked from the acpidump you provided. Possibly the
> infinite loop in AML when executing NVPO method have some effect on
> this.
>
> [snip]
>
> > > No need to send, I can read it from the bugzilla just fine. Can you attach
> > > acpidump there as well?
> >
> > Done. lspci -x too just in case.
>
> Looking at the dmesg:
>
> [ 52.917009] No Local Variables are initialized for Method [NVPO]
> [ 52.917011] No Arguments are initialized for method [NVPO]
> [ 52.917012] ACPI Error: Method parse/execution failed
> \_SB.PCI0.PEG0.PEGP.NVPO, AE_AML_LOOP_TIMEOUT (20181003/psparse-516)
> [ 52.917063] ACPI Error: Method parse/execution failed \_SB.PCI0.PGON,
> AE_AML_LOOP_TIMEOUT (20181003/psparse-516)
> [ 52.917084] ACPI Error: Method parse/execution failed
> \_SB.PCI0.PEG0.PG00._ON, AE_AML_LOOP_TIMEOUT (20181003/psparse-516)
>
> So what happens here is that Linux turns off power resource
> \_SB.PCI0.PEG0.PG00 by calling its _OFF method (happens when the root
> port is runtime suspended). This ends up calling \_SB.PCI0.PGON which
> calls \_SB.PCI0.PEG0.PEGP.NVPO.
>
> The last method looks like this:
>
>Method (NVPO, 0, NotSerialized)
> {
> While ((\_SB.PCI0.P0LS < 0x03))
> {
> Sleep (One)
> }
>
> So basically it polls P0LS register infinitely if the returned value is
> less than 3. I suspect this is the issue and it then makes the other
> like wifi to fail to execute its methods.
>
> P0LS comes from this operation region:
>
> OperationRegion (OPG0, SystemMemory, (XBAS + 0x8000), 0x1000)
> Field (OPG0, AnyAcc, NoLock, Preserve)
> {
> ...
> Offset (0x216),
> P0LS, 4,
>
> This is some host bridge register but not sure which because XBAS value
> cannot be determined from the acpidump.
Oh I think XBAS is in SSDT4:
OperationRegion (SANV, SystemMemory, 0x4FBF7018, 0x01F4)
Field (SANV, AnyAcc, Lock, Preserve)
{
ASLB, 32,
IMON, 8,
IGDS, 8,
IBTT, 8,
IPAT, 8,
IPSC, 8,
IBIA, 8,
ISSC, 8,
IDMS, 8,
IF1E, 8,
HVCO, 8,
GSMI, 8,
PAVP, 8,
CADL, 8,
CSTE, 16,
NSTE, 16,
NDID, 8,
DID1, 32,
DID2, 32,
DID3, 32,
DID4, 32,
DID5, 32,
DID6, 32,
DID7, 32,
DID8, 32,
DID9, 32,
DIDA, 32,
DIDB, 32,
DIDC, 32,
DIDD, 32,
DIDE, 32,
DIDF, 32,
DIDX, 32,
NXD1, 32,
NXD2, 32,
NXD3, 32,
NXD4, 32,
NXD5, 32,
NXD6, 32,
NXD7, 32,
NXD8, 32,
NXDX, 32,
LIDS, 8,
KSV0, 32,
KSV1, 8,
BRTL, 8,
ALSE, 8,
ALAF, 8,
LLOW, 8,
LHIH, 8,
ALFP, 8,
IPTP, 8,
EDPV, 8,
SGMD, 8,
SGFL, 8,
SGGP, 8,
HRE0, 8,
HRG0, 32,
HRA0, 8,
PWE0, 8,
PWG0, 32,
PWA0, 8,
P1GP, 8,
HRE1, 8,
HRG1, 32,
HRA1, 8,
PWE1, 8,
PWG1, 32,
PWA1, 8,
P2GP, 8,
HRE2, 8,
HRG2, 32,
HRA2, 8,
PWE2, 8,
PWG2, 32,
PWA2, 8,
DLPW, 16,
DLHR, 16,
EECP, 8,
XBAS, 32,
GBAS, 16,
NVGA, 32,
NVHA, 32,
Re: [Nouveau] [PATCH v6 3/6] drm/dp_mst: Start tracking per-port VCPI allocations
On Wed, 2018-11-28 at 09:17 +0100, Daniel Vetter wrote: > On Tue, Nov 27, 2018 at 08:44:14PM -0500, Lyude Paul wrote: > > On Tue, 2018-11-27 at 20:44 +0100, Daniel Vetter wrote: > > > On Tue, Nov 27, 2018 at 12:48:59PM -0500, Lyude Paul wrote: > > > > On Mon, 2018-11-26 at 22:22 +0100, Daniel Vetter wrote: > > > > > On Mon, Nov 26, 2018 at 10:04:21PM +0100, Daniel Vetter wrote: > > > > > > On Thu, Nov 15, 2018 at 07:50:05PM -0500, Lyude Paul wrote: > > > > > > > There has been a TODO waiting for quite a long time in > > > > > > > drm_dp_mst_topology.c: > > > > > > > > > > > > > > /* We cannot rely on port->vcpi.num_slots to update > > > > > > >* topology_state->avail_slots as the port may not exist if > > > > > > > the parent > > > > > > >* branch device was unplugged. This should be fixed by > > > > > > > tracking > > > > > > >* per-port slot allocation in drm_dp_mst_topology_state > > > > > > > instead of > > > > > > >* depending on the caller to tell us how many slots to > > > > > > > release. > > > > > > >*/ > > > > > > > > > > > > > > That's not the only reason we should fix this: forcing the > > > > > > > driver to > > > > > > > track the VCPI allocations throughout a state's atomic check is > > > > > > > error prone, because it means that extra care has to be taken > > > > > > > with > > > > > > > the > > > > > > > order that drm_dp_atomic_find_vcpi_slots() and > > > > > > > drm_dp_atomic_release_vcpi_slots() are called in in order to > > > > > > > ensure > > > > > > > idempotency. Currently the only driver actually using these > > > > > > > helpers, > > > > > > > i915, doesn't even do this correctly: multiple ->best_encoder() > > > > > > > checks > > > > > > > with i915's current implementation would not be idempotent and > > > > > > > would > > > > > > > over-allocate VCPI slots, something I learned trying to > > > > > > > implement > > > > > > > fallback retraining in MST. > > > > > > > > > > > > > > So: simplify this whole mess, and teach > > > > > > > drm_dp_atomic_find_vcpi_slots() > > > > > > > and drm_dp_atomic_release_vcpi_slots() to track the VCPI > > > > > > > allocations > > > > > > > for > > > > > > > each port. This allows us to ensure idempotency without having > > > > > > > to > > > > > > > rely > > > > > > > on the driver as much. Additionally: the driver doesn't need to > > > > > > > do > > > > > > > any > > > > > > > kind of VCPI slot tracking anymore if it doesn't need it for > > > > > > > it's > > > > > > > own > > > > > > > internal state. > > > > > > > > > > > > > > Additionally; this adds a new drm_dp_mst_atomic_check() helper > > > > > > > which > > > > > > > must be used by atomic drivers to perform validity checks for > > > > > > > the > > > > > > > new > > > > > > > VCPI allocations incurred by a state. > > > > > > > > > > > > > > Also: update the documentation and make it more obvious that > > > > > > > these > > > > > > > /must/ be called by /all/ atomic drivers supporting MST. > > > > > > > > > > > > > > Changes since v6: > > > > > > > - Keep a kref to all of the ports we have allocations on. This > > > > > > > required > > > > > > >a good bit of changing to when we call > > > > > > > drm_dp_find_vcpi_slots(), > > > > > > >mainly that we need to ensure that we only redo VCPI > > > > > > > allocations > > > > > > > on > > > > > > >actual mode or CRTC changes, not crtc_state->active changes. > > > > > > >Additionally, we no longer take the registration of the DRM > > > > > > > connector > > > > > > >for each port into account because so long as we have a kref > > > > > > > to > > > > > > > the > > > > > > >port in the new or previous atomic state, the connector will > > > > > > > stay > > > > > > >registered. > > > > > > > > > > > > I write an entire pile of small nitpits (still included most of > > > > > > them > > > > > > below), until I realized this here wont work. Delaying the call to > > > > > > destroy > > > > > > the connector (well, unregister it really) wreaks the design we've > > > > > > come up > > > > > > with thus far, resulting in most of my comments here. > > > > > > > > > > > > Instead, all we need to do is delay the kfree(port) at the bottom > > > > > > of > > > > > > drm_dp_destroy_port(). The vcpi allocation structure _only_ needs > > > > > > the > > > > > > pointer value to stay valid, as a lookup key. It doesn't care at > > > > > > all > > > > > > about > > > > > > anything actually stored in there. So the only thing we need to > > > > > > delay > > > > > > is > > > > > > the kfree. I think the simplest way to achieve this is to add a > > > > > > 2nd > > > > > > kref > > > > > > (port->kfree_ref or something like that), with on reference held > > > > > > by > > > > > > the > > > > > > port itself (released in drm_dp_destroy_port), and the other one > > > > > > held > > > > > > as-needed by the vcpi allocation lists. > > > > > > > > > > > > I think if we go with this design instead of retrof
Re: [Nouveau] [PATCH v3 1/3] drm/connector: Add generic underscan properties
Brian Starkey writes: > Hi Boris, > > Just because I happened to read the docs in here, one typo below: > > On Thu, Nov 22, 2018 at 12:23:29PM +0100, Boris Brezillon wrote: >>diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c >>index c555e17ab8d7..170317248da6 100644 >>--- a/drivers/gpu/drm/drm_connector.c >>+++ b/drivers/gpu/drm/drm_connector.c >>@@ -971,6 +971,38 @@ DRM_ENUM_NAME_FN(drm_get_content_protection_name, >>drm_cp_enum_list) >> * can also expose this property to external outputs, in which case they >> * must support "None", which should be the default (since external screens >> * have a built-in scaler). >>+ * >>+ * Connectors for non-analog outputs may also have standardized underscan >>+ * properties (drivers can set this up by calling >>+ * drm_connector_attach_content_protection_property() on initialization): > > Should be drm_connector_attach_underscan_properties() Other than this typo, this series is: Reviewed-by: Eric Anholt signature.asc Description: PGP signature ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [Bug 108857] display becomes unresponsive and keyboard input fails
https://bugs.freedesktop.org/show_bug.cgi?id=108857 --- Comment #4 from tla2...@gmail.com --- Created attachment 142650 --> https://bugs.freedesktop.org/attachment.cgi?id=142650&action=edit dmesg from 4.19.4-300.fc29.x86_64 -- You are receiving this mail because: You are the assignee for the bug.___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [Bug 108857] display becomes unresponsive and keyboard input fails
https://bugs.freedesktop.org/show_bug.cgi?id=108857 --- Comment #3 from tla2...@gmail.com --- Updated to latest Fedora 29 kernel today and the problem is still evident. Linux s0.home 4.19.4-300.fc29.x86_64 #1 SMP Fri Nov 23 13:03:11 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux -- You are receiving this mail because: You are the assignee for the bug.___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] 4.20.0-rc3 nouveau/Quadro P2000 Mobile: runpm causing ACPI errors, lockups
On Wed, Nov 28, 2018 at 10:09:22AM -0500, Michael S. Tsirkin wrote:
> Yea all this is weird, in particular I wonder why does everyone
> using dsm insists on saying Arg4
> when they actually mean Arg3. ACPI numbers arguments from 0.
>
> So it's a bit ugly, and maybe worth fixing but unlikely to be
> an actual issue simply because we end up not using DSM in the end.
I agree.
> Poking at the probing code in nouveau_pr3_present, I started to wonder:
> should I try to hack it to disable d3cold and pr3 and see what
> happens?
I guess it is worth a try. You can do it from sysfs for the graphics
PCI device there is an attribute d3cold_allowed that controls this.
[snip]
> > > 00:14.3 Network controller: Intel Corporation Wireless-AC 9560 [Jefferson
> > > Peak] (rev 10)
> > >
> > > so really shouldn't be affected, but go figure. If driver really is
> > > getting
> > > all-ones from the device, it just might try to poke at a wrong b:d.f by
> > > mistake
> > > maybe ...
> >
> > Or it the power resource is shared by wifi as well.
>
> Is there a way to find out through e.g. sysfs?
It is not shared, I checked from the acpidump you provided. Possibly the
infinite loop in AML when executing NVPO method have some effect on
this.
[snip]
> > No need to send, I can read it from the bugzilla just fine. Can you attach
> > acpidump there as well?
>
> Done. lspci -x too just in case.
Looking at the dmesg:
[ 52.917009] No Local Variables are initialized for Method [NVPO]
[ 52.917011] No Arguments are initialized for method [NVPO]
[ 52.917012] ACPI Error: Method parse/execution failed
\_SB.PCI0.PEG0.PEGP.NVPO, AE_AML_LOOP_TIMEOUT (20181003/psparse-516)
[ 52.917063] ACPI Error: Method parse/execution failed \_SB.PCI0.PGON,
AE_AML_LOOP_TIMEOUT (20181003/psparse-516)
[ 52.917084] ACPI Error: Method parse/execution failed
\_SB.PCI0.PEG0.PG00._ON, AE_AML_LOOP_TIMEOUT (20181003/psparse-516)
So what happens here is that Linux turns off power resource
\_SB.PCI0.PEG0.PG00 by calling its _OFF method (happens when the root
port is runtime suspended). This ends up calling \_SB.PCI0.PGON which
calls \_SB.PCI0.PEG0.PEGP.NVPO.
The last method looks like this:
Method (NVPO, 0, NotSerialized)
{
While ((\_SB.PCI0.P0LS < 0x03))
{
Sleep (One)
}
So basically it polls P0LS register infinitely if the returned value is
less than 3. I suspect this is the issue and it then makes the other
like wifi to fail to execute its methods.
P0LS comes from this operation region:
OperationRegion (OPG0, SystemMemory, (XBAS + 0x8000), 0x1000)
Field (OPG0, AnyAcc, NoLock, Preserve)
{
...
Offset (0x216),
P0LS, 4,
This is some host bridge register but not sure which because XBAS value
cannot be determined from the acpidump.
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH] drm/fbdev: Make skip_vt_switch the default
Am Dienstag, 27. November 2018, 18:34:24 CET schrieb Daniel Vetter: > KMS drivers really should all be able to restore their display state > on resume without fbcon helping out. So make this the default. > > Since I'm not entirely foolish, make it only a default, which drivers > can still override. That way when the inevitable regression report > happens I can fix things up with a one-liner plus FIXME comment that > someone should fix up the suspend/resume code in that driver. > > But at least all new drivers won't be broken by accident as soon as > you turn off fbcon because "suspend/resume worked when I tested it". > > v2: Keep this for radeon because of > > commit 18c437caa5b18a235dd65cec224eab54bebcee65 > Author: Alex Deucher > Date: Tue Nov 14 17:19:29 2017 -0500 > > Revert "drm/radeon: dont switch vt on suspend" > > Thanks to Michel Dänzer for pointing this one out. > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c > b/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c index > e6650553f5d6..361604e51361 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c > @@ -111,8 +111,6 @@ static int rockchip_drm_fbdev_create(struct > drm_fb_helper *helper, rk_obj->kvaddr, > offset, size); > > - fbi->skip_vt_switch = true; > - > return 0; > > out: for the Rockchip-part Acked-by: Heiko Stuebner It looks somewhat obvious for that, as the Rockchip setting was true from the beginning, but I still gave it some suspend-spins on rk3399 so as well Tested-by: Heiko Stuebner ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] 4.20.0-rc3 nouveau/Quadro P2000 Mobile: runpm causing ACPI errors, lockups
On Wed, Nov 28, 2018 at 01:08:57PM +0200, Mika Westerberg wrote:
> On Tue, Nov 27, 2018 at 09:49:44PM -0500, Michael S. Tsirkin wrote:
> > On Tue, Nov 27, 2018 at 11:36:50AM +0200, Mika Westerberg wrote:
> > > +linux-acpi
> > >
> > > Hi Michael,
> > >
> > > On Mon, Nov 26, 2018 at 10:53:26PM -0500, Michael S. Tsirkin wrote:
> > > > So a new thinkpad:
> > > > 01:00.0 VGA compatible controller: NVIDIA Corporation GP107GLM [Quadro
> > > > P2000 Mobile] (rev a1)
> > > >
> > > > Hangs whenever I try to poke at the card. It starts happily enough with
> > > >
> > > > [3.971515] ACPI Warning: \_SB.PCI0.GFX0._DSM: Argument #4 type
> > > > mismatch - Found [Buffer], ACPI requires [Package]
> > > > (20181003/nsarguments-66)
> > > > [3.971553] ACPI Warning: \_SB.PCI0.PEG0.PEGP._DSM: Argument #4 type
> > > > mismatch - Found [Buffer], ACPI requires [Package]
> > > > (20181003/nsarguments-66)
> > > > [3.971721] pci :01:00.0: optimus capabilities: enabled, status
> > > > dynamic power, hda bios codec supported
> > > > [3.971726] VGA switcheroo: detected Optimus DSM method
> > > > \_SB_.PCI0.PEG0.PEGP handle
> > > > [3.971727] nouveau: detected PR support, will not use DSM
> >
> > BTW this is also a bit strange. It says it will not use DSM - but did
> > it maybe use DSM previously? The ACPI Warning seems to indicate that
> > it did ...
> >
> > And just to complete the picture here's the _DSM from ACPI:
> >
> > Method (_DSM, 4, Serialized) // _DSM: Device-Specific Method
> > {
>
> ...
>
> > }
> >
> >
> > I am not sure what makes it think that Arg3 is a buffer and
> > not a package: IIRC Index (decoded C-style here as []) can apply
> > to either.
> >
> > Am I maybe misunderstanding the warning?
>
> It looks like coming from the nouveau driver (assuming I'm reading it right).
>
> drivers/gpu/drm/nouveau/nouveau_acpi.c::nouveau_optimus_dsm()
>
> union acpi_object argv4 = {
> .buffer.type = ACPI_TYPE_BUFFER,
> .buffer.length = 4,
> .buffer.pointer = args_buff
> };
>
> ...
>
> obj = acpi_evaluate_dsm_typed(handle, &nouveau_op_dsm_muid,
> 0x0100,
> func, &argv4, ACPI_TYPE_BUFFER);
>
>
> It passes ACPI_TYPE_BUFFER but ACPI spec _DSM expects package.
Yea all this is weird, in particular I wonder why does everyone
using dsm insists on saying Arg4
when they actually mean Arg3. ACPI numbers arguments from 0.
So it's a bit ugly, and maybe worth fixing but unlikely to be
an actual issue simply because we end up not using DSM in the end.
Poking at the probing code in nouveau_pr3_present, I started to wonder:
should I try to hack it to disable d3cold and pr3 and see what
happens?
> > > > [3.971745] nouveau :01:00.0: enabling device (0006 -> 0007)
> > > > [3.971923] nouveau :01:00.0: NVIDIA GP107 (137000a1)
> > > > [4.009875] PM: Image not found (code -22)
> > > > [4.135752] nouveau :01:00.0: DRM: VRAM: 4096 MiB
> > > > [4.135753] nouveau :01:00.0: DRM: GART: 536870912 MiB
> > > > [4.135754] nouveau :01:00.0: DRM: BIT table 'A' not found
> > > > [4.135755] nouveau :01:00.0: DRM: BIT table 'L' not found
> > > > [4.135756] nouveau :01:00.0: DRM: TMDS table version 2.0
> > > > [4.135756] nouveau :01:00.0: DRM: DCB version 4.1
> > > > [4.135757] nouveau :01:00.0: DRM: DCB outp 00: 02800f76 04600020
> > > > [4.135758] nouveau :01:00.0: DRM: DCB outp 01: 02011f62 00020010
> > > > [4.135759] nouveau :01:00.0: DRM: DCB outp 02: 01022f46 04600010
> > > > [4.135760] nouveau :01:00.0: DRM: DCB outp 03: 01033f56 04600020
> > > > [4.135761] nouveau :01:00.0: DRM: DCB conn 00: 00020047
> > > > [4.135761] nouveau :01:00.0: DRM: DCB conn 01: 00010161
> > > > [4.135762] nouveau :01:00.0: DRM: DCB conn 02: 1246
> > > > [4.135763] nouveau :01:00.0: DRM: DCB conn 03: 2346
> > > > [4.508355] [drm] Supports vblank timestamp caching Rev 2
> > > > (21.10.2013).
> > > > [4.508355] [drm] Driver supports precise vblank timestamp query.
> > > > [4.509812] [drm] Cannot find any crtc or sizes
> > > > [4.510144] [drm] Initialized nouveau 1.3.1 20120801 for
> > > > :01:00.0 on minor 2
> > > >
> > > >
> > > > Although that type mismatch is a bit worrying. And I'm not sure what
> > > > prints PM: Image not found.
> > >
> > > That is fine, it just says it cannot find a hibernation image from swap
> > > device. I guess you have resume=... in the kernel command line.
> > >
> > > > But after a short while it gets pretty busy:
> > > >
> > > >
> > > > [ 52.917009] No Local Variables are initialized for Method [NVPO]
> > > > [ 52.917011] No Arguments are initialized for method [NVPO]
> > > > [ 52.917012] ACPI Error: Method parse/execution failed
> > > > \_SB.PCI0.PEG0.PEGP.NVPO, AE_AML_LOOP_TIMEOUT (201810
[Nouveau] [Bug 108873] nouveau/Quadro P2000 Mobile: runpm causing ACPI errors, lockups
https://bugs.freedesktop.org/show_bug.cgi?id=108873 --- Comment #2 from m...@redhat.com --- Created attachment 142647 --> https://bugs.freedesktop.org/attachment.cgi?id=142647&action=edit output of lspci -x -- You are receiving this mail because: You are the assignee for the bug.___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [Bug 108873] nouveau/Quadro P2000 Mobile: runpm causing ACPI errors, lockups
https://bugs.freedesktop.org/show_bug.cgi?id=108873 --- Comment #1 from m...@redhat.com --- Created attachment 142646 --> https://bugs.freedesktop.org/attachment.cgi?id=142646&action=edit acpi dump from the machine -- You are receiving this mail because: You are the assignee for the bug.___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] 4.20.0-rc3 nouveau/Quadro P2000 Mobile: runpm causing ACPI errors, lockups
On Tue, Nov 27, 2018 at 09:49:44PM -0500, Michael S. Tsirkin wrote:
> On Tue, Nov 27, 2018 at 11:36:50AM +0200, Mika Westerberg wrote:
> > +linux-acpi
> >
> > Hi Michael,
> >
> > On Mon, Nov 26, 2018 at 10:53:26PM -0500, Michael S. Tsirkin wrote:
> > > So a new thinkpad:
> > > 01:00.0 VGA compatible controller: NVIDIA Corporation GP107GLM [Quadro
> > > P2000 Mobile] (rev a1)
> > >
> > > Hangs whenever I try to poke at the card. It starts happily enough with
> > >
> > > [3.971515] ACPI Warning: \_SB.PCI0.GFX0._DSM: Argument #4 type
> > > mismatch - Found [Buffer], ACPI requires [Package]
> > > (20181003/nsarguments-66)
> > > [3.971553] ACPI Warning: \_SB.PCI0.PEG0.PEGP._DSM: Argument #4 type
> > > mismatch - Found [Buffer], ACPI requires [Package]
> > > (20181003/nsarguments-66)
> > > [3.971721] pci :01:00.0: optimus capabilities: enabled, status
> > > dynamic power, hda bios codec supported
> > > [3.971726] VGA switcheroo: detected Optimus DSM method
> > > \_SB_.PCI0.PEG0.PEGP handle
> > > [3.971727] nouveau: detected PR support, will not use DSM
>
> BTW this is also a bit strange. It says it will not use DSM - but did
> it maybe use DSM previously? The ACPI Warning seems to indicate that
> it did ...
>
> And just to complete the picture here's the _DSM from ACPI:
>
> Method (_DSM, 4, Serialized) // _DSM: Device-Specific Method
> {
...
> }
>
>
> I am not sure what makes it think that Arg3 is a buffer and
> not a package: IIRC Index (decoded C-style here as []) can apply
> to either.
>
> Am I maybe misunderstanding the warning?
It looks like coming from the nouveau driver (assuming I'm reading it right).
drivers/gpu/drm/nouveau/nouveau_acpi.c::nouveau_optimus_dsm()
union acpi_object argv4 = {
.buffer.type = ACPI_TYPE_BUFFER,
.buffer.length = 4,
.buffer.pointer = args_buff
};
...
obj = acpi_evaluate_dsm_typed(handle, &nouveau_op_dsm_muid, 0x0100,
func, &argv4, ACPI_TYPE_BUFFER);
It passes ACPI_TYPE_BUFFER but ACPI spec _DSM expects package.
> > > [3.971745] nouveau :01:00.0: enabling device (0006 -> 0007)
> > > [3.971923] nouveau :01:00.0: NVIDIA GP107 (137000a1)
> > > [4.009875] PM: Image not found (code -22)
> > > [4.135752] nouveau :01:00.0: DRM: VRAM: 4096 MiB
> > > [4.135753] nouveau :01:00.0: DRM: GART: 536870912 MiB
> > > [4.135754] nouveau :01:00.0: DRM: BIT table 'A' not found
> > > [4.135755] nouveau :01:00.0: DRM: BIT table 'L' not found
> > > [4.135756] nouveau :01:00.0: DRM: TMDS table version 2.0
> > > [4.135756] nouveau :01:00.0: DRM: DCB version 4.1
> > > [4.135757] nouveau :01:00.0: DRM: DCB outp 00: 02800f76 04600020
> > > [4.135758] nouveau :01:00.0: DRM: DCB outp 01: 02011f62 00020010
> > > [4.135759] nouveau :01:00.0: DRM: DCB outp 02: 01022f46 04600010
> > > [4.135760] nouveau :01:00.0: DRM: DCB outp 03: 01033f56 04600020
> > > [4.135761] nouveau :01:00.0: DRM: DCB conn 00: 00020047
> > > [4.135761] nouveau :01:00.0: DRM: DCB conn 01: 00010161
> > > [4.135762] nouveau :01:00.0: DRM: DCB conn 02: 1246
> > > [4.135763] nouveau :01:00.0: DRM: DCB conn 03: 2346
> > > [4.508355] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
> > > [4.508355] [drm] Driver supports precise vblank timestamp query.
> > > [4.509812] [drm] Cannot find any crtc or sizes
> > > [4.510144] [drm] Initialized nouveau 1.3.1 20120801 for :01:00.0
> > > on minor 2
> > >
> > >
> > > Although that type mismatch is a bit worrying. And I'm not sure what
> > > prints PM: Image not found.
> >
> > That is fine, it just says it cannot find a hibernation image from swap
> > device. I guess you have resume=... in the kernel command line.
> >
> > > But after a short while it gets pretty busy:
> > >
> > >
> > > [ 52.917009] No Local Variables are initialized for Method [NVPO]
> > > [ 52.917011] No Arguments are initialized for method [NVPO]
> > > [ 52.917012] ACPI Error: Method parse/execution failed
> > > \_SB.PCI0.PEG0.PEGP.NVPO, AE_AML_LOOP_TIMEOUT (20181003/psparse-516)
> > > [ 52.917063] ACPI Error: Method parse/execution failed \_SB.PCI0.PGON,
> > > AE_AML_LOOP_TIMEOUT (20181003/psparse-516)
> > > [ 52.917084] ACPI Error: Method parse/execution failed
> > > \_SB.PCI0.PEG0.PG00._ON, AE_AML_LOOP_TIMEOUT (20181003/psparse-516)
> > > [ 52.917108] acpi device:00: Failed to change power state to D0
> >
> > Here it seems to fail to turn on the power resource for the device.
> >
> > > [ 52.969287] video LNXVIDEO:00: Cannot transition to power state D0 for
> > > parent in (unknown)
> > > [ 52.969289] pci_raw_set_power_state: 2 callbacks suppressed
> > > [ 52.969291] nouveau :01:00.0: Refused to change power state,
> > > currently in D3
> >
Re: [Nouveau] [PATCH v6 3/6] drm/dp_mst: Start tracking per-port VCPI allocations
On Tue, Nov 27, 2018 at 08:44:14PM -0500, Lyude Paul wrote: > On Tue, 2018-11-27 at 20:44 +0100, Daniel Vetter wrote: > > On Tue, Nov 27, 2018 at 12:48:59PM -0500, Lyude Paul wrote: > > > On Mon, 2018-11-26 at 22:22 +0100, Daniel Vetter wrote: > > > > On Mon, Nov 26, 2018 at 10:04:21PM +0100, Daniel Vetter wrote: > > > > > On Thu, Nov 15, 2018 at 07:50:05PM -0500, Lyude Paul wrote: > > > > > > There has been a TODO waiting for quite a long time in > > > > > > drm_dp_mst_topology.c: > > > > > > > > > > > > /* We cannot rely on port->vcpi.num_slots to update > > > > > > * topology_state->avail_slots as the port may not exist if > > > > > > the parent > > > > > > * branch device was unplugged. This should be fixed by > > > > > > tracking > > > > > > * per-port slot allocation in drm_dp_mst_topology_state > > > > > > instead of > > > > > > * depending on the caller to tell us how many slots to > > > > > > release. > > > > > > */ > > > > > > > > > > > > That's not the only reason we should fix this: forcing the driver to > > > > > > track the VCPI allocations throughout a state's atomic check is > > > > > > error prone, because it means that extra care has to be taken with > > > > > > the > > > > > > order that drm_dp_atomic_find_vcpi_slots() and > > > > > > drm_dp_atomic_release_vcpi_slots() are called in in order to ensure > > > > > > idempotency. Currently the only driver actually using these helpers, > > > > > > i915, doesn't even do this correctly: multiple ->best_encoder() > > > > > > checks > > > > > > with i915's current implementation would not be idempotent and would > > > > > > over-allocate VCPI slots, something I learned trying to implement > > > > > > fallback retraining in MST. > > > > > > > > > > > > So: simplify this whole mess, and teach > > > > > > drm_dp_atomic_find_vcpi_slots() > > > > > > and drm_dp_atomic_release_vcpi_slots() to track the VCPI allocations > > > > > > for > > > > > > each port. This allows us to ensure idempotency without having to > > > > > > rely > > > > > > on the driver as much. Additionally: the driver doesn't need to do > > > > > > any > > > > > > kind of VCPI slot tracking anymore if it doesn't need it for it's > > > > > > own > > > > > > internal state. > > > > > > > > > > > > Additionally; this adds a new drm_dp_mst_atomic_check() helper which > > > > > > must be used by atomic drivers to perform validity checks for the > > > > > > new > > > > > > VCPI allocations incurred by a state. > > > > > > > > > > > > Also: update the documentation and make it more obvious that these > > > > > > /must/ be called by /all/ atomic drivers supporting MST. > > > > > > > > > > > > Changes since v6: > > > > > > - Keep a kref to all of the ports we have allocations on. This > > > > > > required > > > > > >a good bit of changing to when we call drm_dp_find_vcpi_slots(), > > > > > >mainly that we need to ensure that we only redo VCPI allocations > > > > > > on > > > > > >actual mode or CRTC changes, not crtc_state->active changes. > > > > > >Additionally, we no longer take the registration of the DRM > > > > > > connector > > > > > >for each port into account because so long as we have a kref to > > > > > > the > > > > > >port in the new or previous atomic state, the connector will stay > > > > > >registered. > > > > > > > > > > I write an entire pile of small nitpits (still included most of them > > > > > below), until I realized this here wont work. Delaying the call to > > > > > destroy > > > > > the connector (well, unregister it really) wreaks the design we've > > > > > come up > > > > > with thus far, resulting in most of my comments here. > > > > > > > > > > Instead, all we need to do is delay the kfree(port) at the bottom of > > > > > drm_dp_destroy_port(). The vcpi allocation structure _only_ needs the > > > > > pointer value to stay valid, as a lookup key. It doesn't care at all > > > > > about > > > > > anything actually stored in there. So the only thing we need to delay > > > > > is > > > > > the kfree. I think the simplest way to achieve this is to add a 2nd > > > > > kref > > > > > (port->kfree_ref or something like that), with on reference held by > > > > > the > > > > > port itself (released in drm_dp_destroy_port), and the other one held > > > > > as-needed by the vcpi allocation lists. > > > > > > > > > > I think if we go with this design instead of retrofitting a semantic > > > > > change of the port lifetime itself, all the complications I complain > > > > > about > > > > > below should disappear. > > > > > > > > In the above I meant drm_dp_destroy_port or > > > > drm_dp_destroy_connector_work. > > > > > > > > Aside: I think creating a kref for the final kfree would also solve a > > > > bunch of other issues in a much neater way: In > > > > > > > > commit f038c5b99fc1332f558b495d136d4f433ee65caa > > > > Author: Lyude Paul > > > > Date: Tue Nov 13 17:46:14 2018 -0500 > > > > > > > > drm/dp_mst: Skip
