linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] media: si2157: do some minor improvements at the driver
@ 2021-12-10 12:59 Mauro Carvalho Chehab
  2021-12-10 12:59 ` [PATCH 1/3] media: si2157: add support for ISDB-T and DTMB Mauro Carvalho Chehab
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Mauro Carvalho Chehab @ 2021-12-10 12:59 UTC (permalink / raw)
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Antti Palosaari,
	Mauro Carvalho Chehab, linux-kernel, linux-media

The si21xx terrestrial tuners can support ISDB-T and DTMB (as it is really
just a tuner, and not a demod), but the missing settings are missing.

Some of the devices on this family also support bandwidth filters for 1.7 and 6.1.

Finally, si2158 and si2157 have identical APIs for analog TV. So, it makes
sense to also enable it for si2158.

Compile-tested only.

Mauro Carvalho Chehab (3):
  media: si2157: add support for ISDB-T and DTMB
  media: si2157: add support for 1.7MHz and 6.1 MHz
  media: si2157: add ATV support for si2158

 drivers/media/tuners/si2157.c      | 12 +++++++++++-
 drivers/media/tuners/si2157_priv.h |  8 ++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

-- 
2.33.1



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/3] media: si2157: add support for ISDB-T and DTMB
  2021-12-10 12:59 [PATCH 0/3] media: si2157: do some minor improvements at the driver Mauro Carvalho Chehab
@ 2021-12-10 12:59 ` Mauro Carvalho Chehab
  2021-12-10 12:59 ` [PATCH 2/3] media: si2157: add support for 1.7MHz and 6.1 MHz Mauro Carvalho Chehab
  2021-12-10 12:59 ` [PATCH 3/3] media: si2157: add ATV support for si2158 Mauro Carvalho Chehab
  2 siblings, 0 replies; 9+ messages in thread
From: Mauro Carvalho Chehab @ 2021-12-10 12:59 UTC (permalink / raw)
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Antti Palosaari,
	Mauro Carvalho Chehab, linux-kernel, linux-media

Those two delivery systems are supported by some of the si2146
tuners, but the current code is missing the setup for those.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---

To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH 0/3] at: https://lore.kernel.org/all/cover.1639140967.git.mchehab+huawei@kernel.org/

 drivers/media/tuners/si2157.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index 7e44cba67c38..aeecb38b147f 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -483,6 +483,12 @@ static int si2157_set_params(struct dvb_frontend *fe)
 	case SYS_DVBC_ANNEX_A:
 			delivery_system = 0x30;
 			break;
+	case SYS_ISDBT:
+			delivery_system = 0x40;
+			break;
+	case SYS_DTMB:
+			delivery_system = 0x60;
+			break;
 	default:
 			ret = -EINVAL;
 			goto err;
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/3] media: si2157: add support for 1.7MHz and 6.1 MHz
  2021-12-10 12:59 [PATCH 0/3] media: si2157: do some minor improvements at the driver Mauro Carvalho Chehab
  2021-12-10 12:59 ` [PATCH 1/3] media: si2157: add support for ISDB-T and DTMB Mauro Carvalho Chehab
@ 2021-12-10 12:59 ` Mauro Carvalho Chehab
  2022-01-06 15:39   ` Robert Schlabbach
  2021-12-10 12:59 ` [PATCH 3/3] media: si2157: add ATV support for si2158 Mauro Carvalho Chehab
  2 siblings, 1 reply; 9+ messages in thread
From: Mauro Carvalho Chehab @ 2021-12-10 12:59 UTC (permalink / raw)
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Antti Palosaari,
	Mauro Carvalho Chehab, linux-kernel, linux-media

Some tuners allow setting the bandwidth filter to 1.7MHz and
6.1 MHz. Add support for it upstream, as the narrower is the
bandwidth filter, the better.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---

