linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/7] media: atomisp: Fix missing v4l2_fh_release() in atomisp_open() error exit
@ 2023-06-19 10:52 Hans de Goede
  2023-06-19 10:52 ` [PATCH 2/7] media: atomisp: Remove bogus asd == NULL checks Hans de Goede
                   ` (6 more replies)
  0 siblings, 7 replies; 10+ messages in thread
From: Hans de Goede @ 2023-06-19 10:52 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Sakari Ailus
  Cc: Hans de Goede, Tsuchiya Yuto, Andy Shevchenko, Yury Luneff,
	Nable, andrey.i.trufanov, Fabio Aiuto, Kate Hsuan, linux-media,
	linux-staging, kernel test robot, Dan Carpenter

Fix missing v4l2_fh_release() in atomisp_open()'s
"if (pipe->users)" error exit path.

Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/r/202306180511.XWN0Hr7F-lkp@intel.com/
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/staging/media/atomisp/pci/atomisp_fops.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_fops.c b/drivers/staging/media/atomisp/pci/atomisp_fops.c
index 54466d2f323a..a09087dccbcb 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_fops.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_fops.c
@@ -513,8 +513,8 @@ static int atomisp_open(struct file *file)
 	 */
 	if (pipe->users) {
 		dev_dbg(isp->dev, "video node already opened\n");
-		mutex_unlock(&isp->mutex);
-		return -EBUSY;
+		ret = -EBUSY;
+		goto error;
 	}
 
 	/* runtime power management, turn on ISP */
-- 
2.40.1


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

* [PATCH 2/7] media: atomisp: Remove bogus asd == NULL checks
  2023-06-19 10:52 [PATCH 1/7] media: atomisp: Fix missing v4l2_fh_release() in atomisp_open() error exit Hans de Goede
@ 2023-06-19 10:52 ` Hans de Goede
  2023-06-19 10:52 ` [PATCH 3/7] media: atomisp: Clamp width to max 1920 pixels when in ATOMISP_RUN_MODE_PREVIEW Hans de Goede
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Hans de Goede @ 2023-06-19 10:52 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Sakari Ailus
  Cc: Hans de Goede, Tsuchiya Yuto, Andy Shevchenko, Yury Luneff,
	Nable, andrey.i.trufanov, Fabio Aiuto, Kate Hsuan, linux-media,
	linux-staging, Dan Carpenter

The asd is a sub-structure of the main driver data struct, so it is
never NULL. Drop the unnecessary NULL checks in a couple of places.

Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/linux-media/533f6930-434a-45f3-afff-127003fa64c9@moroto.mountain/
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../staging/media/atomisp/pci/atomisp_cmd.c   | 24 -------------------
 .../media/atomisp/pci/atomisp_compat_css20.c  |  3 ---
 2 files changed, 27 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
index 807e3aa1eb70..cbbf6f728f57 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
@@ -3001,12 +3001,6 @@ void atomisp_handle_parameter_and_buffer(struct atomisp_video_pipe *pipe)
 	bool need_to_enqueue_buffer = false;
 	int i;
 
