[PATCH 0/6] Small Runtime PM API changes

[PATCH 0/6] Small Runtime PM API changes

[Sakari Ailus]

Hi folks,

This small set happily mixes Runtime PM and media patches.

The set does two main things Runtime PM API-wise. Firstly,
pm_runtime_get_if_active() is made more user-friendly by removing the
ign_use_count argument so the users no longer need to call it with that
set to true. Secondly, pm_runtime_put_mark_busy_autosusp() helper is added
to avoid drivers having to call pm_runtime_mark_last_busy() only to be
followed by pm_runtime_autosuspend().

The vast majority of the users of pm_runtime_autosuspend() would probably
have been fine with making pm_runtime_autosuspend() do the last busy
stamping, too, but given the sheer number of users it's hard to tell if
there could be problems here and there. On the other hand, there are
probably a sizable proportion of call sites where the missing
pm_runtime_mark_last_busy() call is simply a bug.

The three last patches are addressing Runtime PM issues in a few sensor
drivers.

Comments would be welcome.


Sakari Ailus (6):
  pm: runtime: Simplify pm_runtime_get_if_active() usage
  pm: runtime: Add pm_runtime_put_mark_busy_autosusp() helper
  media: Documentation: Improve camera sensor runtime PM documentation
  media: ov8858: Use pm_runtime_get_if_active(), put usage_count
    correctly
  media: imx319: Use pm_runtime_get_if_active(), put usage_count
    correctly
  media: imx219: Use pm_runtime_get_if_active(), put usage_count
    correctly

 .../driver-api/media/camera-sensor.rst        | 28 ++++++++---
 Documentation/power/runtime_pm.rst            |  5 +-
 drivers/base/power/runtime.c                  |  9 ++--
 drivers/gpu/drm/i915/intel_runtime_pm.c       |  2 +-
 drivers/media/i2c/ccs/ccs-core.c              |  2 +-
 drivers/media/i2c/imx219.c                    |  8 ++--
 drivers/media/i2c/imx319.c                    |  8 ++--
 drivers/media/i2c/ov8858.c                    |  8 ++--
 drivers/net/ipa/ipa_smp2p.c                   |  2 +-
 drivers/pci/pci.c                             |  2 +-
 include/linux/pm_runtime.h                    | 48 +++++++++++++++++--
 sound/hda/hdac_device.c                       |  2 +-
 12 files changed, 92 insertions(+), 32 deletions(-)


base-commit: 3e238417254bfdcc23fe207780b59cbb08656762
-- 
2.39.2

[PATCH 1/6] pm: runtime: Simplify pm_runtime_get_if_active() usage

[Sakari Ailus]

The majority of the callers currently using pm_runtime_get_if_active()
call it with ign_usage_count argument set to true. There's only one driver
using this feature (and natually implementation of
pm_runtime_get_if_in_use()).

To make this function more practical to use, remove the ign_usage_count
argument from the function. The main implementation is renamed as
__pm_runtime_get_conditional().

This function is expected to gain a large number of users in the future
--- camera sensor drivers using runtime autosuspend have a need to get the
device's usage_count conditionally if it is enabled. Most of these
currently use pm_runtime_get_if_in_use(), which is a bug.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 Documentation/power/runtime_pm.rst      |  5 ++--
 drivers/base/power/runtime.c            |  9 ++++---
 drivers/gpu/drm/i915/intel_runtime_pm.c |  2 +-
 drivers/media/i2c/ccs/ccs-core.c        |  2 +-
 drivers/net/ipa/ipa_smp2p.c             |  2 +-
 drivers/pci/pci.c                       |  2 +-
 include/linux/pm_runtime.h              | 31 +++++++++++++++++++++----
 sound/hda/hdac_device.c                 |  2 +-
 8 files changed, 39 insertions(+), 16 deletions(-)