To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH 0/3] at: https://lore.kernel.org/all/cover.1639140967.git.mchehab+huawei@kernel.org/

 drivers/media/tuners/si2157.c      | 4 ++++
 drivers/media/tuners/si2157_priv.h | 5 +++++
 2 files changed, 9 insertions(+)

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index aeecb38b147f..2d3937af4f5f 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -458,8 +458,12 @@ static int si2157_set_params(struct dvb_frontend *fe)
 		goto err;
 	}
 
+	if (SUPPORTS_1700KHz(dev) && c->bandwidth_hz <= 1700000)
+		bandwidth = 0x09;
 	if (c->bandwidth_hz <= 6000000)
 		bandwidth = 0x06;
+	if (SUPPORTS_1700KHz(dev) && c->bandwidth_hz <= 6100000)
+		bandwidth = 0x10;
 	else if (c->bandwidth_hz <= 7000000)
 		bandwidth = 0x07;
 	else if (c->bandwidth_hz <= 8000000)
diff --git a/drivers/media/tuners/si2157_priv.h b/drivers/media/tuners/si2157_priv.h
index df17a5f03561..24849c8ed398 100644
--- a/drivers/media/tuners/si2157_priv.h
+++ b/drivers/media/tuners/si2157_priv.h
@@ -66,6 +66,11 @@ struct si2157_cmd {
 	unsigned rlen;
 };
 
+#define SUPPORTS_1700KHz(dev) (((dev)->part_id == SI2141) || \
+			       ((dev)->part_id == SI2147) || \
+			       ((dev)->part_id == SI2157) || \
+			       ((dev)->part_id == SI2177))
+
 /* Old firmware namespace */
 #define SI2158_A20_FIRMWARE "dvb-tuner-si2158-a20-01.fw"
 #define SI2141_A10_FIRMWARE "dvb-tuner-si2141-a10-01.fw"
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/3] media: si2157: add ATV support for si2158
  2021-12-10 12:59 [PATCH 0/3] media: si2157: do some minor improvements at the driver Mauro Carvalho Chehab
  2021-12-10 12:59 ` [PATCH 1/3] media: si2157: add support for ISDB-T and DTMB Mauro Carvalho Chehab
  2021-12-10 12:59 ` [PATCH 2/3] media: si2157: add support for 1.7MHz and 6.1 MHz Mauro Carvalho Chehab
@ 2021-12-10 12:59 ` Mauro Carvalho Chehab
  2 siblings, 0 replies; 9+ messages in thread
From: Mauro Carvalho Chehab @ 2021-12-10 12:59 UTC (permalink / raw)
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, Antti Palosaari,
	Mauro Carvalho Chehab, linux-kernel, linux-media

This device also supports ATV, as it has the same API for
setting analog TV tuning parameters.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---

To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH 0/3] at: https://lore.kernel.org/all/cover.1639140967.git.mchehab+huawei@kernel.org/

 drivers/media/tuners/si2157.c      | 2 +-
 drivers/media/tuners/si2157_priv.h | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index 2d3937af4f5f..481c5c3b577d 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -576,7 +576,7 @@ static int si2157_set_analog_params(struct dvb_frontend *fe,
 	u8 color = 0;    /* 0=NTSC/PAL, 0x10=SECAM */
 	u8 invert_analog = 1; /* analog tuner spectrum; 0=normal, 1=inverted */
 
-	if (dev->part_id != SI2157) {
+	if (!SUPPORTS_ATV_IF(dev)) {
 		dev_info(&client->dev, "Analog tuning not supported yet for Si21%d\n",
 			 dev->part_id);
 		ret = -EINVAL;
diff --git a/drivers/media/tuners/si2157_priv.h b/drivers/media/tuners/si2157_priv.h
index 24849c8ed398..8579e80f7af7 100644
--- a/drivers/media/tuners/si2157_priv.h
+++ b/drivers/media/tuners/si2157_priv.h
@@ -71,6 +71,9 @@ struct si2157_cmd {
 			       ((dev)->part_id == SI2157) || \
 			       ((dev)->part_id == SI2177))
 
+#define SUPPORTS_ATV_IF(dev) (((dev)->part_id == SI2157) || \
+			      ((dev)->part_id == SI2158))
+
 /* Old firmware namespace */
 #define SI2158_A20_FIRMWARE "dvb-tuner-si2158-a20-01.fw"
 #define SI2141_A10_FIRMWARE "dvb-tuner-si2141-a10-01.fw"
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3] media: si2157: add support for 1.7MHz and 6.1 MHz
  2021-12-10 12:59 ` [PATCH 2/3] media: si2157: add support for 1.7MHz and 6.1 MHz Mauro Carvalho Chehab
