Re: [PATCH 2/3] media: si2157: add support for 1.7MHz and 6.1 MHz

[Mauro Carvalho Chehab <mchehab@kernel.org>]

Em Fri, 7 Jan 2022 11:38:37 +0100
Robert Schlabbach <robert_s@gmx.net> escreveu:

> "Mauro Carvalho Chehab" <mchehab@kernel.org> wrote:
> 
> > As the DVB core already stores the bandwidth used to tune at props,
> > since the introduction of DVBv5 API, any get calls will return the
> > tuned bandwidth.  
> 
> No, not the _tuned_ bandwidth, the "requested" bandwidth, that was
> estimated. I see no value in that information, as the user app can
> easily calculate that by itself. This is not information that the
> kernel or driver needs to provide, as it is solely derived from
> the information the application has given.
> 
> Whereas the _actually applied_ bandwidth filter is an information
> that only the tuner driver can deliver. For example, there are 5MHz
> DVB-T2 channels, but the si2157 only offers a 6MHz bandwidth filter.
> 
> What should get_frontend() return, the requested/nominal 5MHz, or
> the actually used 6MHz?
> 
> Reading the include file, the answer seems clear to me:
> 
> https://git.linuxtv.org/media_tree.git/tree/include/media/dvb_frontend.h
> 
> > * @get_frontend:	callback function used to inform the parameters
> > *			actuall in use.  
> 
> So following that documentation, I would say the actually used 6MHz
> should be put into the property cache by that callback.

For the better or for the worse, the userspace API clearly states that,
for DVB-C/S/S2, it should return the estimated bandwidth, calculated from
Roll-off and signal rate. So, at least for such delivery systems,
props->bandwidh_hz should not be touched by drivers.

There's actually a couple of reasons why bandwidth_hz is calculated
at the core:

1. It helps to have the minimum required bandwidth filter on a single
   place at the core;
2. It helped to address issues with DVB-C Annex-C;
3. It is interesting to allow checking what bandwidth the Kernel actually
   requested the driver, when debugging them. I used this a couple of
   times when fixing Annex-C support on some drivers (I used to have
   a DVB-C Annex-C 6MHz-spaced cable).

For DVB-T/T2, it is even worse, as the properties returned by
FE_GET_PROPERTY are used to store the channel properties to userspace
by tools like dvbv5-scan. Those should reflect the channel 
properties/requirements, not the actual bandwidth filter applied by the
tuner, as the same channel configs can be used by different tuners.

So, a change like that for DVB-T/T2 can actually cause regressions and
bad channel reports when an user is sending patches to the DTV channel
settings on this git tree:

	https://git.linuxtv.org/dtv-scan-tables.git/

-

Now, get_frontend is an internal kAPI. If I'm not mistaken, it was
designed to be used between tuners and demods, for the cases where the
demod would need to know what was the bandwidth set at the tuner.
So, I'm OK if it returns the actually applied bandwidth, provided that
such value is not returned to userspace via DTV_BANDWIDTH_HZ. So,
props->bandwidth_hz should not be updated after its call.

Now, if you think it would be worth to pass the actual bandwidth set
by the hardware to userspace (frankly, I don't know why userspace
might need it), then another DTV property is needed.

Regards,
Mauro