2020-05-20 07:08:00

by Daniel Almeida

[permalink] [raw]
Subject: [RFC, WIP, v6 00/10] media: vidtv: implement a virtual DVB driver

From: "Daniel W. S. Almeida" <[email protected]>

This series is work in progress. It represents the current work done on a
virtual DVB driver for the Linux media subsystem. I am new to the media
subsystem and to kernel development in general.

This driver aims to:
- Serve as template for new DVB driver writers
- Help userspace application writers in general
- Push fake audio/video to userspace for testing
purposes
- Push debug information to userspace via debugfs

This should currently be able to feed PSI and PCM audio data to
userspace.

I appreciate any feedback!

Changes in v6:
Addressed the following issues:
- Makefile was not actually compiling everything;
- The bridge driver should be a platform driver;
- There are lots of warnings and other errors produced
by the driver.

Changes in v5:

Removed all calls to WARN_ON in favor of pr_warn_ratelimited
Add a #define for pr_fmt
Reworked the byte swapping logic for big/little endian support:
big endian fields now prepended with __beXX for 'sparse' checks
bitfields now laid in reverse order if LITTLE_ENDIAN_BITFIELD
is set

media: vidtv: implement a tuner driver
Return -EINVAL instead of -1 in vidtv_tuner_check_frequency_shift
media: vidtv: implement a demodulator driver
Add POLL_THRD_TIME #define
Implement dvb_frontend_ops as a single struct instead of three
media: vidtv: move config structs into a separate header
Removed this commit, configs are now in vidtv_tuner.h and vidtv_demod.h
media: vidtv: add a bridge driver
module_param: all fs permissions set to 0
removed param 'chosen_delsys'
module_param: removed "optional" string: all module_params are optional
renamed vidtv_bridge -> vidtv_bridg: so the source code and module names
are different
media: vidtv: add wrappers for memcpy and memset
Added kernel-docs
write address is now computed internally
media: vidtv: add MPEG TS common code
Drop packets if the buffer is full
media: vidtv: implement a PSI generator
Removed vidtv_psi_ts_psi_write_stuffing()
Forcibly align misaligned buffers
Drop packets if buffer is full
media: vidtv: implement a PES packetizer
Remove vidtv_extract_bits() in favor of GENMASK() and bitwise &
Forcibly align misaligned buffers
media: vidtv: Implement a SMPTE 302M encoder
Added kernel-docs for struct vidtv_encoder
media: vidtv: Add a MPEG Transport Stream Multiplexer
Added check for minimum and maximum buffer size.
Drop packets if buffer is full


Changes in v4:
Added a PES packetizer
Implemented a minimum version of the SMPTE 302m encoder for AES3 audio
Fixed endianness in the PSI generator, converting fields to big endian where applicable
Added a minimal TS mux abstraction

Changes in v3:
Added a bridge driver
Renamed the driver to vidtv
Renamed/reworked commits into smaller pieces
Moved the driver into its own directory
Fixed the code for the signal strength in the tuner
Removed useless enums in the tuner driver (e.g. lock_status, power_status...)
Reworked the logic for the poll_snr thread in the demodulator driver
Moved MPEG related code to the bridge driver, as it controls the demux logic
Changed literals to #defines, used sizeof in place of integer literals when
computing the size of PSI structs
Moved the MPEG PSI tables to the heap to reduce stack space usage
Now using usleep_range in place of msleep_interruptible in the MPEG TS thread
Wrapped memcpy and memset to protect against buffer overflow when writing to the
MPEG TS buffer.

Changes in v2:
Attempted not to break assignments into multiple lines as much as possible.
Code now passes checkpatch strict mode

media: dvb_dummy_tuner: implement driver skeleton
Changed snr values to mili db
Return value from 0-100 to indicate how far off the requested
frequency is from a valid one

Use the frequency shift to interpolate between 34dB and 10dB if
we can not match against the SNR lookup table
Remove sleep calls for suspend/resume

Fix memcpy call for the config struct

media: dvb_dummy_fe.c: lose TS lock on bad snr
Randomly recover the TS lock if the signal quality improves

media: dvb_dummy_fe.c: write PSI information into DMX buffer
Split the patch into multiple header/source files

Hexadecimal literals are now lower case

Prefer short function names / reduce function signatures

Add #defines for constants when computing section lengths

Change signature for functions that take a dummy channel as
argument (i.e. channels* is now channels[NUM_CHANNELS])

Daniel W. S. Almeida (10):
media: vidtv: add Kconfig entry
media: vidtv: implement a tuner driver
media: vidtv: implement a demodulator driver
media: vidtv: add a bridge driver
media: vidtv: add wrappers for memcpy and memset
media: vidtv: add MPEG TS common code
media: vidtv: implement a PSI generator
media: vidtv: implement a PES packetizer
media: vidtv: Implement a SMPTE 302M encoder
media: vidtv: Add a MPEG Transport Stream Multiplexer

arch/Kconfig | 2 +-
drivers/media/dvb-core/dvbdev.c | 1 +
drivers/media/test-drivers/Kconfig | 10 +
drivers/media/test-drivers/Makefile | 1 +
drivers/media/test-drivers/vidtv/Kconfig | 11 +
drivers/media/test-drivers/vidtv/Makefile | 9 +
.../media/test-drivers/vidtv/vidtv_bridge.c | 511 ++++++++
.../media/test-drivers/vidtv/vidtv_bridge.h | 41 +
.../media/test-drivers/vidtv/vidtv_channel.c | 339 ++++++
.../media/test-drivers/vidtv/vidtv_channel.h | 66 ++
.../media/test-drivers/vidtv/vidtv_common.c | 86 ++
.../media/test-drivers/vidtv/vidtv_common.h | 34 +
.../media/test-drivers/vidtv/vidtv_demod.c | 444 +++++++
.../media/test-drivers/vidtv/vidtv_demod.h | 41 +
.../media/test-drivers/vidtv/vidtv_encoder.h | 103 ++
drivers/media/test-drivers/vidtv/vidtv_mux.c | 434 +++++++
drivers/media/test-drivers/vidtv/vidtv_mux.h | 105 ++
drivers/media/test-drivers/vidtv/vidtv_pes.c | 450 +++++++
drivers/media/test-drivers/vidtv/vidtv_pes.h | 186 +++
drivers/media/test-drivers/vidtv/vidtv_psi.c | 1055 +++++++++++++++++
drivers/media/test-drivers/vidtv/vidtv_psi.h | 420 +++++++
.../media/test-drivers/vidtv/vidtv_s302m.c | 636 ++++++++++
.../media/test-drivers/vidtv/vidtv_s302m.h | 119 ++
drivers/media/test-drivers/vidtv/vidtv_ts.c | 157 +++
drivers/media/test-drivers/vidtv/vidtv_ts.h | 120 ++
.../media/test-drivers/vidtv/vidtv_tuner.c | 408 +++++++
.../media/test-drivers/vidtv/vidtv_tuner.h | 26 +
27 files changed, 5814 insertions(+), 1 deletion(-)
create mode 100644 drivers/media/test-drivers/vidtv/Kconfig
create mode 100644 drivers/media/test-drivers/vidtv/Makefile
create mode 100644 drivers/media/test-drivers/vidtv/vidtv_bridge.c
create mode 100644 drivers/media/test-drivers/vidtv/vidtv_bridge.h
create mode 100644 drivers/media/test-drivers/vidtv/vidtv_channel.c
create mode 100644 drivers/media/test-drivers/vidtv/vidtv_channel.h
create mode 100644 drivers/media/test-drivers/vidtv/vidtv_common.c
create mode 100644 drivers/media/test-drivers/vidtv/vidtv_common.h
create mode 100644 drivers/media/test-drivers/vidtv/vidtv_demod.c
create mode 100644 drivers/media/test-drivers/vidtv/vidtv_demod.h
create mode 100644 drivers/media/test-drivers/vidtv/vidtv_encoder.h
create mode 100644 drivers/media/test-drivers/vidtv/vidtv_mux.c
create mode 100644 drivers/media/test-drivers/vidtv/vidtv_mux.h
create mode 100644 drivers/media/test-drivers/vidtv/vidtv_pes.c
create mode 100644 drivers/media/test-drivers/vidtv/vidtv_pes.h
create mode 100644 drivers/media/test-drivers/vidtv/vidtv_psi.c
create mode 100644 drivers/media/test-drivers/vidtv/vidtv_psi.h
create mode 100644 drivers/media/test-drivers/vidtv/vidtv_s302m.c
create mode 100644 drivers/media/test-drivers/vidtv/vidtv_s302m.h
create mode 100644 drivers/media/test-drivers/vidtv/vidtv_ts.c
create mode 100644 drivers/media/test-drivers/vidtv/vidtv_ts.h
create mode 100644 drivers/media/test-drivers/vidtv/vidtv_tuner.c
create mode 100644 drivers/media/test-drivers/vidtv/vidtv_tuner.h

--
2.26.2


2020-05-20 07:08:13

by Daniel Almeida

[permalink] [raw]
Subject: [RFC, WIP, v6 01/10] media: vidtv: add Kconfig entry

From: "Daniel W. S. Almeida" <[email protected]>

Add the necessary Kconfig entries and a dummy Makefile to compile the new
virtual DVB test driver (vidtv).

Signed-off-by: Daniel W. S. Almeida <[email protected]>
---
drivers/media/test-drivers/Kconfig | 10 ++++++++++
drivers/media/test-drivers/Makefile | 1 +
drivers/media/test-drivers/vidtv/Kconfig | 11 +++++++++++
drivers/media/test-drivers/vidtv/Makefile | 2 ++
4 files changed, 24 insertions(+)
create mode 100644 drivers/media/test-drivers/vidtv/Kconfig
create mode 100644 drivers/media/test-drivers/vidtv/Makefile

diff --git a/drivers/media/test-drivers/Kconfig b/drivers/media/test-drivers/Kconfig
index 188381c855939..7d273a8a7acc2 100644
--- a/drivers/media/test-drivers/Kconfig
+++ b/drivers/media/test-drivers/Kconfig
@@ -4,6 +4,10 @@ menuconfig V4L_TEST_DRIVERS
bool "V4L test drivers"
depends on VIDEO_DEV

+menuconfig DVB_TEST_DRIVERS
+ bool "DVB test drivers"
+ depends on DVB_CORE && MEDIA_SUPPORT && I2C
+
if V4L_TEST_DRIVERS

source "drivers/media/test-drivers/vimc/Kconfig"
@@ -24,3 +28,9 @@ config VIDEO_VIM2M
source "drivers/media/test-drivers/vicodec/Kconfig"

endif #V4L_TEST_DRIVERS
+
+if DVB_TEST_DRIVERS
+
+source "drivers/media/test-drivers/vidtv/Kconfig"
+
+endif #DVB_TEST_DRIVERS
diff --git a/drivers/media/test-drivers/Makefile b/drivers/media/test-drivers/Makefile
index 74410d3a9f2d2..9f0e4ebb2efe7 100644
--- a/drivers/media/test-drivers/Makefile
+++ b/drivers/media/test-drivers/Makefile
@@ -7,3 +7,4 @@ obj-$(CONFIG_VIDEO_VIMC) += vimc/
obj-$(CONFIG_VIDEO_VIVID) += vivid/
obj-$(CONFIG_VIDEO_VIM2M) += vim2m.o
obj-$(CONFIG_VIDEO_VICODEC) += vicodec/
+obj-$(CONFIG_DVB_VIDTV) += vidtv/
diff --git a/drivers/media/test-drivers/vidtv/Kconfig b/drivers/media/test-drivers/vidtv/Kconfig
new file mode 100644
index 0000000000000..22c4fd39461f1
--- /dev/null
+++ b/drivers/media/test-drivers/vidtv/Kconfig
@@ -0,0 +1,11 @@
+# SPDX-License-Identifier: GPL-2.0-only
+config DVB_VIDTV
+ tristate "Virtual DVB Driver (vidtv)"
+ depends on DVB_CORE && MEDIA_SUPPORT && I2C
+ help
+ The virtual DVB test driver serves as a reference DVB driver and helps
+ validate the existing APIs in the media subsystem. It can also aid developers
+ working on userspace applications.
+
+
+ When in doubt, say N.
diff --git a/drivers/media/test-drivers/vidtv/Makefile b/drivers/media/test-drivers/vidtv/Makefile
new file mode 100644
index 0000000000000..d1558d84eeaed
--- /dev/null
+++ b/drivers/media/test-drivers/vidtv/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0
+
--
2.26.2

2020-05-20 07:08:15

by Daniel Almeida

[permalink] [raw]
Subject: [RFC, WIP, v6 04/10] media: vidtv: add a bridge driver

From: "Daniel W. S. Almeida" <[email protected]>

Digital TV devices consist of several independent hardware components which
are controlled by different drivers.
Each media device is controlled by a group of cooperating drivers with the
bridge driver as the main driver.