@ 2022-01-06 15:39   ` Robert Schlabbach
  2022-01-06 18:25     ` Robert Schlabbach
  0 siblings, 1 reply; 9+ messages in thread
From: Robert Schlabbach @ 2022-01-06 15:39 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: mauro.chehab, Antti Palosaari, linux-kernel, linux-media

Sorry for the late test and response, but there is a BAD BUG in this patch:
 
+ if (SUPPORTS_1700KHz(dev) && c->bandwidth_hz <= 1700000)
+ bandwidth = 0x09;
if (c->bandwidth_hz <= 6000000)
bandwidth = 0x06;
+ if (SUPPORTS_1700KHz(dev) && c->bandwidth_hz <= 6100000)
+ bandwidth = 0x10;
else if (c->bandwidth_hz <= 7000000)
bandwidth = 0x07;
else if (c->bandwidth_hz <= 8000000)

The additions omitted the "else", which resulted in the bandwidth setting for
6MHz being overwritten with the one for 6.1MHz - and that completely breaks
6MHz channels, as the tuner then refuses to accept the tune command!

As a result, e.g. MCNS aka ClearQAM aka DVB-C Annex B no longer works after
this patch.

I don't know if the 1.7Mhz and 6.1MHz settings are even usable, if the tuner
(in my case, the Si2157-A30) then no longer accepts the tune command. Maybe
they're not suited for frequency-based tuning, but only for channel-based
tuning? That would not fit the DVB-API, I think.

And the 1.7MHz bandwidth setting currently can't do any harm, as it is always
overwritten by the 6MHz bandwidth setting, also due to an "else" missing.

Best Regards,
-Robert Schlabbach
 
 

Gesendet: Freitag, 10. Dezember 2021 um 13:59 Uhr
Von: "Mauro Carvalho Chehab" <mchehab+huawei@kernel.org>
An: unlisted-recipients:;
Cc: linuxarm@huawei.com, mauro.chehab@huawei.com, "Mauro Carvalho Chehab" <mchehab+huawei@kernel.org>, "Antti Palosaari" <crope@iki.fi>, "Mauro Carvalho Chehab" <mchehab@kernel.org>, linux-kernel@vger.kernel.org, linux-media@vger.kernel.org
Betreff: [PATCH 2/3] media: si2157: add support for 1.7MHz and 6.1 MHz
Some tuners allow setting the bandwidth filter to 1.7MHz and
6.1 MHz. Add support for it upstream, as the narrower is the
bandwidth filter, the better.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---

To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH 0/3] at: https://lore.kernel.org/all/cover.1639140967.git.mchehab+huawei@kernel.org/

drivers/media/tuners/si2157.c | 4 ++++
drivers/media/tuners/si2157_priv.h | 5 +++++
2 files changed, 9 insertions(+)

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index aeecb38b147f..2d3937af4f5f 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -458,8 +458,12 @@ static int si2157_set_params(struct dvb_frontend *fe)
goto err;
}

