Message-ID: <be6ac929-2443-ff55-3e11-6a86d6472e0e@web.de>
Date:   Mon, 23 Aug 2021 16:59:33 +0200
From:   Soeren Moch <smoch@....de>
To:     Mauro Carvalho Chehab <mchehab+huawei@...nel.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Linux Media Mailing List <linux-media@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [Regression 5.14] media: dvb userspace api



On 22.08.21 19:47, Mauro Carvalho Chehab wrote:
> Em Sun, 22 Aug 2021 17:21:41 +0200
> Soeren Moch <smoch@....de> escreveu:
>
>>> There's no regression: a legacy driver (av7110) for a device that stopped
>>> being manufactured 15 years ago and that doesn't work anymore with current
>>> Digital TV transmissions was removed, together with the API that it was
>>> implemented inside such driver's code.
>> What you write here is simply not true.
>>
>> The "device" (saa7146/av7110-based full-featured DVB card) is working
>> well.
> Probably not true - at least for some av7110-based boards - as there was a
> regression that no user ever noticed:
>
> 	https://lore.kernel.org/lkml/20210503115736.2104747-59-gregkh@linuxfoundation.org/
Did you read the patch? Ignoring errors may be wrong, but this causes no
regression for any user because i2c transfers to the frontend simply
just succeed always in normal operation.

BTW, for me this is another mess you created here. Why did you move the
frontend driver out of the dvb-frontends directory into the av7110
driver, that as such is totally independent from the sp8870 frontend?
>
> This was noticed only too late, due to a review of the offended patch
> caused by "hypocrite commits"[1].
>
> [1] https://lwn.net/Articles/854645/.
All this is totally unrelated to this regression report on the DVB
userspace API.
>
>> Also with current Digital TV transmissions (e.g. Astra Satellite
>> TV in Europe). The DVB API never was implemented in driver's code, it is
>> linux userspace API (include/uapi/linux/dvb/).
> The DVB API used by all upstream drivers is implemented inside the DVB
> core (at drivers/media/dvb-core/).
Simply not true.
The headers in include/uapi/linux/dvb/ define the DVB userspace API.
Parts of this API have only one in-tree user: the saa7146/av7110
driver.  Other parts are used from many drivers for tens, probably
hundreds, of different hardware devices, so common helper functions in
dvb-core make sense.
>
> The "full-featured" API consists on the DVB API + some extra ioctls
> defined at the uAPI headers, plus an av7110 implementation, which
> covered only part of the ioctls that were defined at the headers.
The DVB API is a well-designed API, in contrast to what you repeatedly
claim designed independently from special hardware requirements.
There are several parts of the API (frontend, dmx, ca, net, audio,
video, osd). Hardware devices implementing all related functionality are
called "full-featured cards" (in contrast to budget cards or e.g. all
types of DVB sticks that usually implement input functionality related
to the frontend and dmx part of the API, there is no "full-featured API").
All defined API calls are integral part of the API, there are no "extra
ioctls" just because you personally like some API calls more than others.

Yes, not all defined API calls had been used by in-tree drivers. You
already removed these API call definitions some time ago, what causes
real pain for the users of the out-of-tree saa716x driver with no
advantage for in-tree linux-media. But at least this did not cause
regressions for in-tree drivers and the related userspace applications.