-	if (!asd) {
-		dev_err(pipe->isp->dev, "%s(): asd is NULL, device is %s\n",
-			__func__, pipe->vdev.name);
-		return;
-	}
-
 	lockdep_assert_held(&asd->isp->mutex);
 
 	/*
@@ -3068,12 +3062,6 @@ int atomisp_set_parameters(struct video_device *vdev,
 	struct atomisp_css_params *css_param = &asd->params.css_param;
 	int ret;
 
-	if (!asd) {
-		dev_err(pipe->isp->dev, "%s(): asd is NULL, device is %s\n",
-			__func__, vdev->name);
-		return -EINVAL;
-	}
-
 	lockdep_assert_held(&asd->isp->mutex);
 
 	if (!asd->stream_env[ATOMISP_INPUT_STREAM_GENERAL].stream) {
@@ -4067,12 +4055,6 @@ static int atomisp_set_fmt_to_isp(struct video_device *vdev,
 	const struct atomisp_in_fmt_conv *fc = NULL;
 	int ret, i;
 
-	if (!asd) {
-		dev_err(isp->dev, "%s(): asd is NULL, device is %s\n",
-			__func__, vdev->name);
-		return -EINVAL;
-	}
-
 	isp_sink_crop = atomisp_subdev_get_rect(
 			    &asd->subdev, NULL, V4L2_SUBDEV_FORMAT_ACTIVE,
 			    ATOMISP_SUBDEV_PAD_SINK, V4L2_SEL_TGT_CROP);
@@ -4280,12 +4262,6 @@ static int atomisp_set_fmt_to_snr(struct video_device *vdev, const struct v4l2_p
 	    (struct atomisp_input_stream_info *)ffmt->reserved;
 	int ret;
 
-	if (!asd) {
-		dev_err(pipe->isp->dev, "%s(): asd is NULL, device is %s\n",
-			__func__, vdev->name);
-		return -EINVAL;
-	}
-
 	format = atomisp_get_format_bridge(f->pixelformat);
 	if (!format)
 		return -EINVAL;
diff --git a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
index b13d1cb4668d..b97ec85aa0ba 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
@@ -613,9 +613,6 @@ static void __apply_additional_pipe_config(
 static bool is_pipe_valid_to_current_run_mode(struct atomisp_sub_device *asd,
 	enum ia_css_pipe_id pipe_id)
 {
-	if (!asd)
-		return false;
-
 	if (pipe_id == IA_CSS_PIPE_ID_YUVPP)
 		return true;
 
-- 
2.40.1


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

* [PATCH 3/7] media: atomisp: Clamp width to max 1920 pixels when in ATOMISP_RUN_MODE_PREVIEW
  2023-06-19 10:52 [PATCH 1/7] media: atomisp: Fix missing v4l2_fh_release() in atomisp_open() error exit Hans de Goede
  2023-06-19 10:52 ` [PATCH 2/7] media: atomisp: Remove bogus asd == NULL checks Hans de Goede
@ 2023-06-19 10:52 ` Hans de Goede
  2023-06-19 10:52 ` [PATCH 4/7] media: atomisp: Change atomisp_enum_framesizes() too small cut off from 2/3th to 5/8th Hans de Goede
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Hans de Goede @ 2023-06-19 10:52 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Sakari Ailus
  Cc: Hans de Goede, Tsuchiya Yuto, Andy Shevchenko, Yury Luneff,
	Nable, andrey.i.trufanov, Fabio Aiuto, Kate Hsuan, linux-media,
	linux-staging

The pipeline firmware-binaries used in previed mode have
ia_css_binary_xinfo.output.max_width set to 1920.

This causes ia_css_binary_find() to fail when trying to set a higher
resolution resulting in the dump_stack() call in ia_css_binary_find()
triggering and resulting in the try_fmt() or set_fmt() IOCTL failing.

Fix this by clamping the width to max 1920 when in preview mode.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/staging/media/atomisp/pci/atomisp_cmd.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
index cbbf6f728f57..64456aedbf00 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
@@ -3808,6 +3808,10 @@ int atomisp_try_fmt(struct atomisp_device *isp, struct v4l2_pix_format *f,
 			return -EINVAL;
 	}
 
+	/* The preview pipeline does not support width > 1920 */
+	if (asd->run_mode->val == ATOMISP_RUN_MODE_PREVIEW)
+		f->width = min_t(u32, f->width, 1920);
+
 	/*
 	 * atomisp_set_fmt() will set the sensor resolution to the requested
 	 * resolution + padding. Add padding here and remove it again after
-- 
2.40.1


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

* [PATCH 4/7] media: atomisp: Change atomisp_enum_framesizes() too small cut off from 2/3th to 5/8th
  2023-06-19 10:52 [PATCH 1/7] media: atomisp: Fix missing v4l2_fh_release() in atomisp_open() error exit Hans de Goede
  2023-06-19 10:52 ` [PATCH 2/7] media: atomisp: Remove bogus asd == NULL checks Hans de Goede
  2023-06-19 10:52 ` [PATCH 3/7] media: atomisp: Clamp width to max 1920 pixels when in ATOMISP_RUN_MODE_PREVIEW Hans de Goede
@ 2023-06-19 10:52 ` Hans de Goede
  2023-06-19 10:52 ` [PATCH 5/7] media: atomisp: Add some higher resolutions to atomisp_enum_framesizes() Hans de Goede
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Hans de Goede @ 2023-06-19 10:52 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Sakari Ailus
  Cc: Hans de Goede, Tsuchiya Yuto, Andy Shevchenko, Yury Luneff,
	Nable, andrey.i.trufanov, Fabio Aiuto, Kate Hsuan, linux-media,
	linux-staging

Change atomisp_enum_framesizes() cut off for too small resolutions
from 2/3th to 5/8th this results in more resolutions being available
with some sensors.

E.g. this allows using 800x600 with a 1280x960 sensor.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/staging/media/atomisp/pci/atomisp_ioctl.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
index d2174156573a..047b9fb075d0 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
@@ -729,11 +729,11 @@ static int atomisp_enum_framesizes_crop_inner(struct atomisp_device *isp,
 			continue;
 
 		/*
-		 * Skip sizes where width and height are less then 2/3th of the
+		 * Skip sizes where width and height are less then 5/8th of the
 		 * sensor size to avoid sizes with a too small field of view.
 		 */
