Received-SPF: pass (google.com: domain of linux-kernel+bounces-179135-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <432f336e6d4b0666cfbb431591624d0e1fefd773.camel@ndufresne.ca>
Subject: Re: [PATCH v7 7/8] media: imagination: Round to closest multiple
 for cropping region
From: Nicolas Dufresne <nicolas@ndufresne.ca>
To: Devarsh Thakkar <devarsht@ti.com>, Andy Shevchenko
	 <andriy.shevchenko@linux.intel.com>
Cc: mchehab@kernel.org, hverkuil-cisco@xs4all.nl,
 linux-media@vger.kernel.org,  linux-kernel@vger.kernel.org,
 benjamin.gaignard@collabora.com,  sebastian.fricke@collabora.com,
 dri-devel@lists.freedesktop.org,  laurent.pinchart@ideasonboard.com,
 praneeth@ti.com, nm@ti.com, vigneshr@ti.com,  a-bhatia1@ti.com,
 j-luthra@ti.com, b-brnich@ti.com, detheridge@ti.com,  p-mantena@ti.com,
 vijayp@ti.com, andrzej.p@collabora.com, p.zabel@pengutronix.de, 
 airlied@gmail.com, daniel@ffwll.ch, akpm@linux-foundation.org, 
 gregkh@linuxfoundation.org, adobriyan@gmail.com, jani.nikula@intel.com
Date: Tue, 14 May 2024 16:22:28 -0400
In-Reply-To: <7e3ad816-6a2a-4e02-9b41-03a8562812ad@ti.com>
References: <20240509184001.4064820-1-devarsht@ti.com>
	 <Zj447ePSnccbj76v@smile.fi.intel.com>
	 <7e3ad816-6a2a-4e02-9b41-03a8562812ad@ti.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.52.1 (3.52.1-1.fc40) 
Precedence: bulk
MIME-Version: 1.0

Le samedi 11 mai 2024 =C3=A0 22:38 +0530, Devarsh Thakkar a =C3=A9crit=C2=
=A0:
> Hi Andy,
>=20
> Thanks for the quick review.
> On 10/05/24 20:40, Andy Shevchenko wrote:
> > On Fri, May 10, 2024 at 12:10:01AM +0530, Devarsh Thakkar wrote:
> > > If neither of the flags to round down (V4L2_SEL_FLAG_LE) or round up
> > > (V4L2_SEL_FLAG_GE) are specified by the user, then round to nearest
> > > multiple of requested value while updating the crop rectangle coordin=
ates.
> > >=20
> > > Use the rounding macro which gives preference to rounding down in cas=
e two
> > > nearest values (high and low) are possible to raise the probability o=
f
> > > cropping rectangle falling inside the bound region.
> >=20
> > This is arguable. How do we know that the bigger range is supported?
> > The safest side is to go smaller than bigger.
> >=20
>=20
> Yes and that's what the driver does when do when application passes
> V4L2_SEL_FLAG_LE while doing the selection. If application does not
> specify explicitly whether to round down or round up the cropping
> parameters requested by it (i.e app is neither passing V4L2_SEL_FLAG_LE
> nor V4L2_SEL_FLAG_GE flags), then it is preferred by driver to round the
> cropping parameters to nearest possible value by either rounding down or
> rounding up to align with hardware requirements.
>=20
> For e.g. If requested width for cropping region is 127 and HW requires
> width to be multiple of 64 then we would prefer to round it up to 128
> rather than rounding down to a more distant value (i.e. 64), but if
> requested cropping width is 129 then we would prefer to instead round it
> down to 128. But if requested cropping width is 160 then there are two
> nearest possible values 160 - 32 =3D 128 and 160 + 32 =3D 192 and in whic=
h
> case we prefer the smaller value as you suggested and that's why the
> driver uses round_closest_down.
>=20
> For any reason, if still the cropping rectangle falls beyond the bound
> region, then driver will return out of range error (-ERANGE) to
> application.

I would appreciate if this change was based on specification text, meaning
improving the next if that behaviour is undefined. We might not be able to =
fix
it everywhere, but we can recommend something.

Nicolas

>=20
> Regards
> Devarsh
>=20
>=20