Linux ARM-MSM sub-architecture
 help / color / mirror / Atom feed
* [PATCH 00/25] media: v4l2-mem2mem: Reduce cargo-cult
@ 2025-10-08 17:50 Laurent Pinchart
  2025-10-08 17:50 ` [PATCH 15/25] media: qcom: iris: Drop unneeded v4l2_m2m_get_vq() NULL check Laurent Pinchart
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Laurent Pinchart @ 2025-10-08 17:50 UTC (permalink / raw)
  To: linux-media, linux-mediatek, imx, linux-amlogic,
	linux-renesas-soc, linux-stm32, linux-rockchip, linux-arm-kernel,
	linux-arm-msm
  Cc: Nicolas Dufresne, Matthew Majewski, Mirela Rabulea,
	Jacek Anaszewski, Michael Tretter, Devarsh Thakkar,
	Uwe Kleine-König, AngeloGioacchino Del Regno,
	Martin Blumenstingl, Alexandre Torgue, Philipp Zabel,
	Arnd Bergmann, Sylwester Nawrocki, Ming Qian,
	Pengutronix Kernel Team, Kieran Bingham, Nathan Chancellor,
	Sascha Hauer, Hans Verkuil, Mikhail Ulyanov, Kevin Hilman,
	Xavier Roumegue, Shuah Khan, Andrzej Pietrasiewicz, Tiffany Lin,
	Zhou Peng, Dikshita Agarwal, Vikash Garodia, Yunfei Dong,
	Ezequiel Garcia, Neil Armstrong, Jiasheng Jiang, Shawn Guo,
	Łukasz Stelmach, Abhinav Kumar, Heiko Stuebner,
	Benoit Parrot, Andrew-CT Chen, Geert Uytterhoeven, Magnus Damm,
	Sebastian Fricke, Maxime Coquelin, Jacob Chen, Bin Liu,
	Matthias Brugger, Fabio Estevam, Jerome Brunet,
	Bryan O'Donoghue

Hello,

The v4l2_m2m_get_vq() function never returns NULL, but many mem2mem
drivers still check its return value and consider NULL as an error. This
may have originated a long time ago from valid checks when
v4l2_m2m_get_vq() could return NULL, with drivers then just copying the
checks. This series attempts to stop the cargo-cult behaviour.

Patch 01/25 starts by explicitly stating in kerneldoc that the
v4l2_m2m_get_vq() function never returns NULL. All the other patches
drop NULL checks from drivers.

I have carefully checked all patched locations in all drivers. They fall
in 3 categories:

- Checks in the VIDIOC_G_FMT, VIDIOC_TRY_FMT and VIDIOC_S_FMT handlers:
  Those may have been added to ensure that the format type has a valid
  value, but that is ensured by the V4L2 ioctl core before calling the
  handlers. The checks can be dropped without a need to replace them
  with proper type checks.

- Checks in the VIDIOC_S_SELECTION handler: The only location where this
  is performed has an explicit type check, so the NULL check can also be
  dropped.

- Checks in other locations where the type parameter to the
  v4l2_m2m_get_vq() function is hardcoded: The hardcoded type is valid,
  so the NULL check can't have been meant to check the type. It can also
  be removed.

There's no dependency between any of those patches so they can be merged
in any order.

Laurent Pinchart (25):
  media: v4l2-mem2mem: Document that v4l2_m2m_get_vq() never returns
    NULL
  media: allgro-dvt: Drop unneeded v4l2_m2m_get_vq() NULL check
  media: meson-g2d: Drop unneeded v4l2_m2m_get_vq() NULL check
  media: amphion: Drop unneeded v4l2_m2m_get_vq() NULL check
  media: coda: Drop unneeded v4l2_m2m_get_vq() NULL check
  media: imagination: e5010: Drop unneeded v4l2_m2m_get_vq() NULL check
  media: m2m-deinterlace: Drop unneeded v4l2_m2m_get_vq() NULL check
  media: mediatek: jpeg: Drop unneeded v4l2_m2m_get_vq() NULL check
  media: mediatek: vcodec: Drop unneeded v4l2_m2m_get_vq() NULL check
  media: dw100: Drop unneeded v4l2_m2m_get_vq() NULL check
  media: imx-jpeg: Drop unneeded v4l2_m2m_get_vq() NULL check
  media: imx-pxp: Drop unneeded v4l2_m2m_get_vq() NULL check
  media: nxp: imx8-isi: Drop unneeded v4l2_m2m_get_vq() NULL check
  media: mx2_emmaprp: Drop unneeded v4l2_m2m_get_vq() NULL check
  media: qcom: iris: Drop unneeded v4l2_m2m_get_vq() NULL check
  media: qcom: venus: Drop unneeded v4l2_m2m_get_vq() NULL check
  media: renesas: fdp1: Drop unneeded v4l2_m2m_get_vq() NULL check
  media: rcar_jpu: Drop unneeded v4l2_m2m_get_vq() NULL check
  media: platform: rga: Drop unneeded v4l2_m2m_get_vq() NULL check
  media: samsung: s5p-g2d: Drop unneeded v4l2_m2m_get_vq() NULL check
  media: samsung: s5p-jpeg: Drop unneeded v4l2_m2m_get_vq() NULL check
  media: stm32: dma2d: Drop unneeded v4l2_m2m_get_vq() NULL check
  media: ti: vpe: Drop unneeded v4l2_m2m_get_vq() NULL check
  media: vicodec: Drop unneeded v4l2_m2m_get_vq() NULL check
  media: vim2m: Drop unneeded v4l2_m2m_get_vq() NULL check

 drivers/media/platform/allegro-dvt/allegro-core.c    |  2 --
 drivers/media/platform/amlogic/meson-ge2d/ge2d.c     |  5 -----
 drivers/media/platform/amphion/vdec.c                |  2 --
 drivers/media/platform/amphion/venc.c                |  2 --
 .../media/platform/chips-media/coda/coda-common.c    |  4 ----
 drivers/media/platform/imagination/e5010-jpeg-enc.c  |  4 ----
 drivers/media/platform/m2m-deinterlace.c             |  7 -------
 drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c |  7 -------
 .../mediatek/vcodec/decoder/mtk_vcodec_dec.c         |  7 -------
 .../vcodec/decoder/vdec/vdec_av1_req_lat_if.c        |  2 --
 .../vcodec/decoder/vdec/vdec_vp9_req_lat_if.c        |  2 --
 .../mediatek/vcodec/encoder/mtk_vcodec_enc.c         |  8 --------
 drivers/media/platform/nxp/dw100/dw100.c             |  7 -------
 drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c       |  4 ----
 drivers/media/platform/nxp/imx-pxp.c                 |  7 -------
 drivers/media/platform/nxp/imx8-isi/imx8-isi-m2m.c   |  2 --
 drivers/media/platform/nxp/mx2_emmaprp.c             |  7 -------
 drivers/media/platform/qcom/iris/iris_vdec.c         |  2 --
 drivers/media/platform/qcom/venus/vdec.c             |  2 --
 drivers/media/platform/qcom/venus/venc.c             |  2 --
 drivers/media/platform/renesas/rcar_fdp1.c           |  3 ---
 drivers/media/platform/renesas/rcar_jpu.c            |  8 --------
 drivers/media/platform/rockchip/rga/rga.c            |  4 ----
 drivers/media/platform/samsung/s5p-g2d/g2d.c         |  4 ----
 drivers/media/platform/samsung/s5p-jpeg/jpeg-core.c  |  7 -------
 drivers/media/platform/st/stm32/dma2d/dma2d.c        |  5 -----
 drivers/media/platform/ti/vpe/vpe.c                  |  7 -------
 drivers/media/test-drivers/vicodec/vicodec-core.c    |  7 -------
 drivers/media/test-drivers/vim2m.c                   | 12 ------------
 drivers/media/v4l2-core/v4l2-mem2mem.c               | 12 +-----------
 include/media/v4l2-mem2mem.h                         |  3 +++
 31 files changed, 4 insertions(+), 153 deletions(-)


base-commit: e5f0a698b34ed76002dc5cff3804a61c80233a7a
-- 
Regards,

Laurent Pinchart


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

* [PATCH 15/25] media: qcom: iris: Drop unneeded v4l2_m2m_get_vq() NULL check
  2025-10-08 17:50 [PATCH 00/25] media: v4l2-mem2mem: Reduce cargo-cult Laurent Pinchart
@ 2025-10-08 17:50 ` Laurent Pinchart
  2025-10-08 23:51   ` Bryan O'Donoghue
  2025-10-09 13:53   ` Dikshita Agarwal
  2025-10-08 17:50 ` [PATCH 16/25] media: qcom: venus: " Laurent Pinchart
  2025-10-08 18:24 ` [PATCH 00/25] media: v4l2-mem2mem: Reduce cargo-cult Nicolas Dufresne
  2 siblings, 2 replies; 10+ messages in thread
