All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 0/2] drm amdgpu/radeon: clean up d3_delay usage
@ 2017-01-30 20:40 Bjorn Helgaas
  2017-01-30 20:40   ` Bjorn Helgaas
  2017-01-30 20:41   ` Bjorn Helgaas
  0 siblings, 2 replies; 10+ messages in thread
From: Bjorn Helgaas @ 2017-01-30 20:40 UTC (permalink / raw)
  To: Alex Deucher, Christian König
  Cc: linux-pm, linux-pci, linux-kernel, dri-devel, amd-gfx, Maarten Lankhorst

amdgpu doesn't need to touch pdev->d3_delay at all.

radeon has a d3_delay quirk for MacBook Pro, but it only affects
radeon_switcheroo_set_state().  I think it should affect wakeups done by
the PCI core as well.

---

Bjorn Helgaas (2):
      drm/amdgpu: remove unnecessary save/restore of pdev->d3_delay
      drm/radeon: make MacBook Pro d3_delay quirk more generic


 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |    4 ----
 drivers/gpu/drm/radeon/radeon_device.c     |   12 ------------
 drivers/pci/quirks.c                       |   13 +++++++++++++
 3 files changed, 13 insertions(+), 16 deletions(-)

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v1 1/2] drm/amdgpu: remove unnecessary save/restore of pdev->d3_delay
  2017-01-30 20:40 [PATCH v1 0/2] drm amdgpu/radeon: clean up d3_delay usage Bjorn Helgaas
@ 2017-01-30 20:40   ` Bjorn Helgaas
  2017-01-30 20:41   ` Bjorn Helgaas
  1 sibling, 0 replies; 10+ messages in thread
From: Bjorn Helgaas @ 2017-01-30 20:40 UTC (permalink / raw)
  To: Alex Deucher, Christian König
  Cc: linux-pm, linux-pci, linux-kernel, dri-devel, amd-gfx, Maarten Lankhorst

Remove unnecessary save/restore of pdev->d3_delay.

