LinuxLists.cc - [PATCH v3 00/14] staging: media: zoran: fusion in one module

2021-10-27 09:50:35

Subject: [PATCH v3 00/14] staging: media: zoran: fusion in one module

Hello

The main change of this serie is to fusion all zoran related modules in
one.
This fixes the load order problem when everything is built-in.

Regards

Changes since v2:
- added the 4 latest patchs
- removed DEBUGFS kconfig option
- fixed Dan Carpenter's reported codec issues
- fixed kernel test robot's reported issues on vb2_dma_contig_set_max_seg_size()

Changes since v1:
- add missing debugfs cleaning
- clean some remaining module_get/put functions which made impossible to
remove the zoran module
- added the two latest patchs

Corentin Labbe (14):
staging: media: zoran: move module parameter checks to zoran_probe
staging: media: zoran: use module_pci_driver
staging: media: zoran: rename debug module parameter
staging: media: zoran: add debugfs
staging: media: zoran: videocode: remove procfs
staging: media: zoran: fusion all modules
staging: media: zoran: remove vidmem
staging: media: zoran: move videodev alloc
staging: media: zoran: move config select on primary kconfig
staging: media: zoran: introduce zoran_i2c_init
staging: media: zoran: fix usage of vb2_dma_contig_set_max_seg_size
staging: media: zoran: clean unused code
staging: media: zoran: fix counting buffer in reserve
staging: media: zoran: DC30 encoder is not adv7175

drivers/staging/media/zoran/Kconfig | 38 +-
drivers/staging/media/zoran/Makefile | 8 +-
drivers/staging/media/zoran/videocodec.c | 68 +---
drivers/staging/media/zoran/videocodec.h | 4 +-
drivers/staging/media/zoran/zoran.h | 18 +-
drivers/staging/media/zoran/zoran_card.c | 400 +++++++++++++--------
drivers/staging/media/zoran/zoran_device.h | 2 -
drivers/staging/media/zoran/zoran_driver.c | 8 +-
drivers/staging/media/zoran/zr36016.c | 25 +-
drivers/staging/media/zoran/zr36016.h | 2 +
drivers/staging/media/zoran/zr36050.c | 24 +-
drivers/staging/media/zoran/zr36050.h | 2 +
drivers/staging/media/zoran/zr36060.c | 23 +-
drivers/staging/media/zoran/zr36060.h | 2 +
14 files changed, 317 insertions(+), 307 deletions(-)

--
2.32.0

2021-10-27 09:50:35

by Corentin LABBE

[permalink] [raw]

Subject: [PATCH v3 08/14] staging: media: zoran: move videodev alloc

Move some code out of zr36057_init() and create new functions for handling zr->video_dev.
This permit to ease code reading and fix a zr->video_dev memory leak.

Signed-off-by: Corentin Labbe <[email protected]>
---
drivers/staging/media/zoran/zoran.h | 2 +-
drivers/staging/media/zoran/zoran_card.c | 80 ++++++++++++++--------
drivers/staging/media/zoran/zoran_driver.c | 5 +-
3 files changed, 54 insertions(+), 33 deletions(-)

diff --git a/drivers/staging/media/zoran/zoran.h b/drivers/staging/media/zoran/zoran.h
index 981cb63ac9af..c36b33f42b16 100644
--- a/drivers/staging/media/zoran/zoran.h
+++ b/drivers/staging/media/zoran/zoran.h
@@ -315,6 +315,6 @@ static inline struct zoran *to_zoran(struct v4l2_device *v4l2_dev)

#endif

-int zoran_queue_init(struct zoran *zr, struct vb2_queue *vq);
+int zoran_queue_init(struct zoran *zr, struct vb2_queue *vq, int dir);
void zoran_queue_exit(struct zoran *zr);
int zr_set_buf(struct zoran *zr);
diff --git a/drivers/staging/media/zoran/zoran_card.c b/drivers/staging/media/zoran/zoran_card.c
index 9cd49f85a56e..19eb3150074a 100644
--- a/drivers/staging/media/zoran/zoran_card.c
+++ b/drivers/staging/media/zoran/zoran_card.c
@@ -885,6 +885,52 @@ int zoran_check_jpg_settings(struct zoran *zr,
return 0;
}

+static int zoran_init_video_device(struct zoran *zr, struct video_device *video_dev, int dir)
+{
+ int err;
+
+ /* Now add the template and register the device unit. */
+ *video_dev = zoran_template;
+ video_dev->v4l2_dev = &zr->v4l2_dev;
+ video_dev->lock = &zr->lock;
+ video_dev->device_caps = V4L2_CAP_STREAMING | dir;
+
+ strscpy(video_dev->name, ZR_DEVNAME(zr), sizeof(video_dev->name));
+ /*
+ * It's not a mem2mem device, but you can both capture and output from one and the same
+ * device. This should really be split up into two device nodes, but that's a job for
+ * another day.
+ */
+ video_dev->vfl_dir = VFL_DIR_M2M;
+ zoran_queue_init(zr, &zr->vq, V4L2_BUF_TYPE_VIDEO_CAPTURE);
+
+ err = video_register_device(video_dev, VFL_TYPE_VIDEO, video_nr[zr->id]);
+ if (err < 0)
+ return err;
+ video_set_drvdata(video_dev, zr);
+ return 0;
+}
+
+static void zoran_exit_video_devices(struct zoran *zr)
+{
+ video_unregister_device(zr->video_dev);
+ kfree(zr->video_dev);
+}
+
+static int zoran_init_video_devices(struct zoran *zr)
+{
+ int err;
+
+ zr->video_dev = video_device_alloc();
+ if (!zr->video_dev)
+ return -ENOMEM;
+
+ err = zoran_init_video_device(zr, zr->video_dev, V4L2_CAP_VIDEO_CAPTURE);
+ if (err)
+ kfree(zr->video_dev);
+ return err;
+}
+
void zoran_open_init_params(struct zoran *zr)
{
int i;
@@ -956,17 +1002,11 @@ static int zr36057_init(struct zoran *zr)
zoran_open_init_params(zr);

/* allocate memory *before* doing anything to the hardware in case allocation fails */
- zr->video_dev = video_device_alloc();
- if (!zr->video_dev) {
- err = -ENOMEM;
- goto exit;
- }
zr->stat_com = dma_alloc_coherent(&zr->pci_dev->dev,
BUZ_NUM_STAT_COM * sizeof(u32),
&zr->p_sc, GFP_KERNEL);
if (!zr->stat_com) {
- err = -ENOMEM;
- goto exit_video;
+ return -ENOMEM;
}
for (j = 0; j < BUZ_NUM_STAT_COM; j++)
zr->stat_com[j] = cpu_to_le32(1); /* mark as unavailable to zr36057 */
@@ -979,26 +1019,9 @@ static int zr36057_init(struct zoran *zr)
goto exit_statcom;
}

