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@xxxxxxxxxx> Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > --- > 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