+ if (SUPPORTS_1700KHz(dev) && c->bandwidth_hz <= 1700000)
+ bandwidth = 0x09;
if (c->bandwidth_hz <= 6000000)
bandwidth = 0x06;
+ if (SUPPORTS_1700KHz(dev) && c->bandwidth_hz <= 6100000)
+ bandwidth = 0x10;
else if (c->bandwidth_hz <= 7000000)
bandwidth = 0x07;
else if (c->bandwidth_hz <= 8000000)
diff --git a/drivers/media/tuners/si2157_priv.h b/drivers/media/tuners/si2157_priv.h
index df17a5f03561..24849c8ed398 100644
--- a/drivers/media/tuners/si2157_priv.h
+++ b/drivers/media/tuners/si2157_priv.h
@@ -66,6 +66,11 @@ struct si2157_cmd {
unsigned rlen;
};

+#define SUPPORTS_1700KHz(dev) (((dev)->part_id == SI2141) || \
+ ((dev)->part_id == SI2147) || \
+ ((dev)->part_id == SI2157) || \
+ ((dev)->part_id == SI2177))
+
/* Old firmware namespace */
#define SI2158_A20_FIRMWARE "dvb-tuner-si2158-a20-01.fw"
#define SI2141_A10_FIRMWARE "dvb-tuner-si2141-a10-01.fw"
--
2.33.1
 

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3] media: si2157: add support for 1.7MHz and 6.1 MHz
  2022-01-06 15:39   ` Robert Schlabbach
@ 2022-01-06 18:25     ` Robert Schlabbach
  2022-01-06 19:44       ` [PATCH] media: si6157: fix a tune regression for 6.1MHz Mauro Carvalho Chehab
  2022-01-06 20:30       ` [PATCH 2/3] media: si2157: add support for 1.7MHz and 6.1 MHz Mauro Carvalho Chehab
  0 siblings, 2 replies; 9+ messages in thread
From: Robert Schlabbach @ 2022-01-06 18:25 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: Antti Palosaari, linux-kernel, linux-media

It turns out there are actually two bugs in there:
 
+ if (SUPPORTS_1700KHz(dev) && c->bandwidth_hz <= 6100000)
+ bandwidth = 0x10;

The si2157 code for bandwidth 6.1MHz is _decimal_ 10, not
hexadecimal 0x10. Removing the "0x" from the above line makes
the tuner work again, even with the other bug that makes it use
the 6.1MHz setting when 6MHz is requested.

Another issue with the bandwidth setting: The si2157 code is
also stored in dev->bandwidth and returned in the get_bandwidth()
call. Now this was not well before, because the call is supposed
to return the bandwidth in Hz, not in MHz, but now it can return
9 and 10, but those do not correspond to 9 and 10MHz.

Also, the default case uses si2157 code 0x0f, which also seems
like a bad idea.

And another, unrelated issue: The delivery_system switch is
missing case DVBC_ANNEX_C, even though it should operate just as
DVBC_ANNEX_A.

I'll see if I can come up with a patch which addresses all of the
above.

Best Regards,
-Robert Schlabbach 


Gesendet: Donnerstag, 06. Januar 2022 um 16:39 Uhr
Von: "Robert Schlabbach" <Robert.Schlabbach@gmx.net>
An: "Mauro Carvalho Chehab" <mchehab+huawei@kernel.org>
Cc: mauro.chehab@huawei.com, "Antti Palosaari" <crope@iki.fi>, linux-kernel@vger.kernel.org, linux-media@vger.kernel.org
Betreff: Re: [PATCH 2/3] media: si2157: add support for 1.7MHz and 6.1 MHz

Sorry for the late test and response, but there is a BAD BUG in this patch:
 
+ if (SUPPORTS_1700KHz(dev) && c->bandwidth_hz <= 1700000)
+ bandwidth = 0x09;
if (c->bandwidth_hz <= 6000000)
bandwidth = 0x06;
+ if (SUPPORTS_1700KHz(dev) && c->bandwidth_hz <= 6100000)
+ bandwidth = 0x10;
else if (c->bandwidth_hz <= 7000000)
bandwidth = 0x07;
else if (c->bandwidth_hz <= 8000000)