diff --git a/Documentation/power/runtime_pm.rst b/Documentation/power/runtime_pm.rst
index 65b86e487afe..da99379071a4 100644
--- a/Documentation/power/runtime_pm.rst
+++ b/Documentation/power/runtime_pm.rst
@@ -396,10 +396,9 @@ drivers/base/power/runtime.c and include/linux/pm_runtime.h:
       nonzero, increment the counter and return 1; otherwise return 0 without
       changing the counter
 
-  `int pm_runtime_get_if_active(struct device *dev, bool ign_usage_count);`
+  `int pm_runtime_get_if_active(struct device *dev);`
     - return -EINVAL if 'power.disable_depth' is nonzero; otherwise, if the
-      runtime PM status is RPM_ACTIVE, and either ign_usage_count is true
-      or the device's usage_count is non-zero, increment the counter and
+      runtime PM status is RPM_ACTIVE, increment the counter and
       return 1; otherwise return 0 without changing the counter
 
   `void pm_runtime_put_noidle(struct device *dev);`
diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index 4545669cb973..8b56468eca9d 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -1175,7 +1175,7 @@ int __pm_runtime_resume(struct device *dev, int rpmflags)
 EXPORT_SYMBOL_GPL(__pm_runtime_resume);
 
 /**
- * pm_runtime_get_if_active - Conditionally bump up device usage counter.
+ * __pm_runtime_get_conditional - Conditionally bump up device usage counter.
  * @dev: Device to handle.
  * @ign_usage_count: Whether or not to look at the current usage counter value.
  *
@@ -1195,8 +1195,11 @@ EXPORT_SYMBOL_GPL(__pm_runtime_resume);
  *
  * The caller is responsible for decrementing the runtime PM usage counter of
  * @dev after this function has returned a positive value for it.
+ *
+ * This function is not intended to be called by drivers, use
+ * pm_runtime_get_if_active() or pm_runtime_get_if_in_use() instead.
  */
-int pm_runtime_get_if_active(struct device *dev, bool ign_usage_count)
+int __pm_runtime_get_conditional(struct device *dev, bool ign_usage_count)
 {
 	unsigned long flags;
 	int retval;
@@ -1217,7 +1220,7 @@ int pm_runtime_get_if_active(struct device *dev, bool ign_usage_count)
 
 	return retval;
 }
-EXPORT_SYMBOL_GPL(pm_runtime_get_if_active);
+EXPORT_SYMBOL_GPL(__pm_runtime_get_conditional);
 
 /**
  * __pm_runtime_set_status - Set runtime PM status of a device.
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 6d8e5e5c0cba..b163efe80975 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -434,7 +434,7 @@ static intel_wakeref_t __intel_runtime_pm_get_if_active(struct intel_runtime_pm
 		 * function, since the power state is undefined. This applies
 		 * atm to the late/early system suspend/resume handlers.
 		 */
-		if (pm_runtime_get_if_active(rpm->kdev, ignore_usecount) <= 0)
+		if (__pm_runtime_get_conditional(rpm->kdev, ignore_usecount) <= 0)
 			return 0;
 	}
 
diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
index 12e6f0a26fc8..45701a18c845 100644
--- a/drivers/media/i2c/ccs/ccs-core.c
+++ b/drivers/media/i2c/ccs/ccs-core.c
@@ -664,7 +664,7 @@ static int ccs_set_ctrl(struct v4l2_ctrl *ctrl)
 		break;
 	}
 
-	pm_status = pm_runtime_get_if_active(&client->dev, true);
+	pm_status = pm_runtime_get_if_active(&client->dev);
 	if (!pm_status)
 		return 0;
 
diff --git a/drivers/net/ipa/ipa_smp2p.c b/drivers/net/ipa/ipa_smp2p.c
index 5620dc271fac..cbf3d4761ce3 100644
--- a/drivers/net/ipa/ipa_smp2p.c
+++ b/drivers/net/ipa/ipa_smp2p.c
@@ -92,7 +92,7 @@ static void ipa_smp2p_notify(struct ipa_smp2p *smp2p)
 		return;
 
 	dev = &smp2p->ipa->pdev->dev;
