* v4 [PATCH 08/10] TDA18271c2dd: Allow frontend to set DELSYS
@ 2011-12-10 4:44 Manu Abraham
2011-12-10 12:50 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 4+ messages in thread
From: Manu Abraham @ 2011-12-10 4:44 UTC (permalink / raw)
To: Linux Media Mailing List
[-- Attachment #1: Type: text/plain, Size: 1 bytes --]
[-- Attachment #2: 0008-TDA18271c2dd-Allow-frontend-to-set-DELSYS-rather-tha.patch --]
[-- Type: text/x-patch, Size: 2321 bytes --]
From 707877f5a61b3259704d42e7dd5e647e9196e9a4 Mon Sep 17 00:00:00 2001
From: Manu Abraham <abraham.manu@gmail.com>
Date: Thu, 24 Nov 2011 19:56:34 +0530
Subject: [PATCH 08/10] TDA18271c2dd: Allow frontend to set DELSYS, rather than querying fe->ops.info.type
With any tuner that can tune to multiple delivery systems/standards, it does
query fe->ops.info.type to determine frontend type and set the delivery
system type. fe->ops.info.type can handle only 4 delivery systems, viz FE_QPSK,
FE_QAM, FE_OFDM and FE_ATSC.
Signed-off-by: Manu Abraham <abraham.manu@gmail.com>
---
drivers/media/dvb/frontends/tda18271c2dd.c | 42 ++++++++++++++++++++--------
1 files changed, 30 insertions(+), 12 deletions(-)
diff --git a/drivers/media/dvb/frontends/tda18271c2dd.c b/drivers/media/dvb/frontends/tda18271c2dd.c
index 1b1bf20..43a3dd4 100644
--- a/drivers/media/dvb/frontends/tda18271c2dd.c
+++ b/drivers/media/dvb/frontends/tda18271c2dd.c
@@ -1145,28 +1145,46 @@ static int set_params(struct dvb_frontend *fe,
int status = 0;
int Standard;
- state->m_Frequency = params->frequency;
+ u32 bw;
+ fe_delivery_system_t delsys;
- if (fe->ops.info.type == FE_OFDM)
- switch (params->u.ofdm.bandwidth) {
- case BANDWIDTH_6_MHZ:
+ delsys = fe->dtv_property_cache.delivery_system;
+ bw = fe->dtv_property_cache.bandwidth_hz;
+
+ state->m_Frequency = fe->dtv_property_cache.frequency;
+
+ if (!delsys || !state->m_Frequency) {
+ printk(KERN_ERR "Invalid delsys:%d freq:%d\n", delsys, state->m_Frequency);
+ return -EINVAL;
+ }
+
+ switch (delsys) {
+ case SYS_DVBT:
+ case SYS_DVBT2:
+ if (!bw)
+ return -EINVAL;
+ switch (bw) {
+ case 6000000:
Standard = HF_DVBT_6MHZ;
break;
- case BANDWIDTH_7_MHZ:
+ case 7000000:
Standard = HF_DVBT_7MHZ;
break;
default:
- case BANDWIDTH_8_MHZ:
+ case 8000000:
Standard = HF_DVBT_8MHZ;
break;
}
- else if (fe->ops.info.type == FE_QAM) {
- if (params->u.qam.symbol_rate <= MAX_SYMBOL_RATE_6MHz)
- Standard = HF_DVBC_6MHZ;
- else
- Standard = HF_DVBC_8MHZ;
- } else
+ break;
+ case SYS_DVBC_ANNEX_A:
+ Standard = HF_DVBC_6MHZ;
+ break;
+ case SYS_DVBC_ANNEX_C:
+ Standard = HF_DVBC_8MHZ;
+ break;
+ default:
return -EINVAL;
+ }
do {
status = RFTrackingFiltersCorrection(state, params->frequency);
if (status < 0)
--
1.7.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: v4 [PATCH 08/10] TDA18271c2dd: Allow frontend to set DELSYS
2011-12-10 4:44 v4 [PATCH 08/10] TDA18271c2dd: Allow frontend to set DELSYS Manu Abraham
@ 2011-12-10 12:50 ` Mauro Carvalho Chehab
2011-12-12 5:01 ` Manu Abraham
0 siblings, 1 reply; 4+ messages in thread
From: Mauro Carvalho Chehab @ 2011-12-10 12:50 UTC (permalink / raw)
To: Manu Abraham; +Cc: Linux Media Mailing List
On 10-12-2011 02:44, Manu Abraham wrote:
> From 707877f5a61b3259704d42e7dd5e647e9196e9a4 Mon Sep 17 00:00:00 2001
> From: Manu Abraham <abraham.manu@gmail.com>
> Date: Thu, 24 Nov 2011 19:56:34 +0530
> Subject: [PATCH 08/10] TDA18271c2dd: Allow frontend to set DELSYS, rather than querying fe->ops.info.type
>
> With any tuner that can tune to multiple delivery systems/standards, it does
> query fe->ops.info.type to determine frontend type and set the delivery
> system type. fe->ops.info.type can handle only 4 delivery systems, viz FE_QPSK,
> FE_QAM, FE_OFDM and FE_ATSC.
>
> Signed-off-by: Manu Abraham <abraham.manu@gmail.com>
> ---
> drivers/media/dvb/frontends/tda18271c2dd.c | 42 ++++++++++++++++++++--------
> 1 files changed, 30 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/media/dvb/frontends/tda18271c2dd.c b/drivers/media/dvb/frontends/tda18271c2dd.c
> index 1b1bf20..43a3dd4 100644
> --- a/drivers/media/dvb/frontends/tda18271c2dd.c
> +++ b/drivers/media/dvb/frontends/tda18271c2dd.c
> @@ -1145,28 +1145,46 @@ static int set_params(struct dvb_frontend *fe,
> int status = 0;
> int Standard;
>
> - state->m_Frequency = params->frequency;
> + u32 bw;
> + fe_delivery_system_t delsys;
>
> - if (fe->ops.info.type == FE_OFDM)
> - switch (params->u.ofdm.bandwidth) {
> - case BANDWIDTH_6_MHZ:
> + delsys = fe->dtv_property_cache.delivery_system;
> + bw = fe->dtv_property_cache.bandwidth_hz;
> +
> + state->m_Frequency = fe->dtv_property_cache.frequency;
> +
> + if (!delsys || !state->m_Frequency) {
> + printk(KERN_ERR "Invalid delsys:%d freq:%d\n", delsys, state->m_Frequency);
> + return -EINVAL;
> + }
> +
> + switch (delsys) {
> + case SYS_DVBT:
> + case SYS_DVBT2:
> + if (!bw)
> + return -EINVAL;
> + switch (bw) {
> + case 6000000:
> Standard = HF_DVBT_6MHZ;
> break;
> - case BANDWIDTH_7_MHZ:
> + case 7000000:
> Standard = HF_DVBT_7MHZ;
> break;
> default:
> - case BANDWIDTH_8_MHZ:
> + case 8000000:
> Standard = HF_DVBT_8MHZ;
> break;
> }
> - else if (fe->ops.info.type == FE_QAM) {
> - if (params->u.qam.symbol_rate <= MAX_SYMBOL_RATE_6MHz)
> - Standard = HF_DVBC_6MHZ;
> - else
> - Standard = HF_DVBC_8MHZ;
> - } else
> + break;
> + case SYS_DVBC_ANNEX_A:
> + Standard = HF_DVBC_6MHZ;
> + break;
> + case SYS_DVBC_ANNEX_C:
> + Standard = HF_DVBC_8MHZ;
> + break;
No, this is wrong. This patch doesn't apply anymore, due to the recent
changes that estimate the bandwidth based on the roll-off factor. Reverting
it breaks for DVB-C @ 6MHz spaced channels (and likely decreases quality
or breaks for 7MHz spaced ones too).
> + default:
> return -EINVAL;
> + }
> do {
> status = RFTrackingFiltersCorrection(state, params->frequency);
> if (status < 0)
> --
> 1.7.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: v4 [PATCH 08/10] TDA18271c2dd: Allow frontend to set DELSYS
2011-12-10 12:50 ` Mauro Carvalho Chehab
@ 2011-12-12 5:01 ` Manu Abraham
2011-12-12 13:39 ` Mauro Carvalho Chehab
0 siblings, 1 reply; 4+ messages in thread
From: Manu Abraham @ 2011-12-12 5:01 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: Linux Media Mailing List
On Sat, Dec 10, 2011 at 6:20 PM, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:
> On 10-12-2011 02:44, Manu Abraham wrote:
>>
>> From 707877f5a61b3259704d42e7dd5e647e9196e9a4 Mon Sep 17 00:00:00 2001
>> From: Manu Abraham <abraham.manu@gmail.com>
>> Date: Thu, 24 Nov 2011 19:56:34 +0530
>> Subject: [PATCH 08/10] TDA18271c2dd: Allow frontend to set DELSYS, rather
>> than querying fe->ops.info.type
>>
>> With any tuner that can tune to multiple delivery systems/standards, it
>> does
>> query fe->ops.info.type to determine frontend type and set the delivery
>> system type. fe->ops.info.type can handle only 4 delivery systems, viz
>> FE_QPSK,
>> FE_QAM, FE_OFDM and FE_ATSC.
>>
>> Signed-off-by: Manu Abraham <abraham.manu@gmail.com>
>> ---
>> drivers/media/dvb/frontends/tda18271c2dd.c | 42
>> ++++++++++++++++++++--------
>> 1 files changed, 30 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/media/dvb/frontends/tda18271c2dd.c
>> b/drivers/media/dvb/frontends/tda18271c2dd.c
>> index 1b1bf20..43a3dd4 100644
>> --- a/drivers/media/dvb/frontends/tda18271c2dd.c
>> +++ b/drivers/media/dvb/frontends/tda18271c2dd.c
>> @@ -1145,28 +1145,46 @@ static int set_params(struct dvb_frontend *fe,
>> int status = 0;
>> int Standard;
>>
>> - state->m_Frequency = params->frequency;
>> + u32 bw;
>> + fe_delivery_system_t delsys;
>>
>> - if (fe->ops.info.type == FE_OFDM)
>> - switch (params->u.ofdm.bandwidth) {
>> - case BANDWIDTH_6_MHZ:
>> + delsys = fe->dtv_property_cache.delivery_system;
>> + bw = fe->dtv_property_cache.bandwidth_hz;
>> +
>> + state->m_Frequency = fe->dtv_property_cache.frequency;
>> +
>> + if (!delsys || !state->m_Frequency) {
>> + printk(KERN_ERR "Invalid delsys:%d freq:%d\n", delsys,
>> state->m_Frequency);
>> + return -EINVAL;
>> + }
>> +
>> + switch (delsys) {
>> + case SYS_DVBT:
>> + case SYS_DVBT2:
>> + if (!bw)
>> + return -EINVAL;
>> + switch (bw) {
>> + case 6000000:
>> Standard = HF_DVBT_6MHZ;
>> break;
>> - case BANDWIDTH_7_MHZ:
>> + case 7000000:
>> Standard = HF_DVBT_7MHZ;
>> break;
>> default:
>> - case BANDWIDTH_8_MHZ:
>> + case 8000000:
>> Standard = HF_DVBT_8MHZ;
>> break;
>> }
>> - else if (fe->ops.info.type == FE_QAM) {
>> - if (params->u.qam.symbol_rate <= MAX_SYMBOL_RATE_6MHz)
>> - Standard = HF_DVBC_6MHZ;
>> - else
>> - Standard = HF_DVBC_8MHZ;
>> - } else
>> + break;
>> + case SYS_DVBC_ANNEX_A:
>> + Standard = HF_DVBC_6MHZ;
>> + break;
>> + case SYS_DVBC_ANNEX_C:
>> + Standard = HF_DVBC_8MHZ;
>> + break;
>
>
> No, this is wrong. This patch doesn't apply anymore, due to the recent
> changes that estimate the bandwidth based on the roll-off factor. Reverting
> it breaks for DVB-C @ 6MHz spaced channels (and likely decreases quality
> or breaks for 7MHz spaced ones too).
The changes that which you mention (which you state breaks 7MHz
spacing) is much newer than these patch series. Alas, how do you
expect people to push out patches every now and then, when you
simply whack patches in. It is not the issue with the people sending
patches, but how you handled the patch series. I have reworked the
patches the 4th time, while you simply whack patches without any
feedback. Time after time, lot of different people have argued with
you not to simply whack in patches as you seem fit. Who is to blame ?
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: v4 [PATCH 08/10] TDA18271c2dd: Allow frontend to set DELSYS
2011-12-12 5:01 ` Manu Abraham
@ 2011-12-12 13:39 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 4+ messages in thread
From: Mauro Carvalho Chehab @ 2011-12-12 13:39 UTC (permalink / raw)
To: Manu Abraham; +Cc: Linux Media Mailing List
On 12-12-2011 03:01, Manu Abraham wrote:
> On Sat, Dec 10, 2011 at 6:20 PM, Mauro Carvalho Chehab
> <mchehab@redhat.com> wrote:
>> On 10-12-2011 02:44, Manu Abraham wrote:
>>>
>>> From 707877f5a61b3259704d42e7dd5e647e9196e9a4 Mon Sep 17 00:00:00 2001
>>> From: Manu Abraham<abraham.manu@gmail.com>
>>> Date: Thu, 24 Nov 2011 19:56:34 +0530
>>> Subject: [PATCH 08/10] TDA18271c2dd: Allow frontend to set DELSYS, rather
>>> than querying fe->ops.info.type
>>>
>>> With any tuner that can tune to multiple delivery systems/standards, it
>>> does
>>> query fe->ops.info.type to determine frontend type and set the delivery
>>> system type. fe->ops.info.type can handle only 4 delivery systems, viz
>>> FE_QPSK,
>>> FE_QAM, FE_OFDM and FE_ATSC.
>>>
>>> Signed-off-by: Manu Abraham<abraham.manu@gmail.com>
>>> ---
>>> drivers/media/dvb/frontends/tda18271c2dd.c | 42
>>> ++++++++++++++++++++--------
>>> 1 files changed, 30 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/media/dvb/frontends/tda18271c2dd.c
>>> b/drivers/media/dvb/frontends/tda18271c2dd.c
>>> index 1b1bf20..43a3dd4 100644
>>> --- a/drivers/media/dvb/frontends/tda18271c2dd.c
>>> +++ b/drivers/media/dvb/frontends/tda18271c2dd.c
>>> @@ -1145,28 +1145,46 @@ static int set_params(struct dvb_frontend *fe,
>>> int status = 0;
>>> int Standard;
>>>
>>> - state->m_Frequency = params->frequency;
>>> + u32 bw;
>>> + fe_delivery_system_t delsys;
>>>
>>> - if (fe->ops.info.type == FE_OFDM)
>>> - switch (params->u.ofdm.bandwidth) {
>>> - case BANDWIDTH_6_MHZ:
>>> + delsys = fe->dtv_property_cache.delivery_system;
>>> + bw = fe->dtv_property_cache.bandwidth_hz;
>>> +
>>> + state->m_Frequency = fe->dtv_property_cache.frequency;
>>> +
>>> + if (!delsys || !state->m_Frequency) {
>>> + printk(KERN_ERR "Invalid delsys:%d freq:%d\n", delsys,
>>> state->m_Frequency);
>>> + return -EINVAL;
>>> + }
>>> +
>>> + switch (delsys) {
>>> + case SYS_DVBT:
>>> + case SYS_DVBT2:
>>> + if (!bw)
>>> + return -EINVAL;
>>> + switch (bw) {
>>> + case 6000000:
>>> Standard = HF_DVBT_6MHZ;
>>> break;
>>> - case BANDWIDTH_7_MHZ:
>>> + case 7000000:
>>> Standard = HF_DVBT_7MHZ;
>>> break;
>>> default:
>>> - case BANDWIDTH_8_MHZ:
>>> + case 8000000:
>>> Standard = HF_DVBT_8MHZ;
>>> break;
>>> }
>>> - else if (fe->ops.info.type == FE_QAM) {
>>> - if (params->u.qam.symbol_rate<= MAX_SYMBOL_RATE_6MHz)
>>> - Standard = HF_DVBC_6MHZ;
>>> - else
>>> - Standard = HF_DVBC_8MHZ;
>>> - } else
>>> + break;
>>> + case SYS_DVBC_ANNEX_A:
>>> + Standard = HF_DVBC_6MHZ;
>>> + break;
>>> + case SYS_DVBC_ANNEX_C:
>>> + Standard = HF_DVBC_8MHZ;
>>> + break;
>>
>>
>> No, this is wrong. This patch doesn't apply anymore, due to the recent
>> changes that estimate the bandwidth based on the roll-off factor. Reverting
>> it breaks for DVB-C @ 6MHz spaced channels (and likely decreases quality
>> or breaks for 7MHz spaced ones too).
>
>
> The changes that which you mention (which you state breaks 7MHz
> spacing) is much newer than these patch series. Alas, how do you
> expect people to push out patches every now and then, when you
> simply whack patches in. It is not the issue with the people sending
> patches, but how you handled the patch series. I have reworked the
> patches the 4th time, while you simply whack patches without any
> feedback.
> Time after time, lot of different people have argued with
> you not to simply whack in patches as you seem fit. Who is to blame ?
Patches were sent to the ML, and were tested by the ones complaining
about tuner issues with DVB-C. It were merged only after having feedback.
In any case, this is not a reason for me to not merge this patch. Conflicts
like that happens all the time when merging stuff from different authors.
The issue here is that it assumes that Annex A is 6MHz and Annex C is 8MHz.
This is a wrong assumption: there's nothing at ITU-T J.83 associating Annex C
(or even Annex A) with a given bandwidth.
What is said at the spec is that Annex A is "optimized" for 6MHz, but I
won't be really surprised if some broadcaster decides to use it for other
bandwidths.
The logic there should be, instead:
u32 roll_off = 0;
switch (delsys) {
case SYS_DVBC_ANNEX_A:
roll_off = 115;
break;
case SYS_DVBC_ANNEX_C:
roll_off = 113;
break;
}
if (roll_off) {
bw = symbol_rate * roll_off / 100;
if (bw <= 6000000)
Standard = HF_DVBC_6MHZ;
else if (bw <= 7000000)
Standard = HF_DVBC_7MHZ;
else
Standard = HF_DVBC_8MHZ;
}
Regards,
Mauro
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-12-12 13:39 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-12-10 4:44 v4 [PATCH 08/10] TDA18271c2dd: Allow frontend to set DELSYS Manu Abraham
2011-12-10 12:50 ` Mauro Carvalho Chehab
2011-12-12 5:01 ` Manu Abraham
2011-12-12 13:39 ` Mauro Carvalho Chehab
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).