[PATCH 0/4] SkystarS2 pid filtering fix and stream control.

[PATCH 0/4] SkystarS2 pid filtering fix and stream control.

[Jemma Denson]

This patch series finishes off the addition of the SkyStarS2 card -
the patches here aren't strictly required for running the card so
haven't previously been included.

The first patch fixes a bug present in the current flexcop driver -
I've seen it with this card, and it has also been noticed in some
other cards. Those cards will need identifying and patches created to
use this fix as it is not enabled by default.

The other three patches add in a feature that was half complete in the
original binary blob for this card, and allows the demod chip to
control the receive stream on the b2c2 when it turns off it's output
during tuning. I'm not sure it actually needs to turn it's output
stream off, and the patches also make it easy to disable this.

Jemma Denson (4):
  b2c2: Add option to skip the first 6 pid filters
  b2c2: Allow external stream control
  cx24120: Take control of b2c2 receive stream
  b2c2: Always turn off receive stream

 drivers/media/common/b2c2/flexcop-common.h    |  3 +++
 drivers/media/common/b2c2/flexcop-fe-tuner.c  | 13 ++++++++++++
 drivers/media/common/b2c2/flexcop-hw-filter.c | 22 ++++++++++++++++----
 drivers/media/dvb-frontends/cx24120.c         | 29 ++++++++++++++++++---------
 drivers/media/dvb-frontends/cx24120.h         |  1 +
 5 files changed, 55 insertions(+), 13 deletions(-)

-- 
2.1.0

[PATCH 1/4] b2c2: Add option to skip the first 6 pid filters

[Jemma Denson]

The flexcop bridge chip has two banks of hardware pid filters -
an initial 6, and on some chip revisions an additional bank of 32.

A bug is present on the initial 6 - when changing transponders
one of two PAT packets from the old transponder would be included
in the initial packets from the new transponder. This usually
transpired with userspace programs complaining about services
missing, because they are seeing a PAT that they would not be
expecting. Running in full TS mode does not exhibit this problem,
neither does using just the additional 32.

This patch adds in an option to not use the inital 6 and solely use
just the additional 32, and enables this option for the SkystarS2
card. Other cards can be added as required if they also have
this bug.