- /* Now add the template and register the device unit. */
- *zr->video_dev = zoran_template;
- zr->video_dev->v4l2_dev = &zr->v4l2_dev;
- zr->video_dev->lock = &zr->lock;
- zr->video_dev->device_caps = V4L2_CAP_STREAMING | V4L2_CAP_VIDEO_CAPTURE;
-
- strscpy(zr->video_dev->name, ZR_DEVNAME(zr), sizeof(zr->video_dev->name));
- /*
- * It's not a mem2mem device, but you can both capture and output from one and the same
- * device. This should really be split up into two device nodes, but that's a job for
- * another day.
- */
- zr->video_dev->vfl_dir = VFL_DIR_M2M;
-
- zoran_queue_init(zr, &zr->vq);
-
- err = video_register_device(zr->video_dev, VFL_TYPE_VIDEO, video_nr[zr->id]);
- if (err < 0)
+ err = zoran_init_video_devices(zr);
+ if (err)
goto exit_statcomb;
- video_set_drvdata(zr->video_dev, zr);

zoran_init_hardware(zr);
if (!pass_through) {
@@ -1013,9 +1036,6 @@ static int zr36057_init(struct zoran *zr)
dma_free_coherent(&zr->pci_dev->dev, BUZ_NUM_STAT_COM * sizeof(u32) * 2, zr->stat_comb, zr->p_scb);
exit_statcom:
dma_free_coherent(&zr->pci_dev->dev, BUZ_NUM_STAT_COM * sizeof(u32), zr->stat_com, zr->p_sc);
-exit_video:
- kfree(zr->video_dev);
-exit:
return err;
}

@@ -1050,7 +1070,7 @@ static void zoran_remove(struct pci_dev *pdev)
dma_free_coherent(&zr->pci_dev->dev, BUZ_NUM_STAT_COM * sizeof(u32) * 2, zr->stat_comb, zr->p_scb);
pci_release_regions(pdev);
pci_disable_device(zr->pci_dev);
- video_unregister_device(zr->video_dev);
+ zoran_exit_video_devices(zr);
exit_free:
v4l2_ctrl_handler_free(&zr->hdl);
v4l2_device_unregister(&zr->v4l2_dev);
diff --git a/drivers/staging/media/zoran/zoran_driver.c b/drivers/staging/media/zoran/zoran_driver.c
index 46382e43f1bf..551db338c7f7 100644
--- a/drivers/staging/media/zoran/zoran_driver.c
+++ b/drivers/staging/media/zoran/zoran_driver.c
@@ -1008,7 +1008,7 @@ static const struct vb2_ops zr_video_qops = {
.wait_finish = vb2_ops_wait_finish,
};

-int zoran_queue_init(struct zoran *zr, struct vb2_queue *vq)
+int zoran_queue_init(struct zoran *zr, struct vb2_queue *vq, int dir)
{
int err;

@@ -1016,7 +1016,8 @@ int zoran_queue_init(struct zoran *zr, struct vb2_queue *vq)
INIT_LIST_HEAD(&zr->queued_bufs);

vq->dev = &zr->pci_dev->dev;
- vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+ vq->type = dir;
+
vq->io_modes = VB2_USERPTR | VB2_DMABUF | VB2_MMAP | VB2_READ | VB2_WRITE;
vq->drv_priv = zr;
vq->buf_struct_size = sizeof(struct zr_buffer);
--
2.32.0

2021-10-27 09:50:37

by Corentin LABBE

[permalink] [raw]

Subject: [PATCH v3 05/14] staging: media: zoran: videocode: remove procfs

Now we have a debugfs, we can remove all PROCFS stuff.
We keep videocodec_debugfs_show(), it will be used later

Signed-off-by: Corentin Labbe <[email protected]>
---
drivers/staging/media/zoran/videocodec.c | 24 +-----------------------
drivers/staging/media/zoran/videocodec.h | 3 +++
2 files changed, 4 insertions(+), 23 deletions(-)

diff --git a/drivers/staging/media/zoran/videocodec.c b/drivers/staging/media/zoran/videocodec.c
index 31019b5f377e..5bab7ba56257 100644
--- a/drivers/staging/media/zoran/videocodec.c
+++ b/drivers/staging/media/zoran/videocodec.c
@@ -16,14 +16,6 @@
#include <linux/types.h>
#include <linux/slab.h>

-// kernel config is here (procfs flag)
-
-#ifdef CONFIG_PROC_FS
-#include <linux/proc_fs.h>
-#include <linux/seq_file.h>
-#include <linux/uaccess.h>
-#endif
-
#include "videocodec.h"

static int videocodec_debug;
@@ -265,8 +257,7 @@ int videocodec_unregister(const struct videocodec *codec)
}
EXPORT_SYMBOL(videocodec_unregister);

-#ifdef CONFIG_PROC_FS
-static int proc_videocodecs_show(struct seq_file *m, void *v)
+int videocodec_debugfs_show(struct seq_file *m)
{
struct codec_list *h = codeclist_top;
struct attached_list *a;
@@ -293,32 +284,19 @@ static int proc_videocodecs_show(struct seq_file *m, void *v)

return 0;
}
-#endif

/* ===================== */
/* hook in driver module */
/* ===================== */
static int __init videocodec_init(void)
{
-#ifdef CONFIG_PROC_FS
- static struct proc_dir_entry *videocodec_proc_entry;
-#endif
-
pr_info("Linux video codec intermediate layer: %s\n", VIDEOCODEC_VERSION);

-#ifdef CONFIG_PROC_FS
- videocodec_proc_entry = proc_create_single("videocodecs", 0, NULL, proc_videocodecs_show);
- if (!videocodec_proc_entry)
- pr_err("videocodec: can't init procfs.\n");
-#endif
return 0;
}

static void __exit videocodec_exit(void)
{
-#ifdef CONFIG_PROC_FS
- remove_proc_entry("videocodecs", NULL);
-#endif
}

module_init(videocodec_init);
diff --git a/drivers/staging/media/zoran/videocodec.h b/drivers/staging/media/zoran/videocodec.h
index 8a5003dda9f4..3a508d326049 100644
--- a/drivers/staging/media/zoran/videocodec.h
+++ b/drivers/staging/media/zoran/videocodec.h
@@ -123,6 +123,7 @@ M zr36055[1] 0001 0000c001 00000000 (zr36050[1])
#ifndef __LINUX_VIDEOCODEC_H
#define __LINUX_VIDEOCODEC_H

+#include <linux/debugfs.h>
#include <linux/videodev2.h>

#define CODEC_DO_COMPRESSION 0
@@ -305,4 +306,6 @@ extern int videocodec_unregister(const struct videocodec *);

/* the other calls are directly done via the videocodec structure! */

+int videocodec_debugfs_show(struct seq_file *m);
+
#endif /*ifndef __LINUX_VIDEOCODEC_H */
--
2.32.0

2021-10-27 09:50:37

by Corentin LABBE

[permalink] [raw]

Subject: [PATCH v3 07/14] staging: media: zoran: remove vidmem

The vidmem parameter is no longer necessary since we removed framebuffer
support.

Signed-off-by: Corentin Labbe <[email protected]>
---
drivers/staging/media/zoran/zoran_card.c | 15 ---------------
1 file changed, 15 deletions(-)

diff --git a/drivers/staging/media/zoran/zoran_card.c b/drivers/staging/media/zoran/zoran_card.c
index a7750442ef9e..9cd49f85a56e 100644
--- a/drivers/staging/media/zoran/zoran_card.c
+++ b/drivers/staging/media/zoran/zoran_card.c
@@ -39,17 +39,6 @@ static int card[BUZ_MAX] = { [0 ... (BUZ_MAX - 1)] = -1 };
module_param_array(card, int, NULL, 0444);
MODULE_PARM_DESC(card, "Card type");