This patch adds a bridge driver for the Virtual Digital TV driver [vidtv].

The bridge driver binds to the other drivers, that is, vidtv_tuner and
vidtv_demod and implements the digital demux logic, providing userspace
with a MPEG Transport Stream.

Move config structs to a common header so they can be used by the bridge
driver and by their respective drivers.

Signed-off-by: Daniel W. S. Almeida <[email protected]>

BRIDGE WIP
---
arch/Kconfig | 2 +-
drivers/media/dvb-core/dvbdev.c | 1 +
drivers/media/test-drivers/vidtv/Makefile | 4 +-
.../media/test-drivers/vidtv/vidtv_bridge.c | 435 ++++++++++++++++++
.../media/test-drivers/vidtv/vidtv_bridge.h | 39 ++
5 files changed, 479 insertions(+), 2 deletions(-)
create mode 100644 drivers/media/test-drivers/vidtv/vidtv_bridge.c
create mode 100644 drivers/media/test-drivers/vidtv/vidtv_bridge.h

diff --git a/arch/Kconfig b/arch/Kconfig
index 786a85d4ad40d..ddcb4a68ee940 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -868,7 +868,7 @@ config VMAP_STACK
be enabled.

config ARCH_OPTIONAL_KERNEL_RWX
- def_bool n
+ def_bool y

config ARCH_OPTIONAL_KERNEL_RWX_DEFAULT
def_bool n
diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
index 80b6a71aa33e4..38c8b848c921f 100644
--- a/drivers/media/dvb-core/dvbdev.c
+++ b/drivers/media/dvb-core/dvbdev.c
@@ -962,6 +962,7 @@ int dvb_usercopy(struct file *file,
}

#if IS_ENABLED(CONFIG_I2C)
+#pragma clang optimize off
struct i2c_client *dvb_module_probe(const char *module_name,
const char *name,
struct i2c_adapter *adap,
diff --git a/drivers/media/test-drivers/vidtv/Makefile b/drivers/media/test-drivers/vidtv/Makefile
index 21e50c11c94d0..c99cd13f4adaf 100644
--- a/drivers/media/test-drivers/vidtv/Makefile
+++ b/drivers/media/test-drivers/vidtv/Makefile
@@ -2,5 +2,7 @@

dvb-vidtv-tuner-objs := vidtv_tuner.o
dvb-vidtv-demod-objs := vidtv_demod.o
+dvb-vidtv-bridge-objs := vidtv_bridge.o

-obj-$(CONFIG_DVB_VIDTV) += dvb-vidtv-tuner.o dvb-vidtv-demod.o
+obj-$(CONFIG_DVB_VIDTV) += dvb-vidtv-tuner.o dvb-vidtv-demod.o \
+ dvb-vidtv-bridge.o
diff --git a/drivers/media/test-drivers/vidtv/vidtv_bridge.c b/drivers/media/test-drivers/vidtv/vidtv_bridge.c
new file mode 100644
index 0000000000000..bc1c612c23013
--- /dev/null
+++ b/drivers/media/test-drivers/vidtv/vidtv_bridge.c
@@ -0,0 +1,435 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * The Virtual DTV test driver serves as a reference DVB driver and helps
+ * validate the existing APIs in the media subsystem. It can also aid
+ * developers working on userspace applications.
+ *
+ * Written by Daniel W. S. Almeida <[email protected]>
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ":%s, %d: " fmt, __func__, __LINE__
+
+#include <linux/moduleparam.h>
+#include <linux/mutex.h>
+#include <linux/platform_device.h>
+#include <linux/printk.h>
+#include <linux/time.h>
+#include <linux/types.h>
+#include <linux/workqueue.h>
+
+#include "vidtv_bridge.h"
+#include "vidtv_demod.h"
+#include "vidtv_tuner.h"
+
+#define TUNER_DEFAULT_ADDR 0x68
+#define DEMOD_DEFAULT_ADDR 0x60
+
+static unsigned int drop_tslock_prob_on_low_snr;
+module_param(drop_tslock_prob_on_low_snr, uint, 0);
+MODULE_PARM_DESC(drop_tslock_prob_on_low_snr,
+ "Probability of losing the TS lock if the signal quality is bad");
+
+static unsigned int recover_tslock_prob_on_good_snr;
+module_param(recover_tslock_prob_on_good_snr, uint, 0);
+MODULE_PARM_DESC(recover_tslock_prob_on_good_snr,
+ "Probability recovering the TS lock when the signal improves");
+
+static unsigned int mock_power_up_delay_msec;
+module_param(mock_power_up_delay_msec, uint, 0);
+MODULE_PARM_DESC(mock_power_up_delay_msec, "Simulate a power up delay");
+
+static unsigned int mock_tune_delay_msec;
+module_param(mock_tune_delay_msec, uint, 0);
+MODULE_PARM_DESC(mock_tune_delay_msec, "Simulate a tune delay");
+
+static unsigned int vidtv_valid_dvb_t_freqs[8];
+module_param_array(vidtv_valid_dvb_t_freqs, uint, NULL, 0);
+MODULE_PARM_DESC(vidtv_valid_dvb_t_freqs,
+ "Valid DVB-T frequencies to simulate");
+
+static unsigned int vidtv_valid_dvb_c_freqs[8];
+module_param_array(vidtv_valid_dvb_c_freqs, uint, NULL, 0);
+MODULE_PARM_DESC(vidtv_valid_dvb_c_freqs,
+ "Valid DVB-C frequencies to simulate");
+
+static unsigned int vidtv_valid_dvb_s_freqs[8];
+module_param_array(vidtv_valid_dvb_s_freqs, uint, NULL, 0);
+MODULE_PARM_DESC(vidtv_valid_dvb_s_freqs,
+ "Valid DVB-C frequencies to simulate");
+
+static unsigned int max_frequency_shift_hz;
+module_param(max_frequency_shift_hz, uint, 0);
+MODULE_PARM_DESC(max_frequency_shift_hz,
+ "Maximum shift in HZ allowed when tuning in a channel");
+
+DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nums);
+
+static int vidtv_start_streaming(struct vidtv_dvb *dvb)
+{
+ if (dvb->streaming) {
+ pr_warn_ratelimited("Already streaming. Skipping.\n");
+ return 0;
+ }
+
+ dvb->streaming = true;
+ return 0;
+}
+
+static int vidtv_stop_streaming(struct vidtv_dvb *dvb)
+{
+ dvb->streaming = false;
+
+ return 0;
+}
+
+static int vidtv_start_feed(struct dvb_demux_feed *feed)
+{
+ struct dvb_demux *demux = feed->demux;
+ struct vidtv_dvb *dvb = demux->priv;
+ int rc;
+ int ret;
+
+ if (!demux->dmx.frontend)
+ return -EINVAL;
+
+ mutex_lock(&dvb->feed_lock);
+
+ dvb->nfeeds++;
+ rc = dvb->nfeeds;
+
+ if (dvb->nfeeds == 1) {
+ ret = vidtv_start_streaming(dvb);
+ if (ret < 0)
+ rc = ret;
+ }
+
+ mutex_unlock(&dvb->feed_lock);
+ return rc;
+}
+
+static int vidtv_stop_feed(struct dvb_demux_feed *feed)
+{
+ struct dvb_demux *demux = feed->demux;
+ struct vidtv_dvb *dvb = demux->priv;
+ int err = 0;
+
+ mutex_lock(&dvb->feed_lock);
+ dvb->nfeeds--;
+
+ if (!dvb->nfeeds)
+ err = vidtv_stop_streaming(dvb);
+
+ mutex_unlock(&dvb->feed_lock);
+ return err;
+}
+
+static struct dvb_frontend *vidtv_get_frontend_ptr(struct i2c_client *c)
+{
+ /* the demod will set this when its probe function runs */
+ struct vidtv_demod_state *state = i2c_get_clientdata(c);
+
+ return &state->frontend;
+}
+
+static int vidtv_master_xfer(struct i2c_adapter *i2c_adap,
+ struct i2c_msg msgs[],
+ int num)
+{
+ return 0;
+}
+
+static u32 vidtv_i2c_func(struct i2c_adapter *adapter)
+{
+ return I2C_FUNC_I2C;
+}
+
+static const struct i2c_algorithm vidtv_i2c_algorithm = {
+ .master_xfer = vidtv_master_xfer,
+ .functionality = vidtv_i2c_func,
+};
+
+static int vidtv_bridge_i2c_register_adap(struct vidtv_dvb *dvb)
+{
+ struct i2c_adapter *i2c_adapter = &dvb->i2c_adapter;
+
+ strscpy(i2c_adapter->name, "vidtv_i2c", sizeof(i2c_adapter->name));
+ i2c_adapter->owner = THIS_MODULE;
+ i2c_adapter->algo = &vidtv_i2c_algorithm;
+ i2c_adapter->algo_data = NULL;
+ i2c_adapter->timeout = 500;
+ i2c_adapter->retries = 3;
+ i2c_adapter->dev.parent = &dvb->pdev->dev;
+
+ i2c_set_adapdata(i2c_adapter, dvb);
+ return i2c_add_adapter(&dvb->i2c_adapter);
+}
+
+static int vidtv_bridge_register_adap(struct vidtv_dvb *dvb)
+{
+ int ret = 0;
+
+ ret = dvb_register_adapter(&dvb->adapter,
+ KBUILD_MODNAME,
+ THIS_MODULE,
+ &dvb->i2c_adapter.dev,
+ adapter_nums);
+
+ return ret;
+}
+
+static int vidtv_bridge_dmx_init(struct vidtv_dvb *dvb)
+{
+ dvb->demux.dmx.capabilities = DMX_TS_FILTERING |
+ DMX_SECTION_FILTERING;
+
+ dvb->demux.priv = dvb;
+ dvb->demux.filternum = 256;
+ dvb->demux.feednum = 256;
+ dvb->demux.start_feed = vidtv_start_feed;
+ dvb->demux.stop_feed = vidtv_stop_feed;
+
+ return dvb_dmx_init(&dvb->demux);
+}
+
+static int vidtv_bridge_dmxdev_init(struct vidtv_dvb *dvb)
+{
+ dvb->dmx_dev.filternum = 256;
+ dvb->dmx_dev.demux = &dvb->demux.dmx;
+ dvb->dmx_dev.capabilities = 0;
+
+ return dvb_dmxdev_init(&dvb->dmx_dev, &dvb->adapter);
+}
+
+static int vidtv_bridge_probe_demod(struct vidtv_dvb *dvb, u32 n)
+{
+ struct vidtv_demod_config cfg = {};
+
+ cfg.drop_tslock_prob_on_low_snr = drop_tslock_prob_on_low_snr;
+ cfg.recover_tslock_prob_on_good_snr = recover_tslock_prob_on_good_snr;
+
+ dvb->i2c_client_demod[n] = dvb_module_probe("vidtv_demod",
+ NULL,
+ &dvb->i2c_adapter,
+ DEMOD_DEFAULT_ADDR,
+ &cfg);
+
+ /* driver will not work anyways so bail out */
+ if (!dvb->i2c_client_demod[n])
+ return -ENODEV;
+
+ /* retrieve a ptr to the frontend state */
+ dvb->fe[n] = vidtv_get_frontend_ptr(dvb->i2c_client_demod[n]);
+
+ return 0;
+}
+
+static int vidtv_bridge_probe_tuner(struct vidtv_dvb *dvb, u32 n)
+{
+ struct vidtv_tuner_config cfg = {};
+
+ cfg.fe = dvb->fe[n];
+ cfg.mock_power_up_delay_msec = mock_power_up_delay_msec;
+ cfg.mock_tune_delay_msec = mock_tune_delay_msec;
+
+ memcpy(cfg.vidtv_valid_dvb_t_freqs,
+ vidtv_valid_dvb_t_freqs,
+ sizeof(vidtv_valid_dvb_t_freqs));
+
+ memcpy(cfg.vidtv_valid_dvb_c_freqs,
+ vidtv_valid_dvb_c_freqs,
+ sizeof(vidtv_valid_dvb_c_freqs));
+
+ memcpy(cfg.vidtv_valid_dvb_s_freqs,
+ vidtv_valid_dvb_s_freqs,
+ sizeof(vidtv_valid_dvb_s_freqs));
+
+ cfg.max_frequency_shift_hz = max_frequency_shift_hz;
+
+ dvb->i2c_client_tuner[n] = dvb_module_probe("vidtv_tuner",
+ NULL,
+ &dvb->i2c_adapter,
+ TUNER_DEFAULT_ADDR,
+ &cfg);
+
+ return (dvb->i2c_client_tuner[n]) ? 0 : -ENODEV;
+}
+
+static int vidtv_bridge_dvb_init(struct vidtv_dvb *dvb)
+{
+ int ret;
+ int i;
+ int j;
+ int k;
+ int w;
+ int l;
+
+ ret = vidtv_bridge_i2c_register_adap(dvb);
+ if (ret < 0)
+ goto fail_i2c;
+
+ ret = vidtv_bridge_register_adap(dvb);
+ if (ret < 0)
+ goto fail_adapter;
+
+ for (i = 0; i < NUM_FE; ++i) {
+ ret = vidtv_bridge_probe_demod(dvb, i);
+ if (ret < 0)
+ goto fail_demod_probe;
+
+ ret = vidtv_bridge_probe_tuner(dvb, i);
+ if (ret < 0)
+ goto fail_tuner_probe;
+
+ ret = dvb_register_frontend(&dvb->adapter, dvb->fe[i]);
+ if (ret < 0)
+ goto fail_fe;
+ }
+
+ ret = vidtv_bridge_dmx_init(dvb);
+ if (ret < 0)
+ goto fail_dmx;
+
+ ret = vidtv_bridge_dmxdev_init(dvb);
+ if (ret < 0)
+ goto fail_dmx_dev;
+
+ for (j = 0; j < NUM_FE; ++j) {
+ ret = dvb->demux.dmx.connect_frontend(&dvb->demux.dmx,
+ &dvb->dmx_fe[j]);
+ if (ret < 0)
+ goto fail_dmx_conn;
+
+ /*
+ * The source of the demux is a frontend connected
+ * to the demux.
+ */
+ dvb->dmx_fe[j].source = DMX_FRONTEND_0;
+ }
+
+ return ret;
+
+fail_dmx_conn:
+ for (j = j - 1; j >= 0; --j)
+ dvb->demux.dmx.remove_frontend(&dvb->demux.dmx,
+ &dvb->dmx_fe[j]);
+fail_dmx_dev:
+ dvb_dmxdev_release(&dvb->dmx_dev);
+fail_dmx:
+ dvb_dmx_release(&dvb->demux);
+fail_fe:
+ for (k = i; k >= 0; --k)
+ dvb_unregister_frontend(dvb->fe[k]);
+fail_tuner_probe:
+ for (w = i; w >= 0; --w)
+ dvb_module_release(dvb->i2c_client_tuner[w]);
+
+fail_demod_probe:
+ for (l = i; l >= 0; --l)
+ dvb_module_release(dvb->i2c_client_demod[l]);
+
+fail_adapter:
+ dvb_unregister_adapter(&dvb->adapter);
+
+fail_i2c:
+ i2c_del_adapter(&dvb->i2c_adapter);
+
+ return ret;
+}
+
+static int vidtv_bridge_probe(struct platform_device *pdev)
+{
+ int ret;
+ struct vidtv_dvb *dvb;
+
+ dvb = kzalloc(sizeof(*dvb), GFP_KERNEL);
+ if (!dvb)
+ return -ENOMEM;
+
+ dvb->pdev = pdev;
+
+ ret = vidtv_bridge_dvb_init(dvb);
+ if (ret < 0)
+ goto err_dvb;
+
+ mutex_init(&dvb->feed_lock);
+
+ platform_set_drvdata(pdev, dvb);
+
+ return ret;
+
+err_dvb:
+ kfree(dvb);
+ return ret;
+}
+
+static int vidtv_bridge_remove(struct platform_device *pdev)
+{
+ struct vidtv_dvb *dvb;
+ u32 i;
+
+ dvb = platform_get_drvdata(pdev);
+
+ mutex_destroy(&dvb->feed_lock);
+
+ for (i = 0; i < NUM_FE; ++i)
+ dvb->demux.dmx.remove_frontend(&dvb->demux.dmx,
+ &dvb->dmx_fe[i]);
+
+ dvb_dmxdev_release(&dvb->dmx_dev);
+ dvb_dmx_release(&dvb->demux);
+
+ for (i = 0; i < NUM_FE; ++i) {
+ dvb_unregister_frontend(dvb->fe[i]);
+ dvb_frontend_detach(dvb->fe[i]);
+ }
+
+ dvb_unregister_adapter(&dvb->adapter);
+
+ return 0;
+}
+
+static void vidtv_bridge_dev_release(struct device *dev)
+{
+}
+
+static struct platform_device vidtv_bridge_dev = {
+ .name = "vidtv_bridge",
+ .dev.release = vidtv_bridge_dev_release,
+};
+
+static struct platform_driver vidtv_bridge_driver = {
+ .driver = {
+ .name = "vidtv_bridge",
+ .suppress_bind_attrs = true,
+ },
+ .probe = vidtv_bridge_probe,
+ .remove = vidtv_bridge_remove,
+};
+
+static void __exit vidtv_bridge_exit(void)
+{
+ platform_driver_unregister(&vidtv_bridge_driver);
+ platform_device_unregister(&vidtv_bridge_dev);
+}
+
+static int __init vidtv_bridge_init(void)
+{
+ int ret;
+
+ ret = platform_device_register(&vidtv_bridge_dev);
+ if (ret)
+ return ret;
+
+ ret = platform_driver_register(&vidtv_bridge_driver);
+ if (ret)
+ platform_device_unregister(&vidtv_bridge_dev);
+
+ return ret;
+}
+
+module_init(vidtv_bridge_init);
+module_exit(vidtv_bridge_exit);
+
+MODULE_DESCRIPTION("Virtual Digital TV Test Driver");
+MODULE_AUTHOR("Daniel W. S. Almeida");
+MODULE_LICENSE("GPL");
diff --git a/drivers/media/test-drivers/vidtv/vidtv_bridge.h b/drivers/media/test-drivers/vidtv/vidtv_bridge.h
new file mode 100644
index 0000000000000..4931bfb73273c
--- /dev/null
+++ b/drivers/media/test-drivers/vidtv/vidtv_bridge.h
@@ -0,0 +1,39 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * The Virtual DTV test driver serves as a reference DVB driver and helps
+ * validate the existing APIs in the media subsystem. It can also aid
+ * developers working on userspace applications.
+ *
+ * Written by Daniel W. S. Almeida <[email protected]>
+ */
+
+#ifndef VIDTV_BRIDGE_H
+#define VIDTV_BRIDGE_H
+
+#define NUM_FE 1
+
+#include <linux/i2c.h>
+#include <linux/platform_device.h>
+#include <linux/types.h>
+#include <media/dmxdev.h>
+#include <media/dvb_demux.h>
+#include <media/dvb_frontend.h>
+
+struct vidtv_dvb {
+ struct platform_device *pdev;
+ struct dvb_frontend *fe[NUM_FE];
+ struct dvb_adapter adapter;
+ struct dvb_demux demux;
+ struct dmxdev dmx_dev;
+ struct dmx_frontend dmx_fe[NUM_FE];
+ struct i2c_adapter i2c_adapter;
+ struct i2c_client *i2c_client_demod[NUM_FE];
+ struct i2c_client *i2c_client_tuner[NUM_FE];
+
+ u32 nfeeds;
+ struct mutex feed_lock; /* start/stop feed */
+
+ bool streaming;
+};
+
+#endif // VIDTV_BRIDG_H
--
2.26.2