From: Laurent Pinchart @ 2025-10-08 17:50 UTC (permalink / raw)
  To: linux-media, linux-arm-msm
  Cc: Nicolas Dufresne, Vikash Garodia, Dikshita Agarwal, Abhinav Kumar,
	Bryan O'Donoghue

The v4l2_m2m_get_vq() function never returns NULL. The check may have
been intended to catch invalid format types, but that's not needed as
the V4L2 core picks the appropriate VIDIOC_S_FMT ioctl handler based on
the format type, so the type can't be incorrect. Drop the unneeded
return value check.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/platform/qcom/iris/iris_vdec.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/media/platform/qcom/iris/iris_vdec.c b/drivers/media/platform/qcom/iris/iris_vdec.c
index d670b51c5839..1e9ffdbb6e18 100644
--- a/drivers/media/platform/qcom/iris/iris_vdec.c
+++ b/drivers/media/platform/qcom/iris/iris_vdec.c
@@ -191,8 +191,6 @@ int iris_vdec_s_fmt(struct iris_inst *inst, struct v4l2_format *f)
 	u32 codec_align;
 
 	q = v4l2_m2m_get_vq(inst->m2m_ctx, f->type);
-	if (!q)
-		return -EINVAL;
 
 	if (vb2_is_busy(q))
 		return -EBUSY;
-- 
Regards,

Laurent Pinchart


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

