2021-10-13 19:01:05

by Corentin LABBE

[permalink] [raw]
Subject: [PATCH v2 00/10] 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 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 (10):
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

drivers/staging/media/zoran/Kconfig | 46 +--
drivers/staging/media/zoran/Makefile | 8 +-
drivers/staging/media/zoran/videocodec.c | 68 +----
drivers/staging/media/zoran/videocodec.h | 6 +-
drivers/staging/media/zoran/zoran.h | 6 +-
drivers/staging/media/zoran/zoran_card.c | 328 ++++++++++++++-------
drivers/staging/media/zoran/zoran_driver.c | 5 +-
drivers/staging/media/zoran/zr36016.c | 24 +-
drivers/staging/media/zoran/zr36016.h | 2 +
drivers/staging/media/zoran/zr36050.c | 21 +-
drivers/staging/media/zoran/zr36050.h | 2 +
drivers/staging/media/zoran/zr36060.c | 21 +-
drivers/staging/media/zoran/zr36060.h | 2 +
13 files changed, 291 insertions(+), 248 deletions(-)

--
2.32.0


2021-10-13 19:01:09

by Corentin LABBE

[permalink] [raw]
Subject: [PATCH v2 05/10] 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 | 5 +++++
2 files changed, 7 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/media/zoran/videocodec.c b/drivers/staging/media/zoran/videocodec.c
index 31019b5f377e..3d5a83a07e07 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,8 @@ 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)
+#ifdef CONFIG_VIDEO_ZORAN_DEBUG
+int videocodec_debugfs_show(struct seq_file *m)
{
struct codec_list *h = codeclist_top;
struct attached_list *a;
@@ -300,25 +292,13 @@ static int proc_videocodecs_show(struct seq_file *m, void *v)
/* ===================== */
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..f2e17566795e 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,8 @@ extern int videocodec_unregister(const struct videocodec *);

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

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

2021-10-13 19:01:14

by Corentin LABBE

[permalink] [raw]
Subject: [PATCH v2 02/10] staging: media: zoran: use module_pci_driver

Simplify code by using module_pci_driver()

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

diff --git a/drivers/staging/media/zoran/zoran_card.c b/drivers/staging/media/zoran/zoran_card.c
index 3bc0e64f1007..f1465fbf98af 100644
--- a/drivers/staging/media/zoran/zoran_card.c
+++ b/drivers/staging/media/zoran/zoran_card.c
@@ -1314,23 +1314,4 @@ static struct pci_driver zoran_driver = {
.remove = zoran_remove,
};

-static int __init zoran_init(void)
-{
- int res;
-
- res = pci_register_driver(&zoran_driver);
- if (res) {
- pr_err("Unable to register ZR36057 driver\n");
- return res;
- }
-
- return 0;
-}
-
-static void __exit zoran_exit(void)
-{
- pci_unregister_driver(&zoran_driver);
-}
-
-module_init(zoran_init);
-module_exit(zoran_exit);
+module_pci_driver(zoran_driver);
--
2.32.0

2021-10-13 19:01:31

by Corentin LABBE

[permalink] [raw]
Subject: [PATCH v2 07/10] 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 a0739d3462bb..a079fb6e27a3 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 */
@@ -1163,10 +1152,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-13 19:01:51

by Corentin LABBE

[permalink] [raw]
Subject: [PATCH v2 08/10] 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 c37d064ff11d..31bc577f5df5 100644
--- a/drivers/staging/media/zoran/zoran.h
+++ b/drivers/staging/media/zoran/zoran.h
@@ -317,6 +317,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 a079fb6e27a3..9bc5af34d909 100644
--- a/drivers/staging/media/zoran/zoran_card.c
+++ b/drivers/staging/media/zoran/zoran_card.c
@@ -828,6 +828,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;
@@ -899,17 +945,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 */
@@ -922,26 +962,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) {
@@ -956,9 +979,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;
}

@@ -992,7 +1012,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-13 19:02:15

by Corentin LABBE

[permalink] [raw]
Subject: [PATCH v2 09/10] 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 b5a3fc6e98f6..0a9c1ab19016 100644
--- a/drivers/staging/media/zoran/Kconfig
+++ b/drivers/staging/media/zoran/Kconfig
@@ -3,6 +3,16 @@ config VIDEO_ZORAN
depends on PCI && I2C_ALGOBIT && VIDEO_V4L2
depends on !ALPHA
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
@@ -16,8 +26,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
@@ -34,16 +42,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.
@@ -51,8 +55,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.
@@ -60,8 +62,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.
@@ -69,9 +69,6 @@ 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-13 19:02:16

by Corentin LABBE

[permalink] [raw]
Subject: [PATCH v2 10/10] 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 | 67 ++++++++++++++++++------
1 file changed, 51 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/media/zoran/zoran_card.c b/drivers/staging/media/zoran/zoran_card.c
index 9bc5af34d909..fe4d867bf341 100644
--- a/drivers/staging/media/zoran/zoran_card.c
+++ b/drivers/staging/media/zoran/zoran_card.c
@@ -874,6 +874,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\n");
+ 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\n");
+ 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;
@@ -1001,7 +1048,7 @@ static void zoran_remove(struct pci_dev *pdev)
videocodec_detach(zr->vfe);

/* 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 */
@@ -1285,22 +1332,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");

@@ -1377,7 +1412,7 @@ static int zoran_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
zr_detach_codec:
videocodec_detach(zr->codec);
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-13 19:02:28

by Corentin LABBE

[permalink] [raw]
Subject: [PATCH v2 06/10] staging: media: zoran: fusion all modules

The zoran driver is split in many modules, but this lead to some
problems.
One of them is that load order is incorrect when everything is built-in.

Having more than one module is useless, so fusion all zoran modules in
one.

Signed-off-by: Corentin Labbe <[email protected]>
---
drivers/staging/media/zoran/Kconfig | 14 +++----
drivers/staging/media/zoran/Makefile | 8 ++--
drivers/staging/media/zoran/videocodec.c | 36 +-----------------
drivers/staging/media/zoran/videocodec.h | 1 -
drivers/staging/media/zoran/zoran_card.c | 48 +++++++++++++++++++++---
drivers/staging/media/zoran/zr36016.c | 12 +-----
drivers/staging/media/zoran/zr36016.h | 2 +
drivers/staging/media/zoran/zr36050.c | 13 +------
drivers/staging/media/zoran/zr36050.h | 2 +
drivers/staging/media/zoran/zr36060.c | 12 +-----
drivers/staging/media/zoran/zr36060.h | 2 +
11 files changed, 67 insertions(+), 83 deletions(-)

diff --git a/drivers/staging/media/zoran/Kconfig b/drivers/staging/media/zoran/Kconfig
index 06f79b91cda7..b5a3fc6e98f6 100644
--- a/drivers/staging/media/zoran/Kconfig
+++ b/drivers/staging/media/zoran/Kconfig
@@ -14,7 +14,7 @@ config VIDEO_ZORAN
module will be called zr36067.

config VIDEO_ZORAN_DC30
- tristate "Pinnacle/Miro DC30(+) support"
+ bool "Pinnacle/Miro DC30(+) support"
depends on VIDEO_ZORAN
select VIDEO_ADV7175 if MEDIA_SUBDRV_AUTOSELECT
select VIDEO_VPX3220 if MEDIA_SUBDRV_AUTOSELECT
@@ -24,7 +24,7 @@ config VIDEO_ZORAN_DC30
zr36050 MJPEG codec and zr36016 VFE.

config VIDEO_ZORAN_ZR36060
- tristate "Zoran ZR36060"
+ bool "Zoran ZR36060"
depends on VIDEO_ZORAN
help
Say Y to support Zoran boards based on 36060 chips.
@@ -32,7 +32,7 @@ config VIDEO_ZORAN_ZR36060
and 33 R10 and AverMedia 6 boards.

config VIDEO_ZORAN_BUZ
- tristate "Iomega Buz support"
+ bool "Iomega Buz support"
depends on VIDEO_ZORAN_ZR36060
select VIDEO_SAA711X if MEDIA_SUBDRV_AUTOSELECT
select VIDEO_SAA7185 if MEDIA_SUBDRV_AUTOSELECT
@@ -40,7 +40,7 @@ config VIDEO_ZORAN_BUZ
Support for the Iomega Buz MJPEG capture/playback card.

config VIDEO_ZORAN_DC10
- tristate "Pinnacle/Miro DC10(+) support"
+ bool "Pinnacle/Miro DC10(+) support"
depends on VIDEO_ZORAN_ZR36060
select VIDEO_SAA7110 if MEDIA_SUBDRV_AUTOSELECT
select VIDEO_ADV7175 if MEDIA_SUBDRV_AUTOSELECT
@@ -49,7 +49,7 @@ config VIDEO_ZORAN_DC10
card.

config VIDEO_ZORAN_LML33
- tristate "Linux Media Labs LML33 support"
+ 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
@@ -58,7 +58,7 @@ config VIDEO_ZORAN_LML33
card.

config VIDEO_ZORAN_LML33R10
- tristate "Linux Media Labs LML33R10 support"
+ 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
@@ -67,7 +67,7 @@ config VIDEO_ZORAN_LML33R10
card.

config VIDEO_ZORAN_AVS6EYES
- tristate "AverMedia 6 Eyes support"
+ bool "AverMedia 6 Eyes support"
depends on VIDEO_ZORAN_ZR36060
select VIDEO_BT856 if MEDIA_SUBDRV_AUTOSELECT
select VIDEO_BT866 if MEDIA_SUBDRV_AUTOSELECT
diff --git a/drivers/staging/media/zoran/Makefile b/drivers/staging/media/zoran/Makefile
index 7023158e3892..9603bac0195c 100644
--- a/drivers/staging/media/zoran/Makefile
+++ b/drivers/staging/media/zoran/Makefile
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
zr36067-objs := zoran_device.o \
- zoran_driver.o zoran_card.o
+ zoran_driver.o zoran_card.o videocodec.o

-obj-$(CONFIG_VIDEO_ZORAN) += zr36067.o videocodec.o
-obj-$(CONFIG_VIDEO_ZORAN_DC30) += zr36050.o zr36016.o
-obj-$(CONFIG_VIDEO_ZORAN_ZR36060) += zr36060.o
+obj-$(CONFIG_VIDEO_ZORAN) += zr36067.o
+zr36067-$(CONFIG_VIDEO_ZORAN_DC30) += zr36050.o zr36016.o
+zr36067-$(CONFIG_VIDEO_ZORAN_ZR36060) += zr36060.o
diff --git a/drivers/staging/media/zoran/videocodec.c b/drivers/staging/media/zoran/videocodec.c
index 3d5a83a07e07..7390fddd0279 100644
--- a/drivers/staging/media/zoran/videocodec.c
+++ b/drivers/staging/media/zoran/videocodec.c
@@ -8,8 +8,6 @@
* (c) 2002 Wolfgang Scherr <[email protected]>
*/

-#define VIDEOCODEC_VERSION "v0.2"
-
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/init.h>
@@ -72,12 +70,9 @@ struct videocodec *videocodec_attach(struct videocodec_master *master)
if ((master->flags & h->codec->flags) == master->flags) {
dprintk(4, "%s: try '%s'\n", __func__, h->codec->name);

- if (!try_module_get(h->codec->owner))
- return NULL;
-
codec = kmemdup(h->codec, sizeof(struct videocodec), GFP_KERNEL);
if (!codec)
- goto out_module_put;
+ goto out_kfree;

res = strlen(codec->name);
snprintf(codec->name + res, sizeof(codec->name) - res, "[%d]", h->attached);
@@ -113,13 +108,10 @@ struct videocodec *videocodec_attach(struct videocodec_master *master)
pr_err("%s: no codec found!\n", __func__);
return NULL;

- out_module_put:
- module_put(h->codec->owner);
out_kfree:
kfree(codec);
return NULL;
}
-EXPORT_SYMBOL(videocodec_attach);

int videocodec_detach(struct videocodec *codec)
{
@@ -160,7 +152,6 @@ int videocodec_detach(struct videocodec *codec)
prev->next = a->next;
dprintk(4, "videocodec: delete middle\n");
}
- module_put(a->codec->owner);
kfree(a->codec);
kfree(a);
h->attached -= 1;
@@ -175,7 +166,6 @@ int videocodec_detach(struct videocodec *codec)
pr_err("%s: given codec not found!\n", __func__);
return -EINVAL;
}
-EXPORT_SYMBOL(videocodec_detach);

int videocodec_register(const struct videocodec *codec)
{
@@ -208,7 +198,6 @@ int videocodec_register(const struct videocodec *codec)

return 0;
}
-EXPORT_SYMBOL(videocodec_register);

int videocodec_unregister(const struct videocodec *codec)
{
@@ -255,7 +244,6 @@ int videocodec_unregister(const struct videocodec *codec)
pr_err("%s: given codec not found!\n", __func__);
return -EINVAL;
}
-EXPORT_SYMBOL(videocodec_unregister);

#ifdef CONFIG_VIDEO_ZORAN_DEBUG
int videocodec_debugfs_show(struct seq_file *m)
@@ -286,25 +274,3 @@ int videocodec_debugfs_show(struct seq_file *m)
return 0;
}
#endif
-
-/* ===================== */
-/* hook in driver module */
-/* ===================== */
-static int __init videocodec_init(void)
-{
- pr_info("Linux video codec intermediate layer: %s\n", VIDEOCODEC_VERSION);
-
- return 0;
-}
-
-static void __exit videocodec_exit(void)
-{
-}
-
-module_init(videocodec_init);
-module_exit(videocodec_exit);
-
-MODULE_AUTHOR("Wolfgang Scherr <[email protected]>");
-MODULE_DESCRIPTION("Intermediate API module for video codecs "
- VIDEOCODEC_VERSION);
-MODULE_LICENSE("GPL");
diff --git a/drivers/staging/media/zoran/videocodec.h b/drivers/staging/media/zoran/videocodec.h
index f2e17566795e..7d0575554cca 100644
--- a/drivers/staging/media/zoran/videocodec.h
+++ b/drivers/staging/media/zoran/videocodec.h
@@ -234,7 +234,6 @@ struct jpeg_app_marker {
};

struct videocodec {
- struct module *owner;
/* -- filled in by slave device during register -- */
char name[32];
unsigned long magic; /* may be used for client<->master attaching */
diff --git a/drivers/staging/media/zoran/zoran_card.c b/drivers/staging/media/zoran/zoran_card.c
index 6f29986a3fc2..a0739d3462bb 100644
--- a/drivers/staging/media/zoran/zoran_card.c
+++ b/drivers/staging/media/zoran/zoran_card.c
@@ -29,6 +29,9 @@
#include "zoran.h"
#include "zoran_card.h"
#include "zoran_device.h"
+#include "zr36016.h"
+#include "zr36050.h"
+#include "zr36060.h"

extern const struct zoran_format zoran_formats[];

@@ -266,6 +269,39 @@ static const char *codecid_to_modulename(u16 codecid)
return name;
}

+static int load_codec(struct zoran *zr, u16 codecid)
+{
+ switch (codecid) {
+ case CODEC_TYPE_ZR36060:
+#ifdef CONFIG_VIDEO_ZORAN_ZR36060
+ return zr36060_init_module();
+#else
+ pci_err(zr->pci_dev, "ZR36060 support is not enabled\n");
+ return -EINVAL;
+#endif
+ break;
+ case CODEC_TYPE_ZR36050:
+#ifdef CONFIG_VIDEO_ZORAN_DC30
+ return zr36050_init_module();
+#else
+ pci_err(zr->pci_dev, "ZR36050 support is not enabled\n");
+ return -EINVAL;
+#endif
+ break;
+ case CODEC_TYPE_ZR36016:
+#ifdef CONFIG_VIDEO_ZORAN_DC30
+ return zr36016_init_module();
+#else
+ pci_err(zr->pci_dev, "ZR36016 support is not enabled\n");
+ return -EINVAL;
+#endif
+ break;
+ }
+
+ pci_err(zr->pci_dev, "unknown codec id %x\n", codecid);
+ return -EINVAL;
+}
+
// struct tvnorm {
// u16 wt, wa, h_start, h_sync_start, ht, ha, v_start;
// };
@@ -1080,6 +1116,8 @@ static int zoran_debugfs_show(struct seq_file *seq, void *v)

seq_printf(seq, "Prepared %u\n", zr->prepared);
seq_printf(seq, "Queued %u\n", zr->queued);
+
+ videocodec_debugfs_show(seq);
return 0;
}

@@ -1264,17 +1302,17 @@ static int zoran_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
if (zr->card.video_codec) {
codec_name = codecid_to_modulename(zr->card.video_codec);
if (codec_name) {
- result = request_module(codec_name);
- if (result)
- pci_err(pdev, "failed to load modules %s: %d\n", codec_name, result);
+ result = load_codec(zr, zr->card.video_codec);
+ if (result < 0)
+ pci_err(pdev, "failed to load codec %s: %d\n", codec_name, result);
}
}
if (zr->card.video_vfe) {
vfe_name = codecid_to_modulename(zr->card.video_vfe);
if (vfe_name) {
- result = request_module(vfe_name);
+ result = load_codec(zr, zr->card.video_vfe);
if (result < 0)
- pci_err(pdev, "failed to load modules %s: %d\n", vfe_name, result);
+ pci_err(pdev, "failed to load codec %s: %d\n", vfe_name, result);
}
}

diff --git a/drivers/staging/media/zoran/zr36016.c b/drivers/staging/media/zoran/zr36016.c
index 50605460a44b..e3fda82fa479 100644
--- a/drivers/staging/media/zoran/zr36016.c
+++ b/drivers/staging/media/zoran/zr36016.c
@@ -390,7 +390,6 @@ static int zr36016_setup(struct videocodec *codec)
}