2020-05-20 07:08:51

by Daniel Almeida

[permalink] [raw]
Subject: [RFC, WIP, v6 08/10] media: vidtv: implement a PES packetizer

From: "Daniel W. S. Almeida" <[email protected]>

Implement the PES logic to convert encoder data into MPEG TS packets.
These TS packets can then be fed into a TS multiplexer and eventually
into userspace.

Signed-off-by: Daniel W. S. Almeida <[email protected]>
---
.../media/test-drivers/vidtv/vidtv_common.h | 2 +
drivers/media/test-drivers/vidtv/vidtv_pes.c | 450 ++++++++++++++++++
drivers/media/test-drivers/vidtv/vidtv_pes.h | 186 ++++++++
3 files changed, 638 insertions(+)
create mode 100644 drivers/media/test-drivers/vidtv/vidtv_pes.c
create mode 100644 drivers/media/test-drivers/vidtv/vidtv_pes.h

diff --git a/drivers/media/test-drivers/vidtv/vidtv_common.h b/drivers/media/test-drivers/vidtv/vidtv_common.h
index a3cb303cc7423..1a31973f3ac61 100644
--- a/drivers/media/test-drivers/vidtv/vidtv_common.h
+++ b/drivers/media/test-drivers/vidtv/vidtv_common.h
@@ -24,4 +24,6 @@ u32 vidtv_memset(void *to,
int c,
size_t len);