-		if (frame_sizes[i].width < (active->width * 2 / 3) &&
-		    frame_sizes[i].height < (active->height * 2 / 3))
+		if (frame_sizes[i].width < (active->width * 5 / 8) &&
+		    frame_sizes[i].height < (active->height * 5 / 8))
 			continue;
 
 		if (*valid_sizes == fsize->index) {
-- 
2.40.1


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

* [PATCH 5/7] media: atomisp: Add some higher resolutions to atomisp_enum_framesizes()
  2023-06-19 10:52 [PATCH 1/7] media: atomisp: Fix missing v4l2_fh_release() in atomisp_open() error exit Hans de Goede
                   ` (2 preceding siblings ...)
  2023-06-19 10:52 ` [PATCH 4/7] media: atomisp: Change atomisp_enum_framesizes() too small cut off from 2/3th to 5/8th Hans de Goede
@ 2023-06-19 10:52 ` Hans de Goede
  2023-06-19 10:52 ` [PATCH 6/7] media: atomisp: Remove support for custom run-mode v4l2-ctrl on sensors Hans de Goede
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Hans de Goede @ 2023-06-19 10:52 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Sakari Ailus
  Cc: Hans de Goede, Tsuchiya Yuto, Andy Shevchenko, Yury Luneff,
	Nable, andrey.i.trufanov, Fabio Aiuto, Kate Hsuan, linux-media,
	linux-staging

Add some higher resolutions to the fixed list of resolutions which
atomisp_enum_framesizes() uses on sensors which can do cropping and can
thus make any resolution that will fit.

This is useful for higher resolution sensors like the 2560x1920 ov5693
sensor.

Note the highest resolutions added here are 1920x<height> because
the atomisp firmware does not support widths > 1920 with the default
asd->run_mode->val == ATOMISP_RUN_MODE_PREVIEW setting.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/staging/media/atomisp/pci/atomisp_ioctl.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
index 047b9fb075d0..8fd981f49659 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
@@ -708,6 +708,9 @@ static int atomisp_enum_framesizes_crop_inner(struct atomisp_device *isp,
 					      int *valid_sizes)
 {
 	static const struct v4l2_frmsize_discrete frame_sizes[] = {
+		{ 1920, 1440 },
+		{ 1920, 1200 },
+		{ 1920, 1080 },
 		{ 1600, 1200 },
 		{ 1600, 1080 },
 		{ 1600,  900 },
-- 
2.40.1


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

* [PATCH 6/7] media: atomisp: Remove support for custom run-mode v4l2-ctrl on sensors
  2023-06-19 10:52 [PATCH 1/7] media: atomisp: Fix missing v4l2_fh_release() in atomisp_open() error exit Hans de Goede
                   ` (3 preceding siblings ...)
  2023-06-19 10:52 ` [PATCH 5/7] media: atomisp: Add some higher resolutions to atomisp_enum_framesizes() Hans de Goede
@ 2023-06-19 10:52 ` Hans de Goede
  2023-06-19 11:12   ` Andy Shevchenko
  2023-06-19 10:52 ` [PATCH 7/7] media: atomisp: Remove v4l2_ctrl_s_ctrl(asd->run_mode) calls from atomisp_open() Hans de Goede
  2023-06-19 11:12 ` [PATCH 1/7] media: atomisp: Fix missing v4l2_fh_release() in atomisp_open() error exit Andy Shevchenko
  6 siblings, 1 reply; 10+ messages in thread
From: Hans de Goede @ 2023-06-19 10:52 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Sakari Ailus
  Cc: Hans de Goede, Tsuchiya Yuto, Andy Shevchenko, Yury Luneff,
	Nable, andrey.i.trufanov, Fabio Aiuto, Kate Hsuan, linux-media,
	linux-staging

Remove the support to update a V4L2_CID_RUN_MODE run-mode control
on sensors when changing the atomisp run-mode or directly by calling
the custom ATOMISP_IOC_S_SENSOR_RUNMODE IOCTL.

No sensor drivers implement this and having custom controls / IOCTLs
is undesirable.

Even if there was such a control on sensors then userspace should directly
talk to the sensor v4l2-subdev, rather than relying on a custom ioctl-s
on the output /dev/video# node to pass this through to the senor.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../media/atomisp/include/linux/atomisp.h     | 10 ----
 .../staging/media/atomisp/pci/atomisp_cmd.c   | 22 ---------
 .../staging/media/atomisp/pci/atomisp_cmd.h   | 13 -----
 .../staging/media/atomisp/pci/atomisp_ioctl.c | 12 -----
 .../media/atomisp/pci/atomisp_subdev.c        | 48 -------------------
 .../media/atomisp/pci/atomisp_subdev.h        |  2 -
 6 files changed, 107 deletions(-)

diff --git a/drivers/staging/media/atomisp/include/linux/atomisp.h b/drivers/staging/media/atomisp/include/linux/atomisp.h
index 14b1757e6674..bbbd904b696a 100644
--- a/drivers/staging/media/atomisp/include/linux/atomisp.h
+++ b/drivers/staging/media/atomisp/include/linux/atomisp.h
@@ -713,13 +713,6 @@ enum atomisp_burst_capture_options {
 #define EXT_ISP_SHOT_MODE_ANIMATED_PHOTO	10
 #define EXT_ISP_SHOT_MODE_SPORTS	11
 
-/*
- * Set Senor run mode
- */
-struct atomisp_s_runmode {
-	__u32 mode;
-};
-
 /*Private IOCTLs for ISP */
 #define ATOMISP_IOC_G_XNR \
 	_IOR('v', BASE_VIDIOC_PRIVATE + 0, int)
@@ -875,9 +868,6 @@ struct atomisp_s_runmode {
 #define ATOMISP_IOC_S_SENSOR_EE_CONFIG \
 	_IOW('v', BASE_VIDIOC_PRIVATE + 47, unsigned int)
 
-#define ATOMISP_IOC_S_SENSOR_RUNMODE \
-	_IOW('v', BASE_VIDIOC_PRIVATE + 48, struct atomisp_s_runmode)
-
 /*
  * Reserved ioctls. We have customer implementing it internally.
  * We can't use both numbers to not cause ABI conflict.
diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
index 64456aedbf00..2e96d52922e2 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
@@ -1248,28 +1248,6 @@ static void atomisp_update_capture_mode(struct atomisp_sub_device *asd)
 		atomisp_css_capture_set_mode(asd, IA_CSS_CAPTURE_MODE_PRIMARY);
 }
 
-/* ISP2401 */
-int atomisp_set_sensor_runmode(struct atomisp_sub_device *asd,
-			       struct atomisp_s_runmode *runmode)
-{
-	struct atomisp_device *isp = asd->isp;
-	struct v4l2_ctrl *c;
-	int ret = 0;
-
-	if (!(runmode && (runmode->mode & RUNMODE_MASK)))
-		return -EINVAL;
-
-	mutex_lock(asd->ctrl_handler.lock);
-	c = v4l2_ctrl_find(isp->inputs[asd->input_curr].camera->ctrl_handler,
-			   V4L2_CID_RUN_MODE);
-
-	if (c)
-		ret = v4l2_ctrl_s_ctrl(c, runmode->mode);
-
-	mutex_unlock(asd->ctrl_handler.lock);
-	return ret;
-}
-
 /*
  * Function to enable/disable lens geometry distortion correction (GDC) and
  * chromatic aberration correction (CAC)
diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.h b/drivers/staging/media/atomisp/pci/atomisp_cmd.h
index 8305161d2062..b8cd957eebdc 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_cmd.h
+++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.h
@@ -42,13 +42,6 @@ struct ia_css_frame;
 #define INTR_IER		24
 #define INTR_IIR		16
 
-/* ISP2401 */
-#define RUNMODE_MASK (ATOMISP_RUN_MODE_VIDEO | ATOMISP_RUN_MODE_STILL_CAPTURE \
-			| ATOMISP_RUN_MODE_PREVIEW)
-
-/* FIXME: check if can go */
-extern int atomisp_punit_hpll_freq;
-
 /* Helper function */
 void dump_sp_dmem(struct atomisp_device *isp, unsigned int addr,
 		  unsigned int size);
@@ -77,12 +70,6 @@ bool atomisp_is_viewfinder_support(struct atomisp_device *isp);
 
 /* ISP features control function */
 
-/*
- * Function to set sensor runmode by user when
- * ATOMISP_IOC_S_SENSOR_RUNMODE ioctl was called
- */
-int atomisp_set_sensor_runmode(struct atomisp_sub_device *asd,
-			       struct atomisp_s_runmode *runmode);
 /*
  * Function to enable/disable lens geometry distortion correction (GDC) and
  * chromatic aberration correction (CAC)
diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
index 8fd981f49659..a8e4779d007f 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
@@ -665,11 +665,6 @@ static int atomisp_s_input(struct file *file, void *fh, unsigned int input)
 		dev_err(isp->dev, "Failed to power-on sensor\n");
 		return ret;
 	}
-	/*
-	 * Some sensor driver resets the run mode during power-on, thus force
-	 * update the run mode to sensor after power-on.
-	 */
-	atomisp_update_run_mode(asd);
 
 	/* select operating sensor */
 	ret = v4l2_subdev_call(isp->inputs[input].camera, video, s_routing,
@@ -1784,13 +1779,6 @@ static long atomisp_vidioc_default(struct file *file, void *fh,
 	int err;
 
 	switch (cmd) {
-	case ATOMISP_IOC_S_SENSOR_RUNMODE:
-		if (IS_ISP2401)
-			err = atomisp_set_sensor_runmode(asd, arg);
-		else
-			err = -EINVAL;
-		break;
-
 	case ATOMISP_IOC_G_XNR:
 		err = atomisp_xnr(asd, 0, arg);
 		break;
diff --git a/drivers/staging/media/atomisp/pci/atomisp_subdev.c b/drivers/staging/media/atomisp/pci/atomisp_subdev.c
index 45073e401bac..471912dea5cd 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_subdev.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_subdev.c
@@ -663,52 +663,6 @@ static const struct media_entity_operations isp_subdev_media_ops = {
 	/*	 .set_power = v4l2_subdev_set_power,	*/
 };
 
-static int __atomisp_update_run_mode(struct atomisp_sub_device *asd)
-{
-	struct atomisp_device *isp = asd->isp;
-	struct v4l2_ctrl *ctrl = asd->run_mode;
-	struct v4l2_ctrl *c;
-	s32 mode;
-
-	mode = ctrl->val;
-
-	c = v4l2_ctrl_find(
-		isp->inputs[asd->input_curr].camera->ctrl_handler,
-		V4L2_CID_RUN_MODE);
-
-	if (c)
-		return v4l2_ctrl_s_ctrl(c, mode);
-
-	return 0;
-}
-
-int atomisp_update_run_mode(struct atomisp_sub_device *asd)
-{
-	int rval;
-
-	mutex_lock(asd->ctrl_handler.lock);
-	rval = __atomisp_update_run_mode(asd);
-	mutex_unlock(asd->ctrl_handler.lock);
-
-	return rval;
-}
-
-static int s_ctrl(struct v4l2_ctrl *ctrl)
-{
-	struct atomisp_sub_device *asd = container_of(
-					     ctrl->handler, struct atomisp_sub_device, ctrl_handler);
-	switch (ctrl->id) {
-	case V4L2_CID_RUN_MODE:
-		return __atomisp_update_run_mode(asd);
-	}
-
-	return 0;
-}
-
-static const struct v4l2_ctrl_ops ctrl_ops = {
-	.s_ctrl = &s_ctrl,
-};
-
 static const char *const ctrl_run_mode_menu[] = {
 	[ATOMISP_RUN_MODE_VIDEO]		= "Video",
 	[ATOMISP_RUN_MODE_STILL_CAPTURE]	= "Still capture",
@@ -716,7 +670,6 @@ static const char *const ctrl_run_mode_menu[] = {
 };
 
 static const struct v4l2_ctrl_config ctrl_run_mode = {
-	.ops = &ctrl_ops,
 	.id = V4L2_CID_RUN_MODE,
 	.name = "Atomisp run mode",
 	.type = V4L2_CTRL_TYPE_MENU,
@@ -754,7 +707,6 @@ static const struct v4l2_ctrl_config ctrl_vfpp = {
  * the CSS subsystem.
  */
 static const struct v4l2_ctrl_config ctrl_continuous_raw_buffer_size = {
-	.ops = &ctrl_ops,
 	.id = V4L2_CID_ATOMISP_CONTINUOUS_RAW_BUFFER_SIZE,
 	.type = V4L2_CTRL_TYPE_INTEGER,
 	.name = "Continuous raw ringbuffer size",
diff --git a/drivers/staging/media/atomisp/pci/atomisp_subdev.h b/drivers/staging/media/atomisp/pci/atomisp_subdev.h
index 9a04511b9efd..9c1703bf439c 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_subdev.h
+++ b/drivers/staging/media/atomisp/pci/atomisp_subdev.h
@@ -360,8 +360,6 @@ void atomisp_subdev_set_ffmt(struct v4l2_subdev *sd,
 			     uint32_t which,
 			     u32 pad, struct v4l2_mbus_framefmt *ffmt);
 
-int atomisp_update_run_mode(struct atomisp_sub_device *asd);
-
 void atomisp_subdev_cleanup_pending_events(struct atomisp_sub_device *asd);
 
 void atomisp_subdev_unregister_entities(struct atomisp_sub_device *asd);
-- 
2.40.1


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

* [PATCH 7/7] media: atomisp: Remove v4l2_ctrl_s_ctrl(asd->run_mode) calls from atomisp_open()
  2023-06-19 10:52 [PATCH 1/7] media: atomisp: Fix missing v4l2_fh_release() in atomisp_open() error exit Hans de Goede
                   ` (4 preceding siblings ...)
  2023-06-19 10:52 ` [PATCH 6/7] media: atomisp: Remove support for custom run-mode v4l2-ctrl on sensors Hans de Goede
@ 2023-06-19 10:52 ` Hans de Goede
  2023-06-19 11:12 ` [PATCH 1/7] media: atomisp: Fix missing v4l2_fh_release() in atomisp_open() error exit Andy Shevchenko
  6 siblings, 0 replies; 10+ messages in thread
From: Hans de Goede @ 2023-06-19 10:52 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Sakari Ailus
  Cc: Hans de Goede, Tsuchiya Yuto, Andy Shevchenko, Yury Luneff,
	Nable, andrey.i.trufanov, Fabio Aiuto, Kate Hsuan, linux-media,
	linux-staging

The v4l2_ctrl_s_ctrl(asd->run_mode) call in atomisp_subdev_init_struct()
gets immediately overridden by a second call directly after
atomisp_subdev_init_struct() is called.

And the second call in atomisp_open() also is not helpful.
ATOMISP_RUN_MODE_PREVIEW is the default and if changed controls
are supposed to stay changed over an open/close of the /dev/video#
node. So drop both calls.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/staging/media/atomisp/pci/atomisp_fops.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_fops.c b/drivers/staging/media/atomisp/pci/atomisp_fops.c
index a09087dccbcb..4dba6120af39 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_fops.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_fops.c
@@ -460,7 +460,6 @@ static void atomisp_dev_init_struct(struct atomisp_device *isp)
 
 static void atomisp_subdev_init_struct(struct atomisp_sub_device *asd)
 {
-	v4l2_ctrl_s_ctrl(asd->run_mode, ATOMISP_RUN_MODE_STILL_CAPTURE);
 	memset(&asd->params.css_param, 0, sizeof(asd->params.css_param));
 	asd->params.color_effect = V4L2_COLORFX_NONE;
 	asd->params.bad_pixel_en = true;
@@ -533,8 +532,6 @@ static int atomisp_open(struct file *file)
 	}
 
 	atomisp_subdev_init_struct(asd);
-	/* Ensure that a mode is set */
-	v4l2_ctrl_s_ctrl(asd->run_mode, ATOMISP_RUN_MODE_PREVIEW);
 
 	pipe->users++;
 	mutex_unlock(&isp->mutex);
-- 
2.40.1


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

* Re: [PATCH 6/7] media: atomisp: Remove support for custom run-mode v4l2-ctrl on sensors
  2023-06-19 10:52 ` [PATCH 6/7] media: atomisp: Remove support for custom run-mode v4l2-ctrl on sensors Hans de Goede
@ 2023-06-19 11:12   ` Andy Shevchenko
  0 siblings, 0 replies; 10+ messages in thread
From: Andy Shevchenko @ 2023-06-19 11:12 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Mauro Carvalho Chehab, Sakari Ailus, Tsuchiya Yuto, Yury Luneff,
	Nable, andrey.i.trufanov, Fabio Aiuto, Kate Hsuan, linux-media,
	linux-staging

On Mon, Jun 19, 2023 at 12:52:11PM +0200, Hans de Goede wrote:
> Remove the support to update a V4L2_CID_RUN_MODE run-mode control
> on sensors when changing the atomisp run-mode or directly by calling
> the custom ATOMISP_IOC_S_SENSOR_RUNMODE IOCTL.
> 
> No sensor drivers implement this and having custom controls / IOCTLs
> is undesirable.
> 
> Even if there was such a control on sensors then userspace should directly
> talk to the sensor v4l2-subdev, rather than relying on a custom ioctl-s

IOCTLs ?

> on the output /dev/video# node to pass this through to the senor.

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH 1/7] media: atomisp: Fix missing v4l2_fh_release() in atomisp_open() error exit
  2023-06-19 10:52 [PATCH 1/7] media: atomisp: Fix missing v4l2_fh_release() in atomisp_open() error exit Hans de Goede
                   ` (5 preceding siblings ...)
  2023-06-19 10:52 ` [PATCH 7/7] media: atomisp: Remove v4l2_ctrl_s_ctrl(asd->run_mode) calls from atomisp_open() Hans de Goede
@ 2023-06-19 11:12 ` Andy Shevchenko
  2023-06-21 19:09   ` Hans de Goede
  6 siblings, 1 reply; 10+ messages in thread
From: Andy Shevchenko @ 2023-06-19 11:12 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Mauro Carvalho Chehab, Sakari Ailus, Tsuchiya Yuto, Yury Luneff,
	Nable, andrey.i.trufanov, Fabio Aiuto, Kate Hsuan, linux-media,
	linux-staging, kernel test robot, Dan Carpenter

On Mon, Jun 19, 2023 at 12:52:06PM +0200, Hans de Goede wrote:
> Fix missing v4l2_fh_release() in atomisp_open()'s
> "if (pipe->users)" error exit path.

All LGTM,
Reviewed-by: Andy Shevchenko <andy@kernel.org>

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH 1/7] media: atomisp: Fix missing v4l2_fh_release() in atomisp_open() error exit
  2023-06-19 11:12 ` [PATCH 1/7] media: atomisp: Fix missing v4l2_fh_release() in atomisp_open() error exit Andy Shevchenko
@ 2023-06-21 19:09   ` Hans de Goede
  0 siblings, 0 replies; 10+ messages in thread
From: Hans de Goede @ 2023-06-21 19:09 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Mauro Carvalho Chehab, Sakari Ailus, Tsuchiya Yuto, Yury Luneff,
	Nable, andrey.i.trufanov, Fabio Aiuto, Kate Hsuan, linux-media,
	linux-staging, kernel test robot, Dan Carpenter

Hi Andy,

On 6/19/23 13:12, Andy Shevchenko wrote:
> On Mon, Jun 19, 2023 at 12:52:06PM +0200, Hans de Goede wrote:
>> Fix missing v4l2_fh_release() in atomisp_open()'s
>> "if (pipe->users)" error exit path.
> 
> All LGTM,
> Reviewed-by: Andy Shevchenko <andy@kernel.org>

As always thank you for all the reviews.

I've fixed up the ioctl-s -> IOCTLs remark in
the commit message of 6/7 while merging this.

And I've merged this entire series into my media-atomisp
branch now:

https://git.kernel.org/pub/scm/linux/kernel/git/hansg/linux.git/log/?h=media-atomisp

Regards,

Hans



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

end of thread, other threads:[~2023-06-21 19:10 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-19 10:52 [PATCH 1/7] media: atomisp: Fix missing v4l2_fh_release() in atomisp_open() error exit Hans de Goede
2023-06-19 10:52 ` [PATCH 2/7] media: atomisp: Remove bogus asd == NULL checks Hans de Goede
2023-06-19 10:52 ` [PATCH 3/7] media: atomisp: Clamp width to max 1920 pixels when in ATOMISP_RUN_MODE_PREVIEW Hans de Goede
2023-06-19 10:52 ` [PATCH 4/7] media: atomisp: Change atomisp_enum_framesizes() too small cut off from 2/3th to 5/8th Hans de Goede
2023-06-19 10:52 ` [PATCH 5/7] media: atomisp: Add some higher resolutions to atomisp_enum_framesizes() Hans de Goede
2023-06-19 10:52 ` [PATCH 6/7] media: atomisp: Remove support for custom run-mode v4l2-ctrl on sensors Hans de Goede
2023-06-19 11:12   ` Andy Shevchenko
2023-06-19 10:52 ` [PATCH 7/7] media: atomisp: Remove v4l2_ctrl_s_ctrl(asd->run_mode) calls from atomisp_open() Hans de Goede
2023-06-19 11:12 ` [PATCH 1/7] media: atomisp: Fix missing v4l2_fh_release() in atomisp_open() error exit Andy Shevchenko
2023-06-21 19:09   ` Hans de Goede

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).