From 14b4bd01f8e5d663a41ef8cec5857db874b66e88 Mon Sep 17 00:00:00 2001 From: Benjamin Gaignard Date: Tue, 23 May 2023 17:25:15 +0100 Subject: [PATCH 01/10] media: verisilicon: Additional fix for the crash when opening the driver This fixes the following issue observed on Odroid-M1 board: Unable to handle kernel NULL pointer dereference at virtual address 0000000000000008 Mem abort info: ... Modules linked in: crct10dif_ce hantro_vpu snd_soc_simple_card snd_soc_simple_card_utils v4l2_vp9 v4l2_h264 rockchip_saradc v4l2_mem2mem videobuf2_dma_contig videobuf2_memops rtc_rk808 videobuf2_v4l2 industrialio_triggered_buffer rockchip_thermal dwmac_rk stmmac_platform stmmac videodev kfifo_buf display_connector videobuf2_common pcs_xpcs mc rockchipdrm analogix_dp dw_mipi_dsi dw_hdmi drm_display_helper panfrost drm_shmem_helper gpu_sched ip_tables x_tables ipv6 CPU: 3 PID: 176 Comm: v4l_id Not tainted 6.3.0-rc7-next-20230420 #13481 Hardware name: Hardkernel ODROID-M1 (DT) pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) pc : hantro_try_fmt+0xa0/0x278 [hantro_vpu] lr : hantro_try_fmt+0x94/0x278 [hantro_vpu] ... Call trace: hantro_try_fmt+0xa0/0x278 [hantro_vpu] hantro_set_fmt_out+0x3c/0x298 [hantro_vpu] hantro_reset_raw_fmt+0x98/0x128 [hantro_vpu] hantro_set_fmt_cap+0x240/0x254 [hantro_vpu] hantro_reset_encoded_fmt+0x94/0xcc [hantro_vpu] hantro_reset_fmts+0x18/0x38 [hantro_vpu] hantro_open+0xd4/0x20c [hantro_vpu] v4l2_open+0x80/0x120 [videodev] chrdev_open+0xc0/0x22c do_dentry_open+0x13c/0x48c vfs_open+0x2c/0x38 path_openat+0x550/0x934 do_filp_open+0x80/0x12c do_sys_openat2+0xb4/0x168 __arm64_sys_openat+0x64/0xac invoke_syscall+0x48/0x114 el0_svc_common+0x100/0x120 do_el0_svc+0x3c/0xa8 el0_svc+0x40/0xa8 el0t_64_sync_handler+0xb8/0xbc el0t_64_sync+0x190/0x194 Code: 97fc8a7f f940aa80 52864a61 72a686c1 (b9400800) ---[ end trace 0000000000000000 ]--- Fixes: db6f68b51e5c ("media: verisilicon: Do not set context src/dst formats in reset functions") Signed-off-by: Benjamin Gaignard Tested-by: Michael Tretter Tested-by: Diederik de Haas Tested-by: Marek Szyprowski Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/verisilicon/hantro_v4l2.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c b/drivers/media/platform/verisilicon/hantro_v4l2.c index 835518534e3b9..61cfaaf4e927b 100644 --- a/drivers/media/platform/verisilicon/hantro_v4l2.c +++ b/drivers/media/platform/verisilicon/hantro_v4l2.c @@ -397,10 +397,12 @@ hantro_reset_raw_fmt(struct hantro_ctx *ctx, int bit_depth) if (!raw_vpu_fmt) return -EINVAL; - if (ctx->is_encoder) + if (ctx->is_encoder) { encoded_fmt = &ctx->dst_fmt; - else + ctx->vpu_src_fmt = raw_vpu_fmt; + } else { encoded_fmt = &ctx->src_fmt; + } hantro_reset_fmt(&raw_fmt, raw_vpu_fmt); raw_fmt.width = encoded_fmt->width; From 76743b29f4802afdf81d7ff1228c11e697850235 Mon Sep 17 00:00:00 2001 From: Yassine Oudjana Date: Wed, 3 May 2023 08:53:40 +0100 Subject: [PATCH 02/10] media: camss: camss-video: Don't zero subdev format again after initialization In an earlier commit, setting the which field of the subdev format struct in video_get_subdev_format was moved to a designated initializer that also zeroes all other fields. However, the memset call that was zeroing the fields earlier was left in place, causing the which field to be cleared after being set in the initializer. Remove the memset call from video_get_subdev_format to avoid clearing the initialized which field. Fixes: ecefa105cc44 ("media: Zero-initialize all structures passed to subdev pad operations") Signed-off-by: Yassine Oudjana Acked-by: Bryan O'Donoghue Tested-by: Andrey Konovalov Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/qcom/camss/camss-video.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/media/platform/qcom/camss/camss-video.c b/drivers/media/platform/qcom/camss/camss-video.c index 898f32177b12a..8640db3060268 100644 --- a/drivers/media/platform/qcom/camss/camss-video.c +++ b/drivers/media/platform/qcom/camss/camss-video.c @@ -353,7 +353,6 @@ static int video_get_subdev_format(struct camss_video *video, if (subdev == NULL) return -EPIPE; - memset(&fmt, 0, sizeof(fmt)); fmt.pad = pad; ret = v4l2_subdev_call(subdev, pad, get_fmt, NULL, &fmt); From ed17f89e9502f03af493e130620a9bb74c07cf28 Mon Sep 17 00:00:00 2001 From: Pin-yen Lin Date: Sat, 22 Apr 2023 11:39:05 +0100 Subject: [PATCH 03/10] media: mediatek: vcodec: Only apply 4K frame sizes on decoder formats When VCODEC_CAPABILITY_4K_DISABLED is not set in dec_capability, skip formats that are not MTK_FMT_DEC so only decoder formats is updated in mtk_init_vdec_params. Fixes: e25528e1dbe5 ("media: mediatek: vcodec: Use 4K frame size when supported by stateful decoder") Signed-off-by: Pin-yen Lin Reviewed-by: Chen-Yu Tsai Reviewed-by: Yunfei Dong Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab --- .../media/platform/mediatek/vcodec/mtk_vcodec_dec_stateful.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_stateful.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_stateful.c index 29991551cf614..0fbd030026c72 100644 --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_stateful.c +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_stateful.c @@ -584,6 +584,9 @@ static void mtk_init_vdec_params(struct mtk_vcodec_ctx *ctx) if (!(ctx->dev->dec_capability & VCODEC_CAPABILITY_4K_DISABLED)) { for (i = 0; i < num_supported_formats; i++) { + if (mtk_video_formats[i].type != MTK_FMT_DEC) + continue; + mtk_video_formats[i].frmsize.max_width = VCODEC_DEC_4K_CODED_WIDTH; mtk_video_formats[i].frmsize.max_height = From fe4526d99e2e06b08bb80316c3a596ea6a807b75 Mon Sep 17 00:00:00 2001 From: Hans Verkuil Date: Thu, 20 Apr 2023 08:26:53 +0100 Subject: [PATCH 04/10] media: cec: core: disable adapter in cec_devnode_unregister Explicitly disable the CEC adapter in cec_devnode_unregister() Usually this does not really do anything important, but for drivers that use the CEC pin framework this is needed to properly stop the hrtimer. Without this a crash would happen when such a driver is unloaded with rmmod. Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab --- drivers/media/cec/core/cec-adap.c | 5 ++++- drivers/media/cec/core/cec-core.c | 2 ++ drivers/media/cec/core/cec-priv.h | 1 + 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/media/cec/core/cec-adap.c b/drivers/media/cec/core/cec-adap.c index 769ea6b2e1d0b..be0c389694798 100644 --- a/drivers/media/cec/core/cec-adap.c +++ b/drivers/media/cec/core/cec-adap.c @@ -1585,7 +1585,7 @@ static void cec_claim_log_addrs(struct cec_adapter *adap, bool block) * * This function is called with adap->lock held. */ -static int cec_adap_enable(struct cec_adapter *adap) +int cec_adap_enable(struct cec_adapter *adap) { bool enable; int ret = 0; @@ -1595,6 +1595,9 @@ static int cec_adap_enable(struct cec_adapter *adap) if (adap->needs_hpd) enable = enable && adap->phys_addr != CEC_PHYS_ADDR_INVALID; + if (adap->devnode.unregistered) + enable = false; + if (enable == adap->is_enabled) return 0; diff --git a/drivers/media/cec/core/cec-core.c b/drivers/media/cec/core/cec-core.c index af358e901b5f3..7e153c5cad04f 100644 --- a/drivers/media/cec/core/cec-core.c +++ b/drivers/media/cec/core/cec-core.c @@ -191,6 +191,8 @@ static void cec_devnode_unregister(struct cec_adapter *adap) mutex_lock(&adap->lock); __cec_s_phys_addr(adap, CEC_PHYS_ADDR_INVALID, false); __cec_s_log_addrs(adap, NULL, false); + // Disable the adapter (since adap->devnode.unregistered is true) + cec_adap_enable(adap); mutex_unlock(&adap->lock); cdev_device_del(&devnode->cdev, &devnode->dev); diff --git a/drivers/media/cec/core/cec-priv.h b/drivers/media/cec/core/cec-priv.h index b78df931aa74b..ed1f8c67626bf 100644 --- a/drivers/media/cec/core/cec-priv.h +++ b/drivers/media/cec/core/cec-priv.h @@ -47,6 +47,7 @@ int cec_monitor_pin_cnt_inc(struct cec_adapter *adap); void cec_monitor_pin_cnt_dec(struct cec_adapter *adap); int cec_adap_status(struct seq_file *file, void *priv); int cec_thread_func(void *_adap); +int cec_adap_enable(struct cec_adapter *adap); void __cec_s_phys_addr(struct cec_adapter *adap, u16 phys_addr, bool block); int __cec_s_log_addrs(struct cec_adapter *adap, struct cec_log_addrs *log_addrs, bool block); From 73af6c7511038249cad3d5f3b44bf8d78ac0f499 Mon Sep 17 00:00:00 2001 From: Hans Verkuil Date: Mon, 24 Apr 2023 16:07:28 +0100 Subject: [PATCH 05/10] media: cec: core: don't set last_initiator if tx in progress When a message was received the last_initiator is set to 0xff. This will force the signal free time for the next transmit to that for a new initiator. However, if a new transmit is already in progress, then don't set last_initiator, since that's the initiator of the current transmit. Overwriting this would cause the signal free time of a following transmit to be that of the new initiator instead of a next transmit. Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab --- drivers/media/cec/core/cec-adap.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/media/cec/core/cec-adap.c b/drivers/media/cec/core/cec-adap.c index be0c389694798..241b1621b197c 100644 --- a/drivers/media/cec/core/cec-adap.c +++ b/drivers/media/cec/core/cec-adap.c @@ -1091,7 +1091,8 @@ void cec_received_msg_ts(struct cec_adapter *adap, mutex_lock(&adap->lock); dprintk(2, "%s: %*ph\n", __func__, msg->len, msg->msg); - adap->last_initiator = 0xff; + if (!adap->transmit_in_progress) + adap->last_initiator = 0xff; /* Check if this message was for us (directed or broadcast). */ if (!cec_msg_is_broadcast(msg)) { From a05e9aabd9dc27fc8888678391e3bf78624f8253 Mon Sep 17 00:00:00 2001 From: Hans Verkuil Date: Wed, 19 Apr 2023 08:06:02 +0100 Subject: [PATCH 06/10] media: staging: media: atomisp: init high & low vars Fix a compiler warning: include/linux/dev_printk.h: In function 'ov2680_probe': include/linux/dev_printk.h:144:31: warning: 'high' may be used uninitialized [-Wmaybe-uninitialized] 144 | dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__) | ^~~~~~~~ In function 'ov2680_detect', inlined from 'ov2680_s_config' at drivers/staging/media/atomisp/i2c/atomisp-ov2680.c:468:8, inlined from 'ov2680_probe' at drivers/staging/media/atomisp/i2c/atomisp-ov2680.c:647:8: drivers/staging/media/atomisp/i2c/atomisp-ov2680.c:376:13: note: 'high' was declared here 376 | u32 high, low; | ^~~~ 'high' is indeed uninitialized after the ov_read_reg8() call failed, so there is no point showing the value. Just say that the read failed. But low can also be used uninitialized later, so just make it more robust and properly zero the high and low variables. Signed-off-by: Hans Verkuil Reviewed-by: Hans de Goede Signed-off-by: Mauro Carvalho Chehab --- drivers/staging/media/atomisp/i2c/atomisp-ov2680.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c index 63de214916f59..c079368019e87 100644 --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c @@ -373,7 +373,7 @@ static int ov2680_get_fmt(struct v4l2_subdev *sd, static int ov2680_detect(struct i2c_client *client) { struct i2c_adapter *adapter = client->adapter; - u32 high, low; + u32 high = 0, low = 0; int ret; u16 id; u8 revision; @@ -383,7 +383,7 @@ static int ov2680_detect(struct i2c_client *client) ret = ov_read_reg8(client, OV2680_SC_CMMN_CHIP_ID_H, &high); if (ret) { - dev_err(&client->dev, "sensor_id_high = 0x%x\n", high); + dev_err(&client->dev, "sensor_id_high read failed (%d)\n", ret); return -ENODEV; } ret = ov_read_reg8(client, OV2680_SC_CMMN_CHIP_ID_L, &low); From 3a4cdef13fa389ffe8f74c22581155688c827d18 Mon Sep 17 00:00:00 2001 From: Vaishnav Achath Date: Fri, 21 Apr 2023 11:04:30 +0100 Subject: [PATCH 07/10] media: v4l2-mc: Drop subdev check in v4l2_create_fwnode_links_to_pad() While updating v4l2_create_fwnode_links_to_pad() to accept non-subdev sinks, the check is_media_entity_v4l2_subdev() was not removed which prevented the function from being used with non-subdev sinks, Drop the unnecessary check. Fixes: bd5a03bc5be8 ("media: Accept non-subdev sinks in v4l2_create_fwnode_links_to_pad()") Signed-off-by: Vaishnav Achath Reviewed-by: Laurent Pinchart Signed-off-by: Sakari Ailus Signed-off-by: Mauro Carvalho Chehab --- drivers/media/v4l2-core/v4l2-mc.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-mc.c b/drivers/media/v4l2-core/v4l2-mc.c index bf0c18100664f..22fe08fce0a9a 100644 --- a/drivers/media/v4l2-core/v4l2-mc.c +++ b/drivers/media/v4l2-core/v4l2-mc.c @@ -314,8 +314,7 @@ int v4l2_create_fwnode_links_to_pad(struct v4l2_subdev *src_sd, { struct fwnode_handle *endpoint; - if (!(sink->flags & MEDIA_PAD_FL_SINK) || - !is_media_entity_v4l2_subdev(sink->entity)) + if (!(sink->flags & MEDIA_PAD_FL_SINK)) return -EINVAL; fwnode_graph_for_each_endpoint(dev_fwnode(src_sd->dev), endpoint) { From 6970888d38be0a960673e3d203e8517a5c300ae5 Mon Sep 17 00:00:00 2001 From: Hans Verkuil Date: Tue, 18 Apr 2023 08:46:52 +0100 Subject: [PATCH 08/10] media: staging: media: imx: initialize hs_settle to avoid warning Initialize hs_settle to 0 to avoid this compiler warning: imx8mq-mipi-csi2.c: In function 'imx8mq_mipi_csi_start_stream.part.0': imx8mq-mipi-csi2.c:91:55: warning: 'hs_settle' may be used uninitialized [-Wmaybe-uninitialized] 91 | #define GPR_CSI2_1_S_PRG_RXHS_SETTLE(x) (((x) & 0x3f) << 2) | ^~ imx8mq-mipi-csi2.c:357:13: note: 'hs_settle' was declared here 357 | u32 hs_settle; | ^~~~~~~~~ It's a false positive, but it is too complicated for the compiler to detect that. Signed-off-by: Hans Verkuil Reviewed-by: Martin Kepplinger Signed-off-by: Sakari Ailus Signed-off-by: Mauro Carvalho Chehab --- drivers/staging/media/imx/imx8mq-mipi-csi2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/media/imx/imx8mq-mipi-csi2.c b/drivers/staging/media/imx/imx8mq-mipi-csi2.c index 32700cb8bc4d5..ca2efcc21efe3 100644 --- a/drivers/staging/media/imx/imx8mq-mipi-csi2.c +++ b/drivers/staging/media/imx/imx8mq-mipi-csi2.c @@ -354,7 +354,7 @@ static int imx8mq_mipi_csi_start_stream(struct csi_state *state, struct v4l2_subdev_state *sd_state) { int ret; - u32 hs_settle; + u32 hs_settle = 0; ret = imx8mq_mipi_csi_sw_reset(state); if (ret) From b37a356df86b9e56d30cef4673cba2621c7b7a1e Mon Sep 17 00:00:00 2001 From: Tomi Valkeinen Date: Mon, 22 May 2023 11:52:45 +0100 Subject: [PATCH 09/10] media: v4l2-subdev: Fix missing kerneldoc for client_caps Add missing kernel doc for the new 'client_caps' field in struct v4l2_subdev_fh. Signed-off-by: Tomi Valkeinen Reviewed-by: Laurent Pinchart Fixes: f57fa2959244 ("media: v4l2-subdev: Add new ioctl for client capabilities") Signed-off-by: Sakari Ailus Signed-off-by: Mauro Carvalho Chehab --- include/media/v4l2-subdev.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index cfd19e72d0fc4..b325df0d54d61 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -1119,6 +1119,7 @@ struct v4l2_subdev { * @vfh: pointer to &struct v4l2_fh * @state: pointer to &struct v4l2_subdev_state * @owner: module pointer to the owner of this file handle + * @client_caps: bitmask of ``V4L2_SUBDEV_CLIENT_CAP_*`` */ struct v4l2_subdev_fh { struct v4l2_fh vfh; From 81f3affa19d6ab0c32aef46b053838219eef7e71 Mon Sep 17 00:00:00 2001 From: Laurent Pinchart Date: Thu, 20 Apr 2023 10:45:59 +0100 Subject: [PATCH 10/10] media: uvcvideo: Don't expose unsupported formats to userspace When the uvcvideo driver encounters a format descriptor with an unknown format GUID, it creates a corresponding struct uvc_format instance with the fcc field set to 0. Since commit 50459f103edf ("media: uvcvideo: Remove format descriptions"), the driver relies on the V4L2 core to provide the format description string, which the V4L2 core can't do without a valid 4CC. This triggers a WARN_ON. As a format with a zero 4CC can't be selected, it is unusable for applications. Ignore the format completely without creating a uvc_format instance, which fixes the warning. Link: https://bugzilla.kernel.org/show_bug.cgi?id=217252 Link: https://bugzilla.redhat.com/show_bug.cgi?id=2180107 Fixes: 50459f103edf ("media: uvcvideo: Remove format descriptions") Signed-off-by: Laurent Pinchart Reviewed-by: Ricardo Ribalda Signed-off-by: Mauro Carvalho Chehab --- drivers/media/usb/uvc/uvc_driver.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c index 7aefa76a42b31..d631ce4f9f7bb 100644 --- a/drivers/media/usb/uvc/uvc_driver.c +++ b/drivers/media/usb/uvc/uvc_driver.c @@ -251,14 +251,17 @@ static int uvc_parse_format(struct uvc_device *dev, /* Find the format descriptor from its GUID. */ fmtdesc = uvc_format_by_guid(&buffer[5]); - if (fmtdesc != NULL) { - format->fcc = fmtdesc->fcc; - } else { + if (!fmtdesc) { + /* + * Unknown video formats are not fatal errors, the + * caller will skip this descriptor. + */ dev_info(&streaming->intf->dev, "Unknown video format %pUl\n", &buffer[5]); - format->fcc = 0; + return 0; } + format->fcc = fmtdesc->fcc; format->bpp = buffer[21]; /* @@ -675,7 +678,7 @@ static int uvc_parse_streaming(struct uvc_device *dev, interval = (u32 *)&frame[nframes]; streaming->format = format; - streaming->nformats = nformats; + streaming->nformats = 0; /* Parse the format descriptors. */ while (buflen > 2 && buffer[1] == USB_DT_CS_INTERFACE) { @@ -689,7 +692,10 @@ static int uvc_parse_streaming(struct uvc_device *dev, &interval, buffer, buflen); if (ret < 0) goto error; + if (!ret) + break; + streaming->nformats++; frame += format->nframes; format++;