diff mbox series

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

Message ID 20231115181840.1509046-5-sakari.ailus@linux.intel.com (mailing list archive)
State Superseded, archived
Headers show
Series Small Runtime PM API changes | expand

Commit Message

Sakari Ailus Nov. 15, 2023, 6:18 p.m. UTC
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(-)

Comments

kernel test robot Nov. 15, 2023, 10:09 p.m. UTC | #1
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
diff mbox series

Patch

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;
 }