+u64 vidtv_extract_bits(u64 value, u8 start_bit, u8 nbits);
+
#endif // VIDTV_COMMON_H
diff --git a/drivers/media/test-drivers/vidtv/vidtv_pes.c b/drivers/media/test-drivers/vidtv/vidtv_pes.c
new file mode 100644
index 0000000000000..c8ea83a3cf800
--- /dev/null
+++ b/drivers/media/test-drivers/vidtv/vidtv_pes.c
@@ -0,0 +1,450 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Vidtv serves as a reference DVB driver and helps validate the existing APIs
+ * in the media subsystem. It can also aid developers working on userspace
+ * applications.
+ *
+ * This file contains the logic to translate the ES data for one access unit
+ * from an encoder into MPEG TS packets. It does so by first encapsulating it
+ * with a PES header and then splitting it into TS packets.
+ *
+ * Written by Daniel W. S. Almeida <[email protected]>
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ":%s, %d: " fmt, __func__, __LINE__
+
+#include <linux/types.h>
+#include <linux/printk.h>
+#include <linux/ratelimit.h>
+
+#include "vidtv_pes.h"
+#include "vidtv_common.h"
+#include "vidtv_ts.h"
+
+#define PRIVATE_STREAM_1_ID 0xbd /* private_stream_1. See SMPTE 302M-2007 p.6 */
+#define PES_HEADER_MAX_STUFFING_BYTES 32
+#define PES_TS_HEADER_MAX_STUFFING_BYTES 182
+
+static u32 vidtv_pes_op_get_regular_len(bool send_pts, bool send_dts)
+{
+ u32 len = 0;
+
+ /* the flags must always be sent */
+ len += sizeof(struct vidtv_pes_optional);
+
+ /* From all optionals, we might send these for now */
+ if (send_pts && send_dts)
+ len += sizeof(struct vidtv_pes_optional_pts_dts);
+ else if (send_pts)
+ len += sizeof(struct vidtv_pes_optional_pts);
+
+ return len;
+}
+
+static u32 vidtv_pes_h_get_regular_len(bool send_pts, bool send_dts)
+{
+ /* PES header length notwithstanding stuffing bytes */
+ u32 len = 0;
+
+ len += sizeof(struct vidtv_mpeg_pes);
+ len += vidtv_pes_op_get_regular_len(send_pts, send_dts);
+
+ return len;
+}
+
+static u32 vidtv_pes_write_header_stuffing(struct vidtv_mpeg_pes *pes_h,
+ struct pes_header_write_args args)
+{
+ /*
+ * This is a fixed 8-bit value equal to '1111 1111' that can be inserted
+ * by the encoder, for example to meet the requirements of the channel.
+ * It is discarded by the decoder. No more than 32 stuffing bytes shall
+ * be present in one PES packet header.
+ */
+ if (args.n_pes_h_s_bytes > PES_HEADER_MAX_STUFFING_BYTES) {
+ pr_warn_ratelimited("More than 32 stuffing bytes in PES packet header\n");
+ args.n_pes_h_s_bytes = PES_HEADER_MAX_STUFFING_BYTES;
+ }
+
+ /* gives the length of the remainder of the PES header in bytes */
+ pes_h->length += args.n_pes_h_s_bytes;
+
+ return vidtv_memset(args.dest_buf,
+ args.dest_offset,
+ args.dest_buf_sz,
+ TS_FILL_BYTE,
+ args.n_pes_h_s_bytes);
+}
+
+static u32 vidtv_pes_write_pts_dts(struct pes_header_write_args args)
+{
+ u32 nbytes = 0; /* the number of bytes written by this function */
+
+ struct vidtv_pes_optional_pts pts = {};
+ struct vidtv_pes_optional_pts_dts pts_dts = {};
+ void *op = NULL;
+ size_t op_sz = 0;
+
+ if (!args.send_pts && args.send_dts)
+ return 0;
+
+ /* see ISO/IEC 13818-1 : 2000 p. 32 */
+
+ if (args.send_pts && args.send_dts) {
+ pts_dts.three = 0x3;
+
+ pts_dts.pts1 = (args.pts & GENMASK(32, 30)) >> 30;
+ pts_dts.marker1 = 0x1;
+ pts_dts.pts2 = (args.pts & GENMASK(29, 15)) >> 15;
+ pts_dts.marker2 = 0x1;
+ pts_dts.pts3 = args.pts & GENMASK(14, 0);
+ pts_dts.marker3 = 0x1;
+
+ pts_dts.one = 0x1;
+
+ pts_dts.dts1 = (args.dts & GENMASK(32, 30)) >> 30;
+ pts_dts.marker1 = 0x1;
+ pts_dts.dts2 = (args.dts & GENMASK(29, 15)) >> 15;
+ pts_dts.marker2 = 0x1;
+ pts_dts.dts3 = args.dts & GENMASK(14, 0);
+ pts_dts.marker3 = 0x1;
+
+ op = &pts_dts;
+ op_sz = sizeof(pts_dts);
+
+ } else if (args.send_pts) {
+ pts.two = 0x2;
+ pts_dts.pts1 = (args.pts & GENMASK(32, 30)) >> 30;
+ pts_dts.marker1 = 0x1;
+ pts_dts.pts2 = (args.pts & GENMASK(29, 15)) >> 15;
+ pts_dts.marker2 = 0x1;
+ pts_dts.pts3 = args.pts & GENMASK(14, 0);
+ pts_dts.marker3 = 0x1;
+
+ op = &pts;
+ op_sz = sizeof(pts);
+ }
+
+ /* copy PTS/DTS optional */
+ nbytes += vidtv_memcpy(args.dest_buf,
+ args.dest_offset + nbytes,
+ args.dest_buf_sz,
+ op,
+ op_sz);
+
+ return nbytes;
+}
+
+static u32 vidtv_pes_write_h(struct pes_header_write_args args)
+{
+ u32 nbytes = 0; /* the number of bytes written by this function */
+
+ struct vidtv_mpeg_pes pes_header = {};
+ struct vidtv_pes_optional pes_optional = {};
+ struct pes_header_write_args pts_dts_args = args;
+
+ pes_header.packet_start_code_prefix = PES_START_CODE_PREFIX;
+
+ pes_header.stream_id = (args.is_s302m_pkt) ?
+ PRIVATE_STREAM_1_ID :
+ args.stream_id;
+
+ pes_header.length = vidtv_pes_op_get_regular_len(args.send_pts,
+ args.send_dts);
+
+ pes_optional.two = 0x2;
+
+ pes_optional.PTS_DTS = (args.send_pts && args.send_dts) ?
+ 0x3 :
+ (args.send_pts) ?
+ 0x2 :
+ 0x0;
+
+ /* copy header */
+
+ nbytes += vidtv_memcpy(args.dest_buf,
+ args.dest_offset + nbytes,
+ args.dest_buf_sz,
+ &pes_header,
+ sizeof(pes_header));
+
+ /* copy optional header */
+ nbytes += vidtv_memcpy(args.dest_buf,
+ args.dest_offset + nbytes,
+ args.dest_buf_sz,
+ &pes_optional,
+ sizeof(pes_optional));
+
+ pts_dts_args.dest_offset = args.dest_offset + nbytes;
+ nbytes += vidtv_pes_write_pts_dts(pts_dts_args);
+
+ /* write any PES header stuffing */
+ nbytes += vidtv_pes_write_header_stuffing(&pes_header, args);
+
+ return nbytes;
+}
+
+static u32 vidtv_pes_write_stuffing(struct vidtv_mpeg_ts *ts_h,
+ void *dest_buf,
+ u32 dest_offset,
+ u32 n_stuffing_bytes,
+ u32 buf_sz)
+{
+ /*
+ * For Transport Stream packets carrying PES packets, stuffing is
+ * needed when there is insufficient PES packet data to completely
+ * fill the Transport Stream packet payload bytes. Stuffing is
+ * accomplished by defining an adaptation field longer than the sum of
+ * the lengths of the data elements in it, so that the payload bytes
+ * remaining after the adaptation field exactly accommodates the
+ * available PES packet data. The extra space in the adaptation field
+ * is filled with stuffing bytes.
+ *
+ */
+
+ /* the number of bytes written by this function */
+ u32 nbytes = 0;
+ struct vidtv_mpeg_ts_adaption ts_adap = {};
+
+ if (!n_stuffing_bytes)
+ return nbytes;
+
+ ts_h->adaptation_field = 1;
+
+ if (n_stuffing_bytes > PES_TS_HEADER_MAX_STUFFING_BYTES) {
+ pr_warn_ratelimited("More than %d stuffing bytes for a PES packet",
+ PES_TS_HEADER_MAX_STUFFING_BYTES);
+
+ n_stuffing_bytes = PES_TS_HEADER_MAX_STUFFING_BYTES;
+ }
+
+ /* the AF will only be its 'length' field with a value of zero */
+ if (n_stuffing_bytes == 1) {
+ nbytes += vidtv_memset(dest_buf,
+ dest_offset + nbytes,
+ buf_sz,
+ 0,
+ n_stuffing_bytes);
+
+ return nbytes;
+ }
+
+ n_stuffing_bytes -= sizeof(ts_adap);
+
+ /* length _immediately_ following 'adaptation_field_length' */
+ ts_adap.length = sizeof(ts_adap) -
+ sizeof(ts_adap.length) +
+ n_stuffing_bytes;
+
+ /* write the adap after the TS header */
+ nbytes += vidtv_memcpy(dest_buf,
+ dest_offset + nbytes,
+ buf_sz,
+ &ts_adap,
+ sizeof(ts_adap));
+
+ /* write the stuffing bytes */
+ nbytes += vidtv_memset(dest_buf,
+ dest_offset + nbytes,
+ buf_sz,
+ TS_FILL_BYTE,
+ n_stuffing_bytes);
+
+ return nbytes;
+}
+
+static u32 vidtv_pes_write_ts_h(struct pes_ts_header_write_args args)
+{
+ /* number of bytes written by this function */
+ u32 nbytes = 0;
+ struct vidtv_mpeg_ts ts_header = {};
+
+ ts_header.sync_byte = TS_SYNC_BYTE;
+ ts_header.tei = 0;
+ ts_header.pid = args.pid;
+ ts_header.priority = 0;
+ ts_header.scrambling = 0; /* not scrambled */
+ ts_header.adaptation_field = 0;
+ ts_header.payload = 1;
+
+ ts_header.payload_start = (!args.wrote_pes_header) ? 1 : 0;
+ ts_header.continuity_counter = *args.continuity_counter;
+
+ /*
+ * This will trigger a discontinuity if the buffer is full, which is
+ * what we want: the continuity counter will be incremented but nothing
+ * will get copied by vidtv_memcpy/vidtv_memset, effectively dropping
+ * the packet.
+ */
+ vidtv_ts_inc_cc(args.continuity_counter);
+
+ /* write the TS header */
+ nbytes += vidtv_memcpy(args.dest_buf,
+ args.dest_offset + nbytes,
+ args.dest_buf_sz,
+ &ts_header,
+ sizeof(ts_header));
+
+ /* write stuffing, if any */
+ nbytes += vidtv_pes_write_stuffing(&ts_header,
+ args.dest_buf,
+ args.dest_offset + nbytes,
+ args.n_stuffing_bytes,
+ args.dest_buf_sz);
+
+ return nbytes;
+}
+
+u32 vidtv_pes_write_into(struct pes_write_args args)
+{
+ u32 nbytes_past_boundary = (args.dest_offset % TS_PACKET_LEN);
+ bool aligned = (nbytes_past_boundary == 0);
+
+ struct pes_ts_header_write_args ts_header_args = {};
+ struct pes_header_write_args pes_header_args = {};
+
+ /* number of bytes written by this function */
+ u32 nbytes = 0;
+ u32 remaining_len = args.access_unit_len;
+
+ bool wrote_pes_header = false;
+
+ /* whether we need to stuff the TS packet to align the buffer */
+ bool stuff = false;
+
+ u32 available_space = 0;
+ u32 payload_write_len = 0;
+ u32 num_stuffing_bytes = 0;
+
+ /* Just a sanity check, should not really happen because we stuff the
+ * TS packet when we finish writing the PES data, but if this happens
+ * then we have messed up the logic somewhere.
+ *
+ * Also note that, unlike packets for PSI data, we need to carry PES
+ * packets aligned with the payload of transport packets, that is the
+ * first byte of each PES header must be the first byte in the payload
+ * of a transport packet. As a consequence, the last byte of a PES
+ * packet must be the last byte of the payload of a transport packet.
+ */
+ if (!aligned) {
+ pr_warn_ratelimited("Cannot start a PES packet in a misaligned buffer\n");
+
+ /* forcibly align and hope for the best */
+ nbytes += vidtv_memset(args.dest_buf,
+ args.dest_offset + nbytes,
+ args.dest_buf_sz,
+ TS_FILL_BYTE,
+ TS_PACKET_LEN - nbytes_past_boundary);
+
+ aligned = true;
+ }
+
+ if (args.send_dts && !args.send_pts) {
+ pr_warn_ratelimited("forbidden value '01' for PTS_DTS flags\n");
+ args.send_pts = true;
+ args.pts = args.dts;
+ }
+
+ /* see SMPTE 302M clause 6.4 */
+ if (args.is_s302m_pkt) {
+ args.send_dts = false;
+ args.send_pts = true;
+ }
+
+ while (remaining_len) {
+ /*
+ * The amount of space initially available in the TS packet.
+ * if this is the beginning of the PES packet, we need to
+ * take into account the space needed for the TS header _and_
+ * for the PES header
+ */
+ available_space = (!wrote_pes_header) ?
+ TS_PAYLOAD_LEN -
+ vidtv_pes_h_get_regular_len(args.send_pts,
+ args.send_dts) :
+ TS_PAYLOAD_LEN;
+
+ /* if the encoder has inserted stuffing bytes in the PES
+ * header, account for them.
+ */
+ available_space -= args.n_pes_h_s_bytes;
+
+ /* whether we need to stuff the TS packet to align the buffer */
+ stuff = remaining_len < available_space;
+
+ /*
+ * how much of the _actual_ payload we should write in this
+ * packet.
+ */
+ payload_write_len = (stuff) ?
+ remaining_len :
+ available_space;
+
+ num_stuffing_bytes = available_space - payload_write_len;
+
+ /* write ts header */
+ ts_header_args.dest_buf = args.dest_buf;
+ ts_header_args.dest_offset = args.dest_offset + nbytes;
+ ts_header_args.dest_buf_sz = args.dest_buf_sz;
+ ts_header_args.pid = args.pid;
+ ts_header_args.continuity_counter = args.continuity_counter;
+ ts_header_args.wrote_pes_header = wrote_pes_header;
+ ts_header_args.n_stuffing_bytes = num_stuffing_bytes;
+
+ nbytes += vidtv_pes_write_ts_h(ts_header_args);
+
+ if (!wrote_pes_header) {
+ /* write the PES header only once */
+ pes_header_args.dest_buf = args.dest_buf;
+
+ pes_header_args.dest_offset = args.dest_offset +
+ nbytes;
+
+ pes_header_args.dest_buf_sz = args.dest_buf_sz;
+ pes_header_args.is_s302m_pkt = args.is_s302m_pkt;
+ pes_header_args.send_pts = args.send_pts;
+ pes_header_args.pts = args.pts;
+ pes_header_args.send_dts = args.send_dts;
+ pes_header_args.dts = args.dts;
+ pes_header_args.stream_id = args.stream_id;
+ pes_header_args.n_pes_h_s_bytes = args.n_pes_h_s_bytes;
+
+ nbytes += vidtv_pes_write_h(pes_header_args);
+ wrote_pes_header = true;
+ }
+
+ /* write as much of the payload as we possibly can */
+ nbytes += vidtv_memcpy(args.dest_buf,
+ args.dest_offset + nbytes,
+ args.dest_buf_sz,
+ args.from,
+ payload_write_len);
+
+ args.from += payload_write_len;
+ args.dest_offset += nbytes;
+
+ /* Sanity check for underflow. Should not really happen. */
+ if (remaining_len - payload_write_len > remaining_len) {
+ pr_err_ratelimited("Underflow detected\n");
+
+ /* recompute the alignment */
+ nbytes_past_boundary = (args.dest_offset + nbytes) %
+ TS_PACKET_LEN;
+
+ /* forcibly align */
+ nbytes += vidtv_memset(args.dest_buf,
+ args.dest_offset + nbytes,
+ args.dest_buf_sz,
+ TS_FILL_BYTE,
+ TS_PACKET_LEN -
+ nbytes_past_boundary);
+
+ /* bailing out is the best that we can do */
+ return nbytes;
+ }
+
+ remaining_len -= payload_write_len;
+ }
+
+ return nbytes;
+}
diff --git a/drivers/media/test-drivers/vidtv/vidtv_pes.h b/drivers/media/test-drivers/vidtv/vidtv_pes.h
new file mode 100644
index 0000000000000..f2cf3872db545
--- /dev/null
+++ b/drivers/media/test-drivers/vidtv/vidtv_pes.h
@@ -0,0 +1,186 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Vidtv serves as a reference DVB driver and helps validate the existing APIs
+ * in the media subsystem. It can also aid developers working on userspace
+ * applications.
+ *
+ * This file contains the logic to translate the ES data for one access unit
+ * from an encoder into MPEG TS packets. It does so by first encapsulating it
+ * with a PES header and then splitting it into TS packets.
+ *
+ * Written by Daniel W. S. Almeida <[email protected]>
+ */
+
+#ifndef VIDTV_PES_H
+#define VIDTV_PES_H
+
+#include <asm/byteorder.h>
+#include <linux/types.h>
+
+#include "vidtv_common.h"
+
+#define PES_MAX_LEN 65536 /* Set 'length' to 0 if greater */
+#define PES_START_CODE_PREFIX 0x001 /* 00 00 01 */
+
+struct vidtv_pes_optional_pts {
+ struct {
+ #if defined(__LITTLE_ENDIAN_BITFIELD)
+ u8 marker3:1; /* always 0x1 */
+ u16 pts3:15;
+ u8 marker2:1; /* always 0x1 */
+ u16 pts2:15;
+ u8 marker1:1; /* always 0x1 */
+ u8 pts1:3;
+ u8 two:4; /* always 0010b */
+ #elif defined(__BIG_ENDIAN_BITFIELD)
+ u8 two:4; /* always 0010b */
+ u8 pts1:3;
+ u8 marker1:1; /* always 0x1 */
+ u16 pts2:15;
+ u8 marker2:1; /* always 0x1 */
+ u16 pts3:15;
+ u8 marker3:1; /* always 0x1 */
+ #else
+ #error "Unknown bit ordering"
+ #endif
+ } __packed;
+} __packed;
+
+struct vidtv_pes_optional_pts_dts {
+ struct {
+ #if defined(__LITTLE_ENDIAN_BITFIELD)
+ u8 marker6:1; /* always 0x1 */
+ u16 dts3:15;
+ u8 marker5:1; /* always 0x1 */
+ u16 dts2:15;
+ u8 marker4:1; /* always 0x1 */
+ u8 dts1:3;
+ u8 one:4; /* always 0001b */
+ u8 marker3:1; /* always 0x1 */
+ u16 pts3:15;
+ u8 marker2:1; /* always 0x1 */
+ u16 pts2:15;
+ u8 marker1:1; /* always 0x1 */
+ u8 pts1:3;
+ u8 three:4; /* always 0011b */
+ #elif defined(__BIG_ENDIAN_BITFIELD)
+ u8 three:4; /* always 0011b */
+ u8 pts1:3;
+ u8 marker1:1; /* always 0x1 */
+ u16 pts2:15;
+ u8 marker2:1; /* always 0x1 */
+ u16 pts3:15;
+ u8 marker3:1; /* always 0x1 */
+ u8 one:4; /* always 0001b */
+ u8 dts1:3;
+ u8 marker4:1; /* always 0x1 */
+ u16 dts2:15;
+ u8 marker5:1; /* always 0x1 */
+ u16 dts3:15;
+ u8 marker6:1; /* always 0x1 */
+ #else
+ #error "Unknown bit ordering"
+ #endif
+ } __packed;
+} __packed;
+
+struct vidtv_pes_optional {
+ union {
+ __be16 bitfield;
+ struct {
+ u16 two:2; /* always 0x2*/
+ u16 PES_scrambling_control:2;
+ u16 PES_priority:1;
+ u16 data_alignment_indicator:1; /* ununsed for us */
+ u16 copyright:1;
+ u16 original_or_copy:1;
+ u16 PTS_DTS:2;
+ /* These flags show which components are actually
+ * present in the "optinal fields" in the optinal PES
+ * header and which are not. Vidtv currently does
+ * not need any of these.
+ */
+ u16 ESCR:1;
+ u16 ES_rate:1;
+ u16 DSM_trick_mode:1;
+ u16 additional_copy_info:1;
+ u16 PES_CRC:1;
+ u16 PES_extension:1;
+ } __packed;
+ } __packed;
+ u8 length;
+} __packed;
+
+struct vidtv_mpeg_pes {
+ union {
+ __be32 bitfield;
+ struct {
+ /* These two together make the 32-bit start-code */
+ u32 packet_start_code_prefix:24;
+ u32 stream_id:8;
+ } __packed;
+ } __packed;
+ /* after this field until the end of the PES data payload */
+ u16 length;
+ struct vidtv_pes_optional optional[];
+} __packed;
+
+struct pes_header_write_args {
+ void *dest_buf;
+ u32 dest_offset;
+ u32 dest_buf_sz;
+ bool is_s302m_pkt;
+
+ bool send_pts;
+ u64 pts;
+
+ bool send_dts;
+ u64 dts;
+
+ u16 stream_id;
+ /* might be used by an encoder if needed, gets discarded by decoder */
+ u32 n_pes_h_s_bytes;
+};
+
+struct pes_ts_header_write_args {
+ void *dest_buf;
+ u32 dest_offset;
+ u32 dest_buf_sz;
+ u16 pid;
+ u8 *continuity_counter;
+ bool wrote_pes_header;
+ u32 n_stuffing_bytes;
+};
+
+struct pes_write_args {
+ void *dest_buf; /* pointer to a program mux buffer */
+ void *from; /* pointer to the encoder buffer */
+
+ /* the size of one access unit (with any headers it might need) */
+ u32 access_unit_len;
+
+ u32 dest_offset; /* where to start writing in the program mux buffer */
+ u32 dest_buf_sz; /* how big is the program mux buffer */
+ u16 pid; /* TS packet ID */
+
+ /* use SMPTE 302M to packetize the data */
+ bool is_s302m_pkt;
+
+ u8 *continuity_counter; /* incremented for every TS packet */
+
+ /* Examples: Audio streams (0xc0-0xdf), Video streams (0xe0-0xef) */
+ u16 stream_id;
+
+ bool send_pts;
+ u64 pts;
+
+ bool send_dts;
+ u64 dts;
+
+ /* might be used by an encoder if needed, gets discarded by decoder */
+ u32 n_pes_h_s_bytes;
+};
+
+u32 vidtv_pes_write_into(struct pes_write_args args);
+
+#endif // VIDTV_PES_H
--
2.26.2

