diff mbox series

[04/10] media: atomisp: Remove crop_needs_override from atomisp_set_fmt()

Message ID 20230221145906.8113-5-hdegoede@redhat.com (mailing list archive)
State Accepted
Headers
Series media: atomisp: Remove depth-mode and continuous mode support |

Commit Message

Hans de Goede Feb. 21, 2023, 2:59 p.m. UTC 
  Remove the crop_needs_override local helper variable from
atomisp_set_fmt(), as it always is true now.

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

Comments

Andy Shevchenko Feb. 21, 2023, 4 p.m. UTC | #1 
On Tue, Feb 21, 2023 at 03:59:00PM +0100, Hans de Goede wrote:
> Remove the crop_needs_override local helper variable from
> atomisp_set_fmt(), as it always is true now.

...

> +			sink_crop.width = DIV_NEAREST_STEP(
> +					      sink_crop.height *
> +					      f->fmt.pix.width,
> +					      f->fmt.pix.height,
> +					      ATOM_ISP_STEP_WIDTH);

Not sure how long this code stays, I would indent it as

			sink_crop.width =
				DIV_NEAREST_STEP(sink_crop.height *
						 f->fmt.pix.width,
						 f->fmt.pix.height,
						 ATOM_ISP_STEP_WIDTH);

...

> +			sink_crop.height = DIV_NEAREST_STEP(
> +					       sink_crop.width *
> +					       f->fmt.pix.height,
> +					       f->fmt.pix.width,
> +					       ATOM_ISP_STEP_HEIGHT);

Ditto.
Hans de Goede April 1, 2023, 11:06 a.m. UTC | #2 
Hi,

On 2/21/23 17:00, Andy Shevchenko wrote:
> On Tue, Feb 21, 2023 at 03:59:00PM +0100, Hans de Goede wrote:
>> Remove the crop_needs_override local helper variable from
>> atomisp_set_fmt(), as it always is true now.
> 
> ...
> 
>> +			sink_crop.width = DIV_NEAREST_STEP(
>> +					      sink_crop.height *
>> +					      f->fmt.pix.width,
>> +					      f->fmt.pix.height,
>> +					      ATOM_ISP_STEP_WIDTH);
> 
> Not sure how long this code stays, I would indent it as
> 
> 			sink_crop.width =
> 				DIV_NEAREST_STEP(sink_crop.height *
> 						 f->fmt.pix.width,
> 						 f->fmt.pix.height,
> 						 ATOM_ISP_STEP_WIDTH);

Thanks, I've gone with:

			sink_crop.width =
				DIV_NEAREST_STEP(sink_crop.height * f->fmt.pix.width,
						 f->fmt.pix.height,
						 ATOM_ISP_STEP_WIDTH);

Keeping the first DIV_NEAREST_STEP() argument on a single line.

> 
> ...
> 
>> +			sink_crop.height = DIV_NEAREST_STEP(
>> +					       sink_crop.width *
>> +					       f->fmt.pix.height,
>> +					       f->fmt.pix.width,
>> +					       ATOM_ISP_STEP_HEIGHT);
> 
> Ditto.

Ditto :)

Regards,

Hans
diff mbox series

Patch

diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
index 2af0de77c6ae..22b8e13e9c2e 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
@@ -4634,7 +4634,6 @@  int atomisp_set_fmt(struct video_device *vdev, struct v4l2_format *f)
 	struct v4l2_pix_format backup_fmt, s_fmt;
 	unsigned int dvs_env_w = 0, dvs_env_h = 0;
 	unsigned int padding_w = pad_w, padding_h = pad_h;
-	bool res_overflow = false, crop_needs_override = false;
 	struct v4l2_mbus_framefmt *isp_sink_fmt;
 	struct v4l2_mbus_framefmt isp_source_fmt = {0};
 	struct v4l2_subdev_format vformat = {
@@ -4642,6 +4641,7 @@  int atomisp_set_fmt(struct video_device *vdev, struct v4l2_format *f)
 	};
 	struct v4l2_rect isp_sink_crop;
 	u16 source_pad = atomisp_subdev_source_pad(vdev);
+	bool res_overflow = false;
 	struct v4l2_subdev_fh fh;
 	int ret;
 
@@ -4855,7 +4855,6 @@  int atomisp_set_fmt(struct video_device *vdev, struct v4l2_format *f)
 	}
 
 	atomisp_csi_lane_config(isp);
-	crop_needs_override = true;
 
 	atomisp_check_copy_mode(asd, source_pad, &backup_fmt);
 	asd->yuvpp_mode = false;			/* Reset variable */
@@ -4917,30 +4916,29 @@  int atomisp_set_fmt(struct video_device *vdev, struct v4l2_format *f)
 		 * which appears to be related by a hardware
 		 * performance limitation.  It's unclear why this
 		 * particular code triggers the issue. */
-		if (crop_needs_override) {
-			if (isp_sink_crop.width * main_compose.height >
-			    isp_sink_crop.height * main_compose.width) {
-				sink_crop.height = isp_sink_crop.height;
-				sink_crop.width = DIV_NEAREST_STEP(
-						      sink_crop.height *
-						      f->fmt.pix.width,
-						      f->fmt.pix.height,
-						      ATOM_ISP_STEP_WIDTH);
-			} else {
-				sink_crop.width = isp_sink_crop.width;
-				sink_crop.height = DIV_NEAREST_STEP(
-						       sink_crop.width *
-						       f->fmt.pix.height,
-						       f->fmt.pix.width,
-						       ATOM_ISP_STEP_HEIGHT);
-			}
-			atomisp_subdev_set_selection(&asd->subdev, fh.state,
-						     V4L2_SUBDEV_FORMAT_ACTIVE,
-						     ATOMISP_SUBDEV_PAD_SINK,
-						     V4L2_SEL_TGT_CROP,
-						     V4L2_SEL_FLAG_KEEP_CONFIG,
-						     &sink_crop);
+		if (isp_sink_crop.width * main_compose.height >
+		    isp_sink_crop.height * main_compose.width) {
+			sink_crop.height = isp_sink_crop.height;
+			sink_crop.width = DIV_NEAREST_STEP(
+					      sink_crop.height *
+					      f->fmt.pix.width,
+					      f->fmt.pix.height,
+					      ATOM_ISP_STEP_WIDTH);
+		} else {
+			sink_crop.width = isp_sink_crop.width;
+			sink_crop.height = DIV_NEAREST_STEP(
+					       sink_crop.width *
+					       f->fmt.pix.height,
+					       f->fmt.pix.width,
+					       ATOM_ISP_STEP_HEIGHT);
 		}
+		atomisp_subdev_set_selection(&asd->subdev, fh.state,
+					     V4L2_SUBDEV_FORMAT_ACTIVE,
+					     ATOMISP_SUBDEV_PAD_SINK,
+					     V4L2_SEL_TGT_CROP,
+					     V4L2_SEL_FLAG_KEEP_CONFIG,
+					     &sink_crop);
+
 		atomisp_subdev_set_selection(&asd->subdev, fh.state,
 					     V4L2_SUBDEV_FORMAT_ACTIVE,
 					     source_pad,