Re: [Mjpeg-users] [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/4f89b139-13b7-eee6-9662-996626b77...@xs4all.nl/ > > 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 ___ Mjpeg-users mailing list Mjpeg-users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mjpeg-users
Re: [Mjpeg-users] [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/4f89b139-13b7-eee6-9662-996626b77...@xs4all.nl/ 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(-) > ___ Mjpeg-users mailing list Mjpeg-users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mjpeg-users
Re: [Mjpeg-users] [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/4f89b139-13b7-eee6-9662-996626b77...@xs4all.nl/ 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 ___ Mjpeg-users mailing list Mjpeg-users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mjpeg-users
Re: [Mjpeg-users] [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 +, 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 ___ Mjpeg-users mailing list Mjpeg-users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mjpeg-users