* [PATCH 16/25] media: qcom: venus: Drop unneeded v4l2_m2m_get_vq() NULL check
  2025-10-08 17:50 [PATCH 00/25] media: v4l2-mem2mem: Reduce cargo-cult Laurent Pinchart
  2025-10-08 17:50 ` [PATCH 15/25] media: qcom: iris: Drop unneeded v4l2_m2m_get_vq() NULL check Laurent Pinchart
@ 2025-10-08 17:50 ` Laurent Pinchart
  2025-10-08 23:52   ` Bryan O'Donoghue
  2025-10-09  7:42   ` Dikshita Agarwal
  2025-10-08 18:24 ` [PATCH 00/25] media: v4l2-mem2mem: Reduce cargo-cult Nicolas Dufresne
  2 siblings, 2 replies; 10+ messages in thread
From: Laurent Pinchart @ 2025-10-08 17:50 UTC (permalink / raw)
  To: linux-media, linux-arm-msm
  Cc: Nicolas Dufresne, Vikash Garodia, Dikshita Agarwal,
	Bryan O'Donoghue

The v4l2_m2m_get_vq() function never returns NULL. The check may have
been intended to catch invalid format types, but that's not needed as
the V4L2 core picks the appropriate VIDIOC_S_FMT ioctl handler based on
the format type, so the type can't be incorrect. Drop the unneeded
return value check.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/platform/qcom/venus/vdec.c | 2 --
 drivers/media/platform/qcom/venus/venc.c | 2 --
 2 files changed, 4 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
index 29b0d6a5303d..8c77db0f6e76 100644
--- a/drivers/media/platform/qcom/venus/vdec.c
+++ b/drivers/media/platform/qcom/venus/vdec.c
@@ -329,8 +329,6 @@ static int vdec_s_fmt(struct file *file, void *fh, struct v4l2_format *f)
 	struct vb2_queue *q;
 
 	q = v4l2_m2m_get_vq(inst->m2m_ctx, f->type);
-	if (!q)
-		return -EINVAL;
 
 	if (vb2_is_busy(q))
 		return -EBUSY;
diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
index c0a0ccdded80..0fe4cc37118b 100644
--- a/drivers/media/platform/qcom/venus/venc.c
+++ b/drivers/media/platform/qcom/venus/venc.c
@@ -241,8 +241,6 @@ static int venc_s_fmt(struct file *file, void *fh, struct v4l2_format *f)
 	struct vb2_queue *q;
 
 	q = v4l2_m2m_get_vq(inst->m2m_ctx, f->type);
-	if (!q)
-		return -EINVAL;
 
 	if (vb2_is_busy(q))
 		return -EBUSY;
-- 
Regards,

Laurent Pinchart


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

* Re: [PATCH 00/25] media: v4l2-mem2mem: Reduce cargo-cult
  2025-10-08 17:50 [PATCH 00/25] media: v4l2-mem2mem: Reduce cargo-cult Laurent Pinchart
  2025-10-08 17:50 ` [PATCH 15/25] media: qcom: iris: Drop unneeded v4l2_m2m_get_vq() NULL check Laurent Pinchart
  2025-10-08 17:50 ` [PATCH 16/25] media: qcom: venus: " Laurent Pinchart
@ 2025-10-08 18:24 ` Nicolas Dufresne
  2025-10-08 19:21   ` Laurent Pinchart
  2 siblings, 1 reply; 10+ messages in thread
From: Nicolas Dufresne @ 2025-10-08 18:24 UTC (permalink / raw)
  To: Laurent Pinchart, linux-media, linux-mediatek, imx, linux-amlogic,
	linux-renesas-soc, linux-stm32, linux-rockchip, linux-arm-kernel,
	linux-arm-msm
  Cc: Matthew Majewski, Mirela Rabulea, Jacek Anaszewski,
	Michael Tretter, Devarsh Thakkar, Uwe Kleine-König,
	AngeloGioacchino Del Regno, Martin Blumenstingl, Alexandre Torgue,
	Philipp Zabel, Arnd Bergmann, Sylwester Nawrocki, Ming Qian,
	Pengutronix Kernel Team, Kieran Bingham, Nathan Chancellor,
	Sascha Hauer, Hans Verkuil, Mikhail Ulyanov, Kevin Hilman,
	Xavier Roumegue, Shuah Khan, Andrzej Pietrasiewicz, Tiffany Lin,
	Zhou Peng, Dikshita Agarwal, Vikash Garodia, Yunfei Dong,
	Ezequiel Garcia, Neil Armstrong, Jiasheng Jiang, Shawn Guo,
	Łukasz Stelmach, Abhinav Kumar, Heiko Stuebner,
	Benoit Parrot, Andrew-CT Chen, Geert Uytterhoeven, Magnus Damm,
	Sebastian Fricke, Maxime Coquelin, Jacob Chen, Bin Liu,
	Matthias Brugger, Fabio Estevam, Jerome Brunet,
	Bryan O'Donoghue

[-- Attachment #1: Type: text/plain, Size: 6036 bytes --]

Hi,

Le mercredi 08 octobre 2025 à 20:50 +0300, Laurent Pinchart a écrit :
> Hello,
> 
> The v4l2_m2m_get_vq() function never returns NULL, but many mem2mem
> drivers still check its return value and consider NULL as an error. This
> may have originated a long time ago from valid checks when
> v4l2_m2m_get_vq() could return NULL, with drivers then just copying the
> checks. This series attempts to stop the cargo-cult behaviour.
> 
> Patch 01/25 starts by explicitly stating in kerneldoc that the
> v4l2_m2m_get_vq() function never returns NULL. All the other patches
> drop NULL checks from drivers.
> 
> I have carefully checked all patched locations in all drivers. They fall
> in 3 categories:
> 
> - Checks in the VIDIOC_G_FMT, VIDIOC_TRY_FMT and VIDIOC_S_FMT handlers:
>   Those may have been added to ensure that the format type has a valid
>   value, but that is ensured by the V4L2 ioctl core before calling the
>   handlers. The checks can be dropped without a need to replace them
>   with proper type checks.
> 
> - Checks in the VIDIOC_S_SELECTION handler: The only location where this
>   is performed has an explicit type check, so the NULL check can also be
>   dropped.
> 
> - Checks in other locations where the type parameter to the
>   v4l2_m2m_get_vq() function is hardcoded: The hardcoded type is valid,
>   so the NULL check can't have been meant to check the type. It can also
>   be removed.
> 
> There's no dependency between any of those patches so they can be merged
> in any order.
> 
> Laurent Pinchart (25):
>   media: v4l2-mem2mem: Document that v4l2_m2m_get_vq() never returns
>     NULL
>   media: allgro-dvt: Drop unneeded v4l2_m2m_get_vq() NULL check
>   media: meson-g2d: Drop unneeded v4l2_m2m_get_vq() NULL check
>   media: amphion: Drop unneeded v4l2_m2m_get_vq() NULL check
>   media: coda: Drop unneeded v4l2_m2m_get_vq() NULL check
>   media: imagination: e5010: Drop unneeded v4l2_m2m_get_vq() NULL check
>   media: m2m-deinterlace: Drop unneeded v4l2_m2m_get_vq() NULL check
>   media: mediatek: jpeg: Drop unneeded v4l2_m2m_get_vq() NULL check
>   media: mediatek: vcodec: Drop unneeded v4l2_m2m_get_vq() NULL check
>   media: dw100: Drop unneeded v4l2_m2m_get_vq() NULL check
>   media: imx-jpeg: Drop unneeded v4l2_m2m_get_vq() NULL check
>   media: imx-pxp: Drop unneeded v4l2_m2m_get_vq() NULL check
>   media: nxp: imx8-isi: Drop unneeded v4l2_m2m_get_vq() NULL check
>   media: mx2_emmaprp: Drop unneeded v4l2_m2m_get_vq() NULL check
>   media: qcom: iris: Drop unneeded v4l2_m2m_get_vq() NULL check
>   media: qcom: venus: Drop unneeded v4l2_m2m_get_vq() NULL check
>   media: renesas: fdp1: Drop unneeded v4l2_m2m_get_vq() NULL check
>   media: rcar_jpu: Drop unneeded v4l2_m2m_get_vq() NULL check

Why not "renesas: jpu" to match the fdp1 patch naming ?

>   media: platform: rga: Drop unneeded v4l2_m2m_get_vq() NULL check
>   media: samsung: s5p-g2d: Drop unneeded v4l2_m2m_get_vq() NULL check
>   media: samsung: s5p-jpeg: Drop unneeded v4l2_m2m_get_vq() NULL check
>   media: stm32: dma2d: Drop unneeded v4l2_m2m_get_vq() NULL check
>   media: ti: vpe: Drop unneeded v4l2_m2m_get_vq() NULL check
>   media: vicodec: Drop unneeded v4l2_m2m_get_vq() NULL check
>   media: vim2m: Drop unneeded v4l2_m2m_get_vq() NULL check

I reviewed the list and it seems complete to me.

Nicolas

> 
>  drivers/media/platform/allegro-dvt/allegro-core.c    |  2 --
>  drivers/media/platform/amlogic/meson-ge2d/ge2d.c     |  5 -----
>  drivers/media/platform/amphion/vdec.c                |  2 --
>  drivers/media/platform/amphion/venc.c                |  2 --
>  .../media/platform/chips-media/coda/coda-common.c    |  4 ----
>  drivers/media/platform/imagination/e5010-jpeg-enc.c  |  4 ----
>  drivers/media/platform/m2m-deinterlace.c             |  7 -------
>  drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c |  7 -------
>  .../mediatek/vcodec/decoder/mtk_vcodec_dec.c         |  7 -------
>  .../vcodec/decoder/vdec/vdec_av1_req_lat_if.c        |  2 --
>  .../vcodec/decoder/vdec/vdec_vp9_req_lat_if.c        |  2 --
>  .../mediatek/vcodec/encoder/mtk_vcodec_enc.c         |  8 --------
>  drivers/media/platform/nxp/dw100/dw100.c             |  7 -------
>  drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c       |  4 ----
>  drivers/media/platform/nxp/imx-pxp.c                 |  7 -------
>  drivers/media/platform/nxp/imx8-isi/imx8-isi-m2m.c   |  2 --
>  drivers/media/platform/nxp/mx2_emmaprp.c             |  7 -------
>  drivers/media/platform/qcom/iris/iris_vdec.c         |  2 --
>  drivers/media/platform/qcom/venus/vdec.c             |  2 --
>  drivers/media/platform/qcom/venus/venc.c             |  2 --
>  drivers/media/platform/renesas/rcar_fdp1.c           |  3 ---
>  drivers/media/platform/renesas/rcar_jpu.c            |  8 --------
>  drivers/media/platform/rockchip/rga/rga.c            |  4 ----
>  drivers/media/platform/samsung/s5p-g2d/g2d.c         |  4 ----
>  drivers/media/platform/samsung/s5p-jpeg/jpeg-core.c  |  7 -------
>  drivers/media/platform/st/stm32/dma2d/dma2d.c        |  5 -----
>  drivers/media/platform/ti/vpe/vpe.c                  |  7 -------
>  drivers/media/test-drivers/vicodec/vicodec-core.c    |  7 -------
>  drivers/media/test-drivers/vim2m.c                   | 12 ------------
>  drivers/media/v4l2-core/v4l2-mem2mem.c               | 12 +-----------
>  include/media/v4l2-mem2mem.h                         |  3 +++
>  31 files changed, 4 insertions(+), 153 deletions(-)
> 
> 
> base-commit: e5f0a698b34ed76002dc5cff3804a61c80233a7a

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

* Re: [PATCH 00/25] media: v4l2-mem2mem: Reduce cargo-cult
  2025-10-08 18:24 ` [PATCH 00/25] media: v4l2-mem2mem: Reduce cargo-cult Nicolas Dufresne