This time you removed API call definitions that are used by the in-tree
saa7146/av7110 driver. This causes regressions for the users of this
in-tree driver, because the related userspace-application stops to
build. This exactly is what this regression report is about.
>
>> You moved files out of the uapi part of kernel headers, parts of e.g.
>> RedHat userspace stops to build due to this. So it is a userspace
>> regression.
> Again, this is not a Kernel regression.
Yes, it is a userspace regression caused by your changes of the linux
DVB userspace API.
> There isn't a single bit of
> code inside the Kernel that it would require the "full-feat" uapi.
This is obviously wrong, since the in-tree saa7146/av7110 driver
implements the removed API call definitions.
>
> If an app implements support to some OOT driver(s), it has to carry on the
> OOT userspace code for such driver(s), together with any needed headers for
> such support. So, you should submit a patch to such app maintainers
> directly - and/or to the distro packages that would have packages
> providing support for such OOT driver(s).
>
> Btw, as far as I'm aware, Red Hat's Kernels don't come with the
> saa716x OOT driver.
Mauro, please stop spreading FUD.
This regression report is about the part of the DVB API that is
implemented by the in-tree saa7146/av7110 driver and it's related userspace.
Both are part of the RedHat linux distribution.
>
>>> The av7110 production stopped ~15 years ago.
>> But the cards work perfectly well. I own two such cards, there is no
>> problem at all with them. Other members of the community even test with
>> -rc3 kernels and file RedHat bugs. So there clearly is interest in them.
> Nobody is telling otherwise. If people want to use OOT drivers, that's
> OK. Nobody is preventing people to use it, nor to use the apps developed
> for such devices.
This discussion here is about av7110, see the 3rd-level citation "The
av7110 production stopped ~15 years ago." above.
And the saa7146/av7110 driver is in-tree, and the related userspace
experences regressions that I report here.
>
> Keeping av7110 in-kernel has been a waste of limited upstream development
> resources.
Simply not true.
The driver is used, therefore it is not wasted time to maintain it.
And for years I repeatedly offered to do this maintenance, transfer
maintainer-ship and you immediately git rid of this "overwhelming
burden" to maintain this driver.
As additional advantage I understand the driver and related API, do the
required maintenance for the similar saa716x driver (out-of-tree)
anyway, and have hardware to test (for both drivers).
> A couple of years ago, we needed to fix the av7110 API due to
> year-2038 issues.
Simply not true.
Nothing in this driver or the DVB API is related to wall-clock time.
Only DVB timestamps are used. So there is no year-2038 issue.
> From time to time, we get bugs affecting it (even security
> ones), as the code has been bit-rotting for a long time.
Simply not true.
Oliver Endriss did a great job in maintaining this driver, and it still
works perfectly fine.
> The most recent one
> probably broke the driver without nobody noticing it for a couple of Kernel
> reviews, as mentioned above.
No, see above. The linked patch caused a bug, but no user-visible
regression.
And this DVB-T frontend is optional anyway and not very popular.
>
>>> This is a legacy hardware, which supports only the first generation of DVB
>>> standards, and had an integrated MPEG-2 decoder. As most DVB transmissions
>>> use MPEG4 or newer encoding schemas that didn't exist back in 1999, it
>>> doesn't make any sense keeping such driver upstream nowadays.
>> As I wrote in my previous email: most private TV stations in Germany
>> provide their free-to-air satellite programs MPEG-2 encoded only.
>> Therefore this is very popular and it absolutely makes sense to keep
>> this driver upstream.
> It is probably a lot easier to get a modern DVB "budget" card with
> supports not only MPEG-2 but all encoding standards than to find an
> old "full-feat" DVB card with av7110 those days.
Maybe. But when the most popular satellite TV provider broadcasts MPEG-2
encoded video and people are happy with their working cards, why throw
away a running system? Besides that, people usually use full-featured
cards together with additional modern budget cards/sticks to be able to
record different DVB programs while seeing a different program as liveview.
>
> Who still provides saa71xx chips those days? As far as I'm aware,
> Philips semiconductors (who used to produce such chipsets) was split
> into NXP in 2006, and the entire digital TV chipset line moved
> altogether. I can't find any references those days to any saa71xx
> at either NXP or Philips websites.
This documentation is only provided with non-disclosure agreement.
Luckily I received such documentation under NDA together with firmware
code and schematics. So I am in a very good position to maintain this.
>>> The API that got removed was written to control the av7110 MPEG-2 decoder,
>>> and was never integrated at the DVB core: the av7110 had a driver-specific
>>> implementation inside its code.
>> This is simply not true.
>> The DVB API is linux userspace API, absolutely nothing driver specific
>> inside driver's code.
> From upstream PoV, it is an av7110-specific API, as all in-kernel support
> was inside av7110 driver's code.
And from userspace point-of-view ist is linux userspace API.
>
>>> The saa716x driver you're mentioned is an out of tree driver.
>>> We don't keep APIs at the upstream Kernel due to OOT drivers.
>> Strong words to distract from the main point:
>> This regression report is about upstream DVB userspace API and the
>> saa7146/av7110 driver, both part of mainline linux for a long time.
> Removing a legacy driver is not a regression.
This in-tree driver is actively used by a whole community and works
perfectly fine. The corresponding userspace applications are in many
major linux distributions. Why are you fighting so hard to remove this
driver and kill the community and the userspace behind it?