Signed-off-by: Jemma Denson <jdenson@gmail.com>
---
 drivers/media/common/b2c2/flexcop-common.h    |  1 +
 drivers/media/common/b2c2/flexcop-fe-tuner.c  |  3 +++
 drivers/media/common/b2c2/flexcop-hw-filter.c | 16 ++++++++++++++--
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/media/common/b2c2/flexcop-common.h b/drivers/media/common/b2c2/flexcop-common.h
index 437912e..2b2460e 100644
--- a/drivers/media/common/b2c2/flexcop-common.h
+++ b/drivers/media/common/b2c2/flexcop-common.h
@@ -91,6 +91,7 @@ struct flexcop_device {
 	int feedcount;
 	int pid_filtering;
 	int fullts_streaming_state;
+	int skip_6_hw_pid_filter;
 
 	/* bus specific callbacks */
 	flexcop_ibi_value(*read_ibi_reg) (struct flexcop_device *,
diff --git a/drivers/media/common/b2c2/flexcop-fe-tuner.c b/drivers/media/common/b2c2/flexcop-fe-tuner.c
index 2426062..31ebf1e 100644
--- a/drivers/media/common/b2c2/flexcop-fe-tuner.c
+++ b/drivers/media/common/b2c2/flexcop-fe-tuner.c
@@ -651,6 +651,9 @@ static int skystarS2_rev33_attach(struct flexcop_device *fc,
 	}
 	info("ISL6421 successfully attached.");
 
+	if (fc->has_32_hw_pid_filter)
+		fc->skip_6_hw_pid_filter = 1;
+
 	return 1;
 }
 #else
diff --git a/drivers/media/common/b2c2/flexcop-hw-filter.c b/drivers/media/common/b2c2/flexcop-hw-filter.c
index 77e4547..8220257 100644
--- a/drivers/media/common/b2c2/flexcop-hw-filter.c
+++ b/drivers/media/common/b2c2/flexcop-hw-filter.c
@@ -117,6 +117,10 @@ static void flexcop_pid_control(struct flexcop_device *fc,
 	deb_ts("setting pid: %5d %04x at index %d '%s'\n",
 			pid, pid, index, onoff ? "on" : "off");
 
+	/* First 6 can be buggy - skip over them if option set */
+	if (fc->skip_6_hw_pid_filter)
+		index += 6;
+
 	/* We could use bit magic here to reduce source code size.
 	 * I decided against it, but to use the real register names */
 	switch (index) {
@@ -170,7 +174,10 @@ static int flexcop_toggle_fullts_streaming(struct flexcop_device *fc, int onoff)
 int flexcop_pid_feed_control(struct flexcop_device *fc,
 		struct dvb_demux_feed *dvbdmxfeed, int onoff)
 {
-	int max_pid_filter = 6 + fc->has_32_hw_pid_filter*32;
+	int max_pid_filter = 6;
+
+	max_pid_filter -= 6 * fc->skip_6_hw_pid_filter;
+	max_pid_filter += 32 * fc->has_32_hw_pid_filter;
 
 	fc->feedcount += onoff ? 1 : -1; /* the number of PIDs/Feed currently requested */
 	if (dvbdmxfeed->index >= max_pid_filter)
@@ -217,7 +224,12 @@ void flexcop_hw_filter_init(struct flexcop_device *fc)
 {
 	int i;
 	flexcop_ibi_value v;
-	for (i = 0; i < 6 + 32*fc->has_32_hw_pid_filter; i++)
+	int max_pid_filter = 6;
+
+	max_pid_filter -= 6 * fc->skip_6_hw_pid_filter;
+	max_pid_filter += 32 * fc->has_32_hw_pid_filter;
+
+	for (i = 0; i < max_pid_filter; i++)
 		flexcop_pid_control(fc, i, 0x1fff, 0);
 
 	flexcop_pid_group_filter(fc, 0, 0x1fe0);
-- 
2.1.0

[PATCH 2/4] b2c2: Allow external stream control

[Jemma Denson]

This patch brings in a feature present in the gpl patch portion of
the SkystarS2 driver available for download from Technisat. The
patch is identical save for renaming the configuration variable,
and also directly exposing the flexcop_rcv_data_ctrl() instead of
wrapping it around a separate function.

The feature added is to allow passing control of the flexcop
receive stream to another device, such as the demod chip.

Signed-off-by: Jemma Denson <jdenson@gmail.com>
---
 drivers/media/common/b2c2/flexcop-common.h    | 2 ++
 drivers/media/common/b2c2/flexcop-hw-filter.c | 6 ++++--
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/media/common/b2c2/flexcop-common.h b/drivers/media/common/b2c2/flexcop-common.h
index 2b2460e..7075043 100644
--- a/drivers/media/common/b2c2/flexcop-common.h
+++ b/drivers/media/common/b2c2/flexcop-common.h
@@ -91,6 +91,7 @@ struct flexcop_device {
 	int feedcount;
 	int pid_filtering;
 	int fullts_streaming_state;
+	int external_stream_control;
 	int skip_6_hw_pid_filter;
 
 	/* bus specific callbacks */
@@ -174,6 +175,7 @@ void flexcop_dump_reg(struct flexcop_device *fc,
 		flexcop_ibi_register reg, int num);
 
 /* from flexcop-hw-filter.c */
+void flexcop_rcv_data_ctrl(struct flexcop_device *fc, int onoff);
 int flexcop_pid_feed_control(struct flexcop_device *fc,
 		struct dvb_demux_feed *dvbdmxfeed, int onoff);
 void flexcop_hw_filter_init(struct flexcop_device *fc);
diff --git a/drivers/media/common/b2c2/flexcop-hw-filter.c b/drivers/media/common/b2c2/flexcop-hw-filter.c
index 8220257..eceb9c5 100644
--- a/drivers/media/common/b2c2/flexcop-hw-filter.c
+++ b/drivers/media/common/b2c2/flexcop-hw-filter.c
@@ -5,7 +5,7 @@
  */
 #include "flexcop.h"
 
-static void flexcop_rcv_data_ctrl(struct flexcop_device *fc, int onoff)
+void flexcop_rcv_data_ctrl(struct flexcop_device *fc, int onoff)
 {
 	flexcop_set_ibi_value(ctrl_208, Rcv_Data_sig, onoff);
 	deb_ts("rcv_data is now: '%s'\n", onoff ? "on" : "off");
@@ -206,7 +206,9 @@ int flexcop_pid_feed_control(struct flexcop_device *fc,
 
 	/* if it was the first or last feed request change the stream-status */
 	if (fc->feedcount == onoff) {
-		flexcop_rcv_data_ctrl(fc, onoff);
+		if (!fc->external_stream_control)
+			flexcop_rcv_data_ctrl(fc, onoff);
+
 		if (fc->stream_control) /* device specific stream control */
 			fc->stream_control(fc, onoff);
 
-- 
2.1.0

[PATCH 3/4] cx24120: Take control of b2c2 receive stream

[Jemma Denson]

Now that b2c2 has an option to allow us to do so, turn off the
flexcop receive stream when we turn off mpeg output whilst tuning.

This "turning off whilst tuning" feature is a bit unusual, and can
now also be easily disabled by leaving stream_control unset in the
config struct.

Signed-off-by: Jemma Denson <jdenson@gmail.com>
---
 drivers/media/common/b2c2/flexcop-fe-tuner.c | 10 ++++++++++
 drivers/media/dvb-frontends/cx24120.c        | 29 +++++++++++++++++++---------
 drivers/media/dvb-frontends/cx24120.h        |  1 +
 3 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/drivers/media/common/b2c2/flexcop-fe-tuner.c b/drivers/media/common/b2c2/flexcop-fe-tuner.c
index 31ebf1e..fdd3a18 100644
--- a/drivers/media/common/b2c2/flexcop-fe-tuner.c
+++ b/drivers/media/common/b2c2/flexcop-fe-tuner.c
@@ -625,11 +625,20 @@ fail:
 
 /* SkyStar S2 PCI DVB-S/S2 card based on Conexant cx24120/cx24118 */
 #if FE_SUPPORTED(CX24120) && FE_SUPPORTED(ISL6421)
+static int skystarS2_stream_control(struct dvb_frontend *fe, int onoff)
+{
+	struct flexcop_device *fc = fe->dvb->priv;
+
+	flexcop_rcv_data_ctrl(fc, onoff);
+	return 0;
+}
+
 static const struct cx24120_config skystar2_rev3_3_cx24120_config = {
 	.i2c_addr = 0x55,
 	.xtal_khz = 10111,
 	.initial_mpeg_config = { 0xa1, 0x76, 0x07 },
 	.request_firmware = flexcop_fe_request_firmware,
+	.stream_control = skystarS2_stream_control,
 	.i2c_wr_max = 4,
 };
 
@@ -651,6 +660,7 @@ static int skystarS2_rev33_attach(struct flexcop_device *fc,
 	}
 	info("ISL6421 successfully attached.");
 
+	fc->external_stream_control = 1;
 	if (fc->has_32_hw_pid_filter)
 		fc->skip_6_hw_pid_filter = 1;
 
diff --git a/drivers/media/dvb-frontends/cx24120.c b/drivers/media/dvb-frontends/cx24120.c
index a14d0f1..d625c1c 100644
--- a/drivers/media/dvb-frontends/cx24120.c
+++ b/drivers/media/dvb-frontends/cx24120.c
@@ -371,6 +371,8 @@ static void cx24120_check_cmd(struct cx24120_state *state, u8 id)
 	case CMD_DISEQC_BURST:
 		cx24120_msg_mpeg_output_global_config(state, 0);
 		/* Old driver would do a msleep(100) here */
+		state->config->stream_control(&state->frontend, 0);
+		state->mpeg_enabled = 0;
 	default:
 		return;
 	}
@@ -382,10 +384,9 @@ static int cx24120_message_send(struct cx24120_state *state,
 {
 	int ficus;
 
-	if (state->mpeg_enabled) {
-		/* Disable mpeg out on certain commands */
+	/* If controlling stream, turn if off whilst tuning */
+	if (state->config->stream_control && state->mpeg_enabled)
 		cx24120_check_cmd(state, cmd->id);
-	}
 
 	cx24120_writereg(state, CX24120_REG_CMD_START, cmd->id);
 	cx24120_writeregs(state, CX24120_REG_CMD_ARGS, &cmd->arg[0],
@@ -464,7 +465,6 @@ static int cx24120_msg_mpeg_output_global_config(struct cx24120_state *state,
 		return ret;
 	}
 
-	state->mpeg_enabled = enable;
 	dev_dbg(&state->i2c->dev, "MPEG output %s\n",
 		enable ? "enabled" : "disabled");
 
@@ -743,11 +743,13 @@ static int cx24120_read_status(struct dvb_frontend *fe, fe_status_t *status)
 		/* Set clock ratios */
 		cx24120_set_clock_ratios(fe);
 
-		/* Old driver would do a msleep(200) here */
-
 		/* Renable mpeg output */
-		if (!state->mpeg_enabled)
+		if (state->config->stream_control && !state->mpeg_enabled) {
+			/* Old driver would do a msleep(200) here */
 			cx24120_msg_mpeg_output_global_config(state, 1);
+			state->config->stream_control(fe, 1);
+			state->mpeg_enabled = 1;
+		}
 
 		state->need_clock_set = 0;
 	}
@@ -1418,8 +1420,17 @@ static int cx24120_init(struct dvb_frontend *fe)
 
 	/* Initialise mpeg outputs */
 	cx24120_writereg(state, 0xeb, 0x0a);
-	if (cx24120_msg_mpeg_output_global_config(state, 0) ||
-	    cx24120_msg_mpeg_output_config(state, 0) ||
+
+	/* Enable global output now if we're not doing stream control */
+	if (!state->config->stream_control)
+		ret = cx24120_msg_mpeg_output_global_config(state, 1);
+	else
+		ret = cx24120_msg_mpeg_output_global_config(state, 0);
+
+	if (ret != 0)
+		return ret;
+
+	if (cx24120_msg_mpeg_output_config(state, 0) ||
 	    cx24120_msg_mpeg_output_config(state, 1) ||
 	    cx24120_msg_mpeg_output_config(state, 2)) {
 		err("Error initialising mpeg output. :(\n");
diff --git a/drivers/media/dvb-frontends/cx24120.h b/drivers/media/dvb-frontends/cx24120.h
index f097042..313518c 100644
--- a/drivers/media/dvb-frontends/cx24120.h
+++ b/drivers/media/dvb-frontends/cx24120.h
@@ -37,6 +37,7 @@ struct cx24120_config {
 
 	int (*request_firmware)(struct dvb_frontend *fe,
 				const struct firmware **fw, char *name);
+	int (*stream_control)(struct dvb_frontend *fe, int onoff);
 
 	/* max bytes I2C provider can write at once */
 	u16 i2c_wr_max;
-- 
2.1.0

Re: [PATCH 3/4] cx24120: Take control of b2c2 receive stream

[Patrick Boettcher]

Hi Jemma,

On Fri, 22 May 2015 21:28:27 +0100 Jemma Denson <jdenson@gmail.com>
wrote:

> Now that b2c2 has an option to allow us to do so, turn off the
> flexcop receive stream when we turn off mpeg output whilst tuning.

Does this not fix (and your '[PATCH 2/4]') the problem of receiving
PAT from the previously tuned transport-stream?

Then patch 1 and 4 should not be necessary, should they?!

--
Patrick.

Re: [PATCH 3/4] cx24120: Take control of b2c2 receive stream

[Jemma Denson]

Hi Patrick,

On 26/05/15 10:05, Patrick Boettcher wrote:
>> Now that b2c2 has an option to allow us to do so, turn off the
>> flexcop receive stream when we turn off mpeg output whilst tuning.
> Does this not fix (and your '[PATCH 2/4]') the problem of receiving
> PAT from the previously tuned transport-stream?
>
> Then patch 1 and 4 should not be necessary, should they?!

Only patch 1 fixes that problem, so out of the 4 here that one is the 
most necessary. Controlling the flexcop receive stream and/or stopping 
the cx24120 from sending doesn't actually appear to do much of anything 
- it doesn't seem any better or worse doing one or the other, both or 
even neither! (Apart from Patch 4 breaking things, as mentioned).

I'm including it though because I presume the reference driver advised 
it was done, and it does tidy up the cx24120 codebase considerably by 
being able to disable the whole turn off sending the stream whilst 
tuning feature - I'm envisioning that in the future someone might want 
to take on the task of merging cx24117 & cx24120 as they're quite 
similar, and allowing what seems to just be a flexcop oddity to be 
turned off would make this possible.


Jemma.

Re: [PATCH 3/4] cx24120: Take control of b2c2 receive stream

[Mauro Carvalho Chehab]

Em Tue, 26 May 2015 10:21:11 +0100
Jemma Denson <jdenson@gmail.com> escreveu:

> Hi Patrick,
> 
> On 26/05/15 10:05, Patrick Boettcher wrote:
> >> Now that b2c2 has an option to allow us to do so, turn off the
> >> flexcop receive stream when we turn off mpeg output whilst tuning.
> > Does this not fix (and your '[PATCH 2/4]') the problem of receiving
> > PAT from the previously tuned transport-stream?
> >
> > Then patch 1 and 4 should not be necessary, should they?!
> 
> Only patch 1 fixes that problem, so out of the 4 here that one is the 
> most necessary. Controlling the flexcop receive stream and/or stopping 
> the cx24120 from sending doesn't actually appear to do much of anything 
> - it doesn't seem any better or worse doing one or the other, both or 
> even neither! (Apart from Patch 4 breaking things, as mentioned).
> 
> I'm including it though because I presume the reference driver advised 
> it was done, and it does tidy up the cx24120 codebase considerably by 
> being able to disable the whole turn off sending the stream whilst 
> tuning feature - I'm envisioning that in the future someone might want 
> to take on the task of merging cx24117 & cx24120 as they're quite 
> similar, and allowing what seems to just be a flexcop oddity to be 
> turned off would make this possible.

Hmm... if patch 1 is enough to fix the issue, and patch 4 may break
things, I'll apply only patch 1/4 for now.

If Patrick agrees, and you find a way to avoid breakages, please
resubmit for me to apply the other ones.

Regards,
Mauro

> 
> 
> Jemma.

Re: [PATCH 3/4] cx24120: Take control of b2c2 receive stream

[Jemma Denson]

Hi Mauro,

On 10/06/15 00:56, Mauro Carvalho Chehab wrote:
> Hmm... if patch 1 is enough to fix the issue, and patch 4 may break 
> things, I'll apply only patch 1/4 for now. If Patrick agrees, and you 
> find a way to avoid breakages, please resubmit for me to apply the 
> other ones. Regards, Mauro 

I already posted a v2 of this patch series which does the stream control 
in a much better way - that v2 version has been running on my mythtv 
recording box for a few weeks now without issue.

Jemma.

[PATCH 4/4] b2c2: Always turn off receive stream

[Jemma Denson]

When letting an external device control the receive stream, it won't
know when there's demand for any feeds, so won't be turning off our
receive stream. This patch bring back control of turning it off in
this sitation.

The demod can still delay turning it on until it has data to send,
and still turn it off temporarily whilst it knows there's no
stream, such as whilst tuning.

Signed-off-by: Jemma Denson <jdenson@gmail.com>
---
 drivers/media/common/b2c2/flexcop-hw-filter.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/common/b2c2/flexcop-hw-filter.c b/drivers/media/common/b2c2/flexcop-hw-filter.c
index eceb9c5..8926c82 100644
--- a/drivers/media/common/b2c2/flexcop-hw-filter.c
+++ b/drivers/media/common/b2c2/flexcop-hw-filter.c
@@ -206,7 +206,7 @@ int flexcop_pid_feed_control(struct flexcop_device *fc,
 
 	/* if it was the first or last feed request change the stream-status */
 	if (fc->feedcount == onoff) {
-		if (!fc->external_stream_control)
+		if (!fc->external_stream_control || onoff == 0)
 			flexcop_rcv_data_ctrl(fc, onoff);
 
 		if (fc->stream_control) /* device specific stream control */
-- 
2.1.0

Re: [PATCH 4/4] b2c2: Always turn off receive stream

[Jemma Denson]

On 22/05/15 21:28, Jemma Denson wrote:
> When letting an external device control the receive stream, it won't
> know when there's demand for any feeds, so won't be turning off our
> receive stream. This patch bring back control of turning it off in
> this sitation.
>
> The demod can still delay turning it on until it has data to send,
> and still turn it off temporarily whilst it knows there's no
> stream, such as whilst tuning.
>
> Signed-off-by: Jemma Denson <jdenson@gmail.com>
> ---
>   drivers/media/common/b2c2/flexcop-hw-filter.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/common/b2c2/flexcop-hw-filter.c b/drivers/media/common/b2c2/flexcop-hw-filter.c
> index eceb9c5..8926c82 100644
> --- a/drivers/media/common/b2c2/flexcop-hw-filter.c
> +++ b/drivers/media/common/b2c2/flexcop-hw-filter.c
> @@ -206,7 +206,7 @@ int flexcop_pid_feed_control(struct flexcop_device *fc,
>   
>   	/* if it was the first or last feed request change the stream-status */
>   	if (fc->feedcount == onoff) {
> -		if (!fc->external_stream_control)
> +		if (!fc->external_stream_control || onoff == 0)
>   			flexcop_rcv_data_ctrl(fc, onoff);
>   
>   		if (fc->stream_control) /* device specific stream control */

Hmm, OK. I've done some further testing and this last patch needs either 
ignoring or a rethink. It interferes with flexcop_pci_irq_check_work() 
in pci/b2c2/flexcop-pci.c. That function will try and reset all the hw 
filters with calls to flexcop_pid_feed_control() by turning them all off 
and then back on again.
Including this patch causes the receive stream to be turned off and it 
then doesn't get enabled again.


Jemma.