2020-05-28 21:42:42

by Shuah Khan

[permalink] [raw]
Subject: Re: [RFC, WIP, v6 00/10] media: vidtv: implement a virtual DVB driver

On 5/20/20 1:03 AM, Daniel W. S. Almeida wrote:
> From: "Daniel W. S. Almeida" <[email protected]>
>
> This series is work in progress. It represents the current work done on a
> virtual DVB driver for the Linux media subsystem. I am new to the media
> subsystem and to kernel development in general.
>
> This driver aims to:
> - Serve as template for new DVB driver writers
> - Help userspace application writers in general
> - Push fake audio/video to userspace for testing
> purposes
> - Push debug information to userspace via debugfs
>
> This should currently be able to feed PSI and PCM audio data to
> userspace.
>
> I appreciate any feedback!
>

Run checkpatch --strict on these. There are several warns and an error
which could improve the code as well as the improving Kconfig entry
as suggested below.

WARNING: please write a paragraph that describes the config symbol fully
#108: FILE: drivers/media/test-drivers/Kconfig:7:
+menuconfig DVB_TEST_DRIVERS

I will send review comments on individual patches.

thanks,
-- Shuah

2020-05-28 22:55:57

by Shuah Khan

[permalink] [raw]
Subject: Re: [RFC, WIP, v6 01/10] media: vidtv: add Kconfig entry

On 5/20/20 1:03 AM, Daniel W. S. Almeida wrote:
> From: "Daniel W. S. Almeida" <[email protected]>
>
> Add the necessary Kconfig entries and a dummy Makefile to compile the new
> virtual DVB test driver (vidtv).
>
> Signed-off-by: Daniel W. S. Almeida <[email protected]>
> ---
> drivers/media/test-drivers/Kconfig | 10 ++++++++++
> drivers/media/test-drivers/Makefile | 1 +
> drivers/media/test-drivers/vidtv/Kconfig | 11 +++++++++++
> drivers/media/test-drivers/vidtv/Makefile | 2 ++
> 4 files changed, 24 insertions(+)
> create mode 100644 drivers/media/test-drivers/vidtv/Kconfig
> create mode 100644 drivers/media/test-drivers/vidtv/Makefile
>
> diff --git a/drivers/media/test-drivers/Kconfig b/drivers/media/test-drivers/Kconfig
> index 188381c855939..7d273a8a7acc2 100644
> --- a/drivers/media/test-drivers/Kconfig
> +++ b/drivers/media/test-drivers/Kconfig
> @@ -4,6 +4,10 @@ menuconfig V4L_TEST_DRIVERS
> bool "V4L test drivers"
> depends on VIDEO_DEV
>
> +menuconfig DVB_TEST_DRIVERS
> + bool "DVB test drivers"
> + depends on DVB_CORE && MEDIA_SUPPORT && I2C
> +
> if V4L_TEST_DRIVERS
>
> source "drivers/media/test-drivers/vimc/Kconfig"
> @@ -24,3 +28,9 @@ config VIDEO_VIM2M
> source "drivers/media/test-drivers/vicodec/Kconfig"
>
> endif #V4L_TEST_DRIVERS
> +
> +if DVB_TEST_DRIVERS
> +
> +source "drivers/media/test-drivers/vidtv/Kconfig"
> +
> +endif #DVB_TEST_DRIVERS
> diff --git a/drivers/media/test-drivers/Makefile b/drivers/media/test-drivers/Makefile
> index 74410d3a9f2d2..9f0e4ebb2efe7 100644
> --- a/drivers/media/test-drivers/Makefile
> +++ b/drivers/media/test-drivers/Makefile
> @@ -7,3 +7,4 @@ obj-$(CONFIG_VIDEO_VIMC) += vimc/
> obj-$(CONFIG_VIDEO_VIVID) += vivid/
> obj-$(CONFIG_VIDEO_VIM2M) += vim2m.o
> obj-$(CONFIG_VIDEO_VICODEC) += vicodec/
> +obj-$(CONFIG_DVB_VIDTV) += vidtv/
> diff --git a/drivers/media/test-drivers/vidtv/Kconfig b/drivers/media/test-drivers/vidtv/Kconfig
> new file mode 100644
> index 0000000000000..22c4fd39461f1
> --- /dev/null
> +++ b/drivers/media/test-drivers/vidtv/Kconfig
> @@ -0,0 +1,11 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +config DVB_VIDTV
> + tristate "Virtual DVB Driver (vidtv)"
> + depends on DVB_CORE && MEDIA_SUPPORT && I2C
> + help

Add one line summary/name for the driver "Virtual DVB test driver" is
a good description.

> + The virtual DVB test driver serves as a reference DVB driver and helps
> + validate the existing APIs in the media subsystem. It can also aid developers
> + working on userspace applications.
> +

Wrapped lines. Split this to read better.

Take a look at others e.g: drivers/media/test-drivers/vimc/Kconfig
for reference.

> +
> + When in doubt, say N.
> diff --git a/drivers/media/test-drivers/vidtv/Makefile b/drivers/media/test-drivers/vidtv/Makefile
> new file mode 100644
> index 0000000000000..d1558d84eeaed
> --- /dev/null
> +++ b/drivers/media/test-drivers/vidtv/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
>

btw. Did you test it with just this patch?

thanks,
-- Shuah

2020-05-29 00:01:23

by Shuah Khan

[permalink] [raw]
Subject: Re: [RFC, WIP, v6 00/10] media: vidtv: implement a virtual DVB driver

On 5/20/20 1:03 AM, Daniel W. S. Almeida wrote:
> From: "Daniel W. S. Almeida" <[email protected]>
>
> This series is work in progress. It represents the current work done on a
> virtual DVB driver for the Linux media subsystem. I am new to the media
> subsystem and to kernel development in general.
>
> This driver aims to:
> - Serve as template for new DVB driver writers
> - Help userspace application writers in general
> - Push fake audio/video to userspace for testing
> purposes
> - Push debug information to userspace via debugfs
>
> This should currently be able to feed PSI and PCM audio data to
> userspace.
>
> I appreciate any feedback!
>
> Changes in v6:
> Addressed the following issues:
> - Makefile was not actually compiling everything;
> - The bridge driver should be a platform driver;
> - There are lots of warnings and other errors produced
> by the driver.
>
> Changes in v5:
>
> Removed all calls to WARN_ON in favor of pr_warn_ratelimited
> Add a #define for pr_fmt
> Reworked the byte swapping logic for big/little endian support:
> big endian fields now prepended with __beXX for 'sparse' checks
> bitfields now laid in reverse order if LITTLE_ENDIAN_BITFIELD
> is set
>
> media: vidtv: implement a tuner driver
> Return -EINVAL instead of -1 in vidtv_tuner_check_frequency_shift
> media: vidtv: implement a demodulator driver
> Add POLL_THRD_TIME #define
> Implement dvb_frontend_ops as a single struct instead of three
> media: vidtv: move config structs into a separate header
> Removed this commit, configs are now in vidtv_tuner.h and vidtv_demod.h
> media: vidtv: add a bridge driver
> module_param: all fs permissions set to 0
> removed param 'chosen_delsys'
> module_param: removed "optional" string: all module_params are optional
> renamed vidtv_bridge -> vidtv_bridg: so the source code and module names
> are different
> media: vidtv: add wrappers for memcpy and memset
> Added kernel-docs
> write address is now computed internally
> media: vidtv: add MPEG TS common code
> Drop packets if the buffer is full
> media: vidtv: implement a PSI generator
> Removed vidtv_psi_ts_psi_write_stuffing()
> Forcibly align misaligned buffers
> Drop packets if buffer is full
> media: vidtv: implement a PES packetizer
> Remove vidtv_extract_bits() in favor of GENMASK() and bitwise &
> Forcibly align misaligned buffers
> media: vidtv: Implement a SMPTE 302M encoder
> Added kernel-docs for struct vidtv_encoder
> media: vidtv: Add a MPEG Transport Stream Multiplexer
> Added check for minimum and maximum buffer size.
> Drop packets if buffer is full
>
>
> Changes in v4:
> Added a PES packetizer
> Implemented a minimum version of the SMPTE 302m encoder for AES3 audio
> Fixed endianness in the PSI generator, converting fields to big endian where applicable
> Added a minimal TS mux abstraction
>
> Changes in v3:
> Added a bridge driver
> Renamed the driver to vidtv
> Renamed/reworked commits into smaller pieces
> Moved the driver into its own directory
> Fixed the code for the signal strength in the tuner
> Removed useless enums in the tuner driver (e.g. lock_status, power_status...)
> Reworked the logic for the poll_snr thread in the demodulator driver
> Moved MPEG related code to the bridge driver, as it controls the demux logic
> Changed literals to #defines, used sizeof in place of integer literals when
> computing the size of PSI structs
> Moved the MPEG PSI tables to the heap to reduce stack space usage
> Now using usleep_range in place of msleep_interruptible in the MPEG TS thread
> Wrapped memcpy and memset to protect against buffer overflow when writing to the
> MPEG TS buffer.
>
> Changes in v2:
> Attempted not to break assignments into multiple lines as much as possible.
> Code now passes checkpatch strict mode
>
> media: dvb_dummy_tuner: implement driver skeleton
> Changed snr values to mili db
> Return value from 0-100 to indicate how far off the requested
> frequency is from a valid one
>
> Use the frequency shift to interpolate between 34dB and 10dB if
> we can not match against the SNR lookup table
> Remove sleep calls for suspend/resume
>
> Fix memcpy call for the config struct
>
> media: dvb_dummy_fe.c: lose TS lock on bad snr
> Randomly recover the TS lock if the signal quality improves
>
> media: dvb_dummy_fe.c: write PSI information into DMX buffer
> Split the patch into multiple header/source files
>
> Hexadecimal literals are now lower case
>
> Prefer short function names / reduce function signatures
>
> Add #defines for constants when computing section lengths
>
> Change signature for functions that take a dummy channel as
> argument (i.e. channels* is now channels[NUM_CHANNELS])
>
> Daniel W. S. Almeida (10):
> media: vidtv: add Kconfig entry
> media: vidtv: implement a tuner driver

You can collapse patch 1 and 2. I don't see why you need two
separate patches.

> media: vidtv: implement a demodulator driver
> media: vidtv: add a bridge driver
> media: vidtv: add wrappers for memcpy and memset
> media: vidtv: add MPEG TS common code
> media: vidtv: implement a PSI generator
> media: vidtv: implement a PES packetizer
> media: vidtv: Implement a SMPTE 302M encoder
> media: vidtv: Add a MPEG Transport Stream Multiplexer
>
> arch/Kconfig | 2 +-

Why do you need this change. It is part of patch 04 with no
explanation on why this change is necessary.


> drivers/media/dvb-core/dvbdev.c | 1 +

Same here.

> drivers/media/test-drivers/Kconfig | 10 +
> drivers/media/test-drivers/Makefile | 1 +
> drivers/media/test-drivers/vidtv/Kconfig | 11 +
> drivers/media/test-drivers/vidtv/Makefile | 9 +
> .../media/test-drivers/vidtv/vidtv_bridge.c | 511 ++++++++
> .../media/test-drivers/vidtv/vidtv_bridge.h | 41 +
> .../media/test-drivers/vidtv/vidtv_channel.c | 339 ++++++
> .../media/test-drivers/vidtv/vidtv_channel.h | 66 ++
> .../media/test-drivers/vidtv/vidtv_common.c | 86 ++
> .../media/test-drivers/vidtv/vidtv_common.h | 34 +
> .../media/test-drivers/vidtv/vidtv_demod.c | 444 +++++++
> .../media/test-drivers/vidtv/vidtv_demod.h | 41 +
> .../media/test-drivers/vidtv/vidtv_encoder.h | 103 ++
> drivers/media/test-drivers/vidtv/vidtv_mux.c | 434 +++++++
> drivers/media/test-drivers/vidtv/vidtv_mux.h | 105 ++
> drivers/media/test-drivers/vidtv/vidtv_pes.c | 450 +++++++
> drivers/media/test-drivers/vidtv/vidtv_pes.h | 186 +++
> drivers/media/test-drivers/vidtv/vidtv_psi.c | 1055 +++++++++++++++++
> drivers/media/test-drivers/vidtv/vidtv_psi.h | 420 +++++++
> .../media/test-drivers/vidtv/vidtv_s302m.c | 636 ++++++++++
> .../media/test-drivers/vidtv/vidtv_s302m.h | 119 ++
> drivers/media/test-drivers/vidtv/vidtv_ts.c | 157 +++
> drivers/media/test-drivers/vidtv/vidtv_ts.h | 120 ++
> .../media/test-drivers/vidtv/vidtv_tuner.c | 408 +++++++
> .../media/test-drivers/vidtv/vidtv_tuner.h | 26 +
> 27 files changed, 5814 insertions(+), 1 deletion(-)
> create mode 100644 drivers/media/test-drivers/vidtv/Kconfig
> create mode 100644 drivers/media/test-drivers/vidtv/Makefile
> create mode 100644 drivers/media/test-drivers/vidtv/vidtv_bridge.c
> create mode 100644 drivers/media/test-drivers/vidtv/vidtv_bridge.h
> create mode 100644 drivers/media/test-drivers/vidtv/vidtv_channel.c
> create mode 100644 drivers/media/test-drivers/vidtv/vidtv_channel.h
> create mode 100644 drivers/media/test-drivers/vidtv/vidtv_common.c
> create mode 100644 drivers/media/test-drivers/vidtv/vidtv_common.h
> create mode 100644 drivers/media/test-drivers/vidtv/vidtv_demod.c
> create mode 100644 drivers/media/test-drivers/vidtv/vidtv_demod.h
> create mode 100644 drivers/media/test-drivers/vidtv/vidtv_encoder.h
> create mode 100644 drivers/media/test-drivers/vidtv/vidtv_mux.c
> create mode 100644 drivers/media/test-drivers/vidtv/vidtv_mux.h
> create mode 100644 drivers/media/test-drivers/vidtv/vidtv_pes.c
> create mode 100644 drivers/media/test-drivers/vidtv/vidtv_pes.h
> create mode 100644 drivers/media/test-drivers/vidtv/vidtv_psi.c
> create mode 100644 drivers/media/test-drivers/vidtv/vidtv_psi.h
> create mode 100644 drivers/media/test-drivers/vidtv/vidtv_s302m.c
> create mode 100644 drivers/media/test-drivers/vidtv/vidtv_s302m.h
> create mode 100644 drivers/media/test-drivers/vidtv/vidtv_ts.c
> create mode 100644 drivers/media/test-drivers/vidtv/vidtv_ts.h
> create mode 100644 drivers/media/test-drivers/vidtv/vidtv_tuner.c
> create mode 100644 drivers/media/test-drivers/vidtv/vidtv_tuner.h
>

thanks,
-- Shuah

2020-06-02 23:17:32

by Shuah Khan

[permalink] [raw]
Subject: Re: [RFC, WIP, v6 04/10] media: vidtv: add a bridge driver

Hi Daniel,

On 5/20/20 1:03 AM, Daniel W. S. Almeida wrote:
> From: "Daniel W. S. Almeida" <[email protected]>
>
> Digital TV devices consist of several independent hardware components which
> are controlled by different drivers.
> Each media device is controlled by a group of cooperating drivers with the
> bridge driver as the main driver.
>
> This patch adds a bridge driver for the Virtual Digital TV driver [vidtv].
>
> The bridge driver binds to the other drivers, that is, vidtv_tuner and
> vidtv_demod and implements the digital demux logic, providing userspace
> with a MPEG Transport Stream.
>
> Move config structs to a common header so they can be used by the bridge
> driver and by their respective drivers.
>
> Signed-off-by: Daniel W. S. Almeida <[email protected]>
>
> BRIDGE WIP
> ---
> arch/Kconfig | 2 +-
> drivers/media/dvb-core/dvbdev.c | 1 +
> drivers/media/test-drivers/vidtv/Makefile | 4 +-
> .../media/test-drivers/vidtv/vidtv_bridge.c | 435 ++++++++++++++++++
> .../media/test-drivers/vidtv/vidtv_bridge.h | 39 ++
> 5 files changed, 479 insertions(+), 2 deletions(-)
> create mode 100644 drivers/media/test-drivers/vidtv/vidtv_bridge.c
> create mode 100644 drivers/media/test-drivers/vidtv/vidtv_bridge.h
>

I ran a test and does't look like you are releasing demod and tuner
module when removing dvb_vidtv_bridge. Probe for these happens when

After modprobe dvb_vidtv_bridge and lsmod shows all 3 modules.
dvb_vidtv_demod 16384 0
dvb_vidtv_bridge 45056 0
dvb_core 143360 1 dvb_vidtv_bridge

After rmmod dvb_vidtv_bridge, lsmod shows

dvb_vidtv_tuner 16384 0
dvb_vidtv_demod 16384 0

vidtv_bridge_remove() doesn't seem to cleaning up completely and
releasing thes modules vidtv_bridge_init() loads. Also it has to
delete i2c adapter that is added in vidtv_bridge_init(). Call
to i2c_del_adapter() is miising in vidtv_bridge_remove()

Please see commnets inline after vidtv_bridge_remove().

After a couple of modprobe and rmmods on dvb_vidtv_bridge, I see
paninc after modprobe.

[ 3651.863602] dvbdev: DVB: registering new adapter (dvb_vidtv_bridge)
[ 3821.179081] dvbdev: DVB: registering new adapter (dvb_vidtv_bridge)
[ 4087.198087] dvbdev: DVB: registering new adapter (dvb_vidtv_bridge)
[ 4087.202732] i2c i2c-10: DVB: registering adapter 0 frontend 0 (Dummy
demod for DVB-T/T2/C/S/S2)...
[ 4087.203112] BUG: kernel NULL pointer dereference, address:
0000000000000000
[ 4087.203140] #PF: supervisor read access in kernel mode
[ 4087.203142] #PF: error_code(0x0000) - not-present page
[ 4087.203144] PGD 0 P4D 0
[ 4087.203147] Oops: 0000 [#1] SMP NOPTI
[ 4087.203150] CPU: 6 PID: 1875 Comm: modprobe Not tainted 5.7.0+ #3
[ 4087.203151] Hardware name: LENOVO 10VGCTO1WW/3130, BIOS M1XKT45A
08/21/2019
[ 4087.203157] RIP: 0010:strlen+0x0/0x20
[ 4087.203160] Code: 48 89 f9 74 09 48 83 c1 01 80 39 00 75 f7 31 d2 44
0f b6 04 16 44 88 04 11 48 83 c2 01 45 84 c0 75 ee c3 0f 1f 80 00 00 00
00 <80> 3f 00 74 10 48 89 f8 48 83 c0 01 80 38 00 75 f7 48 29 f8 c3 31
[ 4087.203162] RSP: 0018:ffffa604c090b940 EFLAGS: 00010246
[ 4087.203164] RAX: ffff9a5188735cc0 RBX: ffff9a5188735c40 RCX:
0000000000000000
[ 4087.203165] RDX: 0000000000000000 RSI: ffffffffc10d4fd8 RDI:
0000000000000000
[ 4087.203167] RBP: ffffa604c090b9b0 R08: ffff9a51b7bb20a0 R09:
ffff9a51b6807640
[ 4087.203168] R10: 0000000000000000 R11: 0000000000000001 R12:
ffff9a5188735cc0
[ 4087.203170] R13: 0000000000000000 R14: ffff9a51acd74710 R15:
ffff9a519531f000
[ 4087.203172] FS: 00007fd38f2b1540(0000) GS:ffff9a51b7b80000(0000)
knlGS:0000000000000000
[ 4087.203174] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 4087.203175] CR2: 0000000000000000 CR3: 00000001ffc50000 CR4:
00000000003406e0
[ 4087.203177] Call Trace:
[ 4087.203191] ? vidtv_channels_init+0x79/0x130 [dvb_vidtv_bridge]
[ 4087.203197] vidtv_mux_init+0xa6/0x1a0 [dvb_vidtv_bridge]
[ 4087.203201] vidtv_bridge_probe+0x397/0x400 [dvb_vidtv_bridge]
[ 4087.203205] ? vidtv_bridge_remove+0x80/0x80 [dvb_vidtv_bridge]
[ 4087.203210] platform_drv_probe+0x3b/0x80
[ 4087.203213] really_probe+0x2b3/0x3e0
[ 4087.203216] driver_probe_device+0xbc/0x100
[ 4087.203218] device_driver_attach+0x5d/0x70
[ 4087.203220] __driver_attach+0x8f/0x150
[ 4087.203222] ? device_driver_attach+0x70/0x70
[ 4087.203225] bus_for_each_dev+0x7e/0xc0
[ 4087.203227] driver_attach+0x1e/0x20
[ 4087.203229] bus_add_driver+0x152/0x1f0
[ 4087.203232] driver_register+0x74/0xd0
[ 4087.203234] __platform_driver_register+0x36/0x40
[ 4087.203238] vidtv_bridge_init+0x31/0x1000 [dvb_vidtv_bridge]
[ 4087.203240] ? 0xffffffffc0a4a000
[ 4087.203245] do_one_initcall+0x4a/0x200
[ 4087.203248] ? kfree+0x231/0x250
[ 4087.203252] ? _cond_resched+0x19/0x30
[ 4087.203254] ? kmem_cache_alloc_trace+0x16c/0x240
[ 4087.203258] do_init_module+0x62/0x250
[ 4087.203260] load_module+0x2894/0x2af0
[ 4087.203264] __do_sys_finit_module+0xbe/0x120
[ 4087.203266] ? __do_sys_finit_module+0xbe/0x120
[ 4087.203269] __x64_sys_finit_module+0x1a/0x20
[ 4087.203271] do_syscall_64+0x57/0x1b0
[ 4087.203274] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 4087.203276] RIP: 0033:0x7fd38f3f670d
[ 4087.203278] Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa
48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f
05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 53 f7 0c 00 f7 d8 64 89 01 48
[ 4087.203280] RSP: 002b:00007ffe35039d08 EFLAGS: 00000246 ORIG_RAX:
0000000000000139
[ 4087.203282] RAX: ffffffffffffffda RBX: 000055933c9a7b50 RCX:
00007fd38f3f670d
[ 4087.203284] RDX: 0000000000000000 RSI: 000055933bacf358 RDI:
0000000000000003
[ 4087.203285] RBP: 0000000000040000 R08: 0000000000000000 R09:
0000000000000000
[ 4087.203286] R10: 0000000000000003 R11: 0000000000000246 R12:
000055933bacf358
[ 4087.203288] R13: 0000000000000000 R14: 000055933c9af3f0 R15:
000055933c9a7b50
[ 4087.203290] Modules linked in: dvb_vidtv_bridge(+) dvb_vidtv_tuner
dvb_vidtv_demod dvb_core rfcomm ccm cmac algif_hash algif_skcipher
af_alg bnep binfmt_misc nls_iso8859_1 edac_mce_amd kvm_amd
snd_hda_codec_realtek kvm snd_hda_codec_hdmi snd_hda_codec_generic
ledtrig_audio irqbypass snd_hda_intel snd_intel_dspcfg snd_usb_audio
amdgpu snd_hda_codec snd_usbmidi_lib mc snd_hda_core snd_hwdep snd_pcm
snd_seq_midi snd_seq_midi_event btusb snd_rawmidi btrtl btbcm btintel
bluetooth crct10dif_pclmul ath10k_pci ath10k_core snd_seq ath
snd_seq_device amd_iommu_v2 snd_timer ghash_clmulni_intel ecdh_generic
gpu_sched ttm mac80211 aesni_intel drm_kms_helper crypto_simd ecc pl2303
cryptd glue_helper snd cec input_leds cfg80211 serio_raw i2c_algo_bit
wmi_bmof fb_sys_fops efi_pstore syscopyarea sysfillrect snd_pci_acp3x
k10temp ipmi_devintf sysimgblt ccp libarc4 soundcore ipmi_msghandler
mac_hid sch_fq_codel parport_pc ppdev lp parport drm ip_tables x_tables
autofs4 hid_generic usbhid hid crc32_pclmul
[ 4087.203325] ahci psmouse i2c_piix4 libahci nvme nvme_core r8169
realtek wmi video [last unloaded: dvb_vidtv_bridge]
[ 4087.203335] CR2: 0000000000000000
[ 4087.203337] ---[ end trace c8d767383a27d74e ]---
[ 4087.505440] RIP: 0010:strlen+0x0/0x20
[ 4087.505446] Code: 48 89 f9 74 09 48 83 c1 01 80 39 00 75 f7 31 d2 44
0f b6 04 16 44 88 04 11 48 83 c2 01 45 84 c0 75 ee c3 0f 1f 80 00 00 00
00 <80> 3f 00 74 10 48 89 f8 48 83 c0 01 80 38 00 75 f7 48 29 f8 c3 31
[ 4087.505448] RSP: 0018:ffffa604c090b940 EFLAGS: 00010246
[ 4087.505450] RAX: ffff9a5188735cc0 RBX: ffff9a5188735c40 RCX:
0000000000000000
[ 4087.505452] RDX: 0000000000000000 RSI: ffffffffc10d4fd8 RDI:
0000000000000000
[ 4087.505453] RBP: ffffa604c090b9b0 R08: ffff9a51b7bb20a0 R09:
ffff9a51b6807640
[ 4087.505455] R10: 0000000000000000 R11: 0000000000000001 R12:
ffff9a5188735cc0
[ 4087.505456] R13: 0000000000000000 R14: ffff9a51acd74710 R15:
ffff9a519531f000
[ 4087.505458] FS: 00007fd38f2b1540(0000) GS:ffff9a51b7b80000(0000)
knlGS:0000000000000000
[ 4087.505460] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 4087.505461] CR2: 0000000000000000 CR3: 00000001ffc50000 CR4:
00000000003406e0