The only assignments to pdev->d3_delay are in radeon_switcheroo_set_state()
and some quirks, none of which should be relevant in the
amdgpu_switcheroo_set_state() path.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |    4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 60bd4afe45c8..3a403a87ec62 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1042,16 +1042,12 @@ static void amdgpu_switcheroo_set_state(struct pci_dev *pdev, enum vga_switchero
 		return;
 
 	if (state == VGA_SWITCHEROO_ON) {
-		unsigned d3_delay = dev->pdev->d3_delay;
-
 		printk(KERN_INFO "amdgpu: switched on\n");
 		/* don't suspend or resume card normally */
 		dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
 
 		amdgpu_device_resume(dev, true, true);
 
-		dev->pdev->d3_delay = d3_delay;
-
 		dev->switch_power_state = DRM_SWITCH_POWER_ON;
 		drm_kms_helper_poll_enable(dev);
 	} else {

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v1 1/2] drm/amdgpu: remove unnecessary save/restore of pdev->d3_delay
@ 2017-01-30 20:40   ` Bjorn Helgaas
  0 siblings, 0 replies; 10+ messages in thread
From: Bjorn Helgaas @ 2017-01-30 20:40 UTC (permalink / raw)
  To: Alex Deucher, Christian König
  Cc: linux-pm, linux-pci, linux-kernel, amd-gfx, dri-devel, Maarten Lankhorst

Remove unnecessary save/restore of pdev->d3_delay.

The only assignments to pdev->d3_delay are in radeon_switcheroo_set_state()
and some quirks, none of which should be relevant in the
amdgpu_switcheroo_set_state() path.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |    4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 60bd4afe45c8..3a403a87ec62 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1042,16 +1042,12 @@ static void amdgpu_switcheroo_set_state(struct pci_dev *pdev, enum vga_switchero
 		return;
 
 	if (state == VGA_SWITCHEROO_ON) {
-		unsigned d3_delay = dev->pdev->d3_delay;
-
 		printk(KERN_INFO "amdgpu: switched on\n");
 		/* don't suspend or resume card normally */
 		dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
 
 		amdgpu_device_resume(dev, true, true);
 
-		dev->pdev->d3_delay = d3_delay;
-
 		dev->switch_power_state = DRM_SWITCH_POWER_ON;
 		drm_kms_helper_poll_enable(dev);
 	} else {

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v1 2/2] drm/radeon: make MacBook Pro d3_delay quirk more generic
  2017-01-30 20:40 [PATCH v1 0/2] drm amdgpu/radeon: clean up d3_delay usage Bjorn Helgaas
@ 2017-01-30 20:41   ` Bjorn Helgaas
  2017-01-30 20:41   ` Bjorn Helgaas
  1 sibling, 0 replies; 10+ messages in thread
From: Bjorn Helgaas @ 2017-01-30 20:41 UTC (permalink / raw)
  To: Alex Deucher, Christian König
  Cc: linux-pm, linux-pci, linux-kernel, dri-devel, amd-gfx, Maarten Lankhorst

The PCI Power Management Spec, r1.2, sec 5.6.1, requires a 10 millisecond
delay when powering on a device, i.e., transitioning from state D3hot to
D0.

Apparently some devices require more time, and d1f9809ed131 ("drm/radeon:
add quirk for d3 delay during switcheroo poweron for apple macbooks") added
an additional delay for the Radeon device in a MacBook Pro.  4807c5a8a0c8
("drm/radeon: add a PX quirk list") made the affected device more explicit.

Add a generic PCI quirk to increase the d3_delay.  This means we will use
the additional delay for *all* wakeups from D3, not just those initiated by
radeon_switcheroo_set_state().

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
CC: Maarten Lankhorst <maarten.lankhorst@canonical.com>
---
 drivers/gpu/drm/radeon/radeon_device.c |   12 ------------
 drivers/pci/quirks.c                   |   13 +++++++++++++
 2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
index 8a1df2a1afbd..8b8fd981cae5 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -113,7 +113,6 @@ static inline bool radeon_is_atpx_hybrid(void) { return false; }
 #endif
 
 #define RADEON_PX_QUIRK_DISABLE_PX  (1 << 0)
-#define RADEON_PX_QUIRK_LONG_WAKEUP (1 << 1)
 
 struct radeon_px_quirk {
 	u32 chip_vendor;
@@ -136,9 +135,6 @@ static struct radeon_px_quirk radeon_px_quirk_list[] = {
 	 * https://bugzilla.kernel.org/show_bug.cgi?id=51381
 	 */
 	{ PCI_VENDOR_ID_ATI, 0x6840, 0x1043, 0x2122, RADEON_PX_QUIRK_DISABLE_PX },
-	/* macbook pro 8.2 */
-	{ PCI_VENDOR_ID_ATI, 0x6741, PCI_VENDOR_ID_APPLE, 0x00e2, RADEON_PX_QUIRK_LONG_WAKEUP },
-	{ 0, 0, 0, 0, 0 },
 };
 
 bool radeon_is_px(struct drm_device *dev)
@@ -1241,25 +1237,17 @@ static void radeon_check_arguments(struct radeon_device *rdev)
 static void radeon_switcheroo_set_state(struct pci_dev *pdev, enum vga_switcheroo_state state)
 {
 	struct drm_device *dev = pci_get_drvdata(pdev);
-	struct radeon_device *rdev = dev->dev_private;
 
 	if (radeon_is_px(dev) && state == VGA_SWITCHEROO_OFF)
 		return;
 
 	if (state == VGA_SWITCHEROO_ON) {
-		unsigned d3_delay = dev->pdev->d3_delay;
-
 		printk(KERN_INFO "radeon: switched on\n");
 		/* don't suspend or resume card normally */
 		dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
 
-		if (d3_delay < 20 && (rdev->px_quirk_flags & RADEON_PX_QUIRK_LONG_WAKEUP))
-			dev->pdev->d3_delay = 20;
-
 		radeon_resume_kms(dev, true, true);
 
-		dev->pdev->d3_delay = d3_delay;
-
 		dev->switch_power_state = DRM_SWITCH_POWER_ON;
 		drm_kms_helper_poll_enable(dev);
 	} else {
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 1800befa8b8b..512d7a875d62 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -1683,6 +1683,19 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	0x2609, quirk_intel_pcie_pm);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	0x260a, quirk_intel_pcie_pm);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	0x260b, quirk_intel_pcie_pm);
 
+static void quirk_radeon_pm(struct pci_dev *dev)
+{
+	if (dev->subsystem_vendor == PCI_VENDOR_ID_APPLE &&
+	    dev->subsystem_device == 0x00e2) {
+		if (dev->d3_delay < 20) {
+			dev->d3_delay = 20;
+			dev_info(&dev->dev, "extending delay after power-on from D3 to %d msec\n",
+				 dev->d3_delay);
+		}
+	}
+}
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x6741, quirk_radeon_pm);
+
 #ifdef CONFIG_X86_IO_APIC
 /*
  * Boot interrupts on some chipsets cannot be turned off. For these chipsets,

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v1 2/2] drm/radeon: make MacBook Pro d3_delay quirk more generic
@ 2017-01-30 20:41   ` Bjorn Helgaas
  0 siblings, 0 replies; 10+ messages in thread
From: Bjorn Helgaas @ 2017-01-30 20:41 UTC (permalink / raw)
  To: Alex Deucher, Christian König
  Cc: linux-pm, linux-pci, linux-kernel, amd-gfx, dri-devel, Maarten Lankhorst

The PCI Power Management Spec, r1.2, sec 5.6.1, requires a 10 millisecond
delay when powering on a device, i.e., transitioning from state D3hot to
D0.

Apparently some devices require more time, and d1f9809ed131 ("drm/radeon:
add quirk for d3 delay during switcheroo poweron for apple macbooks") added
an additional delay for the Radeon device in a MacBook Pro.  4807c5a8a0c8
("drm/radeon: add a PX quirk list") made the affected device more explicit.

Add a generic PCI quirk to increase the d3_delay.  This means we will use
the additional delay for *all* wakeups from D3, not just those initiated by
radeon_switcheroo_set_state().

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
CC: Maarten Lankhorst <maarten.lankhorst@canonical.com>
---
 drivers/gpu/drm/radeon/radeon_device.c |   12 ------------
 drivers/pci/quirks.c                   |   13 +++++++++++++
 2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
index 8a1df2a1afbd..8b8fd981cae5 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -113,7 +113,6 @@ static inline bool radeon_is_atpx_hybrid(void) { return false; }
 #endif
 
 #define RADEON_PX_QUIRK_DISABLE_PX  (1 << 0)
-#define RADEON_PX_QUIRK_LONG_WAKEUP (1 << 1)
 
 struct radeon_px_quirk {
 	u32 chip_vendor;
@@ -136,9 +135,6 @@ static struct radeon_px_quirk radeon_px_quirk_list[] = {
 	 * https://bugzilla.kernel.org/show_bug.cgi?id=51381
 	 */
 	{ PCI_VENDOR_ID_ATI, 0x6840, 0x1043, 0x2122, RADEON_PX_QUIRK_DISABLE_PX },
-	/* macbook pro 8.2 */
-	{ PCI_VENDOR_ID_ATI, 0x6741, PCI_VENDOR_ID_APPLE, 0x00e2, RADEON_PX_QUIRK_LONG_WAKEUP },
-	{ 0, 0, 0, 0, 0 },
 };
 
 bool radeon_is_px(struct drm_device *dev)
@@ -1241,25 +1237,17 @@ static void radeon_check_arguments(struct radeon_device *rdev)
 static void radeon_switcheroo_set_state(struct pci_dev *pdev, enum vga_switcheroo_state state)
 {
 	struct drm_device *dev = pci_get_drvdata(pdev);
-	struct radeon_device *rdev = dev->dev_private;
 
 	if (radeon_is_px(dev) && state == VGA_SWITCHEROO_OFF)
 		return;
 
 	if (state == VGA_SWITCHEROO_ON) {
-		unsigned d3_delay = dev->pdev->d3_delay;
-
 		printk(KERN_INFO "radeon: switched on\n");
 		/* don't suspend or resume card normally */
 		dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
 
-		if (d3_delay < 20 && (rdev->px_quirk_flags & RADEON_PX_QUIRK_LONG_WAKEUP))
-			dev->pdev->d3_delay = 20;
-
 		radeon_resume_kms(dev, true, true);
 
-		dev->pdev->d3_delay = d3_delay;
-
 		dev->switch_power_state = DRM_SWITCH_POWER_ON;
 		drm_kms_helper_poll_enable(dev);
 	} else {
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 1800befa8b8b..512d7a875d62 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -1683,6 +1683,19 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	0x2609, quirk_intel_pcie_pm);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	0x260a, quirk_intel_pcie_pm);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	0x260b, quirk_intel_pcie_pm);
 
+static void quirk_radeon_pm(struct pci_dev *dev)
+{
+	if (dev->subsystem_vendor == PCI_VENDOR_ID_APPLE &&
+	    dev->subsystem_device == 0x00e2) {
+		if (dev->d3_delay < 20) {
+			dev->d3_delay = 20;
+			dev_info(&dev->dev, "extending delay after power-on from D3 to %d msec\n",
+				 dev->d3_delay);
+		}
+	}
+}
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x6741, quirk_radeon_pm);
+
 #ifdef CONFIG_X86_IO_APIC
 /*
  * Boot interrupts on some chipsets cannot be turned off. For these chipsets,

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* RE: [PATCH v1 2/2] drm/radeon: make MacBook Pro d3_delay quirk more generic
  2017-01-30 20:41   ` Bjorn Helgaas
  (?)
@ 2017-01-30 20:57     ` Deucher, Alexander
  -1 siblings, 0 replies; 10+ messages in thread
From: Deucher, Alexander @ 2017-01-30 20:57 UTC (permalink / raw)
  To: 'Bjorn Helgaas', Koenig, Christian
  Cc: linux-pm, linux-pci, linux-kernel, amd-gfx, dri-devel, Maarten Lankhorst

> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf
> Of Bjorn Helgaas
> Sent: Monday, January 30, 2017 3:41 PM
> To: Deucher, Alexander; Koenig, Christian
> Cc: linux-pm@vger.kernel.org; linux-pci@vger.kernel.org; linux-
> kernel@vger.kernel.org; amd-gfx@lists.freedesktop.org; dri-
> devel@lists.freedesktop.org; Maarten Lankhorst
> Subject: [PATCH v1 2/2] drm/radeon: make MacBook Pro d3_delay quirk
> more generic
> 
> The PCI Power Management Spec, r1.2, sec 5.6.1, requires a 10 millisecond
> delay when powering on a device, i.e., transitioning from state D3hot to
> D0.
> 
> Apparently some devices require more time, and d1f9809ed131
> ("drm/radeon:
> add quirk for d3 delay during switcheroo poweron for apple macbooks")
> added
> an additional delay for the Radeon device in a MacBook Pro.  4807c5a8a0c8
> ("drm/radeon: add a PX quirk list") made the affected device more explicit.
> 
> Add a generic PCI quirk to increase the d3_delay.  This means we will use
> the additional delay for *all* wakeups from D3, not just those initiated by
> radeon_switcheroo_set_state().
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> CC: Maarten Lankhorst <maarten.lankhorst@canonical.com>

For the series:
Acked-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/gpu/drm/radeon/radeon_device.c |   12 ------------
>  drivers/pci/quirks.c                   |   13 +++++++++++++
>  2 files changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/radeon/radeon_device.c
> b/drivers/gpu/drm/radeon/radeon_device.c
> index 8a1df2a1afbd..8b8fd981cae5 100644
> --- a/drivers/gpu/drm/radeon/radeon_device.c
> +++ b/drivers/gpu/drm/radeon/radeon_device.c
> @@ -113,7 +113,6 @@ static inline bool radeon_is_atpx_hybrid(void) {
> return false; }
>  #endif
> 
>  #define RADEON_PX_QUIRK_DISABLE_PX  (1 << 0)
> -#define RADEON_PX_QUIRK_LONG_WAKEUP (1 << 1)
> 
>  struct radeon_px_quirk {
>  	u32 chip_vendor;
> @@ -136,9 +135,6 @@ static struct radeon_px_quirk radeon_px_quirk_list[]
> = {
>  	 * https://bugzilla.kernel.org/show_bug.cgi?id=51381
>  	 */
>  	{ PCI_VENDOR_ID_ATI, 0x6840, 0x1043, 0x2122,
> RADEON_PX_QUIRK_DISABLE_PX },
> -	/* macbook pro 8.2 */
> -	{ PCI_VENDOR_ID_ATI, 0x6741, PCI_VENDOR_ID_APPLE, 0x00e2,
> RADEON_PX_QUIRK_LONG_WAKEUP },
> -	{ 0, 0, 0, 0, 0 },
>  };
> 
>  bool radeon_is_px(struct drm_device *dev)
> @@ -1241,25 +1237,17 @@ static void radeon_check_arguments(struct
> radeon_device *rdev)
>  static void radeon_switcheroo_set_state(struct pci_dev *pdev, enum
> vga_switcheroo_state state)
>  {
>  	struct drm_device *dev = pci_get_drvdata(pdev);
> -	struct radeon_device *rdev = dev->dev_private;
> 
>  	if (radeon_is_px(dev) && state == VGA_SWITCHEROO_OFF)
>  		return;
> 
>  	if (state == VGA_SWITCHEROO_ON) {
> -		unsigned d3_delay = dev->pdev->d3_delay;
> -
>  		printk(KERN_INFO "radeon: switched on\n");
>  		/* don't suspend or resume card normally */
>  		dev->switch_power_state =
> DRM_SWITCH_POWER_CHANGING;
> 
> -		if (d3_delay < 20 && (rdev->px_quirk_flags &
> RADEON_PX_QUIRK_LONG_WAKEUP))
> -			dev->pdev->d3_delay = 20;
> -
>  		radeon_resume_kms(dev, true, true);
> 
> -		dev->pdev->d3_delay = d3_delay;
> -
>  		dev->switch_power_state = DRM_SWITCH_POWER_ON;
>  		drm_kms_helper_poll_enable(dev);
>  	} else {
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 1800befa8b8b..512d7a875d62 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -1683,6 +1683,19 @@
> DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	0x2609,
> quirk_intel_pcie_pm);
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	0x260a,
> quirk_intel_pcie_pm);
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	0x260b,
> quirk_intel_pcie_pm);
> 
> +static void quirk_radeon_pm(struct pci_dev *dev)
> +{
> +	if (dev->subsystem_vendor == PCI_VENDOR_ID_APPLE &&
> +	    dev->subsystem_device == 0x00e2) {
> +		if (dev->d3_delay < 20) {
> +			dev->d3_delay = 20;
> +			dev_info(&dev->dev, "extending delay after power-
> on from D3 to %d msec\n",
> +				 dev->d3_delay);
> +		}
> +	}
> +}
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x6741,
> quirk_radeon_pm);
> +
>  #ifdef CONFIG_X86_IO_APIC
>  /*
>   * Boot interrupts on some chipsets cannot be turned off. For these
> chipsets,
> 
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH v1 2/2] drm/radeon: make MacBook Pro d3_delay quirk more generic
@ 2017-01-30 20:57     ` Deucher, Alexander
  0 siblings, 0 replies; 10+ messages in thread
From: Deucher, Alexander @ 2017-01-30 20:57 UTC (permalink / raw)
  To: 'Bjorn Helgaas', Koenig, Christian
  Cc: linux-pm, linux-pci, linux-kernel, amd-gfx, dri-devel, Maarten Lankhorst

PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBhbWQtZ2Z4IFttYWlsdG86YW1k
LWdmeC1ib3VuY2VzQGxpc3RzLmZyZWVkZXNrdG9wLm9yZ10gT24gQmVoYWxmDQo+IE9mIEJqb3Ju
IEhlbGdhYXMNCj4gU2VudDogTW9uZGF5LCBKYW51YXJ5IDMwLCAyMDE3IDM6NDEgUE0NCj4gVG86
IERldWNoZXIsIEFsZXhhbmRlcjsgS29lbmlnLCBDaHJpc3RpYW4NCj4gQ2M6IGxpbnV4LXBtQHZn
ZXIua2VybmVsLm9yZzsgbGludXgtcGNpQHZnZXIua2VybmVsLm9yZzsgbGludXgtDQo+IGtlcm5l
bEB2Z2VyLmtlcm5lbC5vcmc7IGFtZC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnOyBkcmktDQo+
IGRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZzsgTWFhcnRlbiBMYW5raG9yc3QNCj4gU3ViamVj
dDogW1BBVENIIHYxIDIvMl0gZHJtL3JhZGVvbjogbWFrZSBNYWNCb29rIFBybyBkM19kZWxheSBx
dWlyaw0KPiBtb3JlIGdlbmVyaWMNCj4gDQo+IFRoZSBQQ0kgUG93ZXIgTWFuYWdlbWVudCBTcGVj
LCByMS4yLCBzZWMgNS42LjEsIHJlcXVpcmVzIGEgMTAgbWlsbGlzZWNvbmQNCj4gZGVsYXkgd2hl
biBwb3dlcmluZyBvbiBhIGRldmljZSwgaS5lLiwgdHJhbnNpdGlvbmluZyBmcm9tIHN0YXRlIEQz
aG90IHRvDQo+IEQwLg0KPiANCj4gQXBwYXJlbnRseSBzb21lIGRldmljZXMgcmVxdWlyZSBtb3Jl
IHRpbWUsIGFuZCBkMWY5ODA5ZWQxMzENCj4gKCJkcm0vcmFkZW9uOg0KPiBhZGQgcXVpcmsgZm9y
IGQzIGRlbGF5IGR1cmluZyBzd2l0Y2hlcm9vIHBvd2Vyb24gZm9yIGFwcGxlIG1hY2Jvb2tzIikN
Cj4gYWRkZWQNCj4gYW4gYWRkaXRpb25hbCBkZWxheSBmb3IgdGhlIFJhZGVvbiBkZXZpY2UgaW4g
YSBNYWNCb29rIFByby4gIDQ4MDdjNWE4YTBjOA0KPiAoImRybS9yYWRlb246IGFkZCBhIFBYIHF1
aXJrIGxpc3QiKSBtYWRlIHRoZSBhZmZlY3RlZCBkZXZpY2UgbW9yZSBleHBsaWNpdC4NCj4gDQo+
IEFkZCBhIGdlbmVyaWMgUENJIHF1aXJrIHRvIGluY3JlYXNlIHRoZSBkM19kZWxheS4gIFRoaXMg
bWVhbnMgd2Ugd2lsbCB1c2UNCj4gdGhlIGFkZGl0aW9uYWwgZGVsYXkgZm9yICphbGwqIHdha2V1
cHMgZnJvbSBEMywgbm90IGp1c3QgdGhvc2UgaW5pdGlhdGVkIGJ5DQo+IHJhZGVvbl9zd2l0Y2hl
cm9vX3NldF9zdGF0ZSgpLg0KPiANCj4gU2lnbmVkLW9mZi1ieTogQmpvcm4gSGVsZ2FhcyA8Ymhl
bGdhYXNAZ29vZ2xlLmNvbT4NCj4gQ0M6IE1hYXJ0ZW4gTGFua2hvcnN0IDxtYWFydGVuLmxhbmto
b3JzdEBjYW5vbmljYWwuY29tPg0KDQpGb3IgdGhlIHNlcmllczoNCkFja2VkLWJ5OiBBbGV4IERl
dWNoZXIgPGFsZXhhbmRlci5kZXVjaGVyQGFtZC5jb20+DQoNCj4gLS0tDQo+ICBkcml2ZXJzL2dw
dS9kcm0vcmFkZW9uL3JhZGVvbl9kZXZpY2UuYyB8ICAgMTIgLS0tLS0tLS0tLS0tDQo+ICBkcml2
ZXJzL3BjaS9xdWlya3MuYyAgICAgICAgICAgICAgICAgICB8ICAgMTMgKysrKysrKysrKysrKw0K
PiAgMiBmaWxlcyBjaGFuZ2VkLCAxMyBpbnNlcnRpb25zKCspLCAxMiBkZWxldGlvbnMoLSkNCj4g
DQo+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2dwdS9kcm0vcmFkZW9uL3JhZGVvbl9kZXZpY2UuYw0K
PiBiL2RyaXZlcnMvZ3B1L2RybS9yYWRlb24vcmFkZW9uX2RldmljZS5jDQo+IGluZGV4IDhhMWRm
MmExYWZiZC4uOGI4ZmQ5ODFjYWU1IDEwMDY0NA0KPiAtLS0gYS9kcml2ZXJzL2dwdS9kcm0vcmFk
ZW9uL3JhZGVvbl9kZXZpY2UuYw0KPiArKysgYi9kcml2ZXJzL2dwdS9kcm0vcmFkZW9uL3JhZGVv
bl9kZXZpY2UuYw0KPiBAQCAtMTEzLDcgKzExMyw2IEBAIHN0YXRpYyBpbmxpbmUgYm9vbCByYWRl
b25faXNfYXRweF9oeWJyaWQodm9pZCkgew0KPiByZXR1cm4gZmFsc2U7IH0NCj4gICNlbmRpZg0K
PiANCj4gICNkZWZpbmUgUkFERU9OX1BYX1FVSVJLX0RJU0FCTEVfUFggICgxIDw8IDApDQo+IC0j
ZGVmaW5lIFJBREVPTl9QWF9RVUlSS19MT05HX1dBS0VVUCAoMSA8PCAxKQ0KPiANCj4gIHN0cnVj
dCByYWRlb25fcHhfcXVpcmsgew0KPiAgCXUzMiBjaGlwX3ZlbmRvcjsNCj4gQEAgLTEzNiw5ICsx
MzUsNiBAQCBzdGF0aWMgc3RydWN0IHJhZGVvbl9weF9xdWlyayByYWRlb25fcHhfcXVpcmtfbGlz
dFtdDQo+ID0gew0KPiAgCSAqIGh0dHBzOi8vYnVnemlsbGEua2VybmVsLm9yZy9zaG93X2J1Zy5j
Z2k/aWQ9NTEzODENCj4gIAkgKi8NCj4gIAl7IFBDSV9WRU5ET1JfSURfQVRJLCAweDY4NDAsIDB4
MTA0MywgMHgyMTIyLA0KPiBSQURFT05fUFhfUVVJUktfRElTQUJMRV9QWCB9LA0KPiAtCS8qIG1h
Y2Jvb2sgcHJvIDguMiAqLw0KPiAtCXsgUENJX1ZFTkRPUl9JRF9BVEksIDB4Njc0MSwgUENJX1ZF
TkRPUl9JRF9BUFBMRSwgMHgwMGUyLA0KPiBSQURFT05fUFhfUVVJUktfTE9OR19XQUtFVVAgfSwN
Cj4gLQl7IDAsIDAsIDAsIDAsIDAgfSwNCj4gIH07DQo+IA0KPiAgYm9vbCByYWRlb25faXNfcHgo
c3RydWN0IGRybV9kZXZpY2UgKmRldikNCj4gQEAgLTEyNDEsMjUgKzEyMzcsMTcgQEAgc3RhdGlj
IHZvaWQgcmFkZW9uX2NoZWNrX2FyZ3VtZW50cyhzdHJ1Y3QNCj4gcmFkZW9uX2RldmljZSAqcmRl
dikNCj4gIHN0YXRpYyB2b2lkIHJhZGVvbl9zd2l0Y2hlcm9vX3NldF9zdGF0ZShzdHJ1Y3QgcGNp
X2RldiAqcGRldiwgZW51bQ0KPiB2Z2Ffc3dpdGNoZXJvb19zdGF0ZSBzdGF0ZSkNCj4gIHsNCj4g
IAlzdHJ1Y3QgZHJtX2RldmljZSAqZGV2ID0gcGNpX2dldF9kcnZkYXRhKHBkZXYpOw0KPiAtCXN0
cnVjdCByYWRlb25fZGV2aWNlICpyZGV2ID0gZGV2LT5kZXZfcHJpdmF0ZTsNCj4gDQo+ICAJaWYg
KHJhZGVvbl9pc19weChkZXYpICYmIHN0YXRlID09IFZHQV9TV0lUQ0hFUk9PX09GRikNCj4gIAkJ
cmV0dXJuOw0KPiANCj4gIAlpZiAoc3RhdGUgPT0gVkdBX1NXSVRDSEVST09fT04pIHsNCj4gLQkJ
dW5zaWduZWQgZDNfZGVsYXkgPSBkZXYtPnBkZXYtPmQzX2RlbGF5Ow0KPiAtDQo+ICAJCXByaW50
ayhLRVJOX0lORk8gInJhZGVvbjogc3dpdGNoZWQgb25cbiIpOw0KPiAgCQkvKiBkb24ndCBzdXNw
ZW5kIG9yIHJlc3VtZSBjYXJkIG5vcm1hbGx5ICovDQo+ICAJCWRldi0+c3dpdGNoX3Bvd2VyX3N0
YXRlID0NCj4gRFJNX1NXSVRDSF9QT1dFUl9DSEFOR0lORzsNCj4gDQo+IC0JCWlmIChkM19kZWxh
eSA8IDIwICYmIChyZGV2LT5weF9xdWlya19mbGFncyAmDQo+IFJBREVPTl9QWF9RVUlSS19MT05H
X1dBS0VVUCkpDQo+IC0JCQlkZXYtPnBkZXYtPmQzX2RlbGF5ID0gMjA7DQo+IC0NCj4gIAkJcmFk
ZW9uX3Jlc3VtZV9rbXMoZGV2LCB0cnVlLCB0cnVlKTsNCj4gDQo+IC0JCWRldi0+cGRldi0+ZDNf
ZGVsYXkgPSBkM19kZWxheTsNCj4gLQ0KPiAgCQlkZXYtPnN3aXRjaF9wb3dlcl9zdGF0ZSA9IERS
TV9TV0lUQ0hfUE9XRVJfT047DQo+ICAJCWRybV9rbXNfaGVscGVyX3BvbGxfZW5hYmxlKGRldik7
DQo+ICAJfSBlbHNlIHsNCj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvcGNpL3F1aXJrcy5jIGIvZHJp
dmVycy9wY2kvcXVpcmtzLmMNCj4gaW5kZXggMTgwMGJlZmE4YjhiLi41MTJkN2E4NzVkNjIgMTAw
NjQ0DQo+IC0tLSBhL2RyaXZlcnMvcGNpL3F1aXJrcy5jDQo+ICsrKyBiL2RyaXZlcnMvcGNpL3F1
aXJrcy5jDQo+IEBAIC0xNjgzLDYgKzE2ODMsMTkgQEANCj4gREVDTEFSRV9QQ0lfRklYVVBfRklO
QUwoUENJX1ZFTkRPUl9JRF9JTlRFTCwJMHgyNjA5LA0KPiBxdWlya19pbnRlbF9wY2llX3BtKTsN
Cj4gIERFQ0xBUkVfUENJX0ZJWFVQX0ZJTkFMKFBDSV9WRU5ET1JfSURfSU5URUwsCTB4MjYwYSwN
Cj4gcXVpcmtfaW50ZWxfcGNpZV9wbSk7DQo+ICBERUNMQVJFX1BDSV9GSVhVUF9GSU5BTChQQ0lf
VkVORE9SX0lEX0lOVEVMLAkweDI2MGIsDQo+IHF1aXJrX2ludGVsX3BjaWVfcG0pOw0KPiANCj4g
K3N0YXRpYyB2b2lkIHF1aXJrX3JhZGVvbl9wbShzdHJ1Y3QgcGNpX2RldiAqZGV2KQ0KPiArew0K
PiArCWlmIChkZXYtPnN1YnN5c3RlbV92ZW5kb3IgPT0gUENJX1ZFTkRPUl9JRF9BUFBMRSAmJg0K
PiArCSAgICBkZXYtPnN1YnN5c3RlbV9kZXZpY2UgPT0gMHgwMGUyKSB7DQo+ICsJCWlmIChkZXYt
PmQzX2RlbGF5IDwgMjApIHsNCj4gKwkJCWRldi0+ZDNfZGVsYXkgPSAyMDsNCj4gKwkJCWRldl9p
bmZvKCZkZXYtPmRldiwgImV4dGVuZGluZyBkZWxheSBhZnRlciBwb3dlci0NCj4gb24gZnJvbSBE
MyB0byAlZCBtc2VjXG4iLA0KPiArCQkJCSBkZXYtPmQzX2RlbGF5KTsNCj4gKwkJfQ0KPiArCX0N
Cj4gK30NCj4gK0RFQ0xBUkVfUENJX0ZJWFVQX0ZJTkFMKFBDSV9WRU5ET1JfSURfQVRJLCAweDY3
NDEsDQo+IHF1aXJrX3JhZGVvbl9wbSk7DQo+ICsNCj4gICNpZmRlZiBDT05GSUdfWDg2X0lPX0FQ
SUMNCj4gIC8qDQo+ICAgKiBCb290IGludGVycnVwdHMgb24gc29tZSBjaGlwc2V0cyBjYW5ub3Qg
YmUgdHVybmVkIG9mZi4gRm9yIHRoZXNlDQo+IGNoaXBzZXRzLA0KPiANCj4gX19fX19fX19fX19f
X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18NCj4gYW1kLWdmeCBtYWlsaW5nIGxp
c3QNCj4gYW1kLWdmeEBsaXN0cy5mcmVlZGVza3RvcC5vcmcNCj4gaHR0cHM6Ly9saXN0cy5mcmVl
ZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9hbWQtZ2Z4DQo=

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH v1 2/2] drm/radeon: make MacBook Pro d3_delay quirk more generic
@ 2017-01-30 20:57     ` Deucher, Alexander
  0 siblings, 0 replies; 10+ messages in thread
From: Deucher, Alexander @ 2017-01-30 20:57 UTC (permalink / raw)
  To: 'Bjorn Helgaas', Koenig, Christian
  Cc: linux-pm-u79uwXL29TY76Z2rM5mHXA,
	linux-pci-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Maarten Lankhorst

> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf
> Of Bjorn Helgaas
> Sent: Monday, January 30, 2017 3:41 PM
> To: Deucher, Alexander; Koenig, Christian
> Cc: linux-pm@vger.kernel.org; linux-pci@vger.kernel.org; linux-
> kernel@vger.kernel.org; amd-gfx@lists.freedesktop.org; dri-
> devel@lists.freedesktop.org; Maarten Lankhorst
> Subject: [PATCH v1 2/2] drm/radeon: make MacBook Pro d3_delay quirk
> more generic
> 
> The PCI Power Management Spec, r1.2, sec 5.6.1, requires a 10 millisecond
> delay when powering on a device, i.e., transitioning from state D3hot to
> D0.
> 
> Apparently some devices require more time, and d1f9809ed131
> ("drm/radeon:
> add quirk for d3 delay during switcheroo poweron for apple macbooks")
> added
> an additional delay for the Radeon device in a MacBook Pro.  4807c5a8a0c8
> ("drm/radeon: add a PX quirk list") made the affected device more explicit.
> 
> Add a generic PCI quirk to increase the d3_delay.  This means we will use
> the additional delay for *all* wakeups from D3, not just those initiated by
> radeon_switcheroo_set_state().
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> CC: Maarten Lankhorst <maarten.lankhorst@canonical.com>

For the series:
Acked-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/gpu/drm/radeon/radeon_device.c |   12 ------------
>  drivers/pci/quirks.c                   |   13 +++++++++++++
>  2 files changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/radeon/radeon_device.c
> b/drivers/gpu/drm/radeon/radeon_device.c
> index 8a1df2a1afbd..8b8fd981cae5 100644
> --- a/drivers/gpu/drm/radeon/radeon_device.c
> +++ b/drivers/gpu/drm/radeon/radeon_device.c
> @@ -113,7 +113,6 @@ static inline bool radeon_is_atpx_hybrid(void) {
> return false; }
>  #endif
> 
>  #define RADEON_PX_QUIRK_DISABLE_PX  (1 << 0)
> -#define RADEON_PX_QUIRK_LONG_WAKEUP (1 << 1)
> 
>  struct radeon_px_quirk {
>  	u32 chip_vendor;
> @@ -136,9 +135,6 @@ static struct radeon_px_quirk radeon_px_quirk_list[]
> = {
>  	 * https://bugzilla.kernel.org/show_bug.cgi?id=51381
>  	 */
>  	{ PCI_VENDOR_ID_ATI, 0x6840, 0x1043, 0x2122,
> RADEON_PX_QUIRK_DISABLE_PX },
> -	/* macbook pro 8.2 */
> -	{ PCI_VENDOR_ID_ATI, 0x6741, PCI_VENDOR_ID_APPLE, 0x00e2,
> RADEON_PX_QUIRK_LONG_WAKEUP },
> -	{ 0, 0, 0, 0, 0 },
>  };
> 
>  bool radeon_is_px(struct drm_device *dev)
> @@ -1241,25 +1237,17 @@ static void radeon_check_arguments(struct
> radeon_device *rdev)
>  static void radeon_switcheroo_set_state(struct pci_dev *pdev, enum
> vga_switcheroo_state state)
>  {
>  	struct drm_device *dev = pci_get_drvdata(pdev);
> -	struct radeon_device *rdev = dev->dev_private;
> 
>  	if (radeon_is_px(dev) && state == VGA_SWITCHEROO_OFF)
>  		return;
> 
>  	if (state == VGA_SWITCHEROO_ON) {
> -		unsigned d3_delay = dev->pdev->d3_delay;
> -
>  		printk(KERN_INFO "radeon: switched on\n");
>  		/* don't suspend or resume card normally */
>  		dev->switch_power_state =
> DRM_SWITCH_POWER_CHANGING;
> 
> -		if (d3_delay < 20 && (rdev->px_quirk_flags &
> RADEON_PX_QUIRK_LONG_WAKEUP))
> -			dev->pdev->d3_delay = 20;
> -
>  		radeon_resume_kms(dev, true, true);
> 
> -		dev->pdev->d3_delay = d3_delay;
> -
>  		dev->switch_power_state = DRM_SWITCH_POWER_ON;
>  		drm_kms_helper_poll_enable(dev);
>  	} else {
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 1800befa8b8b..512d7a875d62 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -1683,6 +1683,19 @@
> DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	0x2609,
> quirk_intel_pcie_pm);
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	0x260a,
> quirk_intel_pcie_pm);
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	0x260b,
> quirk_intel_pcie_pm);
> 
> +static void quirk_radeon_pm(struct pci_dev *dev)
> +{
> +	if (dev->subsystem_vendor == PCI_VENDOR_ID_APPLE &&
> +	    dev->subsystem_device == 0x00e2) {
> +		if (dev->d3_delay < 20) {
> +			dev->d3_delay = 20;
> +			dev_info(&dev->dev, "extending delay after power-
> on from D3 to %d msec\n",
> +				 dev->d3_delay);
> +		}
> +	}
> +}
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x6741,
> quirk_radeon_pm);
> +
>  #ifdef CONFIG_X86_IO_APIC
>  /*
>   * Boot interrupts on some chipsets cannot be turned off. For these
> chipsets,
> 
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 2/2] drm/radeon: make MacBook Pro d3_delay quirk more generic
  2017-01-30 20:41   ` Bjorn Helgaas
  (?)
  (?)
@ 2017-02-01 15:45   ` Andreas Boll
  2017-02-01 16:15     ` Bjorn Helgaas
  -1 siblings, 1 reply; 10+ messages in thread
From: Andreas Boll @ 2017-02-01 15:45 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Alex Deucher, Christian König, linux-pm, linux-pci,
	linux-kernel, amd-gfx, dri-devel, Maarten Lankhorst

2017-01-30 21:41 GMT+01:00 Bjorn Helgaas <bhelgaas@google.com>:
> The PCI Power Management Spec, r1.2, sec 5.6.1, requires a 10 millisecond
> delay when powering on a device, i.e., transitioning from state D3hot to
> D0.
>
> Apparently some devices require more time, and d1f9809ed131 ("drm/radeon:
> add quirk for d3 delay during switcheroo poweron for apple macbooks") added
> an additional delay for the Radeon device in a MacBook Pro.  4807c5a8a0c8
> ("drm/radeon: add a PX quirk list") made the affected device more explicit.
>
> Add a generic PCI quirk to increase the d3_delay.  This means we will use
> the additional delay for *all* wakeups from D3, not just those initiated by
> radeon_switcheroo_set_state().
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> CC: Maarten Lankhorst <maarten.lankhorst@canonical.com>
> ---
>  drivers/gpu/drm/radeon/radeon_device.c |   12 ------------
>  drivers/pci/quirks.c                   |   13 +++++++++++++
>  2 files changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
> index 8a1df2a1afbd..8b8fd981cae5 100644
> --- a/drivers/gpu/drm/radeon/radeon_device.c
> +++ b/drivers/gpu/drm/radeon/radeon_device.c
> @@ -113,7 +113,6 @@ static inline bool radeon_is_atpx_hybrid(void) { return false; }
>  #endif
>
>  #define RADEON_PX_QUIRK_DISABLE_PX  (1 << 0)
> -#define RADEON_PX_QUIRK_LONG_WAKEUP (1 << 1)
>
>  struct radeon_px_quirk {
>         u32 chip_vendor;
> @@ -136,9 +135,6 @@ static struct radeon_px_quirk radeon_px_quirk_list[] = {
>          * https://bugzilla.kernel.org/show_bug.cgi?id=51381
>          */
>         { PCI_VENDOR_ID_ATI, 0x6840, 0x1043, 0x2122, RADEON_PX_QUIRK_DISABLE_PX },
> -       /* macbook pro 8.2 */
> -       { PCI_VENDOR_ID_ATI, 0x6741, PCI_VENDOR_ID_APPLE, 0x00e2, RADEON_PX_QUIRK_LONG_WAKEUP },

