Message ID | 20240509184001.4064820-1-devarsht@ti.com (mailing list archive) |
---|---|
State | Superseded |
Delegated to: | Sebastian Fricke |
Headers |
Received: from ny.mirrors.kernel.org ([147.75.199.223]) by linuxtv.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from <linux-media+bounces-11243-patchwork=linuxtv.org@vger.kernel.org>) id 1s58i1-0006ID-26 for patchwork@linuxtv.org; Thu, 09 May 2024 18:41:38 +0000 Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ny.mirrors.kernel.org (Postfix) with ESMTPS id 660761C21591 for <patchwork@linuxtv.org>; Thu, 9 May 2024 18:41:36 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 8755D7FBDA; Thu, 9 May 2024 18:40:18 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=ti.com header.i=@ti.com header.b="VamAP/m3" X-Original-To: linux-media@vger.kernel.org Received: from lelv0143.ext.ti.com (lelv0143.ext.ti.com [198.47.23.248]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8C5897F464; Thu, 9 May 2024 18:40:16 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=198.47.23.248 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1715280018; cv=none; b=cMgKrVu0dKdPsvk+4KqiTf1yj6ShzN0xytusZKVlMEk7bIONg5Spid9s4JVkKTfCdwdZd3YW36OL+EH+ZUVHppmBm6Hu8PjVCvkI8chnWlhostYzn+dyp7AE0FklR0mWz9E3m+0SI8NHv1Xx0eQd9AqGfNfzm/lhl24E3JMtG3o= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1715280018; c=relaxed/simple; bh=LnGsKCLtG4HWV/QhMgM9/90EbCEjqHlXW0pCCjWpP/0=; h=From:To:CC:Subject:Date:Message-ID:MIME-Version:Content-Type; b=Q4PJUFsgmqRmiGGRReAMHuuSx4YVTsyd4fvKBxg8NLzG3ckbsMcLWpPqkUT7aZR64XMs1ysgLvgQqZQfBG18kOVqFsRXodlNxmMrSMT9GL+oPOZ3MpWP3gj2oTPdnGwOZLSLbO8H2ZG0ZivFlcTOmRxQEe6tw6pdl6GXlKiUmpM= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=ti.com; spf=pass smtp.mailfrom=ti.com; dkim=pass (1024-bit key) header.d=ti.com header.i=@ti.com header.b=VamAP/m3; arc=none smtp.client-ip=198.47.23.248 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=ti.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=ti.com Received: from lelv0266.itg.ti.com ([10.180.67.225]) by lelv0143.ext.ti.com (8.15.2/8.15.2) with ESMTP id 449Ie3XV081919; Thu, 9 May 2024 13:40:03 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ti.com; s=ti-com-17Q1; t=1715280003; bh=6QVqq24uwbSeQ4NfbKqiY+EtSE8IhElBvO4qm2S3v2s=; h=From:To:CC:Subject:Date; b=VamAP/m3qfOULlxA9+BkvBvgWoY5IKNwlRek948+HsRajaUeDwTZqXdkD4+ZSHmGJ SlQy3IJhPwo/qfrkYcVjgzWiMzPwVpZL/7fkKRe9M6kc5vbp80cxeL/jw3dW9katWv xXur6bWRkwAptAz6MuKvuSrZKXLutv7P2SBkuuQo= Received: from DFLE115.ent.ti.com (dfle115.ent.ti.com [10.64.6.36]) by lelv0266.itg.ti.com (8.15.2/8.15.2) with ESMTPS id 449Ie28x003457 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=FAIL); Thu, 9 May 2024 13:40:03 -0500 Received: from DFLE113.ent.ti.com (10.64.6.34) by DFLE115.ent.ti.com (10.64.6.36) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2507.23; Thu, 9 May 2024 13:40:02 -0500 Received: from lelvsmtp5.itg.ti.com (10.180.75.250) by DFLE113.ent.ti.com (10.64.6.34) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2507.23 via Frontend Transport; Thu, 9 May 2024 13:40:02 -0500 Received: from localhost (ti.dhcp.ti.com [172.24.227.95] (may be forged)) by lelvsmtp5.itg.ti.com (8.15.2/8.15.2) with ESMTP id 449Ie1qO097210; Thu, 9 May 2024 13:40:02 -0500 From: Devarsh Thakkar <devarsht@ti.com> To: <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> CC: <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>, <devarsht@ti.com>, <andrzej.p@collabora.com>, <nicolas@ndufresne.ca>, <p.zabel@pengutronix.de>, <airlied@gmail.com>, <daniel@ffwll.ch>, <akpm@linux-foundation.org>, <gregkh@linuxfoundation.org>, <andriy.shevchenko@linux.intel.com>, <adobriyan@gmail.com>, <jani.nikula@intel.com> Subject: [PATCH v7 7/8] media: imagination: Round to closest multiple for cropping region Date: Fri, 10 May 2024 00:10:01 +0530 Message-ID: <20240509184001.4064820-1-devarsht@ti.com> X-Mailer: git-send-email 2.39.1 Precedence: bulk X-Mailing-List: linux-media@vger.kernel.org List-Id: <linux-media.vger.kernel.org> List-Subscribe: <mailto:linux-media+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-media+unsubscribe@vger.kernel.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain X-EXCLAIMER-MD-CONFIG: e1e8a2fd-e40a-4ac6-ac9b-f7e9cc9ee180 X-LSpam-Score: -3.6 (---) X-LSpam-Report: No, score=-3.6 required=5.0 tests=ARC_SIGNED=0.001,ARC_VALID=-0.1,BAYES_00=-1.9,DKIMWL_WL_HIGH=-1,DKIM_SIGNED=0.1,DKIM_VALID=-0.1,DKIM_VALID_AU=-0.1,DMARC_PASS=-0.001,HEADER_FROM_DIFFERENT_DOMAINS=0.5,MAILING_LIST_MULTI=-1,SPF_HELO_NONE=0.001,SPF_PASS=-0.001,T_PDS_OTHER_BAD_TLD=0.01 autolearn=ham autolearn_force=no |
Series |
[v7,1/8] media: dt-bindings: Add Imagination E5010 JPEG Encoder
|
|
Commit Message
Devarsh Thakkar
May 9, 2024, 6:40 p.m. UTC
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 coordinates.
Use the rounding macro which gives preference to rounding down in case two
nearest values (high and low) are possible to raise the probability of
cropping rectangle falling inside the bound region.
Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
---
V1->V6 (No change, patch introduced in V7)
---
drivers/media/platform/imagination/e5010-jpeg-enc.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
Comments
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 coordinates. > > Use the rounding macro which gives preference to rounding down in case two > nearest values (high and low) are possible to raise the probability of > cropping rectangle falling inside the bound region. This is arguable. How do we know that the bigger range is supported? The safest side is to go smaller than bigger.
Hi Andy, 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 coordinates. >> >> Use the rounding macro which gives preference to rounding down in case two >> nearest values (high and low) are possible to raise the probability of >> cropping rectangle falling inside the bound region. > > This is arguable. How do we know that the bigger range is supported? > The safest side is to go smaller than bigger. > 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. 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 = 128 and 160 + 32 = 192 and in which case we prefer the smaller value as you suggested and that's why the driver uses round_closest_down. For any reason, if still the cropping rectangle falls beyond the bound region, then driver will return out of range error (-ERANGE) to application. Regards Devarsh
Le samedi 11 mai 2024 à 22:38 +0530, Devarsh Thakkar a écrit : > Hi Andy, > > 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 coordinates. > > > > > > Use the rounding macro which gives preference to rounding down in case two > > > nearest values (high and low) are possible to raise the probability of > > > cropping rectangle falling inside the bound region. > > > > This is arguable. How do we know that the bigger range is supported? > > The safest side is to go smaller than bigger. > > > > 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. > > 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 = 128 and 160 + 32 = 192 and in which > case we prefer the smaller value as you suggested and that's why the > driver uses round_closest_down. > > 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 > > Regards > Devarsh > >
Hi Nicolas, Thanks for the review. On 15/05/24 01:52, Nicolas Dufresne wrote: > Le samedi 11 mai 2024 à 22:38 +0530, Devarsh Thakkar a écrit : >> Hi Andy, >> >> 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 coordinates. >>>> >>>> Use the rounding macro which gives preference to rounding down in case two >>>> nearest values (high and low) are possible to raise the probability of >>>> cropping rectangle falling inside the bound region. >>> >>> This is arguable. How do we know that the bigger range is supported? >>> The safest side is to go smaller than bigger. >>> >> >> Yes and that's what the driver does when do when application passes >> 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. >> >> 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 = 128 and 160 + 32 = 192 and in which >> case we prefer the smaller value as you suggested and that's why the >> driver uses round_closest_down. >> >> 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. > Yes, this change is based on specification text. This complies with the VIDIOC_G_SELECTION, VIDIOC_S_SELECTION ioctl description as documented in v4l uapi [1] which means driver should choose crop rectangle as close as possible if no flags are passed by user-space, as quoted below : "``0`` - The driver can adjust the rectangle size freely and shall choose a crop/compose rectangle as close as possible to the requested one." If the user-space has specific requirement to either round down, round up or honor exact values, it should pass V4L2_SEL_FLAG_LE, V4L2_SEL_FLAG_GE or V4L2_SEL_FLAG_LE | V4L2_SEL_FLAG_GE flags respectively. [1] https://www.kernel.org/doc/Documentation/userspace-api/media/v4l/vidioc-g-selection.rst#:~:text=compose%20rectangle%20as-,close,-as%20possible%20to Regards Devarsh
diff --git a/drivers/media/platform/imagination/e5010-jpeg-enc.c b/drivers/media/platform/imagination/e5010-jpeg-enc.c index 189e2a99c43d..abd66bc9b96c 100644 --- a/drivers/media/platform/imagination/e5010-jpeg-enc.c +++ b/drivers/media/platform/imagination/e5010-jpeg-enc.c @@ -517,10 +517,10 @@ static int e5010_s_selection(struct file *file, void *fh, struct v4l2_selection switch (s->flags) { case 0: - s->r.width = round_down(s->r.width, queue->fmt->frmsize.step_width); - s->r.height = round_down(s->r.height, queue->fmt->frmsize.step_height); - s->r.left = round_down(s->r.left, queue->fmt->frmsize.step_width); - s->r.top = round_down(s->r.top, 2); + s->r.width = round_closest_down(s->r.width, queue->fmt->frmsize.step_width); + s->r.height = round_closest_down(s->r.height, queue->fmt->frmsize.step_height); + s->r.left = round_closest_down(s->r.left, queue->fmt->frmsize.step_width); + s->r.top = round_closest_down(s->r.top, 2); if (s->r.left + s->r.width > queue->width) s->r.width = round_down(s->r.width + s->r.left - queue->width,