On 27-12-2011 12:33, Andreas Oberritter wrote:
> On 27.12.2011 14:28, Mauro Carvalho Chehab wrote:
>> On 27-12-2011 10:11, Andreas Oberritter wrote:
>>> On 27.12.2011 02:07, Mauro Carvalho Chehab wrote:
>>>> DVB-S and DVB-T, as those were the standards supported by DVBv3.
>>>
>>> The description seems to be incomplete.
>>>
>>>> New standards like DSS, ISDB and CTTB don't fit on any of the
>>>> above types.
>>>>
>>>> while there's a way for the drivers to explicitly change whatever
>>>> default DELSYS were filled inside the core, still a fake value is
>>>> needed there, and a "compat" code to allow DVBv3 applications to
>>>> work with those delivery systems is needed. This is good for a
>>>> short term solution, while applications aren't using DVBv5 directly.
>>>>
>>>> However, at long term, this is bad, as the compat code runs even
>>>> if the application is using DVBv5. Also, the compat code is not
>>>> perfect, and only works when the frontend is capable of auto-detecting
>>>> the parameters that aren't visible by the faked delivery systems.
>>>>
>>>> So, let the frontend fill the supported delivery systems at the
>>>> device properties directly, and, in the future, let the core to use
>>>> the delsys to fill the reported info::type based on the delsys.
>>>>
>>>> Signed-off-by: Mauro Carvalho Chehab <mchehab@xxxxxxxxxx>
>>>> ---
>>>> drivers/media/dvb/dvb-core/dvb_frontend.c | 13 +++++++++++++
>>>> drivers/media/dvb/dvb-core/dvb_frontend.h | 8 ++++++++
>>>> 2 files changed, 21 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/drivers/media/dvb/dvb-core/dvb_frontend.c b/drivers/media/dvb/dvb-core/dvb_frontend.c
>>>> index 8dedff4..f17c411 100644
>>>> --- a/drivers/media/dvb/dvb-core/dvb_frontend.c
>>>> +++ b/drivers/media/dvb/dvb-core/dvb_frontend.c
>>>> @@ -1252,6 +1252,19 @@ static void dtv_set_default_delivery_caps(const struct dvb_frontend *fe, struct
>>>> const struct dvb_frontend_info *info = &fe->ops.info;
>>>> u32 ncaps = 0;
>>>>
>>>> + /*
>>>> + * If the frontend explicitly sets a list, use it, instead of
>>>> + * filling based on the info->type
>>>> + */
>>>> + if (fe->ops.delsys[ncaps]) {
>>>> + while (fe->ops.delsys[ncaps] && ncaps < MAX_DELSYS) {
>>>> + p->u.buffer.data[ncaps] = fe->ops.delsys[ncaps];
>>>> + ncaps++;
>>>> + }
>>>> + p->u.buffer.len = ncaps;
>>>> + return;
>>>> + }
>>>> +
>>>
>>> I don't understand what this is trying to solve. This is already handled
>>> by the get_property driver callback.
>>>
>>> dtv_set_default_delivery_caps() only sets some defaults for drivers not
>>> implementing get_property yet.
>>
>> dtv_set_default_delivery_caps() does the wrong thing for delivery systems
>> like ISDB-T, ISDB-S, DSS, DMB-TH, as it fills data with a fake value that
>> is there at fe->ops.info.type.
>>
>> The fake values there should be used only for DVBv3 legacy calls emulation
>> on those delivery systems that are not fully compatible with a DVBv3 call.
>
> That's right. Still, there's no need to introduce fe->ops.delsys,
> because the drivers in question could just implement get_property
> instead. At least that's what we discussed and AFAIR agreed upon when
> Manu recently submitted his patches regarding enumeration of delivery
> systems.
Manu's patches were applied (well, except for two patches related to af9013
driver that are/were under discussion between Manu and Antti).
Manu's approach is good, as it provided a way to enumerate the
standards without much changes, offering a way for userspace to
query the delivery system, at the expense of serializing a driver
call for each property.
Yet, it doesn't allow the DVB core to detect the supported
delivery systems on a sane way [1].
The addition of fe->ops.delsys is going one step further, as it will
allow, at the long term, the removal of info.type.
There are two reasons why we need to get rid of info.type:
1) dvb_frontend core can be changed to use fe->ops.delsys
internally, instead of info.type, in order to fix some
bugs inside it, where it does the wrong assumption, because
the frontend is lying about the delivery system;
2) There is no sane way to fill fe->ops.info.type for Multi delivery
system frontends, like DRX-K, that supports both DVB-T and DVB-C.
The type can be filled with either FE_QAM or FE_OFDM, not with both.
So, choosing either type will be plain wrong, and may cause bad
side effects inside dvb_frontend.
[1] Ok, it would be possible to do a hack internally, for it to call
get_property() but it is a way more sane to just read fe->ops.delsys.
>
>> At the end, I think we should deprecate the fe->ops.info.type, as its
>> contents is not reliable (as it can represent something else).
>>
>> Btw, there are several places at dvb_frontend.c that uses the info.type
>> to assume the delivery system. This leads DVB core to do the wrong assumptions
>> for non-DVBv3 supported systems. The right way is to use a new field that
>> really represents the supported delivery systems by a given frontend, instead
>> of relying on fe->ops.info.type.
>>
>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-media" in
>>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