If linux would only contain drivers for hardware that is personally used
by subsystem maintainers, then I think this would be a very poor user
experience.
> See, you're the one
> trying to distract from the main point:
>
> The saa716x driver is OOT. There was never any upstream support
> for it. None of the patches you're mentioning prevents anyone
> to keep building it as an OOT driver.
>
> What it was removed was the av7110, together with the API header
> files that were used only by this single driver.
And the userspace. And this causes regressions, e.g. in RedHat. And this
is what this regression report is about.
>
>> you stripped almost everything from my previous email, you did not
>> answer any questions or gave any explanation for your behavior.
> I striped the points already discussed with you when I gave you
> feedback about the saa716x patchset [2], as this is a completely
> separate discussion than the removal of av7110 support.
It's indeed a separate discussion. But you brought up this topic here.
If someone wants to learn how something works and why it is implemented
the way it is, I'm happy to explain. Unfortunately you always
immediately shoot down this discussion with: implement something else,
no explanation why, no technical discussion about pros and cons of
alternative implementations.
>
> In summary, it makes no sense to claim that saa716x OOT driver
> broke because a different driver was removed upstream.
I nowhere claimed that. It's not me who is bending truth and twisting
words all the time.
>
> Thanks,
> Mauro
>
> [2] https://lore.kernel.org/linux-media/20180307121438.389f411c@vento.lan/
Mauro, please explain why you act as you do, especially as this is so
totally different from what I know from other linux subsystems.

There is a great DVB API and lots of drivers implementing it upstream
for decades. There is a community behind it and related userspace in
many major linux distributions. Working hardware also in newer generations.

You do not understand this API (as you wrote yourself), but you kill all
technical discussions immediately when I explain details.

You write maintaining this DVB API and drivers is "waste of limited
upstream development resources", but you completely ignore my offer to
take over maintainer-ship and kill every reference to this offer
immediately in your answers. On the other side there is apparently
plenty of time for whitespace cleanup, comment cleanup, character
encoding cleanup, documentation reference fixes. There is nothing wrong
with this, but I think it's a bad excuse for not having time to maintain
DVB drivers (for what you are paid according to the MAINTAINERS entry).

You recommend to maintain drivers out-of-tree, just because you don't
like the API the driver implements, although this API is part of linux
media for decades. No technical discussions, no idea how to support the
community.

Now you try to kill working in-tree drivers, you cause regressions for
existing userspace applications. No explanation of any reasons, no idea
how to solve this issue. No effort to work with the community.
No progress here in all the emails in this thread. For me every answer
sounds like: I do what I want anyway, shut up! Or which point I missed here?


Linus,

Is what I described directly above the new linux maintenance policy?  Or
is linux media a private kingdom where the community should keep away?
Is this a place where the subsystem maintainer is on a mission to
destroy everything instead of maintaining and improving it? Please tell
me what I understood wrong here.

Thanks,
Soeren

Powered by blists - more mailing lists