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

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.

Changes from v1 to v2:
  - Fix accidental removal of "{ 0, 0, 0, 0, 0 }" quirk list termination
    (thanks, Andreas Boll!)
  - Add ack from Alex Deucher
---

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     |   11 -----------
 drivers/pci/quirks.c                       |   13 +++++++++++++
 3 files changed, 13 insertions(+), 15 deletions(-)

^ permalink raw reply	[flat|nested] 7+ messages in thread
* [PATCH v2 0/2] drm amdgpu/radeon: clean up d3_delay usage
@ 2017-02-01 16:22 ` Bjorn Helgaas
  0 siblings, 0 replies; 7+ messages in thread
From: Bjorn Helgaas @ 2017-02-01 16:22 UTC (permalink / raw)
  To: Alex Deucher, Christian König
  Cc: linux-pm, linux-pci, linux-kernel, amd-gfx, dri-devel, 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.

Changes from v1 to v2:
  - Fix accidental removal of "{ 0, 0, 0, 0, 0 }" quirk list termination
    (thanks, Andreas Boll!)
  - Add ack from Alex Deucher
---

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     |   11 -----------
 drivers/pci/quirks.c                       |   13 +++++++++++++
 3 files changed, 13 insertions(+), 15 deletions(-)
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread
* [PATCH v2 1/2] drm/amdgpu: remove unnecessary save/restore of pdev->d3_delay
  2017-02-01 16:22 ` Bjorn Helgaas
@ 2017-02-01 16:22   ` Bjorn Helgaas
  -1 siblings, 0 replies; 7+ messages in thread
From: Bjorn Helgaas @ 2017-02-01 16:22 UTC (permalink / raw)
  To: Alex Deucher, Christian König
  Cc: linux-pm, linux-pci, linux-kernel, dri-devel, amd-gfx,
	Maarten Lankhorst, Andreas Boll

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>
Acked-by: Alex Deucher <alexander.deucher@amd.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] 7+ messages in thread
* [PATCH v2 1/2] drm/amdgpu: remove unnecessary save/restore of pdev->d3_delay
@ 2017-02-01 16:22   ` Bjorn Helgaas
  0 siblings, 0 replies; 7+ messages in thread
From: Bjorn Helgaas @ 2017-02-01 16:22 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>
Acked-by: Alex Deucher <alexander.deucher@amd.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] 7+ messages in thread
* [PATCH v2 2/2] drm/radeon: make MacBook Pro d3_delay quirk more generic
  2017-02-01 16:22 ` Bjorn Helgaas
@ 2017-02-01 16:22   ` Bjorn Helgaas
  -1 siblings, 0 replies; 7+ messages in thread
From: Bjorn Helgaas @ 2017-02-01 16:22 UTC (permalink / raw)
  To: Alex Deucher, Christian König
  Cc: linux-pm, linux-pci, linux-kernel, dri-devel, amd-gfx,
	Maarten Lankhorst, Andreas Boll

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>
Acked-by: Alex Deucher <alexander.deucher@amd.com>
CC: Maarten Lankhorst <maarten.lankhorst@canonical.com>
---
 drivers/gpu/drm/radeon/radeon_device.c |   11 -----------
 drivers/pci/quirks.c                   |   13 +++++++++++++
 2 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
index 8a1df2a1afbd..e954c9cdcfc1 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,8 +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 },
 };
 
@@ -1241,25 +1238,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] 7+ messages in thread
* [PATCH v2 2/2] drm/radeon: make MacBook Pro d3_delay quirk more generic
@ 2017-02-01 16:22   ` Bjorn Helgaas
  0 siblings, 0 replies; 7+ messages in thread
From: Bjorn Helgaas @ 2017-02-01 16:22 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>
Acked-by: Alex Deucher <alexander.deucher@amd.com>
CC: Maarten Lankhorst <maarten.lankhorst@canonical.com>
---
 drivers/gpu/drm/radeon/radeon_device.c |   11 -----------
 drivers/pci/quirks.c                   |   13 +++++++++++++
 2 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
index 8a1df2a1afbd..e954c9cdcfc1 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,8 +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 },
 };
 
@@ -1241,25 +1238,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] 7+ messages in thread
* Re: [PATCH v2 0/2] drm amdgpu/radeon: clean up d3_delay usage
  2017-02-01 16:22 ` Bjorn Helgaas
                   ` (2 preceding siblings ...)
  (?)
@ 2017-02-01 16:30 ` Andreas Boll
  -1 siblings, 0 replies; 7+ messages in thread
From: Andreas Boll @ 2017-02-01 16:30 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Alex Deucher, Christian König, linux-pm, linux-pci,
	linux-kernel, amd-gfx, dri-devel, Maarten Lankhorst

For the series:
Reviewed-by: Andreas Boll <andreas.boll.dev@gmail.com>

2017-02-01 17:22 GMT+01:00 Bjorn Helgaas <bhelgaas@google.com>:
> 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.
>
> Changes from v1 to v2:
>   - Fix accidental removal of "{ 0, 0, 0, 0, 0 }" quirk list termination
>     (thanks, Andreas Boll!)
>   - Add ack from Alex Deucher
> ---
>
> 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     |   11 -----------
>  drivers/pci/quirks.c                       |   13 +++++++++++++
>  3 files changed, 13 insertions(+), 15 deletions(-)
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-02-01 16:30 UTC | newest]

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

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.