On 08/06/2020 17:30, Marc Gonzalez wrote:
> Suppose we know that several channels are transmitted on a given frequency
> by terrestrial antenna. However, we don't know if the signal is "encoded"
> (not sure this is the right term) in DVB-T or DVB-T2 modulation.
>
> Do we have to scan the frequency /twice/
> first with DTV_DELIVERY_SYSTEM = SYS_DVBT
> then with DTV_DELIVERY_SYSTEM = SYS_DVBT2 ?
>
> Or is there some optimization where both modulations are handled
> in a single step?
>
> Or maybe it depends on the tuner driver?
Digging a bit deeper...
My system sports a Silicon Labs Si2168 DVB-T2/T/C demodulator
The data sheet states:
"For DVB-T2 and T2-Lite:
DVB-T2 versus DVB-T automatic detection"
"In DVB-T and DVB-T2 reception, parameters necessary for device synchronization
are broadcast in the transmission parameters (respectively TPS and P1 symbol).
When these transmission parameters are found and decoded, the demodulator is
automatically and accordingly configured to achieve full synchronization."
This HW is supported upstream by drivers/media/dvb-frontends/si2168.c
https://elixir.bootlin.com/linux/latest/source/drivers/media/dvb-frontends/si2168.c
I have a nagging feeling because si2168_set_frontend() appears to
behave differently for SYS_DVBT and for SYS_DVBT2...
https://elixir.bootlin.com/linux/latest/source/drivers/media/dvb-frontends/si2168.c#L250
https://elixir.bootlin.com/linux/latest/source/drivers/media/dvb-frontends/si2168.c#L297
https://elixir.bootlin.com/linux/latest/source/drivers/media/dvb-frontends/si2168.c#L345
Maybe setting SYS_DVBT2 would also work for DVB-T transmissions?
(TODO: test that)
Any insight would be greatly appreciated.
Regards.
On 09/06/2020 16:31, Marc Gonzalez wrote:
> On 08/06/2020 17:30, Marc Gonzalez wrote:
>
>> Suppose we know that several channels are transmitted on a given frequency
>> by terrestrial antenna. However, we don't know if the signal is "encoded"
>> (not sure this is the right term) in DVB-T or DVB-T2 modulation.
>>
>> Do we have to scan the frequency /twice/
>> first with DTV_DELIVERY_SYSTEM = SYS_DVBT
>> then with DTV_DELIVERY_SYSTEM = SYS_DVBT2 ?
>>
>> Or is there some optimization where both modulations are handled
>> in a single step?
>>
>> Or maybe it depends on the tuner driver?
>
> Digging a bit deeper...
>
> My system sports a Silicon Labs Si2168 DVB-T2/T/C demodulator
>
> The data sheet states:
>
> "For DVB-T2 and T2-Lite:
> DVB-T2 versus DVB-T automatic detection"
>
> "In DVB-T and DVB-T2 reception, parameters necessary for device synchronization
> are broadcast in the transmission parameters (respectively TPS and P1 symbol).
> When these transmission parameters are found and decoded, the demodulator is
> automatically and accordingly configured to achieve full synchronization."
>
>
> This HW is supported upstream by drivers/media/dvb-frontends/si2168.c
> https://elixir.bootlin.com/linux/latest/source/drivers/media/dvb-frontends/si2168.c
>
> I have a nagging feeling because si2168_set_frontend() appears to
> behave differently for SYS_DVBT and for SYS_DVBT2...
>
> https://elixir.bootlin.com/linux/latest/source/drivers/media/dvb-frontends/si2168.c#L250
> https://elixir.bootlin.com/linux/latest/source/drivers/media/dvb-frontends/si2168.c#L297
> https://elixir.bootlin.com/linux/latest/source/drivers/media/dvb-frontends/si2168.c#L345
>
> Maybe setting SYS_DVBT2 would also work for DVB-T transmissions?
> (TODO: test that)
>
> Any insight would be greatly appreciated.
FTR, on IRC, Brad pointed out this patch of his:
https://patchwork.kernel.org/patch/10744999/
On 10/06/2020 17:22, Marc Gonzalez wrote:
> FTR, on IRC, Brad pointed out this patch of his:
> https://patchwork.kernel.org/patch/10744999/
I suggest the following patch on top of Brad's patch:
Author: Marc Gonzalez <[email protected]>
Date: Fri Jun 19 22:09:26 2020 +0200
si2168: wait for carrier lock before next step
diff --git a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c
index 31d3dc0216c2..e127e842f671 100644
--- a/drivers/media/dvb-frontends/si2168.c
+++ b/drivers/media/dvb-frontends/si2168.c
@@ -152,6 +152,11 @@ static int si2168_ts_bus_ctrl(struct dvb_frontend *fe, int acquire)
return ret;
}
+static bool carrier_locked(struct si2168_cmd *cmd)
+{
+ return cmd->args[2] & BIT(1);
+}
+
static int si2168_read_status(struct dvb_frontend *fe, enum fe_status *status)
{
struct i2c_client *client = fe->demodulator_priv;
@@ -180,6 +185,9 @@ static int si2168_read_status(struct dvb_frontend *fe, enum fe_status *status)
if (ret)
goto err;
+ if (!carrier_locked(&cmd))
+ goto parse_response;
+
if ((cmd.args[3] & 0x0f) == 7)
sys = SYS_DVBT2;
}
@@ -206,27 +214,10 @@ static int si2168_read_status(struct dvb_frontend *fe, enum fe_status *status)
}
ret = si2168_cmd_execute(client, &cmd);
- if (dvbt_auto_plp && (ret == -EREMOTEIO)) {
- /* In auto-PLP mode it is possible to read 0x8701 while
- * the frontend is in switchover transition. This causes
- * a status read failure, due to incorrect system. Check
- * the other sys if we hit this race condition.
- */
- if (sys == SYS_DVBT) {
- memcpy(cmd.args, "\x50\x01", 2); /* DVB-T2 */
- cmd.wlen = 2;
- cmd.rlen = 14;
- ret = si2168_cmd_execute(client, &cmd);
- } else if (sys == SYS_DVBT2) {
- memcpy(cmd.args, "\xa0\x01", 2); /* DVB-T */
- cmd.wlen = 2;
- cmd.rlen = 13;
- ret = si2168_cmd_execute(client, &cmd);
- }
- }
if (ret)
goto err;
+parse_response:
switch ((cmd.args[2] >> 1) & 0x03) {
case 0x01:
*status = FE_HAS_SIGNAL | FE_HAS_CARRIER;
Hi Marc,
On Fri, Jun 19, 2020 at 3:15 PM Marc Gonzalez <[email protected]> wrote:
>
> On 10/06/2020 17:22, Marc Gonzalez wrote:
>
> > FTR, on IRC, Brad pointed out this patch of his:
> > https://patchwork.kernel.org/patch/10744999/
>
> I suggest the following patch on top of Brad's patch:
>
> Author: Marc Gonzalez <[email protected]>
> Date: Fri Jun 19 22:09:26 2020 +0200
>
> si2168: wait for carrier lock before next step
>
> diff --git a/drivers/media/dvb-frontends/si2168.c b/drivers/media/dvb-frontends/si2168.c
> index 31d3dc0216c2..e127e842f671 100644
> --- a/drivers/media/dvb-frontends/si2168.c
> +++ b/drivers/media/dvb-frontends/si2168.c
> @@ -152,6 +152,11 @@ static int si2168_ts_bus_ctrl(struct dvb_frontend *fe, int acquire)
> return ret;
> }
>
> +static bool carrier_locked(struct si2168_cmd *cmd)
> +{
> + return cmd->args[2] & BIT(1);
> +}
> +
> static int si2168_read_status(struct dvb_frontend *fe, enum fe_status *status)
> {
> struct i2c_client *client = fe->demodulator_priv;
> @@ -180,6 +185,9 @@ static int si2168_read_status(struct dvb_frontend *fe, enum fe_status *status)
> if (ret)
> goto err;
>
> + if (!carrier_locked(&cmd))
> + goto parse_response;
> +
My original patch has been well tested and is currently deployed in
many thousands of assorted systems across Europe. Unless you can
guarantee that the frontend switchover race condition will *never*
happen *ever* across any system, including a large amount of
architectures and array of cpu types and speeds, I don't think it's
beneficial to remove it.
Hence, I'm very hesitant to deploy your patch and break this auto plp
detection for someone, just to save <=10ms.
Regards,
Brad
> if ((cmd.args[3] & 0x0f) == 7)
> sys = SYS_DVBT2;
> }
> @@ -206,27 +214,10 @@ static int si2168_read_status(struct dvb_frontend *fe, enum fe_status *status)
> }
>
> ret = si2168_cmd_execute(client, &cmd);
> - if (dvbt_auto_plp && (ret == -EREMOTEIO)) {
> - /* In auto-PLP mode it is possible to read 0x8701 while
> - * the frontend is in switchover transition. This causes
> - * a status read failure, due to incorrect system. Check
> - * the other sys if we hit this race condition.
> - */
> - if (sys == SYS_DVBT) {
> - memcpy(cmd.args, "\x50\x01", 2); /* DVB-T2 */
> - cmd.wlen = 2;
> - cmd.rlen = 14;
> - ret = si2168_cmd_execute(client, &cmd);
> - } else if (sys == SYS_DVBT2) {
> - memcpy(cmd.args, "\xa0\x01", 2); /* DVB-T */
> - cmd.wlen = 2;
> - cmd.rlen = 13;
> - ret = si2168_cmd_execute(client, &cmd);
> - }
> - }
> if (ret)
> goto err;
>
> +parse_response:
> switch ((cmd.args[2] >> 1) & 0x03) {
> case 0x01:
> *status = FE_HAS_SIGNAL | FE_HAS_CARRIER;
>