-/*
- * The video mem address of the video card. The driver has a little database
- * for some videocards to determine it from there. If your video card is not
- * in there you have either to give it to the driver as a parameter or set
- * in a VIDIOCSFBUF ioctl
- */
-
-static unsigned long vidmem; /* default = 0 - Video memory base address */
-module_param_hw(vidmem, ulong, iomem, 0444);
-MODULE_PARM_DESC(vidmem, "Default video memory base address");
-
/* Default input and video norm at startup of the driver. */

static unsigned int default_input; /* default 0 = Composite, 1 = S-Video */
@@ -1218,10 +1207,6 @@ static int zoran_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
jpg_bufsize = 8192;
if (jpg_bufsize > (512 * 1024))
jpg_bufsize = 512 * 1024;
- /* Use parameter for vidmem or try to find a video card */
- if (vidmem)
- pci_info(pdev, "%s: Using supplied video memory base address @ 0x%lx\n",
- ZORAN_NAME, vidmem);

/* some mainboards might not do PCI-PCI data transfer well */
if (pci_pci_problems & (PCIPCI_FAIL | PCIAGP_FAIL | PCIPCI_ALIMAGIK))
--
2.32.0

2021-10-27 09:50:44

by Corentin LABBE

[permalink] [raw]

Subject: [PATCH v3 09/14] staging: media: zoran: move config select on primary kconfig

Since all kconfigs for card selection are bool, this causes all selected modules
to be always built-in.
Prevent this by moving selects to the main tristate kconfig.

By doing this, remove also all "if MEDIA_SUBDRV_AUTOSELECT" which are
wrong, since zoran always need them to work.

Signed-off-by: Corentin Labbe <[email protected]>
---
drivers/staging/media/zoran/Kconfig | 23 ++++++++++-------------
1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/media/zoran/Kconfig b/drivers/staging/media/zoran/Kconfig
index 4067fa93d44d..faef008b8554 100644
--- a/drivers/staging/media/zoran/Kconfig
+++ b/drivers/staging/media/zoran/Kconfig
@@ -4,6 +4,16 @@ config VIDEO_ZORAN
depends on !ALPHA
depends on DEBUG_FS
select VIDEOBUF2_DMA_CONTIG
+ select VIDEO_ADV7170 if VIDEO_ZORAN_LML33R10
+ select VIDEO_ADV7175 if VIDEO_ZORAN_DC10 || VIDEO_ZORAN_DC30
+ select VIDEO_BT819 if VIDEO_ZORAN_LML33
+ select VIDEO_BT856 if VIDEO_ZORAN_LML33 || VIDEO_ZORAN_AVS6EYES
+ select VIDEO_BT866 if VIDEO_ZORAN_AVS6EYES
+ select VIDEO_KS0127 if VIDEO_ZORAN_AVS6EYES
+ select VIDEO_SAA711X if VIDEO_ZORAN_BUZ || VIDEO_ZORAN_LML33R10
+ select VIDEO_SAA7110 if VIDEO_ZORAN_DC10
+ select VIDEO_SAA7185 if VIDEO_ZORAN_BUZ
+ select VIDEO_VPX3220 if VIDEO_ZORAN_DC30
help
Say Y for support for MJPEG capture cards based on the Zoran
36057/36067 PCI controller chipset. This includes the Iomega
@@ -17,8 +27,6 @@ config VIDEO_ZORAN
config VIDEO_ZORAN_DC30
bool "Pinnacle/Miro DC30(+) support"
depends on VIDEO_ZORAN
- select VIDEO_ADV7175 if MEDIA_SUBDRV_AUTOSELECT
- select VIDEO_VPX3220 if MEDIA_SUBDRV_AUTOSELECT
help
Support for the Pinnacle/Miro DC30(+) MJPEG capture/playback
card. This also supports really old DC10 cards based on the
@@ -35,16 +43,12 @@ config VIDEO_ZORAN_ZR36060
config VIDEO_ZORAN_BUZ
bool "Iomega Buz support"
depends on VIDEO_ZORAN_ZR36060
- select VIDEO_SAA711X if MEDIA_SUBDRV_AUTOSELECT
- select VIDEO_SAA7185 if MEDIA_SUBDRV_AUTOSELECT
help
Support for the Iomega Buz MJPEG capture/playback card.

config VIDEO_ZORAN_DC10
bool "Pinnacle/Miro DC10(+) support"
depends on VIDEO_ZORAN_ZR36060
- select VIDEO_SAA7110 if MEDIA_SUBDRV_AUTOSELECT
- select VIDEO_ADV7175 if MEDIA_SUBDRV_AUTOSELECT
help
Support for the Pinnacle/Miro DC10(+) MJPEG capture/playback
card.
@@ -52,8 +56,6 @@ config VIDEO_ZORAN_DC10
config VIDEO_ZORAN_LML33
bool "Linux Media Labs LML33 support"
depends on VIDEO_ZORAN_ZR36060
- select VIDEO_BT819 if MEDIA_SUBDRV_AUTOSELECT
- select VIDEO_BT856 if MEDIA_SUBDRV_AUTOSELECT
help
Support for the Linux Media Labs LML33 MJPEG capture/playback
card.
@@ -61,8 +63,6 @@ config VIDEO_ZORAN_LML33
config VIDEO_ZORAN_LML33R10
bool "Linux Media Labs LML33R10 support"
depends on VIDEO_ZORAN_ZR36060
- select VIDEO_SAA711X if MEDIA_SUBDRV_AUTOSELECT
- select VIDEO_ADV7170 if MEDIA_SUBDRV_AUTOSELECT
help
support for the Linux Media Labs LML33R10 MJPEG capture/playback
card.
@@ -70,8 +70,5 @@ config VIDEO_ZORAN_LML33R10
config VIDEO_ZORAN_AVS6EYES
bool "AverMedia 6 Eyes support"
depends on VIDEO_ZORAN_ZR36060
- select VIDEO_BT856 if MEDIA_SUBDRV_AUTOSELECT
- select VIDEO_BT866 if MEDIA_SUBDRV_AUTOSELECT
- select VIDEO_KS0127 if MEDIA_SUBDRV_AUTOSELECT
help
Support for the AverMedia 6 Eyes video surveillance card.
--
2.32.0

2021-10-27 09:50:45

by Corentin LABBE

[permalink] [raw]

Subject: [PATCH v3 03/14] staging: media: zoran: rename debug module parameter

All zoran module will be merged, so to prevent conflict, the debug
module parameter need to be renamed

Signed-off-by: Corentin Labbe <[email protected]>
---
drivers/staging/media/zoran/videocodec.c | 8 ++++----
drivers/staging/media/zoran/zr36016.c | 12 ++++++------
drivers/staging/media/zoran/zr36050.c | 8 ++++----
drivers/staging/media/zoran/zr36060.c | 9 ++++-----
4 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/media/zoran/videocodec.c b/drivers/staging/media/zoran/videocodec.c
index 28031d3fd757..31019b5f377e 100644
--- a/drivers/staging/media/zoran/videocodec.c
+++ b/drivers/staging/media/zoran/videocodec.c
@@ -26,13 +26,13 @@

#include "videocodec.h"