> diff --git a/arch/Kconfig b/arch/Kconfig
> index 786a85d4ad40d..ddcb4a68ee940 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -868,7 +868,7 @@ config VMAP_STACK
> be enabled.
>
> config ARCH_OPTIONAL_KERNEL_RWX
> - def_bool n
> + def_bool y
>
> config ARCH_OPTIONAL_KERNEL_RWX_DEFAULT
> def_bool n
> diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
> index 80b6a71aa33e4..38c8b848c921f 100644
> --- a/drivers/media/dvb-core/dvbdev.c
> +++ b/drivers/media/dvb-core/dvbdev.c
> @@ -962,6 +962,7 @@ int dvb_usercopy(struct file *file,
> }
>
> #if IS_ENABLED(CONFIG_I2C)
> +#pragma clang optimize off
> struct i2c_client *dvb_module_probe(const char *module_name,
> const char *name,
> struct i2c_adapter *adap,
> diff --git a/drivers/media/test-drivers/vidtv/Makefile b/drivers/media/test-drivers/vidtv/Makefile
> index 21e50c11c94d0..c99cd13f4adaf 100644
> --- a/drivers/media/test-drivers/vidtv/Makefile
> +++ b/drivers/media/test-drivers/vidtv/Makefile
> @@ -2,5 +2,7 @@
>
> dvb-vidtv-tuner-objs := vidtv_tuner.o
> dvb-vidtv-demod-objs := vidtv_demod.o
> +dvb-vidtv-bridge-objs := vidtv_bridge.o
>
> -obj-$(CONFIG_DVB_VIDTV) += dvb-vidtv-tuner.o dvb-vidtv-demod.o
> +obj-$(CONFIG_DVB_VIDTV) += dvb-vidtv-tuner.o dvb-vidtv-demod.o \
> + dvb-vidtv-bridge.o
> diff --git a/drivers/media/test-drivers/vidtv/vidtv_bridge.c b/drivers/media/test-drivers/vidtv/vidtv_bridge.c
> new file mode 100644
> index 0000000000000..bc1c612c23013
> --- /dev/null
> +++ b/drivers/media/test-drivers/vidtv/vidtv_bridge.c
> @@ -0,0 +1,435 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * The Virtual DTV test driver serves as a reference DVB driver and helps
> + * validate the existing APIs in the media subsystem. It can also aid
> + * developers working on userspace applications.
> + *
> + * Written by Daniel W. S. Almeida <[email protected]>
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ":%s, %d: " fmt, __func__, __LINE__
> +
> +#include <linux/moduleparam.h>
> +#include <linux/mutex.h>
> +#include <linux/platform_device.h>
> +#include <linux/printk.h>
> +#include <linux/time.h>
> +#include <linux/types.h>
> +#include <linux/workqueue.h>
> +
> +#include "vidtv_bridge.h"
> +#include "vidtv_demod.h"
> +#include "vidtv_tuner.h"
> +
> +#define TUNER_DEFAULT_ADDR 0x68
> +#define DEMOD_DEFAULT_ADDR 0x60
> +
> +static unsigned int drop_tslock_prob_on_low_snr;
> +module_param(drop_tslock_prob_on_low_snr, uint, 0);
> +MODULE_PARM_DESC(drop_tslock_prob_on_low_snr,
> + "Probability of losing the TS lock if the signal quality is bad");
> +
> +static unsigned int recover_tslock_prob_on_good_snr;
> +module_param(recover_tslock_prob_on_good_snr, uint, 0);
> +MODULE_PARM_DESC(recover_tslock_prob_on_good_snr,
> + "Probability recovering the TS lock when the signal improves");
> +
> +static unsigned int mock_power_up_delay_msec;
> +module_param(mock_power_up_delay_msec, uint, 0);
> +MODULE_PARM_DESC(mock_power_up_delay_msec, "Simulate a power up delay");
> +
> +static unsigned int mock_tune_delay_msec;
> +module_param(mock_tune_delay_msec, uint, 0);
> +MODULE_PARM_DESC(mock_tune_delay_msec, "Simulate a tune delay");
> +
> +static unsigned int vidtv_valid_dvb_t_freqs[8];
> +module_param_array(vidtv_valid_dvb_t_freqs, uint, NULL, 0);
> +MODULE_PARM_DESC(vidtv_valid_dvb_t_freqs,
> + "Valid DVB-T frequencies to simulate");
> +
> +static unsigned int vidtv_valid_dvb_c_freqs[8];
> +module_param_array(vidtv_valid_dvb_c_freqs, uint, NULL, 0);
> +MODULE_PARM_DESC(vidtv_valid_dvb_c_freqs,
> + "Valid DVB-C frequencies to simulate");
> +
> +static unsigned int vidtv_valid_dvb_s_freqs[8];
> +module_param_array(vidtv_valid_dvb_s_freqs, uint, NULL, 0);
> +MODULE_PARM_DESC(vidtv_valid_dvb_s_freqs,
> + "Valid DVB-C frequencies to simulate");
> +
> +static unsigned int max_frequency_shift_hz;
> +module_param(max_frequency_shift_hz, uint, 0);
> +MODULE_PARM_DESC(max_frequency_shift_hz,
> + "Maximum shift in HZ allowed when tuning in a channel");
> +
> +DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nums);
> +
> +static int vidtv_start_streaming(struct vidtv_dvb *dvb)
> +{
> + if (dvb->streaming) {
> + pr_warn_ratelimited("Already streaming. Skipping.\n");
> + return 0;
> + }
> +
> + dvb->streaming = true;
> + return 0;
> +}
> +
> +static int vidtv_stop_streaming(struct vidtv_dvb *dvb)
> +{
> + dvb->streaming = false;
> +
> + return 0;
> +}
> +
> +static int vidtv_start_feed(struct dvb_demux_feed *feed)
> +{
> + struct dvb_demux *demux = feed->demux;
> + struct vidtv_dvb *dvb = demux->priv;
> + int rc;
> + int ret;
> +
> + if (!demux->dmx.frontend)
> + return -EINVAL;
> +
> + mutex_lock(&dvb->feed_lock);
> +
> + dvb->nfeeds++;
> + rc = dvb->nfeeds;
> +
> + if (dvb->nfeeds == 1) {
> + ret = vidtv_start_streaming(dvb);
> + if (ret < 0)
> + rc = ret;
> + }
> +
> + mutex_unlock(&dvb->feed_lock);
> + return rc;
> +}
> +
> +static int vidtv_stop_feed(struct dvb_demux_feed *feed)
> +{
> + struct dvb_demux *demux = feed->demux;
> + struct vidtv_dvb *dvb = demux->priv;
> + int err = 0;
> +
> + mutex_lock(&dvb->feed_lock);
> + dvb->nfeeds--;
> +
> + if (!dvb->nfeeds)
> + err = vidtv_stop_streaming(dvb);
> +
> + mutex_unlock(&dvb->feed_lock);
> + return err;
> +}
> +
> +static struct dvb_frontend *vidtv_get_frontend_ptr(struct i2c_client *c)
> +{
> + /* the demod will set this when its probe function runs */
> + struct vidtv_demod_state *state = i2c_get_clientdata(c);
> +
> + return &state->frontend;
> +}
> +
> +static int vidtv_master_xfer(struct i2c_adapter *i2c_adap,
> + struct i2c_msg msgs[],
> + int num)
> +{
> + return 0;
> +}
> +
> +static u32 vidtv_i2c_func(struct i2c_adapter *adapter)
> +{
> + return I2C_FUNC_I2C;
> +}
> +
> +static const struct i2c_algorithm vidtv_i2c_algorithm = {
> + .master_xfer = vidtv_master_xfer,
> + .functionality = vidtv_i2c_func,
> +};
> +
> +static int vidtv_bridge_i2c_register_adap(struct vidtv_dvb *dvb)
> +{
> + struct i2c_adapter *i2c_adapter = &dvb->i2c_adapter;
> +
> + strscpy(i2c_adapter->name, "vidtv_i2c", sizeof(i2c_adapter->name));
> + i2c_adapter->owner = THIS_MODULE;
> + i2c_adapter->algo = &vidtv_i2c_algorithm;
> + i2c_adapter->algo_data = NULL;
> + i2c_adapter->timeout = 500;
> + i2c_adapter->retries = 3;
> + i2c_adapter->dev.parent = &dvb->pdev->dev;
> +
> + i2c_set_adapdata(i2c_adapter, dvb);
> + return i2c_add_adapter(&dvb->i2c_adapter);
> +}
> +
> +static int vidtv_bridge_register_adap(struct vidtv_dvb *dvb)
> +{
> + int ret = 0;
> +
> + ret = dvb_register_adapter(&dvb->adapter,
> + KBUILD_MODNAME,
> + THIS_MODULE,
> + &dvb->i2c_adapter.dev,
> + adapter_nums);
> +
> + return ret;
> +}
> +
> +static int vidtv_bridge_dmx_init(struct vidtv_dvb *dvb)
> +{
> + dvb->demux.dmx.capabilities = DMX_TS_FILTERING |
> + DMX_SECTION_FILTERING;
> +
> + dvb->demux.priv = dvb;
> + dvb->demux.filternum = 256;
> + dvb->demux.feednum = 256;
> + dvb->demux.start_feed = vidtv_start_feed;
> + dvb->demux.stop_feed = vidtv_stop_feed;
> +
> + return dvb_dmx_init(&dvb->demux);
> +}
> +
> +static int vidtv_bridge_dmxdev_init(struct vidtv_dvb *dvb)
> +{
> + dvb->dmx_dev.filternum = 256;
> + dvb->dmx_dev.demux = &dvb->demux.dmx;
> + dvb->dmx_dev.capabilities = 0;
> +
> + return dvb_dmxdev_init(&dvb->dmx_dev, &dvb->adapter);
> +}
> +
> +static int vidtv_bridge_probe_demod(struct vidtv_dvb *dvb, u32 n)
> +{
> + struct vidtv_demod_config cfg = {};
> +
> + cfg.drop_tslock_prob_on_low_snr = drop_tslock_prob_on_low_snr;
> + cfg.recover_tslock_prob_on_good_snr = recover_tslock_prob_on_good_snr;
> +
> + dvb->i2c_client_demod[n] = dvb_module_probe("vidtv_demod",
> + NULL,
> + &dvb->i2c_adapter,
> + DEMOD_DEFAULT_ADDR,
> + &cfg);
> +
> + /* driver will not work anyways so bail out */
> + if (!dvb->i2c_client_demod[n])
> + return -ENODEV;
> +
> + /* retrieve a ptr to the frontend state */
> + dvb->fe[n] = vidtv_get_frontend_ptr(dvb->i2c_client_demod[n]);
> +
> + return 0;
> +}
> +
> +static int vidtv_bridge_probe_tuner(struct vidtv_dvb *dvb, u32 n)
> +{
> + struct vidtv_tuner_config cfg = {};
> +
> + cfg.fe = dvb->fe[n];
> + cfg.mock_power_up_delay_msec = mock_power_up_delay_msec;
> + cfg.mock_tune_delay_msec = mock_tune_delay_msec;
> +
> + memcpy(cfg.vidtv_valid_dvb_t_freqs,
> + vidtv_valid_dvb_t_freqs,
> + sizeof(vidtv_valid_dvb_t_freqs));
> +
> + memcpy(cfg.vidtv_valid_dvb_c_freqs,
> + vidtv_valid_dvb_c_freqs,
> + sizeof(vidtv_valid_dvb_c_freqs));
> +
> + memcpy(cfg.vidtv_valid_dvb_s_freqs,
> + vidtv_valid_dvb_s_freqs,
> + sizeof(vidtv_valid_dvb_s_freqs));
> +
> + cfg.max_frequency_shift_hz = max_frequency_shift_hz;
> +
> + dvb->i2c_client_tuner[n] = dvb_module_probe("vidtv_tuner",
> + NULL,
> + &dvb->i2c_adapter,
> + TUNER_DEFAULT_ADDR,
> + &cfg);
> +
> + return (dvb->i2c_client_tuner[n]) ? 0 : -ENODEV;
> +}
> +
> +static int vidtv_bridge_dvb_init(struct vidtv_dvb *dvb)
> +{
> + int ret;
> + int i;
> + int j;
> + int k;
> + int w;
> + int l;
> +
> + ret = vidtv_bridge_i2c_register_adap(dvb);
> + if (ret < 0)
> + goto fail_i2c;
> +
> + ret = vidtv_bridge_register_adap(dvb);
> + if (ret < 0)
> + goto fail_adapter;
> +
> + for (i = 0; i < NUM_FE; ++i) {
> + ret = vidtv_bridge_probe_demod(dvb, i);
> + if (ret < 0)
> + goto fail_demod_probe;
> +
> + ret = vidtv_bridge_probe_tuner(dvb, i);
> + if (ret < 0)
> + goto fail_tuner_probe;
> +
> + ret = dvb_register_frontend(&dvb->adapter, dvb->fe[i]);
> + if (ret < 0)
> + goto fail_fe;
> + }
> +
> + ret = vidtv_bridge_dmx_init(dvb);
> + if (ret < 0)
> + goto fail_dmx;
> +
> + ret = vidtv_bridge_dmxdev_init(dvb);
> + if (ret < 0)
> + goto fail_dmx_dev;
> +
> + for (j = 0; j < NUM_FE; ++j) {
> + ret = dvb->demux.dmx.connect_frontend(&dvb->demux.dmx,
> + &dvb->dmx_fe[j]);
> + if (ret < 0)
> + goto fail_dmx_conn;
> +
> + /*
> + * The source of the demux is a frontend connected
> + * to the demux.
> + */
> + dvb->dmx_fe[j].source = DMX_FRONTEND_0;
> + }
> +
> + return ret;
> +
> +fail_dmx_conn:
> + for (j = j - 1; j >= 0; --j)
> + dvb->demux.dmx.remove_frontend(&dvb->demux.dmx,
> + &dvb->dmx_fe[j]);
> +fail_dmx_dev:
> + dvb_dmxdev_release(&dvb->dmx_dev);
> +fail_dmx:
> + dvb_dmx_release(&dvb->demux);
> +fail_fe:
> + for (k = i; k >= 0; --k)
> + dvb_unregister_frontend(dvb->fe[k]);
> +fail_tuner_probe:
> + for (w = i; w >= 0; --w)
> + dvb_module_release(dvb->i2c_client_tuner[w]);
> +
> +fail_demod_probe:
> + for (l = i; l >= 0; --l)
> + dvb_module_release(dvb->i2c_client_demod[l]);
> +
> +fail_adapter:
> + dvb_unregister_adapter(&dvb->adapter);
> +
> +fail_i2c:
> + i2c_del_adapter(&dvb->i2c_adapter);
> +
> + return ret;
> +}
> +
> +static int vidtv_bridge_probe(struct platform_device *pdev)
> +{
> + int ret;
> + struct vidtv_dvb *dvb;
> +
> + dvb = kzalloc(sizeof(*dvb), GFP_KERNEL);
> + if (!dvb)
> + return -ENOMEM;
> +
> + dvb->pdev = pdev;
> +
> + ret = vidtv_bridge_dvb_init(dvb);
> + if (ret < 0)
> + goto err_dvb;
> +
> + mutex_init(&dvb->feed_lock);
> +
> + platform_set_drvdata(pdev, dvb);
> +
> + return ret;
> +
> +err_dvb:
> + kfree(dvb);
> + return ret;
> +}
> +
> +static int vidtv_bridge_remove(struct platform_device *pdev)
> +{
> + struct vidtv_dvb *dvb;
> + u32 i;
> +
> + dvb = platform_get_drvdata(pdev);
> +
> + mutex_destroy(&dvb->feed_lock);
> +
> + for (i = 0; i < NUM_FE; ++i)

Using i++ is more common though

> + dvb->demux.dmx.remove_frontend(&dvb->demux.dmx,
> + &dvb->dmx_fe[i]);
> +
> + dvb_dmxdev_release(&dvb->dmx_dev);
> + dvb_dmx_release(&dvb->demux);
> +
> + for (i = 0; i < NUM_FE; ++i) {
> + dvb_unregister_frontend(dvb->fe[i]);
> + dvb_frontend_detach(dvb->fe[i]);
> + }
> +

Where do you release tuner and demod? probe is happening
from vidtv_bridge_dvb_init(). Don't you have to do the
reverse here?

> + dvb_unregister_adapter(&dvb->adapter);

Don't you have to do i2c_del_adapter() here?
> +
> + return 0;
> +}
> +
> +static void vidtv_bridge_dev_release(struct device *dev)
> +{
> +}
> +
> +static struct platform_device vidtv_bridge_dev = {
> + .name = "vidtv_bridge",
> + .dev.release = vidtv_bridge_dev_release,
> +};
> +
> +static struct platform_driver vidtv_bridge_driver = {
> + .driver = {
> + .name = "vidtv_bridge",
> + .suppress_bind_attrs = true,
> + },
> + .probe = vidtv_bridge_probe,
> + .remove = vidtv_bridge_remove,
> +};
> +
> +static void __exit vidtv_bridge_exit(void)
> +{
> + platform_driver_unregister(&vidtv_bridge_driver);
> + platform_device_unregister(&vidtv_bridge_dev);
> +}
> +
> +static int __init vidtv_bridge_init(void)
> +{
> + int ret;
> +
> + ret = platform_device_register(&vidtv_bridge_dev);
> + if (ret)
> + return ret;
> +
> + ret = platform_driver_register(&vidtv_bridge_driver);
> + if (ret)
> + platform_device_unregister(&vidtv_bridge_dev);
> +
> + return ret;
> +}
> +
> +module_init(vidtv_bridge_init);
> +module_exit(vidtv_bridge_exit);
> +
> +MODULE_DESCRIPTION("Virtual Digital TV Test Driver");
> +MODULE_AUTHOR("Daniel W. S. Almeida");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/media/test-drivers/vidtv/vidtv_bridge.h b/drivers/media/test-drivers/vidtv/vidtv_bridge.h
> new file mode 100644
> index 0000000000000..4931bfb73273c
> --- /dev/null
> +++ b/drivers/media/test-drivers/vidtv/vidtv_bridge.h
> @@ -0,0 +1,39 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * The Virtual DTV test driver serves as a reference DVB driver and helps
> + * validate the existing APIs in the media subsystem. It can also aid
> + * developers working on userspace applications.
> + *
> + * Written by Daniel W. S. Almeida <[email protected]>
> + */
> +
> +#ifndef VIDTV_BRIDGE_H
> +#define VIDTV_BRIDGE_H
> +
> +#define NUM_FE 1
> +
> +#include <linux/i2c.h>
> +#include <linux/platform_device.h>
> +#include <linux/types.h>
> +#include <media/dmxdev.h>
> +#include <media/dvb_demux.h>
> +#include <media/dvb_frontend.h>
> +
> +struct vidtv_dvb {
> + struct platform_device *pdev;
> + struct dvb_frontend *fe[NUM_FE];
> + struct dvb_adapter adapter;
> + struct dvb_demux demux;
> + struct dmxdev dmx_dev;
> + struct dmx_frontend dmx_fe[NUM_FE];
> + struct i2c_adapter i2c_adapter;
> + struct i2c_client *i2c_client_demod[NUM_FE];
> + struct i2c_client *i2c_client_tuner[NUM_FE];
> +
> + u32 nfeeds;
> + struct mutex feed_lock; /* start/stop feed */
> +
> + bool streaming;
> +};
> +
> +#endif // VIDTV_BRIDG_H
>

thanks,
-- Shuah