@ 2025-10-08 19:21   ` Laurent Pinchart
  0 siblings, 0 replies; 10+ messages in thread
From: Laurent Pinchart @ 2025-10-08 19:21 UTC (permalink / raw)
  To: Nicolas Dufresne
  Cc: linux-media, linux-mediatek, imx, linux-amlogic,
	linux-renesas-soc, linux-stm32, linux-rockchip, linux-arm-kernel,
	linux-arm-msm, Matthew Majewski, Mirela Rabulea, Jacek Anaszewski,
	Michael Tretter, Devarsh Thakkar, Uwe Kleine-König,
	AngeloGioacchino Del Regno, Martin Blumenstingl, Alexandre Torgue,
	Philipp Zabel, Arnd Bergmann, Sylwester Nawrocki, Ming Qian,
	Pengutronix Kernel Team, Kieran Bingham, Nathan Chancellor,
	Sascha Hauer, Hans Verkuil, Kevin Hilman, Xavier Roumegue,
	Shuah Khan, Andrzej Pietrasiewicz, Tiffany Lin, Zhou Peng,
	Dikshita Agarwal, Vikash Garodia, Yunfei Dong, Ezequiel Garcia,
	Neil Armstrong, Jiasheng Jiang, Shawn Guo, Łukasz Stelmach,
	Abhinav Kumar, Heiko Stuebner, Benoit Parrot, Andrew-CT Chen,
	Geert Uytterhoeven, Magnus Damm, Maxime Coquelin, Jacob Chen,
	Bin Liu, Matthias Brugger, Fabio Estevam, Jerome Brunet,
	Bryan O'Donoghue

On Wed, Oct 08, 2025 at 02:24:30PM -0400, Nicolas Dufresne wrote:
> Hi,
> 
> Le mercredi 08 octobre 2025 à 20:50 +0300, Laurent Pinchart a écrit :
> > Hello,
> > 
> > The v4l2_m2m_get_vq() function never returns NULL, but many mem2mem
> > drivers still check its return value and consider NULL as an error. This
> > may have originated a long time ago from valid checks when
> > v4l2_m2m_get_vq() could return NULL, with drivers then just copying the
> > checks. This series attempts to stop the cargo-cult behaviour.
> > 
> > Patch 01/25 starts by explicitly stating in kerneldoc that the
> > v4l2_m2m_get_vq() function never returns NULL. All the other patches
> > drop NULL checks from drivers.
> > 
> > I have carefully checked all patched locations in all drivers. They fall
> > in 3 categories:
> > 
> > - Checks in the VIDIOC_G_FMT, VIDIOC_TRY_FMT and VIDIOC_S_FMT handlers:
> >   Those may have been added to ensure that the format type has a valid
> >   value, but that is ensured by the V4L2 ioctl core before calling the
> >   handlers. The checks can be dropped without a need to replace them
> >   with proper type checks.
> > 
> > - Checks in the VIDIOC_S_SELECTION handler: The only location where this
> >   is performed has an explicit type check, so the NULL check can also be
> >   dropped.
> > 
> > - Checks in other locations where the type parameter to the
> >   v4l2_m2m_get_vq() function is hardcoded: The hardcoded type is valid,
> >   so the NULL check can't have been meant to check the type. It can also
> >   be removed.
> > 
> > There's no dependency between any of those patches so they can be merged
> > in any order.
> > 
> > Laurent Pinchart (25):
> >   media: v4l2-mem2mem: Document that v4l2_m2m_get_vq() never returns
> >     NULL
> >   media: allgro-dvt: Drop unneeded v4l2_m2m_get_vq() NULL check
> >   media: meson-g2d: Drop unneeded v4l2_m2m_get_vq() NULL check
> >   media: amphion: Drop unneeded v4l2_m2m_get_vq() NULL check
> >   media: coda: Drop unneeded v4l2_m2m_get_vq() NULL check
> >   media: imagination: e5010: Drop unneeded v4l2_m2m_get_vq() NULL check
> >   media: m2m-deinterlace: Drop unneeded v4l2_m2m_get_vq() NULL check
> >   media: mediatek: jpeg: Drop unneeded v4l2_m2m_get_vq() NULL check
> >   media: mediatek: vcodec: Drop unneeded v4l2_m2m_get_vq() NULL check
> >   media: dw100: Drop unneeded v4l2_m2m_get_vq() NULL check
> >   media: imx-jpeg: Drop unneeded v4l2_m2m_get_vq() NULL check
> >   media: imx-pxp: Drop unneeded v4l2_m2m_get_vq() NULL check
> >   media: nxp: imx8-isi: Drop unneeded v4l2_m2m_get_vq() NULL check
> >   media: mx2_emmaprp: Drop unneeded v4l2_m2m_get_vq() NULL check
> >   media: qcom: iris: Drop unneeded v4l2_m2m_get_vq() NULL check
> >   media: qcom: venus: Drop unneeded v4l2_m2m_get_vq() NULL check
> >   media: renesas: fdp1: Drop unneeded v4l2_m2m_get_vq() NULL check
> >   media: rcar_jpu: Drop unneeded v4l2_m2m_get_vq() NULL check
> 
> Why not "renesas: jpu" to match the fdp1 patch naming ?

I tried to go with the most common prefix as reported by git log. I
don't mind changing this, I'll wait for more reviews to see if a v2 is
needed, otherwise this can be updated when applying if desired.

> >   media: platform: rga: Drop unneeded v4l2_m2m_get_vq() NULL check
> >   media: samsung: s5p-g2d: Drop unneeded v4l2_m2m_get_vq() NULL check
> >   media: samsung: s5p-jpeg: Drop unneeded v4l2_m2m_get_vq() NULL check
> >   media: stm32: dma2d: Drop unneeded v4l2_m2m_get_vq() NULL check
> >   media: ti: vpe: Drop unneeded v4l2_m2m_get_vq() NULL check
> >   media: vicodec: Drop unneeded v4l2_m2m_get_vq() NULL check
> >   media: vim2m: Drop unneeded v4l2_m2m_get_vq() NULL check
> 
> I reviewed the list and it seems complete to me.

Thank you.

> >  drivers/media/platform/allegro-dvt/allegro-core.c    |  2 --
> >  drivers/media/platform/amlogic/meson-ge2d/ge2d.c     |  5 -----
> >  drivers/media/platform/amphion/vdec.c                |  2 --
> >  drivers/media/platform/amphion/venc.c                |  2 --
> >  .../media/platform/chips-media/coda/coda-common.c    |  4 ----
> >  drivers/media/platform/imagination/e5010-jpeg-enc.c  |  4 ----
> >  drivers/media/platform/m2m-deinterlace.c             |  7 -------
> >  drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c |  7 -------
> >  .../mediatek/vcodec/decoder/mtk_vcodec_dec.c         |  7 -------
> >  .../vcodec/decoder/vdec/vdec_av1_req_lat_if.c        |  2 --
> >  .../vcodec/decoder/vdec/vdec_vp9_req_lat_if.c        |  2 --
> >  .../mediatek/vcodec/encoder/mtk_vcodec_enc.c         |  8 --------
> >  drivers/media/platform/nxp/dw100/dw100.c             |  7 -------
> >  drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c       |  4 ----
> >  drivers/media/platform/nxp/imx-pxp.c                 |  7 -------
> >  drivers/media/platform/nxp/imx8-isi/imx8-isi-m2m.c   |  2 --
> >  drivers/media/platform/nxp/mx2_emmaprp.c             |  7 -------
> >  drivers/media/platform/qcom/iris/iris_vdec.c         |  2 --
> >  drivers/media/platform/qcom/venus/vdec.c             |  2 --
> >  drivers/media/platform/qcom/venus/venc.c             |  2 --
> >  drivers/media/platform/renesas/rcar_fdp1.c           |  3 ---
> >  drivers/media/platform/renesas/rcar_jpu.c            |  8 --------
> >  drivers/media/platform/rockchip/rga/rga.c            |  4 ----
> >  drivers/media/platform/samsung/s5p-g2d/g2d.c         |  4 ----
> >  drivers/media/platform/samsung/s5p-jpeg/jpeg-core.c  |  7 -------
> >  drivers/media/platform/st/stm32/dma2d/dma2d.c        |  5 -----
> >  drivers/media/platform/ti/vpe/vpe.c                  |  7 -------
> >  drivers/media/test-drivers/vicodec/vicodec-core.c    |  7 -------
> >  drivers/media/test-drivers/vim2m.c                   | 12 ------------
> >  drivers/media/v4l2-core/v4l2-mem2mem.c               | 12 +-----------
> >  include/media/v4l2-mem2mem.h                         |  3 +++
> >  31 files changed, 4 insertions(+), 153 deletions(-)
> > 
> > 
> > base-commit: e5f0a698b34ed76002dc5cff3804a61c80233a7a

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH 15/25] media: qcom: iris: Drop unneeded v4l2_m2m_get_vq() NULL check
  2025-10-08 17:50 ` [PATCH 15/25] media: qcom: iris: Drop unneeded v4l2_m2m_get_vq() NULL check Laurent Pinchart