-static int debug;
-module_param(debug, int, 0);
-MODULE_PARM_DESC(debug, "Debug level (0-4)");
+static int videocodec_debug;
+module_param(videocodec_debug, int, 0);
+MODULE_PARM_DESC(videocodec_debug, "Debug level (0-4)");

#define dprintk(num, format, args...) \
do { \
- if (debug >= num) \
+ if (videocodec_debug >= num) \
printk(format, ##args); \
} while (0)

diff --git a/drivers/staging/media/zoran/zr36016.c b/drivers/staging/media/zoran/zr36016.c
index 9b350a885879..50605460a44b 100644
--- a/drivers/staging/media/zoran/zr36016.c
+++ b/drivers/staging/media/zoran/zr36016.c
@@ -22,14 +22,14 @@
/* amount of chips attached via this driver */
static int zr36016_codecs;

-/* debugging is available via module parameter */
-static int debug;
-module_param(debug, int, 0);
-MODULE_PARM_DESC(debug, "Debug level (0-4)");
+static int zr36016_debug;
+module_param(zr36016_debug, int, 0);
+MODULE_PARM_DESC(zr36016_debug, "Debug level (0-4)");
+

#define dprintk(num, format, args...) \
do { \
- if (debug >= num) \
+ if (zr36016_debug >= num) \
printk(format, ##args); \
} while (0)

@@ -120,7 +120,7 @@ static u8 zr36016_read_version(struct zr36016 *ptr)

static int zr36016_basic_test(struct zr36016 *ptr)
{
- if (debug) {
+ if (zr36016_debug) {
int i;

zr36016_writei(ptr, ZR016I_PAX_LO, 0x55);
diff --git a/drivers/staging/media/zoran/zr36050.c b/drivers/staging/media/zoran/zr36050.c
index c62af27f2683..4dc7927fefc3 100644
--- a/drivers/staging/media/zoran/zr36050.c
+++ b/drivers/staging/media/zoran/zr36050.c
@@ -32,13 +32,13 @@
static int zr36050_codecs;

/* debugging is available via module parameter */
-static int debug;
-module_param(debug, int, 0);
-MODULE_PARM_DESC(debug, "Debug level (0-4)");
+static int zr36050_debug;
+module_param(zr36050_debug, int, 0);
+MODULE_PARM_DESC(zr36050_debug, "Debug level (0-4)");

#define dprintk(num, format, args...) \
do { \
- if (debug >= num) \
+ if (zr36050_debug >= num) \
printk(format, ##args); \
} while (0)

diff --git a/drivers/staging/media/zoran/zr36060.c b/drivers/staging/media/zoran/zr36060.c
index 1c3af11b5f24..7904d5b1f402 100644
--- a/drivers/staging/media/zoran/zr36060.c
+++ b/drivers/staging/media/zoran/zr36060.c
@@ -34,14 +34,13 @@ static bool low_bitrate;
module_param(low_bitrate, bool, 0);
MODULE_PARM_DESC(low_bitrate, "Buz compatibility option, halves bitrate");

-/* debugging is available via module parameter */
-static int debug;
-module_param(debug, int, 0);
-MODULE_PARM_DESC(debug, "Debug level (0-4)");
+static int zr36060_debug;
+module_param(zr36060_debug, int, 0);
+MODULE_PARM_DESC(zr36060_debug, "Debug level (0-4)");

#define dprintk(num, format, args...) \
do { \
- if (debug >= num) \
+ if (zr36060_debug >= num) \
printk(format, ##args); \
} while (0)

--
2.32.0

2021-10-27 09:50:47

by Corentin LABBE

[permalink] [raw]

Subject: [PATCH v3 10/14] staging: media: zoran: introduce zoran_i2c_init

Reduces the size of the probe function by adding zoran_i2c_init/zoran_i2c_exit
functions.

Signed-off-by: Corentin Labbe <[email protected]>
---
drivers/staging/media/zoran/zoran_card.c | 73 ++++++++++++++++++------
1 file changed, 54 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/media/zoran/zoran_card.c b/drivers/staging/media/zoran/zoran_card.c
index 19eb3150074a..a00ad40244d0 100644
--- a/drivers/staging/media/zoran/zoran_card.c
+++ b/drivers/staging/media/zoran/zoran_card.c
@@ -931,6 +931,53 @@ static int zoran_init_video_devices(struct zoran *zr)
return err;
}

+/*
+ * v4l2_device_unregister() will care about removing zr->encoder/zr->decoder
+ * via v4l2_i2c_subdev_unregister()
+ */
+static int zoran_i2c_init(struct zoran *zr)
+{
+ int err;
+
+ pci_info(zr->pci_dev, "Initializing i2c bus...\n");
+
+ err = zoran_register_i2c(zr);
+ if (err) {
+ pci_err(zr->pci_dev, "%s - cannot initialize i2c bus\n", __func__);
+ return err;
+ }
+
+ zr->decoder = v4l2_i2c_new_subdev(&zr->v4l2_dev, &zr->i2c_adapter,
+ zr->card.i2c_decoder, 0,
+ zr->card.addrs_decoder);
+ if (!zr->decoder) {
+ pci_err(zr->pci_dev, "Fail to get decoder %s\n", zr->card.i2c_decoder);
+ err = -EINVAL;
+ goto error_decoder;
+ }
+
+ if (zr->card.i2c_encoder) {
+ zr->encoder = v4l2_i2c_new_subdev(&zr->v4l2_dev, &zr->i2c_adapter,
+ zr->card.i2c_encoder, 0,
+ zr->card.addrs_encoder);
+ if (!zr->encoder) {
+ pci_err(zr->pci_dev, "Fail to get encoder %s\n", zr->card.i2c_encoder);
+ err = -EINVAL;
+ goto error_decoder;
+ }
+ }
+ return 0;
+
+error_decoder:
+ zoran_unregister_i2c(zr);
+ return err;
+}
+
+static void zoran_i2c_exit(struct zoran *zr)
+{
+ zoran_unregister_i2c(zr);
+}
+
void zoran_open_init_params(struct zoran *zr)
{
int i;
@@ -1059,7 +1106,7 @@ static void zoran_remove(struct pci_dev *pdev)
videocodec_exit(zr);

/* unregister i2c bus */
- zoran_unregister_i2c(zr);
+ zoran_i2c_exit(zr);
/* disable PCI bus-mastering */
zoran_set_pci_master(zr, 0);
/* put chip into reset */
@@ -1340,22 +1387,10 @@ static int zoran_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
}

zr36057_restart(zr);
- /* i2c */
- pci_info(zr->pci_dev, "Initializing i2c bus...\n");

- if (zoran_register_i2c(zr) < 0) {
- pci_err(pdev, "%s - can't initialize i2c bus\n", __func__);
+ err = zoran_i2c_init(zr);
+ if (err)
goto zr_free_irq;
- }
-
- zr->decoder = v4l2_i2c_new_subdev(&zr->v4l2_dev, &zr->i2c_adapter,
- zr->card.i2c_decoder, 0,
- zr->card.addrs_decoder);
-
- if (zr->card.i2c_encoder)
- zr->encoder = v4l2_i2c_new_subdev(&zr->v4l2_dev, &zr->i2c_adapter,
- zr->card.i2c_encoder, 0,
- zr->card.addrs_encoder);

pci_info(zr->pci_dev, "Initializing videocodec bus...\n");
err = videocodec_init(zr);
@@ -1370,15 +1405,15 @@ static int zoran_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
if (zr->card.video_codec != 0) {
master_codec = zoran_setup_videocodec(zr, zr->card.video_codec);
if (!master_codec)
- goto zr_unreg_i2c;
+ goto zr_unreg_videocodec;
zr->codec = videocodec_attach(master_codec);
if (!zr->codec) {
pci_err(pdev, "%s - no codec found\n", __func__);
- goto zr_unreg_i2c;
+ goto zr_unreg_videocodec;
}
if (zr->codec->type != zr->card.video_codec) {
pci_err(pdev, "%s - wrong codec\n", __func__);
- goto zr_detach_codec;
+ goto zr_unreg_videocodec;
}
}
if (zr->card.video_vfe != 0) {
@@ -1417,7 +1452,7 @@ static int zoran_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
zr_unreg_videocodec:
videocodec_exit(zr);
zr_unreg_i2c:
- zoran_unregister_i2c(zr);
+ zoran_i2c_exit(zr);
zr_free_irq:
btwrite(0, ZR36057_SPGPPCR);
pci_free_irq(zr->pci_dev, 0, zr);
--
2.32.0

2021-10-27 09:50:51

by Corentin LABBE

[permalink] [raw]

Subject: [PATCH v3 14/14] staging: media: zoran: DC30 encoder is not adv7175

The DC30 uses a non-i2c ITT MSE3000 encoder and not an adv7175 as stated
in the card list.
So remove adv7175 from DC30.

Signed-off-by: Corentin Labbe <[email protected]>
---
drivers/staging/media/zoran/zoran_card.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/drivers/staging/media/zoran/zoran_card.c b/drivers/staging/media/zoran/zoran_card.c
index 59df1e7691f9..a9b0316cd688 100644
--- a/drivers/staging/media/zoran/zoran_card.c
+++ b/drivers/staging/media/zoran/zoran_card.c
@@ -472,8 +472,6 @@ static struct card_info zoran_cards[NUM_CARDS] = {
.name = "DC30",
.i2c_decoder = "vpx3220a",
.addrs_decoder = vpx3220_addrs,
- .i2c_encoder = "adv7175",
- .addrs_encoder = adv717x_addrs,
.video_codec = CODEC_TYPE_ZR36050,
.video_vfe = CODEC_TYPE_ZR36016,

--
2.32.0

2021-10-27 09:50:51

by Corentin LABBE

[permalink] [raw]

Subject: [PATCH v3 11/14] staging: media: zoran: fix usage of vb2_dma_contig_set_max_seg_size

vb2_dma_contig_set_max_seg_size need to have a size in parameter and not
a DMA_BIT_MASK().
While fixing this issue, also fix error handling of all DMA size
setting.

Reported-by: kernel test robot <[email protected]>
Fixes: d4ae3689226e5 ("media: zoran: device support only 32bit DMA address")
Signed-off-by: Corentin Labbe <[email protected]>
---
drivers/staging/media/zoran/zoran_card.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/zoran/zoran_card.c b/drivers/staging/media/zoran/zoran_card.c
index a00ad40244d0..4ea2fbf189b9 100644
--- a/drivers/staging/media/zoran/zoran_card.c
+++ b/drivers/staging/media/zoran/zoran_card.c
@@ -1282,8 +1282,10 @@ static int zoran_probe(struct pci_dev *pdev, const struct pci_device_id *ent)

err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
if (err)
- return -ENODEV;
- vb2_dma_contig_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));
+ return err;
+ err = vb2_dma_contig_set_max_seg_size(&pdev->dev, U32_MAX);
+ if (err)
+ return err;

nr = zoran_num++;
if (nr >= BUZ_MAX) {
--
2.32.0

2021-11-03 15:23:02

by Hans Verkuil

[permalink] [raw]

Subject: Re: [PATCH v3 00/14] staging: media: zoran: fusion in one module

Hi Corentin,

On 26/10/2021 21:34, Corentin Labbe wrote:
> Hello
>
> The main change of this serie is to fusion all zoran related modules in
> one.
> This fixes the load order problem when everything is built-in.

I've been testing this series, and while the module load/unload is now working,
I'm running into a lot of other v4l2 compliance issues.

I've fixed various issues in some follow-up patches available in my tree:

https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=zoran

At least some of the worst offenders are now resolved. Note that the patch
dropping read/write support relies on this patch:

https://patchwork.linuxtv.org/project/linux-media/patch/[email protected]/

But there is one really major bug that makes me hesitant to merge this:

This works:

v4l2-ctl -v pixelformat=MJPG,width=768,height=576
v4l2-ctl --stream-mmap

This fails:

v4l2-ctl -v pixelformat=MJPG,width=768,height=288
v4l2-ctl --stream-mmap

It's an immediate lock up with nothing to indicate what is wrong.
As soon as the height is 288 or less, this happens.

Both with my DC30 and DC30D.

Do you see the same? Any idea what is going on? I would feel much happier
if this is fixed.

Note that the same problem is present without this patch series, so it's
been there for some time.

Regards,

Hans

>
> Regards
>
> Changes since v2:
> - added the 4 latest patchs
> - removed DEBUGFS kconfig option
> - fixed Dan Carpenter's reported codec issues
> - fixed kernel test robot's reported issues on vb2_dma_contig_set_max_seg_size()
>
> Changes since v1:
> - add missing debugfs cleaning
> - clean some remaining module_get/put functions which made impossible to
> remove the zoran module
> - added the two latest patchs
>
> Corentin Labbe (14):
> staging: media: zoran: move module parameter checks to zoran_probe
> staging: media: zoran: use module_pci_driver
> staging: media: zoran: rename debug module parameter
> staging: media: zoran: add debugfs
> staging: media: zoran: videocode: remove procfs
> staging: media: zoran: fusion all modules
> staging: media: zoran: remove vidmem
> staging: media: zoran: move videodev alloc
> staging: media: zoran: move config select on primary kconfig
> staging: media: zoran: introduce zoran_i2c_init
> staging: media: zoran: fix usage of vb2_dma_contig_set_max_seg_size
> staging: media: zoran: clean unused code
> staging: media: zoran: fix counting buffer in reserve
> staging: media: zoran: DC30 encoder is not adv7175
>
> drivers/staging/media/zoran/Kconfig | 38 +-
> drivers/staging/media/zoran/Makefile | 8 +-
> drivers/staging/media/zoran/videocodec.c | 68 +---
> drivers/staging/media/zoran/videocodec.h | 4 +-
> drivers/staging/media/zoran/zoran.h | 18 +-
> drivers/staging/media/zoran/zoran_card.c | 400 +++++++++++++--------
> drivers/staging/media/zoran/zoran_device.h | 2 -
> drivers/staging/media/zoran/zoran_driver.c | 8 +-
> drivers/staging/media/zoran/zr36016.c | 25 +-
> drivers/staging/media/zoran/zr36016.h | 2 +
> drivers/staging/media/zoran/zr36050.c | 24 +-
> drivers/staging/media/zoran/zr36050.h | 2 +
> drivers/staging/media/zoran/zr36060.c | 23 +-
> drivers/staging/media/zoran/zr36060.h | 2 +
> 14 files changed, 317 insertions(+), 307 deletions(-)
>

2021-11-03 16:01:44

by Corentin LABBE

[permalink] [raw]

Subject: Re: [PATCH v3 00/14] staging: media: zoran: fusion in one module

Le Wed, Nov 03, 2021 at 04:21:02PM +0100, Hans Verkuil a ?crit :
> Hi Corentin,
>
> On 26/10/2021 21:34, Corentin Labbe wrote:
> > Hello
> >
> > The main change of this serie is to fusion all zoran related modules in
> > one.
> > This fixes the load order problem when everything is built-in.
>
> I've been testing this series, and while the module load/unload is now working,
> I'm running into a lot of other v4l2 compliance issues.
>
> I've fixed various issues in some follow-up patches available in my tree:
>
> https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=zoran
>
> At least some of the worst offenders are now resolved. Note that the patch
> dropping read/write support relies on this patch:
>
> https://patchwork.linuxtv.org/project/linux-media/patch/[email protected]/

Hello

My test branch already included your "zoran: fix various V4L2 compliance errors"
I have quickly checked other patch and I am ok with them.
I will add and test with them.

>
> But there is one really major bug that makes me hesitant to merge this:
>
> This works:
>
> v4l2-ctl -v pixelformat=MJPG,width=768,height=576
> v4l2-ctl --stream-mmap
>
> This fails:
>
> v4l2-ctl -v pixelformat=MJPG,width=768,height=288
> v4l2-ctl --stream-mmap
>
> It's an immediate lock up with nothing to indicate what is wrong.
> As soon as the height is 288 or less, this happens.
>
> Both with my DC30 and DC30D.

Just for curiosity, what is the difference between thoses two ?

>
> Do you see the same? Any idea what is going on? I would feel much happier
> if this is fixed.
>
> Note that the same problem is present without this patch series, so it's
> been there for some time.
>

I will start on digging this problem and add thoses commands to my CI.
And I know there are a huge quantity of problem since origins.
A simple example is that just setting MJPEG as default input format does not work.

But since it is not related to my serie, can you please merge it.

Thanks
Regards

2021-11-03 16:31:31

by Hans Verkuil

[permalink] [raw]

Subject: Re: [PATCH v3 00/14] staging: media: zoran: fusion in one module

On 03/11/2021 16:57, LABBE Corentin wrote:
> Le Wed, Nov 03, 2021 at 04:21:02PM +0100, Hans Verkuil a écrit :
>> Hi Corentin,
>>
>> On 26/10/2021 21:34, Corentin Labbe wrote:
>>> Hello
>>>
>>> The main change of this serie is to fusion all zoran related modules in
>>> one.
>>> This fixes the load order problem when everything is built-in.
>>
>> I've been testing this series, and while the module load/unload is now working,
>> I'm running into a lot of other v4l2 compliance issues.
>>
>> I've fixed various issues in some follow-up patches available in my tree:
>>
>> https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=zoran
>>
>> At least some of the worst offenders are now resolved. Note that the patch
>> dropping read/write support relies on this patch:
>>
>> https://patchwork.linuxtv.org/project/linux-media/patch/[email protected]/
>
> Hello
>
> My test branch already included your "zoran: fix various V4L2 compliance errors"
> I have quickly checked other patch and I am ok with them.
> I will add and test with them.
>
>>
>> But there is one really major bug that makes me hesitant to merge this:
>>
>> This works:
>>
>> v4l2-ctl -v pixelformat=MJPG,width=768,height=576
>> v4l2-ctl --stream-mmap
>>
>> This fails:
>>
>> v4l2-ctl -v pixelformat=MJPG,width=768,height=288
>> v4l2-ctl --stream-mmap
>>
>> It's an immediate lock up with nothing to indicate what is wrong.
>> As soon as the height is 288 or less, this happens.
>>
>> Both with my DC30 and DC30D.
>
> Just for curiosity, what is the difference between thoses two ?

It's the DC30 variant without an adv7175.

>
>>
>> Do you see the same? Any idea what is going on? I would feel much happier
>> if this is fixed.
>>
>> Note that the same problem is present without this patch series, so it's
>> been there for some time.
>>
>
> I will start on digging this problem and add thoses commands to my CI.
> And I know there are a huge quantity of problem since origins.
> A simple example is that just setting MJPEG as default input format does not work.
>
> But since it is not related to my serie, can you please merge it.

Before I do that, I would really like to know a bit more about this issue:
can you reproduce it? Is it DC30 specific or a general problem with zoran?

The problem with this hard hang is that it is hard to do regression testing
with v4l2-compliance, since it will hang as soon as MJPG pixelformat is
tested.

I would feel much happier if the hang can be avoided, even if it is just
with a temporary hack. It will make it much easier going forward.

Regards,

Hans

2021-11-05 16:42:44

by Corentin LABBE

[permalink] [raw]

Subject: Re: [PATCH v3 00/14] staging: media: zoran: fusion in one module

Le Wed, Nov 03, 2021 at 05:29:46PM +0100, Hans Verkuil a ?crit :
> On 03/11/2021 16:57, LABBE Corentin wrote:
> > Le Wed, Nov 03, 2021 at 04:21:02PM +0100, Hans Verkuil a ?crit :
> >> Hi Corentin,
> >>
> >> On 26/10/2021 21:34, Corentin Labbe wrote:
> >>> Hello
> >>>
> >>> The main change of this serie is to fusion all zoran related modules in
> >>> one.
> >>> This fixes the load order problem when everything is built-in.
> >>
> >> I've been testing this series, and while the module load/unload is now working,
> >> I'm running into a lot of other v4l2 compliance issues.
> >>
> >> I've fixed various issues in some follow-up patches available in my tree:
> >>
> >> https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=zoran
> >>
> >> At least some of the worst offenders are now resolved. Note that the patch
> >> dropping read/write support relies on this patch:
> >>
> >> https://patchwork.linuxtv.org/project/linux-media/patch/[email protected]/
> >
> > Hello
> >
> > My test branch already included your "zoran: fix various V4L2 compliance errors"
> > I have quickly checked other patch and I am ok with them.
> > I will add and test with them.
> >
> >>
> >> But there is one really major bug that makes me hesitant to merge this:
> >>
> >> This works:
> >>
> >> v4l2-ctl -v pixelformat=MJPG,width=768,height=576
> >> v4l2-ctl --stream-mmap
> >>
> >> This fails:
> >>
> >> v4l2-ctl -v pixelformat=MJPG,width=768,height=288
> >> v4l2-ctl --stream-mmap
> >>
> >> It's an immediate lock up with nothing to indicate what is wrong.
> >> As soon as the height is 288 or less, this happens.
> >>
> >> Both with my DC30 and DC30D.
> >
> > Just for curiosity, what is the difference between thoses two ?
>
> It's the DC30 variant without an adv7175.

So my patch removing adv7175 from DC30 is wrong.
I need to add a new DC30D.

>
> >
> >>
> >> Do you see the same? Any idea what is going on? I would feel much happier
> >> if this is fixed.
> >>
> >> Note that the same problem is present without this patch series, so it's
> >> been there for some time.
> >>
> >
> > I will start on digging this problem and add thoses commands to my CI.
> > And I know there are a huge quantity of problem since origins.
> > A simple example is that just setting MJPEG as default input format does not work.
> >
> > But since it is not related to my serie, can you please merge it.
>
> Before I do that, I would really like to know a bit more about this issue:
> can you reproduce it? Is it DC30 specific or a general problem with zoran?
>
> The problem with this hard hang is that it is hard to do regression testing
> with v4l2-compliance, since it will hang as soon as MJPG pixelformat is
> tested.
>
> I would feel much happier if the hang can be avoided, even if it is just
> with a temporary hack. It will make it much easier going forward.
>

I hit the same problem with my DC10+.
I got the following trace:
[ 97.022391] BUG: kernel NULL pointer dereference, address: 0000000000000018
[ 97.029357] #PF: supervisor write access in kernel mode
[ 97.034579] #PF: error_code(0x0002) - not-present page
[ 97.039712] PGD 100e30067 P4D 100e30067 PUD 11c958067 PMD 0
[ 97.045370] Oops: 0002 [#1] PREEMPT SMP NOPTI
[ 97.049723] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G C 5.15.0-next-20211105+ #126
[ 97.058500] Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./K10N78, BIOS P2.00 07/01/2010
[ 97.067791] RIP: 0010:zoran_irq+0x178/0x2e0 [zr36067]
[ 97.072845] Code: 01 8d 5c 00 01 48 8b 85 90 0c 00 00 48 63 db 44 8b 2c 98 41 f6 c5 01 0f 84 64 01 00 00 4c 8b bc dd 38 0d 00 00 e8 98 72 a1 fa <49> 89 47 18 83 bd 90 0b 00 00 01 0f 84 da 00 00 00 48 8b 85 68 0c
[ 97.091590] RSP: 0018:ffffa57040003f00 EFLAGS: 00010016
[ 97.096807] RAX: 000000169273a7d2 RBX: 0000000000000001 RCX: 0000000000000018
[ 97.103932] RDX: 000000830c927d90 RSI: 000000000000d33a RDI: 00041965ba87a734
[ 97.111063] RBP: ffff9d845cce1028 R08: 00000000005b6db7 R09: 0000000000000000
[ 97.118188] R10: 0000000000000000 R11: ffffa57040003ff8 R12: 0000000000000065
[ 97.125312] R13: 0000000004027541 R14: ffff9d845cce1d58 R15: 0000000000000000
[ 97.132434] FS: 0000000000000000(0000) GS:ffff9d845fc00000(0000) knlGS:0000000000000000
[ 97.140513] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 97.146249] CR2: 0000000000000018 CR3: 000000011c956000 CR4: 00000000000006f0
[ 97.153374] Call Trace:
[ 97.155822] <IRQ>
[ 97.157840] __handle_irq_event_percpu+0x35/0x180
[ 97.162546] handle_irq_event+0x50/0xb0
[ 97.166384] handle_fasteoi_irq+0x8b/0x1e0
[ 97.170483] __common_interrupt+0x64/0x100
[ 97.174581] common_interrupt+0x9f/0xc0
[ 97.178414] </IRQ>
[ 97.180510] <TASK>
[ 97.182609] asm_common_interrupt+0x1e/0x40
[ 97.186793] RIP: 0010:acpi_idle_do_entry+0x47/0x50
[ 97.191585] Code: 08 48 8b 15 1f db 5d 01 ed c3 e9 64 fd ff ff 65 48 8b 04 25 00 ad 01 00 48 8b 00 a8 08 75 ea eb 07 0f 00 2d 0b 80 5b 00 fb f4 <fa> c3 cc cc cc cc cc cc cc 41 56 49 89 f6 41 55 41 89 d5 41 54 55
[ 97.210323] RSP: 0018:ffffffffbc603e30 EFLAGS: 00000246
[ 97.215539] RAX: 0000000000004000 RBX: ffff9d84413f1c00 RCX: 000000000000001f
[ 97.222666] RDX: ffff9d845fc00000 RSI: ffff9d8440165000 RDI: ffff9d8440165064
[ 97.229798] RBP: ffff9d8440165064 R08: 000000000001184d R09: 0000000000000018
[ 97.236921] R10: 00000000000029cc R11: 000000000000318c R12: 0000000000000001
[ 97.244046] R13: ffffffffbc7c3e20 R14: 0000000000000001 R15: 0000000000000000
[ 97.251178] acpi_idle_enter+0x99/0xe0
[ 97.254931] cpuidle_enter_state+0x84/0x360
[ 97.259118] cpuidle_enter+0x24/0x40
[ 97.262698] do_idle+0x1d0/0x250
[ 97.265928] cpu_startup_entry+0x14/0x20
[ 97.269846] start_kernel+0x63a/0x65f
[ 97.273514] secondary_startup_64_no_verify+0xc2/0xcb
[ 97.278565] </TASK>
[ 97.280748] Modules linked in: adv7175 saa7110 zr36067(C) videobuf2_dma_contig
[ 97.287970] CR2: 0000000000000018
[ 97.291279] ---[ end trace 0ee22c5269015e89 ]---
[ 97.295888] RIP: 0010:zoran_irq+0x178/0x2e0 [zr36067]
[ 97.300941] Code: 01 8d 5c 00 01 48 8b 85 90 0c 00 00 48 63 db 44 8b 2c 98 41 f6 c5 01 0f 84 64 01 00 00 4c 8b bc dd 38 0d 00 00 e8 98 72 a1 fa <49> 89 47 18 83 bd 90 0b 00 00 01 0f 84 da 00 00 00 48 8b 85 68 0c
[ 97.319679] RSP: 0018:ffffa57040003f00 EFLAGS: 00010016
[ 97.324896] RAX: 000000169273a7d2 RBX: 0000000000000001 RCX: 0000000000000018
[ 97.332019] RDX: 000000830c927d90 RSI: 000000000000d33a RDI: 00041965ba87a734
[ 97.339144] RBP: ffff9d845cce1028 R08: 00000000005b6db7 R09: 0000000000000000
[ 97.346276] R10: 0000000000000000 R11: ffffa57040003ff8 R12: 0000000000000065
[ 97.353401] R13: 0000000004027541 R14: ffff9d845cce1d58 R15: 0000000000000000
[ 97.360525] FS: 0000000000000000(0000) GS:ffff9d845fc00000(0000) knlGS:0000000000000000
[ 97.368603] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 97.374340] CR2: 0000000000000018 CR3: 000000011c956000 CR4: 00000000000006f0
[ 97.381464] Kernel panic - not syncing: Fatal exception in interrupt
[ 97.387810] Kernel Offset: 0x39c00000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff)
[ 97.398580] ---[ end Kernel panic - not syncing: Fatal exception in interrupt ]---

x86_64-pc-linux-gnu-addr2line -e vmlinux zoran_irq+0x16f/0x2e0
/usr/src/linux-next/arch/x86/include/asm/processor.h:443

I have no more clue for the moment.

2021-11-07 22:49:14

by Corentin LABBE

[permalink] [raw]

Subject: Re: [PATCH v3 00/14] staging: media: zoran: fusion in one module

2021-11-08 10:16:51

by Hans Verkuil

[permalink] [raw]

Subject: Re: [PATCH v3 00/14] staging: media: zoran: fusion in one module

On 07/11/2021 17:35, LABBE Corentin wrote:
> Le Wed, Nov 03, 2021 at 05:29:46PM +0100, Hans Verkuil a écrit :
>> On 03/11/2021 16:57, LABBE Corentin wrote:
>>> Le Wed, Nov 03, 2021 at 04:21:02PM +0100, Hans Verkuil a écrit :
>>>> Hi Corentin,
>>>>
>>>> On 26/10/2021 21:34, Corentin Labbe wrote:
>>>>> Hello
>>>>>
>>>>> The main change of this serie is to fusion all zoran related modules in
>>>>> one.
>>>>> This fixes the load order problem when everything is built-in.
>>>>
>>>> I've been testing this series, and while the module load/unload is now working,
>>>> I'm running into a lot of other v4l2 compliance issues.
>>>>
>>>> I've fixed various issues in some follow-up patches available in my tree:
>>>>
>>>> https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=zoran
>>>>
>>>> At least some of the worst offenders are now resolved. Note that the patch
>>>> dropping read/write support relies on this patch:
>>>>
>>>> https://patchwork.linuxtv.org/project/linux-media/patch/[email protected]/
>>>
>>> Hello
>>>
>>> My test branch already included your "zoran: fix various V4L2 compliance errors"
>>> I have quickly checked other patch and I am ok with them.
>>> I will add and test with them.
>>>
>>>>
>>>> But there is one really major bug that makes me hesitant to merge this:
>>>>
>>>> This works:
>>>>
>>>> v4l2-ctl -v pixelformat=MJPG,width=768,height=576
>>>> v4l2-ctl --stream-mmap
>>>>
>>>> This fails:
>>>>
>>>> v4l2-ctl -v pixelformat=MJPG,width=768,height=288
>>>> v4l2-ctl --stream-mmap
>>>>
>>>> It's an immediate lock up with nothing to indicate what is wrong.
>>>> As soon as the height is 288 or less, this happens.
>>>>
>>>> Both with my DC30 and DC30D.
>>>
>>> Just for curiosity, what is the difference between thoses two ?
>>
>> It's the DC30 variant without an adv7175.
>>
>>>
>>>>
>>>> Do you see the same? Any idea what is going on? I would feel much happier
>>>> if this is fixed.
>>>>
>>>> Note that the same problem is present without this patch series, so it's
>>>> been there for some time.
>>>>
>>>
>>> I will start on digging this problem and add thoses commands to my CI.
>>> And I know there are a huge quantity of problem since origins.
>>> A simple example is that just setting MJPEG as default input format does not work.
>>>
>>> But since it is not related to my serie, can you please merge it.
>>
>> Before I do that, I would really like to know a bit more about this issue:
>> can you reproduce it? Is it DC30 specific or a general problem with zoran?
>>
>> The problem with this hard hang is that it is hard to do regression testing
>> with v4l2-compliance, since it will hang as soon as MJPG pixelformat is
>> tested.
>>
>> I would feel much happier if the hang can be avoided, even if it is just
>> with a temporary hack. It will make it much easier going forward.
>>
>
> I found the bug
>
> The null pointer deref was in zoran_reap_stat_com() due to
> buf = zr->inuse[i];
> ...
> buf->vbuf.vb2_buf.timestamp = ktime_get_ns();
> with buf = NULL;
>
> It is due to miscalculation of "i".
>
> I will resend my serie with the fix for that.

Excellent news! Thank you for tracking this one down.

When you post your series, can you include my patches from
https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=zoran as well?

Regards,

Hans

2021-11-16 14:13:31

by Corentin LABBE

[permalink] [raw]

Subject: Re: [PATCH v3 00/14] staging: media: zoran: fusion in one module

Le Mon, Nov 08, 2021 at 09:21:22AM +0100, Hans Verkuil a ?crit :
> On 07/11/2021 17:35, LABBE Corentin wrote:
> > Le Wed, Nov 03, 2021 at 05:29:46PM +0100, Hans Verkuil a ?crit :
> >> On 03/11/2021 16:57, LABBE Corentin wrote:
> >>> Le Wed, Nov 03, 2021 at 04:21:02PM +0100, Hans Verkuil a ?crit :
> >>>> Hi Corentin,
> >>>>
> >>>> On 26/10/2021 21:34, Corentin Labbe wrote:
> >>>>> Hello
> >>>>>
> >>>>> The main change of this serie is to fusion all zoran related modules in
> >>>>> one.
> >>>>> This fixes the load order problem when everything is built-in.
> >>>>
> >>>> I've been testing this series, and while the module load/unload is now working,
> >>>> I'm running into a lot of other v4l2 compliance issues.
> >>>>
> >>>> I've fixed various issues in some follow-up patches available in my tree:
> >>>>
> >>>> https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=zoran
> >>>>
> >>>> At least some of the worst offenders are now resolved. Note that the patch
> >>>> dropping read/write support relies on this patch:
> >>>>
> >>>> https://patchwork.linuxtv.org/project/linux-media/patch/[email protected]/
> >>>
> >>> Hello
> >>>
> >>> My test branch already included your "zoran: fix various V4L2 compliance errors"
> >>> I have quickly checked other patch and I am ok with them.
> >>> I will add and test with them.
> >>>
> >>>>
> >>>> But there is one really major bug that makes me hesitant to merge this:
> >>>>
> >>>> This works:
> >>>>
> >>>> v4l2-ctl -v pixelformat=MJPG,width=768,height=576
> >>>> v4l2-ctl --stream-mmap
> >>>>
> >>>> This fails:
> >>>>
> >>>> v4l2-ctl -v pixelformat=MJPG,width=768,height=288
> >>>> v4l2-ctl --stream-mmap
> >>>>
> >>>> It's an immediate lock up with nothing to indicate what is wrong.
> >>>> As soon as the height is 288 or less, this happens.
> >>>>
> >>>> Both with my DC30 and DC30D.
> >>>
> >>> Just for curiosity, what is the difference between thoses two ?
> >>
> >> It's the DC30 variant without an adv7175.
> >>
> >>>
> >>>>
> >>>> Do you see the same? Any idea what is going on? I would feel much happier
> >>>> if this is fixed.
> >>>>
> >>>> Note that the same problem is present without this patch series, so it's
> >>>> been there for some time.
> >>>>
> >>>
> >>> I will start on digging this problem and add thoses commands to my CI.
> >>> And I know there are a huge quantity of problem since origins.
> >>> A simple example is that just setting MJPEG as default input format does not work.
> >>>
> >>> But since it is not related to my serie, can you please merge it.
> >>
> >> Before I do that, I would really like to know a bit more about this issue:
> >> can you reproduce it? Is it DC30 specific or a general problem with zoran?
> >>
> >> The problem with this hard hang is that it is hard to do regression testing
> >> with v4l2-compliance, since it will hang as soon as MJPG pixelformat is
> >> tested.
> >>
> >> I would feel much happier if the hang can be avoided, even if it is just
> >> with a temporary hack. It will make it much easier going forward.
> >>
> >
> > I found the bug
> >
> > The null pointer deref was in zoran_reap_stat_com() due to
> > buf = zr->inuse[i];
> > ...
> > buf->vbuf.vb2_buf.timestamp = ktime_get_ns();
> > with buf = NULL;
> >
> > It is due to miscalculation of "i".
> >
> > I will resend my serie with the fix for that.
>
> Excellent news! Thank you for tracking this one down.
>
> When you post your series, can you include my patches from
> https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=zoran as well?
>

Hello

Yes, I will include them.

Regards