-	smp2p->power_on = pm_runtime_get_if_active(dev, true) > 0;
+	smp2p->power_on = pm_runtime_get_if_active(dev) > 0;
 
 	/* Signal whether the IPA power is enabled */
 	mask = BIT(smp2p->enabled_bit);
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 59c01d68c6d5..d8d4abbc936f 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2439,7 +2439,7 @@ static void pci_pme_list_scan(struct work_struct *work)
 			 * If the device is in a low power state it
 			 * should not be polled either.
 			 */
-			pm_status = pm_runtime_get_if_active(dev, true);
+			pm_status = pm_runtime_get_if_active(dev);
 			if (!pm_status)
 				continue;
 
diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
index 7c9b35448563..810330bb802b 100644
--- a/include/linux/pm_runtime.h
+++ b/include/linux/pm_runtime.h
@@ -72,7 +72,8 @@ extern int pm_runtime_force_resume(struct device *dev);
 extern int __pm_runtime_idle(struct device *dev, int rpmflags);
 extern int __pm_runtime_suspend(struct device *dev, int rpmflags);
 extern int __pm_runtime_resume(struct device *dev, int rpmflags);
-extern int pm_runtime_get_if_active(struct device *dev, bool ign_usage_count);
+extern int __pm_runtime_get_conditional(struct device *dev,
+					bool ign_usage_count);
 extern int pm_schedule_suspend(struct device *dev, unsigned int delay);
 extern int __pm_runtime_set_status(struct device *dev, unsigned int status);
 extern int pm_runtime_barrier(struct device *dev);
@@ -94,16 +95,33 @@ extern void pm_runtime_release_supplier(struct device_link *link);
 
 extern int devm_pm_runtime_enable(struct device *dev);
 
