public inbox for linux-staging@lists.linux.dev
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: Mauro Carvalho Chehab <mchehab@kernel.org>,
	Sakari Ailus <sakari.ailus@linux.intel.com>,
	Andy Shevchenko <andy@kernel.org>
Cc: Hans de Goede <hdegoede@redhat.com>, Kate Hsuan <hpa@redhat.com>,
	Tsuchiya Yuto <kitakar@gmail.com>,
	Yury Luneff <yury.lunev@gmail.com>,
	Nable <nable.maininbox@googlemail.com>,
	andrey.i.trufanov@gmail.com, Fabio Aiuto <fabioaiuto83@gmail.com>,
	linux-media@vger.kernel.org, linux-staging@lists.linux.dev
Subject: [PATCH 02/15] media: atomisp: Refactor sensor crop + fmt setting
Date: Sun, 31 Dec 2023 11:30:44 +0100	[thread overview]
Message-ID: <20231231103057.35837-3-hdegoede@redhat.com> (raw)
In-Reply-To: <20231231103057.35837-1-hdegoede@redhat.com>

There are 3 code-paths all of 3 which need to do:

1. Get try or active state
2. lock state
3. Call atomisp_set_crop()
4. Call v4l2_subdev_call(input->camera, pad, set_fmt, state, fmt)
5. unlock state

Change atomisp_set_crop into atomisp_set_crop_and_fmt() which does all of
this and change the 3 code-paths which need this to use the new
atomisp_set_crop_and_fmt() function.

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

diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
index 555814012fce..bfa15fb45971 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
@@ -3721,22 +3721,34 @@ void atomisp_get_padding(struct atomisp_device *isp, u32 width, u32 height,
 	*padding_h = max_t(u32, *padding_h, min_pad_h);
 }
 
