Re: [Nouveau] [PATCH v4] vga_switcheroo: Add helper for deferred probing

2016-05-19 Thread Emil Velikov
Hi Lukas,

On 19 May 2016 at 15:39, Lukas Wunner  wrote:

> +bool vga_switcheroo_client_probe_defer(struct pci_dev *pdev)
> +{
> +   if ((pdev->class >> 8) == PCI_CLASS_DISPLAY_VGA) {
Not sure if we want/need this, yet at least. This changes behaviour
which is not what refactoring patches should be doing, right ? if
needed it ought to be a separate patch, imho.

Furthermore on nouveau and AMD (iirc) front, some dual-gpu/optimus
systems use PCI_CLASS_DISPLAY_3D. So if we add DISPLAY_VGA perhaps we
should also check for DISPLAY_3D ?

Regards,
Emil
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau


[Nouveau] [Bug 94637] system crash, no messages, GT215, ubuntu 16.04 when running glmark2 for a few minutes.

2016-05-19 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=94637

--- Comment #13 from Pierre Moreau  ---
Just that single line? That’s sad…
Which version of Mesa are you using?

What happen if you boot with `nouveau.config=NvForcePost=1` on the kernel
command line, and start Minecraft or glmark2 without doing any suspend/resume
in between booting and starting the application? Probably more far fetched, but
what happens if you reclock to a higher clock before starting the application?

* On 4.4, boot with nouveau.pstate=1 on the kernel command line, and run `echo
XX > /sys/class/drm/card0/device/pstate` as root, where XX is a performance
level; you can find the different performance level available on your card by
cat’ing that file.
* On 4.5+, run `echo XX > /sys/kernel/debug/dri/0/pstate` as root, where XX is
a performance level; you can find the different performance level available on
your card by cat’ing that file.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau


[Nouveau] [PATCH v4] vga_switcheroo: Add helper for deferred probing

2016-05-19 Thread Lukas Wunner
So far we've got one condition when DRM drivers need to defer probing
on a dual GPU system and it's coded separately into each of the relevant
drivers. As suggested by Daniel Vetter, deduplicate that code in the
drivers and move it to a new vga_switcheroo helper. This yields better
encapsulation of concepts and lets us add further checks in a central
place. (The existing check pertains to pre-retina MacBook Pros and an
additional check is expected to be needed for retinas.)

v2: This helper could eventually be used by audio clients as well,
so rephrase kerneldoc to refer to "client" instead of "GPU"
and move the single existing check in an if block specific
to PCI_CLASS_DISPLAY_VGA devices. Move documentation on
that check from kerneldoc to a comment. (Daniel Vetter)

v3: Mandate in kerneldoc that registration of client shall only
happen after calling this helper. (Daniel Vetter)

v4: Rebase on 412c8f7de011 ("drm/radeon: Return -EPROBE_DEFER when
amdkfd not loaded")

Cc: Daniel Vetter 
Cc: Ben Skeggs 
Cc: Alex Deucher 
Signed-off-by: Lukas Wunner 
---
 drivers/gpu/drm/i915/i915_drv.c   | 10 +-
 drivers/gpu/drm/nouveau/nouveau_drm.c | 10 +-
 drivers/gpu/drm/radeon/radeon_drv.c   | 10 +-
 drivers/gpu/vga/vga_switcheroo.c  | 34 --
 include/linux/vga_switcheroo.h|  2 ++
 5 files changed, 37 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index dba03c0..61bf5a9 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -35,11 +35,9 @@
 #include "i915_trace.h"
 #include "intel_drv.h"
 
-#include 
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 
@@ -1030,13 +1028,7 @@ static int i915_pci_probe(struct pci_dev *pdev, const 
struct pci_device_id *ent)
if (PCI_FUNC(pdev->devfn))
return -ENODEV;
 
-   /*
-* apple-gmux is needed on dual GPU MacBook Pro
-* to probe the panel if we're the inactive GPU.
-*/
-   if (IS_ENABLED(CONFIG_VGA_ARB) && IS_ENABLED(CONFIG_VGA_SWITCHEROO) &&
-   apple_gmux_present() && pdev != vga_default_device() &&
-   !vga_switcheroo_handler_flags())
+   if (vga_switcheroo_client_probe_defer(pdev))
return -EPROBE_DEFER;
 
return drm_get_pci_dev(pdev, ent, &driver);
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c 
b/drivers/gpu/drm/nouveau/nouveau_drm.c
index db5c7d0..a0bb1d0 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -22,13 +22,11 @@
  * Authors: Ben Skeggs
  */
 
-#include 
 #include 
 #include 
 #include 
 #include 
 #include 
-#include 
 #include 
 
 #include "drmP.h"
@@ -315,13 +313,7 @@ static int nouveau_drm_probe(struct pci_dev *pdev,
bool boot = false;
int ret;
 
-   /*
-* apple-gmux is needed on dual GPU MacBook Pro
-* to probe the panel if we're the inactive GPU.
-*/
-   if (IS_ENABLED(CONFIG_VGA_ARB) && IS_ENABLED(CONFIG_VGA_SWITCHEROO) &&
-   apple_gmux_present() && pdev != vga_default_device() &&
-   !vga_switcheroo_handler_flags())
+   if (vga_switcheroo_client_probe_defer(pdev))
return -EPROBE_DEFER;
 
/* remove conflicting drivers (vesafb, efifb etc) */
diff --git a/drivers/gpu/drm/radeon/radeon_drv.c 
b/drivers/gpu/drm/radeon/radeon_drv.c
index b55aa74..a455dc7 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -34,11 +34,9 @@
 #include "radeon_drv.h"
 
 #include 
-#include 
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 
@@ -340,13 +338,7 @@ static int radeon_pci_probe(struct pci_dev *pdev,
if (ret == -EPROBE_DEFER)
return ret;
 
-   /*
-* apple-gmux is needed on dual GPU MacBook Pro
-* to probe the panel if we're the inactive GPU.
-*/
-   if (IS_ENABLED(CONFIG_VGA_ARB) && IS_ENABLED(CONFIG_VGA_SWITCHEROO) &&
-   apple_gmux_present() && pdev != vga_default_device() &&
-   !vga_switcheroo_handler_flags())
+   if (vga_switcheroo_client_probe_defer(pdev))
return -EPROBE_DEFER;
 
/* Get rid of things like offb */
diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
index cbd7c98..101e14c 100644
--- a/drivers/gpu/vga/vga_switcheroo.c
+++ b/drivers/gpu/vga/vga_switcheroo.c
@@ -30,6 +30,7 @@
 
 #define pr_fmt(fmt) "vga_switcheroo: " fmt
 
+#include 
 #include 
 #include 
 #include 
@@ -308,7 +309,8 @@ static int register_client(struct pci_dev *pdev,
  *
  * Register vga client (GPU). Enable vga_switcheroo if another GPU and a
  * handler have already registered. The power state of the client is assumed
- * to be ON.
+ * to be ON. Beforehand, vga_switcheroo_client_probe_defer() shall be called
+ * to ensure that all prerequisites are met.
  *
  * Return

Re: [Nouveau] [PATCH] gpu/nouveau/nouveau_acpi.c: Fix Type Mismatch ACPI warning

2016-05-19 Thread Pierre Moreau
Hello Marcos,

I sent a serie a year ago to fix some of the ACPI handling in Nouveau and add
runtime pm support for laptops with an Apple GMUX (see [0], and especially [1]
and [2]). I was told that a more generic work for the runtime pm was in the
work, so I let the whole serie slip away. I was thinking of resubmitting the
serie without the runtime pm additions, to at least fix those warning/error
messages. I thought I had a patch to fix this issue in my serie, but going
through it again, it looks like I did not.
Note that even, though the spec says the 4th argument should be a package, some
old BIOSes expect a different type, like a buffer (see the comment above [5]; I
think there was also a comment about it in the spec, but I can’t find it
anymore).

I added some comments below. I’ll test this patch tonight, to see how it works
on my laptop as I think it expects a buffer as 4th argument.

Thank you!

Regards,
Pierre


[0]: https://lists.freedesktop.org/archives/dri-devel/2015-May/083444.html
[1]: https://lists.freedesktop.org/archives/dri-devel/2015-May/083448.html
[2]: https://lists.freedesktop.org/archives/dri-devel/2015-May/083449.html

On 09:42 PM - May 18 2016, Marcos Paulo de Souza wrote:
> nouveau_optimus_dsm is using ACPI_TYPE_BUFFER, and this triggers warnings on 
> ACPI:
> [7.730564] ACPI: \_SB_.PCI0.RP05.PEGP: failed to evaluate _DSM
> [7.730570] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4 type 
> mismatch - Found [Buffer], ACPI requires [Package] (201509
> 30/nsarguments-95)
> 
> To fix it, change ACPI_TYPE_BUFFER to ACPI_TYPE_PACKAGE, as the warning tells 
> to do.
> 
> Signed-off-by: Marcos Paulo de Souza 
> ---
> 
> After booting my Asus Laptop, and after a suspend/resume too, dmesg shows 
> warnings:
> [1.633361] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4 type 
> mismatch - Found [Buffer], ACPI requires [Package] (20150930/nsarguments-95)
> [1.633434] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4 type 
> mismatch - Found [Buffer], ACPI requires [Package] (20150930/nsarguments-95)
> [7.730176] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4 type 
> mismatch - Found [Buffer], ACPI requires [Package] (20150930/nsarguments-95)
> [7.730564] ACPI: \_SB_.PCI0.RP05.PEGP: failed to evaluate _DSM
> [7.730570] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4 type 
> mismatch - Found [Buffer], ACPI requires [Package] (20150930/nsarguments-95)
> [   49.732059] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4 type 
> mismatch - Found [Buffer], ACPI requires [Package] (20150930/nsarguments-95)
> [   49.732424] ACPI: \_SB_.PCI0.RP05.PEGP: failed to evaluate _DSM
> [   49.732430] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4 type 
> mismatch - Found [Buffer], ACPI requires [Package] (20150930/nsarguments-95)
> [   74.366300] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4 type 
> mismatch - Found [Buffer], ACPI requires [Package] (20150930/nsarguments-95)
> [   74.366657] ACPI: \_SB_.PCI0.RP05.PEGP: failed to evaluate _DSM
> [   74.33] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4 type 
> mismatch - Found [Buffer], ACPI requires [Package] (20150930/nsarguments-95)
> [  140.357789] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4 type 
> mismatch - Found [Buffer], ACPI requires [Package] (20150930/nsarguments-95)
> [  140.358532] ACPI: \_SB_.PCI0.RP05.PEGP: failed to evaluate _DSM
> [  140.358547] ACPI Warning: \_SB_.PCI0.RP05.PEGP._DSM: Argument #4 type 
> mismatch - Found [Buffer], ACPI requires [Package] (20150930/nsarguments-95)

Out of curiosity, which GPU(s) do you have in your laptop? I guess there is at
least an NVIDIA one, but is it an Optimus configuration? And if so, does the
unused card go automatically to sleep after a few seconds?

> 
> I don't know if this is the right thing to do, I just looked at intel_acpi.c 
> to check how to use/check for ACPI Package.
> The patch below silenced the "type mismatch" warnings, and some of the 
> "evaluated _DSM" ones.
> 
> If this is not the right approach, please let me know how to fix it, I don't 
> have knowledge in ACPI, but I really want to help.
> 
>  drivers/gpu/drm/nouveau/nouveau_acpi.c | 14 +-
>  1 file changed, 1 insertion(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_acpi.c 
> b/drivers/gpu/drm/nouveau/nouveau_acpi.c
> index cdf5227..f04aef3 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_acpi.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_acpi.c
> @@ -73,22 +73,10 @@ static const char nouveau_op_dsm_muid[] = {
>  
>  static int nouveau_optimus_dsm(acpi_handle handle, int func, int arg, 
> uint32_t *result)
>  {
> - int i;
>   union acpi_object *obj;
> - char args_buff[4];
> - union acpi_object argv4 = {
> - .buffer.type = ACPI_TYPE_BUFFER,
> - .buffer.length = 4,
> - .buffer.pointer = args_buff
> - };
> -
> - /* ACPI is little endian, AABBCCDD becomes {DD,CC