static const struct videocodec zr36016_codec = {
- .owner = THIS_MODULE,
.name = "zr36016",
.magic = 0L, /* magic not used */
.flags =
@@ -409,14 +408,14 @@ static const struct videocodec zr36016_codec = {
HOOK IN DRIVER AS KERNEL MODULE
========================================================================= */

-static int __init zr36016_init_module(void)
+int zr36016_init_module(void)
{
//dprintk(1, "ZR36016 driver %s\n",ZR016_VERSION);
zr36016_codecs = 0;
return videocodec_register(&zr36016_codec);
}

-static void __exit zr36016_cleanup_module(void)
+void zr36016_cleanup_module(void)
{
if (zr36016_codecs) {
dprintk(1,
@@ -425,10 +424,3 @@ static void __exit zr36016_cleanup_module(void)
}
videocodec_unregister(&zr36016_codec);
}
-
-module_init(zr36016_init_module);
-module_exit(zr36016_cleanup_module);
-
-MODULE_AUTHOR("Wolfgang Scherr <[email protected]>");
-MODULE_DESCRIPTION("Driver module for ZR36016 video frontends");
-MODULE_LICENSE("GPL");
diff --git a/drivers/staging/media/zoran/zr36016.h b/drivers/staging/media/zoran/zr36016.h
index 1475f971cc24..04afba35669d 100644
--- a/drivers/staging/media/zoran/zr36016.h
+++ b/drivers/staging/media/zoran/zr36016.h
@@ -89,4 +89,6 @@ struct zr36016 {
#define ZR016_SIGN 0x02
#define ZR016_YMCS 0x01

+int zr36016_init_module(void);
+void zr36016_cleanup_module(void);
#endif /*fndef ZR36016_H */
diff --git a/drivers/staging/media/zoran/zr36050.c b/drivers/staging/media/zoran/zr36050.c
index 4dc7927fefc3..45d0eda69c7f 100644
--- a/drivers/staging/media/zoran/zr36050.c
+++ b/drivers/staging/media/zoran/zr36050.c
@@ -798,7 +798,6 @@ static int zr36050_setup(struct videocodec *codec)
}

static const struct videocodec zr36050_codec = {
- .owner = THIS_MODULE,
.name = "zr36050",
.magic = 0L, // magic not used
.flags =
@@ -817,14 +816,14 @@ static const struct videocodec zr36050_codec = {
HOOK IN DRIVER AS KERNEL MODULE
========================================================================= */

-static int __init zr36050_init_module(void)
+int zr36050_init_module(void)
{
//dprintk(1, "ZR36050 driver %s\n",ZR050_VERSION);
zr36050_codecs = 0;
return videocodec_register(&zr36050_codec);
}

-static void __exit zr36050_cleanup_module(void)
+void zr36050_cleanup_module(void)
{
if (zr36050_codecs) {
dprintk(1,
@@ -833,11 +832,3 @@ static void __exit zr36050_cleanup_module(void)
}
videocodec_unregister(&zr36050_codec);
}
-
-module_init(zr36050_init_module);
-module_exit(zr36050_cleanup_module);
-
-MODULE_AUTHOR("Wolfgang Scherr <[email protected]>");
-MODULE_DESCRIPTION("Driver module for ZR36050 jpeg processors "
- ZR050_VERSION);
-MODULE_LICENSE("GPL");
diff --git a/drivers/staging/media/zoran/zr36050.h b/drivers/staging/media/zoran/zr36050.h
index 8f972d045b58..f9b58f4c77b9 100644
--- a/drivers/staging/media/zoran/zr36050.h
+++ b/drivers/staging/media/zoran/zr36050.h
@@ -160,4 +160,6 @@ struct zr36050 {
#define ZR050_U_COMPONENT 1
#define ZR050_V_COMPONENT 2

+int zr36050_init_module(void);
+void zr36050_cleanup_module(void);
#endif /*fndef ZR36050_H */
diff --git a/drivers/staging/media/zoran/zr36060.c b/drivers/staging/media/zoran/zr36060.c
index 7904d5b1f402..afbb5624e98f 100644
--- a/drivers/staging/media/zoran/zr36060.c
+++ b/drivers/staging/media/zoran/zr36060.c
@@ -831,7 +831,6 @@ static int zr36060_setup(struct videocodec *codec)
}

static const struct videocodec zr36060_codec = {
- .owner = THIS_MODULE,
.name = "zr36060",
.magic = 0L, // magic not used
.flags =
@@ -846,13 +845,13 @@ static const struct videocodec zr36060_codec = {
// others are not used
};

-static int __init zr36060_init_module(void)
+int zr36060_init_module(void)
{
zr36060_codecs = 0;
return videocodec_register(&zr36060_codec);
}

-static void __exit zr36060_cleanup_module(void)
+void zr36060_cleanup_module(void)
{
if (zr36060_codecs) {
dprintk(1,
@@ -863,10 +862,3 @@ static void __exit zr36060_cleanup_module(void)
/* however, we can't just stay alive */
videocodec_unregister(&zr36060_codec);
}
-
-module_init(zr36060_init_module);
-module_exit(zr36060_cleanup_module);
-
-MODULE_AUTHOR("Laurent Pinchart <[email protected]>");
-MODULE_DESCRIPTION("Driver module for ZR36060 jpeg processors " ZR060_VERSION);
-MODULE_LICENSE("GPL");
diff --git a/drivers/staging/media/zoran/zr36060.h b/drivers/staging/media/zoran/zr36060.h
index d2cdc26bf625..fbf5429534ac 100644
--- a/drivers/staging/media/zoran/zr36060.h
+++ b/drivers/staging/media/zoran/zr36060.h
@@ -198,4 +198,6 @@ struct zr36060 {
#define ZR060_SR_H_SCALE2 BIT(0)
#define ZR060_SR_H_SCALE4 (2 << 0)

+int zr36060_init_module(void);
+void zr36060_cleanup_module(void);
#endif /*fndef ZR36060_H */
--
2.32.0

2021-10-13 19:03:29

by Corentin LABBE

[permalink] [raw]
Subject: [PATCH v2 04/10] staging: media: zoran: add debugfs

Add debugfs for displaying zoran debug and stats information.

Signed-off-by: Corentin Labbe <[email protected]>
---
drivers/staging/media/zoran/Kconfig | 9 ++++++
drivers/staging/media/zoran/zoran.h | 4 +++
drivers/staging/media/zoran/zoran_card.c | 41 ++++++++++++++++++++++++
3 files changed, 54 insertions(+)

diff --git a/drivers/staging/media/zoran/Kconfig b/drivers/staging/media/zoran/Kconfig
index 7874842033ca..06f79b91cda7 100644
--- a/drivers/staging/media/zoran/Kconfig
+++ b/drivers/staging/media/zoran/Kconfig
@@ -74,3 +74,12 @@ config VIDEO_ZORAN_AVS6EYES
select VIDEO_KS0127 if MEDIA_SUBDRV_AUTOSELECT
help
Support for the AverMedia 6 Eyes video surveillance card.
+
+config VIDEO_ZORAN_DEBUG
+ bool "Enable zoran debugfs"
+ depends on VIDEO_ZORAN
+ depends on DEBUG_FS
+ help
+ Say y to enable zoran debug file.
+ This will create /sys/kernel/debug/CARD_NAME/debug for displaying
+ stats and debug information.
diff --git a/drivers/staging/media/zoran/zoran.h b/drivers/staging/media/zoran/zoran.h
index b1ad2a2b914c..c37d064ff11d 100644
--- a/drivers/staging/media/zoran/zoran.h
+++ b/drivers/staging/media/zoran/zoran.h
@@ -18,6 +18,7 @@
#ifndef _BUZ_H_
#define _BUZ_H_

+#include <linux/debugfs.h>
#include <media/v4l2-device.h>
#include <media/v4l2-ctrls.h>
#include <media/videobuf2-core.h>
@@ -295,6 +296,9 @@ struct zoran {
struct list_head queued_bufs;
spinlock_t queued_bufs_lock; /* Protects queued_bufs */
struct zr_buffer *inuse[BUZ_NUM_STAT_COM * 2];
+#ifdef CONFIG_VIDEO_ZORAN_DEBUG
+ struct dentry *dbgfs_dir;
+#endif
};

static inline struct zoran *to_zoran(struct v4l2_device *v4l2_dev)
diff --git a/drivers/staging/media/zoran/zoran_card.c b/drivers/staging/media/zoran/zoran_card.c
index f1465fbf98af..6f29986a3fc2 100644
--- a/drivers/staging/media/zoran/zoran_card.c
+++ b/drivers/staging/media/zoran/zoran_card.c
@@ -945,6 +945,8 @@ static void zoran_remove(struct pci_dev *pdev)
if (!zr->initialized)
goto exit_free;

+ debugfs_remove_recursive(zr->dbgfs_dir);
+
zoran_queue_exit(zr);

/* unregister videocodec bus */
@@ -1051,6 +1053,39 @@ static const struct v4l2_ctrl_ops zoran_video_ctrl_ops = {
.s_ctrl = zoran_video_set_ctrl,
};

+#ifdef CONFIG_VIDEO_ZORAN_DEBUG
+static int zoran_debugfs_show(struct seq_file *seq, void *v)
+{
+ struct zoran *zr = seq->private;
+
+ seq_printf(seq, "Running mode %x\n", zr->running);
+ seq_printf(seq, "Codec mode %x\n", zr->codec_mode);
+ seq_printf(seq, "Norm %llx\n", zr->norm);
+ seq_printf(seq, "Input %d\n", zr->input);
+ seq_printf(seq, "Buffersize %d\n", zr->buffer_size);
+
+ seq_printf(seq, "V4L width %dx%d\n", zr->v4l_settings.width, zr->v4l_settings.height);
+ seq_printf(seq, "V4L bytesperline %d\n", zr->v4l_settings.bytesperline);
+
+ seq_printf(seq, "JPG decimation %u\n", zr->jpg_settings.decimation);
+ seq_printf(seq, "JPG hor_dcm %u\n", zr->jpg_settings.hor_dcm);
+ seq_printf(seq, "JPG ver_dcm %u\n", zr->jpg_settings.ver_dcm);
+ seq_printf(seq, "JPG tmp_dcm %u\n", zr->jpg_settings.tmp_dcm);
+ seq_printf(seq, "JPG odd_even %u\n", zr->jpg_settings.odd_even);
+ seq_printf(seq, "JPG crop %dx%d %d %d\n",
+ zr->jpg_settings.img_x,
+ zr->jpg_settings.img_y,
+ zr->jpg_settings.img_width,
+ zr->jpg_settings.img_height);
+
+ seq_printf(seq, "Prepared %u\n", zr->prepared);
+ seq_printf(seq, "Queued %u\n", zr->queued);
+ return 0;
+}
+
+DEFINE_SHOW_ATTRIBUTE(zoran_debugfs);
+#endif
+
/*
* Scan for a Buz card (actually for the PCI controller ZR36057),
* request the irq and map the io memory
@@ -1286,6 +1321,12 @@ static int zoran_probe(struct pci_dev *pdev, const struct pci_device_id *ent)

zr->map_mode = ZORAN_MAP_MODE_RAW;

+#ifdef CONFIG_VIDEO_ZORAN_DEBUG
+ zr->dbgfs_dir = debugfs_create_dir(ZR_DEVNAME(zr), NULL);
+ debugfs_create_file("debug", 0444,
+ zr->dbgfs_dir, zr,
+ &zoran_debugfs_fops);
+#endif
return 0;

zr_detach_vfe:
--
2.32.0

2021-10-14 07:40:03

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH v2 04/10] staging: media: zoran: add debugfs

On Wed, Oct 13, 2021 at 06:58:06PM +0000, Corentin Labbe wrote:
> +config VIDEO_ZORAN_DEBUG
> + bool "Enable zoran debugfs"
> + depends on VIDEO_ZORAN
> + depends on DEBUG_FS
> + help
> + Say y to enable zoran debug file.
> + This will create /sys/kernel/debug/CARD_NAME/debug for displaying
> + stats and debug information.

Why bother with a CONFIG? Just make it always on?

> @@ -1286,6 +1321,12 @@ static int zoran_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>
> zr->map_mode = ZORAN_MAP_MODE_RAW;
>
> +#ifdef CONFIG_VIDEO_ZORAN_DEBUG
> + zr->dbgfs_dir = debugfs_create_dir(ZR_DEVNAME(zr), NULL);
> + debugfs_create_file("debug", 0444,
> + zr->dbgfs_dir, zr,
> + &zoran_debugfs_fops);

This whitespace is weird.

regards,
dan carpenter

2021-10-14 07:59:32

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH v2 06/10] staging: media: zoran: fusion all modules

On Wed, Oct 13, 2021 at 06:58:08PM +0000, Corentin Labbe wrote:
> +static int load_codec(struct zoran *zr, u16 codecid)
> +{
> + switch (codecid) {
> + case CODEC_TYPE_ZR36060:
> +#ifdef CONFIG_VIDEO_ZORAN_ZR36060
> + return zr36060_init_module();
> +#else
> + pci_err(zr->pci_dev, "ZR36060 support is not enabled\n");
> + return -EINVAL;
> +#endif
> + break;
> + case CODEC_TYPE_ZR36050:
> +#ifdef CONFIG_VIDEO_ZORAN_DC30
> + return zr36050_init_module();
> +#else
> + pci_err(zr->pci_dev, "ZR36050 support is not enabled\n");
> + return -EINVAL;
> +#endif
> + break;
> + case CODEC_TYPE_ZR36016:
> +#ifdef CONFIG_VIDEO_ZORAN_DC30
> + return zr36016_init_module();
> +#else
> + pci_err(zr->pci_dev, "ZR36016 support is not enabled\n");
> + return -EINVAL;
> +#endif

The caller already prints an error message. Can you look through the
dmesg and make sure were not printing a bunch of duplicate stuff? Also
if load_codec() fails, the probe function still does
zoran_setup_videocodec() on the failed codec.

These would be better in a .h file.

#ifdef CONFIG_VIDEO_ZORAN_ZR36060
int zr36060_init_module(void);
#else
int zr36060_init_module(void) { return -EINVAL; }
#endif

regards,
dan carpenter

2021-10-14 08:05:02

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH v2 06/10] staging: media: zoran: fusion all modules

On Wed, Oct 13, 2021 at 06:58:08PM +0000, Corentin Labbe wrote:
> The zoran driver is split in many modules, but this lead to some
> problems.
> One of them is that load order is incorrect when everything is built-in.
>
> Having more than one module is useless, so fusion all zoran modules in
^^^^^^
"fusion" isn't the right word. It should be "fuse" or even better
"merge". Same in the subject.

> one.
>
> Signed-off-by: Corentin Labbe <[email protected]>

[ snip ]

> +static int load_codec(struct zoran *zr, u16 codecid)
> +{
> + switch (codecid) {
> + case CODEC_TYPE_ZR36060:
> +#ifdef CONFIG_VIDEO_ZORAN_ZR36060
> + return zr36060_init_module();
> +#else
> + pci_err(zr->pci_dev, "ZR36060 support is not enabled\n");
> + return -EINVAL;
> +#endif
> + break;
> + case CODEC_TYPE_ZR36050:
> +#ifdef CONFIG_VIDEO_ZORAN_DC30
> + return zr36050_init_module();
> +#else
> + pci_err(zr->pci_dev, "ZR36050 support is not enabled\n");
> + return -EINVAL;
> +#endif
> + break;
> + case CODEC_TYPE_ZR36016:
> +#ifdef CONFIG_VIDEO_ZORAN_DC30
> + return zr36016_init_module();
> +#else
> + pci_err(zr->pci_dev, "ZR36016 support is not enabled\n");
> + return -EINVAL;
> +#endif
> + break;
> + }

Not related to your patch but if load_codec() fails, the probe function
still calls zoran_setup_videocodec() on the failed codec. It might be
better to set the codec to zero?

result = load_codec(zr, zr->card.video_codec);
if (result < 0) {
pci_err(pdev, "failed to load codec %s: %d\n", codec_name, result);
zr->card.video_codec = 0;
}

regards,
dan carpenter

2021-10-14 13:06:30

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v2 04/10] staging: media: zoran: add debugfs

Hi Corentin,

I love your patch! Perhaps something to improve:

[auto build test WARNING on staging/staging-testing]

url: https://github.com/0day-ci/linux/commits/Corentin-Labbe/staging-media-zoran-fusion-in-one-module/20211014-025945
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git 6ac113f741a7674e4268eea3eb13972732d83571
config: x86_64-randconfig-a016-20211014 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 6c76d0101193aa4eb891a6954ff047eda2f9cf71)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/74fc116256f23b2c65d0c813f1d90b617ce9c97d
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Corentin-Labbe/staging-media-zoran-fusion-in-one-module/20211014-025945
git checkout 74fc116256f23b2c65d0c813f1d90b617ce9c97d
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=x86_64

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

drivers/staging/media/zoran/zoran_card.c:948:31: error: no member named 'dbgfs_dir' in 'struct zoran'
debugfs_remove_recursive(zr->dbgfs_dir);
~~ ^
>> drivers/staging/media/zoran/zoran_card.c:1141:46: warning: implicit conversion from 'unsigned long long' to 'unsigned int' changes value from 18446744073709551615 to 4294967295 [-Wconstant-conversion]
vb2_dma_contig_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^~~~~~~~~~~~~~~~
include/linux/dma-mapping.h:76:40: note: expanded from macro 'DMA_BIT_MASK'
#define DMA_BIT_MASK(n) (((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
^~~~~
1 warning and 1 error generated.


vim +1141 drivers/staging/media/zoran/zoran_card.c

74fc116256f23b Corentin Labbe 2021-10-13 1088
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1089 /*
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1090 * Scan for a Buz card (actually for the PCI controller ZR36057),
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1091 * request the irq and map the io memory
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1092 */
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1093 static int zoran_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1094 {
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1095 unsigned char latency, need_latency;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1096 struct zoran *zr;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1097 int result;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1098 struct videocodec_master *master_vfe = NULL;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1099 struct videocodec_master *master_codec = NULL;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1100 int card_num;
d61c7451fcb712 Corentin Labbe 2020-09-25 1101 const char *codec_name, *vfe_name;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1102 unsigned int nr;
d4ae3689226e56 Corentin Labbe 2020-09-25 1103 int err;
d4ae3689226e56 Corentin Labbe 2020-09-25 1104
26edeeecea59d6 Corentin Labbe 2021-10-13 1105 pci_info(pdev, "Zoran MJPEG board driver version %s\n", ZORAN_VERSION);
26edeeecea59d6 Corentin Labbe 2021-10-13 1106
26edeeecea59d6 Corentin Labbe 2021-10-13 1107 /* check the parameters we have been given, adjust if necessary */
26edeeecea59d6 Corentin Labbe 2021-10-13 1108 if (v4l_nbufs < 2)
26edeeecea59d6 Corentin Labbe 2021-10-13 1109 v4l_nbufs = 2;
26edeeecea59d6 Corentin Labbe 2021-10-13 1110 if (v4l_nbufs > VIDEO_MAX_FRAME)
26edeeecea59d6 Corentin Labbe 2021-10-13 1111 v4l_nbufs = VIDEO_MAX_FRAME;
26edeeecea59d6 Corentin Labbe 2021-10-13 1112 /* The user specifies the in KB, we want them in byte (and page aligned) */
26edeeecea59d6 Corentin Labbe 2021-10-13 1113 v4l_bufsize = PAGE_ALIGN(v4l_bufsize * 1024);
26edeeecea59d6 Corentin Labbe 2021-10-13 1114 if (v4l_bufsize < 32768)
26edeeecea59d6 Corentin Labbe 2021-10-13 1115 v4l_bufsize = 32768;
26edeeecea59d6 Corentin Labbe 2021-10-13 1116 /* 2 MB is arbitrary but sufficient for the maximum possible images */
26edeeecea59d6 Corentin Labbe 2021-10-13 1117 if (v4l_bufsize > 2048 * 1024)
26edeeecea59d6 Corentin Labbe 2021-10-13 1118 v4l_bufsize = 2048 * 1024;
26edeeecea59d6 Corentin Labbe 2021-10-13 1119 if (jpg_nbufs < 4)
26edeeecea59d6 Corentin Labbe 2021-10-13 1120 jpg_nbufs = 4;
26edeeecea59d6 Corentin Labbe 2021-10-13 1121 if (jpg_nbufs > BUZ_MAX_FRAME)
26edeeecea59d6 Corentin Labbe 2021-10-13 1122 jpg_nbufs = BUZ_MAX_FRAME;
26edeeecea59d6 Corentin Labbe 2021-10-13 1123 jpg_bufsize = PAGE_ALIGN(jpg_bufsize * 1024);
26edeeecea59d6 Corentin Labbe 2021-10-13 1124 if (jpg_bufsize < 8192)
26edeeecea59d6 Corentin Labbe 2021-10-13 1125 jpg_bufsize = 8192;
26edeeecea59d6 Corentin Labbe 2021-10-13 1126 if (jpg_bufsize > (512 * 1024))
26edeeecea59d6 Corentin Labbe 2021-10-13 1127 jpg_bufsize = 512 * 1024;
26edeeecea59d6 Corentin Labbe 2021-10-13 1128 /* Use parameter for vidmem or try to find a video card */
26edeeecea59d6 Corentin Labbe 2021-10-13 1129 if (vidmem)
26edeeecea59d6 Corentin Labbe 2021-10-13 1130 pci_info(pdev, "%s: Using supplied video memory base address @ 0x%lx\n",
26edeeecea59d6 Corentin Labbe 2021-10-13 1131 ZORAN_NAME, vidmem);
26edeeecea59d6 Corentin Labbe 2021-10-13 1132
26edeeecea59d6 Corentin Labbe 2021-10-13 1133 /* some mainboards might not do PCI-PCI data transfer well */
26edeeecea59d6 Corentin Labbe 2021-10-13 1134 if (pci_pci_problems & (PCIPCI_FAIL | PCIAGP_FAIL | PCIPCI_ALIMAGIK))
26edeeecea59d6 Corentin Labbe 2021-10-13 1135 pci_warn(pdev, "%s: chipset does not support reliable PCI-PCI DMA\n",
26edeeecea59d6 Corentin Labbe 2021-10-13 1136 ZORAN_NAME);
26edeeecea59d6 Corentin Labbe 2021-10-13 1137
d4ae3689226e56 Corentin Labbe 2020-09-25 1138 err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
d4ae3689226e56 Corentin Labbe 2020-09-25 1139 if (err)
d4ae3689226e56 Corentin Labbe 2020-09-25 1140 return -ENODEV;
d4ae3689226e56 Corentin Labbe 2020-09-25 @1141 vb2_dma_contig_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1142
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1143 nr = zoran_num++;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1144 if (nr >= BUZ_MAX) {
b7c3b2bb9db412 Corentin Labbe 2020-09-25 1145 pci_err(pdev, "driver limited to %d card(s) maximum\n", BUZ_MAX);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1146 return -ENOENT;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1147 }
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1148
6d1d9ba2c4396f Corentin Labbe 2020-09-25 1149 zr = devm_kzalloc(&pdev->dev, sizeof(*zr), GFP_KERNEL);
5e195bbddabdd9 Corentin Labbe 2020-09-25 1150 if (!zr)
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1151 return -ENOMEM;
5e195bbddabdd9 Corentin Labbe 2020-09-25 1152
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1153 zr->v4l2_dev.notify = zoran_subdev_notify;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1154 if (v4l2_device_register(&pdev->dev, &zr->v4l2_dev))
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1155 goto zr_free_mem;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1156 zr->pci_dev = pdev;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1157 zr->id = nr;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1158 snprintf(ZR_DEVNAME(zr), sizeof(ZR_DEVNAME(zr)), "MJPEG[%u]", zr->id);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1159 if (v4l2_ctrl_handler_init(&zr->hdl, 10))
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1160 goto zr_unreg;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1161 zr->v4l2_dev.ctrl_handler = &zr->hdl;
8cb356d4eaae11 Corentin Labbe 2020-09-25 1162 v4l2_ctrl_new_std(&zr->hdl, &zoran_video_ctrl_ops,
8cb356d4eaae11 Corentin Labbe 2020-09-25 1163 V4L2_CID_JPEG_COMPRESSION_QUALITY, 0,
8cb356d4eaae11 Corentin Labbe 2020-09-25 1164 100, 1, 50);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1165 spin_lock_init(&zr->spinlock);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1166 mutex_init(&zr->lock);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1167 if (pci_enable_device(pdev))
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1168 goto zr_unreg;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1169 zr->revision = zr->pci_dev->revision;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1170
9bb2720293a04f Corentin Labbe 2020-09-25 1171 pci_info(zr->pci_dev, "Zoran ZR360%c7 (rev %d), irq: %d, memory: 0x%08llx\n",
9bb2720293a04f Corentin Labbe 2020-09-25 1172 zr->revision < 2 ? '5' : '6', zr->revision,
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1173 zr->pci_dev->irq, (uint64_t)pci_resource_start(zr->pci_dev, 0));
9bb2720293a04f Corentin Labbe 2020-09-25 1174 if (zr->revision >= 2)
9bb2720293a04f Corentin Labbe 2020-09-25 1175 pci_info(zr->pci_dev, "Subsystem vendor=0x%04x id=0x%04x\n",
9bb2720293a04f Corentin Labbe 2020-09-25 1176 zr->pci_dev->subsystem_vendor, zr->pci_dev->subsystem_device);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1177
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1178 /* Use auto-detected card type? */
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1179 if (card[nr] == -1) {
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1180 if (zr->revision < 2) {
b7c3b2bb9db412 Corentin Labbe 2020-09-25 1181 pci_err(pdev, "No card type specified, please use the card=X module parameter\n");
b7c3b2bb9db412 Corentin Labbe 2020-09-25 1182 pci_err(pdev, "It is not possible to auto-detect ZR36057 based cards\n");
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1183 goto zr_unreg;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1184 }
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1185
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1186 card_num = ent->driver_data;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1187 if (card_num >= NUM_CARDS) {
b7c3b2bb9db412 Corentin Labbe 2020-09-25 1188 pci_err(pdev, "Unknown card, try specifying card=X module parameter\n");
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1189 goto zr_unreg;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1190 }
daae1da762c1e3 Corentin Labbe 2020-09-25 1191 pci_info(zr->pci_dev, "%s() - card %s detected\n", __func__, zoran_cards[card_num].name);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1192 } else {
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1193 card_num = card[nr];
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1194 if (card_num >= NUM_CARDS || card_num < 0) {
b7c3b2bb9db412 Corentin Labbe 2020-09-25 1195 pci_err(pdev, "User specified card type %d out of range (0 .. %d)\n",
b7c3b2bb9db412 Corentin Labbe 2020-09-25 1196 card_num, NUM_CARDS - 1);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1197 goto zr_unreg;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1198 }
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1199 }
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1200
5e195bbddabdd9 Corentin Labbe 2020-09-25 1201 /*
5e195bbddabdd9 Corentin Labbe 2020-09-25 1202 * even though we make this a non pointer and thus
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1203 * theoretically allow for making changes to this struct
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1204 * on a per-individual card basis at runtime, this is
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1205 * strongly discouraged. This structure is intended to
5e195bbddabdd9 Corentin Labbe 2020-09-25 1206 * keep general card information, no settings or anything
5e195bbddabdd9 Corentin Labbe 2020-09-25 1207 */
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1208 zr->card = zoran_cards[card_num];
5e195bbddabdd9 Corentin Labbe 2020-09-25 1209 snprintf(ZR_DEVNAME(zr), sizeof(ZR_DEVNAME(zr)), "%s[%u]",
5e195bbddabdd9 Corentin Labbe 2020-09-25 1210 zr->card.name, zr->id);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1211
845556fd8027b8 Corentin Labbe 2020-09-25 1212 err = pci_request_regions(pdev, ZR_DEVNAME(zr));
845556fd8027b8 Corentin Labbe 2020-09-25 1213 if (err)
845556fd8027b8 Corentin Labbe 2020-09-25 1214 goto zr_unreg;
845556fd8027b8 Corentin Labbe 2020-09-25 1215
e83bf68b5827e0 Corentin Labbe 2020-09-25 1216 zr->zr36057_mem = devm_ioremap(&pdev->dev, pci_resource_start(pdev, 0), pci_resource_len(pdev, 0));
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1217 if (!zr->zr36057_mem) {
b7c3b2bb9db412 Corentin Labbe 2020-09-25 1218 pci_err(pdev, "%s() - ioremap failed\n", __func__);
845556fd8027b8 Corentin Labbe 2020-09-25 1219 goto zr_pci_release;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1220 }
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1221
ce72671d5d2d93 Corentin Labbe 2020-09-25 1222 result = pci_request_irq(pdev, 0, zoran_irq, NULL, zr, ZR_DEVNAME(zr));
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1223 if (result < 0) {
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1224 if (result == -EINVAL) {
b7c3b2bb9db412 Corentin Labbe 2020-09-25 1225 pci_err(pdev, "%s - bad IRQ number or handler\n", __func__);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1226 } else if (result == -EBUSY) {
b7c3b2bb9db412 Corentin Labbe 2020-09-25 1227 pci_err(pdev, "%s - IRQ %d busy, change your PnP config in BIOS\n",
b7c3b2bb9db412 Corentin Labbe 2020-09-25 1228 __func__, zr->pci_dev->irq);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1229 } else {
b7c3b2bb9db412 Corentin Labbe 2020-09-25 1230 pci_err(pdev, "%s - cannot assign IRQ, error code %d\n", __func__, result);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1231 }
e83bf68b5827e0 Corentin Labbe 2020-09-25 1232 goto zr_pci_release;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1233 }
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1234
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1235 /* set PCI latency timer */
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1236 pci_read_config_byte(zr->pci_dev, PCI_LATENCY_TIMER,
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1237 &latency);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1238 need_latency = zr->revision > 1 ? 32 : 48;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1239 if (latency != need_latency) {
9bb2720293a04f Corentin Labbe 2020-09-25 1240 pci_info(zr->pci_dev, "Changing PCI latency from %d to %d\n", latency, need_latency);
5e195bbddabdd9 Corentin Labbe 2020-09-25 1241 pci_write_config_byte(zr->pci_dev, PCI_LATENCY_TIMER, need_latency);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1242 }
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1243
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1244 zr36057_restart(zr);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1245 /* i2c */
9bb2720293a04f Corentin Labbe 2020-09-25 1246 pci_info(zr->pci_dev, "Initializing i2c bus...\n");
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1247
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1248 if (zoran_register_i2c(zr) < 0) {
b7c3b2bb9db412 Corentin Labbe 2020-09-25 1249 pci_err(pdev, "%s - can't initialize i2c bus\n", __func__);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1250 goto zr_free_irq;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1251 }
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1252
5e195bbddabdd9 Corentin Labbe 2020-09-25 1253 zr->decoder = v4l2_i2c_new_subdev(&zr->v4l2_dev, &zr->i2c_adapter,
5e195bbddabdd9 Corentin Labbe 2020-09-25 1254 zr->card.i2c_decoder, 0,
5e195bbddabdd9 Corentin Labbe 2020-09-25 1255 zr->card.addrs_decoder);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1256
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1257 if (zr->card.i2c_encoder)
5e195bbddabdd9 Corentin Labbe 2020-09-25 1258 zr->encoder = v4l2_i2c_new_subdev(&zr->v4l2_dev, &zr->i2c_adapter,
5e195bbddabdd9 Corentin Labbe 2020-09-25 1259 zr->card.i2c_encoder, 0,
5e195bbddabdd9 Corentin Labbe 2020-09-25 1260 zr->card.addrs_encoder);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1261
9bb2720293a04f Corentin Labbe 2020-09-25 1262 pci_info(zr->pci_dev, "Initializing videocodec bus...\n");
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1263
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1264 if (zr->card.video_codec) {
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1265 codec_name = codecid_to_modulename(zr->card.video_codec);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1266 if (codec_name) {
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1267 result = request_module(codec_name);
b7c3b2bb9db412 Corentin Labbe 2020-09-25 1268 if (result)
b7c3b2bb9db412 Corentin Labbe 2020-09-25 1269 pci_err(pdev, "failed to load modules %s: %d\n", codec_name, result);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1270 }
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1271 }
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1272 if (zr->card.video_vfe) {
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1273 vfe_name = codecid_to_modulename(zr->card.video_vfe);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1274 if (vfe_name) {
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1275 result = request_module(vfe_name);
b7c3b2bb9db412 Corentin Labbe 2020-09-25 1276 if (result < 0)
b7c3b2bb9db412 Corentin Labbe 2020-09-25 1277 pci_err(pdev, "failed to load modules %s: %d\n", vfe_name, result);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1278 }
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1279 }
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1280
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1281 /* reset JPEG codec */
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1282 jpeg_codec_sleep(zr, 1);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1283 jpeg_codec_reset(zr);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1284 /* video bus enabled */
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1285 /* display codec revision */
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1286 if (zr->card.video_codec != 0) {
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1287 master_codec = zoran_setup_videocodec(zr, zr->card.video_codec);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1288 if (!master_codec)
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1289 goto zr_unreg_i2c;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1290 zr->codec = videocodec_attach(master_codec);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1291 if (!zr->codec) {
b7c3b2bb9db412 Corentin Labbe 2020-09-25 1292 pci_err(pdev, "%s - no codec found\n", __func__);
4bae5db2f28d64 Corentin Labbe 2020-09-25 1293 goto zr_unreg_i2c;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1294 }
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1295 if (zr->codec->type != zr->card.video_codec) {
b7c3b2bb9db412 Corentin Labbe 2020-09-25 1296 pci_err(pdev, "%s - wrong codec\n", __func__);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1297 goto zr_detach_codec;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1298 }
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1299 }
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1300 if (zr->card.video_vfe != 0) {
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1301 master_vfe = zoran_setup_videocodec(zr, zr->card.video_vfe);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1302 if (!master_vfe)
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1303 goto zr_detach_codec;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1304 zr->vfe = videocodec_attach(master_vfe);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1305 if (!zr->vfe) {
b7c3b2bb9db412 Corentin Labbe 2020-09-25 1306 pci_err(pdev, "%s - no VFE found\n", __func__);
4bae5db2f28d64 Corentin Labbe 2020-09-25 1307 goto zr_detach_codec;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1308 }
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1309 if (zr->vfe->type != zr->card.video_vfe) {
b7c3b2bb9db412 Corentin Labbe 2020-09-25 1310 pci_err(pdev, "%s = wrong VFE\n", __func__);
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1311 goto zr_detach_vfe;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1312 }
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1313 }
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1314
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1315 /* take care of Natoma chipset and a revision 1 zr36057 */
83f89a8bcbc3c5 Corentin Labbe 2020-09-25 1316 if ((pci_pci_problems & PCIPCI_NATOMA) && zr->revision <= 1)
9bb2720293a04f Corentin Labbe 2020-09-25 1317 pci_info(zr->pci_dev, "ZR36057/Natoma bug, max. buffer size is 128K\n");
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1318
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1319 if (zr36057_init(zr) < 0)
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1320 goto zr_detach_vfe;
61c3b19f7b9eb7 Corentin Labbe 2020-09-25 1321
b564cb6e0bd587 Corentin Labbe 2020-09-25 1322 zr->map_mode = ZORAN_MAP_MODE_RAW;
b564cb6e0bd587 Corentin Labbe 2020-09-25 1323

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (20.83 kB)
.config.gz (39.53 kB)
Download all attachments

2021-10-18 03:48:42

by Corentin LABBE

[permalink] [raw]
Subject: Re: [PATCH v2 04/10] staging: media: zoran: add debugfs

Le Thu, Oct 14, 2021 at 10:37:52AM +0300, Dan Carpenter a ?crit :
> On Wed, Oct 13, 2021 at 06:58:06PM +0000, Corentin Labbe wrote:
> > +config VIDEO_ZORAN_DEBUG
> > + bool "Enable zoran debugfs"
> > + depends on VIDEO_ZORAN
> > + depends on DEBUG_FS
> > + help
> > + Say y to enable zoran debug file.
> > + This will create /sys/kernel/debug/CARD_NAME/debug for displaying
> > + stats and debug information.
>
> Why bother with a CONFIG? Just make it always on?
>

Hello

I love to provides choice to user (and so avoid a dep on DEBUG_FS), even if I think I am the only one remaining user.

> > @@ -1286,6 +1321,12 @@ static int zoran_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> >
> > zr->map_mode = ZORAN_MAP_MODE_RAW;
> >
> > +#ifdef CONFIG_VIDEO_ZORAN_DEBUG
> > + zr->dbgfs_dir = debugfs_create_dir(ZR_DEVNAME(zr), NULL);
> > + debugfs_create_file("debug", 0444,
> > + zr->dbgfs_dir, zr,
> > + &zoran_debugfs_fops);
>
> This whitespace is weird.

Definitively Yes, fixed!

Thanks
Regards

2021-10-18 03:48:45

by Corentin LABBE

[permalink] [raw]
Subject: Re: [PATCH v2 06/10] staging: media: zoran: fusion all modules

Le Thu, Oct 14, 2021 at 11:01:55AM +0300, Dan Carpenter a ?crit :
> On Wed, Oct 13, 2021 at 06:58:08PM +0000, Corentin Labbe wrote:
> > The zoran driver is split in many modules, but this lead to some
> > problems.
> > One of them is that load order is incorrect when everything is built-in.
> >
> > Having more than one module is useless, so fusion all zoran modules in
> ^^^^^^
> "fusion" isn't the right word. It should be "fuse" or even better
> "merge". Same in the subject.
>

Hello

I will use merge, thanks for the suggestion.

> > +static int load_codec(struct zoran *zr, u16 codecid)
> > +{
> > + switch (codecid) {
> > + case CODEC_TYPE_ZR36060:
> > +#ifdef CONFIG_VIDEO_ZORAN_ZR36060
> > + return zr36060_init_module();
> > +#else
> > + pci_err(zr->pci_dev, "ZR36060 support is not enabled\n");
> > + return -EINVAL;
> > +#endif
> > + break;
> > + case CODEC_TYPE_ZR36050:
> > +#ifdef CONFIG_VIDEO_ZORAN_DC30
> > + return zr36050_init_module();
> > +#else
> > + pci_err(zr->pci_dev, "ZR36050 support is not enabled\n");
> > + return -EINVAL;
> > +#endif
> > + break;
> > + case CODEC_TYPE_ZR36016:
> > +#ifdef CONFIG_VIDEO_ZORAN_DC30
> > + return zr36016_init_module();
> > +#else
> > + pci_err(zr->pci_dev, "ZR36016 support is not enabled\n");
> > + return -EINVAL;
> > +#endif
> > + break;
> > + }
>
> Not related to your patch but if load_codec() fails, the probe function
> still calls zoran_setup_videocodec() on the failed codec. It might be
> better to set the codec to zero?
>
> result = load_codec(zr, zr->card.video_codec);
> if (result < 0) {
> pci_err(pdev, "failed to load codec %s: %d\n", codec_name, result);
> zr->card.video_codec = 0;
> }
>

I will rework by adding a video_codec_init/exit, it will help tracking error (current behavour to ignore error is bad).
Furthermore, my patch forgot to add call to all "old module" exits, so dedicated function will be better.

Thanks for the review
Regards

2021-10-18 09:59:36

by Hans Verkuil

[permalink] [raw]
Subject: Re: [PATCH v2 00/10] staging: media: zoran: fusion in one module

Hi Corentin,

I noticed some code review comments from Dan and a kernel test robot issue.
Can you post a v3 fixing those by the end of the week? Next week I will have
access again to my zoran board, so then I can test the v3 series.

BTW, I agree with Dan, just drop the 'Enable zoran debugfs' config option. It's
not worth the additional complexity. Instead, just #ifdef CONFIG_DEBUG_FS
where necessary (in most cases you shouldn't even have to do that since the
since you have dummy debug_fs_* functions if CONFIG_DEBUG_FS isn't set).

Regards,

Hans

On 13/10/2021 20:58, 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.
>
> Regards
>
> 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 (10):
> 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
>
> drivers/staging/media/zoran/Kconfig | 46 +--
> drivers/staging/media/zoran/Makefile | 8 +-
> drivers/staging/media/zoran/videocodec.c | 68 +----
> drivers/staging/media/zoran/videocodec.h | 6 +-
> drivers/staging/media/zoran/zoran.h | 6 +-
> drivers/staging/media/zoran/zoran_card.c | 328 ++++++++++++++-------
> drivers/staging/media/zoran/zoran_driver.c | 5 +-
> drivers/staging/media/zoran/zr36016.c | 24 +-
> drivers/staging/media/zoran/zr36016.h | 2 +
> drivers/staging/media/zoran/zr36050.c | 21 +-
> drivers/staging/media/zoran/zr36050.h | 2 +
> drivers/staging/media/zoran/zr36060.c | 21 +-
> drivers/staging/media/zoran/zr36060.h | 2 +
> 13 files changed, 291 insertions(+), 248 deletions(-)
>

2021-10-25 14:33:30

by Corentin LABBE

[permalink] [raw]
Subject: Re: [PATCH v2 00/10] staging: media: zoran: fusion in one module

Le Mon, Oct 18, 2021 at 11:55:40AM +0200, Hans Verkuil a ?crit :
> Hi Corentin,
>
> I noticed some code review comments from Dan and a kernel test robot issue.
> Can you post a v3 fixing those by the end of the week? Next week I will have
> access again to my zoran board, so then I can test the v3 series.
>
> BTW, I agree with Dan, just drop the 'Enable zoran debugfs' config option. It's
> not worth the additional complexity. Instead, just #ifdef CONFIG_DEBUG_FS
> where necessary (in most cases you shouldn't even have to do that since the
> since you have dummy debug_fs_* functions if CONFIG_DEBUG_FS isn't set).
>

Hello

Ok I started fixing issues and will send V3 this week.

Regards

2021-10-25 14:35:59

by Corentin LABBE

[permalink] [raw]
Subject: Re: [PATCH v2 00/10] staging: media: zoran: fusion in one module

Le Mon, Oct 25, 2021 at 02:45:02PM +0200, Hans Verkuil a ?crit :
> Hi Corentin,
>
> On 13/10/2021 20:58, 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.
> >
> > Regards
> >
> > 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
>
> Something weird is wrong with this series. I have a DC30, but loading this with:
>
> modprobe zr36067 card=3
>
> results in this error message in the kernel log:
>
> [ 58.645557] zr36067: module is from the staging directory, the quality is unknown, you have been warned.
> [ 58.646658] zr36067 0000:03:00.0: Zoran MJPEG board driver version 0.10.1
> [ 58.646793] zr36067 0000:03:00.0: Zoran ZR36057 (rev 1), irq: 18, memory: 0xf4000000
> [ 58.648821] zr36067 0000:03:00.0: Initializing i2c bus...
> [ 58.662420] vpx3220 22-0047: vpx3216b found @ 0x8e (DC30[0])
> [ 58.737445] zr36067 0000:03:00.0: Fail to get encoder
>
> This works before, so why this is now failing is not clear to me.
>
> It does work with 'card=0', but I really have a DC30.
>
> If I test with 'card=0' then the rmmod issue is now solved.

Everything normal, since card 0 does not have encoder.
Could you check that adv7175 is compiled ?

I got the same problem with my DC10+ where saa7110 was not compiled.
This issue was reproduced randomly and I have no explanation. (kconfig problem ?)

Regards

2021-10-25 18:46:39

by Hans Verkuil

[permalink] [raw]
Subject: Re: [PATCH v2 00/10] staging: media: zoran: fusion in one module

Hi Corentin,

On 13/10/2021 20:58, 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.
>
> Regards
>
> 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

Something weird is wrong with this series. I have a DC30, but loading this with:

modprobe zr36067 card=3

results in this error message in the kernel log:

[ 58.645557] zr36067: module is from the staging directory, the quality is unknown, you have been warned.
[ 58.646658] zr36067 0000:03:00.0: Zoran MJPEG board driver version 0.10.1
[ 58.646793] zr36067 0000:03:00.0: Zoran ZR36057 (rev 1), irq: 18, memory: 0xf4000000
[ 58.648821] zr36067 0000:03:00.0: Initializing i2c bus...
[ 58.662420] vpx3220 22-0047: vpx3216b found @ 0x8e (DC30[0])
[ 58.737445] zr36067 0000:03:00.0: Fail to get encoder

This works before, so why this is now failing is not clear to me.

It does work with 'card=0', but I really have a DC30.

If I test with 'card=0' then the rmmod issue is now solved.

Regards,

Hans

>
> Corentin Labbe (10):
> 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
>
> drivers/staging/media/zoran/Kconfig | 46 +--
> drivers/staging/media/zoran/Makefile | 8 +-
> drivers/staging/media/zoran/videocodec.c | 68 +----
> drivers/staging/media/zoran/videocodec.h | 6 +-
> drivers/staging/media/zoran/zoran.h | 6 +-
> drivers/staging/media/zoran/zoran_card.c | 328 ++++++++++++++-------
> drivers/staging/media/zoran/zoran_driver.c | 5 +-
> drivers/staging/media/zoran/zr36016.c | 24 +-
> drivers/staging/media/zoran/zr36016.h | 2 +
> drivers/staging/media/zoran/zr36050.c | 21 +-
> drivers/staging/media/zoran/zr36050.h | 2 +
> drivers/staging/media/zoran/zr36060.c | 21 +-
> drivers/staging/media/zoran/zr36060.h | 2 +
> 13 files changed, 291 insertions(+), 248 deletions(-)
>

2021-10-25 19:04:33

by Hans Verkuil

[permalink] [raw]
Subject: Re: [PATCH v2 00/10] staging: media: zoran: fusion in one module

On 25/10/2021 16:21, LABBE Corentin wrote:
> Le Mon, Oct 25, 2021 at 02:45:02PM +0200, Hans Verkuil a écrit :
>> Hi Corentin,
>>
>> On 13/10/2021 20:58, 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.
>>>
>>> Regards
>>>
>>> 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
>>
>> Something weird is wrong with this series. I have a DC30, but loading this with:
>>
>> modprobe zr36067 card=3
>>
>> results in this error message in the kernel log:
>>
>> [ 58.645557] zr36067: module is from the staging directory, the quality is unknown, you have been warned.
>> [ 58.646658] zr36067 0000:03:00.0: Zoran MJPEG board driver version 0.10.1
>> [ 58.646793] zr36067 0000:03:00.0: Zoran ZR36057 (rev 1), irq: 18, memory: 0xf4000000
>> [ 58.648821] zr36067 0000:03:00.0: Initializing i2c bus...
>> [ 58.662420] vpx3220 22-0047: vpx3216b found @ 0x8e (DC30[0])
>> [ 58.737445] zr36067 0000:03:00.0: Fail to get encoder
>>
>> This works before, so why this is now failing is not clear to me.
>>
>> It does work with 'card=0', but I really have a DC30.
>>
>> If I test with 'card=0' then the rmmod issue is now solved.
>
> Everything normal, since card 0 does not have encoder.
> Could you check that adv7175 is compiled ?

Yes, and it loaded as well (I see it with lsmod).

However, there is no adv7175 on my board, instead it appears to have an ITT MSE3000.
There is no driver for this one (and I don't even think it is an i2c device), so
I suspect that before the driver just continued without encoder support, whereas now
it fails when it can't load the encoder.

Could that be the reason? In the absence of an encoder, I think it should just
continue, esp. since the driver doesn't use the encoder anyway.

Regards,

Hans

>
> I got the same problem with my DC10+ where saa7110 was not compiled.
> This issue was reproduced randomly and I have no explanation. (kconfig problem ?)
>
> Regards
>

2021-10-25 19:19:43

by Corentin LABBE

[permalink] [raw]
Subject: Re: [PATCH v2 00/10] staging: media: zoran: fusion in one module

Le Mon, Oct 25, 2021 at 05:13:04PM +0200, Hans Verkuil a ?crit :
> On 25/10/2021 16:21, LABBE Corentin wrote:
> > Le Mon, Oct 25, 2021 at 02:45:02PM +0200, Hans Verkuil a ?crit :
> >> Hi Corentin,
> >>
> >> On 13/10/2021 20:58, 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.
> >>>
> >>> Regards
> >>>
> >>> 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
> >>
> >> Something weird is wrong with this series. I have a DC30, but loading this with:
> >>
> >> modprobe zr36067 card=3
> >>
> >> results in this error message in the kernel log:
> >>
> >> [ 58.645557] zr36067: module is from the staging directory, the quality is unknown, you have been warned.
> >> [ 58.646658] zr36067 0000:03:00.0: Zoran MJPEG board driver version 0.10.1
> >> [ 58.646793] zr36067 0000:03:00.0: Zoran ZR36057 (rev 1), irq: 18, memory: 0xf4000000
> >> [ 58.648821] zr36067 0000:03:00.0: Initializing i2c bus...
> >> [ 58.662420] vpx3220 22-0047: vpx3216b found @ 0x8e (DC30[0])
> >> [ 58.737445] zr36067 0000:03:00.0: Fail to get encoder
> >>
> >> This works before, so why this is now failing is not clear to me.
> >>
> >> It does work with 'card=0', but I really have a DC30.
> >>
> >> If I test with 'card=0' then the rmmod issue is now solved.
> >
> > Everything normal, since card 0 does not have encoder.
> > Could you check that adv7175 is compiled ?
>
> Yes, and it loaded as well (I see it with lsmod).
>
> However, there is no adv7175 on my board, instead it appears to have an ITT MSE3000.
> There is no driver for this one (and I don't even think it is an i2c device), so
> I suspect that before the driver just continued without encoder support, whereas now
> it fails when it can't load the encoder.
>
> Could that be the reason? In the absence of an encoder, I think it should just
> continue, esp. since the driver doesn't use the encoder anyway.
>

So probably the card list is wrong against DC30.
I checked high resolution photo of DC30 on internet, and it confirms the fact that DC30 does not have adv7175.

Since DC30 and DC30+ are identical in the card list, perhaps it is a very old copy/paste error.

So I will add a patch removing adv7175 from DC30.

Thanks for the report
Regards

2021-11-02 17:44:02

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH v2 04/10] staging: media: zoran: add debugfs

On Sun, Oct 17, 2021 at 10:05:06PM +0200, LABBE Corentin wrote:
> Le Thu, Oct 14, 2021 at 10:37:52AM +0300, Dan Carpenter a ?crit :
> > On Wed, Oct 13, 2021 at 06:58:06PM +0000, Corentin Labbe wrote:
> > > +config VIDEO_ZORAN_DEBUG
> > > + bool "Enable zoran debugfs"
> > > + depends on VIDEO_ZORAN
> > > + depends on DEBUG_FS
> > > + help
> > > + Say y to enable zoran debug file.
> > > + This will create /sys/kernel/debug/CARD_NAME/debug for displaying
> > > + stats and debug information.
> >
> > Why bother with a CONFIG? Just make it always on?
> >
>
> Hello
>
> I love to provides choice to user (and so avoid a dep on DEBUG_FS), even if I think I am the only one remaining user.

Sorry, for the delay, I was on vacation.

No, there is no depends on DEBUG_FS in the method that I am describing.

How that works is when DEBUG_FS is turned on then it's on for everything,
but when it's disabled it's disabled for everything. You do not need
the "depends on DEBUG_FS" and if you make this an option the it feels
like it should be a selects DEBUG_FS instead.

How this normally works is that when you have debugfs disabled, there
are dummy files in the debugfs .h files. I bet the compiler can tell
most of those are empty and removes them. So if you have DEBUG_FS then
it doesn't use that much more memory than when VIDEO_ZORAN_DEBUG is
disabled.

I don't know if I'm being clear at all #jetlag.

It should be easy to check. Just remove the "depends on DEBUG_FS" and
enable VIDEO_ZORAN_DEBUG. Disable DEBUG_FS. It should still build fine
because of the dummy functions. That's build 1. Then disable
VIDEO_ZORAN_DEBUG and that's build 2. See how much memory difference
there is between 1 and 2.

regards,
dan carpenter

2021-11-02 21:35:04

by Corentin LABBE

[permalink] [raw]
Subject: Re: [PATCH v2 04/10] staging: media: zoran: add debugfs

Le Tue, Nov 02, 2021 at 08:40:28PM +0300, Dan Carpenter a ?crit :
> On Sun, Oct 17, 2021 at 10:05:06PM +0200, LABBE Corentin wrote:
> > Le Thu, Oct 14, 2021 at 10:37:52AM +0300, Dan Carpenter a ?crit :
> > > On Wed, Oct 13, 2021 at 06:58:06PM +0000, Corentin Labbe wrote:
> > > > +config VIDEO_ZORAN_DEBUG
> > > > + bool "Enable zoran debugfs"
> > > > + depends on VIDEO_ZORAN
> > > > + depends on DEBUG_FS
> > > > + help
> > > > + Say y to enable zoran debug file.
> > > > + This will create /sys/kernel/debug/CARD_NAME/debug for displaying
> > > > + stats and debug information.
> > >
> > > Why bother with a CONFIG? Just make it always on?
> > >
> >
> > Hello
> >
> > I love to provides choice to user (and so avoid a dep on DEBUG_FS), even if I think I am the only one remaining user.
>
> Sorry, for the delay, I was on vacation.
>
> No, there is no depends on DEBUG_FS in the method that I am describing.
>
> How that works is when DEBUG_FS is turned on then it's on for everything,
> but when it's disabled it's disabled for everything. You do not need
> the "depends on DEBUG_FS" and if you make this an option the it feels
> like it should be a selects DEBUG_FS instead.
>
> How this normally works is that when you have debugfs disabled, there
> are dummy files in the debugfs .h files. I bet the compiler can tell
> most of those are empty and removes them. So if you have DEBUG_FS then
> it doesn't use that much more memory than when VIDEO_ZORAN_DEBUG is
> disabled.
>
> I don't know if I'm being clear at all #jetlag.
>
> It should be easy to check. Just remove the "depends on DEBUG_FS" and
> enable VIDEO_ZORAN_DEBUG. Disable DEBUG_FS. It should still build fine
> because of the dummy functions. That's build 1. Then disable
> VIDEO_ZORAN_DEBUG and that's build 2. See how much memory difference
> there is between 1 and 2.
>

No worry for the delay.
Anyway, I have removed VIDEO_ZORAN_DEBUG in v3 since Hans Verkuil also asked for its removing.

Regards