-static int atomisp_set_crop(struct atomisp_device *isp,
-			    const struct v4l2_mbus_framefmt *format,
-			    struct v4l2_subdev_state *sd_state,
-			    int which)
+static int atomisp_set_crop_and_fmt(struct atomisp_device *isp,
+				    struct v4l2_mbus_framefmt *ffmt,
+				    int which)
 {
 	struct atomisp_input_subdev *input = &isp->inputs[isp->asd.input_curr];
 	struct v4l2_subdev_selection sel = {
 		.which = which,
 		.target = V4L2_SEL_TGT_CROP,
-		.r.width = format->width,
-		.r.height = format->height,
+		.r.width = ffmt->width,
+		.r.height = ffmt->height,
 	};
-	int ret;
+	struct v4l2_subdev_format format = {
+		.which = which,
+		.format = *ffmt,
+	};
+	struct v4l2_subdev_state *sd_state;
+	int ret = 0;
+
+	if (!input->camera)
+		return -EINVAL;
+
+	sd_state = (which == V4L2_SUBDEV_FORMAT_TRY) ? input->try_sd_state :
+						       input->camera->active_state;
+	if (sd_state)
+		v4l2_subdev_lock_state(sd_state);
 
 	if (!input->crop_support)
-		return 0;
+		goto set_fmt;
 
 	/* Cropping is done before binning, when binning double the crop rect */
 	if (input->binning_support && sel.r.width <= (input->native_rect.width / 2) &&
@@ -3757,6 +3769,14 @@ static int atomisp_set_crop(struct atomisp_device *isp,
 		dev_err(isp->dev, "Error setting crop to %ux%u @%ux%u: %d\n",
 			sel.r.width, sel.r.height, sel.r.left, sel.r.top, ret);
 
+set_fmt:
+	if (ret == 0)
+		ret = v4l2_subdev_call(input->camera, pad, set_fmt, sd_state, &format);
+
+	if (sd_state)
+		v4l2_subdev_unlock_state(sd_state);
+
+	*ffmt = format.format;
 	return ret;
 }
 
@@ -3767,16 +3787,10 @@ int atomisp_try_fmt(struct atomisp_device *isp, struct v4l2_pix_format *f,
 {
 	const struct atomisp_format_bridge *fmt, *snr_fmt;
 	struct atomisp_sub_device *asd = &isp->asd;
-	struct atomisp_input_subdev *input = &isp->inputs[asd->input_curr];
-	struct v4l2_subdev_format format = {
-		.which = V4L2_SUBDEV_FORMAT_TRY,
-	};
+	struct v4l2_mbus_framefmt ffmt = { };
 	u32 padding_w, padding_h;
 	int ret;
 
-	if (!input->camera)
-		return -EINVAL;
-
 	fmt = atomisp_get_format_bridge(f->pixelformat);
 	/* Currently, raw formats are broken!!! */
 	if (!fmt || fmt->sh_fmt == IA_CSS_FRAME_FORMAT_RAW) {
@@ -3797,38 +3811,27 @@ int atomisp_try_fmt(struct atomisp_device *isp, struct v4l2_pix_format *f,
 	 * the set_fmt call, like atomisp_set_fmt_to_snr() does.
 	 */
 	atomisp_get_padding(isp, f->width, f->height, &padding_w, &padding_h);
-	v4l2_fill_mbus_format(&format.format, f, fmt->mbus_code);
-	format.format.width += padding_w;
-	format.format.height += padding_h;
+	v4l2_fill_mbus_format(&ffmt, f, fmt->mbus_code);
+	ffmt.width += padding_w;
+	ffmt.height += padding_h;
 
-	dev_dbg(isp->dev, "try_mbus_fmt: asking for %ux%u\n",
-		format.format.width, format.format.height);
-
-	v4l2_subdev_lock_state(input->try_sd_state);
-
-	ret = atomisp_set_crop(isp, &format.format, input->try_sd_state,
-			       V4L2_SUBDEV_FORMAT_TRY);
-	if (ret == 0)
-		ret = v4l2_subdev_call(input->camera, pad, set_fmt,
-				       input->try_sd_state, &format);
-
-	v4l2_subdev_unlock_state(input->try_sd_state);
+	dev_dbg(isp->dev, "try_mbus_fmt: try %ux%u\n", ffmt.width, ffmt.height);
 
+	ret = atomisp_set_crop_and_fmt(isp, &ffmt, V4L2_SUBDEV_FORMAT_TRY);
 	if (ret)
 		return ret;
 
-	dev_dbg(isp->dev, "try_mbus_fmt: got %ux%u\n",
-		format.format.width, format.format.height);
+	dev_dbg(isp->dev, "try_mbus_fmt: got %ux%u\n", ffmt.width, ffmt.height);
 
-	snr_fmt = atomisp_get_format_bridge_from_mbus(format.format.code);
+	snr_fmt = atomisp_get_format_bridge_from_mbus(ffmt.code);
 	if (!snr_fmt) {
 		dev_err(isp->dev, "unknown sensor format 0x%8.8x\n",
-			format.format.code);
+			ffmt.code);
 		return -EINVAL;
 	}
 
-	f->width = format.format.width - padding_w;
-	f->height = format.format.height - padding_h;
+	f->width = ffmt.width - padding_w;
+	f->height = ffmt.height - padding_h;
 
 	/*
 	 * If the format is jpeg or custom RAW, then the width and height will
@@ -4236,28 +4239,22 @@ static int atomisp_set_fmt_to_snr(struct video_device *vdev, const struct v4l2_p
 	struct atomisp_video_pipe *pipe = atomisp_to_video_pipe(vdev);
 	struct atomisp_sub_device *asd = pipe->asd;
 	struct atomisp_device *isp = asd->isp;
-	struct atomisp_input_subdev *input = &isp->inputs[asd->input_curr];
 	const struct atomisp_format_bridge *format;
-	struct v4l2_subdev_state *act_sd_state;
-	struct v4l2_subdev_format vformat = {
-		.which = V4L2_SUBDEV_FORMAT_TRY,
-	};
-	struct v4l2_mbus_framefmt *ffmt = &vformat.format;
-	struct v4l2_mbus_framefmt *req_ffmt;
+	struct v4l2_mbus_framefmt req_ffmt, ffmt = { };
 	struct atomisp_input_stream_info *stream_info =
-	    (struct atomisp_input_stream_info *)ffmt->reserved;
+	    (struct atomisp_input_stream_info *)&ffmt.reserved;
 	int ret;
 
 	format = atomisp_get_format_bridge(f->pixelformat);
 	if (!format)
 		return -EINVAL;
 
-	v4l2_fill_mbus_format(ffmt, f, format->mbus_code);
-	ffmt->height += asd->sink_pad_padding_h + dvs_env_h;
-	ffmt->width += asd->sink_pad_padding_w + dvs_env_w;
+	v4l2_fill_mbus_format(&ffmt, f, format->mbus_code);
+	ffmt.height += asd->sink_pad_padding_h + dvs_env_h;
+	ffmt.width += asd->sink_pad_padding_w + dvs_env_w;
 
 	dev_dbg(isp->dev, "s_mbus_fmt: ask %ux%u (padding %ux%u, dvs %ux%u)\n",
-		ffmt->width, ffmt->height, asd->sink_pad_padding_w, asd->sink_pad_padding_h,
+		ffmt.width, ffmt.height, asd->sink_pad_padding_w, asd->sink_pad_padding_h,
 		dvs_env_w, dvs_env_h);
 
 	__atomisp_init_stream_info(ATOMISP_INPUT_STREAM_GENERAL, stream_info);
@@ -4266,28 +4263,17 @@ static int atomisp_set_fmt_to_snr(struct video_device *vdev, const struct v4l2_p
 
 	/* Disable dvs if resolution can't be supported by sensor */
 	if (asd->params.video_dis_en && asd->run_mode->val == ATOMISP_RUN_MODE_VIDEO) {
-		v4l2_subdev_lock_state(input->try_sd_state);
-
-		ret = atomisp_set_crop(isp, &vformat.format, input->try_sd_state,
-				       V4L2_SUBDEV_FORMAT_TRY);
-		if (ret == 0) {
-			vformat.which = V4L2_SUBDEV_FORMAT_TRY;
-			ret = v4l2_subdev_call(input->camera, pad, set_fmt,
-					       input->try_sd_state, &vformat);
-		}
-
-		v4l2_subdev_unlock_state(input->try_sd_state);
-
+		ret = atomisp_set_crop_and_fmt(isp, &ffmt, V4L2_SUBDEV_FORMAT_TRY);
 		if (ret)
 			return ret;
 
 		dev_dbg(isp->dev, "video dis: sensor width: %d, height: %d\n",
-			ffmt->width, ffmt->height);
+			ffmt.width, ffmt.height);
 
-		if (ffmt->width < req_ffmt->width ||
-		    ffmt->height < req_ffmt->height) {
-			req_ffmt->height -= dvs_env_h;
-			req_ffmt->width -= dvs_env_w;
+		if (ffmt.width < req_ffmt.width ||
+		    ffmt.height < req_ffmt.height) {
+			req_ffmt.height -= dvs_env_h;
+			req_ffmt.width -= dvs_env_w;
 			ffmt = req_ffmt;
 			dev_warn(isp->dev,
 				 "can not enable video dis due to sensor limitation.");
@@ -4295,32 +4281,21 @@ static int atomisp_set_fmt_to_snr(struct video_device *vdev, const struct v4l2_p
 		}
 	}
 
-	act_sd_state = v4l2_subdev_lock_and_get_active_state(input->camera);
-
-	ret = atomisp_set_crop(isp, &vformat.format, act_sd_state,
-			       V4L2_SUBDEV_FORMAT_ACTIVE);
-	if (ret == 0) {
-		vformat.which = V4L2_SUBDEV_FORMAT_ACTIVE;
-		ret = v4l2_subdev_call(input->camera, pad, set_fmt, act_sd_state, &vformat);
-	}
-
-	if (act_sd_state)
-		v4l2_subdev_unlock_state(act_sd_state);
-
+	ret = atomisp_set_crop_and_fmt(isp, &ffmt, V4L2_SUBDEV_FORMAT_ACTIVE);
 	if (ret)
 		return ret;
 
 	__atomisp_update_stream_env(asd, ATOMISP_INPUT_STREAM_GENERAL, stream_info);
 
 	dev_dbg(isp->dev, "sensor width: %d, height: %d\n",
-		ffmt->width, ffmt->height);
+		ffmt.width, ffmt.height);
 
-	if (ffmt->width < ATOM_ISP_STEP_WIDTH ||
-	    ffmt->height < ATOM_ISP_STEP_HEIGHT)
+	if (ffmt.width < ATOM_ISP_STEP_WIDTH ||
+	    ffmt.height < ATOM_ISP_STEP_HEIGHT)
 		return -EINVAL;
 
 	if (asd->params.video_dis_en && asd->run_mode->val == ATOMISP_RUN_MODE_VIDEO &&
-	    (ffmt->width < req_ffmt->width || ffmt->height < req_ffmt->height)) {
+	    (ffmt.width < req_ffmt.width || ffmt.height < req_ffmt.height)) {
 		dev_warn(isp->dev,
 			 "can not enable video dis due to sensor limitation.");
 		asd->params.video_dis_en = false;
@@ -4328,9 +4303,9 @@ static int atomisp_set_fmt_to_snr(struct video_device *vdev, const struct v4l2_p
 
 	atomisp_subdev_set_ffmt(&asd->subdev, NULL,
 				V4L2_SUBDEV_FORMAT_ACTIVE,
-				ATOMISP_SUBDEV_PAD_SINK, ffmt);
+				ATOMISP_SUBDEV_PAD_SINK, &ffmt);
 
-	return css_input_resolution_changed(asd, ffmt);
+	return css_input_resolution_changed(asd, &ffmt);
 }
 
 int atomisp_set_fmt(struct video_device *vdev, struct v4l2_format *f)
-- 
2.43.0


  parent reply	other threads:[~2023-12-31 10:31 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-31 10:30 [PATCH 00/15] media: atomisp: NULL pointer deref + missing firmware fixes Hans de Goede
2023-12-31 10:30 ` [PATCH 01/15] media: atomisp: Adjust for v4l2_subdev_state handling changes in 6.8 Hans de Goede
2024-01-02  0:14   ` Andy Shevchenko
2023-12-31 10:30 ` Hans de Goede [this message]
2024-01-02  0:17   ` [PATCH 02/15] media: atomisp: Refactor sensor crop + fmt setting Andy Shevchenko
2023-12-31 10:30 ` [PATCH 03/15] media: atomisp: Remove s_routing subdev call Hans de Goede
2023-12-31 10:30 ` [PATCH 04/15] media: atomisp: Remove remaining deferred firmware loading code Hans de Goede
2023-12-31 10:30 ` [PATCH 05/15] media: atomisp: Drop is_valid_device() function Hans de Goede
2024-01-02  0:19   ` Andy Shevchenko
2024-02-19 13:25     ` Hans de Goede
2023-12-31 10:30 ` [PATCH 06/15] media: atomisp: Call pcim_enable_device() and pcim_iomap_regions() later Hans de Goede
2024-01-02  0:22   ` Andy Shevchenko
2023-12-31 10:30 ` [PATCH 07/15] media: atomisp: Fix probe error-exit path Hans de Goede
2024-01-02  0:24   ` Andy Shevchenko
2023-12-31 10:30 ` [PATCH 08/15] media: atomisp: Fix atomisp_pci_remove() Hans de Goede
2024-01-02  0:26   ` Andy Shevchenko
2023-12-31 10:30 ` [PATCH 09/15] media: atomisp: Group cpu_latency_qos_add_request() call together with other PM calls Hans de Goede
2023-12-31 10:30 ` [PATCH 10/15] media: atomisp: Fix probe()/remove() power-management Hans de Goede
2024-01-02  0:29   ` Andy Shevchenko
2023-12-31 10:30 ` [PATCH 11/15] media: atomisp: Replace atomisp_drvfs attr with using driver.dev_groups attr Hans de Goede
2024-01-02  0:33   ` Andy Shevchenko
2024-01-02 11:30     ` Hans de Goede
2024-01-02 21:23       ` Andy Shevchenko
2024-01-17 15:03         ` Hans de Goede
2023-12-31 10:30 ` [PATCH 12/15] media: atomisp: Move power-management [un]init into atomisp_pm_[un]init() Hans de Goede
2023-12-31 10:30 ` [PATCH 13/15] media: atomisp: Bind and do power-management without firmware Hans de Goede
2023-12-31 10:30 ` [PATCH 14/15] media: atomisp: Remove unnecessary msleep(10) from atomisp_mrfld_power() error path Hans de Goede
2023-12-31 10:30 ` [PATCH 15/15] media: atomisp: Update TODO Hans de Goede
2024-01-02  0:40 ` [PATCH 00/15] media: atomisp: NULL pointer deref + missing firmware fixes Andy Shevchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231231103057.35837-3-hdegoede@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=andrey.i.trufanov@gmail.com \
    --cc=andy@kernel.org \
    --cc=fabioaiuto83@gmail.com \
    --cc=hpa@redhat.com \
    --cc=kitakar@gmail.com \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=mchehab@kernel.org \
    --cc=nable.maininbox@googlemail.com \
    --cc=sakari.ailus@linux.intel.com \
    --cc=yury.lunev@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox