linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] media: rkisp1: Increase ISP input resolution limit
@ 2021-03-26  6:23 Sebastian Fricke
  2021-03-29  5:40 ` Laurent Pinchart
  0 siblings, 1 reply; 2+ messages in thread
From: Sebastian Fricke @ 2021-03-26  6:23 UTC (permalink / raw)
  To: linux-media
  Cc: dafna.hirschfeld, laurent.pinchart, helen.koike, heiko, Sebastian Fricke

The current implementation limits the maximum sink pad resolution to
4032x3024, which is mentioned by the Rockchip TRM as the maximum size
to handle black level calibration. But the ISP can actually set it's
sink pad format to a size of 4416x3312.
Allow higher sink pad resolutions in order to allow a bigger range of
sensor modes to be used with the RkISP1.
Apply the previous limit to the sink pad crop instead of the format to
satisfy the requirement of the ISP.

Signed-off-by: Sebastian Fricke <sebastian.fricke@posteo.net>
---
Changes since v1:
- Improve the name of the new macro -> s/*_PROCESSING/*_CROP/
- Add a descriptive comment to explain why we need those 2 new macros
- Combine the separated 2 patches into a single one as patch 2 was
  depending on patch 1, which would have introduced a bisection breakage
  (Thanks to Laurent Pinchart)

This patch was tested with a NanoPC-T4 and a OV13850, which provides a
resolution of 4224x3136.
---
 .../platform/rockchip/rkisp1/rkisp1-common.h   | 18 +++++++++++++-----
 .../platform/rockchip/rkisp1/rkisp1-isp.c      |  8 ++++++--
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
index 038c303a8aed..553a4b12becf 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
@@ -29,11 +29,19 @@
 #define RKISP1_ISP_SD_SRC BIT(0)
 #define RKISP1_ISP_SD_SINK BIT(1)
 
-/* min and max values for the widths and heights of the entities */
-#define RKISP1_ISP_MAX_WIDTH		4032
-#define RKISP1_ISP_MAX_HEIGHT		3024
-#define RKISP1_ISP_MIN_WIDTH		32
-#define RKISP1_ISP_MIN_HEIGHT		32
+/*
+ * min and max values for the widths and heights of the entities
+ * The ISP device accepts input resolutions of up to 4416x3312, but
+ * it can only process resolutions of 4032x3024 internally.
+ * Therefore the crop resolution is limited to 4032x3024, the
+ * sink pad crop is applied automatically when the format is set.
+ */
+#define RKISP1_ISP_MAX_WIDTH			4416
+#define RKISP1_ISP_MAX_HEIGHT			3312
+#define RKISP1_ISP_MAX_WIDTH_CROP		4032
+#define RKISP1_ISP_MAX_HEIGHT_CROP		3024
+#define RKISP1_ISP_MIN_WIDTH			32
+#define RKISP1_ISP_MIN_HEIGHT			32
 
 #define RKISP1_RSZ_MP_SRC_MAX_WIDTH		4416
 #define RKISP1_RSZ_MP_SRC_MAX_HEIGHT		3312
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
index 2e5b57e3aedc..a8274e84a64b 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
@@ -758,9 +758,13 @@ static void rkisp1_isp_set_sink_crop(struct rkisp1_isp *isp,
 					  which);
 
 	sink_crop->left = ALIGN(r->left, 2);
-	sink_crop->width = ALIGN(r->width, 2);
+	sink_crop->width = clamp_t(u32, ALIGN(r->width, 2),
+				   RKISP1_ISP_MIN_WIDTH,
+				   RKISP1_ISP_MAX_WIDTH_CROP);
 	sink_crop->top = r->top;
-	sink_crop->height = r->height;
+	sink_crop->height = clamp_t(u32, r->height,
+				    RKISP1_ISP_MIN_HEIGHT,
+				    RKISP1_ISP_MAX_HEIGHT_CROP);
 	rkisp1_sd_adjust_crop(sink_crop, sink_fmt);
 
 	*r = *sink_crop;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] media: rkisp1: Increase ISP input resolution limit
  2021-03-26  6:23 [PATCH v2] media: rkisp1: Increase ISP input resolution limit Sebastian Fricke
@ 2021-03-29  5:40 ` Laurent Pinchart
  0 siblings, 0 replies; 2+ messages in thread
From: Laurent Pinchart @ 2021-03-29  5:40 UTC (permalink / raw)
  To: Sebastian Fricke; +Cc: linux-media, dafna.hirschfeld, helen.koike, heiko

Hi Sebastian,

Thank you for the patch.

On Fri, Mar 26, 2021 at 07:23:30AM +0100, Sebastian Fricke wrote:
> The current implementation limits the maximum sink pad resolution to
> 4032x3024, which is mentioned by the Rockchip TRM as the maximum size
> to handle black level calibration. But the ISP can actually set it's

s/calibration/correction/
s/it's/its/

> sink pad format to a size of 4416x3312.
> Allow higher sink pad resolutions in order to allow a bigger range of
> sensor modes to be used with the RkISP1.
> Apply the previous limit to the sink pad crop instead of the format to
> satisfy the requirement of the ISP.
> 
> Signed-off-by: Sebastian Fricke <sebastian.fricke@posteo.net>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
> Changes since v1:
> - Improve the name of the new macro -> s/*_PROCESSING/*_CROP/
> - Add a descriptive comment to explain why we need those 2 new macros
> - Combine the separated 2 patches into a single one as patch 2 was
>   depending on patch 1, which would have introduced a bisection breakage
>   (Thanks to Laurent Pinchart)
> 
> This patch was tested with a NanoPC-T4 and a OV13850, which provides a
> resolution of 4224x3136.
> ---
>  .../platform/rockchip/rkisp1/rkisp1-common.h   | 18 +++++++++++++-----
>  .../platform/rockchip/rkisp1/rkisp1-isp.c      |  8 ++++++--
>  2 files changed, 19 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> index 038c303a8aed..553a4b12becf 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h
> @@ -29,11 +29,19 @@
>  #define RKISP1_ISP_SD_SRC BIT(0)
>  #define RKISP1_ISP_SD_SINK BIT(1)
>  
> -/* min and max values for the widths and heights of the entities */
> -#define RKISP1_ISP_MAX_WIDTH		4032
> -#define RKISP1_ISP_MAX_HEIGHT		3024
> -#define RKISP1_ISP_MIN_WIDTH		32
> -#define RKISP1_ISP_MIN_HEIGHT		32
> +/*
> + * min and max values for the widths and heights of the entities
> + * The ISP device accepts input resolutions of up to 4416x3312, but
> + * it can only process resolutions of 4032x3024 internally.
> + * Therefore the crop resolution is limited to 4032x3024, the
> + * sink pad crop is applied automatically when the format is set.
> + */
> +#define RKISP1_ISP_MAX_WIDTH			4416
> +#define RKISP1_ISP_MAX_HEIGHT			3312
> +#define RKISP1_ISP_MAX_WIDTH_CROP		4032
> +#define RKISP1_ISP_MAX_HEIGHT_CROP		3024
> +#define RKISP1_ISP_MIN_WIDTH			32
> +#define RKISP1_ISP_MIN_HEIGHT			32
>  
>  #define RKISP1_RSZ_MP_SRC_MAX_WIDTH		4416
>  #define RKISP1_RSZ_MP_SRC_MAX_HEIGHT		3312
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> index 2e5b57e3aedc..a8274e84a64b 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c
> @@ -758,9 +758,13 @@ static void rkisp1_isp_set_sink_crop(struct rkisp1_isp *isp,
>  					  which);
>  
>  	sink_crop->left = ALIGN(r->left, 2);
> -	sink_crop->width = ALIGN(r->width, 2);
> +	sink_crop->width = clamp_t(u32, ALIGN(r->width, 2),
> +				   RKISP1_ISP_MIN_WIDTH,
> +				   RKISP1_ISP_MAX_WIDTH_CROP);
>  	sink_crop->top = r->top;
> -	sink_crop->height = r->height;
> +	sink_crop->height = clamp_t(u32, r->height,
> +				    RKISP1_ISP_MIN_HEIGHT,
> +				    RKISP1_ISP_MAX_HEIGHT_CROP);
>  	rkisp1_sd_adjust_crop(sink_crop, sink_fmt);
>  
>  	*r = *sink_crop;

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-03-29  5:41 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-26  6:23 [PATCH v2] media: rkisp1: Increase ISP input resolution limit Sebastian Fricke
2021-03-29  5:40 ` Laurent Pinchart

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).