The additions omitted the "else", which resulted in the bandwidth setting for
6MHz being overwritten with the one for 6.1MHz - and that completely breaks
6MHz channels, as the tuner then refuses to accept the tune command!

As a result, e.g. MCNS aka ClearQAM aka DVB-C Annex B no longer works after
this patch.

I don't know if the 1.7Mhz and 6.1MHz settings are even usable, if the tuner
(in my case, the Si2157-A30) then no longer accepts the tune command. Maybe
they're not suited for frequency-based tuning, but only for channel-based
tuning? That would not fit the DVB-API, I think.

And the 1.7MHz bandwidth setting currently can't do any harm, as it is always
overwritten by the 6MHz bandwidth setting, also due to an "else" missing.

Best Regards,
-Robert Schlabbach
 
 

Gesendet: Freitag, 10. Dezember 2021 um 13:59 Uhr
Von: "Mauro Carvalho Chehab" <mchehab+huawei@kernel.org>
An: unlisted-recipients:;
Cc: linuxarm@huawei.com, mauro.chehab@huawei.com, "Mauro Carvalho Chehab" <mchehab+huawei@kernel.org>, "Antti Palosaari" <crope@iki.fi>, "Mauro Carvalho Chehab" <mchehab@kernel.org>, linux-kernel@vger.kernel.org, linux-media@vger.kernel.org
Betreff: [PATCH 2/3] media: si2157: add support for 1.7MHz and 6.1 MHz
Some tuners allow setting the bandwidth filter to 1.7MHz and
6.1 MHz. Add support for it upstream, as the narrower is the
bandwidth filter, the better.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---

To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
See [PATCH 0/3] at: https://lore.kernel.org/all/cover.1639140967.git.mchehab+huawei@kernel.org/

drivers/media/tuners/si2157.c | 4 ++++
drivers/media/tuners/si2157_priv.h | 5 +++++
2 files changed, 9 insertions(+)

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index aeecb38b147f..2d3937af4f5f 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -458,8 +458,12 @@ static int si2157_set_params(struct dvb_frontend *fe)
goto err;
}

+ if (SUPPORTS_1700KHz(dev) && c->bandwidth_hz <= 1700000)
+ bandwidth = 0x09;
if (c->bandwidth_hz <= 6000000)
bandwidth = 0x06;
+ if (SUPPORTS_1700KHz(dev) && c->bandwidth_hz <= 6100000)
+ bandwidth = 0x10;
else if (c->bandwidth_hz <= 7000000)
bandwidth = 0x07;
else if (c->bandwidth_hz <= 8000000)
diff --git a/drivers/media/tuners/si2157_priv.h b/drivers/media/tuners/si2157_priv.h
index df17a5f03561..24849c8ed398 100644
--- a/drivers/media/tuners/si2157_priv.h
+++ b/drivers/media/tuners/si2157_priv.h
@@ -66,6 +66,11 @@ struct si2157_cmd {
unsigned rlen;
};

+#define SUPPORTS_1700KHz(dev) (((dev)->part_id == SI2141) || \
+ ((dev)->part_id == SI2147) || \
+ ((dev)->part_id == SI2157) || \
+ ((dev)->part_id == SI2177))
+
/* Old firmware namespace */
#define SI2158_A20_FIRMWARE "dvb-tuner-si2158-a20-01.fw"
#define SI2141_A10_FIRMWARE "dvb-tuner-si2141-a10-01.fw"
--
2.33.1
 

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH] media: si6157: fix a tune regression for 6.1MHz
  2022-01-06 18:25     ` Robert Schlabbach
@ 2022-01-06 19:44       ` Mauro Carvalho Chehab
  2022-01-06 20:13         ` Mauro Carvalho Chehab
  2022-01-06 20:30       ` [PATCH 2/3] media: si2157: add support for 1.7MHz and 6.1 MHz Mauro Carvalho Chehab
  1 sibling, 1 reply; 9+ messages in thread