@ 2025-10-08 23:51   ` Bryan O'Donoghue
  2025-10-09 13:53   ` Dikshita Agarwal
  1 sibling, 0 replies; 10+ messages in thread
From: Bryan O'Donoghue @ 2025-10-08 23:51 UTC (permalink / raw)
  To: Laurent Pinchart, linux-media, linux-arm-msm
  Cc: Nicolas Dufresne, Vikash Garodia, Dikshita Agarwal, Abhinav Kumar

On 08/10/2025 18:50, Laurent Pinchart wrote:
> The v4l2_m2m_get_vq() function never returns NULL. The check may have
> been intended to catch invalid format types, but that's not needed as
> the V4L2 core picks the appropriate VIDIOC_S_FMT ioctl handler based on
> the format type, so the type can't be incorrect. Drop the unneeded
> return value check.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>   drivers/media/platform/qcom/iris/iris_vdec.c | 2 --
>   1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/iris/iris_vdec.c b/drivers/media/platform/qcom/iris/iris_vdec.c
> index d670b51c5839..1e9ffdbb6e18 100644
> --- a/drivers/media/platform/qcom/iris/iris_vdec.c
> +++ b/drivers/media/platform/qcom/iris/iris_vdec.c
> @@ -191,8 +191,6 @@ int iris_vdec_s_fmt(struct iris_inst *inst, struct v4l2_format *f)
>   	u32 codec_align;
>   
>   	q = v4l2_m2m_get_vq(inst->m2m_ctx, f->type);
> -	if (!q)
> -		return -EINVAL;
>   
>   	if (vb2_is_busy(q))
>   		return -EBUSY;
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

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

