Re: DD support improvements (was: Re: [PATCH v3 00/13] stv0367/ddbridge: support CTv6/FlexCT hardware)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am Tue, 20 Jun 2017 16:10:43 -0300
schrieb Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxxxxx>:

> Em Tue, 20 Jun 2017 20:41:21 +0200
> Daniel Scheller <d.scheller.oss@xxxxxxxxx> escreveu:
> 
> > Not sure about Thomas, but I know that Jasmin doesn't own and/ore uses
> > such cards. However, for upcoming patches, I'll try to drag people to
> > the list for some comments, thanks for the pointer.  
> 
> Yeah, if you can drag people to help reviewing/testing (and even
> coding), that would be really cool, as we'll be able to better
> do our reviews.

For the upcoming bigger changes around ddbridge and new demod/tuner drivers, I'll try to do so. Though this would mostly be for Tested-By's, but still.


> > - Maybe for 4.14: Support for the CineS2 V7 and FlexV4 line of
> >   cards/modules. This mainly involves a new demod driver (stv0910) and
> >   a new tuner driver (stv6111). Permissions for this are cleared with
> >   Ralph already. The glue code needed in ddbridge is rather easy, and
> >   as some ground work (mostly the MachXO2 support from the 2841 series)
> >   is now in, the changes are quite small. Patches are ready so far but
> >   need more cleanup (checkpatch fixes, camel case and such things).  
> 
> Please try to sync it with Ralph, in a way that his code won't
> diverge from the upstream one, as this will make easier for both
> sides to keep the Kernel in track with driver improvements.

This is not going to work. DD (Ralph and Manfred Voelkel) sort of maintain a shared code base between their Windows driver and the Linux kernel driver sources. While they didn't explicitely stated this, this can be noticed by the remarks and commented code in their OSS code, and the commit messages in their dddvb GIT (e.g. "sync with windows driver"). I've already cleaned up a lot of this (I believe no one wants to see such stuff in the linux kernel tree). If we're additionally going to replace all things camel case, with some more cleaning and maybe code shuffling after like a V4 patch series, those two sources are out of sync in a way that no automatic sync by applying patches will be possible anymore. So, pushing from vendor's upstream to the kernel seems to be the only option, and in fact, if the whole driver/package stack completely lives in the kernel sources, maybe DD even decide to directly commit upstream (kernel) again.

Putting Ralph into "To:", really like to hear an opinion from him on this whole subject.

> > - The "big" ddbridge update. I'm thinking of two ways to do this:
> > 
> >   * Do this in one commit, being a huge code bump, bringing ddbridge to
> >     version 0.9.28 (as per vendor versioning). This is mostly ready and
> >     successful in use by many testers and in my Gentoo ddbridge kernel
> >     sources overlay. Should get some more cleanups though (still some
> >     GTL link bits left which are not needed), and all fixes which went
> >     to the in-kernel driver like __user annotations need to be put back
> >     in. Big drawback: A mess to review.
> > 
> >   * Try to tear apart most if not all upstream vendor driver tar
> >     archives and recreate individual patches out of this. For
> >     reference, we need to go from what is in the kernel which is
> >     something inbetween v0.5 and v0.8 up to and including version
> >     0.9.29. I'm currently working on this from time to time, and I can
> >     assure you that this is an extremely tedious and unthankful thing
> >     to do (currently nearly done with 0.9.9b, approx. 20 releases
> >     left). This might be better to review, but this will also result in
> >     something like 100-200 commits, without guarantee of having
> >     everything correct.  
> 
> The second approach is preferred, but, as you said, it is a very
> complex task, and has bad effect that, at the time you're updating
> it, the DD driver will be changed.

I'd welcome it if we could put approach two somewhat aside for at least the reasons outlined :)

> The first approach will require some things to work, though:
> 
> - the "legacy" driver should be kept at the Kernel for some time,
>   in order to provide a "fall back" for those that find issues with
>   the new version;

>From what can be gathered from forums and such, the current ("upstream") version doesn't show some (if any) issues. Though I'm aware that MSI interrupts are still an issue on a lot of hardware platforms, but disabling that option by default in the driver and toggling this via a Kconfig option works around this, since w/o MSI things work like a charm. Thats a problem that needs to be resolved by the vendor though.

Still, I understand you that you'd like to keep this around. Not exactly sure though how to achieve this in detail. Renaming media/pci/ddbridge to e.g. media/pci/ddbridge-legacy is the easy part, but if we don't want to mix up commits, one point in the commit history remains where there's a driver named ddbridge-legacy and no ddbridge (driver missing) exists (commit: "rename ddbridge to ddbridge-legacy", commit+1: "import updated and cleaned ddbridge driver from vendor package"). Also, it must be made sure that both drivers at best won't be built at the same time due to overlapping PCI IDs, possibly leading to additional issues on users installs.

> - you'll still need to patch DD tree, as I'm pretty sure there are
>   changes on the upstream driver that will need to be ported there;

The same as for the stv0910 code applies here, in addition that it's not sure if DD even wants this. DD even has KERNEL_VERSION_CODE ifdefs in some places. And - most importantly - they carry around an old version of the DVB core API (from what it looks, around linux-3.10, not exactly sure) which even was modified by some IOCTLs, vars, defines and the netstream and modulator support. I managed to remove all core API change deps from everything tuner related (and thats the reason things work in harmony with and in current kernels), but getting all this over to DD or even merge things from DD into the media subsystem will get "interesting".

> - This is a very painful thing to do. While I might accept do it
>   once, I won't accept repeat it again a second time. So, if we do
>   that, we need to agree with you and Ralph that any change at the
>   DD tree will be submitted ASAP upstream, in order to avoid future
>   gaps.

Well, I can offer you: For the new STV0910, STV6111 and MXL5XX, as well as ddbridge itself when it's done, I can take over maintainership, in a way that when dddvb upstream changes emerge, I'll take care of submitting (relevant) things to linux-media quickly (as my work and private time schedule allows). However, since I'm not too far into all things kernel and DVB core yet, I would need help on other things from others.

This task is definitely doable when we get to a point that the current bridge code is in the kernel tree. DD recently started to publish driver changes in small increments, which - if you know a few things about the internals and know what differs in both versions - is a rather easy task.

Yet, to achieve this, I'd really propose doing the "big bang" once and do the final cleanup during that process (and yes, the cleanup IS neccessary!).

> > If you like, I'm happy and very open to discuss this further with you!  
> 
> Feel free to do it ;)

On it :)

> > BTW, you might have seen this - I posted four more patches which'll add
> > DVBv5 signal stats to the DDB part of the stv0367 code. If you don't
> > mind doing a quick inbetween-review of this, this would be nice if this
> > could go in alongside the DD support (so, for 4.13 aswell).  
> 
> I intend to do more patch reviews this week. I'll try to take a look
> on them.

Series already posted as v2 with (most of) the remarks from Antti addressed.

Best regards,
Daniel Scheller
-- 
https://github.com/herrnst




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux