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

From: Hans de Goede <hdegoede@redhat.com>
To: Andy Shevchenko <andy@kernel.org>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Sakari Ailus <sakari.ailus@linux.intel.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: Re: [PATCH 04/10] media: atomisp: Remove crop_needs_override from atomisp_set_fmt()
Date: Sat, 1 Apr 2023 13:06:19 +0200	[thread overview]
Message-ID: <d452fe83-e063-8f18-2582-af04c7cec867@redhat.com> (raw)
In-Reply-To: <Y/Tqs3hEYzeZ9Om4@smile.fi.intel.com>

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