dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/27] add pm_runtime_last_busy_and_autosuspend() helper
@ 2014-09-24 16:14 Vinod Koul
  2014-09-24 16:14 ` [PATCH 04/27] drm/i915: use pm_runtime_last_busy_and_autosuspend helper Vinod Koul
                   ` (4 more replies)
  0 siblings, 5 replies; 18+ messages in thread
From: Vinod Koul @ 2014-09-24 16:14 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-wireless, Baruch Siach, patches, linux-doc, Seungwon Jeon,
	Daniel Vetter, Chris Ball, dri-devel, Marcin Wojtas,
	Rafael J. Wysocki, Laurent Pinchart, David Herrmann,
	Shinya Kuribayashi, Laurent Pinchart, vinod.koul, Jingoo Han,
	Jaehoon Chung, Jani Nikula, Tomi Valkeinen, Alan Stern,
	Ben Skeggs, Bjorn Andersson, Wolfram Sang

This patch series adds a simple macro pm_runtime_last_busy_and_autosuspend()
which invokes pm_runtime_mark_last_busy() and pm_runtime_put_autosuspend()
sequentially. Then we do a tree wide update of current patterns which are
present. As evident from log below this pattern is frequent in the
kernel.

This series can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/vkoul/slave-dma.git
topic/pm_runtime_last_busy_and_autosuspend

Fengguang's kbuild has tested it so it shouldn't break things for anyone.
Barring one patch (explictyly mentioned in its changelog) rest are simple
replacements.

If all are okay, this should be merged thru PM tree as it depends on macro
addition.

Subhransu S. Prusty (1):
  PM: Add helper pm_runtime_last_busy_and_autosuspend()

Vinod Koul (26):
  dmaengine: ste_dma: use pm_runtime_last_busy_and_autosuspend helper
  extcon: arizona: use pm_runtime_last_busy_and_autosuspend helper
  drm/i915: use pm_runtime_last_busy_and_autosuspend helper
  drm/nouveau: use pm_runtime_last_busy_and_autosuspend helper
  drm/radeon: use pm_runtime_last_busy_and_autosuspend helper
  vga_switcheroo: use pm_runtime_last_busy_and_autosuspend helper
  i2c: designware: use pm_runtime_last_busy_and_autosuspend helper
  i2c: omap: use pm_runtime_last_busy_and_autosuspend helper
  i2c: qup: use pm_runtime_last_busy_and_autosuspend helper
  mfd: ab8500-gpadc: use pm_runtime_last_busy_and_autosuspend helper
  mfd: arizona: use pm_runtime_last_busy_and_autosuspend helper
  mei: use pm_runtime_last_busy_and_autosuspend helper
  mmc: use pm_runtime_last_busy_and_autosuspend helper
  mmc: mmci: use pm_runtime_last_busy_and_autosuspend helper
  mmc: omap_hsmmc: use pm_runtime_last_busy_and_autosuspend helper
  mmc: sdhci-pxav3: use pm_runtime_last_busy_and_autosuspend helper
  mmc: sdhci: use pm_runtime_last_busy_and_autosuspend helper
  NFC: trf7970a: use pm_runtime_last_busy_and_autosuspend helper
  pm2301-charger: use pm_runtime_last_busy_and_autosuspend helper
  spi: omap2-mcspi: use pm_runtime_last_busy_and_autosuspend helper
  spi: orion: use pm_runtime_last_busy_and_autosuspend helper
  spi: ti-qspi: use pm_runtime_last_busy_and_autosuspend helper
  spi: core: use pm_runtime_last_busy_and_autosuspend helper
  tty: serial: omap: use pm_runtime_last_busy_and_autosuspend helper
  usb: musb: omap2430: use pm_runtime_last_busy_and_autosuspend helper
  video: fbdev: use pm_runtime_last_busy_and_autosuspend helper

 Documentation/power/runtime_pm.txt          |    4 ++
 drivers/dma/ste_dma40.c                     |   30 ++++---------
 drivers/extcon/extcon-arizona.c             |    6 +--
 drivers/gpu/drm/i915/intel_pm.c             |    3 +-
 drivers/gpu/drm/nouveau/nouveau_connector.c |    3 +-
 drivers/gpu/drm/nouveau/nouveau_drm.c       |    9 +---
 drivers/gpu/drm/radeon/radeon_connectors.c  |   15 ++----
 drivers/gpu/drm/radeon/radeon_drv.c         |    5 +-
 drivers/gpu/drm/radeon/radeon_kms.c         |    6 +--
 drivers/gpu/vga/vga_switcheroo.c            |    7 +--
 drivers/i2c/busses/i2c-designware-core.c    |    3 +-
 drivers/i2c/busses/i2c-omap.c               |    6 +--
 drivers/i2c/busses/i2c-qup.c                |    3 +-
 drivers/mfd/ab8500-gpadc.c                  |    6 +--
 drivers/mfd/arizona-irq.c                   |    3 +-
 drivers/misc/mei/client.c                   |   12 ++----
 drivers/mmc/core/core.c                     |    3 +-
 drivers/mmc/host/mmci.c                     |   12 ++----
 drivers/mmc/host/omap_hsmmc.c               |   19 ++-------
 drivers/mmc/host/sdhci-pxav3.c              |    6 +--
 drivers/mmc/host/sdhci.c                    |    3 +-
 drivers/nfc/trf7970a.c                      |    3 +-
 drivers/power/pm2301_charger.c              |    3 +-
 drivers/spi/spi-omap2-mcspi.c               |    9 +---
 drivers/spi/spi-orion.c                     |    3 +-
 drivers/spi/spi-ti-qspi.c                   |    5 +-
 drivers/spi/spi.c                           |    6 +--
 drivers/tty/serial/omap-serial.c            |   60 +++++++++------------------
 drivers/usb/musb/omap2430.c                 |    6 +--
 drivers/video/fbdev/auo_k190x.c             |    9 +---
 include/linux/pm_runtime.h                  |    6 +++
 31 files changed, 97 insertions(+), 177 deletions(-)


Thanks
-- 
~Vinod

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

* [PATCH 04/27] drm/i915: use pm_runtime_last_busy_and_autosuspend helper
  2014-09-24 16:14 [PATCH 00/27] add pm_runtime_last_busy_and_autosuspend() helper Vinod Koul
@ 2014-09-24 16:14 ` Vinod Koul
  2014-09-24 18:35   ` Daniel Vetter
  2014-09-24 16:14 ` [PATCH 05/27] drm/nouveau: " Vinod Koul
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 18+ messages in thread
From: Vinod Koul @ 2014-09-24 16:14 UTC (permalink / raw)
  To: linux-kernel
  Cc: vinod.koul, Daniel Vetter, intel-gfx, David Airlie, dri-devel,
	subhransu.s.prusty

Use the new pm_runtime_last_busy_and_autosuspend helper instead of open
coding the same code

Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 40c1229..1ec9b8d 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6729,8 +6729,7 @@ void intel_runtime_pm_put(struct drm_i915_private *dev_priv)
 	if (!HAS_RUNTIME_PM(dev))
 		return;
 
-	pm_runtime_mark_last_busy(device);
-	pm_runtime_put_autosuspend(device);
+	pm_runtime_last_busy_and_autosuspend(device);
 }
 
 void intel_init_runtime_pm(struct drm_i915_private *dev_priv)
-- 
1.7.0.4

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

* [PATCH 05/27] drm/nouveau: use pm_runtime_last_busy_and_autosuspend helper
  2014-09-24 16:14 [PATCH 00/27] add pm_runtime_last_busy_and_autosuspend() helper Vinod Koul
  2014-09-24 16:14 ` [PATCH 04/27] drm/i915: use pm_runtime_last_busy_and_autosuspend helper Vinod Koul
@ 2014-09-24 16:14 ` Vinod Koul
  2014-09-24 16:14 ` [PATCH 06/27] drm/radeon: " Vinod Koul
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 18+ messages in thread
From: Vinod Koul @ 2014-09-24 16:14 UTC (permalink / raw)
  To: linux-kernel
  Cc: subhransu.s.prusty, vinod.koul, David Airlie, Ben Skeggs,
	Dave Airlie, Daniel Vetter, David Herrmann, Ilia Mirkin,
	Jani Nikula, Alexandre Courbot, Rob Clark, Thierry Reding,
	dri-devel

Use the new pm_runtime_last_busy_and_autosuspend helper instead of open
coding the same code

Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 drivers/gpu/drm/nouveau/nouveau_connector.c |    3 +--
 drivers/gpu/drm/nouveau/nouveau_drm.c       |    9 +++------
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
index 1ec44c8..0e3108a 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -323,8 +323,7 @@ detect_analog:
 
  out:
 
-	pm_runtime_mark_last_busy(connector->dev->dev);
-	pm_runtime_put_autosuspend(connector->dev->dev);
+	pm_runtime_last_busy_and_autosuspend(connector->dev->dev);
 
 	return conn_status;
 }
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 250a5e8..719eabf 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -766,8 +766,7 @@ nouveau_drm_open(struct drm_device *dev, struct drm_file *fpriv)
 	mutex_unlock(&drm->client.mutex);
 
 out_suspend:
-	pm_runtime_mark_last_busy(dev->dev);
-	pm_runtime_put_autosuspend(dev->dev);
+	pm_runtime_last_busy_and_autosuspend(dev->dev);
 
 	return ret;
 }
@@ -794,8 +793,7 @@ nouveau_drm_postclose(struct drm_device *dev, struct drm_file *fpriv)
 {
 	struct nouveau_cli *cli = nouveau_cli(fpriv);
 	nouveau_cli_destroy(cli);
-	pm_runtime_mark_last_busy(dev->dev);
-	pm_runtime_put_autosuspend(dev->dev);
+	pm_runtime_last_busy_and_autosuspend(dev->dev);
 }
 
 static const struct drm_ioctl_desc
@@ -834,8 +832,7 @@ nouveau_drm_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		break;
 	}
 
-	pm_runtime_mark_last_busy(dev->dev);
-	pm_runtime_put_autosuspend(dev->dev);
+	pm_runtime_last_busy_and_autosuspend(dev->dev);
 	return ret;
 }
 
-- 
1.7.0.4

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

* [PATCH 06/27] drm/radeon: use pm_runtime_last_busy_and_autosuspend helper
  2014-09-24 16:14 [PATCH 00/27] add pm_runtime_last_busy_and_autosuspend() helper Vinod Koul
  2014-09-24 16:14 ` [PATCH 04/27] drm/i915: use pm_runtime_last_busy_and_autosuspend helper Vinod Koul
  2014-09-24 16:14 ` [PATCH 05/27] drm/nouveau: " Vinod Koul
@ 2014-09-24 16:14 ` Vinod Koul
  2014-09-24 20:37   ` Alex Deucher
  2014-09-24 16:14 ` [PATCH 07/27] vga_switcheroo: " Vinod Koul
  2014-09-24 20:28 ` [PATCH 00/27] add pm_runtime_last_busy_and_autosuspend() helper Rafael J. Wysocki
  4 siblings, 1 reply; 18+ messages in thread
From: Vinod Koul @ 2014-09-24 16:14 UTC (permalink / raw)
  To: linux-kernel
  Cc: subhransu.s.prusty, vinod.koul, Alex Deucher,
	Christian König, David Airlie, dri-devel

Use the new pm_runtime_last_busy_and_autosuspend helper instead of open
coding the same code

Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 drivers/gpu/drm/radeon/radeon_connectors.c |   15 +++++----------
 drivers/gpu/drm/radeon/radeon_drv.c        |    5 ++---
 drivers/gpu/drm/radeon/radeon_kms.c        |    6 ++----
 3 files changed, 9 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c
index 300c4b3..b48cf9d 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -852,8 +852,7 @@ radeon_lvds_detect(struct drm_connector *connector, bool force)
 	/* check acpi lid status ??? */
 
 	radeon_connector_update_scratch_regs(connector, ret);
-	pm_runtime_mark_last_busy(connector->dev->dev);
-	pm_runtime_put_autosuspend(connector->dev->dev);
+	pm_runtime_last_busy_and_autosuspend(connector->dev->dev);
 	return ret;
 }
 
@@ -1025,8 +1024,7 @@ radeon_vga_detect(struct drm_connector *connector, bool force)
 	radeon_connector_update_scratch_regs(connector, ret);
 
 out:
-	pm_runtime_mark_last_busy(connector->dev->dev);
-	pm_runtime_put_autosuspend(connector->dev->dev);
+	pm_runtime_last_busy_and_autosuspend(connector->dev->dev);
 
 	return ret;
 }
@@ -1103,8 +1101,7 @@ radeon_tv_detect(struct drm_connector *connector, bool force)
 	if (ret == connector_status_connected)
 		ret = radeon_connector_analog_encoder_conflict_solve(connector, encoder, ret, false);
 	radeon_connector_update_scratch_regs(connector, ret);
-	pm_runtime_mark_last_busy(connector->dev->dev);
-	pm_runtime_put_autosuspend(connector->dev->dev);
+	pm_runtime_last_busy_and_autosuspend(connector->dev->dev);
 	return ret;
 }
 
@@ -1321,8 +1318,7 @@ out:
 	radeon_connector_update_scratch_regs(connector, ret);
 
 exit:
-	pm_runtime_mark_last_busy(connector->dev->dev);
-	pm_runtime_put_autosuspend(connector->dev->dev);
+	pm_runtime_last_busy_and_autosuspend(connector->dev->dev);
 
 	return ret;
 }
@@ -1638,8 +1634,7 @@ radeon_dp_detect(struct drm_connector *connector, bool force)
 
 	radeon_connector_update_scratch_regs(connector, ret);
 out:
-	pm_runtime_mark_last_busy(connector->dev->dev);
-	pm_runtime_put_autosuspend(connector->dev->dev);
+	pm_runtime_last_busy_and_autosuspend(connector->dev->dev);
 
 	return ret;
 }
diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
index 092d067..548d129 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -503,9 +503,8 @@ long radeon_drm_ioctl(struct file *filp,
 		return ret;
 
 	ret = drm_ioctl(filp, cmd, arg);
-	
-	pm_runtime_mark_last_busy(dev->dev);
-	pm_runtime_put_autosuspend(dev->dev);
+
+	pm_runtime_last_busy_and_autosuspend(dev->dev);
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c
index eb7164d..80f061d 100644
--- a/drivers/gpu/drm/radeon/radeon_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_kms.c
@@ -147,8 +147,7 @@ int radeon_driver_load_kms(struct drm_device *dev, unsigned long flags)
 		pm_runtime_set_autosuspend_delay(dev->dev, 5000);
 		pm_runtime_set_active(dev->dev);
 		pm_runtime_allow(dev->dev);
-		pm_runtime_mark_last_busy(dev->dev);
-		pm_runtime_put_autosuspend(dev->dev);
+		pm_runtime_last_busy_and_autosuspend(dev->dev);
 	}
 
 out:
@@ -632,8 +631,7 @@ int radeon_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv)
 		file_priv->driver_priv = fpriv;
 	}
 
-	pm_runtime_mark_last_busy(dev->dev);
-	pm_runtime_put_autosuspend(dev->dev);
+	pm_runtime_last_busy_and_autosuspend(dev->dev);
 	return 0;
 }
 
-- 
1.7.0.4

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

* [PATCH 07/27] vga_switcheroo: use pm_runtime_last_busy_and_autosuspend helper
  2014-09-24 16:14 [PATCH 00/27] add pm_runtime_last_busy_and_autosuspend() helper Vinod Koul
                   ` (2 preceding siblings ...)
  2014-09-24 16:14 ` [PATCH 06/27] drm/radeon: " Vinod Koul
@ 2014-09-24 16:14 ` Vinod Koul
  2014-09-24 20:37   ` Alex Deucher
  2014-09-24 20:28 ` [PATCH 00/27] add pm_runtime_last_busy_and_autosuspend() helper Rafael J. Wysocki
  4 siblings, 1 reply; 18+ messages in thread
From: Vinod Koul @ 2014-09-24 16:14 UTC (permalink / raw)
  To: linux-kernel; +Cc: subhransu.s.prusty, vinod.koul, David Airlie, dri-devel

Use the new pm_runtime_last_busy_and_autosuspend helper instead of open
coding the same code

Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 drivers/gpu/vga/vga_switcheroo.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
index 6866448..697ffbd 100644
--- a/drivers/gpu/vga/vga_switcheroo.c
+++ b/drivers/gpu/vga/vga_switcheroo.c
@@ -682,10 +682,9 @@ static int vga_switcheroo_runtime_resume_hdmi_audio(struct device *dev)
 	ret = dev->bus->pm->runtime_resume(dev);
 
 	/* put the reference for the gpu */
-	if (found) {
-		pm_runtime_mark_last_busy(&found->pdev->dev);
-		pm_runtime_put_autosuspend(&found->pdev->dev);
-	}
+	if (found)
+		pm_runtime_last_busy_and_autosuspend(&found->pdev->dev);
+
 	return ret;
 }
 
-- 
1.7.0.4

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

* Re: [PATCH 04/27] drm/i915: use pm_runtime_last_busy_and_autosuspend helper
  2014-09-24 16:14 ` [PATCH 04/27] drm/i915: use pm_runtime_last_busy_and_autosuspend helper Vinod Koul
@ 2014-09-24 18:35   ` Daniel Vetter
  0 siblings, 0 replies; 18+ messages in thread
From: Daniel Vetter @ 2014-09-24 18:35 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Daniel Vetter, intel-gfx, linux-kernel, dri-devel,
	subhransu.s.prusty

On Wed, Sep 24, 2014 at 09:44:54PM +0530, Vinod Koul wrote:
> Use the new pm_runtime_last_busy_and_autosuspend helper instead of open
> coding the same code
> 
> Signed-off-by: Vinod Koul <vinod.koul@intel.com>

Ack to merge through whatever tree is appropriate for this. Or tell me
when I should pick this up for drm-intel.
-Daniel
> ---
>  drivers/gpu/drm/i915/intel_pm.c |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 40c1229..1ec9b8d 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6729,8 +6729,7 @@ void intel_runtime_pm_put(struct drm_i915_private *dev_priv)
>  	if (!HAS_RUNTIME_PM(dev))
>  		return;
>  
> -	pm_runtime_mark_last_busy(device);
> -	pm_runtime_put_autosuspend(device);
> +	pm_runtime_last_busy_and_autosuspend(device);
>  }
>  
>  void intel_init_runtime_pm(struct drm_i915_private *dev_priv)
> -- 
> 1.7.0.4
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

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

* Re: [PATCH 00/27] add pm_runtime_last_busy_and_autosuspend() helper
  2014-09-24 20:28 ` [PATCH 00/27] add pm_runtime_last_busy_and_autosuspend() helper Rafael J. Wysocki
@ 2014-09-24 20:15   ` Felipe Balbi
  2014-09-24 20:46     ` Rafael J. Wysocki
  2014-09-25  7:46   ` Vinod Koul
  1 sibling, 1 reply; 18+ messages in thread
From: Felipe Balbi @ 2014-09-24 20:15 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: linux-wireless, Baruch Siach, patches, linux-doc, Seungwon Jeon,
	Daniel Vetter, Chris Ball, dri-devel, Marcin Wojtas,
	Rafael J. Wysocki, Laurent Pinchart, Shinya Kuribayashi,
	Laurent Pinchart, Vinod Koul, Jaehoon Chung, Jani Nikula,
	Tomi Valkeinen, Alan Stern, Ben Skeggs, Bjorn Andersson,
	Wolfram Sang, Dave Airlie, Ulf Hansson


[-- Attachment #1.1: Type: text/plain, Size: 5110 bytes --]

On Wed, Sep 24, 2014 at 10:28:07PM +0200, Rafael J. Wysocki wrote:
> On Wednesday, September 24, 2014 09:44:50 PM Vinod Koul wrote:
> > This patch series adds a simple macro pm_runtime_last_busy_and_autosuspend()
> > which invokes pm_runtime_mark_last_busy() and pm_runtime_put_autosuspend()
> > sequentially. Then we do a tree wide update of current patterns which are
> > present. As evident from log below this pattern is frequent in the
> > kernel.
> > 
> > This series can be found at
> > git://git.kernel.org/pub/scm/linux/kernel/git/vkoul/slave-dma.git
> > topic/pm_runtime_last_busy_and_autosuspend
> > 
> > Fengguang's kbuild has tested it so it shouldn't break things for anyone.
> > Barring one patch (explictyly mentioned in its changelog) rest are simple
> > replacements.
> > 
> > If all are okay, this should be merged thru PM tree as it depends on macro
> > addition.
> > 
> > Subhransu S. Prusty (1):
> >   PM: Add helper pm_runtime_last_busy_and_autosuspend()
> > 
> > Vinod Koul (26):
> >   dmaengine: ste_dma: use pm_runtime_last_busy_and_autosuspend helper
> >   extcon: arizona: use pm_runtime_last_busy_and_autosuspend helper
> >   drm/i915: use pm_runtime_last_busy_and_autosuspend helper
> >   drm/nouveau: use pm_runtime_last_busy_and_autosuspend helper
> >   drm/radeon: use pm_runtime_last_busy_and_autosuspend helper
> >   vga_switcheroo: use pm_runtime_last_busy_and_autosuspend helper
> >   i2c: designware: use pm_runtime_last_busy_and_autosuspend helper
> >   i2c: omap: use pm_runtime_last_busy_and_autosuspend helper
> >   i2c: qup: use pm_runtime_last_busy_and_autosuspend helper
> >   mfd: ab8500-gpadc: use pm_runtime_last_busy_and_autosuspend helper
> >   mfd: arizona: use pm_runtime_last_busy_and_autosuspend helper
> >   mei: use pm_runtime_last_busy_and_autosuspend helper
> >   mmc: use pm_runtime_last_busy_and_autosuspend helper
> >   mmc: mmci: use pm_runtime_last_busy_and_autosuspend helper
> >   mmc: omap_hsmmc: use pm_runtime_last_busy_and_autosuspend helper
> >   mmc: sdhci-pxav3: use pm_runtime_last_busy_and_autosuspend helper
> >   mmc: sdhci: use pm_runtime_last_busy_and_autosuspend helper
> >   NFC: trf7970a: use pm_runtime_last_busy_and_autosuspend helper
> >   pm2301-charger: use pm_runtime_last_busy_and_autosuspend helper
> >   spi: omap2-mcspi: use pm_runtime_last_busy_and_autosuspend helper
> >   spi: orion: use pm_runtime_last_busy_and_autosuspend helper
> >   spi: ti-qspi: use pm_runtime_last_busy_and_autosuspend helper
> >   spi: core: use pm_runtime_last_busy_and_autosuspend helper
> >   tty: serial: omap: use pm_runtime_last_busy_and_autosuspend helper
> >   usb: musb: omap2430: use pm_runtime_last_busy_and_autosuspend helper
> >   video: fbdev: use pm_runtime_last_busy_and_autosuspend helper
> > 
> >  Documentation/power/runtime_pm.txt          |    4 ++
> >  drivers/dma/ste_dma40.c                     |   30 ++++---------
> >  drivers/extcon/extcon-arizona.c             |    6 +--
> >  drivers/gpu/drm/i915/intel_pm.c             |    3 +-
> >  drivers/gpu/drm/nouveau/nouveau_connector.c |    3 +-
> >  drivers/gpu/drm/nouveau/nouveau_drm.c       |    9 +---
> >  drivers/gpu/drm/radeon/radeon_connectors.c  |   15 ++----
> >  drivers/gpu/drm/radeon/radeon_drv.c         |    5 +-
> >  drivers/gpu/drm/radeon/radeon_kms.c         |    6 +--
> >  drivers/gpu/vga/vga_switcheroo.c            |    7 +--
> >  drivers/i2c/busses/i2c-designware-core.c    |    3 +-
> >  drivers/i2c/busses/i2c-omap.c               |    6 +--
> >  drivers/i2c/busses/i2c-qup.c                |    3 +-
> >  drivers/mfd/ab8500-gpadc.c                  |    6 +--
> >  drivers/mfd/arizona-irq.c                   |    3 +-
> >  drivers/misc/mei/client.c                   |   12 ++----
> >  drivers/mmc/core/core.c                     |    3 +-
> >  drivers/mmc/host/mmci.c                     |   12 ++----
> >  drivers/mmc/host/omap_hsmmc.c               |   19 ++-------
> >  drivers/mmc/host/sdhci-pxav3.c              |    6 +--
> >  drivers/mmc/host/sdhci.c                    |    3 +-
> >  drivers/nfc/trf7970a.c                      |    3 +-
> >  drivers/power/pm2301_charger.c              |    3 +-
> >  drivers/spi/spi-omap2-mcspi.c               |    9 +---
> >  drivers/spi/spi-orion.c                     |    3 +-
> >  drivers/spi/spi-ti-qspi.c                   |    5 +-
> >  drivers/spi/spi.c                           |    6 +--
> >  drivers/tty/serial/omap-serial.c            |   60 +++++++++------------------
> >  drivers/usb/musb/omap2430.c                 |    6 +--
> >  drivers/video/fbdev/auo_k190x.c             |    9 +---
> >  include/linux/pm_runtime.h                  |    6 +++
> >  31 files changed, 97 insertions(+), 177 deletions(-)
> 
> OK, I guess this is as good as it gets.
> 
> What tree would you like it go through?

Do we really need this new helper ? I mean, the very moment when we
decide to implement ->runtime_idle() we will need to get rid of this
change. I wonder if it's really valid...

-- 
balbi

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

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

* Re: [PATCH 00/27] add pm_runtime_last_busy_and_autosuspend() helper
  2014-09-24 16:14 [PATCH 00/27] add pm_runtime_last_busy_and_autosuspend() helper Vinod Koul
                   ` (3 preceding siblings ...)
  2014-09-24 16:14 ` [PATCH 07/27] vga_switcheroo: " Vinod Koul
@ 2014-09-24 20:28 ` Rafael J. Wysocki
  2014-09-24 20:15   ` Felipe Balbi
  2014-09-25  7:46   ` Vinod Koul
  4 siblings, 2 replies; 18+ messages in thread
From: Rafael J. Wysocki @ 2014-09-24 20:28 UTC (permalink / raw)
  To: Vinod Koul
  Cc: linux-kernel, subhransu.s.prusty, Alan Stern, Alexandre Courbot,
	Andrew Morton, Andy Gross, Baruch Siach, Ben Skeggs,
	Bjorn Andersson, Chew, Chiau Ee, Chris Ball, Dan Carpenter,
	Daniel Vetter, Dave Airlie, David Herrmann, dmaengine, dri-devel,
	Du, Wenkai, Grant Grundler, Ilia Mirkin, intel-gfx,
	Ivan T. Ivanov, Jaehoon Chung, Jani Nikula <jani.nikul>

On Wednesday, September 24, 2014 09:44:50 PM Vinod Koul wrote:
> This patch series adds a simple macro pm_runtime_last_busy_and_autosuspend()
> which invokes pm_runtime_mark_last_busy() and pm_runtime_put_autosuspend()
> sequentially. Then we do a tree wide update of current patterns which are
> present. As evident from log below this pattern is frequent in the
> kernel.
> 
> This series can be found at
> git://git.kernel.org/pub/scm/linux/kernel/git/vkoul/slave-dma.git
> topic/pm_runtime_last_busy_and_autosuspend
> 
> Fengguang's kbuild has tested it so it shouldn't break things for anyone.
> Barring one patch (explictyly mentioned in its changelog) rest are simple
> replacements.
> 
> If all are okay, this should be merged thru PM tree as it depends on macro
> addition.
> 
> Subhransu S. Prusty (1):
>   PM: Add helper pm_runtime_last_busy_and_autosuspend()
> 
> Vinod Koul (26):
>   dmaengine: ste_dma: use pm_runtime_last_busy_and_autosuspend helper
>   extcon: arizona: use pm_runtime_last_busy_and_autosuspend helper
>   drm/i915: use pm_runtime_last_busy_and_autosuspend helper
>   drm/nouveau: use pm_runtime_last_busy_and_autosuspend helper
>   drm/radeon: use pm_runtime_last_busy_and_autosuspend helper
>   vga_switcheroo: use pm_runtime_last_busy_and_autosuspend helper
>   i2c: designware: use pm_runtime_last_busy_and_autosuspend helper
>   i2c: omap: use pm_runtime_last_busy_and_autosuspend helper
>   i2c: qup: use pm_runtime_last_busy_and_autosuspend helper
>   mfd: ab8500-gpadc: use pm_runtime_last_busy_and_autosuspend helper
>   mfd: arizona: use pm_runtime_last_busy_and_autosuspend helper
>   mei: use pm_runtime_last_busy_and_autosuspend helper
>   mmc: use pm_runtime_last_busy_and_autosuspend helper
>   mmc: mmci: use pm_runtime_last_busy_and_autosuspend helper
>   mmc: omap_hsmmc: use pm_runtime_last_busy_and_autosuspend helper
>   mmc: sdhci-pxav3: use pm_runtime_last_busy_and_autosuspend helper
>   mmc: sdhci: use pm_runtime_last_busy_and_autosuspend helper
>   NFC: trf7970a: use pm_runtime_last_busy_and_autosuspend helper
>   pm2301-charger: use pm_runtime_last_busy_and_autosuspend helper
>   spi: omap2-mcspi: use pm_runtime_last_busy_and_autosuspend helper
>   spi: orion: use pm_runtime_last_busy_and_autosuspend helper
>   spi: ti-qspi: use pm_runtime_last_busy_and_autosuspend helper
>   spi: core: use pm_runtime_last_busy_and_autosuspend helper
>   tty: serial: omap: use pm_runtime_last_busy_and_autosuspend helper
>   usb: musb: omap2430: use pm_runtime_last_busy_and_autosuspend helper
>   video: fbdev: use pm_runtime_last_busy_and_autosuspend helper
> 
>  Documentation/power/runtime_pm.txt          |    4 ++
>  drivers/dma/ste_dma40.c                     |   30 ++++---------
>  drivers/extcon/extcon-arizona.c             |    6 +--
>  drivers/gpu/drm/i915/intel_pm.c             |    3 +-
>  drivers/gpu/drm/nouveau/nouveau_connector.c |    3 +-
>  drivers/gpu/drm/nouveau/nouveau_drm.c       |    9 +---
>  drivers/gpu/drm/radeon/radeon_connectors.c  |   15 ++----
>  drivers/gpu/drm/radeon/radeon_drv.c         |    5 +-
>  drivers/gpu/drm/radeon/radeon_kms.c         |    6 +--
>  drivers/gpu/vga/vga_switcheroo.c            |    7 +--
>  drivers/i2c/busses/i2c-designware-core.c    |    3 +-
>  drivers/i2c/busses/i2c-omap.c               |    6 +--
>  drivers/i2c/busses/i2c-qup.c                |    3 +-
>  drivers/mfd/ab8500-gpadc.c                  |    6 +--
>  drivers/mfd/arizona-irq.c                   |    3 +-
>  drivers/misc/mei/client.c                   |   12 ++----
>  drivers/mmc/core/core.c                     |    3 +-
>  drivers/mmc/host/mmci.c                     |   12 ++----
>  drivers/mmc/host/omap_hsmmc.c               |   19 ++-------
>  drivers/mmc/host/sdhci-pxav3.c              |    6 +--
>  drivers/mmc/host/sdhci.c                    |    3 +-
>  drivers/nfc/trf7970a.c                      |    3 +-
>  drivers/power/pm2301_charger.c              |    3 +-
>  drivers/spi/spi-omap2-mcspi.c               |    9 +---
>  drivers/spi/spi-orion.c                     |    3 +-
>  drivers/spi/spi-ti-qspi.c                   |    5 +-
>  drivers/spi/spi.c                           |    6 +--
>  drivers/tty/serial/omap-serial.c            |   60 +++++++++------------------
>  drivers/usb/musb/omap2430.c                 |    6 +--
>  drivers/video/fbdev/auo_k190x.c             |    9 +---
>  include/linux/pm_runtime.h                  |    6 +++
>  31 files changed, 97 insertions(+), 177 deletions(-)

OK, I guess this is as good as it gets.

What tree would you like it go through?


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

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

* Re: [PATCH 00/27] add pm_runtime_last_busy_and_autosuspend() helper
  2014-09-24 20:46     ` Rafael J. Wysocki
@ 2014-09-24 20:32       ` Felipe Balbi
  2014-09-25  7:57         ` Vinod Koul
  0 siblings, 1 reply; 18+ messages in thread
From: Felipe Balbi @ 2014-09-24 20:32 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: linux-wireless, Baruch Siach, patches, linux-doc, Seungwon Jeon,
	Daniel Vetter, Chris Ball, dri-devel, Marcin Wojtas,
	Rafael J. Wysocki, Laurent Pinchart, Shinya Kuribayashi,
	Laurent Pinchart, Vinod Koul, Jaehoon Chung, Jani Nikula,
	Tomi Valkeinen, Alan Stern, Ben Skeggs, Bjorn Andersson,
	Wolfram Sang, Dave Airlie, Ulf Hansson


[-- Attachment #1.1: Type: text/plain, Size: 6265 bytes --]

On Wed, Sep 24, 2014 at 10:46:17PM +0200, Rafael J. Wysocki wrote:
> On Wednesday, September 24, 2014 03:15:58 PM Felipe Balbi wrote:
> > On Wed, Sep 24, 2014 at 10:28:07PM +0200, Rafael J. Wysocki wrote:
> > > On Wednesday, September 24, 2014 09:44:50 PM Vinod Koul wrote:
> > > > This patch series adds a simple macro pm_runtime_last_busy_and_autosuspend()
> > > > which invokes pm_runtime_mark_last_busy() and pm_runtime_put_autosuspend()
> > > > sequentially. Then we do a tree wide update of current patterns which are
> > > > present. As evident from log below this pattern is frequent in the
> > > > kernel.
> > > > 
> > > > This series can be found at
> > > > git://git.kernel.org/pub/scm/linux/kernel/git/vkoul/slave-dma.git
> > > > topic/pm_runtime_last_busy_and_autosuspend
> > > > 
> > > > Fengguang's kbuild has tested it so it shouldn't break things for anyone.
> > > > Barring one patch (explictyly mentioned in its changelog) rest are simple
> > > > replacements.
> > > > 
> > > > If all are okay, this should be merged thru PM tree as it depends on macro
> > > > addition.
> > > > 
> > > > Subhransu S. Prusty (1):
> > > >   PM: Add helper pm_runtime_last_busy_and_autosuspend()
> > > > 
> > > > Vinod Koul (26):
> > > >   dmaengine: ste_dma: use pm_runtime_last_busy_and_autosuspend helper
> > > >   extcon: arizona: use pm_runtime_last_busy_and_autosuspend helper
> > > >   drm/i915: use pm_runtime_last_busy_and_autosuspend helper
> > > >   drm/nouveau: use pm_runtime_last_busy_and_autosuspend helper
> > > >   drm/radeon: use pm_runtime_last_busy_and_autosuspend helper
> > > >   vga_switcheroo: use pm_runtime_last_busy_and_autosuspend helper
> > > >   i2c: designware: use pm_runtime_last_busy_and_autosuspend helper
> > > >   i2c: omap: use pm_runtime_last_busy_and_autosuspend helper
> > > >   i2c: qup: use pm_runtime_last_busy_and_autosuspend helper
> > > >   mfd: ab8500-gpadc: use pm_runtime_last_busy_and_autosuspend helper
> > > >   mfd: arizona: use pm_runtime_last_busy_and_autosuspend helper
> > > >   mei: use pm_runtime_last_busy_and_autosuspend helper
> > > >   mmc: use pm_runtime_last_busy_and_autosuspend helper
> > > >   mmc: mmci: use pm_runtime_last_busy_and_autosuspend helper
> > > >   mmc: omap_hsmmc: use pm_runtime_last_busy_and_autosuspend helper
> > > >   mmc: sdhci-pxav3: use pm_runtime_last_busy_and_autosuspend helper
> > > >   mmc: sdhci: use pm_runtime_last_busy_and_autosuspend helper
> > > >   NFC: trf7970a: use pm_runtime_last_busy_and_autosuspend helper
> > > >   pm2301-charger: use pm_runtime_last_busy_and_autosuspend helper
> > > >   spi: omap2-mcspi: use pm_runtime_last_busy_and_autosuspend helper
> > > >   spi: orion: use pm_runtime_last_busy_and_autosuspend helper
> > > >   spi: ti-qspi: use pm_runtime_last_busy_and_autosuspend helper
> > > >   spi: core: use pm_runtime_last_busy_and_autosuspend helper
> > > >   tty: serial: omap: use pm_runtime_last_busy_and_autosuspend helper
> > > >   usb: musb: omap2430: use pm_runtime_last_busy_and_autosuspend helper
> > > >   video: fbdev: use pm_runtime_last_busy_and_autosuspend helper
> > > > 
> > > >  Documentation/power/runtime_pm.txt          |    4 ++
> > > >  drivers/dma/ste_dma40.c                     |   30 ++++---------
> > > >  drivers/extcon/extcon-arizona.c             |    6 +--
> > > >  drivers/gpu/drm/i915/intel_pm.c             |    3 +-
> > > >  drivers/gpu/drm/nouveau/nouveau_connector.c |    3 +-
> > > >  drivers/gpu/drm/nouveau/nouveau_drm.c       |    9 +---
> > > >  drivers/gpu/drm/radeon/radeon_connectors.c  |   15 ++----
> > > >  drivers/gpu/drm/radeon/radeon_drv.c         |    5 +-
> > > >  drivers/gpu/drm/radeon/radeon_kms.c         |    6 +--
> > > >  drivers/gpu/vga/vga_switcheroo.c            |    7 +--
> > > >  drivers/i2c/busses/i2c-designware-core.c    |    3 +-
> > > >  drivers/i2c/busses/i2c-omap.c               |    6 +--
> > > >  drivers/i2c/busses/i2c-qup.c                |    3 +-
> > > >  drivers/mfd/ab8500-gpadc.c                  |    6 +--
> > > >  drivers/mfd/arizona-irq.c                   |    3 +-
> > > >  drivers/misc/mei/client.c                   |   12 ++----
> > > >  drivers/mmc/core/core.c                     |    3 +-
> > > >  drivers/mmc/host/mmci.c                     |   12 ++----
> > > >  drivers/mmc/host/omap_hsmmc.c               |   19 ++-------
> > > >  drivers/mmc/host/sdhci-pxav3.c              |    6 +--
> > > >  drivers/mmc/host/sdhci.c                    |    3 +-
> > > >  drivers/nfc/trf7970a.c                      |    3 +-
> > > >  drivers/power/pm2301_charger.c              |    3 +-
> > > >  drivers/spi/spi-omap2-mcspi.c               |    9 +---
> > > >  drivers/spi/spi-orion.c                     |    3 +-
> > > >  drivers/spi/spi-ti-qspi.c                   |    5 +-
> > > >  drivers/spi/spi.c                           |    6 +--
> > > >  drivers/tty/serial/omap-serial.c            |   60 +++++++++------------------
> > > >  drivers/usb/musb/omap2430.c                 |    6 +--
> > > >  drivers/video/fbdev/auo_k190x.c             |    9 +---
> > > >  include/linux/pm_runtime.h                  |    6 +++
> > > >  31 files changed, 97 insertions(+), 177 deletions(-)
> > > 
> > > OK, I guess this is as good as it gets.
> > > 
> > > What tree would you like it go through?
> > 
> > Do we really need this new helper ? I mean, the very moment when we
> > decide to implement ->runtime_idle() we will need to get rid of this
> > change. I wonder if it's really valid...
> 
> I'm not sure I'm following?  This seems to simply implement what drivers
> have been doing already as one function.  Why would it be invalid to reduce
> code duplication?

For two reasons:

1) the helper has no inteligence whatsoever. It just calls the same
functions.

2) the duplication will vanish whenever someone implements
->runtime_idle() and have that call pm_runtime_autosuspend() (like PCI
and USB buses are doing today). This will just be yet another line that
needs to change.

Frankly though, no strong feelings, I just think it's a commit that
doesn't bring that any benefits other than looking like one line was
removed.

-- 
balbi

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

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

* Re: [PATCH 07/27] vga_switcheroo: use pm_runtime_last_busy_and_autosuspend helper
  2014-09-24 16:14 ` [PATCH 07/27] vga_switcheroo: " Vinod Koul
@ 2014-09-24 20:37   ` Alex Deucher
  0 siblings, 0 replies; 18+ messages in thread
From: Alex Deucher @ 2014-09-24 20:37 UTC (permalink / raw)
  To: Vinod Koul; +Cc: subhransu.s.prusty, LKML, Maling list - DRI developers

On Wed, Sep 24, 2014 at 12:14 PM, Vinod Koul <vinod.koul@intel.com> wrote:
> Use the new pm_runtime_last_busy_and_autosuspend helper instead of open
> coding the same code
>
> Signed-off-by: Vinod Koul <vinod.koul@intel.com>

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

> ---
>  drivers/gpu/vga/vga_switcheroo.c |    7 +++----
>  1 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
> index 6866448..697ffbd 100644
> --- a/drivers/gpu/vga/vga_switcheroo.c
> +++ b/drivers/gpu/vga/vga_switcheroo.c
> @@ -682,10 +682,9 @@ static int vga_switcheroo_runtime_resume_hdmi_audio(struct device *dev)
>         ret = dev->bus->pm->runtime_resume(dev);
>
>         /* put the reference for the gpu */
> -       if (found) {
> -               pm_runtime_mark_last_busy(&found->pdev->dev);
> -               pm_runtime_put_autosuspend(&found->pdev->dev);
> -       }
> +       if (found)
> +               pm_runtime_last_busy_and_autosuspend(&found->pdev->dev);
> +
>         return ret;
>  }
>
> --
> 1.7.0.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 06/27] drm/radeon: use pm_runtime_last_busy_and_autosuspend helper
  2014-09-24 16:14 ` [PATCH 06/27] drm/radeon: " Vinod Koul
@ 2014-09-24 20:37   ` Alex Deucher
  0 siblings, 0 replies; 18+ messages in thread
From: Alex Deucher @ 2014-09-24 20:37 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Alex Deucher, subhransu.s.prusty, LKML,
	Maling list - DRI developers, Christian König

On Wed, Sep 24, 2014 at 12:14 PM, Vinod Koul <vinod.koul@intel.com> wrote:
> Use the new pm_runtime_last_busy_and_autosuspend helper instead of open
> coding the same code
>
> Signed-off-by: Vinod Koul <vinod.koul@intel.com>

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

I don't care which tree this goes through.

> ---
>  drivers/gpu/drm/radeon/radeon_connectors.c |   15 +++++----------
>  drivers/gpu/drm/radeon/radeon_drv.c        |    5 ++---
>  drivers/gpu/drm/radeon/radeon_kms.c        |    6 ++----
>  3 files changed, 9 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c
> index 300c4b3..b48cf9d 100644
> --- a/drivers/gpu/drm/radeon/radeon_connectors.c
> +++ b/drivers/gpu/drm/radeon/radeon_connectors.c
> @@ -852,8 +852,7 @@ radeon_lvds_detect(struct drm_connector *connector, bool force)
>         /* check acpi lid status ??? */
>
>         radeon_connector_update_scratch_regs(connector, ret);
> -       pm_runtime_mark_last_busy(connector->dev->dev);
> -       pm_runtime_put_autosuspend(connector->dev->dev);
> +       pm_runtime_last_busy_and_autosuspend(connector->dev->dev);
>         return ret;
>  }
>
> @@ -1025,8 +1024,7 @@ radeon_vga_detect(struct drm_connector *connector, bool force)
>         radeon_connector_update_scratch_regs(connector, ret);
>
>  out:
> -       pm_runtime_mark_last_busy(connector->dev->dev);
> -       pm_runtime_put_autosuspend(connector->dev->dev);
> +       pm_runtime_last_busy_and_autosuspend(connector->dev->dev);
>
>         return ret;
>  }
> @@ -1103,8 +1101,7 @@ radeon_tv_detect(struct drm_connector *connector, bool force)
>         if (ret == connector_status_connected)
>                 ret = radeon_connector_analog_encoder_conflict_solve(connector, encoder, ret, false);
>         radeon_connector_update_scratch_regs(connector, ret);
> -       pm_runtime_mark_last_busy(connector->dev->dev);
> -       pm_runtime_put_autosuspend(connector->dev->dev);
> +       pm_runtime_last_busy_and_autosuspend(connector->dev->dev);
>         return ret;
>  }
>
> @@ -1321,8 +1318,7 @@ out:
>         radeon_connector_update_scratch_regs(connector, ret);
>
>  exit:
> -       pm_runtime_mark_last_busy(connector->dev->dev);
> -       pm_runtime_put_autosuspend(connector->dev->dev);
> +       pm_runtime_last_busy_and_autosuspend(connector->dev->dev);
>
>         return ret;
>  }
> @@ -1638,8 +1634,7 @@ radeon_dp_detect(struct drm_connector *connector, bool force)
>
>         radeon_connector_update_scratch_regs(connector, ret);
>  out:
> -       pm_runtime_mark_last_busy(connector->dev->dev);
> -       pm_runtime_put_autosuspend(connector->dev->dev);
> +       pm_runtime_last_busy_and_autosuspend(connector->dev->dev);
>
>         return ret;
>  }
> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
> index 092d067..548d129 100644
> --- a/drivers/gpu/drm/radeon/radeon_drv.c
> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> @@ -503,9 +503,8 @@ long radeon_drm_ioctl(struct file *filp,
>                 return ret;
>
>         ret = drm_ioctl(filp, cmd, arg);
> -
> -       pm_runtime_mark_last_busy(dev->dev);
> -       pm_runtime_put_autosuspend(dev->dev);
> +
> +       pm_runtime_last_busy_and_autosuspend(dev->dev);
>         return ret;
>  }
>
> diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c
> index eb7164d..80f061d 100644
> --- a/drivers/gpu/drm/radeon/radeon_kms.c
> +++ b/drivers/gpu/drm/radeon/radeon_kms.c
> @@ -147,8 +147,7 @@ int radeon_driver_load_kms(struct drm_device *dev, unsigned long flags)
>                 pm_runtime_set_autosuspend_delay(dev->dev, 5000);
>                 pm_runtime_set_active(dev->dev);
>                 pm_runtime_allow(dev->dev);
> -               pm_runtime_mark_last_busy(dev->dev);
> -               pm_runtime_put_autosuspend(dev->dev);
> +               pm_runtime_last_busy_and_autosuspend(dev->dev);
>         }
>
>  out:
> @@ -632,8 +631,7 @@ int radeon_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv)
>                 file_priv->driver_priv = fpriv;
>         }
>
> -       pm_runtime_mark_last_busy(dev->dev);
> -       pm_runtime_put_autosuspend(dev->dev);
> +       pm_runtime_last_busy_and_autosuspend(dev->dev);
>         return 0;
>  }
>
> --
> 1.7.0.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 00/27] add pm_runtime_last_busy_and_autosuspend() helper
  2014-09-24 20:15   ` Felipe Balbi
@ 2014-09-24 20:46     ` Rafael J. Wysocki
  2014-09-24 20:32       ` Felipe Balbi
  0 siblings, 1 reply; 18+ messages in thread
From: Rafael J. Wysocki @ 2014-09-24 20:46 UTC (permalink / raw)
  To: balbi
  Cc: linux-wireless, Baruch Siach, patches, linux-doc, Seungwon Jeon,
	Daniel Vetter, Chris Ball, dri-devel, Marcin Wojtas,
	Rafael J. Wysocki, Laurent Pinchart, David Herrmann,
	Shinya Kuribayashi, Laurent Pinchart, Vinod Koul, Jingoo Han,
	Jaehoon Chung, Jani Nikula, Tomi Valkeinen, Alan Stern,
	Ben Skeggs, Bjorn Andersson, Wolfram Sang


[-- Attachment #1.1: Type: text/plain, Size: 5602 bytes --]

On Wednesday, September 24, 2014 03:15:58 PM Felipe Balbi wrote:
> On Wed, Sep 24, 2014 at 10:28:07PM +0200, Rafael J. Wysocki wrote:
> > On Wednesday, September 24, 2014 09:44:50 PM Vinod Koul wrote:
> > > This patch series adds a simple macro pm_runtime_last_busy_and_autosuspend()
> > > which invokes pm_runtime_mark_last_busy() and pm_runtime_put_autosuspend()
> > > sequentially. Then we do a tree wide update of current patterns which are
> > > present. As evident from log below this pattern is frequent in the
> > > kernel.
> > > 
> > > This series can be found at
> > > git://git.kernel.org/pub/scm/linux/kernel/git/vkoul/slave-dma.git
> > > topic/pm_runtime_last_busy_and_autosuspend
> > > 
> > > Fengguang's kbuild has tested it so it shouldn't break things for anyone.
> > > Barring one patch (explictyly mentioned in its changelog) rest are simple
> > > replacements.
> > > 
> > > If all are okay, this should be merged thru PM tree as it depends on macro
> > > addition.
> > > 
> > > Subhransu S. Prusty (1):
> > >   PM: Add helper pm_runtime_last_busy_and_autosuspend()
> > > 
> > > Vinod Koul (26):
> > >   dmaengine: ste_dma: use pm_runtime_last_busy_and_autosuspend helper
> > >   extcon: arizona: use pm_runtime_last_busy_and_autosuspend helper
> > >   drm/i915: use pm_runtime_last_busy_and_autosuspend helper
> > >   drm/nouveau: use pm_runtime_last_busy_and_autosuspend helper
> > >   drm/radeon: use pm_runtime_last_busy_and_autosuspend helper
> > >   vga_switcheroo: use pm_runtime_last_busy_and_autosuspend helper
> > >   i2c: designware: use pm_runtime_last_busy_and_autosuspend helper
> > >   i2c: omap: use pm_runtime_last_busy_and_autosuspend helper
> > >   i2c: qup: use pm_runtime_last_busy_and_autosuspend helper
> > >   mfd: ab8500-gpadc: use pm_runtime_last_busy_and_autosuspend helper
> > >   mfd: arizona: use pm_runtime_last_busy_and_autosuspend helper
> > >   mei: use pm_runtime_last_busy_and_autosuspend helper
> > >   mmc: use pm_runtime_last_busy_and_autosuspend helper
> > >   mmc: mmci: use pm_runtime_last_busy_and_autosuspend helper
> > >   mmc: omap_hsmmc: use pm_runtime_last_busy_and_autosuspend helper
> > >   mmc: sdhci-pxav3: use pm_runtime_last_busy_and_autosuspend helper
> > >   mmc: sdhci: use pm_runtime_last_busy_and_autosuspend helper
> > >   NFC: trf7970a: use pm_runtime_last_busy_and_autosuspend helper
> > >   pm2301-charger: use pm_runtime_last_busy_and_autosuspend helper
> > >   spi: omap2-mcspi: use pm_runtime_last_busy_and_autosuspend helper
> > >   spi: orion: use pm_runtime_last_busy_and_autosuspend helper
> > >   spi: ti-qspi: use pm_runtime_last_busy_and_autosuspend helper
> > >   spi: core: use pm_runtime_last_busy_and_autosuspend helper
> > >   tty: serial: omap: use pm_runtime_last_busy_and_autosuspend helper
> > >   usb: musb: omap2430: use pm_runtime_last_busy_and_autosuspend helper
> > >   video: fbdev: use pm_runtime_last_busy_and_autosuspend helper
> > > 
> > >  Documentation/power/runtime_pm.txt          |    4 ++
> > >  drivers/dma/ste_dma40.c                     |   30 ++++---------
> > >  drivers/extcon/extcon-arizona.c             |    6 +--
> > >  drivers/gpu/drm/i915/intel_pm.c             |    3 +-
> > >  drivers/gpu/drm/nouveau/nouveau_connector.c |    3 +-
> > >  drivers/gpu/drm/nouveau/nouveau_drm.c       |    9 +---
> > >  drivers/gpu/drm/radeon/radeon_connectors.c  |   15 ++----
> > >  drivers/gpu/drm/radeon/radeon_drv.c         |    5 +-
> > >  drivers/gpu/drm/radeon/radeon_kms.c         |    6 +--
> > >  drivers/gpu/vga/vga_switcheroo.c            |    7 +--
> > >  drivers/i2c/busses/i2c-designware-core.c    |    3 +-
> > >  drivers/i2c/busses/i2c-omap.c               |    6 +--
> > >  drivers/i2c/busses/i2c-qup.c                |    3 +-
> > >  drivers/mfd/ab8500-gpadc.c                  |    6 +--
> > >  drivers/mfd/arizona-irq.c                   |    3 +-
> > >  drivers/misc/mei/client.c                   |   12 ++----
> > >  drivers/mmc/core/core.c                     |    3 +-
> > >  drivers/mmc/host/mmci.c                     |   12 ++----
> > >  drivers/mmc/host/omap_hsmmc.c               |   19 ++-------
> > >  drivers/mmc/host/sdhci-pxav3.c              |    6 +--
> > >  drivers/mmc/host/sdhci.c                    |    3 +-
> > >  drivers/nfc/trf7970a.c                      |    3 +-
> > >  drivers/power/pm2301_charger.c              |    3 +-
> > >  drivers/spi/spi-omap2-mcspi.c               |    9 +---
> > >  drivers/spi/spi-orion.c                     |    3 +-
> > >  drivers/spi/spi-ti-qspi.c                   |    5 +-
> > >  drivers/spi/spi.c                           |    6 +--
> > >  drivers/tty/serial/omap-serial.c            |   60 +++++++++------------------
> > >  drivers/usb/musb/omap2430.c                 |    6 +--
> > >  drivers/video/fbdev/auo_k190x.c             |    9 +---
> > >  include/linux/pm_runtime.h                  |    6 +++
> > >  31 files changed, 97 insertions(+), 177 deletions(-)
> > 
> > OK, I guess this is as good as it gets.
> > 
> > What tree would you like it go through?
> 
> Do we really need this new helper ? I mean, the very moment when we
> decide to implement ->runtime_idle() we will need to get rid of this
> change. I wonder if it's really valid...

I'm not sure I'm following?  This seems to simply implement what drivers
have been doing already as one function.  Why would it be invalid to reduce
code duplication?

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 00/27] add pm_runtime_last_busy_and_autosuspend() helper
  2014-09-24 20:28 ` [PATCH 00/27] add pm_runtime_last_busy_and_autosuspend() helper Rafael J. Wysocki
  2014-09-24 20:15   ` Felipe Balbi
@ 2014-09-25  7:46   ` Vinod Koul
  1 sibling, 0 replies; 18+ messages in thread
From: Vinod Koul @ 2014-09-25  7:46 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: linux-wireless, Baruch Siach, patches, linux-doc, Seungwon Jeon,
	Daniel Vetter, Chris Ball, dri-devel, Marcin Wojtas,
	Rafael J. Wysocki, Laurent Pinchart, David Herrmann,
	Shinya Kuribayashi, Laurent Pinchart, Jingoo Han, Jaehoon Chung,
	Jani Nikula, Tomi Valkeinen, Alan Stern, Ben Skeggs,
	Bjorn Andersson, Wolfram Sang, Dave Airlie

On Wed, Sep 24, 2014 at 10:28:07PM +0200, Rafael J. Wysocki wrote:
> 
> OK, I guess this is as good as it gets.
> 
> What tree would you like it go through?

Since rest of the patches are dependent upon 1st patch which should go thru
your tree, we should merge this thru your tree

Thanks
-- 
~Vinod

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

* Re: [PATCH 00/27] add pm_runtime_last_busy_and_autosuspend() helper
  2014-09-24 20:32       ` Felipe Balbi
@ 2014-09-25  7:57         ` Vinod Koul
  2014-09-25 14:22           ` Felipe Balbi
  0 siblings, 1 reply; 18+ messages in thread
From: Vinod Koul @ 2014-09-25  7:57 UTC (permalink / raw)
  To: Felipe Balbi
  Cc: linux-wireless, Baruch Siach, patches, linux-doc, linux-mmc,
	Daniel Vetter, Chris Ball, dri-devel, Marcin Wojtas,
	Rafael J. Wysocki, Laurent Pinchart, David Herrmann,
	Shinya Kuribayashi, Laurent Pinchart, Jingoo Han, Seungwon Jeon,
	Jaehoon Chung, Jani Nikula, Tomi Valkeinen, Alan Stern,
	Ben Skeggs, Bjorn Andersson, Wolfram Sang


[-- Attachment #1.1: Type: text/plain, Size: 1306 bytes --]

On Wed, Sep 24, 2014 at 03:32:19PM -0500, Felipe Balbi wrote:
> > > > OK, I guess this is as good as it gets.
> > > > 
> > > > What tree would you like it go through?
> > > 
> > > Do we really need this new helper ? I mean, the very moment when we
> > > decide to implement ->runtime_idle() we will need to get rid of this
> > > change. I wonder if it's really valid...
> > 
> > I'm not sure I'm following?  This seems to simply implement what drivers
> > have been doing already as one function.  Why would it be invalid to reduce
> > code duplication?
> 
> For two reasons:
> 
> 1) the helper has no inteligence whatsoever. It just calls the same
> functions.
> 
> 2) the duplication will vanish whenever someone implements
> ->runtime_idle() and have that call pm_runtime_autosuspend() (like PCI
> and USB buses are doing today). This will just be yet another line that
> needs to change.
> 
> Frankly though, no strong feelings, I just think it's a commit that
> doesn't bring that any benefits other than looking like one line was
> removed.
and yes that is what it tries to do nothing more nothing less. If in future
there are no users (today we have quite a few), then we can remove the dead
macro, no harm. But that is not the situation today.

Thanks
-- 
~Vinod


[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 00/27] add pm_runtime_last_busy_and_autosuspend() helper
  2014-09-25  7:57         ` Vinod Koul
@ 2014-09-25 14:22           ` Felipe Balbi
  2014-09-25 14:27             ` Wolfram Sang
  0 siblings, 1 reply; 18+ messages in thread
From: Felipe Balbi @ 2014-09-25 14:22 UTC (permalink / raw)
  To: Vinod Koul
  Cc: linux-wireless, Baruch Siach, patches, linux-doc, linux-mmc,
	Daniel Vetter, Chris Ball, dri-devel, Marcin Wojtas,
	Rafael J. Wysocki, Laurent Pinchart, David Herrmann,
	Shinya Kuribayashi, Laurent Pinchart, Jingoo Han, Seungwon Jeon,
	Jaehoon Chung, Jani Nikula, Tomi Valkeinen, Alan Stern,
	Ben Skeggs, Bjorn Andersson, Wolfram Sang


[-- Attachment #1.1: Type: text/plain, Size: 1737 bytes --]

On Thu, Sep 25, 2014 at 01:27:18PM +0530, Vinod Koul wrote:
> On Wed, Sep 24, 2014 at 03:32:19PM -0500, Felipe Balbi wrote:
> > > > > OK, I guess this is as good as it gets.
> > > > > 
> > > > > What tree would you like it go through?
> > > > 
> > > > Do we really need this new helper ? I mean, the very moment when we
> > > > decide to implement ->runtime_idle() we will need to get rid of this
> > > > change. I wonder if it's really valid...
> > > 
> > > I'm not sure I'm following?  This seems to simply implement what drivers
> > > have been doing already as one function.  Why would it be invalid to reduce
> > > code duplication?
> > 
> > For two reasons:
> > 
> > 1) the helper has no inteligence whatsoever. It just calls the same
> > functions.
> > 
> > 2) the duplication will vanish whenever someone implements
> > ->runtime_idle() and have that call pm_runtime_autosuspend() (like PCI
> > and USB buses are doing today). This will just be yet another line that
> > needs to change.
> > 
> > Frankly though, no strong feelings, I just think it's a commit that
> > doesn't bring that any benefits other than looking like one line was
> > removed.
> and yes that is what it tries to do nothing more nothing less. If in future
> there are no users (today we have quite a few), then we can remove the dead
> macro, no harm. But that is not the situation today.

as I said, a commit that's bound to be useless. It's not like you're
saving 10 lines of code, it's only one. Replacing two simple lines with
a function which takes <joke> almost as many characters to type </joke>.

IMO, this is pretty useless and I'd rather not see them in the drivers I
maintain, sorry.

cheers

-- 
balbi

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 00/27] add pm_runtime_last_busy_and_autosuspend() helper
  2014-09-25 14:22           ` Felipe Balbi
@ 2014-09-25 14:27             ` Wolfram Sang
  2014-09-25 19:54               ` Rafael J. Wysocki
  0 siblings, 1 reply; 18+ messages in thread
From: Wolfram Sang @ 2014-09-25 14:27 UTC (permalink / raw)
  To: Felipe Balbi
  Cc: linux-wireless, Baruch Siach, patches, linux-doc, linux-mmc,
	Daniel Vetter, Chris Ball, dri-devel, Marcin Wojtas,
	Rafael J. Wysocki, Laurent Pinchart, David Herrmann,
	Shinya Kuribayashi, Laurent Pinchart, Vinod Koul, Jingoo Han,
	Seungwon Jeon, Jaehoon Chung, Jani Nikula, Tomi Valkeinen,
	Alan Stern, Ben Skeggs, Bjorn Andersson


[-- Attachment #1.1: Type: text/plain, Size: 1937 bytes --]

On Thu, Sep 25, 2014 at 09:22:01AM -0500, Felipe Balbi wrote:
> On Thu, Sep 25, 2014 at 01:27:18PM +0530, Vinod Koul wrote:
> > On Wed, Sep 24, 2014 at 03:32:19PM -0500, Felipe Balbi wrote:
> > > > > > OK, I guess this is as good as it gets.
> > > > > > 
> > > > > > What tree would you like it go through?
> > > > > 
> > > > > Do we really need this new helper ? I mean, the very moment when we
> > > > > decide to implement ->runtime_idle() we will need to get rid of this
> > > > > change. I wonder if it's really valid...
> > > > 
> > > > I'm not sure I'm following?  This seems to simply implement what drivers
> > > > have been doing already as one function.  Why would it be invalid to reduce
> > > > code duplication?
> > > 
> > > For two reasons:
> > > 
> > > 1) the helper has no inteligence whatsoever. It just calls the same
> > > functions.
> > > 
> > > 2) the duplication will vanish whenever someone implements
> > > ->runtime_idle() and have that call pm_runtime_autosuspend() (like PCI
> > > and USB buses are doing today). This will just be yet another line that
> > > needs to change.
> > > 
> > > Frankly though, no strong feelings, I just think it's a commit that
> > > doesn't bring that any benefits other than looking like one line was
> > > removed.
> > and yes that is what it tries to do nothing more nothing less. If in future
> > there are no users (today we have quite a few), then we can remove the dead
> > macro, no harm. But that is not the situation today.
> 
> as I said, a commit that's bound to be useless. It's not like you're
> saving 10 lines of code, it's only one. Replacing two simple lines with
> a function which takes <joke> almost as many characters to type </joke>.
> 
> IMO, this is pretty useless and I'd rather not see them in the drivers I
> maintain, sorry.