+/**
+ * pm_runtime_get_if_active - Bump up runtime PM usage counter if the device is
+ *			      in active state
+ * @dev: Target device.
+ *
+ * Increment the runtime PM usage counter of @dev if its runtime PM status is
+ * %RPM_ACTIVE, in which case it returns 1. If the device is in a different
+ * state, 0 is returned. -EINVAL is returned if runtime PM is disabled for the
+ * device.
+ */
+static inline int pm_runtime_get_if_active(struct device *dev)
+{
+	return __pm_runtime_get_conditional(dev, true);
+}
+
 /**
  * pm_runtime_get_if_in_use - Conditionally bump up runtime PM usage counter.
  * @dev: Target device.
  *
  * Increment the runtime PM usage counter of @dev if its runtime PM status is
- * %RPM_ACTIVE and its runtime PM usage counter is greater than 0.
+ * %RPM_ACTIVE and its runtime PM usage counter is greater than 0, in which case
+ * it returns 1. If the device is in a different state or its usage_count is 0,
+ * 0 is returned. -EINVAL is returned if runtime PM is disabled for the device.
  */
 static inline int pm_runtime_get_if_in_use(struct device *dev)
 {
-	return pm_runtime_get_if_active(dev, false);
+	return __pm_runtime_get_conditional(dev, false);
 }
 
 /**
@@ -275,8 +293,11 @@ static inline int pm_runtime_get_if_in_use(struct device *dev)
 {
 	return -EINVAL;
 }
-static inline int pm_runtime_get_if_active(struct device *dev,
-					   bool ign_usage_count)
+static inline int pm_runtime_get_if_active(struct device *dev)
+{
+	return -EINVAL;
+}
+static inline int __pm_runtime_get_conditional(struct device *dev)
 {
 	return -EINVAL;
 }
diff --git a/sound/hda/hdac_device.c b/sound/hda/hdac_device.c
index bbf7bcdb449a..0a9223c18d77 100644
--- a/sound/hda/hdac_device.c
+++ b/sound/hda/hdac_device.c
@@ -611,7 +611,7 @@ EXPORT_SYMBOL_GPL(snd_hdac_power_up_pm);
 int snd_hdac_keep_power_up(struct hdac_device *codec)
 {
 	if (!atomic_inc_not_zero(&codec->in_pm)) {
-		int ret = pm_runtime_get_if_active(&codec->dev, true);
+		int ret = pm_runtime_get_if_active(&codec->dev);
 		if (!ret)
 			return -1;
 		if (ret < 0)
-- 
2.39.2

[PATCH 2/6] pm: runtime: Add pm_runtime_put_mark_busy_autosusp() helper

[Sakari Ailus]

Add pm_runtime_put_mark_busy_autosusp() helper function for users that
wish to set the last_busy timestamp to current time and put the
usage_count of the device and set the autosuspend timer.

Essentially calling pm_runtime_suspend_mark_busy_autosusp() equal to
calling first pm_runtime_mark_last_busy() and then
pm_runtime_put_autosuspend().

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 include/linux/pm_runtime.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
index 810330bb802b..59871ab1532a 100644
--- a/include/linux/pm_runtime.h
+++ b/include/linux/pm_runtime.h
@@ -494,6 +494,23 @@ static inline int pm_runtime_put_autosuspend(struct device *dev)
 	    RPM_GET_PUT | RPM_ASYNC | RPM_AUTO);
 }
 
+/**
+ * pm_runtime_put_mark_busy_autosusp - Update the last access time of a device
+ *				       and drop device usage counter and queue
+ *				       autosuspend if 0.
+ * @dev: Target device.
+ *
+ * Update the last access time of @dev using pm_runtime_mark_last_busy(), then
+ * decrement the runtime PM usage counter of @dev and if it turns out to be
+ * equal to 0, queue up a work item for @dev like in pm_request_autosuspend().
+ */
+static inline int pm_runtime_put_mark_busy_autosusp(struct device *dev)
+{
+	pm_runtime_mark_last_busy(dev);
+
+	return pm_runtime_autosuspend(dev);
+}
+
 /**
  * pm_runtime_put_sync - Drop device usage counter and run "idle check" if 0.
  * @dev: Target device.
-- 
2.39.2

[PATCH 3/6] media: Documentation: Improve camera sensor runtime PM documentation

[Sakari Ailus]

Endorse the use of pm_runtime_get_sync() in order to resume the device and
pm_runtime_get_if_active() to increment its usage_count if the device is
in RPM_ACTIVE state.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 .../driver-api/media/camera-sensor.rst        | 28 ++++++++++++++-----
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/Documentation/driver-api/media/camera-sensor.rst b/Documentation/driver-api/media/camera-sensor.rst
index 6456145f96ed..a15c09f8347e 100644
--- a/Documentation/driver-api/media/camera-sensor.rst
+++ b/Documentation/driver-api/media/camera-sensor.rst
@@ -67,11 +67,23 @@ system resources required to power the sensor up and down. For drivers that
 don't use any of those resources (such as drivers that support ACPI systems
 only), the runtime PM handlers may be left unimplemented.
 
-In general, the device shall be powered on at least when its registers are
-being accessed and when it is streaming. Drivers should use
-``pm_runtime_resume_and_get()`` when starting streaming and
-``pm_runtime_put()`` or ``pm_runtime_put_autosuspend()`` when stopping
-streaming. They may power the device up at probe time (for example to read
+In general, the device shall be powered on at least when its registers are being
+accessed and when it is streaming. Drivers not using autosuspend should use
+:c:func:`pm_runtime_resume_and_get()` to ensure the device is powered on. The
+function increments Runtime PM usage_count which the driver is responsible for
+decrementing using e.g. :c:func:`pm_runtime_put()` in order to power off the
+device. Drivers using autosuspend in order to avoid needless powering the sensor
+off and on again, should use :c:func:`pm_runtime_get_sync()` and
+:c:func:`pm_runtime_put_mark_busy_autosusp()` respectively. Note that runtime PM
+usage_count of the device must be put even if :c:func:`pm_runtime_get_sync()`
+fails. :c:func:`pm_runtime_get_sync()` returns 1 if the device was already
+powered on.
+
+Drivers that support Devicetree shall also power on the device explicitly in
+driver's probe() function and power the device off in driver's remove() function
+without relying on Runtime PM.
+
+Drivers may power the device up at probe time (for example to read
 identification registers), but should not keep it powered unconditionally after
 probe.
 
@@ -103,11 +115,13 @@ of the device. This is because the power state of the device is only changed
 after the power state transition has taken place. The ``s_ctrl`` callback can be
 used to obtain device's power state after the power state transition:
 
-.. c:function:: int pm_runtime_get_if_in_use(struct device *dev);
+.. c:function:: int pm_runtime_get_if_active(struct device *dev);
 
 The function returns a non-zero value if it succeeded getting the power count or
 runtime PM was disabled, in either of which cases the driver may proceed to
-access the device.
+access the device. Note that the device's usage_count is not incremented if the
+function returns an error, in which case the usage_count also must not be put
+using pm_runtime_put() or its variants.
 
 Rotation, orientation and flipping
 ----------------------------------
-- 
2.39.2

[PATCH 4/6] media: ov8858: Use pm_runtime_get_if_active(), put usage_count correctly

[Sakari Ailus]

Use pm_runtime_get_if_active() to get the device's runtime PM usage_count
and set controls, then use runtime PM autosuspend once the controls have
been set (instead of likely transitioning to suspended state immediately).

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ov8858.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/ov8858.c b/drivers/media/i2c/ov8858.c
index 3af6125a2eee..a99b91700a8d 100644
--- a/drivers/media/i2c/ov8858.c
+++ b/drivers/media/i2c/ov8858.c
@@ -1538,7 +1538,7 @@ static int ov8858_set_ctrl(struct v4l2_ctrl *ctrl)
 	struct v4l2_subdev_state *state;
 	u16 digi_gain;
 	s64 max_exp;
-	int ret;
+	int ret, pm_status;
 
 	/*
 	 * The control handler and the subdev state use the same mutex and the
@@ -1561,7 +1561,8 @@ static int ov8858_set_ctrl(struct v4l2_ctrl *ctrl)
 		break;
 	}
 
-	if (!pm_runtime_get_if_in_use(&client->dev))
+	pm_status = pm_runtime_get_if_active(&client->dev);
+	if (!pm_status)
 		return 0;
 
 	switch (ctrl->id) {
@@ -1601,7 +1602,8 @@ static int ov8858_set_ctrl(struct v4l2_ctrl *ctrl)
 		break;
 	}
 
-	pm_runtime_put(&client->dev);
+	if (pm_status > 0)
+		pm_runtime_mark_busy_autosusp(&client->dev);
 
 	return ret;
 }
-- 
2.39.2

[PATCH 5/6] media: imx319: Use pm_runtime_get_if_active(), put usage_count correctly

[Sakari Ailus]

Use pm_runtime_get_if_active() to get the device's runtime PM usage_count
and set controls, then use runtime PM autosuspend once the controls have
been set (instead of likely transitioning to suspended state immediately).

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/imx319.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/imx319.c b/drivers/media/i2c/imx319.c
index 5378f607f340..b1031bba71b7 100644
--- a/drivers/media/i2c/imx319.c
+++ b/drivers/media/i2c/imx319.c
@@ -1880,8 +1880,8 @@ static int imx319_set_ctrl(struct v4l2_ctrl *ctrl)
 	struct imx319 *imx319 = container_of(ctrl->handler,
 					     struct imx319, ctrl_handler);
 	struct i2c_client *client = v4l2_get_subdevdata(&imx319->sd);
+	int ret, pm_status;
 	s64 max;
-	int ret;
 
 	/* Propagate change of current control to all related controls */
 	switch (ctrl->id) {
@@ -1898,7 +1898,8 @@ static int imx319_set_ctrl(struct v4l2_ctrl *ctrl)
 	 * Applying V4L2 control value only happens
 	 * when power is up for streaming
 	 */
-	if (!pm_runtime_get_if_in_use(&client->dev))
+	pm_status = pm_runtime_get_if_active(&client->dev);
+	if (!pm_status)
 		return 0;
 
 	switch (ctrl->id) {
@@ -1937,7 +1938,8 @@ static int imx319_set_ctrl(struct v4l2_ctrl *ctrl)
 		break;
 	}
 
-	pm_runtime_put(&client->dev);
+	if (pm_status > 0)
+		pm_runtime_put(&client->dev);
 
 	return ret;
 }
