On Sun, 22 Feb 2009, Hans Verkuil wrote:
> Please pull from http://www.linuxtv.org/hg/~hverkuil/v4l-dvb-zoran for the
> following:
I have some questions about your changes.
> - zoran: convert to video_ioctl2 and remove 'ready_to_be_freed' hack.
It looks like this patch deleted the code relating to this comment:
* We don't free the buffers right on munmap() because that
* causes oopses (kfree() inside munmap() oopses for no
* apparent reason - it's also not reproduceable in any way,
* but moving the free code outside the munmap() handler fixes
* all this... If someone knows why, please explain me (Ronald)
Do you have an explanation of why it's safe to do this? There's nothing in
the patch description (there is no patch description) about it.
> - zoran: remove broken BIGPHYS_AREA and BUZ_HIMEM code, and allow for
> kmallocs > 128 kB
It looks like the last time the bigphys_area patch was updated was to
2.6.11 so there probably isn't much point is supporting that anymore. But
is the highmem code broken? Trying to allocate multiple megabyte buffers
on systems without gigabytes of memory doesn't work very well. You get
nice things like this in your kernel log:
tvtime: page allocation failure. order:8, mode:0x40d0
[<b0138243>] __alloc_pages+0x29b/0x2aa
[<b014a371>] __slab_alloc+0x192/0x343
> - zoran: use slider flag with volume etc. controls.
Volume control? zoran has no audio.
> - zoran: fix field typo.
Why not merge this patch into the patch that created the typo? It only
takes seconds if you use the features of modern SCMs.
> - zoran: set bytesperline to 0 when using MJPEG.
Why not merge this patch into the one that created the bug?
> - zoran: fix G_FMT
You sure about that? Each jpeg buffer only has one field.
> - zoran: if reqbufs is called with count == 0, do a streamoff.
Did you mean to change the debug level of those printks? It's not in your
patch description.
> - zoran: TRY_FMT and S_FMT now do the same parameter checks.
Is this is mistake that was left off of a previous patch? No patch
description so I'm not sure.
> - bt819: convert to v4l2_subdev.
> - bt819: that delay include is needed after all.
Can these two not be folded?
> - zoran: convert to v4l2_device/v4l2_subdev.
+static const unsigned short adv717x_addrs[] = { 0x6a, 0x6b, 0x2a, 0x2b,
I2C_CLIENT_END };
adv7171/5 are at 0x2a and 0x2b, while adv7170 is at 0x6a and 0x6b.
> - zoran: increase bufsize to a value suitable for 768x576.
How about folding this into the first patch that changed v4l_bufsize to
810?
> This is a huge patch series that brings the zoran driver completely up to
> date with the latest v4l2 framework. In particular it converts it to use
> video_ioctl2, the communication with the i2c modules is converted to from
> v4l1 to v4l2 and that made the conversion to v4l2_device/v4l2_subdev
> possible.
>
> As a result of this the saa7111.c and saa7114.c drivers are removed and
> instead the saa7115.c driver is used which can handle these older saa711x
> devices as well.
Any figures for driver size before and after?
> In addition the zoran driver contained experimental support for highmem
> allocations which relied on a 'big phys area' kernel patch that was never
> merged into the kernel, and the only reason this was needed at all is that
bigphys_area and highmem are different. highmem lets you leave off some
memory by giving the kernel a mem parameter and the zoran driver attempts
to access it via ioremap(). bigphys_area was a patch that never made it
into the kernel.
> a long time ago kmalloc could only allocate up to 128 kB of contiguous
> memory. These days those limitations are gone, and all this code could be
Even though kmalloc supports higher order allocations now, they can easily
fail.
> behaviors with i2c (misdetections and not detecting decoders at all
> sometimes) are now fixed. The v4l2 behavior has also been improved
> considerably.
Any examples of what's improved?
> What is not working is playback of video. However, it turns out that that
> has been broken for some time now. The hope is that now that the driver has
> been cleaned up someone can actually go in and try to discover what broke.
> Note that video pass-through is working fine.
I tried playback with the driver from last month and it appears to work
fine.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