* Re: [PATCH 16/25] media: qcom: venus: Drop unneeded v4l2_m2m_get_vq() NULL check
  2025-10-08 17:50 ` [PATCH 16/25] media: qcom: venus: " Laurent Pinchart
@ 2025-10-08 23:52   ` Bryan O'Donoghue
  2025-10-09  7:42   ` Dikshita Agarwal
  1 sibling, 0 replies; 10+ messages in thread
From: Bryan O'Donoghue @ 2025-10-08 23:52 UTC (permalink / raw)
  To: Laurent Pinchart, linux-media, linux-arm-msm
  Cc: Nicolas Dufresne, Vikash Garodia, Dikshita Agarwal

On 08/10/2025 18:50, Laurent Pinchart wrote:
> The v4l2_m2m_get_vq() function never returns NULL. The check may have
> been intended to catch invalid format types, but that's not needed as
> the V4L2 core picks the appropriate VIDIOC_S_FMT ioctl handler based on
> the format type, so the type can't be incorrect. Drop the unneeded
> return value check.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>   drivers/media/platform/qcom/venus/vdec.c | 2 --
>   drivers/media/platform/qcom/venus/venc.c | 2 --
>   2 files changed, 4 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
> index 29b0d6a5303d..8c77db0f6e76 100644
> --- a/drivers/media/platform/qcom/venus/vdec.c
> +++ b/drivers/media/platform/qcom/venus/vdec.c
> @@ -329,8 +329,6 @@ static int vdec_s_fmt(struct file *file, void *fh, struct v4l2_format *f)
>   	struct vb2_queue *q;
>   
>   	q = v4l2_m2m_get_vq(inst->m2m_ctx, f->type);
> -	if (!q)
> -		return -EINVAL;
>   
>   	if (vb2_is_busy(q))
>   		return -EBUSY;
> diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
> index c0a0ccdded80..0fe4cc37118b 100644
> --- a/drivers/media/platform/qcom/venus/venc.c
> +++ b/drivers/media/platform/qcom/venus/venc.c
> @@ -241,8 +241,6 @@ static int venc_s_fmt(struct file *file, void *fh, struct v4l2_format *f)
>   	struct vb2_queue *q;
>   
>   	q = v4l2_m2m_get_vq(inst->m2m_ctx, f->type);
> -	if (!q)
> -		return -EINVAL;
>   
>   	if (vb2_is_busy(q))
>   		return -EBUSY;
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

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

* Re: [PATCH 16/25] media: qcom: venus: Drop unneeded v4l2_m2m_get_vq() NULL check
  2025-10-08 17:50 ` [PATCH 16/25] media: qcom: venus: " Laurent Pinchart
  2025-10-08 23:52   ` Bryan O'Donoghue
@ 2025-10-09  7:42   ` Dikshita Agarwal
  1 sibling, 0 replies; 10+ messages in thread