Do you intentionally removed the following line?
> -       { 0, 0, 0, 0, 0 },

>  };
>
>  bool radeon_is_px(struct drm_device *dev)
> @@ -1241,25 +1237,17 @@ static void radeon_check_arguments(struct radeon_device *rdev)
>  static void radeon_switcheroo_set_state(struct pci_dev *pdev, enum vga_switcheroo_state state)
>  {
>         struct drm_device *dev = pci_get_drvdata(pdev);
> -       struct radeon_device *rdev = dev->dev_private;
>
>         if (radeon_is_px(dev) && state == VGA_SWITCHEROO_OFF)
>                 return;
>
>         if (state == VGA_SWITCHEROO_ON) {
> -               unsigned d3_delay = dev->pdev->d3_delay;
> -
>                 printk(KERN_INFO "radeon: switched on\n");
>                 /* don't suspend or resume card normally */
>                 dev->switch_power_state = DRM_SWITCH_POWER_CHANGING;
>
> -               if (d3_delay < 20 && (rdev->px_quirk_flags & RADEON_PX_QUIRK_LONG_WAKEUP))
> -                       dev->pdev->d3_delay = 20;
> -
>                 radeon_resume_kms(dev, true, true);
>
> -               dev->pdev->d3_delay = d3_delay;
> -
>                 dev->switch_power_state = DRM_SWITCH_POWER_ON;
>                 drm_kms_helper_poll_enable(dev);
>         } else {
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 1800befa8b8b..512d7a875d62 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -1683,6 +1683,19 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,     0x2609, quirk_intel_pcie_pm);
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,   0x260a, quirk_intel_pcie_pm);
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,   0x260b, quirk_intel_pcie_pm);
>
> +static void quirk_radeon_pm(struct pci_dev *dev)
> +{
> +       if (dev->subsystem_vendor == PCI_VENDOR_ID_APPLE &&
> +           dev->subsystem_device == 0x00e2) {
> +               if (dev->d3_delay < 20) {
> +                       dev->d3_delay = 20;
> +                       dev_info(&dev->dev, "extending delay after power-on from D3 to %d msec\n",
> +                                dev->d3_delay);
> +               }
> +       }
> +}
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x6741, quirk_radeon_pm);
> +
>  #ifdef CONFIG_X86_IO_APIC
>  /*
>   * Boot interrupts on some chipsets cannot be turned off. For these chipsets,
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 2/2] drm/radeon: make MacBook Pro d3_delay quirk more generic
  2017-02-01 15:45   ` Andreas Boll
@ 2017-02-01 16:15     ` Bjorn Helgaas
  0 siblings, 0 replies; 10+ messages in thread
From: Bjorn Helgaas @ 2017-02-01 16:15 UTC (permalink / raw)
  To: Andreas Boll
  Cc: Bjorn Helgaas, Alex Deucher, Christian König, linux-pm,
	linux-pci, linux-kernel, amd-gfx, dri-devel, Maarten Lankhorst

On Wed, Feb 01, 2017 at 04:45:42PM +0100, Andreas Boll wrote:
> 2017-01-30 21:41 GMT+01:00 Bjorn Helgaas <bhelgaas@google.com>:
> > The PCI Power Management Spec, r1.2, sec 5.6.1, requires a 10 millisecond
> > delay when powering on a device, i.e., transitioning from state D3hot to
> > D0.
> >
> > Apparently some devices require more time, and d1f9809ed131 ("drm/radeon:
> > add quirk for d3 delay during switcheroo poweron for apple macbooks") added
> > an additional delay for the Radeon device in a MacBook Pro.  4807c5a8a0c8
> > ("drm/radeon: add a PX quirk list") made the affected device more explicit.
> >
> > Add a generic PCI quirk to increase the d3_delay.  This means we will use
> > the additional delay for *all* wakeups from D3, not just those initiated by
> > radeon_switcheroo_set_state().
> >
> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> > CC: Maarten Lankhorst <maarten.lankhorst@canonical.com>
> > ---
> >  drivers/gpu/drm/radeon/radeon_device.c |   12 ------------
> >  drivers/pci/quirks.c                   |   13 +++++++++++++
> >  2 files changed, 13 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
> > index 8a1df2a1afbd..8b8fd981cae5 100644
> > --- a/drivers/gpu/drm/radeon/radeon_device.c
> > +++ b/drivers/gpu/drm/radeon/radeon_device.c
> > @@ -113,7 +113,6 @@ static inline bool radeon_is_atpx_hybrid(void) { return false; }
> >  #endif
> >
> >  #define RADEON_PX_QUIRK_DISABLE_PX  (1 << 0)
> > -#define RADEON_PX_QUIRK_LONG_WAKEUP (1 << 1)
> >
> >  struct radeon_px_quirk {
> >         u32 chip_vendor;
> > @@ -136,9 +135,6 @@ static struct radeon_px_quirk radeon_px_quirk_list[] = {
> >          * https://bugzilla.kernel.org/show_bug.cgi?id=51381
> >          */
> >         { PCI_VENDOR_ID_ATI, 0x6840, 0x1043, 0x2122, RADEON_PX_QUIRK_DISABLE_PX },
> > -       /* macbook pro 8.2 */
> > -       { PCI_VENDOR_ID_ATI, 0x6741, PCI_VENDOR_ID_APPLE, 0x00e2, RADEON_PX_QUIRK_LONG_WAKEUP },
> 
> Do you intentionally removed the following line?
> > -       { 0, 0, 0, 0, 0 },

Ooh, definitely not!  Thanks for catching this.  I'll post a v2 that fixes
this.

Bjorn

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-02-01 16:15 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-30 20:40 [PATCH v1 0/2] drm amdgpu/radeon: clean up d3_delay usage Bjorn Helgaas
2017-01-30 20:40 ` [PATCH v1 1/2] drm/amdgpu: remove unnecessary save/restore of pdev->d3_delay Bjorn Helgaas
2017-01-30 20:40   ` Bjorn Helgaas
2017-01-30 20:41 ` [PATCH v1 2/2] drm/radeon: make MacBook Pro d3_delay quirk more generic Bjorn Helgaas
2017-01-30 20:41   ` Bjorn Helgaas
2017-01-30 20:57   ` Deucher, Alexander
2017-01-30 20:57     ` Deucher, Alexander
2017-01-30 20:57     ` Deucher, Alexander
2017-02-01 15:45   ` Andreas Boll
2017-02-01 16:15     ` Bjorn Helgaas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.