From: Mauro Carvalho Chehab @ 2022-01-06 19:44 UTC (permalink / raw)
  To: linux-kernel
  Cc: Mauro Carvalho Chehab, Robert Schlabbach, Antti Palosaari, linux-media

The patch which added support for 1.7MHz and 6.1 MHz has
two issues: there's a missing else for 1.7 and the value
for 6.1 MHz filter is decimal 10 (0x0a).

Fix those.

Reported-by: Robert Schlabbach <Robert.Schlabbach@gmx.net>
Fixes: 98c65a3dac95 ("media: si2157: add support for 1.7MHz and 6.1 MHz")
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
---
 drivers/media/tuners/si2157.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
index 481c5c3b577d..a484239333ef 100644
--- a/drivers/media/tuners/si2157.c
+++ b/drivers/media/tuners/si2157.c
@@ -460,10 +460,10 @@ static int si2157_set_params(struct dvb_frontend *fe)
 
 	if (SUPPORTS_1700KHz(dev) && c->bandwidth_hz <= 1700000)
 		bandwidth = 0x09;
-	if (c->bandwidth_hz <= 6000000)
+	else if (c->bandwidth_hz <= 6000000)
 		bandwidth = 0x06;
-	if (SUPPORTS_1700KHz(dev) && c->bandwidth_hz <= 6100000)
-		bandwidth = 0x10;
+	else if (SUPPORTS_1700KHz(dev) && c->bandwidth_hz <= 6100000)
+		bandwidth = 0x0a;
 	else if (c->bandwidth_hz <= 7000000)
 		bandwidth = 0x07;
 	else if (c->bandwidth_hz <= 8000000)
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] media: si6157: fix a tune regression for 6.1MHz
  2022-01-06 19:44       ` [PATCH] media: si6157: fix a tune regression for 6.1MHz Mauro Carvalho Chehab
@ 2022-01-06 20:13         ` Mauro Carvalho Chehab
  0 siblings, 0 replies; 9+ messages in thread
From: Mauro Carvalho Chehab @ 2022-01-06 20:13 UTC (permalink / raw)
  To: linux-kernel; +Cc: Robert Schlabbach, Antti Palosaari, linux-media

Em Thu,  6 Jan 2022 20:44:29 +0100
Mauro Carvalho Chehab <mchehab@kernel.org> escreveu:

> The patch which added support for 1.7MHz and 6.1 MHz has
> two issues: there's a missing else for 1.7 and the value
> for 6.1 MHz filter is decimal 10 (0x0a).
> 
> Fix those.

Please ignore this one. I was too quick writing it: it doesn't
address all reported issues.

Regards,
Mauro

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3] media: si2157: add support for 1.7MHz and 6.1 MHz
  2022-01-06 18:25     ` Robert Schlabbach
  2022-01-06 19:44       ` [PATCH] media: si6157: fix a tune regression for 6.1MHz Mauro Carvalho Chehab