From: Dikshita Agarwal @ 2025-10-09  7:42 UTC (permalink / raw)
  To: Laurent Pinchart, linux-media, linux-arm-msm
  Cc: Nicolas Dufresne, Vikash Garodia, Bryan O'Donoghue



On 10/8/2025 11:20 PM, Laurent Pinchart wrote:
> The v4l2_m2m_get_vq() function never returns NULL. The check may have
> been intended to catch invalid format types, but that's not needed as
> the V4L2 core picks the appropriate VIDIOC_S_FMT ioctl handler based on
> the format type, so the type can't be incorrect. Drop the unneeded
> return value check.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/media/platform/qcom/venus/vdec.c | 2 --
>  drivers/media/platform/qcom/venus/venc.c | 2 --
>  2 files changed, 4 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
> index 29b0d6a5303d..8c77db0f6e76 100644
> --- a/drivers/media/platform/qcom/venus/vdec.c
> +++ b/drivers/media/platform/qcom/venus/vdec.c
> @@ -329,8 +329,6 @@ static int vdec_s_fmt(struct file *file, void *fh, struct v4l2_format *f)
>  	struct vb2_queue *q;
>  
>  	q = v4l2_m2m_get_vq(inst->m2m_ctx, f->type);
> -	if (!q)
> -		return -EINVAL;
>  
>  	if (vb2_is_busy(q))
>  		return -EBUSY;
> diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
> index c0a0ccdded80..0fe4cc37118b 100644
> --- a/drivers/media/platform/qcom/venus/venc.c
> +++ b/drivers/media/platform/qcom/venus/venc.c
> @@ -241,8 +241,6 @@ static int venc_s_fmt(struct file *file, void *fh, struct v4l2_format *f)
>  	struct vb2_queue *q;
>  
>  	q = v4l2_m2m_get_vq(inst->m2m_ctx, f->type);
> -	if (!q)
> -		return -EINVAL;
>  
>  	if (vb2_is_busy(q))
>  		return -EBUSY;

Reviewed-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>

Thanks,
Dikshita

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

