From: Paul Kocialkowski paulk@sys-base.io
[ Upstream commit 73d50aa92f28ee8414fbfde011974fce970b82cc ]
Call the dedicated v4l2_disable_ioctl helper instead of manually checking whether the current context is an encoder for the selection api ioctls.
Signed-off-by: Paul Kocialkowski paulk@sys-base.io Reviewed-by: Nicolas Dufresne nicolas.dufresne@collabora.com Signed-off-by: Nicolas Dufresne nicolas.dufresne@collabora.com Signed-off-by: Hans Verkuil hverkuil+cisco@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
YES
**Why This Fix Matters** - Correctly hides unsupported ioctls on decoder nodes: Previously, the driver exposed `VIDIOC_G_SELECTION`/`VIDIOC_S_SELECTION` to decoders but rejected them at runtime with `-EINVAL`. This incorrectly advertised capability and confused userspace and core heuristics. With this change, those ioctls are explicitly disabled and return `-ENOTTY`, which is the correct “not supported” error and matches V4L2 expectations (drivers/media/v4l2-core/v4l2-ioctl.c:3073, 3111). - Avoids misleading legacy crop exposure: V4L2 core auto-enables legacy crop ioctls if selection is available. Disabling selection for decoders prevents the core from enabling `VIDIOC_G_CROP/CROPCAP` on decoder nodes (drivers/media/v4l2-core/v4l2-dev.c:657, drivers/media/v4l2-core/v4l2-dev.c:659, drivers/media/v4l2-core/v4l2-dev.c:662, drivers/media/v4l2-core/v4l2-dev.c:663, drivers/media/v4l2-core/v4l2-dev.c:664, drivers/media/v4l2-core/v4l2-dev.c:665). This fixes a user-visible API correctness issue.
**Change Details** - Disables selection ioctls for decoder device nodes using the standard helper, before registration: - `v4l2_disable_ioctl(vfd, VIDIOC_G_SELECTION);` - `v4l2_disable_ioctl(vfd, VIDIOC_S_SELECTION);` - Location: drivers/media/platform/verisilicon/hantro_drv.c:918, drivers/media/platform/verisilicon/hantro_drv.c:919 - Called before `video_register_device`, as required (drivers/media/platform/verisilicon/hantro_drv.c:924). - Simplifies selection handlers to only enforce buffer type, removing runtime checks on context role: - Dropped `!ctx->is_encoder` checks; now only `sel->type != V4L2_BUF_TYPE_VIDEO_OUTPUT` is validated. - `vidioc_g_selection`: drivers/media/platform/verisilicon/hantro_v4l2.c:666–667 - `vidioc_s_selection`: drivers/media/platform/verisilicon/hantro_v4l2.c:698–699 - Effect: No functional change for encoders (where `ctx->is_encoder` is always true), and decoders won’t reach these handlers since the ioctls are disabled.
**Risk and Side Effects** - Behavior change is limited to decoders for selection ioctls: return code changes from `-EINVAL` to `-ENOTTY` via core gating (`is_valid_ioctl()` fails, `ret` remains `-ENOTTY`, drivers/media/v4l2-core/v4l2-ioctl.c:3073, 3111–3113). This is the correct semantics for “unsupported ioctl” and improves userspace detection. - No architectural changes; confined to the Verisilicon Hantro driver. Encoder behavior is unchanged. - Very small, contained patch; unlikely to introduce regressions. Aligns with common media driver practice of disabling non-applicable ioctls for a given node.
**Stable Backport Fit** - Fixes a user-visible API bug (misadvertised capability and wrong errno) with minimal, localized changes. - No new features or interfaces; follows stable rules for correctness fixes. - Touches a non-core subsystem (media, platform driver), minimizing cross-subsystem risk.
Given the above, this commit is a good candidate for stable backporting.
drivers/media/platform/verisilicon/hantro_drv.c | 2 ++ drivers/media/platform/verisilicon/hantro_v4l2.c | 6 ++---- 2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/media/platform/verisilicon/hantro_drv.c b/drivers/media/platform/verisilicon/hantro_drv.c index fa972effd4a2c..9d5e50fedae1f 100644 --- a/drivers/media/platform/verisilicon/hantro_drv.c +++ b/drivers/media/platform/verisilicon/hantro_drv.c @@ -917,6 +917,8 @@ static int hantro_add_func(struct hantro_dev *vpu, unsigned int funcid) vpu->decoder = func; v4l2_disable_ioctl(vfd, VIDIOC_TRY_ENCODER_CMD); v4l2_disable_ioctl(vfd, VIDIOC_ENCODER_CMD); + v4l2_disable_ioctl(vfd, VIDIOC_G_SELECTION); + v4l2_disable_ioctl(vfd, VIDIOC_S_SELECTION); }
video_set_drvdata(vfd, vpu); diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c b/drivers/media/platform/verisilicon/hantro_v4l2.c index 7c3515cf7d64a..4598f9b4bd21c 100644 --- a/drivers/media/platform/verisilicon/hantro_v4l2.c +++ b/drivers/media/platform/verisilicon/hantro_v4l2.c @@ -663,8 +663,7 @@ static int vidioc_g_selection(struct file *file, void *priv, struct hantro_ctx *ctx = fh_to_ctx(priv);
/* Crop only supported on source. */ - if (!ctx->is_encoder || - sel->type != V4L2_BUF_TYPE_VIDEO_OUTPUT) + if (sel->type != V4L2_BUF_TYPE_VIDEO_OUTPUT) return -EINVAL;
switch (sel->target) { @@ -696,8 +695,7 @@ static int vidioc_s_selection(struct file *file, void *priv, struct vb2_queue *vq;
/* Crop only supported on source. */ - if (!ctx->is_encoder || - sel->type != V4L2_BUF_TYPE_VIDEO_OUTPUT) + if (sel->type != V4L2_BUF_TYPE_VIDEO_OUTPUT) return -EINVAL;
/* Change not allowed if the queue is streaming. */