[PATCH v5 18/29] media: rockchip: rga: check scaling factor
Sven Püschel
s.pueschel at pengutronix.de
Wed May 13 02:09:36 PDT 2026
Hi Nicolas,
On 5/9/26 1:11 AM, Nicolas Dufresne wrote:
> Le mardi 28 avril 2026 à 11:00 +0200, Sven Püschel a écrit :
>> Check the scaling factor to avoid potential problems. This is relevant
>> for the upcoming RGA3 support, as it can hang when the scaling factor
>> is exceeded.
>>
>> There are two relevant scenarios that have to be considered to protect
>> against invalid scaling values:
>>
>> When the output or capture is already streaming, setting the format on
>> the other side should consider the max scaling factor and clamp it
>> accordingly. This is only done in the streaming case, as it otherwise
>> may unintentionally clamp the value when the application sets the first
>> format (due to a default format on the other side).
>>
>> When the format is set on both sides first, then the format won't be
>> corrected by above means. Therefore the second streamon call has to
>> check the scaling factor and fail otherwise.
>>
>> As try functions should only be state aware if specified, the scaling
>> limitation is only done in s_fmt.
>>
>> Signed-off-by: Sven Püschel <s.pueschel at pengutronix.de>
>> ---
>> drivers/media/platform/rockchip/rga/rga-hw.c | 1 +
>> drivers/media/platform/rockchip/rga/rga-hw.h | 1 +
>> drivers/media/platform/rockchip/rga/rga.c | 47 ++++++++++++++++++++++++++++
>> drivers/media/platform/rockchip/rga/rga.h | 1 +
>> 4 files changed, 50 insertions(+)
>>
>> diff --git a/drivers/media/platform/rockchip/rga/rga-hw.c b/drivers/media/platform/rockchip/rga/rga-hw.c
>> index 11079477a3008..11a1a914668f6 100644
>> --- a/drivers/media/platform/rockchip/rga/rga-hw.c
>> +++ b/drivers/media/platform/rockchip/rga/rga-hw.c
>> @@ -595,6 +595,7 @@ const struct rga_hw rga2_hw = {
>> .max_width = MAX_WIDTH,
>> .min_height = MIN_HEIGHT,
>> .max_height = MAX_HEIGHT,
>> + .max_scaling_factor = MAX_SCALING_FACTOR,
>> .stride_alignment = 4,
>>
>> .setup_cmdbuf = rga_hw_setup_cmdbuf,
>> diff --git a/drivers/media/platform/rockchip/rga/rga-hw.h b/drivers/media/platform/rockchip/rga/rga-hw.h
>> index c2e34be751939..805ec23e5e3f4 100644
>> --- a/drivers/media/platform/rockchip/rga/rga-hw.h
>> +++ b/drivers/media/platform/rockchip/rga/rga-hw.h
>> @@ -14,6 +14,7 @@
>>
>> #define MIN_WIDTH 34
>> #define MIN_HEIGHT 34
>> +#define MAX_SCALING_FACTOR 16
>>
>> #define RGA_TIMEOUT 500
>>
>> diff --git a/drivers/media/platform/rockchip/rga/rga.c b/drivers/media/platform/rockchip/rga/rga.c
>> index d111b348255e2..75d05c86b1c00 100644
>> --- a/drivers/media/platform/rockchip/rga/rga.c
>> +++ b/drivers/media/platform/rockchip/rga/rga.c
>> @@ -405,10 +405,36 @@ static int vidioc_s_fmt(struct file *file, void *priv, struct v4l2_format *f)
>> struct v4l2_pix_format_mplane *pix_fmt = &f->fmt.pix_mp;
>> struct rga_ctx *ctx = file_to_rga_ctx(file);
>> struct rockchip_rga *rga = ctx->rga;
>> + const struct rga_hw *hw = rga->hw;
>> struct vb2_queue *vq;
>> struct rga_frame *frm;
>> int ret = 0;
>> int i;
>> + struct rga_frame *limit_frm = NULL;
>> +
>> + /* Limit before try_fmt to avoid recalculating the stride */
>> + if (V4L2_TYPE_IS_OUTPUT(f->type) &&
>> + v4l2_m2m_get_dst_vq(ctx->fh.m2m_ctx)->streaming)
>> + limit_frm = &ctx->out;
> If you need to, use helpers such as vb2_is_streaming(), though in this case, I
> think you want to use vb2_is_busy(), which protects against changing the format
> of a queue that is already allocated. This is needed because drivers, except vp9
> and av1 stateless decoders, don't track the format per buffer.
>
>> + if (V4L2_TYPE_IS_CAPTURE(f->type) &&
>> + v4l2_m2m_get_src_vq(ctx->fh.m2m_ctx)->streaming)
> Same.
>
>> + limit_frm = &ctx->in;
>> + if (limit_frm) {
>> + const struct v4l2_frmsize_stepwise frmsize = {
>> + .min_width = DIV_ROUND_UP(limit_frm->pix.width,
>> + hw->max_scaling_factor),
>> + .max_width =
>> + limit_frm->pix.width * hw->max_scaling_factor,
> Shouldn't you control the absolute min/max for this IP ?
That is done later in try_fmt. This is separately done in s_fmt to avoid
making try_fmt stateful. I'll add a comment to make it more clear that
this is only for the theoretical scaling limits.
>
>> + .min_height = DIV_ROUND_UP(limit_frm->pix.height,
>> + hw->max_scaling_factor),
>> + .max_height =
>> + limit_frm->pix.height * hw->max_scaling_factor,
>> + .step_width = 1,
>> + .step_height = 1,
> Shouldn't that step match the subsampling like you did earlier ?
same as above.
Sincerely
Sven
>
> Nicolas
>
>> + };
>> + v4l2_apply_frmsize_constraints(&pix_fmt->width,
>> + &pix_fmt->height, &frmsize);
>> + }
>>
>> /* Adjust all values accordingly to the hardware capabilities
>> * and chosen format.
>> @@ -568,12 +594,33 @@ static int vidioc_s_selection(struct file *file, void *priv,
>> return ret;
>> }
>>
>> +static bool check_scaling(const struct rga_hw *hw, u32 src_size, u32 dst_size)
>> +{
>> + if (src_size < dst_size)
>> + return src_size * hw->max_scaling_factor >= dst_size;
>> + else
>> + return dst_size * hw->max_scaling_factor >= src_size;
>> +}
>> +
>> static int vidioc_streamon(struct file *file, void *priv,
>> enum v4l2_buf_type type)
>> {
>> struct rga_ctx *ctx = file_to_rga_ctx(file);
>> const struct rga_hw *hw = ctx->rga->hw;
>>
>> + if ((V4L2_TYPE_IS_OUTPUT(type) &&
>> + v4l2_m2m_get_dst_vq(ctx->fh.m2m_ctx)->streaming) ||
>> + (V4L2_TYPE_IS_CAPTURE(type) &&
>> + v4l2_m2m_get_src_vq(ctx->fh.m2m_ctx)->streaming)) {
>> + /*
>> + * As the other side is already streaming,
>> + * check that the max scaling factor isn't exceeded.
>> + */
>> + if (!check_scaling(hw, ctx->in.pix.width, ctx->out.pix.width) ||
>> + !check_scaling(hw, ctx->in.pix.height, ctx->out.pix.height))
>> + return -EINVAL;
>> + }
>> +
>> hw->setup_cmdbuf(ctx);
>>
>> return v4l2_m2m_streamon(file, ctx->fh.m2m_ctx, type);
>> diff --git a/drivers/media/platform/rockchip/rga/rga.h b/drivers/media/platform/rockchip/rga/rga.h
>> index c741213710b32..454af283b1694 100644
>> --- a/drivers/media/platform/rockchip/rga/rga.h
>> +++ b/drivers/media/platform/rockchip/rga/rga.h
>> @@ -150,6 +150,7 @@ struct rga_hw {
>> size_t cmdbuf_size;
>> u32 min_width, min_height;
>> u32 max_width, max_height;
>> + u8 max_scaling_factor;
>> u8 stride_alignment;
>>
>> void (*setup_cmdbuf)(struct rga_ctx *ctx);
More information about the Linux-rockchip
mailing list