From 42d62b7e47d58273c64fc1540e5d81ccfdb60f77 Mon Sep 17 00:00:00 2001 From: Laurent Pinchart Date: Tue, 24 Oct 2023 17:18:17 +0300 Subject: [PATCH 1/4] media: vsp1: Remove unbalanced .s_stream(0) calls The VSP1 driver uses the subdev .s_stream() operation to stop WPF instances, without a corresponding call to start them. The V4L2 subdev core started warning about unbalanced .s_stream() calls in commit 009905ec5043 ("media: v4l2-subdev: Document and enforce .s_stream() requirements"), causing a regression with this driver. Fix the problem by replacing the .s_stream() operation with an explicit function call for WPF instances. This allows sharing an additional data structure between RPF and WPF instances. Fixes: 009905ec5043 ("media: v4l2-subdev: Document and enforce .s_stream() requirements") Reported-by: Geert Uytterhoeven Closes: https://lore.kernel.org/linux-media/2221395-6a9b-9527-d697-e76aebc6af@linux-m68k.org/ Signed-off-by: Laurent Pinchart Tested-by: Geert Uytterhoeven --- .../media/platform/renesas/vsp1/vsp1_pipe.c | 2 +- .../media/platform/renesas/vsp1/vsp1_rpf.c | 10 +------ .../media/platform/renesas/vsp1/vsp1_rwpf.c | 8 +++-- .../media/platform/renesas/vsp1/vsp1_rwpf.h | 4 ++- .../media/platform/renesas/vsp1/vsp1_wpf.c | 29 ++----------------- 5 files changed, 14 insertions(+), 39 deletions(-) diff --git a/drivers/media/platform/renesas/vsp1/vsp1_pipe.c b/drivers/media/platform/renesas/vsp1/vsp1_pipe.c index f8093ba9539e..68d05243c3ee 100644 --- a/drivers/media/platform/renesas/vsp1/vsp1_pipe.c +++ b/drivers/media/platform/renesas/vsp1/vsp1_pipe.c @@ -373,7 +373,7 @@ int vsp1_pipeline_stop(struct vsp1_pipeline *pipe) (7 << VI6_DPR_SMPPT_TGW_SHIFT) | (VI6_DPR_NODE_UNUSED << VI6_DPR_SMPPT_PT_SHIFT)); - v4l2_subdev_call(&pipe->output->entity.subdev, video, s_stream, 0); + vsp1_wpf_stop(pipe->output); return ret; } diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c index 3b17f5fa4067..ea12c3f12c92 100644 --- a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c +++ b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c @@ -43,14 +43,6 @@ static inline void vsp1_rpf_write(struct vsp1_rwpf *rpf, data); } -/* ----------------------------------------------------------------------------- - * V4L2 Subdevice Operations - */ - -static const struct v4l2_subdev_ops rpf_ops = { - .pad = &vsp1_rwpf_pad_ops, -}; - /* ----------------------------------------------------------------------------- * VSP1 Entity Operations */ @@ -411,7 +403,7 @@ struct vsp1_rwpf *vsp1_rpf_create(struct vsp1_device *vsp1, unsigned int index) rpf->entity.index = index; sprintf(name, "rpf.%u", index); - ret = vsp1_entity_init(vsp1, &rpf->entity, name, 2, &rpf_ops, + ret = vsp1_entity_init(vsp1, &rpf->entity, name, 2, &vsp1_rwpf_subdev_ops, MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER); if (ret < 0) return ERR_PTR(ret); diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c index 22a82d218152..e0f87c8103ca 100644 --- a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c +++ b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.c @@ -24,7 +24,7 @@ struct v4l2_rect *vsp1_rwpf_get_crop(struct vsp1_rwpf *rwpf, } /* ----------------------------------------------------------------------------- - * V4L2 Subdevice Pad Operations + * V4L2 Subdevice Operations */ static int vsp1_rwpf_enum_mbus_code(struct v4l2_subdev *subdev, @@ -243,7 +243,7 @@ done: return ret; } -const struct v4l2_subdev_pad_ops vsp1_rwpf_pad_ops = { +static const struct v4l2_subdev_pad_ops vsp1_rwpf_pad_ops = { .init_cfg = vsp1_entity_init_cfg, .enum_mbus_code = vsp1_rwpf_enum_mbus_code, .enum_frame_size = vsp1_rwpf_enum_frame_size, @@ -253,6 +253,10 @@ const struct v4l2_subdev_pad_ops vsp1_rwpf_pad_ops = { .set_selection = vsp1_rwpf_set_selection, }; +const struct v4l2_subdev_ops vsp1_rwpf_subdev_ops = { + .pad = &vsp1_rwpf_pad_ops, +}; + /* ----------------------------------------------------------------------------- * Controls */ diff --git a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.h b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.h index eac5c04c2239..e0d212c70b2f 100644 --- a/drivers/media/platform/renesas/vsp1/vsp1_rwpf.h +++ b/drivers/media/platform/renesas/vsp1/vsp1_rwpf.h @@ -79,9 +79,11 @@ static inline struct vsp1_rwpf *entity_to_rwpf(struct vsp1_entity *entity) struct vsp1_rwpf *vsp1_rpf_create(struct vsp1_device *vsp1, unsigned int index); struct vsp1_rwpf *vsp1_wpf_create(struct vsp1_device *vsp1, unsigned int index); +void vsp1_wpf_stop(struct vsp1_rwpf *wpf); + int vsp1_rwpf_init_ctrls(struct vsp1_rwpf *rwpf, unsigned int ncontrols); -extern const struct v4l2_subdev_pad_ops vsp1_rwpf_pad_ops; +extern const struct v4l2_subdev_ops vsp1_rwpf_subdev_ops; struct v4l2_rect *vsp1_rwpf_get_crop(struct vsp1_rwpf *rwpf, struct v4l2_subdev_state *sd_state); diff --git a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c index d0074ca00920..cab4445eca69 100644 --- a/drivers/media/platform/renesas/vsp1/vsp1_wpf.c +++ b/drivers/media/platform/renesas/vsp1/vsp1_wpf.c @@ -186,17 +186,13 @@ static int wpf_init_controls(struct vsp1_rwpf *wpf) } /* ----------------------------------------------------------------------------- - * V4L2 Subdevice Core Operations + * VSP1 Entity Operations */ -static int wpf_s_stream(struct v4l2_subdev *subdev, int enable) +void vsp1_wpf_stop(struct vsp1_rwpf *wpf) { - struct vsp1_rwpf *wpf = to_rwpf(subdev); struct vsp1_device *vsp1 = wpf->entity.vsp1; - if (enable) - return 0; - /* * Write to registers directly when stopping the stream as there will be * no pipeline run to apply the display list. @@ -204,27 +200,8 @@ static int wpf_s_stream(struct v4l2_subdev *subdev, int enable) vsp1_write(vsp1, VI6_WPF_IRQ_ENB(wpf->entity.index), 0); vsp1_write(vsp1, wpf->entity.index * VI6_WPF_OFFSET + VI6_WPF_SRCRPF, 0); - - return 0; } -/* ----------------------------------------------------------------------------- - * V4L2 Subdevice Operations - */ - -static const struct v4l2_subdev_video_ops wpf_video_ops = { - .s_stream = wpf_s_stream, -}; - -static const struct v4l2_subdev_ops wpf_ops = { - .video = &wpf_video_ops, - .pad = &vsp1_rwpf_pad_ops, -}; - -/* ----------------------------------------------------------------------------- - * VSP1 Entity Operations - */ - static void vsp1_wpf_destroy(struct vsp1_entity *entity) { struct vsp1_rwpf *wpf = entity_to_rwpf(entity); @@ -583,7 +560,7 @@ struct vsp1_rwpf *vsp1_wpf_create(struct vsp1_device *vsp1, unsigned int index) wpf->entity.index = index; sprintf(name, "wpf.%u", index); - ret = vsp1_entity_init(vsp1, &wpf->entity, name, 2, &wpf_ops, + ret = vsp1_entity_init(vsp1, &wpf->entity, name, 2, &vsp1_rwpf_subdev_ops, MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER); if (ret < 0) return ERR_PTR(ret); From 9ea7be95d3d4d76df63cef13b4360cd58548b2ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20T=C5=AFma?= Date: Wed, 25 Oct 2023 18:51:47 +0200 Subject: [PATCH 2/4] media: mgb4: Added support for T200 card variant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit T200 card variants use the XC7A200T FPGA instead of XC7A100T. The SPI FLASH memory layout is different as the FPGA requires bigger FW images. Signed-off-by: Martin Tůma Signed-off-by: Hans Verkuil --- drivers/media/pci/mgb4/mgb4_core.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/drivers/media/pci/mgb4/mgb4_core.c b/drivers/media/pci/mgb4/mgb4_core.c index 3efb33fbf40c..5bfb8a06202e 100644 --- a/drivers/media/pci/mgb4/mgb4_core.c +++ b/drivers/media/pci/mgb4/mgb4_core.c @@ -42,6 +42,10 @@ #define MGB4_USER_IRQS 16 +#define DIGITEQ_VID 0x1ed8 +#define T100_DID 0x0101 +#define T200_DID 0x0201 + ATTRIBUTE_GROUPS(mgb4_pci); static int flashid; @@ -151,7 +155,7 @@ static struct spi_master *get_spi_adap(struct platform_device *pdev) return dev ? container_of(dev, struct spi_master, dev) : NULL; } -static int init_spi(struct mgb4_dev *mgbdev) +static int init_spi(struct mgb4_dev *mgbdev, u32 devid) { struct resource spi_resources[] = { { @@ -213,8 +217,13 @@ static int init_spi(struct mgb4_dev *mgbdev) snprintf(mgbdev->fw_part_name, sizeof(mgbdev->fw_part_name), "mgb4-fw.%d", flashid); mgbdev->partitions[0].name = mgbdev->fw_part_name; - mgbdev->partitions[0].size = 0x400000; - mgbdev->partitions[0].offset = 0x400000; + if (devid == T200_DID) { + mgbdev->partitions[0].size = 0x950000; + mgbdev->partitions[0].offset = 0x1000000; + } else { + mgbdev->partitions[0].size = 0x400000; + mgbdev->partitions[0].offset = 0x400000; + } mgbdev->partitions[0].mask_flags = 0; snprintf(mgbdev->data_part_name, sizeof(mgbdev->data_part_name), @@ -551,7 +560,7 @@ static int mgb4_probe(struct pci_dev *pdev, const struct pci_device_id *id) goto err_video_regs; /* SPI FLASH */ - rv = init_spi(mgbdev); + rv = init_spi(mgbdev, id->device); if (rv < 0) goto err_cmt_regs; @@ -666,7 +675,8 @@ static void mgb4_remove(struct pci_dev *pdev) } static const struct pci_device_id mgb4_pci_ids[] = { - { PCI_DEVICE(0x1ed8, 0x0101), }, + { PCI_DEVICE(DIGITEQ_VID, T100_DID), }, + { PCI_DEVICE(DIGITEQ_VID, T200_DID), }, { 0, } }; MODULE_DEVICE_TABLE(pci, mgb4_pci_ids); From 5d33213fac5929a2e7766c88d78779fd443b0fe8 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Fri, 3 Nov 2023 10:39:24 +0300 Subject: [PATCH 3/4] media: v4l2-subdev: Fix a 64bit bug The problem is this line here from subdev_do_ioctl(). client_cap->capabilities &= ~V4L2_SUBDEV_CLIENT_CAP_STREAMS; The "client_cap->capabilities" variable is a u64. The AND operation is supposed to clear out the V4L2_SUBDEV_CLIENT_CAP_STREAMS flag. But because it's a 32 bit variable it accidentally clears out the high 32 bits as well. Currently we only use the first bit and none of the upper bits so this doesn't affect runtime behavior. Fixes: f57fa2959244 ("media: v4l2-subdev: Add new ioctl for client capabilities") Signed-off-by: Dan Carpenter Reviewed-by: Tomi Valkeinen Signed-off-by: Hans Verkuil --- include/uapi/linux/v4l2-subdev.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h index 4a195b68f28f..b383c2fe0cf3 100644 --- a/include/uapi/linux/v4l2-subdev.h +++ b/include/uapi/linux/v4l2-subdev.h @@ -239,7 +239,7 @@ struct v4l2_subdev_routing { * set (which is the default), the 'stream' fields will be forced to 0 by the * kernel. */ - #define V4L2_SUBDEV_CLIENT_CAP_STREAMS (1U << 0) + #define V4L2_SUBDEV_CLIENT_CAP_STREAMS (1ULL << 0) /** * struct v4l2_subdev_client_capability - Capabilities of the client accessing From 173d167ce350961d7d689a4d59d66a5f2ad70a76 Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Mon, 23 Oct 2023 18:05:31 +0200 Subject: [PATCH 4/4] media: pci: mgb4: add COMMON_CLK dependency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This driver fails to build when HAVE_CLK and COMMON_CLK are disabled: x86_64-linux-ld: vmlinux.o: in function `mgb4_remove': mgb4_core.c:(.text+0x1915e8c): undefined reference to `clkdev_drop' x86_64-linux-ld: mgb4_core.c:(.text+0x1915e98): undefined reference to `clk_hw_unregister' Add a Kconfig dependency to enforce a clean build. Fixes: 0ab13674a9bd ("media: pci: mgb4: Added Digiteq Automotive MGB4 driver") Signed-off-by: Arnd Bergmann Reviewed-by: Martin Tůma Signed-off-by: Hans Verkuil --- drivers/media/pci/mgb4/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/media/pci/mgb4/Kconfig b/drivers/media/pci/mgb4/Kconfig index 13fad15a434c..f2a05a1c8ffa 100644 --- a/drivers/media/pci/mgb4/Kconfig +++ b/drivers/media/pci/mgb4/Kconfig @@ -2,6 +2,7 @@ config VIDEO_MGB4 tristate "Digiteq Automotive MGB4 support" depends on VIDEO_DEV && PCI && I2C && DMADEVICES && SPI && MTD && IIO + depends on COMMON_CLK select VIDEOBUF2_DMA_SG select IIO_BUFFER select IIO_TRIGGERED_BUFFER