* Re: [PATCH 15/25] media: qcom: iris: Drop unneeded v4l2_m2m_get_vq() NULL check
  2025-10-08 17:50 ` [PATCH 15/25] media: qcom: iris: Drop unneeded v4l2_m2m_get_vq() NULL check Laurent Pinchart
  2025-10-08 23:51   ` Bryan O'Donoghue
@ 2025-10-09 13:53   ` Dikshita Agarwal
  2025-10-10 11:14     ` Laurent Pinchart
  1 sibling, 1 reply; 10+ messages in thread
From: Dikshita Agarwal @ 2025-10-09 13:53 UTC (permalink / raw)
  To: Laurent Pinchart, linux-media, linux-arm-msm
  Cc: Nicolas Dufresne, Vikash Garodia, Abhinav Kumar,
	Bryan O'Donoghue



On 10/8/2025 11:20 PM, Laurent Pinchart wrote:
> The v4l2_m2m_get_vq() function never returns NULL. The check may have
> been intended to catch invalid format types, but that's not needed as
> the V4L2 core picks the appropriate VIDIOC_S_FMT ioctl handler based on
> the format type, so the type can't be incorrect. Drop the unneeded
> return value check.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/media/platform/qcom/iris/iris_vdec.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/iris/iris_vdec.c b/drivers/media/platform/qcom/iris/iris_vdec.c
> index d670b51c5839..1e9ffdbb6e18 100644
> --- a/drivers/media/platform/qcom/iris/iris_vdec.c
> +++ b/drivers/media/platform/qcom/iris/iris_vdec.c
> @@ -191,8 +191,6 @@ int iris_vdec_s_fmt(struct iris_inst *inst, struct v4l2_format *f)
>  	u32 codec_align;
>  
>  	q = v4l2_m2m_get_vq(inst->m2m_ctx, f->type);
> -	if (!q)
> -		return -EINVAL;
>  
>  	if (vb2_is_busy(q))
>  		return -EBUSY;

The same change would be required for iris_venc.c as well which is part of
linux-next[1]

[1]:
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/media/platform/qcom/iris/iris_venc.c#n271

Thanks,
Dikshita




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

* Re: [PATCH 15/25] media: qcom: iris: Drop unneeded v4l2_m2m_get_vq() NULL check
  2025-10-09 13:53   ` Dikshita Agarwal
@ 2025-10-10 11:14     ` Laurent Pinchart
  0 siblings, 0 replies; 10+ messages in thread
From: Laurent Pinchart @ 2025-10-10 11:14 UTC (permalink / raw)
  To: Dikshita Agarwal
  Cc: linux-media, linux-arm-msm, Nicolas Dufresne, Vikash Garodia,
	Abhinav Kumar, Bryan O'Donoghue

Hi Dikshita,

On Thu, Oct 09, 2025 at 07:23:20PM +0530, Dikshita Agarwal wrote:
> On 10/8/2025 11:20 PM, Laurent Pinchart wrote:
> > The v4l2_m2m_get_vq() function never returns NULL. The check may have
> > been intended to catch invalid format types, but that's not needed as
> > the V4L2 core picks the appropriate VIDIOC_S_FMT ioctl handler based on
> > the format type, so the type can't be incorrect. Drop the unneeded
> > return value check.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  drivers/media/platform/qcom/iris/iris_vdec.c | 2 --
> >  1 file changed, 2 deletions(-)
> > 
> > diff --git a/drivers/media/platform/qcom/iris/iris_vdec.c b/drivers/media/platform/qcom/iris/iris_vdec.c
> > index d670b51c5839..1e9ffdbb6e18 100644
> > --- a/drivers/media/platform/qcom/iris/iris_vdec.c
> > +++ b/drivers/media/platform/qcom/iris/iris_vdec.c
> > @@ -191,8 +191,6 @@ int iris_vdec_s_fmt(struct iris_inst *inst, struct v4l2_format *f)
> >  	u32 codec_align;
> >  
> >  	q = v4l2_m2m_get_vq(inst->m2m_ctx, f->type);
> > -	if (!q)
> > -		return -EINVAL;
> >  
> >  	if (vb2_is_busy(q))
> >  		return -EBUSY;
> 
> The same change would be required for iris_venc.c as well which is part of
> linux-next[1]

Thank you for noticing. I'll rebase this series on v6.18-rc1 once it
gets released and will make sure iris_venc.c is addressed.

> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/media/platform/qcom/iris/iris_venc.c#n271

-- 
Regards,

Laurent Pinchart

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

end of thread, other threads:[~2025-10-10 11:14 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-08 17:50 [PATCH 00/25] media: v4l2-mem2mem: Reduce cargo-cult Laurent Pinchart
2025-10-08 17:50 ` [PATCH 15/25] media: qcom: iris: Drop unneeded v4l2_m2m_get_vq() NULL check Laurent Pinchart
2025-10-08 23:51   ` Bryan O'Donoghue
2025-10-09 13:53   ` Dikshita Agarwal
2025-10-10 11:14     ` Laurent Pinchart
2025-10-08 17:50 ` [PATCH 16/25] media: qcom: venus: " Laurent Pinchart
2025-10-08 23:52   ` Bryan O'Donoghue
2025-10-09  7:42   ` Dikshita Agarwal
2025-10-08 18:24 ` [PATCH 00/25] media: v4l2-mem2mem: Reduce cargo-cult Nicolas Dufresne
2025-10-08 19:21   ` Laurent Pinchart

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox