media: renesas: vsp1: rpf: Fix crop left and top clamping

The RPF doesn't enforces the alignment constraint on the sink pad
format, which could have an odd size, possibly down to 1x1. In that
case, the upper bounds for the left and top coordinates clamping would
become negative, cast to a very large positive value. Incorrect crop
rectangle coordinates would then be incorrectly accepted.

A second issue can occur when the requested left and top coordinates are
negative. They are cast to a large unsigned value, clamped to the
maximum. While the calculation will produce valid values for the
hardware, this is not compliant with the V4L2 specification that
requires values to be adjusted to the closest valid value.

Fix both issues by switching to signed clamping, with an explicit
minimum to adjust negative values, and adjusting the clamp bounds to
avoid negative upper bounds.

Tested-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Tested-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> # HiHope RZ/G2M
Link: https://patch.msgid.link/20260318235907.831556-5-laurent.pinchart+renesas@ideasonboard.com
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>

diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
index 56464875a6c5e3b6e56e4ca6022a38bbee3ff22b..ffc1b3ab54e2494127aa19eef4ae11a013e6ac2e 100644 (file)
--- a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
+++ b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c
@@ -201,6 +201,8 @@ static int vsp1_rwpf_set_selection(struct v4l2_subdev *subdev,
                                   struct v4l2_subdev_state *sd_state,
                                   struct v4l2_subdev_selection *sel)
 {
+       unsigned int min_width = RWPF_MIN_WIDTH;
+       unsigned int min_height = RWPF_MIN_HEIGHT;
        struct vsp1_rwpf *rwpf = to_rwpf(subdev);
        struct v4l2_subdev_state *state;
        struct v4l2_mbus_framefmt *format;
@@ -229,18 +231,36 @@ static int vsp1_rwpf_set_selection(struct v4l2_subdev *subdev,
        format = v4l2_subdev_state_get_format(state, RWPF_PAD_SINK);
 
        /*
-        * Restrict the crop rectangle coordinates to multiples of 2 to avoid
-        * shifting the color plane.
+        * For YUV formats, restrict the crop rectangle coordinates to multiples
+        * of 2 to avoid shifting the color plane.
         */
        if (format->code == MEDIA_BUS_FMT_AYUV8_1X32) {
                sel->r.left = ALIGN(sel->r.left, 2);
                sel->r.top = ALIGN(sel->r.top, 2);
                sel->r.width = round_down(sel->r.width, 2);
                sel->r.height = round_down(sel->r.height, 2);
+
+               /*
+                * The RPF doesn't enforces the alignment constraint on the sink
+                * pad format, which could have an odd size, possibly down to
+                * 1x1. In that case, the minimum width and height would be
+                * smaller than the sink pad format, leading to a negative upper
+                * bound in the left and top clamping. Clamp the minimum width
+                * and height to the format width and height to avoid this.
+                *
+                * In such a situation, odd values for the crop rectangle size
+                * would be accepted when clamping the width and height below.
+                * While that would create an invalid hardware configuration,
+                * the video device enforces proper alignment of the pixel
+                * format, and the mismatch will then result in link validation
+                * failure. Incorrect operation of the hardware is not possible.
+                */
+               min_width = min(ALIGN(min_width, 2), format->width);
+               min_height = min(ALIGN(min_height, 2), format->height);
        }
 
-       sel->r.left = min_t(unsigned int, sel->r.left, format->width - 2);
-       sel->r.top = min_t(unsigned int, sel->r.top, format->height - 2);
+       sel->r.left = clamp_t(int, sel->r.left, 0, format->width - min_width);
+       sel->r.top = clamp_t(int, sel->r.top, 0, format->height - min_height);
        sel->r.width = min_t(unsigned int, sel->r.width,
                             format->width - sel->r.left);
        sel->r.height = min_t(unsigned int, sel->r.height,