-- 
2.39.2

[PATCH 6/6] media: imx219: Use pm_runtime_get_if_active(), put usage_count correctly

[Sakari Ailus]

Use pm_runtime_get_if_active() to get the device's runtime PM usage_count
and set controls, then use runtime PM autosuspend once the controls have
been set (instead of likely transitioning to suspended state immediately).

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/imx219.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
index 8436880dcf7a..b38ee8b3e073 100644
--- a/drivers/media/i2c/imx219.c
+++ b/drivers/media/i2c/imx219.c
@@ -371,7 +371,7 @@ static int imx219_set_ctrl(struct v4l2_ctrl *ctrl)
 	struct i2c_client *client = v4l2_get_subdevdata(&imx219->sd);
 	const struct v4l2_mbus_framefmt *format;
 	struct v4l2_subdev_state *state;
-	int ret = 0;
+	int ret = 0, pm_status;
 
 	state = v4l2_subdev_get_locked_active_state(&imx219->sd);
 	format = v4l2_subdev_get_pad_format(&imx219->sd, state, 0);
@@ -393,7 +393,8 @@ static int imx219_set_ctrl(struct v4l2_ctrl *ctrl)
 	 * Applying V4L2 control value only happens
 	 * when power is up for streaming
 	 */
-	if (pm_runtime_get_if_in_use(&client->dev) == 0)
+	pm_status = pm_runtime_get_if_active(&client->dev);
+	if (!pm_status)
 		return 0;
 
 	switch (ctrl->id) {
@@ -446,7 +447,8 @@ static int imx219_set_ctrl(struct v4l2_ctrl *ctrl)
 		break;
 	}
 
-	pm_runtime_put(&client->dev);
+	if (pm_status > 0)
+		pm_runtime_put(&client->dev);
 
 	return ret;
 }
-- 
2.39.2

Re: [PATCH 1/6] pm: runtime: Simplify pm_runtime_get_if_active() usage

[kernel test robot]

Hi Sakari,

kernel test robot noticed the following build errors:

[auto build test ERROR on 3e238417254bfdcc23fe207780b59cbb08656762]

url:    https://github.com/intel-lab-lkp/linux/commits/Sakari-Ailus/pm-runtime-Simplify-pm_runtime_get_if_active-usage/20231116-022051
base:   3e238417254bfdcc23fe207780b59cbb08656762
patch link:    https://lore.kernel.org/r/20231115181840.1509046-2-sakari.ailus%40linux.intel.com
patch subject: [PATCH 1/6] pm: runtime: Simplify pm_runtime_get_if_active() usage
config: i386-buildonly-randconfig-006-20231116 (https://download.01.org/0day-ci/archive/20231116/202311160402.sZooXBaB-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231116/202311160402.sZooXBaB-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311160402.sZooXBaB-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/gpu/drm/i915/intel_runtime_pm.c: In function '__intel_runtime_pm_get_if_active':
>> drivers/gpu/drm/i915/intel_runtime_pm.c:437:21: error: too many arguments to function '__pm_runtime_get_conditional'
     437 |                 if (__pm_runtime_get_conditional(rpm->kdev, ignore_usecount) <= 0)
         |                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   In file included from drivers/gpu/drm/i915/intel_runtime_pm.c:29:
   include/linux/pm_runtime.h:300:19: note: declared here
     300 | static inline int __pm_runtime_get_conditional(struct device *dev)
         |                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~


vim +/__pm_runtime_get_conditional +437 drivers/gpu/drm/i915/intel_runtime_pm.c

   404	
   405	/**
   406	 * __intel_runtime_pm_get_if_active - grab a runtime pm reference if device is active
   407	 * @rpm: the intel_runtime_pm structure
   408	 * @ignore_usecount: get a ref even if dev->power.usage_count is 0
   409	 *
   410	 * This function grabs a device-level runtime pm reference if the device is
   411	 * already active and ensures that it is powered up. It is illegal to try
   412	 * and access the HW should intel_runtime_pm_get_if_active() report failure.
   413	 *
   414	 * If @ignore_usecount is true, a reference will be acquired even if there is no
   415	 * user requiring the device to be powered up (dev->power.usage_count == 0).
   416	 * If the function returns false in this case then it's guaranteed that the
   417	 * device's runtime suspend hook has been called already or that it will be
   418	 * called (and hence it's also guaranteed that the device's runtime resume
   419	 * hook will be called eventually).
   420	 *
   421	 * Any runtime pm reference obtained by this function must have a symmetric
   422	 * call to intel_runtime_pm_put() to release the reference again.
   423	 *
   424	 * Returns: the wakeref cookie to pass to intel_runtime_pm_put(), evaluates
   425	 * as True if the wakeref was acquired, or False otherwise.
   426	 */
   427	static intel_wakeref_t __intel_runtime_pm_get_if_active(struct intel_runtime_pm *rpm,
   428								bool ignore_usecount)
   429	{
   430		if (IS_ENABLED(CONFIG_PM)) {
   431			/*
   432			 * In cases runtime PM is disabled by the RPM core and we get
   433			 * an -EINVAL return value we are not supposed to call this
   434			 * function, since the power state is undefined. This applies
   435			 * atm to the late/early system suspend/resume handlers.
   436			 */
 > 437			if (__pm_runtime_get_conditional(rpm->kdev, ignore_usecount) <= 0)
   438				return 0;
   439		}
   440	
   441		intel_runtime_pm_acquire(rpm, true);
   442	
   443		return track_intel_runtime_pm_wakeref(rpm);
   444	}
   445	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 4/6] media: ov8858: Use pm_runtime_get_if_active(), put usage_count correctly

[kernel test robot]

Hi Sakari,

kernel test robot noticed the following build errors:

[auto build test ERROR on 3e238417254bfdcc23fe207780b59cbb08656762]

url:    https://github.com/intel-lab-lkp/linux/commits/Sakari-Ailus/pm-runtime-Simplify-pm_runtime_get_if_active-usage/20231116-022051
base:   3e238417254bfdcc23fe207780b59cbb08656762
patch link:    https://lore.kernel.org/r/20231115181840.1509046-5-sakari.ailus%40linux.intel.com
patch subject: [PATCH 4/6] media: ov8858: Use pm_runtime_get_if_active(), put usage_count correctly
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20231116/202311160555.lMNVgFkA-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231116/202311160555.lMNVgFkA-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311160555.lMNVgFkA-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/media/i2c/ov8858.c: In function 'ov8858_set_ctrl':
>> drivers/media/i2c/ov8858.c:1606:17: error: implicit declaration of function 'pm_runtime_mark_busy_autosusp'; did you mean 'pm_runtime_put_mark_busy_autosusp'? [-Werror=implicit-function-declaration]
    1606 |                 pm_runtime_mark_busy_autosusp(&client->dev);
         |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         |                 pm_runtime_put_mark_busy_autosusp
   cc1: some warnings being treated as errors