It is not a NACK from me; yet from a high-level perspective I agree with
Felipe.


[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 00/27] add pm_runtime_last_busy_and_autosuspend() helper
  2014-09-25 14:27             ` Wolfram Sang
@ 2014-09-25 19:54               ` Rafael J. Wysocki
  2014-09-28 15:37                 ` Vinod Koul
  0 siblings, 1 reply; 18+ messages in thread
From: Rafael J. Wysocki @ 2014-09-25 19:54 UTC (permalink / raw)
  To: Wolfram Sang, Vinod Koul
  Cc: linux-wireless, Baruch Siach, patches, linux-doc, Seungwon Jeon,
	Daniel Vetter, Chris Ball, dri-devel, Marcin Wojtas,
	Rafael J. Wysocki, Laurent Pinchart, David Herrmann,
	Shinya Kuribayashi, Laurent Pinchart, Jingoo Han, Jaehoon Chung,
	Jani Nikula, Tomi Valkeinen, Alan Stern, Ben Skeggs,
	Bjorn Andersson, Dave Airlie, Ulf Hansson <ulf.hanss>

On Thursday, September 25, 2014 04:27:58 PM Wolfram Sang wrote:
> 
> --Bn2rw/3z4jIqBvZU
> Content-Type: text/plain; charset=us-ascii
> Content-Disposition: inline
> Content-Transfer-Encoding: quoted-printable
> 
> On Thu, Sep 25, 2014 at 09:22:01AM -0500, Felipe Balbi wrote:
> > On Thu, Sep 25, 2014 at 01:27:18PM +0530, Vinod Koul wrote:
> > > On Wed, Sep 24, 2014 at 03:32:19PM -0500, Felipe Balbi wrote:
> > > > > > > OK, I guess this is as good as it gets.
> > > > > > >=20
> > > > > > > What tree would you like it go through?
> > > > > >=20
> > > > > > Do we really need this new helper ? I mean, the very moment when =
> we
> > > > > > decide to implement ->runtime_idle() we will need to get rid of t=
> his
> > > > > > change. I wonder if it's really valid...
> > > > >=20
> > > > > I'm not sure I'm following?  This seems to simply implement what dr=
> ivers
> > > > > have been doing already as one function.  Why would it be invalid t=
> o reduce
> > > > > code duplication?
> > > >=20
> > > > For two reasons:
> > > >=20
> > > > 1) the helper has no inteligence whatsoever. It just calls the same
> > > > functions.
> > > >=20
> > > > 2) the duplication will vanish whenever someone implements
> > > > ->runtime_idle() and have that call pm_runtime_autosuspend() (like PCI
> > > > and USB buses are doing today). This will just be yet another line th=
> at
> > > > needs to change.
> > > >=20
> > > > Frankly though, no strong feelings, I just think it's a commit that
> > > > doesn't bring that any benefits other than looking like one line was
> > > > removed.
> > > and yes that is what it tries to do nothing more nothing less. If in fu=
> ture
> > > there are no users (today we have quite a few), then we can remove the =
> dead
> > > macro, no harm. But that is not the situation today.
> >=20
> > as I said, a commit that's bound to be useless. It's not like you're
> > saving 10 lines of code, it's only one. Replacing two simple lines with
> > a function which takes <joke> almost as many characters to type </joke>.
> >=20
> > IMO, this is pretty useless and I'd rather not see them in the drivers I
> > maintain, sorry.
> 
> It is not a NACK from me; yet from a high-level perspective I agree with
> Felipe.

OK

I'd rather not merge something that driver people don't want to use.

Vinod?

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

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

* Re: [PATCH 00/27] add pm_runtime_last_busy_and_autosuspend() helper
  2014-09-25 19:54               ` Rafael J. Wysocki
@ 2014-09-28 15:37                 ` Vinod Koul
  0 siblings, 0 replies; 18+ messages in thread
From: Vinod Koul @ 2014-09-28 15:37 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: linux-wireless, Baruch Siach, patches, Wolfram Sang, linux-mmc,
	Daniel Vetter, Chris Ball, dri-devel, Marcin Wojtas,
	Rafael J. Wysocki, Laurent Pinchart, David Herrmann,
	Shinya Kuribayashi, Laurent Pinchart, Jingoo Han, Seungwon Jeon,
	Jaehoon Chung, Jani Nikula, Tomi Valkeinen, Alan Stern,
	Ben Skeggs, Bjorn Andersson, Dave Airlie <airlied@

On Thu, Sep 25, 2014 at 09:54:36PM +0200, Rafael J. Wysocki wrote:
> On Thursday, September 25, 2014 04:27:58 PM Wolfram Sang wrote:
> > 
> > --Bn2rw/3z4jIqBvZU
> > Content-Type: text/plain; charset=us-ascii
> > Content-Disposition: inline
> > Content-Transfer-Encoding: quoted-printable
> > 
> > On Thu, Sep 25, 2014 at 09:22:01AM -0500, Felipe Balbi wrote:
> > > On Thu, Sep 25, 2014 at 01:27:18PM +0530, Vinod Koul wrote:
> > > > On Wed, Sep 24, 2014 at 03:32:19PM -0500, Felipe Balbi wrote:
> > > > > > > > OK, I guess this is as good as it gets.
> > > > > > > >=20
> > > > > > > > What tree would you like it go through?
> > > > > > >=20
> > > > > > > Do we really need this new helper ? I mean, the very moment when =
> > we
> > > > > > > decide to implement ->runtime_idle() we will need to get rid of t=
> > his
> > > > > > > change. I wonder if it's really valid...
> > > > > >=20
> > > > > > I'm not sure I'm following?  This seems to simply implement what dr=
> > ivers
> > > > > > have been doing already as one function.  Why would it be invalid t=
> > o reduce
> > > > > > code duplication?
> > > > >=20
> > > > > For two reasons:
> > > > >=20
> > > > > 1) the helper has no inteligence whatsoever. It just calls the same
> > > > > functions.
> > > > >=20
> > > > > 2) the duplication will vanish whenever someone implements
> > > > > ->runtime_idle() and have that call pm_runtime_autosuspend() (like PCI
> > > > > and USB buses are doing today). This will just be yet another line th=
> > at
> > > > > needs to change.
> > > > >=20
> > > > > Frankly though, no strong feelings, I just think it's a commit that
> > > > > doesn't bring that any benefits other than looking like one line was
> > > > > removed.
> > > > and yes that is what it tries to do nothing more nothing less. If in fu=
> > ture
> > > > there are no users (today we have quite a few), then we can remove the =
> > dead
> > > > macro, no harm. But that is not the situation today.
> > >=20
> > > as I said, a commit that's bound to be useless. It's not like you're
> > > saving 10 lines of code, it's only one. Replacing two simple lines with
> > > a function which takes <joke> almost as many characters to type </joke>.
> > >=20
> > > IMO, this is pretty useless and I'd rather not see them in the drivers I
> > > maintain, sorry.
> > 
> > It is not a NACK from me; yet from a high-level perspective I agree with
> > Felipe.
> 
> OK
> 
> I'd rather not merge something that driver people don't want to use.
> 
> Vinod?
There have been quite a few ACKs as well. Either way am okay. If you feel
this will get removed as discussed, then there is no point in merging

-- 
~Vinod

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

end of thread, other threads:[~2014-09-28 15:37 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-24 16:14 [PATCH 00/27] add pm_runtime_last_busy_and_autosuspend() helper Vinod Koul
2014-09-24 16:14 ` [PATCH 04/27] drm/i915: use pm_runtime_last_busy_and_autosuspend helper Vinod Koul
2014-09-24 18:35   ` Daniel Vetter
2014-09-24 16:14 ` [PATCH 05/27] drm/nouveau: " Vinod Koul
2014-09-24 16:14 ` [PATCH 06/27] drm/radeon: " Vinod Koul
2014-09-24 20:37   ` Alex Deucher
2014-09-24 16:14 ` [PATCH 07/27] vga_switcheroo: " Vinod Koul
2014-09-24 20:37   ` Alex Deucher
2014-09-24 20:28 ` [PATCH 00/27] add pm_runtime_last_busy_and_autosuspend() helper Rafael J. Wysocki
2014-09-24 20:15   ` Felipe Balbi
2014-09-24 20:46     ` Rafael J. Wysocki
2014-09-24 20:32       ` Felipe Balbi
2014-09-25  7:57         ` Vinod Koul
2014-09-25 14:22           ` Felipe Balbi
2014-09-25 14:27             ` Wolfram Sang
2014-09-25 19:54               ` Rafael J. Wysocki
2014-09-28 15:37                 ` Vinod Koul
2014-09-25  7:46   ` Vinod Koul

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).