@ 2022-01-06 20:30       ` Mauro Carvalho Chehab
  1 sibling, 0 replies; 9+ messages in thread
From: Mauro Carvalho Chehab @ 2022-01-06 20:30 UTC (permalink / raw)
  To: Robert Schlabbach
  Cc: Mauro Carvalho Chehab, Antti Palosaari, linux-kernel, linux-media

Em Thu, 6 Jan 2022 19:25:25 +0100
Robert Schlabbach <Robert.Schlabbach@gmx.net> escreveu:

> It turns out there are actually two bugs in there:
>  
> + if (SUPPORTS_1700KHz(dev) && c->bandwidth_hz <= 6100000)
> + bandwidth = 0x10;
> 
> The si2157 code for bandwidth 6.1MHz is _decimal_ 10, not
> hexadecimal 0x10. Removing the "0x" from the above line makes
> the tuner work again, even with the other bug that makes it use
> the 6.1MHz setting when 6MHz is requested.

Gah, true.

> Another issue with the bandwidth setting: The si2157 code is
> also stored in dev->bandwidth and returned in the get_bandwidth()
> call. Now this was not well before, because the call is supposed
> to return the bandwidth in Hz, not in MHz, but now it can return
> 9 and 10, but those do not correspond to 9 and 10MHz.

I suspect that the entire get_bandwidth() code on drivers are
dead code, as the core doesn't call it anymore. This used to be
needed before DVBv5 API.

Probably, the right fix here would be to simply strip this function
from all drivers.

> Also, the default case uses si2157 code 0x0f, which also seems
> like a bad idea.

Yes. it should default to the maximum bandwidth (8MHz on those
chipsets).

> And another, unrelated issue: The delivery_system switch is
> missing case DVBC_ANNEX_C, even though it should operate just as
> DVBC_ANNEX_A.

The rolloff is different for Annex/A and Annex/C. Annex/A uses
1.15, while Annex/C uses 1.13.

The DVB core already handles it when it calculates the bandwidth
for a given symbol rate, though, so, it shouldn't make any
difference at the tuner, except if the rolloff filter there
can be adjusted.

Btw, I suspect that the 6.1 MHz bandwidth could be there due to
Annex/A and Annex/C differences.

> I'll see if I can come up with a patch which addresses all of the
> above.

OK! I'll wait for your patch.

> 
> Best Regards,
> -Robert Schlabbach 
> 
> 
> Gesendet: Donnerstag, 06. Januar 2022 um 16:39 Uhr
> Von: "Robert Schlabbach" <Robert.Schlabbach@gmx.net>
> An: "Mauro Carvalho Chehab" <mchehab+huawei@kernel.org>
> Cc: mauro.chehab@huawei.com, "Antti Palosaari" <crope@iki.fi>, linux-kernel@vger.kernel.org, linux-media@vger.kernel.org
> Betreff: Re: [PATCH 2/3] media: si2157: add support for 1.7MHz and 6.1 MHz
> 
> Sorry for the late test and response, but there is a BAD BUG in this patch:
>  
> + if (SUPPORTS_1700KHz(dev) && c->bandwidth_hz <= 1700000)
> + bandwidth = 0x09;
> if (c->bandwidth_hz <= 6000000)
> bandwidth = 0x06;
> + if (SUPPORTS_1700KHz(dev) && c->bandwidth_hz <= 6100000)
> + bandwidth = 0x10;
> else if (c->bandwidth_hz <= 7000000)
> bandwidth = 0x07;
> else if (c->bandwidth_hz <= 8000000)
> 
> The additions omitted the "else", which resulted in the bandwidth setting for
> 6MHz being overwritten with the one for 6.1MHz - and that completely breaks
> 6MHz channels, as the tuner then refuses to accept the tune command!
> 
> As a result, e.g. MCNS aka ClearQAM aka DVB-C Annex B no longer works after
> this patch.
> 
> I don't know if the 1.7Mhz and 6.1MHz settings are even usable, if the tuner
> (in my case, the Si2157-A30) then no longer accepts the tune command. Maybe
> they're not suited for frequency-based tuning, but only for channel-based
> tuning? That would not fit the DVB-API, I think.
> 
> And the 1.7MHz bandwidth setting currently can't do any harm, as it is always
> overwritten by the 6MHz bandwidth setting, also due to an "else" missing.
> 
> Best Regards,
> -Robert Schlabbach
>  
>  
> 
> Gesendet: Freitag, 10. Dezember 2021 um 13:59 Uhr
> Von: "Mauro Carvalho Chehab" <mchehab+huawei@kernel.org>
> An: unlisted-recipients:;
> Cc: linuxarm@huawei.com, mauro.chehab@huawei.com, "Mauro Carvalho Chehab" <mchehab+huawei@kernel.org>, "Antti Palosaari" <crope@iki.fi>, "Mauro Carvalho Chehab" <mchehab@kernel.org>, linux-kernel@vger.kernel.org, linux-media@vger.kernel.org
> Betreff: [PATCH 2/3] media: si2157: add support for 1.7MHz and 6.1 MHz
> Some tuners allow setting the bandwidth filter to 1.7MHz and
> 6.1 MHz. Add support for it upstream, as the narrower is the
> bandwidth filter, the better.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---
> 
> To avoid mailbombing on a large number of people, only mailing lists were C/C on the cover.
> See [PATCH 0/3] at: https://lore.kernel.org/all/cover.1639140967.git.mchehab+huawei@kernel.org/
> 
> drivers/media/tuners/si2157.c | 4 ++++
> drivers/media/tuners/si2157_priv.h | 5 +++++
> 2 files changed, 9 insertions(+)
> 
> diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c
> index aeecb38b147f..2d3937af4f5f 100644
> --- a/drivers/media/tuners/si2157.c
> +++ b/drivers/media/tuners/si2157.c
> @@ -458,8 +458,12 @@ static int si2157_set_params(struct dvb_frontend *fe)
> goto err;
> }
> 
> + if (SUPPORTS_1700KHz(dev) && c->bandwidth_hz <= 1700000)
> + bandwidth = 0x09;
> if (c->bandwidth_hz <= 6000000)
> bandwidth = 0x06;
> + if (SUPPORTS_1700KHz(dev) && c->bandwidth_hz <= 6100000)
> + bandwidth = 0x10;
> else if (c->bandwidth_hz <= 7000000)
> bandwidth = 0x07;
> else if (c->bandwidth_hz <= 8000000)
> diff --git a/drivers/media/tuners/si2157_priv.h b/drivers/media/tuners/si2157_priv.h
> index df17a5f03561..24849c8ed398 100644
> --- a/drivers/media/tuners/si2157_priv.h
> +++ b/drivers/media/tuners/si2157_priv.h
> @@ -66,6 +66,11 @@ struct si2157_cmd {
> unsigned rlen;
> };
> 
> +#define SUPPORTS_1700KHz(dev) (((dev)->part_id == SI2141) || \
> + ((dev)->part_id == SI2147) || \
> + ((dev)->part_id == SI2157) || \
> + ((dev)->part_id == SI2177))
> +
> /* Old firmware namespace */
> #define SI2158_A20_FIRMWARE "dvb-tuner-si2158-a20-01.fw"
> #define SI2141_A10_FIRMWARE "dvb-tuner-si2141-a10-01.fw"
> --
> 2.33.1
>  



Thanks,
Mauro

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-01-06 20:30 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-10 12:59 [PATCH 0/3] media: si2157: do some minor improvements at the driver Mauro Carvalho Chehab
2021-12-10 12:59 ` [PATCH 1/3] media: si2157: add support for ISDB-T and DTMB Mauro Carvalho Chehab
2021-12-10 12:59 ` [PATCH 2/3] media: si2157: add support for 1.7MHz and 6.1 MHz Mauro Carvalho Chehab
2022-01-06 15:39   ` Robert Schlabbach
2022-01-06 18:25     ` Robert Schlabbach
2022-01-06 19:44       ` [PATCH] media: si6157: fix a tune regression for 6.1MHz Mauro Carvalho Chehab
2022-01-06 20:13         ` Mauro Carvalho Chehab
2022-01-06 20:30       ` [PATCH 2/3] media: si2157: add support for 1.7MHz and 6.1 MHz Mauro Carvalho Chehab
2021-12-10 12:59 ` [PATCH 3/3] media: si2157: add ATV support for si2158 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).