vim +1606 drivers/media/i2c/ov8858.c

  1530	
  1531	static int ov8858_set_ctrl(struct v4l2_ctrl *ctrl)
  1532	{
  1533		struct ov8858 *ov8858 = container_of(ctrl->handler,
  1534						     struct ov8858, ctrl_handler);
  1535	
  1536		struct i2c_client *client = v4l2_get_subdevdata(&ov8858->subdev);
  1537		struct v4l2_mbus_framefmt *format;
  1538		struct v4l2_subdev_state *state;
  1539		u16 digi_gain;
  1540		s64 max_exp;
  1541		int ret, pm_status;
  1542	
  1543		/*
  1544		 * The control handler and the subdev state use the same mutex and the
  1545		 * mutex is guaranteed to be locked:
  1546		 * - by the core when s_ctrl is called int the VIDIOC_S_CTRL call path
  1547		 * - by the driver when s_ctrl is called in the s_stream(1) call path
  1548		 */
  1549		state = v4l2_subdev_get_locked_active_state(&ov8858->subdev);
  1550		format = v4l2_subdev_get_pad_format(&ov8858->subdev, state, 0);
  1551	
  1552		/* Propagate change of current control to all related controls */
  1553		switch (ctrl->id) {
  1554		case V4L2_CID_VBLANK:
  1555			/* Update max exposure while meeting expected vblanking */
  1556			max_exp = format->height + ctrl->val - OV8858_EXPOSURE_MARGIN;
  1557			__v4l2_ctrl_modify_range(ov8858->exposure,
  1558						 ov8858->exposure->minimum, max_exp,
  1559						 ov8858->exposure->step,
  1560						 ov8858->exposure->default_value);
  1561			break;
  1562		}
  1563	
  1564		pm_status = pm_runtime_get_if_active(&client->dev);
  1565		if (!pm_status)
  1566			return 0;
  1567	
  1568		switch (ctrl->id) {
  1569		case V4L2_CID_EXPOSURE:
  1570			/* 4 least significant bits of exposure are fractional part */
  1571			ret = ov8858_write(ov8858, OV8858_REG_LONG_EXPO,
  1572					   ctrl->val << 4, NULL);
  1573			break;
  1574		case V4L2_CID_ANALOGUE_GAIN:
  1575			ret = ov8858_write(ov8858, OV8858_REG_LONG_GAIN,
  1576					   ctrl->val, NULL);
  1577			break;
  1578		case V4L2_CID_DIGITAL_GAIN:
  1579			/*
  1580			 * Digital gain is assembled as:
  1581			 * 0x350a[7:0] = dgain[13:6]
  1582			 * 0x350b[5:0] = dgain[5:0]
  1583			 * Reassemble the control value to write it in one go.
  1584			 */
  1585			digi_gain = (ctrl->val & OV8858_LONG_DIGIGAIN_L_MASK)
  1586				  | ((ctrl->val & OV8858_LONG_DIGIGAIN_H_MASK) <<
  1587				      OV8858_LONG_DIGIGAIN_H_SHIFT);
  1588			ret = ov8858_write(ov8858, OV8858_REG_LONG_DIGIGAIN,
  1589					   digi_gain, NULL);
  1590			break;
  1591		case V4L2_CID_VBLANK:
  1592			ret = ov8858_write(ov8858, OV8858_REG_VTS,
  1593					   ctrl->val + format->height, NULL);
  1594			break;
  1595		case V4L2_CID_TEST_PATTERN:
  1596			ret = ov8858_enable_test_pattern(ov8858, ctrl->val);
  1597			break;
  1598		default:
  1599			ret = -EINVAL;
  1600			dev_warn(&client->dev, "%s Unhandled id: 0x%x\n",
  1601				 __func__, ctrl->id);
  1602			break;
  1603		}
  1604	
  1605		if (pm_status > 0)
> 1606			pm_runtime_mark_busy_autosusp(&client->dev);
  1607	
  1608		return ret;
  1609	}
  1610	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki