Re: [RFC v2] DT-based tuner/demod init

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/07/2019 18:21, Marc Gonzalez wrote:

> This is a follow-up RFC to my first request for comments:
> "[RFC] SW connection between DVB Transport Stream demuxer and I2C-based frontend"
> https://www.spinics.net/lists/arm-kernel/msg739972.html
> 
> Background: my SoC provides a "Transport Stream Interface" on-chip
> (for which I wrote a small driver, tsif.c) as well as a tuner/demod combo
> (si2141/si2168) on the board.
> 
> My original goal was: being able to link the tuner/demod from the device tree,
> instead of hard-coding them in the TSIF driver.
> 
> (Please see the resulting code at the end of this message)

Below is an analysis of the proposed driver, after a few exchanges with Mauro
on IRC.


> diff --git a/drivers/media/platform/tsif.c b/drivers/media/platform/tsif.c
> new file mode 100644
> index 000000000000..b136f334e9c6
> --- /dev/null
> +++ b/drivers/media/platform/tsif.c
> @@ -0,0 +1,185 @@
> +#include <linux/clk.h>
> +#include <linux/interrupt.h>
> +#include <linux/platform_device.h>
> +#include <media/dvb_frontend.h>
> +#include <media/dvb_demux.h>
> +#include <media/dmxdev.h>
> +
> +/* TSIF register offsets */
> +#define TSIF_STS_CTL	0x0	/* status and control */
> +#define TSIF_DATA_PORT	0x100
> +
> +/* TSIF_STS_CTL bits */
> +#define ENABLE_IRQ	BIT(28)
> +#define TSIF_STOP	BIT(3)
> +#define TSIF_START	BIT(0)
> +
> +struct tsif {
> +	void __iomem *base;
> +	struct clk *clk;
> +	int ref_count; /*** TODO: use atomic_t ??? or refcount_t ??? or kref ??? ***/
> +	u32 buf[48];
> +	struct dvb_frontend *fe;
> +	/*** DO I NEED ALL 4 ***/
> +	//struct dmx_frontend dmx_frontend;
> +	struct dvb_adapter dvb_adapter;
> +	struct dvb_demux dvb_demux;
> +	struct dmxdev dmxdev;
> +};
> +
> +static int start_tsif(struct dvb_demux_feed *feed)
> +{
> +	struct tsif *tsif = feed->demux->priv;
> +	printk("%s: feed PID=%u\n", __func__, feed->pid);
> +
> +	if (tsif->ref_count++ == 0) {
> +		u32 val = TSIF_START | ENABLE_IRQ | BIT(29);
> +		writel_relaxed(val, tsif->base + TSIF_STS_CTL);
> +	}
> +
> +	return 0;
> +}
> +
> +static int stop_tsif(struct dvb_demux_feed *feed)
> +{
> +	struct tsif *tsif = feed->demux->priv;
> +	printk("%s: feed PID=%u\n", __func__, feed->pid);
> +
> +	if (--tsif->ref_count == 0) {
> +		writel_relaxed(TSIF_STOP, tsif->base + TSIF_STS_CTL);
> +	}
> +
> +	return 0;
> +}
> +
> +static irqreturn_t tsif_isr(int irq, void *arg)
> +{
> +	int i;
> +	u32 status;
> +	struct tsif *tsif = arg;
> +
> +	status = readl_relaxed(tsif->base + TSIF_STS_CTL);
> +	writel_relaxed(status, tsif->base + TSIF_STS_CTL);
> +
> +	for (i = 0; i < 48; ++i)
> +		tsif->buf[i] = readl_relaxed(tsif->base + TSIF_DATA_PORT);
> +
> +	dvb_dmx_swfilter_packets(&tsif->dvb_demux, (void *)tsif->buf, 1);
> +
> +	return IRQ_HANDLED;
> +}

What may not be apparent here is that (in this mode) the HW generates
one interrupt for *every* *single* TS packet (i.e. 1504 bits). And it
can buffer only a single packet. Pretty hard real-time constraints...

Since I'm dealing with 25 Mbps streams (French DVB-T2 multiplex)
25*10e6 / 1504 = 16600 packets per second -- i.e. 60 µs between IRQs

Even after:

1) moving the ISR to its own dedicated core,
2) moving dvb_dmx_swfilter_packets() to a work_queue,
3) removing interrupt masking from DVB functions,
4) using large SW buffers (1024 TS packets)

I still drop a few packets here and there (~1 per minute).

Conclusion: it seems this HW mode cannot work reliably on the types
of streams I'm interested in. Thus, there's no point in supporting it
in the final driver. I need to test "advanced" mode.

Regards.



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux