PCTV nanoStick T2 290e (Sony CXD2820R DVB-T/T2/C) - DVB-C channel scan in mythtv - missing
Hi, I just tested a PCTV 290e device using the latest media_build drivers in MythTV as a DVB-C device, and ran into some problems. The adapter is recognized by the em28xx-dvb driver [1] and dmesg output seems to be correct [2]. I can successfully scan for channels using the scan utility in dvb-apps but when I try to scan for channels in mythtv I get the following errors logged by mythtv-setup: 2011-06-12 00:57:20.971556 PIDInfo(/dev/dvb/adapter0/ frontend1): Failed to open demux device /dev/dvb/adapter0/demux1 for filter on pid 0x0 The demux1 does not exist, I only have the following nodes under /dev/dvb/adapter0: demux0 dvr0 frontend0 frontend1 net0 When searching the linx-media I came across this thread: http://www.mail-archive.com/linux-media@vger.kernel.org/msg31839.html Is there any way to circumvent with the current driver that there is no corresponding demux1 for frontend1? Or can the DVB-T/T2 part be disabled somehow so that there is only one DVB-C frontend registered which corresponds to the demux0? Best regards, Rune [1] modinfo: modinfo em28xx-dvb filename: /lib/modules/2.6.38-8-generic/kernel/drivers/media/video/em28xx/em28xx-dvb.ko license: GPL author: Mauro Carvalho Chehab description: driver for em28xx based DVB cards srcversion: 663F353AB97767017FDEC27 depends: em28xx,dvb-core,cxd2820r vermagic: 2.6.38-8-generic SMP mod_unload modversions 686 parm: debug:enable debug messages [dvb] (int) parm: adapter_nr:DVB adapter numbers (array of short) [2] dmesg: [ 54.724067] usb 1-3: new high speed USB device using ehci_hcd and address 4 [ 54.941327] WARNING: You are using an experimental version of the media stack. [ 54.941332] As the driver is backported to an older kernel, it doesn't offer [ 54.941335] enough quality for its usage in production. [ 54.941338] Use it with care. [ 54.941339] Latest git patches (needed if you report a bug to linux-media@vger.kernel.org): [ 54.941343] 75125b9d44456e0cf2d1fbb72ae33c 13415299d1 [media] DocBook: Don't be noisy at make cleanmediadocs [ 54.941346] 0fba2f7ff0c4d9f48a5c334826a22db32f816a76 Revert [media] dvb/audio.h: Remove definition for AUDIO_GET_PTS [ 54.941350] 4f75ad768da3c5952d1e7080045a5b5ce7b0d85d [media] DocBook/video.xml: Document the remaining data structures [ 54.961881] IR NEC protocol handler initialized [ 54.987012] IR RC5(x) protocol handler initialized [ 55.008281] IR RC6 protocol handler initialized [ 55.032239] IR JVC protocol handler initialized [ 55.052312] IR Sony protocol handler initialized [ 55.092892] em28xx: New device PCTV Systems PCTV 290e @ 480 Mbps (2013:024f, interface 0, class 0) [ 55.093169] lirc_dev: IR Remote Control driver registered, major 250 [ 55.100171] em28xx #0: chip ID is em28174 [ 55.110254] IR LIRC bridge handler initialized [ 55.419763] em28xx #0: Identified as PCTV nanoStick T2 290e (card=78) [ 55.464076] Registered IR keymap rc-pinnacle-pctv-hd [ 55.472069] input: em28xx IR (em28xx #0) as /devices/pci:00/:00:1d.7/usb1/1-3/rc/rc0/input11 [ 55.472505] rc0: em28xx IR (em28xx #0) as /devices/pci:00/:00:1d.7/usb1/1-3/rc/rc0 [ 55.483777] em28xx #0: v4l2 driver version 0.1.2 [ 55.493073] em28xx #0: V4L2 video device registered as video1 [ 55.494836] usbcore: registered new interface driver em28xx [ 55.494844] em28xx driver loaded [ 55.552425] WARNING: You are using an experimental version of the media stack. [ 55.552430] As the driver is backported to an older kernel, it doesn't offer [ 55.552434] enough quality for its usage in production. [ 55.552436] Use it with care. [ 55.552438] Latest git patches (needed if you report a bug to linux-media@vger.kernel.org): [ 55.552441] 75125b9d44456e0cf2d1fbb72ae33c13415299d1 [media] DocBook: Don't be noisy at make cleanmediadocs [ 55.552445] 0fba2f7ff0c4d9f48a5c334826a22db32f816a76 Revert [media] dvb/audio.h: Remove definition for AUDIO_GET_PTS [ 55.552449] 4f75ad768da3c5952d1e7080045a5b5ce7b0d85d [media] DocBook/video.xml: Document the remaining data structures [ 55.610886] tda18271 15-0060: creating new instance [ 55.613267] TDA18271HD/C2 detected @ 15-0060 [ 55.845831] tda18271 15-0060: attaching existing instance [ 55.845842] DVB: registering new adapter (em28xx #0) [ 55.845850] DVB: registering adapter 0 frontend 0 (Sony CXD2820R (DVB-T/T2))... [ 55.846111] DVB: registering adapter 0 frontend 1 (Sony CXD2820R (DVB-C))... [ 55.846891] em28xx #0: Successfully loaded em28xx-dvb [ 55.846899] Em28xx: Initialized (Em28xx dvb Extension) extension -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
tuner-core: fix g_freq/s_std and g/s_tuner
OK, this is the fourth version of this patch series. The first five patches are the same as before. But for fixing the g_frequency and g/s_tuner subdev ops I've decided to follow Mauro's lead and let the core fill in the tuner type for those ioctls. Trying to do this in the drivers is a tricky business, but doing this in video_ioctl2 is dead easy. The only driver not using video_ioctl2 and that has tuner support as well is pvrusb2, so I did that manually. Mike, can you review that single patch? All it does is fill in v4l2_tuner's type based on whether the radio or TV is active. The last patch fixes a typo in the bttv radio s_tuner implementation making VIDIOC_S_TUNER work again. This patch series has been tested with bttv, cx18, ivtv and pvrusb2. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFCv4 PATCH 1/8] tuner-core: rename check_mode to supported_mode
From: Hans Verkuil
The check_mode function checks whether a mode is supported. So calling it
supported_mode is more appropriate. In addition it returned either 0 or
-EINVAL which suggests that the -EINVAL error should be passed on. However,
that's not the case so change the return type to bool.
Signed-off-by: Hans Verkuil
---
drivers/media/video/tuner-core.c | 19 ---
1 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/drivers/media/video/tuner-core.c b/drivers/media/video/tuner-core.c
index 5748d04..083b9f1 100644
--- a/drivers/media/video/tuner-core.c
+++ b/drivers/media/video/tuner-core.c
@@ -723,22 +723,19 @@ static int tuner_remove(struct i2c_client *client)
*/
/**
- * check_mode - Verify if tuner supports the requested mode
+ * supported_mode - Verify if tuner supports the requested mode
* @t: a pointer to the module's internal struct_tuner
*
* This function checks if the tuner is capable of tuning analog TV,
* digital TV or radio, depending on what the caller wants. If the
- * tuner can't support that mode, it returns -EINVAL. Otherwise, it
- * returns 0.
+ * tuner can't support that mode, it returns false. Otherwise, it
+ * returns true.
* This function is needed for boards that have a separate tuner for
* radio (like devices with tea5767).
*/
-static inline int check_mode(struct tuner *t, enum v4l2_tuner_type mode)
+static bool supported_mode(struct tuner *t, enum v4l2_tuner_type mode)
{
- if ((1 << mode & t->mode_mask) == 0)
- return -EINVAL;
-
- return 0;
+ return 1 << mode & t->mode_mask;
}
/**
@@ -759,7 +756,7 @@ static int set_mode_freq(struct i2c_client *client, struct
tuner *t,
struct analog_demod_ops *analog_ops = &t->fe.ops.analog_ops;
if (mode != t->mode) {
- if (check_mode(t, mode) == -EINVAL) {
+ if (!supported_mode(t, mode)) {
tuner_dbg("Tuner doesn't support mode %d. "
"Putting tuner to sleep\n", mode);
t->standby = true;
@@ -1138,7 +1135,7 @@ static int tuner_g_frequency(struct v4l2_subdev *sd,
struct v4l2_frequency *f)
struct tuner *t = to_tuner(sd);
struct dvb_tuner_ops *fe_tuner_ops = &t->fe.ops.tuner_ops;
- if (check_mode(t, f->type) == -EINVAL)
+ if (!supported_mode(t, f->type))
return 0;
f->type = t->mode;
if (fe_tuner_ops->get_frequency && !t->standby) {
@@ -1161,7 +1158,7 @@ static int tuner_g_tuner(struct v4l2_subdev *sd, struct
v4l2_tuner *vt)
struct analog_demod_ops *analog_ops = &t->fe.ops.analog_ops;
struct dvb_tuner_ops *fe_tuner_ops = &t->fe.ops.tuner_ops;
- if (check_mode(t, vt->type) == -EINVAL)
+ if (!supported_mode(t, vt->type))
return 0;
vt->type = t->mode;
if (analog_ops->get_afc)
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFCv4 PATCH 2/8] tuner-core: change return type of set_mode_freq to bool
From: Hans Verkuil
set_mode_freq currently returns 0 or -EINVAL. But -EINVAL does not
indicate a error that should be passed on, it just indicates that the
tuner does not support the requested mode. So change the return type to
bool.
Signed-off-by: Hans Verkuil
---
drivers/media/video/tuner-core.c | 23 ++-
1 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/drivers/media/video/tuner-core.c b/drivers/media/video/tuner-core.c
index 083b9f1..ee43e0a 100644
--- a/drivers/media/video/tuner-core.c
+++ b/drivers/media/video/tuner-core.c
@@ -746,11 +746,11 @@ static bool supported_mode(struct tuner *t, enum
v4l2_tuner_type mode)
* @freq: frequency to set (0 means to use the previous one)
*
* If tuner doesn't support the needed mode (radio or TV), prints a
- * debug message and returns -EINVAL, changing its state to standby.
- * Otherwise, changes the state and sets frequency to the last value, if
- * the tuner can sleep or if it supports both Radio and TV.
+ * debug message and returns false, changing its state to standby.
+ * Otherwise, changes the state and sets frequency to the last value
+ * and returns true.
*/
-static int set_mode_freq(struct i2c_client *client, struct tuner *t,
+static bool set_mode_freq(struct i2c_client *client, struct tuner *t,
enum v4l2_tuner_type mode, unsigned int freq)
{
struct analog_demod_ops *analog_ops = &t->fe.ops.analog_ops;
@@ -762,7 +762,7 @@ static int set_mode_freq(struct i2c_client *client, struct
tuner *t,
t->standby = true;
if (analog_ops->standby)
analog_ops->standby(&t->fe);
- return -EINVAL;
+ return false;
}
t->mode = mode;
tuner_dbg("Changing to mode %d\n", mode);
@@ -777,7 +777,7 @@ static int set_mode_freq(struct i2c_client *client, struct
tuner *t,
set_tv_freq(client, t->tv_freq);
}
- return 0;
+ return true;
}
/*
@@ -1075,8 +1075,7 @@ static int tuner_s_radio(struct v4l2_subdev *sd)
struct tuner *t = to_tuner(sd);
struct i2c_client *client = v4l2_get_subdevdata(sd);
- if (set_mode_freq(client, t, V4L2_TUNER_RADIO, 0) == -EINVAL)
- return 0;
+ set_mode_freq(client, t, V4L2_TUNER_RADIO, 0);
return 0;
}
@@ -1110,7 +1109,7 @@ static int tuner_s_std(struct v4l2_subdev *sd,
v4l2_std_id std)
struct tuner *t = to_tuner(sd);
struct i2c_client *client = v4l2_get_subdevdata(sd);
- if (set_mode_freq(client, t, V4L2_TUNER_ANALOG_TV, 0) == -EINVAL)
+ if (!set_mode_freq(client, t, V4L2_TUNER_ANALOG_TV, 0))
return 0;
t->std = std;
@@ -1124,9 +1123,7 @@ static int tuner_s_frequency(struct v4l2_subdev *sd,
struct v4l2_frequency *f)
struct tuner *t = to_tuner(sd);
struct i2c_client *client = v4l2_get_subdevdata(sd);
- if (set_mode_freq(client, t, f->type, f->frequency) == -EINVAL)
- return 0;
-
+ set_mode_freq(client, t, f->type, f->frequency);
return 0;
}
@@ -1197,7 +1194,7 @@ static int tuner_s_tuner(struct v4l2_subdev *sd, struct
v4l2_tuner *vt)
struct tuner *t = to_tuner(sd);
struct i2c_client *client = v4l2_get_subdevdata(sd);
- if (set_mode_freq(client, t, vt->type, 0) == -EINVAL)
+ if (!set_mode_freq(client, t, vt->type, 0))
return 0;
if (t->mode == V4L2_TUNER_RADIO)
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFCv4 PATCH 4/8] tuner-core: fix s_std and s_tuner.
From: Hans Verkuil
Both s_std and s_tuner are broken because set_mode_freq is called before the
new std (for s_std) and audmode (for s_tuner) are set.
This patch splits set_mode_freq in a set_mode and a set_freq and in s_std
first calls set_mode, and if that returns true (i.e. the mode is supported)
then they set t->std/t->audmode and call set_freq.
This fixes a bug where changing std or audmode would actually change it to
the previous value.
Discovered while testing analog TV standards for cx18 with a tda18271 tuner.
Signed-off-by: Hans Verkuil
---
drivers/media/video/tuner-core.c | 60 +-
1 files changed, 33 insertions(+), 27 deletions(-)
diff --git a/drivers/media/video/tuner-core.c b/drivers/media/video/tuner-core.c
index 462a8f4..bf7fc33 100644
--- a/drivers/media/video/tuner-core.c
+++ b/drivers/media/video/tuner-core.c
@@ -739,19 +739,15 @@ static bool supported_mode(struct tuner *t, enum
v4l2_tuner_type mode)
}
/**
- * set_mode_freq - Switch tuner to other mode.
- * @client:struct i2c_client pointer
+ * set_mode - Switch tuner to other mode.
* @t: a pointer to the module's internal struct_tuner
* @mode: enum v4l2_type (radio or TV)
- * @freq: frequency to set (0 means to use the previous one)
*
* If tuner doesn't support the needed mode (radio or TV), prints a
* debug message and returns false, changing its state to standby.
- * Otherwise, changes the state and sets frequency to the last value
- * and returns true.
+ * Otherwise, changes the state and returns true.
*/
-static bool set_mode_freq(struct i2c_client *client, struct tuner *t,
-enum v4l2_tuner_type mode, unsigned int freq)
+static bool set_mode(struct tuner *t, enum v4l2_tuner_type mode)
{
struct analog_demod_ops *analog_ops = &t->fe.ops.analog_ops;
@@ -767,17 +763,27 @@ static bool set_mode_freq(struct i2c_client *client,
struct tuner *t,
t->mode = mode;
tuner_dbg("Changing to mode %d\n", mode);
}
+ return true;
+}
+
+/**
+ * set_freq - Set the tuner to the desired frequency.
+ * @t: a pointer to the module's internal struct_tuner
+ * @freq: frequency to set (0 means to use the current frequency)
+ */
+static void set_freq(struct tuner *t, unsigned int freq)
+{
+ struct i2c_client *client = v4l2_get_subdevdata(&t->sd);
+
if (t->mode == V4L2_TUNER_RADIO) {
- if (freq)
- t->radio_freq = freq;
- set_radio_freq(client, t->radio_freq);
+ if (!freq)
+ freq = t->radio_freq;
+ set_radio_freq(client, freq);
} else {
- if (freq)
- t->tv_freq = freq;
- set_tv_freq(client, t->tv_freq);
+ if (!freq)
+ freq = t->tv_freq;
+ set_tv_freq(client, freq);
}
-
- return true;
}
/*
@@ -1034,9 +1040,9 @@ static void tuner_status(struct dvb_frontend *fe)
static int tuner_s_radio(struct v4l2_subdev *sd)
{
struct tuner *t = to_tuner(sd);
- struct i2c_client *client = v4l2_get_subdevdata(sd);
- set_mode_freq(client, t, V4L2_TUNER_RADIO, 0);
+ if (set_mode(t, V4L2_TUNER_RADIO))
+ set_freq(t, 0);
return 0;
}
@@ -1068,24 +1074,23 @@ static int tuner_s_power(struct v4l2_subdev *sd, int on)
static int tuner_s_std(struct v4l2_subdev *sd, v4l2_std_id std)
{
struct tuner *t = to_tuner(sd);
- struct i2c_client *client = v4l2_get_subdevdata(sd);
- if (!set_mode_freq(client, t, V4L2_TUNER_ANALOG_TV, 0))
+ if (!set_mode(t, V4L2_TUNER_ANALOG_TV))
return 0;
t->std = tuner_fixup_std(t, std);
if (t->std != std)
tuner_dbg("Fixup standard %llx to %llx\n", std, t->std);
-
+ set_freq(t, 0);
return 0;
}
static int tuner_s_frequency(struct v4l2_subdev *sd, struct v4l2_frequency *f)
{
struct tuner *t = to_tuner(sd);
- struct i2c_client *client = v4l2_get_subdevdata(sd);
- set_mode_freq(client, t, f->type, f->frequency);
+ if (set_mode(t, f->type))
+ set_freq(t, f->frequency);
return 0;
}
@@ -1154,13 +1159,14 @@ static int tuner_g_tuner(struct v4l2_subdev *sd, struct
v4l2_tuner *vt)
static int tuner_s_tuner(struct v4l2_subdev *sd, struct v4l2_tuner *vt)
{
struct tuner *t = to_tuner(sd);
- struct i2c_client *client = v4l2_get_subdevdata(sd);
- if (!set_mode_freq(client, t, vt->type, 0))
+ if (!set_mode(t, vt->type))
return 0;
- if (t->mode == V4L2_TUNER_RADIO)
+ if (t->mode == V4L2_TUNER_RADIO) {
t->audmode = vt->audmode;
+ set_freq(t, 0);
+ }
return 0;
}
@@ -1195,8 +1201,8 @@ static int tuner_resume(struct i2c_client *c)
tuner_dbg("resume\n");
if (
[RFCv4 PATCH 3/8] tuner-core: simplify the standard fixup.
From: Hans Verkuil
Get rid of a number of unnecessary tuner_dbg messages by simplifying
the std fixup function.
Signed-off-by: Hans Verkuil
---
drivers/media/video/tuner-core.c | 92 +++---
1 files changed, 27 insertions(+), 65 deletions(-)
diff --git a/drivers/media/video/tuner-core.c b/drivers/media/video/tuner-core.c
index ee43e0a..462a8f4 100644
--- a/drivers/media/video/tuner-core.c
+++ b/drivers/media/video/tuner-core.c
@@ -832,7 +832,7 @@ static void set_tv_freq(struct i2c_client *c, unsigned int
freq)
/**
* tuner_fixup_std - force a given video standard variant
*
- * @t: tuner internal struct
+ * @std: TV standard
*
* A few devices or drivers have problem to detect some standard variations.
* On other operational systems, the drivers generally have a per-country
@@ -842,57 +842,39 @@ static void set_tv_freq(struct i2c_client *c, unsigned
int freq)
* to distinguish all video standard variations, a modprobe parameter can
* be used to force a video standard match.
*/
-static int tuner_fixup_std(struct tuner *t)
+static v4l2_std_id tuner_fixup_std(struct tuner *t, v4l2_std_id std)
{
- if ((t->std & V4L2_STD_PAL) == V4L2_STD_PAL) {
+ if (pal[0] != '-' && (std & V4L2_STD_PAL) == V4L2_STD_PAL) {
switch (pal[0]) {
case '6':
- tuner_dbg("insmod fixup: PAL => PAL-60\n");
- t->std = V4L2_STD_PAL_60;
- break;
+ return V4L2_STD_PAL_60;
case 'b':
case 'B':
case 'g':
case 'G':
- tuner_dbg("insmod fixup: PAL => PAL-BG\n");
- t->std = V4L2_STD_PAL_BG;
- break;
+ return V4L2_STD_PAL_BG;
case 'i':
case 'I':
- tuner_dbg("insmod fixup: PAL => PAL-I\n");
- t->std = V4L2_STD_PAL_I;
- break;
+ return V4L2_STD_PAL_I;
case 'd':
case 'D':
case 'k':
case 'K':
- tuner_dbg("insmod fixup: PAL => PAL-DK\n");
- t->std = V4L2_STD_PAL_DK;
- break;
+ return V4L2_STD_PAL_DK;
case 'M':
case 'm':
- tuner_dbg("insmod fixup: PAL => PAL-M\n");
- t->std = V4L2_STD_PAL_M;
- break;
+ return V4L2_STD_PAL_M;
case 'N':
case 'n':
- if (pal[1] == 'c' || pal[1] == 'C') {
- tuner_dbg("insmod fixup: PAL => PAL-Nc\n");
- t->std = V4L2_STD_PAL_Nc;
- } else {
- tuner_dbg("insmod fixup: PAL => PAL-N\n");
- t->std = V4L2_STD_PAL_N;
- }
- break;
- case '-':
- /* default parameter, do nothing */
- break;
+ if (pal[1] == 'c' || pal[1] == 'C')
+ return V4L2_STD_PAL_Nc;
+ return V4L2_STD_PAL_N;
default:
tuner_warn("pal= argument not recognised\n");
break;
}
}
- if ((t->std & V4L2_STD_SECAM) == V4L2_STD_SECAM) {
+ if (secam[0] != '-' && (std & V4L2_STD_SECAM) == V4L2_STD_SECAM) {
switch (secam[0]) {
case 'b':
case 'B':
@@ -900,63 +882,42 @@ static int tuner_fixup_std(struct tuner *t)
case 'G':
case 'h':
case 'H':
- tuner_dbg("insmod fixup: SECAM => SECAM-BGH\n");
- t->std = V4L2_STD_SECAM_B |
-V4L2_STD_SECAM_G |
-V4L2_STD_SECAM_H;
- break;
+ return V4L2_STD_SECAM_B |
+ V4L2_STD_SECAM_G |
+ V4L2_STD_SECAM_H;
case 'd':
case 'D':
case 'k':
case 'K':
- tuner_dbg("insmod fixup: SECAM => SECAM-DK\n");
- t->std = V4L2_STD_SECAM_DK;
- break;
+ return V4L2_STD_SECAM_DK;
case 'l':
case 'L':
- if ((secam[1] == 'C') || (secam[1] == 'c')) {
- tuner_dbg("insmod fixup: SECAM => SECAM-L'\n");
- t->std = V4L2_STD_SECAM_LC;
- } else {
- tuner_dbg("insmod fixup: SECAM => SECAM-
[RFCv4 PATCH 5/8] tuner-core: fix tuner_resume: use t->mode instead of t->type.
From: Hans Verkuil
set_mode is called with t->type, which is the tuner type. Instead, use
t->mode which is the actual tuner mode (i.e. radio vs tv).
Signed-off-by: Hans Verkuil
---
drivers/media/video/tuner-core.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/media/video/tuner-core.c b/drivers/media/video/tuner-core.c
index bf7fc33..ffe5de3 100644
--- a/drivers/media/video/tuner-core.c
+++ b/drivers/media/video/tuner-core.c
@@ -1201,7 +1201,7 @@ static int tuner_resume(struct i2c_client *c)
tuner_dbg("resume\n");
if (!t->standby)
- if (set_mode(t, t->type))
+ if (set_mode(t, t->mode))
set_freq(t, 0);
return 0;
}
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFCv4 PATCH 6/8] v4l2-ioctl.c: prefill tuner type for g_frequency and g/s_tuner.
From: Hans Verkuil The subdevs are supposed to receive a valid tuner type for the g_frequency and g/s_tuner subdev ops. Some drivers do this, others don't. So prefill this in v4l2-ioctl.c based on whether the device node from which this is called is a radio node or not. The spec does not require applications to fill in the type, and if they leave it at 0 then the 'supported_mode' call in tuner-core.c will return false and the ioctl does nothing. Signed-off-by: Hans Verkuil --- drivers/media/video/v4l2-ioctl.c |6 ++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c index 213ba7d..26bf3bf 100644 --- a/drivers/media/video/v4l2-ioctl.c +++ b/drivers/media/video/v4l2-ioctl.c @@ -1822,6 +1822,8 @@ static long __video_do_ioctl(struct file *file, if (!ops->vidioc_g_tuner) break; + p->type = (vfd->vfl_type == VFL_TYPE_RADIO) ? + V4L2_TUNER_RADIO : V4L2_TUNER_ANALOG_TV; ret = ops->vidioc_g_tuner(file, fh, p); if (!ret) dbgarg(cmd, "index=%d, name=%s, type=%d, " @@ -1840,6 +1842,8 @@ static long __video_do_ioctl(struct file *file, if (!ops->vidioc_s_tuner) break; + p->type = (vfd->vfl_type == VFL_TYPE_RADIO) ? + V4L2_TUNER_RADIO : V4L2_TUNER_ANALOG_TV; dbgarg(cmd, "index=%d, name=%s, type=%d, " "capability=0x%x, rangelow=%d, " "rangehigh=%d, signal=%d, afc=%d, " @@ -1858,6 +1862,8 @@ static long __video_do_ioctl(struct file *file, if (!ops->vidioc_g_frequency) break; + p->type = (vfd->vfl_type == VFL_TYPE_RADIO) ? + V4L2_TUNER_RADIO : V4L2_TUNER_ANALOG_TV; ret = ops->vidioc_g_frequency(file, fh, p); if (!ret) dbgarg(cmd, "tuner=%d, type=%d, frequency=%d\n", -- 1.7.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFCv4 PATCH 8/8] bttv: fix s_tuner for radio.
From: Hans Verkuil Fix typo: g_tuner should have been s_tuner. Tested with a bttv card. Signed-off-by: Hans Verkuil --- drivers/media/video/bt8xx/bttv-driver.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/media/video/bt8xx/bttv-driver.c b/drivers/media/video/bt8xx/bttv-driver.c index a97cf27..834a483 100644 --- a/drivers/media/video/bt8xx/bttv-driver.c +++ b/drivers/media/video/bt8xx/bttv-driver.c @@ -3474,7 +3474,7 @@ static int radio_s_tuner(struct file *file, void *priv, if (0 != t->index) return -EINVAL; - bttv_call_all(btv, tuner, g_tuner, t); + bttv_call_all(btv, tuner, s_tuner, t); return 0; } -- 1.7.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFCv4 PATCH 7/8] pvrusb2: fix g/s_tuner support.
From: Hans Verkuil
The tuner-core subdev requires that the type field of v4l2_tuner is
filled in correctly. This is done in v4l2-ioctl.c, but pvrusb2 doesn't
use that yet, so we have to do it manually based on whether the current
input is radio or not.
Tested with my pvrusb2.
Signed-off-by: Hans Verkuil
---
drivers/media/video/pvrusb2/pvrusb2-hdw.c |4
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/drivers/media/video/pvrusb2/pvrusb2-hdw.c
b/drivers/media/video/pvrusb2/pvrusb2-hdw.c
index 9d0dd08..e98d382 100644
--- a/drivers/media/video/pvrusb2/pvrusb2-hdw.c
+++ b/drivers/media/video/pvrusb2/pvrusb2-hdw.c
@@ -3046,6 +3046,8 @@ static void pvr2_subdev_update(struct pvr2_hdw *hdw)
if (hdw->input_dirty || hdw->audiomode_dirty || hdw->force_dirty) {
struct v4l2_tuner vt;
memset(&vt, 0, sizeof(vt));
+ vt.type = (hdw->input_val == PVR2_CVAL_INPUT_RADIO) ?
+ V4L2_TUNER_RADIO : V4L2_TUNER_ANALOG_TV;
vt.audmode = hdw->audiomode_val;
v4l2_device_call_all(&hdw->v4l2_dev, 0, tuner, s_tuner, &vt);
}
@@ -5171,6 +5173,8 @@ void pvr2_hdw_status_poll(struct pvr2_hdw *hdw)
{
struct v4l2_tuner *vtp = &hdw->tuner_signal_info;
memset(vtp, 0, sizeof(*vtp));
+ vtp->type = (hdw->input_val == PVR2_CVAL_INPUT_RADIO) ?
+ V4L2_TUNER_RADIO : V4L2_TUNER_ANALOG_TV;
hdw->tuner_signal_stale = 0;
/* Note: There apparently is no replacement for VIDIOC_CROPCAP
using v4l2-subdev - therefore we can't support that AT ALL right
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: PCTV nanoStick T2 290e (Sony CXD2820R DVB-T/T2/C) - DVB-C channel scan in mythtv - missing
On 06/12/2011 11:23 AM, Rune Evjen wrote: I just tested a PCTV 290e device using the latest media_build drivers in MythTV as a DVB-C device, and ran into some problems. The adapter is recognized by the em28xx-dvb driver [1] and dmesg output seems to be correct [2]. I can successfully scan for channels using the scan utility in dvb-apps but when I try to scan for channels in mythtv I get the following errors logged by mythtv-setup: 2011-06-12 00:57:20.971556 PIDInfo(/dev/dvb/adapter0/ frontend1): Failed to open demux device /dev/dvb/adapter0/demux1 for filter on pid 0x0 The demux1 does not exist, I only have the following nodes under /dev/dvb/adapter0: demux0 dvr0 frontend0 frontend1 net0 When searching the linx-media I came across this thread: http://www.mail-archive.com/linux-media@vger.kernel.org/msg31839.html Is there any way to circumvent with the current driver that there is no corresponding demux1 for frontend1? Or can the DVB-T/T2 part be disabled somehow so that there is only one DVB-C frontend registered which corresponds to the demux0? There is no way to say driver to create demux1 for frontend1. After all it is not 100% clear even for me if that's correct or not, but most likely it is correct as far as I understood. regards, Antti -- http://palosaari.fi/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: dual sveon stv22 Afatech af9015 support (kworld clone)
Could you send lsusb -vvd output from that device?
Also remote controller keytable is needed. You can enable remote debug
by modprobe dvb_usb_af9015 debug=2. After that it should output remote
codes to the message.log.
regards,
Antti
On 06/11/2011 12:38 PM, Lopez Lopez wrote:
Hello:
I have patched af9015.c and dvb-usb-ids to support sveon stv22 ( KWorld USB
Dual DVB-T TV Stick (DVB-T 399U) clone ) dual with
-
#define USB_PID_SVEON_STV220xe401
--
in dvb-usb-ids.h file
and
-
/* 30 */{USB_DEVICE(USB_VID_KWORLD_2, USB_PID_KWORLD_UB383_T)},
{USB_DEVICE(USB_VID_KWORLD_2,
USB_PID_KWORLD_395U_4)},
{USB_DEVICE(USB_VID_KWORLD_2, USB_PID_SVEON_STV22)},
{0},
};
--
{
.name = "Sveon STV22 Dual USB DVB-T Tuner HDTV ",
.cold_ids = {&af9015_usb_table[32], NULL},
.warm_ids = {NULL},
},
-
in af9015.c
i expect to help you to extends linux dvb usb support.
thanks for your time
Emilio David Diaus Lopez
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
http://palosaari.fi/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv1 PATCH 7/7] tuner-core: s_tuner should not change tuner mode.
On Saturday, June 11, 2011 21:04:57 Mauro Carvalho Chehab wrote: > Em 11-06-2011 14:27, Hans Verkuil escreveu: > > On Saturday, June 11, 2011 15:54:59 Mauro Carvalho Chehab wrote: > >> Em 11-06-2011 10:34, Hans Verkuil escreveu: > >>> From: Hans Verkuil > >>> > >>> According to the spec the tuner type field is not used when calling > >>> S_TUNER: index, audmode and reserved are the only writable fields. > >>> > >>> So remove the type check. Instead, just set the audmode if the current > >>> tuner mode is set to radio. > >> > >> I suspect that this patch also breaks support for a separate radio tuner. > >> if tuner-type is not properly filled, then the easiest fix would be to > >> revert some changes done at the tuner cleanup/fixup patches applied on .39. > >> Yet, the previous logic were trying to hint the device mode, with is bad > >> (that's why it was changed). > >> > >> The proper change seems to add a parameter to this callback, set by the > >> bridge driver, informing if the device is using radio or video mode. > >> We need also to patch the V4L API specs, as it allows using a video node > >> for radio, and vice versa. This is not well supported, and it seems silly > >> to keep it at the specs and needing to add hints at the drivers due to > >> that. > > > > So, just to make sure I understand correctly what you want. The bridge or > > platform drivers will fill in the vf->type (for g/s_frequency) or vt->type > > (for g/s_tuner) based on the device node: RADIO if /dev/radio is used, > > TV for anything else. > > Yes. I remember I've reviewed the bridge drivers when I rewrote the tuner > code. > Of course, I might have left something else. Btw, the older code were also > requiring it. > > The cx18 implementation were merged after the changes, so maybe it is not > doing > the right thing. Actually, G_TUNER was wrong for bttv, ivtv, cx18 and pvrusb2. So it wasn't just some random driver that failed to set vf/vt->type. > > > > > What about VIDIOC_S_FREQUENCY? The spec says that the app needs to fill this > > in. Will we just overwrite vf->type or will we check and return -EINVAL if > > the app tries to set e.g. a TV frequency on /dev/radio? > > That's a very good question. What happens is that the V4L2 API used to allow > opening a /dev/radio device for TV (or even for VBI). IMHO, this were a > trouble > at the API specs. I think that this were changed on newer versions of the > spec. If you want, then I can add an extra patch to my v4 patch series that returns -EINVAL in video_ioctl2 if S_FREQUENCY is called with an inconsistent tuner type. (inconsistent with the device node's type, that is). > > Is VIDIOC_S_FREQUENCY allowed to change the tuner mode? E.g. if /dev/radio > > was > > opened, and now I open /dev/video and call S_FREQUENCY with the TV tuner > > type, > > should that change the tuner to tv mode? > > Yes. I think that some applications like kradio just keeps the device node > opened. > If we return -EBUSY, those applications will break. The reverse is more > tricky: > e. g. if /dev/video is streaming, I think that the bridge driver should return > -EBUSY if the device can't do both TV and radio at the same time, but this is > something that it is device-specific, so such logic, if needed, should be > implemented > at the bridge driver. Agreed. > > I think the type passed to S_FREQUENCY should 1) match the device node's > > type > > (if not, then return -EINVAL) and 2) should match the current mode (if not, > > then return -EBUSY). So attempts to change the TV frequency when in radio > > mode should fail. This second rule should also be valid for S_TUNER. > > See above. > > > What should G_TUNER return on a video node when in radio mode or vice versa? > > For G_FREQUENCY you can still return the last used frequency, but that's > > much more ambiguous for G_TUNER. One option is to set rxsubchans, signal and > > afc all to 0 if you query G_TUNER when 'in the wrong mode'. > > The current logic should handle this case well. I tested it carefully. > Basically, > if the device is on Radio mode, and has a separate tuner for TV, the TV tuner > should not touch the structure. The Radio tuner should properly fill the > values. I analyzed it again and you are correct. > Calls to G_TUNER/G_FREQUENCY shouldn't switch the device mode, or they may > break > applications like kradio, that may be always there during the entire KDE > section. Obviously. > > The VIDIOC_G/S_MODULATOR ioctls do not have a type and they are RADIO only, > > so that's OK. > > > > And how do we switch between radio and TV? Right now opening the radio node > > will set the tuner in radio mode, and calling S_STD will change the mode to > > TV again. As mentioned above, what S_FREQUENCY is supposed to do is > > undefined > > at the moment. > > If S_FREQUENCY is called from /dev/video (or /dev/vbi), it should set it to > TV. If > it is called from /dev/radio, it should put the device on radio mode
Re: Improving kernel -> userspace (usbfs) usb device hand off
Hi, On 06/11/2011 06:19 PM, Theodore Kilgore wrote: On Sat, 11 Jun 2011, Hans de Goede wrote: Hi, Given the many comments in this thread, I'm just going reply to this one, and try to also answer any other ones in this mail. As far as the dual mode camera is involved, I agree that that should be fixed in the existing v4l2 drivers + libgphoto. I think that Felipe's solution to also handle the stillcam part in kernel space for dual mode cameras (and add a libgphoto cam driver which knows how to talk the new kernel API for this), is the best solution. Unfortunately this will involve quite a bit of work, but so be it. Hans, It appears to me that the solution ought to be at hand, actually. I was not aware of the recent changes in libusb, which I understand are supposed to allow a kernel driver to be hooked up again. To review the situation: 1. As of approximately 2 years ago, libusb already was so configured as to suspend the kernel module for a dual-mode device if a userspace-based program tried to claim the device. 2. At this point with the more recent versions of libusb (see the last message from yesterday, from Xiaofan Chen), we are supposed to be able to re-activate the kernel module for the device when it is relinquished by userspace. This ought to take care of the problems completely, provided that the new capabilities of libusb are actually used and called upon in libgphoto2. I have checked on what is happening, just now, on my own machine. I have libusb version 1.08 which ought to be recent enough. The advertised abilities did not work, however. Presumably, what is missing is on the other end of the problem, most likely in the functions in libgphoto2 which hook up a camera. That code would presumably need to call upon the new functionality of libusb. My currently installed version of libgphoto2 (from svn, but several months old) clearly does not contain the needed functionality. But it might have been put in recently and I did not notice. I guess that the first thing to do is to update my gphoto tree and then to see what happens. If things still don't work, then something needs to be updated and then things ought to work. I will try to see that something gets done about this. Thank you for raising the old issue of dual-mode devices yet again, and thanks to Xiaofan Chen for pointing out that the needed missing half of the functionality is supposed to exist now in libusb. That had escaped my attention. Actually libusb and libgphoto have been using the rebind orginal driver functionality of the code for quite a while now, unfortunately this does not solve the problem, unless we somehow move to 1 central coordinator for the device the user experience will stay subpar. Example, user downloads pictures from the camera using shotwell, gthumb, fspot or whatever, keeps the app in question open and the app in question keeps the gphoto2 device handle open. User wants to do some skyping with video chat, skype complains it cannot find the device, since the kernel driver currently is unbound. -> Poor user experience. With having both functions in the kernel, the kernel could actually allow skype to use the dual mode cameras as video source, and if the user then were to switch to f-spot and try to import more photo's then he will get an -ebusy in f-spot. If he finishes skyping and then returns to f-spot everything will just continue working. This is the kind of "seamless" user experience I'm aiming for here. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv1 PATCH 7/7] tuner-core: s_tuner should not change tuner mode.
Em 12-06-2011 08:36, Hans Verkuil escreveu: > On Saturday, June 11, 2011 21:04:57 Mauro Carvalho Chehab wrote: >> Em 11-06-2011 14:27, Hans Verkuil escreveu: >>> On Saturday, June 11, 2011 15:54:59 Mauro Carvalho Chehab wrote: Em 11-06-2011 10:34, Hans Verkuil escreveu: > From: Hans Verkuil > > According to the spec the tuner type field is not used when calling > S_TUNER: index, audmode and reserved are the only writable fields. > > So remove the type check. Instead, just set the audmode if the current > tuner mode is set to radio. I suspect that this patch also breaks support for a separate radio tuner. if tuner-type is not properly filled, then the easiest fix would be to revert some changes done at the tuner cleanup/fixup patches applied on .39. Yet, the previous logic were trying to hint the device mode, with is bad (that's why it was changed). The proper change seems to add a parameter to this callback, set by the bridge driver, informing if the device is using radio or video mode. We need also to patch the V4L API specs, as it allows using a video node for radio, and vice versa. This is not well supported, and it seems silly to keep it at the specs and needing to add hints at the drivers due to that. >>> >>> So, just to make sure I understand correctly what you want. The bridge or >>> platform drivers will fill in the vf->type (for g/s_frequency) or vt->type >>> (for g/s_tuner) based on the device node: RADIO if /dev/radio is used, >>> TV for anything else. >> >> Yes. I remember I've reviewed the bridge drivers when I rewrote the tuner >> code. >> Of course, I might have left something else. Btw, the older code were also >> requiring it. >> >> The cx18 implementation were merged after the changes, so maybe it is not >> doing >> the right thing. > > Actually, G_TUNER was wrong for bttv, ivtv, cx18 and pvrusb2. So it wasn't > just some random driver that failed to set vf/vt->type. G_FREQUENCY were broken just on cx18. As I said before, filling the type were always required. Anyway, I've added a proper documentation for it. See the patch bellow. The same patch should be done also for G_TUNER. >>> What about VIDIOC_S_FREQUENCY? The spec says that the app needs to fill this >>> in. Will we just overwrite vf->type or will we check and return -EINVAL if >>> the app tries to set e.g. a TV frequency on /dev/radio? >> >> That's a very good question. What happens is that the V4L2 API used to allow >> opening a /dev/radio device for TV (or even for VBI). IMHO, this were a >> trouble >> at the API specs. I think that this were changed on newer versions of the >> spec. > > If you want, then I can add an extra patch to my v4 patch series that returns > -EINVAL in video_ioctl2 if S_FREQUENCY is called with an inconsistent tuner > type. > (inconsistent with the device node's type, that is). The reason why check_mode returns -EINVAL is that this error code may need to be returned to the caller. You should note, however, that the bridge code may need to be fixed if you return the check_mode error code, as otherwise the error will simply be discarded or it it will break devices with two tuners. For example, currently, bttv driver uses v4l2_device_call_all() for it. So, any errors returned by it will be simply discarded. If some bridge driver is using v4l2_device_call_until_err(), it will stop on the first tuner that gets an error. The solution is to implement a v4l2_device_call_until_not_err() and use it for the tuner commands. >>> Is VIDIOC_S_FREQUENCY allowed to change the tuner mode? E.g. if /dev/radio >>> was >>> opened, and now I open /dev/video and call S_FREQUENCY with the TV tuner >>> type, >>> should that change the tuner to tv mode? >> >> Yes. I think that some applications like kradio just keeps the device node >> opened. >> If we return -EBUSY, those applications will break. The reverse is more >> tricky: >> e. g. if /dev/video is streaming, I think that the bridge driver should >> return >> -EBUSY if the device can't do both TV and radio at the same time, but this is >> something that it is device-specific, so such logic, if needed, should be >> implemented >> at the bridge driver. > > Agreed. > >>> I think the type passed to S_FREQUENCY should 1) match the device node's >>> type >>> (if not, then return -EINVAL) and 2) should match the current mode (if not, >>> then return -EBUSY). So attempts to change the TV frequency when in radio >>> mode should fail. This second rule should also be valid for S_TUNER. >> >> See above. >> >>> What should G_TUNER return on a video node when in radio mode or vice versa? >>> For G_FREQUENCY you can still return the last used frequency, but that's >>> much more ambiguous for G_TUNER. One option is to set rxsubchans, signal and >>> afc all to 0 if you query G_TUNER when 'in the wrong mode'. >> >> The current logic should handle this case well. I test
Re: [RFCv1 PATCH 7/7] tuner-core: s_tuner should not change tuner mode.
Em 12-06-2011 08:59, Mauro Carvalho Chehab escreveu: > Em 12-06-2011 08:36, Hans Verkuil escreveu: >> On Saturday, June 11, 2011 21:04:57 Mauro Carvalho Chehab wrote: >>> Em 11-06-2011 14:27, Hans Verkuil escreveu: On Saturday, June 11, 2011 15:54:59 Mauro Carvalho Chehab wrote: > Em 11-06-2011 10:34, Hans Verkuil escreveu: >> From: Hans Verkuil >> >> According to the spec the tuner type field is not used when calling >> S_TUNER: index, audmode and reserved are the only writable fields. >> >> So remove the type check. Instead, just set the audmode if the current >> tuner mode is set to radio. > > I suspect that this patch also breaks support for a separate radio tuner. > if tuner-type is not properly filled, then the easiest fix would be to > revert some changes done at the tuner cleanup/fixup patches applied on > .39. > Yet, the previous logic were trying to hint the device mode, with is bad > (that's why it was changed). > > The proper change seems to add a parameter to this callback, set by the > bridge driver, informing if the device is using radio or video mode. > We need also to patch the V4L API specs, as it allows using a video node > for radio, and vice versa. This is not well supported, and it seems silly > to keep it at the specs and needing to add hints at the drivers due to > that. So, just to make sure I understand correctly what you want. The bridge or platform drivers will fill in the vf->type (for g/s_frequency) or vt->type (for g/s_tuner) based on the device node: RADIO if /dev/radio is used, TV for anything else. >>> >>> Yes. I remember I've reviewed the bridge drivers when I rewrote the tuner >>> code. >>> Of course, I might have left something else. Btw, the older code were also >>> requiring it. >>> >>> The cx18 implementation were merged after the changes, so maybe it is not >>> doing >>> the right thing. >> >> Actually, G_TUNER was wrong for bttv, ivtv, cx18 and pvrusb2. So it wasn't >> just some random driver that failed to set vf/vt->type. > > G_FREQUENCY were broken just on cx18. As I said before, filling the type were > always required. Anyway, I've added a proper documentation for it. See the > patch bellow. > > The same patch should be done also for G_TUNER. > What about VIDIOC_S_FREQUENCY? The spec says that the app needs to fill this in. Will we just overwrite vf->type or will we check and return -EINVAL if the app tries to set e.g. a TV frequency on /dev/radio? >>> >>> That's a very good question. What happens is that the V4L2 API used to allow >>> opening a /dev/radio device for TV (or even for VBI). IMHO, this were a >>> trouble >>> at the API specs. I think that this were changed on newer versions of the >>> spec. >> >> If you want, then I can add an extra patch to my v4 patch series that returns >> -EINVAL in video_ioctl2 if S_FREQUENCY is called with an inconsistent tuner >> type. >> (inconsistent with the device node's type, that is). > > The reason why check_mode returns -EINVAL is that this error code may need to > be > returned to the caller. You should note, however, that the bridge code may > need > to be fixed if you return the check_mode error code, as otherwise the error > will > simply be discarded or it it will break devices with two tuners. > > For example, currently, bttv driver uses v4l2_device_call_all() for it. > So, any errors returned by it will be simply discarded. > > If some bridge driver is using v4l2_device_call_until_err(), it will stop on > the first > tuner that gets an error. > > The solution is to implement a v4l2_device_call_until_not_err() and use it > for the > tuner commands. > Is VIDIOC_S_FREQUENCY allowed to change the tuner mode? E.g. if /dev/radio was opened, and now I open /dev/video and call S_FREQUENCY with the TV tuner type, should that change the tuner to tv mode? >>> >>> Yes. I think that some applications like kradio just keeps the device node >>> opened. >>> If we return -EBUSY, those applications will break. The reverse is more >>> tricky: >>> e. g. if /dev/video is streaming, I think that the bridge driver should >>> return >>> -EBUSY if the device can't do both TV and radio at the same time, but this >>> is >>> something that it is device-specific, so such logic, if needed, should be >>> implemented >>> at the bridge driver. >> >> Agreed. >> I think the type passed to S_FREQUENCY should 1) match the device node's type (if not, then return -EINVAL) and 2) should match the current mode (if not, then return -EBUSY). So attempts to change the TV frequency when in radio mode should fail. This second rule should also be valid for S_TUNER. >>> >>> See above. >>> What should G_TUNER return on a video node when in radio mode or vice versa? For G_FREQUENCY you can still return the last used frequency,
Re: [RFCv1 PATCH 7/7] tuner-core: s_tuner should not change tuner mode.
On Sunday, June 12, 2011 13:59:59 Mauro Carvalho Chehab wrote: > Em 12-06-2011 08:36, Hans Verkuil escreveu: > > On Saturday, June 11, 2011 21:04:57 Mauro Carvalho Chehab wrote: > >> Em 11-06-2011 14:27, Hans Verkuil escreveu: > >>> On Saturday, June 11, 2011 15:54:59 Mauro Carvalho Chehab wrote: > Em 11-06-2011 10:34, Hans Verkuil escreveu: > > From: Hans Verkuil > > > > According to the spec the tuner type field is not used when calling > > S_TUNER: index, audmode and reserved are the only writable fields. > > > > So remove the type check. Instead, just set the audmode if the current > > tuner mode is set to radio. > > I suspect that this patch also breaks support for a separate radio tuner. > if tuner-type is not properly filled, then the easiest fix would be to > revert some changes done at the tuner cleanup/fixup patches applied on > .39. > Yet, the previous logic were trying to hint the device mode, with is bad > (that's why it was changed). > > The proper change seems to add a parameter to this callback, set by the > bridge driver, informing if the device is using radio or video mode. > We need also to patch the V4L API specs, as it allows using a video node > for radio, and vice versa. This is not well supported, and it seems silly > to keep it at the specs and needing to add hints at the drivers due to > that. > >>> > >>> So, just to make sure I understand correctly what you want. The bridge or > >>> platform drivers will fill in the vf->type (for g/s_frequency) or vt->type > >>> (for g/s_tuner) based on the device node: RADIO if /dev/radio is used, > >>> TV for anything else. > >> > >> Yes. I remember I've reviewed the bridge drivers when I rewrote the tuner > >> code. > >> Of course, I might have left something else. Btw, the older code were also > >> requiring it. > >> > >> The cx18 implementation were merged after the changes, so maybe it is not > >> doing > >> the right thing. > > > > Actually, G_TUNER was wrong for bttv, ivtv, cx18 and pvrusb2. So it wasn't > > just some random driver that failed to set vf/vt->type. > > G_FREQUENCY were broken just on cx18. As I said before, filling the type were > always required. No, it wasn't required in the past. I'm happy with that change, but that should be documented in v4l2-subdev.h since that's where the subdev ops are documented. I will add such documentation in a RFCv5 patch series. > Anyway, I've added a proper documentation for it. See the > patch bellow. > > The same patch should be done also for G_TUNER. And s_tuner. > > >>> What about VIDIOC_S_FREQUENCY? The spec says that the app needs to fill > >>> this > >>> in. Will we just overwrite vf->type or will we check and return -EINVAL if > >>> the app tries to set e.g. a TV frequency on /dev/radio? > >> > >> That's a very good question. What happens is that the V4L2 API used to > >> allow > >> opening a /dev/radio device for TV (or even for VBI). IMHO, this were a > >> trouble > >> at the API specs. I think that this were changed on newer versions of the > >> spec. > > > > If you want, then I can add an extra patch to my v4 patch series that > > returns > > -EINVAL in video_ioctl2 if S_FREQUENCY is called with an inconsistent tuner > > type. > > (inconsistent with the device node's type, that is). > > The reason why check_mode returns -EINVAL is that this error code may need to > be > returned to the caller. You should note, however, that the bridge code may > need > to be fixed if you return the check_mode error code, as otherwise the error > will > simply be discarded or it it will break devices with two tuners. > > For example, currently, bttv driver uses v4l2_device_call_all() for it. > So, any errors returned by it will be simply discarded. > > If some bridge driver is using v4l2_device_call_until_err(), it will stop on > the first > tuner that gets an error. > > The solution is to implement a v4l2_device_call_until_not_err() and use it > for the > tuner commands. That's not unreasonably to do at some point in time, but it doesn't actually answer my question, which is: should the core refuse VIDIOC_S_FREQUENCY calls where the type doesn't match the device node (i.e. radio vs tv)? I think it makes no sense to call VIDIOC_S_FREQUENCY on a radio node with type ANALOG_TV. ... > >>> What about this: > >>> > >>> Opening /dev/radio effectively starts the radio mode. So if there is TV > >>> capture in progress, then the open should return -EBUSY. Otherwise it > >>> switches the tuner to radio mode. And it stays in radio mode until the > >>> last filehandle of /dev/radio is closed. At that point it will > >>> automatically > >>> switch back to TV mode (if there is one, of course). > >> > >> No. This would break existing applications. The mode switch should be done > >> at S_FREQUENCY (e. g. when the radio application is tuning into a channel). > > > > This is not w
Re: [RFCv1 PATCH 7/7] tuner-core: s_tuner should not change tuner mode.
On Sunday, June 12, 2011 14:13:30 Mauro Carvalho Chehab wrote: > Em 12-06-2011 08:59, Mauro Carvalho Chehab escreveu: > > Em 12-06-2011 08:36, Hans Verkuil escreveu: > What about this: > > Opening /dev/radio effectively starts the radio mode. So if there is TV > capture in progress, then the open should return -EBUSY. Otherwise it > switches the tuner to radio mode. And it stays in radio mode until the > last filehandle of /dev/radio is closed. At that point it will > automatically > switch back to TV mode (if there is one, of course). > >>> > >>> No. This would break existing applications. The mode switch should be done > >>> at S_FREQUENCY (e. g. when the radio application is tuning into a > >>> channel). > >> > >> This is not what happens today as the switch to radio occurs as soon as > >> you open > >> the radio node. It's the reason for the s_radio op. > > > > The s_radio op is something that I wanted to remove. It was there in the > > past to feed > > the TV/radio hint logic. I wrote a patch for it, but I ended by discarding > > from my > > final queue (I can't remember why). > > > > I think that the hint logic were completely removed, but we may need to > > take a look > > on the callers for s_radio. I'll check it right now. > > > > The s_radio callback requires some care, as it is used on several places. It > is probably > safe to remove it from tuner, but a few sub-drivers like msp3400 needs it. > The actual > troubles seem to happen at the bridge drivers that call it during open(). It > should be > called only at s_frequency. I opted to keep the callback just to avoid having > a bridge > driver switching its registers to radio mode, and not having the tuner > following it. > > If we move the radio mode switch at the bridge drivers to s_frequency only, > we can just > remove this callback from tuner, letting it to be implemented only at the > audio decoders. Why would the audio decoders need it? If we do the mode switch when s_freq is called, then the audio decoders can do the same and s_radio can disappear completely. I would like that, but I'm a bit afraid of application breakage since we're changing the behavior of /dev/radio. It seems that pretty much every video driver with radio capability is calling s_radio during open(): bttv, ivtv, saa7134, usbvision, em28xx, cx18, cx88, cx231xx and tm6000. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv4 PATCH 6/8] v4l2-ioctl.c: prefill tuner type for g_frequency and g/s_tuner.
On Sun, 2011-06-12 at 12:59 +0200, Hans Verkuil wrote: > From: Hans Verkuil > > The subdevs are supposed to receive a valid tuner type for the g_frequency > and g/s_tuner subdev ops. Some drivers do this, others don't. So prefill > this in v4l2-ioctl.c based on whether the device node from which this is > called is a radio node or not. > > The spec does not require applications to fill in the type, and if they > leave it at 0 then the 'supported_mode' call in tuner-core.c will return > false and the ioctl does nothing. > > Signed-off-by: Hans Verkuil > --- > drivers/media/video/v4l2-ioctl.c |6 ++ > 1 files changed, 6 insertions(+), 0 deletions(-) > > diff --git a/drivers/media/video/v4l2-ioctl.c > b/drivers/media/video/v4l2-ioctl.c > index 213ba7d..26bf3bf 100644 > --- a/drivers/media/video/v4l2-ioctl.c > +++ b/drivers/media/video/v4l2-ioctl.c > @@ -1822,6 +1822,8 @@ static long __video_do_ioctl(struct file *file, > if (!ops->vidioc_g_tuner) > break; > > + p->type = (vfd->vfl_type == VFL_TYPE_RADIO) ? > + V4L2_TUNER_RADIO : V4L2_TUNER_ANALOG_TV; > ret = ops->vidioc_g_tuner(file, fh, p); > if (!ret) > dbgarg(cmd, "index=%d, name=%s, type=%d, " > @@ -1840,6 +1842,8 @@ static long __video_do_ioctl(struct file *file, > > if (!ops->vidioc_s_tuner) > break; > + p->type = (vfd->vfl_type == VFL_TYPE_RADIO) ? > + V4L2_TUNER_RADIO : V4L2_TUNER_ANALOG_TV; > dbgarg(cmd, "index=%d, name=%s, type=%d, " > "capability=0x%x, rangelow=%d, " > "rangehigh=%d, signal=%d, afc=%d, " > @@ -1858,6 +1862,8 @@ static long __video_do_ioctl(struct file *file, > if (!ops->vidioc_g_frequency) > break; > > + p->type = (vfd->vfl_type == VFL_TYPE_RADIO) ? > + V4L2_TUNER_RADIO : V4L2_TUNER_ANALOG_TV; > ret = ops->vidioc_g_frequency(file, fh, p); > if (!ret) > dbgarg(cmd, "tuner=%d, type=%d, frequency=%d\n", Wow, that was easy. And from what I can tell, it is spec compliant too. :) Reviewed-by: Andy Walls -Andy -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv1 PATCH 7/7] tuner-core: s_tuner should not change tuner mode.
On Sun, 2011-06-12 at 14:30 +0200, Hans Verkuil wrote: > On Sunday, June 12, 2011 14:13:30 Mauro Carvalho Chehab wrote: > > Em 12-06-2011 08:59, Mauro Carvalho Chehab escreveu: > > > Em 12-06-2011 08:36, Hans Verkuil escreveu: > > What about this: > > > > Opening /dev/radio effectively starts the radio mode. So if there is TV > > capture in progress, then the open should return -EBUSY. Otherwise it > > switches the tuner to radio mode. And it stays in radio mode until the > > last filehandle of /dev/radio is closed. At that point it will > > automatically > > switch back to TV mode (if there is one, of course). > > >>> > > >>> No. This would break existing applications. The mode switch should be > > >>> done > > >>> at S_FREQUENCY (e. g. when the radio application is tuning into a > > >>> channel). > > >> > > >> This is not what happens today as the switch to radio occurs as soon as > > >> you open > > >> the radio node. It's the reason for the s_radio op. > > > > > > The s_radio op is something that I wanted to remove. It was there in the > > > past to feed > > > the TV/radio hint logic. I wrote a patch for it, but I ended by > > > discarding from my > > > final queue (I can't remember why). > > > > > > I think that the hint logic were completely removed, but we may need to > > > take a look > > > on the callers for s_radio. I'll check it right now. > > > > > > > The s_radio callback requires some care, as it is used on several places. > > It is probably > > safe to remove it from tuner, but a few sub-drivers like msp3400 needs it. > > The actual > > troubles seem to happen at the bridge drivers that call it during open(). > > It should be > > called only at s_frequency. I opted to keep the callback just to avoid > > having a bridge > > driver switching its registers to radio mode, and not having the tuner > > following it. > > > > If we move the radio mode switch at the bridge drivers to s_frequency only, > > we can just > > remove this callback from tuner, letting it to be implemented only at the > > audio decoders. > > Why would the audio decoders need it? If we do the mode switch when s_freq is > called, then the audio decoders can do the same and s_radio can disappear > completely. > > I would like that, but I'm a bit afraid of application breakage since we're > changing > the behavior of /dev/radio. It seems that pretty much every video driver with > radio > capability is calling s_radio during open(): bttv, ivtv, saa7134, usbvision, > em28xx, > cx18, cx88, cx231xx and tm6000. I think ivtvhopper relies on it: http://www.gateways-home.org/wb/pages/mycoding/--ivtvhopper-java.php Also, per my recommendation, ivtvhopper changes radio freq by using /dev/video24, since V4L2 priorities got in the way: http://ivtvdriver.org/pipermail/ivtv-users/2010-December/010097.html Regards, Andy -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv1 PATCH 7/7] tuner-core: s_tuner should not change tuner mode.
On Sunday, June 12, 2011 14:53:06 Andy Walls wrote: > On Sun, 2011-06-12 at 14:30 +0200, Hans Verkuil wrote: > > On Sunday, June 12, 2011 14:13:30 Mauro Carvalho Chehab wrote: > > > Em 12-06-2011 08:59, Mauro Carvalho Chehab escreveu: > > > > Em 12-06-2011 08:36, Hans Verkuil escreveu: > > > What about this: > > > > > > Opening /dev/radio effectively starts the radio mode. So if there is > > > TV > > > capture in progress, then the open should return -EBUSY. Otherwise it > > > switches the tuner to radio mode. And it stays in radio mode until > > > the > > > last filehandle of /dev/radio is closed. At that point it will > > > automatically > > > switch back to TV mode (if there is one, of course). > > > >>> > > > >>> No. This would break existing applications. The mode switch should be > > > >>> done > > > >>> at S_FREQUENCY (e. g. when the radio application is tuning into a > > > >>> channel). > > > >> > > > >> This is not what happens today as the switch to radio occurs as soon > > > >> as you open > > > >> the radio node. It's the reason for the s_radio op. > > > > > > > > The s_radio op is something that I wanted to remove. It was there in > > > > the past to feed > > > > the TV/radio hint logic. I wrote a patch for it, but I ended by > > > > discarding from my > > > > final queue (I can't remember why). > > > > > > > > I think that the hint logic were completely removed, but we may need to > > > > take a look > > > > on the callers for s_radio. I'll check it right now. > > > > > > > > > > The s_radio callback requires some care, as it is used on several places. > > > It is probably > > > safe to remove it from tuner, but a few sub-drivers like msp3400 needs > > > it. The actual > > > troubles seem to happen at the bridge drivers that call it during open(). > > > It should be > > > called only at s_frequency. I opted to keep the callback just to avoid > > > having a bridge > > > driver switching its registers to radio mode, and not having the tuner > > > following it. > > > > > > If we move the radio mode switch at the bridge drivers to s_frequency > > > only, we can just > > > remove this callback from tuner, letting it to be implemented only at the > > > audio decoders. > > > > Why would the audio decoders need it? If we do the mode switch when s_freq > > is > > called, then the audio decoders can do the same and s_radio can disappear > > completely. > > > > I would like that, but I'm a bit afraid of application breakage since we're > > changing > > the behavior of /dev/radio. It seems that pretty much every video driver > > with radio > > capability is calling s_radio during open(): bttv, ivtv, saa7134, > > usbvision, em28xx, > > cx18, cx88, cx231xx and tm6000. > > I think ivtvhopper relies on it: > > http://www.gateways-home.org/wb/pages/mycoding/--ivtvhopper-java.php > > Also, per my recommendation, ivtvhopper changes radio freq by > using /dev/video24, since V4L2 priorities got in the way: > > http://ivtvdriver.org/pipermail/ivtv-users/2010-December/010097.html Well, radio support for ivtv is weird and we really need a ivtv-alsa (easier said than done). Because it is so non-standard, I am not terribly concerned about it. BTW, one problem with /dev/radio and ivtv (and I think cx18 might have the same problem) is that /dev/radio can be opened only once. A second attempt to open it will result in -EBUSY. That's a driver bug. I wonder if that's really the problem described in the link above instead of priority handling. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv1 PATCH 7/7] tuner-core: s_tuner should not change tuner mode.
On Sun, 2011-06-12 at 15:23 +0200, Hans Verkuil wrote: > On Sunday, June 12, 2011 14:53:06 Andy Walls wrote: > > On Sun, 2011-06-12 at 14:30 +0200, Hans Verkuil wrote: > > > On Sunday, June 12, 2011 14:13:30 Mauro Carvalho Chehab wrote: > > > > Em 12-06-2011 08:59, Mauro Carvalho Chehab escreveu: > > > > > Em 12-06-2011 08:36, Hans Verkuil escreveu: > > > > What about this: > > > > > > > > Opening /dev/radio effectively starts the radio mode. So if there > > > > is TV > > > > capture in progress, then the open should return -EBUSY. Otherwise > > > > it > > > > switches the tuner to radio mode. And it stays in radio mode until > > > > the > > > > last filehandle of /dev/radio is closed. At that point it will > > > > automatically > > > > switch back to TV mode (if there is one, of course). > > > > >>> > > > > >>> No. This would break existing applications. The mode switch should > > > > >>> be done > > > > >>> at S_FREQUENCY (e. g. when the radio application is tuning into a > > > > >>> channel). > > > > >> > > > > >> This is not what happens today as the switch to radio occurs as soon > > > > >> as you open > > > > >> the radio node. It's the reason for the s_radio op. > > > > > > > > > > The s_radio op is something that I wanted to remove. It was there in > > > > > the past to feed > > > > > the TV/radio hint logic. I wrote a patch for it, but I ended by > > > > > discarding from my > > > > > final queue (I can't remember why). > > > > > > > > > > I think that the hint logic were completely removed, but we may need > > > > > to take a look > > > > > on the callers for s_radio. I'll check it right now. > > > > > > > > > > > > > The s_radio callback requires some care, as it is used on several > > > > places. It is probably > > > > safe to remove it from tuner, but a few sub-drivers like msp3400 needs > > > > it. The actual > > > > troubles seem to happen at the bridge drivers that call it during > > > > open(). It should be > > > > called only at s_frequency. I opted to keep the callback just to avoid > > > > having a bridge > > > > driver switching its registers to radio mode, and not having the tuner > > > > following it. > > > > > > > > If we move the radio mode switch at the bridge drivers to s_frequency > > > > only, we can just > > > > remove this callback from tuner, letting it to be implemented only at > > > > the audio decoders. > > > > > > Why would the audio decoders need it? If we do the mode switch when > > > s_freq is > > > called, then the audio decoders can do the same and s_radio can disappear > > > completely. > > > > > > I would like that, but I'm a bit afraid of application breakage since > > > we're changing > > > the behavior of /dev/radio. It seems that pretty much every video driver > > > with radio > > > capability is calling s_radio during open(): bttv, ivtv, saa7134, > > > usbvision, em28xx, > > > cx18, cx88, cx231xx and tm6000. > > > > I think ivtvhopper relies on it: > > > > http://www.gateways-home.org/wb/pages/mycoding/--ivtvhopper-java.php > > > > Also, per my recommendation, ivtvhopper changes radio freq by > > using /dev/video24, since V4L2 priorities got in the way: > > > > http://ivtvdriver.org/pipermail/ivtv-users/2010-December/010097.html > > Well, radio support for ivtv is weird and we really need a ivtv-alsa (easier > said than done). Because it is so non-standard, I am not terribly concerned > about it. I use /dev/radio & /dev/video24 for FM radio using ivtv-radio, myself. BTW, the cx18-alsa module annoys me as a developer. PulseAudio holds the device nodes open, pinning the cx18-alsa and cx18 modules in kernel. When killed, PulseAudio respawns rapidly and reopens the nodes. Unloading cx18 for development purposes is a real pain when the cx18-alsa module exists. > BTW, one problem with /dev/radio and ivtv (and I think cx18 might have the > same > problem) is that /dev/radio can be opened only once. A second attempt to open > it will result in -EBUSY. That's a driver bug. I wonder if that's really the > problem described in the link above instead of priority handling. Gah, I think you are right. It probably was a multiple open() problem on /dev/radio for the app author. I do remember researching that cx18 and ivtv are single open() on /dev/radio. I also remember finding that the V4L2 spec doesn't require multiple opens, and implies drivers need not support it in at least two places: "Multiple Opens In general, V4L2 devices can be opened more than once. When this is supported by the driver, ..." "Name v4l2-open — Open a V4L2 device ... EBUSY The driver does not support multiple opens and the device is already in use. ..." Regards, Andy -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to
Re: [RFCv1 PATCH 7/7] tuner-core: s_tuner should not change tuner mode.
On Sun, Jun 12, 2011 at 9:44 AM, Andy Walls wrote: > BTW, the cx18-alsa module annoys me as a developer. PulseAudio holds > the device nodes open, pinning the cx18-alsa and cx18 modules in kernel. > When killed, PulseAudio respawns rapidly and reopens the nodes. > Unloading cx18 for development purposes is a real pain when the > cx18-alsa module exists. We've talked about this before, but something just feels wrong about this. I don't have this problem with other drivers that provide an "-alsa" module. For example, my ngene tree has four ALSA PCM devices and 16 mixer controls, yet PulseAudio doesn't keep the module in use. The more I think about this, the more I suspect this is just some sort of subtle bug in the cx18 ALSA driver where some resource is not being freed. Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv1 PATCH 7/7] tuner-core: s_tuner should not change tuner mode.
On Sunday, June 12, 2011 15:44:45 Andy Walls wrote: > On Sun, 2011-06-12 at 15:23 +0200, Hans Verkuil wrote: > > On Sunday, June 12, 2011 14:53:06 Andy Walls wrote: > > > On Sun, 2011-06-12 at 14:30 +0200, Hans Verkuil wrote: > > > > On Sunday, June 12, 2011 14:13:30 Mauro Carvalho Chehab wrote: > > > > > Em 12-06-2011 08:59, Mauro Carvalho Chehab escreveu: > > > > > > Em 12-06-2011 08:36, Hans Verkuil escreveu: > > > > > What about this: > > > > > > > > > > Opening /dev/radio effectively starts the radio mode. So if > > > > > there is TV > > > > > capture in progress, then the open should return -EBUSY. > > > > > Otherwise it > > > > > switches the tuner to radio mode. And it stays in radio mode > > > > > until the > > > > > last filehandle of /dev/radio is closed. At that point it will > > > > > automatically > > > > > switch back to TV mode (if there is one, of course). > > > > > >>> > > > > > >>> No. This would break existing applications. The mode switch > > > > > >>> should be done > > > > > >>> at S_FREQUENCY (e. g. when the radio application is tuning into a > > > > > >>> channel). > > > > > >> > > > > > >> This is not what happens today as the switch to radio occurs as > > > > > >> soon as you open > > > > > >> the radio node. It's the reason for the s_radio op. > > > > > > > > > > > > The s_radio op is something that I wanted to remove. It was there > > > > > > in the past to feed > > > > > > the TV/radio hint logic. I wrote a patch for it, but I ended by > > > > > > discarding from my > > > > > > final queue (I can't remember why). > > > > > > > > > > > > I think that the hint logic were completely removed, but we may > > > > > > need to take a look > > > > > > on the callers for s_radio. I'll check it right now. > > > > > > > > > > > > > > > > The s_radio callback requires some care, as it is used on several > > > > > places. It is probably > > > > > safe to remove it from tuner, but a few sub-drivers like msp3400 > > > > > needs it. The actual > > > > > troubles seem to happen at the bridge drivers that call it during > > > > > open(). It should be > > > > > called only at s_frequency. I opted to keep the callback just to > > > > > avoid having a bridge > > > > > driver switching its registers to radio mode, and not having the > > > > > tuner following it. > > > > > > > > > > If we move the radio mode switch at the bridge drivers to s_frequency > > > > > only, we can just > > > > > remove this callback from tuner, letting it to be implemented only at > > > > > the audio decoders. > > > > > > > > Why would the audio decoders need it? If we do the mode switch when > > > > s_freq is > > > > called, then the audio decoders can do the same and s_radio can > > > > disappear completely. > > > > > > > > I would like that, but I'm a bit afraid of application breakage since > > > > we're changing > > > > the behavior of /dev/radio. It seems that pretty much every video > > > > driver with radio > > > > capability is calling s_radio during open(): bttv, ivtv, saa7134, > > > > usbvision, em28xx, > > > > cx18, cx88, cx231xx and tm6000. > > > > > > I think ivtvhopper relies on it: > > > > > > http://www.gateways-home.org/wb/pages/mycoding/--ivtvhopper-java.php > > > > > > Also, per my recommendation, ivtvhopper changes radio freq by > > > using /dev/video24, since V4L2 priorities got in the way: > > > > > > http://ivtvdriver.org/pipermail/ivtv-users/2010-December/010097.html > > > > Well, radio support for ivtv is weird and we really need a ivtv-alsa (easier > > said than done). Because it is so non-standard, I am not terribly concerned > > about it. > > I use /dev/radio & /dev/video24 for FM radio using ivtv-radio, myself. > > BTW, the cx18-alsa module annoys me as a developer. PulseAudio holds > the device nodes open, pinning the cx18-alsa and cx18 modules in kernel. > When killed, PulseAudio respawns rapidly and reopens the nodes. > Unloading cx18 for development purposes is a real pain when the > cx18-alsa module exists. > > > > BTW, one problem with /dev/radio and ivtv (and I think cx18 might have the > > same > > problem) is that /dev/radio can be opened only once. A second attempt to > > open > > it will result in -EBUSY. That's a driver bug. I wonder if that's really the > > problem described in the link above instead of priority handling. > > Gah, I think you are right. It probably was a multiple open() problem > on /dev/radio for the app author. > > I do remember researching that cx18 and ivtv are single open() > on /dev/radio. > > I also remember finding that the V4L2 spec doesn't require multiple > opens, and implies drivers need not support it in at least two places: > > "Multiple Opens > > In general, V4L2 devices can be opened more than once. When this > is supported by the driver, ..." > > > "Name > v4l2-open — Open a V4L2 device > >
Re: [RFCv1 PATCH 7/7] tuner-core: s_tuner should not change tuner mode.
Em 12-06-2011 09:23, Hans Verkuil escreveu: > On Sunday, June 12, 2011 13:59:59 Mauro Carvalho Chehab wrote: >> Em 12-06-2011 08:36, Hans Verkuil escreveu: >>> On Saturday, June 11, 2011 21:04:57 Mauro Carvalho Chehab wrote: Em 11-06-2011 14:27, Hans Verkuil escreveu: > On Saturday, June 11, 2011 15:54:59 Mauro Carvalho Chehab wrote: >> Em 11-06-2011 10:34, Hans Verkuil escreveu: >>> From: Hans Verkuil >>> >>> According to the spec the tuner type field is not used when calling >>> S_TUNER: index, audmode and reserved are the only writable fields. >>> >>> So remove the type check. Instead, just set the audmode if the current >>> tuner mode is set to radio. >> >> I suspect that this patch also breaks support for a separate radio tuner. >> if tuner-type is not properly filled, then the easiest fix would be to >> revert some changes done at the tuner cleanup/fixup patches applied on >> .39. >> Yet, the previous logic were trying to hint the device mode, with is bad >> (that's why it was changed). >> >> The proper change seems to add a parameter to this callback, set by the >> bridge driver, informing if the device is using radio or video mode. >> We need also to patch the V4L API specs, as it allows using a video node >> for radio, and vice versa. This is not well supported, and it seems silly >> to keep it at the specs and needing to add hints at the drivers due to >> that. > > So, just to make sure I understand correctly what you want. The bridge or > platform drivers will fill in the vf->type (for g/s_frequency) or vt->type > (for g/s_tuner) based on the device node: RADIO if /dev/radio is used, > TV for anything else. Yes. I remember I've reviewed the bridge drivers when I rewrote the tuner code. Of course, I might have left something else. Btw, the older code were also requiring it. The cx18 implementation were merged after the changes, so maybe it is not doing the right thing. >>> >>> Actually, G_TUNER was wrong for bttv, ivtv, cx18 and pvrusb2. So it wasn't >>> just some random driver that failed to set vf/vt->type. >> >> G_FREQUENCY were broken just on cx18. As I said before, filling the type were >> always required. > > No, it wasn't required in the past. I'm happy with that change, but that > should be documented in v4l2-subdev.h since that's where the subdev ops are > documented. I will add such documentation in a RFCv5 patch series. > >> Anyway, I've added a proper documentation for it. See the >> patch bellow. >> >> The same patch should be done also for G_TUNER. > > And s_tuner. > >> > What about VIDIOC_S_FREQUENCY? The spec says that the app needs to fill > this > in. Will we just overwrite vf->type or will we check and return -EINVAL if > the app tries to set e.g. a TV frequency on /dev/radio? That's a very good question. What happens is that the V4L2 API used to allow opening a /dev/radio device for TV (or even for VBI). IMHO, this were a trouble at the API specs. I think that this were changed on newer versions of the spec. >>> >>> If you want, then I can add an extra patch to my v4 patch series that >>> returns >>> -EINVAL in video_ioctl2 if S_FREQUENCY is called with an inconsistent tuner >>> type. >>> (inconsistent with the device node's type, that is). >> >> The reason why check_mode returns -EINVAL is that this error code may need >> to be >> returned to the caller. You should note, however, that the bridge code may >> need >> to be fixed if you return the check_mode error code, as otherwise the error >> will >> simply be discarded or it it will break devices with two tuners. >> >> For example, currently, bttv driver uses v4l2_device_call_all() for it. >> So, any errors returned by it will be simply discarded. >> >> If some bridge driver is using v4l2_device_call_until_err(), it will stop on >> the first >> tuner that gets an error. >> >> The solution is to implement a v4l2_device_call_until_not_err() and use it >> for the >> tuner commands. > > That's not unreasonably to do at some point in time, but it doesn't actually > answer my question, which is: should the core refuse VIDIOC_S_FREQUENCY calls > where the type doesn't match the device node (i.e. radio vs tv)? I think it > makes no sense to call VIDIOC_S_FREQUENCY on a radio node with type ANALOG_TV. No. The core shouldn't do it, otherwise tuner will break. The code doesn't know if the opened device is radio or video. > > ... > > What about this: > > Opening /dev/radio effectively starts the radio mode. So if there is TV > capture in progress, then the open should return -EBUSY. Otherwise it > switches the tuner to radio mode. And it stays in radio mode until the > last filehandle of /dev/radio is closed. At that point it will > automatically > switch back to TV mod
Re: [RFCv1 PATCH 7/7] tuner-core: s_tuner should not change tuner mode.
Em 12-06-2011 10:57, Devin Heitmueller escreveu: > On Sun, Jun 12, 2011 at 9:44 AM, Andy Walls wrote: >> BTW, the cx18-alsa module annoys me as a developer. PulseAudio holds >> the device nodes open, pinning the cx18-alsa and cx18 modules in kernel. >> When killed, PulseAudio respawns rapidly and reopens the nodes. >> Unloading cx18 for development purposes is a real pain when the >> cx18-alsa module exists. > > We've talked about this before, but something just feels wrong about > this. I don't have this problem with other drivers that provide an > "-alsa" module. For example, my ngene tree has four ALSA PCM devices > and 16 mixer controls, yet PulseAudio doesn't keep the module in use. > > The more I think about this, the more I suspect this is just some sort > of subtle bug in the cx18 ALSA driver where some resource is not being > freed. It is not just cx18 that have this trouble. All drivers under media/video with *-alsa have this issue. Also, all sound drivers suffer from the same issue: # lsmod|grep snd_hda snd_hda_codec_analog84955 1 snd_hda_intel 25261 2 See: pulseaudio keep the device opened, so dev refcount were incremented. # rmmod snd_hda_codec_analog snd_hda_intel ERROR: Module snd_hda_codec_analog is in use ERROR: Module snd_hda_intel is in use The same happens, for example, with em28xx with snd-usb-audio: # lsmod |grep snd snd_usb_audio 91303 1 # rmmod snd-usb-audio ERROR: Module snd_usb_audio is in use What happens is that open() increments the device refcount. Maybe the ngene has some trick for allowing it, or PulseAudio has some logic to detect ngene (or otherwise it fails to open ngene audio nodes). It may have some dirty ways to trick PulseAudio, for example returning -ENODEV if the process name is pulseaudio, but I can't think on a proper kernel solution for it. The proper solution is to fix PulseAudio. Cheers, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv1 PATCH 7/7] tuner-core: s_tuner should not change tuner mode.
On Sunday, June 12, 2011 16:11:30 Mauro Carvalho Chehab wrote: > Em 12-06-2011 09:23, Hans Verkuil escreveu: > > That's not unreasonably to do at some point in time, but it doesn't actually > > answer my question, which is: should the core refuse VIDIOC_S_FREQUENCY > > calls > > where the type doesn't match the device node (i.e. radio vs tv)? I think it > > makes no sense to call VIDIOC_S_FREQUENCY on a radio node with type > > ANALOG_TV. > > No. The core shouldn't do it, otherwise tuner will break. The code doesn't > know if > the opened device is radio or video. I don't follow this. In v4l2-ioctl.c it is easy to tell if the opened device is radio or not by looking at vfd->vfl_type. > >> diff --git a/drivers/media/video/cx18/cx18-ioctl.c > >> b/drivers/media/video/cx18/cx18-ioctl.c > >> index 1933d4d..5463548 100644 > >> --- a/drivers/media/video/cx18/cx18-ioctl.c > >> +++ b/drivers/media/video/cx18/cx18-ioctl.c > >> @@ -611,6 +611,11 @@ static int cx18_g_frequency(struct file *file, void > >> *fh, > >>if (vf->tuner != 0) > >>return -EINVAL; > >> > >> + if (test_bit(CX18_F_I_RADIO_USER, &cx->i_flags)) > >> + vf->type = V4L2_TUNER_RADIO; > >> + else > >> + vf->type = V4L2_TUNER_ANALOG_TV; > >> + > > > > NACK. > > > > This sets the type to the current mode. But what we want is to set the type > > to > > the current device node. That's what my RFCv4 does (and that patch requires > > no > > driver change). > > I didn't get your RFCv4 patches here yet, but the fix should be at the > driver: it > needs to set the type before calling g_frequency. G_FREQUENCY shouldn't > change the > device mode, but, instead, to return the frequency and mode currently in > usage.. Why bother changing drivers (and probably missing a few) if you can do it in v4l2-ioctl.c and let drivers just pass it on? This is the patch in question, BTW: diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c index 213ba7d..26bf3bf 100644 --- a/drivers/media/video/v4l2-ioctl.c +++ b/drivers/media/video/v4l2-ioctl.c @@ -1822,6 +1822,8 @@ static long __video_do_ioctl(struct file *file, if (!ops->vidioc_g_tuner) break; + p->type = (vfd->vfl_type == VFL_TYPE_RADIO) ? + V4L2_TUNER_RADIO : V4L2_TUNER_ANALOG_TV; ret = ops->vidioc_g_tuner(file, fh, p); if (!ret) dbgarg(cmd, "index=%d, name=%s, type=%d, " @@ -1840,6 +1842,8 @@ static long __video_do_ioctl(struct file *file, if (!ops->vidioc_s_tuner) break; + p->type = (vfd->vfl_type == VFL_TYPE_RADIO) ? + V4L2_TUNER_RADIO : V4L2_TUNER_ANALOG_TV; dbgarg(cmd, "index=%d, name=%s, type=%d, " "capability=0x%x, rangelow=%d, " "rangehigh=%d, signal=%d, afc=%d, " @@ -1858,6 +1862,8 @@ static long __video_do_ioctl(struct file *file, if (!ops->vidioc_g_frequency) break; + p->type = (vfd->vfl_type == VFL_TYPE_RADIO) ? + V4L2_TUNER_RADIO : V4L2_TUNER_ANALOG_TV; ret = ops->vidioc_g_frequency(file, fh, p); if (!ret) dbgarg(cmd, "tuner=%d, type=%d, frequency=%d\n", Neither of these three ioctls will change the tuner mode, BTW. With this code in place drivers that use video_ioctl2 can now rely on the type field being a sensible value. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv4 PATCH 6/8] v4l2-ioctl.c: prefill tuner type for g_frequency and g/s_tuner.
Em 12-06-2011 07:59, Hans Verkuil escreveu: > From: Hans Verkuil > > The subdevs are supposed to receive a valid tuner type for the g_frequency > and g/s_tuner subdev ops. Some drivers do this, others don't. So prefill > this in v4l2-ioctl.c based on whether the device node from which this is > called is a radio node or not. > > The spec does not require applications to fill in the type, and if they > leave it at 0 then the 'supported_mode' call in tuner-core.c will return > false and the ioctl does nothing. Interesting solution. Yes, this is the proper fix, but only after being sure that no drivers allow switch to radio using the video device, and vice-versa. Unfortunately, this is not the case, currently. Most drivers allow this, following the previous V4L2 specs. Changing such behavior will probably require to write something at Documentation/feature-removal-schedule.txt, as we're changing the behavior. > > Signed-off-by: Hans Verkuil > --- > drivers/media/video/v4l2-ioctl.c |6 ++ > 1 files changed, 6 insertions(+), 0 deletions(-) > > diff --git a/drivers/media/video/v4l2-ioctl.c > b/drivers/media/video/v4l2-ioctl.c > index 213ba7d..26bf3bf 100644 > --- a/drivers/media/video/v4l2-ioctl.c > +++ b/drivers/media/video/v4l2-ioctl.c > @@ -1822,6 +1822,8 @@ static long __video_do_ioctl(struct file *file, > if (!ops->vidioc_g_tuner) > break; > > + p->type = (vfd->vfl_type == VFL_TYPE_RADIO) ? > + V4L2_TUNER_RADIO : V4L2_TUNER_ANALOG_TV; > ret = ops->vidioc_g_tuner(file, fh, p); > if (!ret) > dbgarg(cmd, "index=%d, name=%s, type=%d, " > @@ -1840,6 +1842,8 @@ static long __video_do_ioctl(struct file *file, > > if (!ops->vidioc_s_tuner) > break; > + p->type = (vfd->vfl_type == VFL_TYPE_RADIO) ? > + V4L2_TUNER_RADIO : V4L2_TUNER_ANALOG_TV; > dbgarg(cmd, "index=%d, name=%s, type=%d, " > "capability=0x%x, rangelow=%d, " > "rangehigh=%d, signal=%d, afc=%d, " > @@ -1858,6 +1862,8 @@ static long __video_do_ioctl(struct file *file, > if (!ops->vidioc_g_frequency) > break; > > + p->type = (vfd->vfl_type == VFL_TYPE_RADIO) ? > + V4L2_TUNER_RADIO : V4L2_TUNER_ANALOG_TV; > ret = ops->vidioc_g_frequency(file, fh, p); > if (!ret) > dbgarg(cmd, "tuner=%d, type=%d, frequency=%d\n", -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv4 PATCH 1/8] tuner-core: rename check_mode to supported_mode
Em 12-06-2011 07:59, Hans Verkuil escreveu:
> From: Hans Verkuil
>
> The check_mode function checks whether a mode is supported. So calling it
> supported_mode is more appropriate. In addition it returned either 0 or
> -EINVAL which suggests that the -EINVAL error should be passed on. However,
> that's not the case so change the return type to bool.
I prefer to keep returning -EINVAL. This is the proper thing to do, and
to return the result to the caller. A fixme should be added though, so,
after someone add a subdev call that would properly handle the -EINVAL
code for multiple tuners, the functions should return the error code
instead of 0.
>
> Signed-off-by: Hans Verkuil
> ---
> drivers/media/video/tuner-core.c | 19 ---
> 1 files changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/media/video/tuner-core.c
> b/drivers/media/video/tuner-core.c
> index 5748d04..083b9f1 100644
> --- a/drivers/media/video/tuner-core.c
> +++ b/drivers/media/video/tuner-core.c
> @@ -723,22 +723,19 @@ static int tuner_remove(struct i2c_client *client)
> */
>
> /**
> - * check_mode - Verify if tuner supports the requested mode
> + * supported_mode - Verify if tuner supports the requested mode
> * @t: a pointer to the module's internal struct_tuner
> *
> * This function checks if the tuner is capable of tuning analog TV,
> * digital TV or radio, depending on what the caller wants. If the
> - * tuner can't support that mode, it returns -EINVAL. Otherwise, it
> - * returns 0.
> + * tuner can't support that mode, it returns false. Otherwise, it
> + * returns true.
> * This function is needed for boards that have a separate tuner for
> * radio (like devices with tea5767).
> */
> -static inline int check_mode(struct tuner *t, enum v4l2_tuner_type mode)
> +static bool supported_mode(struct tuner *t, enum v4l2_tuner_type mode)
> {
> - if ((1 << mode & t->mode_mask) == 0)
> - return -EINVAL;
> -
> - return 0;
> + return 1 << mode & t->mode_mask;
> }
>
> /**
> @@ -759,7 +756,7 @@ static int set_mode_freq(struct i2c_client *client,
> struct tuner *t,
> struct analog_demod_ops *analog_ops = &t->fe.ops.analog_ops;
>
> if (mode != t->mode) {
> - if (check_mode(t, mode) == -EINVAL) {
> + if (!supported_mode(t, mode)) {
> tuner_dbg("Tuner doesn't support mode %d. "
> "Putting tuner to sleep\n", mode);
> t->standby = true;
> @@ -1138,7 +1135,7 @@ static int tuner_g_frequency(struct v4l2_subdev *sd,
> struct v4l2_frequency *f)
> struct tuner *t = to_tuner(sd);
> struct dvb_tuner_ops *fe_tuner_ops = &t->fe.ops.tuner_ops;
>
> - if (check_mode(t, f->type) == -EINVAL)
> + if (!supported_mode(t, f->type))
> return 0;
> f->type = t->mode;
> if (fe_tuner_ops->get_frequency && !t->standby) {
> @@ -1161,7 +1158,7 @@ static int tuner_g_tuner(struct v4l2_subdev *sd, struct
> v4l2_tuner *vt)
> struct analog_demod_ops *analog_ops = &t->fe.ops.analog_ops;
> struct dvb_tuner_ops *fe_tuner_ops = &t->fe.ops.tuner_ops;
>
> - if (check_mode(t, vt->type) == -EINVAL)
> + if (!supported_mode(t, vt->type))
> return 0;
> vt->type = t->mode;
> if (analog_ops->get_afc)
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv4 PATCH 2/8] tuner-core: change return type of set_mode_freq to bool
Em 12-06-2011 07:59, Hans Verkuil escreveu:
> From: Hans Verkuil
>
> set_mode_freq currently returns 0 or -EINVAL. But -EINVAL does not
> indicate a error that should be passed on, it just indicates that the
> tuner does not supportthe requested mode. So change the return type to
> bool.
NACK. Tuner core doesn't return the error code just because the subdev
functions don't allow, currently, at the multiple tuners case.
>
> Signed-off-by: Hans Verkuil
> ---
> drivers/media/video/tuner-core.c | 23 ++-
> 1 files changed, 10 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/media/video/tuner-core.c
> b/drivers/media/video/tuner-core.c
> index 083b9f1..ee43e0a 100644
> --- a/drivers/media/video/tuner-core.c
> +++ b/drivers/media/video/tuner-core.c
> @@ -746,11 +746,11 @@ static bool supported_mode(struct tuner *t, enum
> v4l2_tuner_type mode)
> * @freq:frequency to set (0 means to use the previous one)
> *
> * If tuner doesn't support the needed mode (radio or TV), prints a
> - * debug message and returns -EINVAL, changing its state to standby.
> - * Otherwise, changes the state and sets frequency to the last value, if
> - * the tuner can sleep or if it supports both Radio and TV.
> + * debug message and returns false, changing its state to standby.
> + * Otherwise, changes the state and sets frequency to the last value
> + * and returns true.
> */
> -static int set_mode_freq(struct i2c_client *client, struct tuner *t,
> +static bool set_mode_freq(struct i2c_client *client, struct tuner *t,
>enum v4l2_tuner_type mode, unsigned int freq)
> {
> struct analog_demod_ops *analog_ops = &t->fe.ops.analog_ops;
> @@ -762,7 +762,7 @@ static int set_mode_freq(struct i2c_client *client,
> struct tuner *t,
> t->standby = true;
> if (analog_ops->standby)
> analog_ops->standby(&t->fe);
> - return -EINVAL;
> + return false;
> }
> t->mode = mode;
> tuner_dbg("Changing to mode %d\n", mode);
> @@ -777,7 +777,7 @@ static int set_mode_freq(struct i2c_client *client,
> struct tuner *t,
> set_tv_freq(client, t->tv_freq);
> }
>
> - return 0;
> + return true;
> }
>
> /*
> @@ -1075,8 +1075,7 @@ static int tuner_s_radio(struct v4l2_subdev *sd)
> struct tuner *t = to_tuner(sd);
> struct i2c_client *client = v4l2_get_subdevdata(sd);
>
> - if (set_mode_freq(client, t, V4L2_TUNER_RADIO, 0) == -EINVAL)
> - return 0;
> + set_mode_freq(client, t, V4L2_TUNER_RADIO, 0);
> return 0;
> }
>
> @@ -1110,7 +1109,7 @@ static int tuner_s_std(struct v4l2_subdev *sd,
> v4l2_std_id std)
> struct tuner *t = to_tuner(sd);
> struct i2c_client *client = v4l2_get_subdevdata(sd);
>
> - if (set_mode_freq(client, t, V4L2_TUNER_ANALOG_TV, 0) == -EINVAL)
> + if (!set_mode_freq(client, t, V4L2_TUNER_ANALOG_TV, 0))
> return 0;
>
> t->std = std;
> @@ -1124,9 +1123,7 @@ static int tuner_s_frequency(struct v4l2_subdev *sd,
> struct v4l2_frequency *f)
> struct tuner *t = to_tuner(sd);
> struct i2c_client *client = v4l2_get_subdevdata(sd);
>
> - if (set_mode_freq(client, t, f->type, f->frequency) == -EINVAL)
> - return 0;
> -
> + set_mode_freq(client, t, f->type, f->frequency);
> return 0;
> }
>
> @@ -1197,7 +1194,7 @@ static int tuner_s_tuner(struct v4l2_subdev *sd, struct
> v4l2_tuner *vt)
> struct tuner *t = to_tuner(sd);
> struct i2c_client *client = v4l2_get_subdevdata(sd);
>
> - if (set_mode_freq(client, t, vt->type, 0) == -EINVAL)
> + if (!set_mode_freq(client, t, vt->type, 0))
> return 0;
>
> if (t->mode == V4L2_TUNER_RADIO)
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv4 PATCH 3/8] tuner-core: simplify the standard fixup.
Em 12-06-2011 07:59, Hans Verkuil escreveu:
> From: Hans Verkuil
>
> Get rid of a number of unnecessary tuner_dbg messages by simplifying
> the std fixup function.
Seems ok to me.
>
> Signed-off-by: Hans Verkuil
> ---
> drivers/media/video/tuner-core.c | 92
> +++---
> 1 files changed, 27 insertions(+), 65 deletions(-)
>
> diff --git a/drivers/media/video/tuner-core.c
> b/drivers/media/video/tuner-core.c
> index ee43e0a..462a8f4 100644
> --- a/drivers/media/video/tuner-core.c
> +++ b/drivers/media/video/tuner-core.c
> @@ -832,7 +832,7 @@ static void set_tv_freq(struct i2c_client *c, unsigned
> int freq)
> /**
> * tuner_fixup_std - force a given video standard variant
> *
> - * @t: tuner internal struct
> + * @std: TV standard
> *
> * A few devices or drivers have problem to detect some standard variations.
> * On other operational systems, the drivers generally have a per-country
> @@ -842,57 +842,39 @@ static void set_tv_freq(struct i2c_client *c, unsigned
> int freq)
> * to distinguish all video standard variations, a modprobe parameter can
> * be used to force a video standard match.
> */
> -static int tuner_fixup_std(struct tuner *t)
> +static v4l2_std_id tuner_fixup_std(struct tuner *t, v4l2_std_id std)
> {
> - if ((t->std & V4L2_STD_PAL) == V4L2_STD_PAL) {
> + if (pal[0] != '-' && (std & V4L2_STD_PAL) == V4L2_STD_PAL) {
> switch (pal[0]) {
> case '6':
> - tuner_dbg("insmod fixup: PAL => PAL-60\n");
> - t->std = V4L2_STD_PAL_60;
> - break;
> + return V4L2_STD_PAL_60;
> case 'b':
> case 'B':
> case 'g':
> case 'G':
> - tuner_dbg("insmod fixup: PAL => PAL-BG\n");
> - t->std = V4L2_STD_PAL_BG;
> - break;
> + return V4L2_STD_PAL_BG;
> case 'i':
> case 'I':
> - tuner_dbg("insmod fixup: PAL => PAL-I\n");
> - t->std = V4L2_STD_PAL_I;
> - break;
> + return V4L2_STD_PAL_I;
> case 'd':
> case 'D':
> case 'k':
> case 'K':
> - tuner_dbg("insmod fixup: PAL => PAL-DK\n");
> - t->std = V4L2_STD_PAL_DK;
> - break;
> + return V4L2_STD_PAL_DK;
> case 'M':
> case 'm':
> - tuner_dbg("insmod fixup: PAL => PAL-M\n");
> - t->std = V4L2_STD_PAL_M;
> - break;
> + return V4L2_STD_PAL_M;
> case 'N':
> case 'n':
> - if (pal[1] == 'c' || pal[1] == 'C') {
> - tuner_dbg("insmod fixup: PAL => PAL-Nc\n");
> - t->std = V4L2_STD_PAL_Nc;
> - } else {
> - tuner_dbg("insmod fixup: PAL => PAL-N\n");
> - t->std = V4L2_STD_PAL_N;
> - }
> - break;
> - case '-':
> - /* default parameter, do nothing */
> - break;
> + if (pal[1] == 'c' || pal[1] == 'C')
> + return V4L2_STD_PAL_Nc;
> + return V4L2_STD_PAL_N;
> default:
> tuner_warn("pal= argument not recognised\n");
> break;
> }
> }
> - if ((t->std & V4L2_STD_SECAM) == V4L2_STD_SECAM) {
> + if (secam[0] != '-' && (std & V4L2_STD_SECAM) == V4L2_STD_SECAM) {
> switch (secam[0]) {
> case 'b':
> case 'B':
> @@ -900,63 +882,42 @@ static int tuner_fixup_std(struct tuner *t)
> case 'G':
> case 'h':
> case 'H':
> - tuner_dbg("insmod fixup: SECAM => SECAM-BGH\n");
> - t->std = V4L2_STD_SECAM_B |
> - V4L2_STD_SECAM_G |
> - V4L2_STD_SECAM_H;
> - break;
> + return V4L2_STD_SECAM_B |
> +V4L2_STD_SECAM_G |
> +V4L2_STD_SECAM_H;
> case 'd':
> case 'D':
> case 'k':
> case 'K':
> - tuner_dbg("insmod fixup: SECAM => SECAM-DK\n");
> - t->std = V4L2_STD_SECAM_DK;
> - break;
> + return V4L2_STD_SECAM_DK;
> case 'l':
> case 'L':
> - if ((secam[1] == 'C') || (secam[1] == 'c')) {
> - tuner_dbg("insmod fixup: SECAM => SECAM-L'\n");
> -
Re: [RFCv4 PATCH 4/8] tuner-core: fix s_std and s_tuner.
Em 12-06-2011 07:59, Hans Verkuil escreveu:
> From: Hans Verkuil
>
> Both s_std and s_tuner are broken because set_mode_freq is called before the
> new std (for s_std) and audmode (for s_tuner) are set.
>
> This patch splits set_mode_freq in a set_mode and a set_freq and in s_std
> first calls set_mode, and if that returns true (i.e. the mode is supported)
> then they set t->std/t->audmode and call set_freq.
>
> This fixes a bug where changing std or audmode would actually change it to
> the previous value.
>
> Discovered while testing analog TV standards for cx18 with a tda18271 tuner.
I need to better analyse and test this patch.
>
> Signed-off-by: Hans Verkuil
> ---
> drivers/media/video/tuner-core.c | 60
> +-
> 1 files changed, 33 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/media/video/tuner-core.c
> b/drivers/media/video/tuner-core.c
> index 462a8f4..bf7fc33 100644
> --- a/drivers/media/video/tuner-core.c
> +++ b/drivers/media/video/tuner-core.c
> @@ -739,19 +739,15 @@ static bool supported_mode(struct tuner *t, enum
> v4l2_tuner_type mode)
> }
>
> /**
> - * set_mode_freq - Switch tuner to other mode.
> - * @client: struct i2c_client pointer
> + * set_mode - Switch tuner to other mode.
> * @t: a pointer to the module's internal struct_tuner
> * @mode:enum v4l2_type (radio or TV)
> - * @freq:frequency to set (0 means to use the previous one)
> *
> * If tuner doesn't support the needed mode (radio or TV), prints a
> * debug message and returns false, changing its state to standby.
> - * Otherwise, changes the state and sets frequency to the last value
> - * and returns true.
> + * Otherwise, changes the state and returns true.
> */
> -static bool set_mode_freq(struct i2c_client *client, struct tuner *t,
> - enum v4l2_tuner_type mode, unsigned int freq)
> +static bool set_mode(struct tuner *t, enum v4l2_tuner_type mode)
> {
> struct analog_demod_ops *analog_ops = &t->fe.ops.analog_ops;
>
> @@ -767,17 +763,27 @@ static bool set_mode_freq(struct i2c_client *client,
> struct tuner *t,
> t->mode = mode;
> tuner_dbg("Changing to mode %d\n", mode);
> }
> + return true;
> +}
> +
> +/**
> + * set_freq - Set the tuner to the desired frequency.
> + * @t: a pointer to the module's internal struct_tuner
> + * @freq:frequency to set (0 means to use the current frequency)
> + */
> +static void set_freq(struct tuner *t, unsigned int freq)
> +{
> + struct i2c_client *client = v4l2_get_subdevdata(&t->sd);
> +
> if (t->mode == V4L2_TUNER_RADIO) {
> - if (freq)
> - t->radio_freq = freq;
> - set_radio_freq(client, t->radio_freq);
> + if (!freq)
> + freq = t->radio_freq;
> + set_radio_freq(client, freq);
> } else {
> - if (freq)
> - t->tv_freq = freq;
> - set_tv_freq(client, t->tv_freq);
> + if (!freq)
> + freq = t->tv_freq;
> + set_tv_freq(client, freq);
> }
> -
> - return true;
> }
>
> /*
> @@ -1034,9 +1040,9 @@ static void tuner_status(struct dvb_frontend *fe)
> static int tuner_s_radio(struct v4l2_subdev *sd)
> {
> struct tuner *t = to_tuner(sd);
> - struct i2c_client *client = v4l2_get_subdevdata(sd);
>
> - set_mode_freq(client, t, V4L2_TUNER_RADIO, 0);
> + if (set_mode(t, V4L2_TUNER_RADIO))
> + set_freq(t, 0);
> return 0;
> }
>
> @@ -1068,24 +1074,23 @@ static int tuner_s_power(struct v4l2_subdev *sd, int
> on)
> static int tuner_s_std(struct v4l2_subdev *sd, v4l2_std_id std)
> {
> struct tuner *t = to_tuner(sd);
> - struct i2c_client *client = v4l2_get_subdevdata(sd);
>
> - if (!set_mode_freq(client, t, V4L2_TUNER_ANALOG_TV, 0))
> + if (!set_mode(t, V4L2_TUNER_ANALOG_TV))
> return 0;
>
> t->std = tuner_fixup_std(t, std);
> if (t->std != std)
> tuner_dbg("Fixup standard %llx to %llx\n", std, t->std);
> -
> + set_freq(t, 0);
> return 0;
> }
>
> static int tuner_s_frequency(struct v4l2_subdev *sd, struct v4l2_frequency
> *f)
> {
> struct tuner *t = to_tuner(sd);
> - struct i2c_client *client = v4l2_get_subdevdata(sd);
>
> - set_mode_freq(client, t, f->type, f->frequency);
> + if (set_mode(t, f->type))
> + set_freq(t, f->frequency);
> return 0;
> }
>
> @@ -1154,13 +1159,14 @@ static int tuner_g_tuner(struct v4l2_subdev *sd,
> struct v4l2_tuner *vt)
> static int tuner_s_tuner(struct v4l2_subdev *sd, struct v4l2_tuner *vt)
> {
> struct tuner *t = to_tuner(sd);
> - struct i2c_client *client = v4l2_get_subdevdata(sd);
>
> - if (!set_mode_freq(client, t, vt->type, 0))
> + if (!set_mode(t, vt->type))
> return 0;
>
> - if (t->mode == V
Re: [RFCv4 PATCH 5/8] tuner-core: fix tuner_resume: use t->mode instead of t->type.
Em 12-06-2011 07:59, Hans Verkuil escreveu:
> From: Hans Verkuil
>
> set_mode is called with t->type, which is the tuner type. Instead, use
> t->mode which is the actual tuner mode (i.e. radio vs tv).
>
> Signed-off-by: Hans Verkuil
Not tested, but it seems ok.
> ---
> drivers/media/video/tuner-core.c |2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/media/video/tuner-core.c
> b/drivers/media/video/tuner-core.c
> index bf7fc33..ffe5de3 100644
> --- a/drivers/media/video/tuner-core.c
> +++ b/drivers/media/video/tuner-core.c
> @@ -1201,7 +1201,7 @@ static int tuner_resume(struct i2c_client *c)
> tuner_dbg("resume\n");
>
> if (!t->standby)
> - if (set_mode(t, t->type))
> + if (set_mode(t, t->mode))
> set_freq(t, 0);
> return 0;
> }
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv4 PATCH 8/8] bttv: fix s_tuner for radio.
Em 12-06-2011 07:59, Hans Verkuil escreveu: > From: Hans Verkuil > > Fix typo: g_tuner should have been s_tuner. > > Tested with a bttv card. OK. > > Signed-off-by: Hans Verkuil > --- > drivers/media/video/bt8xx/bttv-driver.c |2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/drivers/media/video/bt8xx/bttv-driver.c > b/drivers/media/video/bt8xx/bttv-driver.c > index a97cf27..834a483 100644 > --- a/drivers/media/video/bt8xx/bttv-driver.c > +++ b/drivers/media/video/bt8xx/bttv-driver.c > @@ -3474,7 +3474,7 @@ static int radio_s_tuner(struct file *file, void *priv, > if (0 != t->index) > return -EINVAL; > > - bttv_call_all(btv, tuner, g_tuner, t); > + bttv_call_all(btv, tuner, s_tuner, t); > return 0; > } > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv4 PATCH 7/8] pvrusb2: fix g/s_tuner support.
Em 12-06-2011 07:59, Hans Verkuil escreveu:
> From: Hans Verkuil
>
> The tuner-core subdev requires that the type field of v4l2_tuner is
> filled in correctly. This is done in v4l2-ioctl.c, but pvrusb2 doesn't
> use that yet, so we have to do it manually based on whether the current
> input is radio or not.
>
> Tested with my pvrusb2.
OK.
>
> Signed-off-by: Hans Verkuil
> ---
> drivers/media/video/pvrusb2/pvrusb2-hdw.c |4
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/media/video/pvrusb2/pvrusb2-hdw.c
> b/drivers/media/video/pvrusb2/pvrusb2-hdw.c
> index 9d0dd08..e98d382 100644
> --- a/drivers/media/video/pvrusb2/pvrusb2-hdw.c
> +++ b/drivers/media/video/pvrusb2/pvrusb2-hdw.c
> @@ -3046,6 +3046,8 @@ static void pvr2_subdev_update(struct pvr2_hdw *hdw)
> if (hdw->input_dirty || hdw->audiomode_dirty || hdw->force_dirty) {
> struct v4l2_tuner vt;
> memset(&vt, 0, sizeof(vt));
> + vt.type = (hdw->input_val == PVR2_CVAL_INPUT_RADIO) ?
> + V4L2_TUNER_RADIO : V4L2_TUNER_ANALOG_TV;
> vt.audmode = hdw->audiomode_val;
> v4l2_device_call_all(&hdw->v4l2_dev, 0, tuner, s_tuner, &vt);
> }
> @@ -5171,6 +5173,8 @@ void pvr2_hdw_status_poll(struct pvr2_hdw *hdw)
> {
> struct v4l2_tuner *vtp = &hdw->tuner_signal_info;
> memset(vtp, 0, sizeof(*vtp));
> + vtp->type = (hdw->input_val == PVR2_CVAL_INPUT_RADIO) ?
> + V4L2_TUNER_RADIO : V4L2_TUNER_ANALOG_TV;
> hdw->tuner_signal_stale = 0;
> /* Note: There apparently is no replacement for VIDIOC_CROPCAP
> using v4l2-subdev - therefore we can't support that AT ALL right
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv1 PATCH 7/7] tuner-core: s_tuner should not change tuner mode.
Hans Verkuil wrote: >On Sunday, June 12, 2011 16:11:30 Mauro Carvalho Chehab wrote: >> Em 12-06-2011 09:23, Hans Verkuil escreveu: >> > That's not unreasonably to do at some point in time, but it doesn't >actually >> > answer my question, which is: should the core refuse >VIDIOC_S_FREQUENCY calls >> > where the type doesn't match the device node (i.e. radio vs tv)? I >think it >> > makes no sense to call VIDIOC_S_FREQUENCY on a radio node with type >ANALOG_TV. >> >> No. The core shouldn't do it, otherwise tuner will break. The code >doesn't know if >> the opened device is radio or video. > >I don't follow this. In v4l2-ioctl.c it is easy to tell if the opened >device >is radio or not by looking at vfd->vfl_type. > >> >> diff --git a/drivers/media/video/cx18/cx18-ioctl.c >b/drivers/media/video/cx18/cx18-ioctl.c >> >> index 1933d4d..5463548 100644 >> >> --- a/drivers/media/video/cx18/cx18-ioctl.c >> >> +++ b/drivers/media/video/cx18/cx18-ioctl.c >> >> @@ -611,6 +611,11 @@ static int cx18_g_frequency(struct file >*file, void *fh, >> >> if (vf->tuner != 0) >> >> return -EINVAL; >> >> >> >> + if (test_bit(CX18_F_I_RADIO_USER, &cx->i_flags)) >> >> + vf->type = V4L2_TUNER_RADIO; >> >> + else >> >> + vf->type = V4L2_TUNER_ANALOG_TV; >> >> + >> > >> > NACK. >> > >> > This sets the type to the current mode. But what we want is to set >the type to >> > the current device node. That's what my RFCv4 does (and that patch >requires no >> > driver change). >> >> I didn't get your RFCv4 patches here yet, but the fix should be at >the driver: it >> needs to set the type before calling g_frequency. G_FREQUENCY >shouldn't change the >> device mode, but, instead, to return the frequency and mode currently >in usage.. > >Why bother changing drivers (and probably missing a few) if you can do >it >in v4l2-ioctl.c and let drivers just pass it on? > >This is the patch in question, BTW: > >diff --git a/drivers/media/video/v4l2-ioctl.c >b/drivers/media/video/v4l2-ioctl.c >index 213ba7d..26bf3bf 100644 >--- a/drivers/media/video/v4l2-ioctl.c >+++ b/drivers/media/video/v4l2-ioctl.c >@@ -1822,6 +1822,8 @@ static long __video_do_ioctl(struct file *file, >if (!ops->vidioc_g_tuner) >break; > >+ p->type = (vfd->vfl_type == VFL_TYPE_RADIO) ? >+ V4L2_TUNER_RADIO : V4L2_TUNER_ANALOG_TV; >ret = ops->vidioc_g_tuner(file, fh, p); >if (!ret) >dbgarg(cmd, "index=%d, name=%s, type=%d, " >@@ -1840,6 +1842,8 @@ static long __video_do_ioctl(struct file *file, > >if (!ops->vidioc_s_tuner) >break; >+ p->type = (vfd->vfl_type == VFL_TYPE_RADIO) ? >+ V4L2_TUNER_RADIO : V4L2_TUNER_ANALOG_TV; >dbgarg(cmd, "index=%d, name=%s, type=%d, " >"capability=0x%x, rangelow=%d, " >"rangehigh=%d, signal=%d, afc=%d, " >@@ -1858,6 +1862,8 @@ static long __video_do_ioctl(struct file *file, >if (!ops->vidioc_g_frequency) >break; > >+ p->type = (vfd->vfl_type == VFL_TYPE_RADIO) ? >+ V4L2_TUNER_RADIO : V4L2_TUNER_ANALOG_TV; >ret = ops->vidioc_g_frequency(file, fh, p); >if (!ret) > dbgarg(cmd, "tuner=%d, type=%d, frequency=%d\n", > >Neither of these three ioctls will change the tuner mode, BTW. With >this >code in place drivers that use video_ioctl2 can now rely on the type >field >being a sensible value. > >Regards, > > Hans >-- >To unsubscribe from this list: send the line "unsubscribe linux-media" >in >the body of a message to majord...@vger.kernel.org >More majordomo info at http://vger.kernel.org/majordomo-info.html Right. In fact for s_tuner (and g_tuner) the spec implies that app writers should not fill in the field. BTW, S_tuner only really changes analog audio decoding right? Regards, Andy -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv1 PATCH 7/7] tuner-core: s_tuner should not change tuner mode.
Devin Heitmueller wrote: >On Sun, Jun 12, 2011 at 9:44 AM, Andy Walls >wrote: >> BTW, the cx18-alsa module annoys me as a developer. PulseAudio holds >> the device nodes open, pinning the cx18-alsa and cx18 modules in >kernel. >> When killed, PulseAudio respawns rapidly and reopens the nodes. >> Unloading cx18 for development purposes is a real pain when the >> cx18-alsa module exists. > >We've talked about this before, but something just feels wrong about >this. I don't have this problem with other drivers that provide an >"-alsa" module. For example, my ngene tree has four ALSA PCM devices >and 16 mixer controls, yet PulseAudio doesn't keep the module in use. > >The more I think about this, the more I suspect this is just some sort >of subtle bug in the cx18 ALSA driver where some resource is not being >freed. > >Devin > >-- >Devin J. Heitmueller - Kernel Labs >http://www.kernellabs.com I'll recheck with my shiny new Fedora 15 install maybe later tonight. The only thing that springs to mind that PA may not like is no mixer controls. Some basic code is there in cx18-alsa-mixer.c, but never registered. Pactl does have some magic cmd to let go of the nodes, but I can never remember it. Regards, Andy -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv4 PATCH 6/8] v4l2-ioctl.c: prefill tuner type for g_frequency and g/s_tuner.
On Sunday, June 12, 2011 16:36:11 Mauro Carvalho Chehab wrote: > Em 12-06-2011 07:59, Hans Verkuil escreveu: > > From: Hans Verkuil > > > > The subdevs are supposed to receive a valid tuner type for the g_frequency > > and g/s_tuner subdev ops. Some drivers do this, others don't. So prefill > > this in v4l2-ioctl.c based on whether the device node from which this is > > called is a radio node or not. > > > > The spec does not require applications to fill in the type, and if they > > leave it at 0 then the 'supported_mode' call in tuner-core.c will return > > false and the ioctl does nothing. > > Interesting solution. Yes, this is the proper fix, but only after being sure > that no drivers allow switch to radio using the video device, and vice-versa. Why would that be a problem? What this patch does is that it fixes those drivers that do *not* set vf/vt->type (i.e. leave it at 0). For those that already set it nothing changes. > Unfortunately, this is not the case, currently. > > Most drivers allow this, following the previous V4L2 specs. Changing such > behavior will probably require to write something at > Documentation/feature-removal-schedule.txt, as we're changing the behavior. I think in the longer term we need to change the spec so that: 1) Opening a radio node no longer switches to radio mode. Instead, you need to call VIDIOC_S_FREQUENCY for that. 2) When VIDIOC_S_FREQUENCY the type field should match the video/radio node it is called on. So for /dev/radio type should be RADIO, for others it should be ANALOG_TV. Otherwise -EINVAL is called. So this might be a good feature removal for 3.2 or 3.3. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv4 PATCH 1/8] tuner-core: rename check_mode to supported_mode
On Sunday, June 12, 2011 16:37:55 Mauro Carvalho Chehab wrote:
> Em 12-06-2011 07:59, Hans Verkuil escreveu:
> > From: Hans Verkuil
> >
> > The check_mode function checks whether a mode is supported. So calling it
> > supported_mode is more appropriate. In addition it returned either 0 or
> > -EINVAL which suggests that the -EINVAL error should be passed on. However,
> > that's not the case so change the return type to bool.
>
> I prefer to keep returning -EINVAL. This is the proper thing to do, and
> to return the result to the caller. A fixme should be added though, so,
> after someone add a subdev call that would properly handle the -EINVAL
> code for multiple tuners, the functions should return the error code
> instead of 0.
No, you can't return -EINVAL here. It is the responsibility of the bridge
driver to determine whether there is e.g. a radio tuner. So if one of these
tuner subdevs is called with mode radio while it is a tv tuner, then that
is not an error, but instead it is a request that can safely be ignored
as not relevant for that tuner. It is up to the bridge driver to ensure
that a tuner is loaded that is actually valid for the radio mode.
Subdev ops should only return errors when there is a real problem (e.g. i2c
errors) and should just return 0 if a request is not for them.
That's why I posted these first two patches: these functions suggest that you
can return an error if the mode doesn't match when you really cannot.
If I call v4l2_device_call_until_err() for e.g. s_frequency, then the error
that is returned should match a real error (e.g. an i2c error), not that one
of the tv tuners refused the radio mode. I know there is a radio tuner
somewhere,
otherwise there wouldn't be a /dev/radio node.
I firmly believe that the RFCv4 series is correct and just needs an additional
patch adding some documentation.
That said, it would make sense to move the first three patches to the end
instead if you prefer. Since these are cleanups, not bug fixes like the others.
Regards,
Hans
>
> >
> > Signed-off-by: Hans Verkuil
> > ---
> > drivers/media/video/tuner-core.c | 19 ---
> > 1 files changed, 8 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/media/video/tuner-core.c
> > b/drivers/media/video/tuner-core.c
> > index 5748d04..083b9f1 100644
> > --- a/drivers/media/video/tuner-core.c
> > +++ b/drivers/media/video/tuner-core.c
> > @@ -723,22 +723,19 @@ static int tuner_remove(struct i2c_client *client)
> > */
> >
> > /**
> > - * check_mode - Verify if tuner supports the requested mode
> > + * supported_mode - Verify if tuner supports the requested mode
> > * @t: a pointer to the module's internal struct_tuner
> > *
> > * This function checks if the tuner is capable of tuning analog TV,
> > * digital TV or radio, depending on what the caller wants. If the
> > - * tuner can't support that mode, it returns -EINVAL. Otherwise, it
> > - * returns 0.
> > + * tuner can't support that mode, it returns false. Otherwise, it
> > + * returns true.
> > * This function is needed for boards that have a separate tuner for
> > * radio (like devices with tea5767).
> > */
> > -static inline int check_mode(struct tuner *t, enum v4l2_tuner_type mode)
> > +static bool supported_mode(struct tuner *t, enum v4l2_tuner_type mode)
> > {
> > - if ((1 << mode & t->mode_mask) == 0)
> > - return -EINVAL;
> > -
> > - return 0;
> > + return 1 << mode & t->mode_mask;
> > }
> >
> > /**
> > @@ -759,7 +756,7 @@ static int set_mode_freq(struct i2c_client *client,
> > struct tuner *t,
> > struct analog_demod_ops *analog_ops = &t->fe.ops.analog_ops;
> >
> > if (mode != t->mode) {
> > - if (check_mode(t, mode) == -EINVAL) {
> > + if (!supported_mode(t, mode)) {
> > tuner_dbg("Tuner doesn't support mode %d. "
> > "Putting tuner to sleep\n", mode);
> > t->standby = true;
> > @@ -1138,7 +1135,7 @@ static int tuner_g_frequency(struct v4l2_subdev *sd,
> > struct v4l2_frequency *f)
> > struct tuner *t = to_tuner(sd);
> > struct dvb_tuner_ops *fe_tuner_ops = &t->fe.ops.tuner_ops;
> >
> > - if (check_mode(t, f->type) == -EINVAL)
> > + if (!supported_mode(t, f->type))
> > return 0;
> > f->type = t->mode;
> > if (fe_tuner_ops->get_frequency && !t->standby) {
> > @@ -1161,7 +1158,7 @@ static int tuner_g_tuner(struct v4l2_subdev *sd,
> > struct v4l2_tuner *vt)
> > struct analog_demod_ops *analog_ops = &t->fe.ops.analog_ops;
> > struct dvb_tuner_ops *fe_tuner_ops = &t->fe.ops.tuner_ops;
> >
> > - if (check_mode(t, vt->type) == -EINVAL)
> > + if (!supported_mode(t, vt->type))
> > return 0;
> > vt->type = t->mode;
> > if (analog_ops->get_afc)
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at
Re: [alsa-devel] [PATCH] tea575x: allow multiple opens
At Sat, 11 Jun 2011 15:36:39 +0200,
Hans Verkuil wrote:
>
> On Saturday, June 11, 2011 15:28:59 Ondrej Zary wrote:
> > Change locking to allow tea575x-radio device to be opened multiple times.
>
> Very nice!
>
> Signed-off-by: Hans Verkuil
Hans, would you apply this and another one via v4l tree or shall I
apply them via sound tree?
Takashi
>
> Regards,
>
> Hans
>
> > Signed-off-by: Ondrej Zary
> >
> > --- linux-2.6.39-rc2-/include/sound/tea575x-tuner.h 2011-06-11
> > 15:21:50.0 +0200
> > +++ linux-2.6.39-rc2/include/sound/tea575x-tuner.h 2011-06-11
> > 14:50:55.0 +0200
> > @@ -49,7 +49,7 @@ struct snd_tea575x {
> > bool tuned; /* tuned to a station */
> > unsigned int val; /* hw value */
> > unsigned long freq; /* frequency */
> > - unsigned long in_use; /* set if the device is in use */
> > + struct mutex mutex;
> > struct snd_tea575x_ops *ops;
> > void *private_data;
> > u8 card[32];
> > --- linux-2.6.39-rc2-/sound/i2c/other/tea575x-tuner.c 2011-06-11
> > 15:21:50.0 +0200
> > +++ linux-2.6.39-rc2/sound/i2c/other/tea575x-tuner.c2011-06-11
> > 14:57:28.0 +0200
> > @@ -282,26 +282,9 @@ static int vidioc_s_input(struct file *f
> > return 0;
> > }
> >
> > -static int snd_tea575x_exclusive_open(struct file *file)
> > -{
> > - struct snd_tea575x *tea = video_drvdata(file);
> > -
> > - return test_and_set_bit(0, &tea->in_use) ? -EBUSY : 0;
> > -}
> > -
> > -static int snd_tea575x_exclusive_release(struct file *file)
> > -{
> > - struct snd_tea575x *tea = video_drvdata(file);
> > -
> > - clear_bit(0, &tea->in_use);
> > - return 0;
> > -}
> > -
> > static const struct v4l2_file_operations tea575x_fops = {
> > .owner = THIS_MODULE,
> > - .open = snd_tea575x_exclusive_open,
> > - .release= snd_tea575x_exclusive_release,
> > - .ioctl = video_ioctl2,
> > + .unlocked_ioctl = video_ioctl2,
> > };
> >
> > static const struct v4l2_ioctl_ops tea575x_ioctl_ops = {
> > @@ -340,13 +323,14 @@ int snd_tea575x_init(struct snd_tea575x
> > if (snd_tea575x_read(tea) != 0x55AA)
> > return -ENODEV;
> >
> > - tea->in_use = 0;
> > tea->val = TEA575X_BIT_BAND_FM | TEA575X_BIT_SEARCH_10_40;
> > tea->freq = 90500 * 16; /* 90.5Mhz default */
> > snd_tea575x_set_freq(tea);
> >
> > tea->vd = tea575x_radio;
> > video_set_drvdata(&tea->vd, tea);
> > + mutex_init(&tea->mutex);
> > + tea->vd.lock = &tea->mutex;
> >
> > v4l2_ctrl_handler_init(&tea->ctrl_handler, 1);
> > tea->vd.ctrl_handler = &tea->ctrl_handler;
> >
> >
> >
> ___
> Alsa-devel mailing list
> alsa-de...@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv4 PATCH 6/8] v4l2-ioctl.c: prefill tuner type for g_frequency and g/s_tuner.
Em 12-06-2011 12:46, Hans Verkuil escreveu: > On Sunday, June 12, 2011 16:36:11 Mauro Carvalho Chehab wrote: >> Em 12-06-2011 07:59, Hans Verkuil escreveu: >>> From: Hans Verkuil >>> >>> The subdevs are supposed to receive a valid tuner type for the g_frequency >>> and g/s_tuner subdev ops. Some drivers do this, others don't. So prefill >>> this in v4l2-ioctl.c based on whether the device node from which this is >>> called is a radio node or not. >>> >>> The spec does not require applications to fill in the type, and if they >>> leave it at 0 then the 'supported_mode' call in tuner-core.c will return >>> false and the ioctl does nothing. >> >> Interesting solution. Yes, this is the proper fix, but only after being sure >> that no drivers allow switch to radio using the video device, and vice-versa. > > Why would that be a problem? What this patch does is that it fixes those > drivers that do *not* set vf/vt->type (i.e. leave it at 0). For those that > already > set it nothing changes. Yeah, I realized it after after answering. Yes, your patch seems to be ok, as bridge drivers can override it. > >> Unfortunately, this is not the case, currently. >> >> Most drivers allow this, following the previous V4L2 specs. Changing such >> behavior will probably require to write something at >> Documentation/feature-removal-schedule.txt, as we're changing the behavior. > > I think in the longer term we need to change the spec so that: > > 1) Opening a radio node no longer switches to radio mode. Instead, you need to >call VIDIOC_S_FREQUENCY for that. > 2) When VIDIOC_S_FREQUENCY the type field should match the video/radio node it >is called on. So for /dev/radio type should be RADIO, for others it should > be >ANALOG_TV. Otherwise -EINVAL is called. > > So this might be a good feature removal for 3.2 or 3.3. I'm OK with that. > > Regards, > > Hans > -- > To unsubscribe from this list: send the line "unsubscribe linux-media" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv4 PATCH 1/8] tuner-core: rename check_mode to supported_mode
Em 12-06-2011 13:07, Hans Verkuil escreveu:
> On Sunday, June 12, 2011 16:37:55 Mauro Carvalho Chehab wrote:
>> Em 12-06-2011 07:59, Hans Verkuil escreveu:
>>> From: Hans Verkuil
>>>
>>> The check_mode function checks whether a mode is supported. So calling it
>>> supported_mode is more appropriate. In addition it returned either 0 or
>>> -EINVAL which suggests that the -EINVAL error should be passed on. However,
>>> that's not the case so change the return type to bool.
>>
>> I prefer to keep returning -EINVAL. This is the proper thing to do, and
>> to return the result to the caller. A fixme should be added though, so,
>> after someone add a subdev call that would properly handle the -EINVAL
>> code for multiple tuners, the functions should return the error code
>> instead of 0.
>
> No, you can't return -EINVAL here. It is the responsibility of the bridge
> driver to determine whether there is e.g. a radio tuner. So if one of these
> tuner subdevs is called with mode radio while it is a tv tuner, then that
> is not an error, but instead it is a request that can safely be ignored
> as not relevant for that tuner. It is up to the bridge driver to ensure
> that a tuner is loaded that is actually valid for the radio mode.
>
> Subdev ops should only return errors when there is a real problem (e.g. i2c
> errors) and should just return 0 if a request is not for them.
>
> That's why I posted these first two patches: these functions suggest that you
> can return an error if the mode doesn't match when you really cannot.
>
> If I call v4l2_device_call_until_err() for e.g. s_frequency, then the error
> that is returned should match a real error (e.g. an i2c error), not that one
> of the tv tuners refused the radio mode. I know there is a radio tuner
> somewhere,
> otherwise there wouldn't be a /dev/radio node.
>
> I firmly believe that the RFCv4 series is correct and just needs an additional
> patch adding some documentation.
>
> That said, it would make sense to move the first three patches to the end
> instead if you prefer. Since these are cleanups, not bug fixes like the
> others.
The errors at tuner should be propagated. If there's just one tuner, the error
code should just be returned by the ioctl. But, if there are two tuners, if the
bridge
driver calls G_TUNER (or any other tuner subdev call) and both tuners return
-EINVAL,
then it needs to return -EINVAL to userspace. If just one returns -EINVAL, and
the
other tuner returns 0, then it should return 0. So, it is about the opposite
behaviour
implemented at v4l2_device_call_until_err().
In order to implement the correct behaviour, the tuner driver should return
-EINVAL if
check_mode/set_mode fails. However, this breaks any bridge that may be using
v4l2_device_call_until_err(). That's why the current code returns 0.
The proper fix for it is:
1) create a call_all function that returns 0 if one of the subdevs
returned 0,
or returns an error code otherwise;
2) change all bridge calls to tuner stuff to the new call_all function;
3) return the check_mode/set_mode error to the bridge.
One alternative for (1) would be to simply change the v4l2_device_call_all() to
return 0 if
one of the subdrivers returned 0. Something like (not tested):
#define __v4l2_device_call_subdevs_p(v4l2_dev, sd, cond, o, f, args..$
({ \
long __rc = 0, __err = 0; \
\
list_for_each_entry((sd), &(v4l2_dev)->subdevs, list) { \
if ((cond) && (sd)->ops->o && (sd)->ops->o->f) {\
__err = (sd)->ops->o->f((sd) , ##args); \
if (_err) \
__rc = __err; \
} \
} \
__rc; \
})
#define v4l2_device_call_all(v4l2_dev, grpid, o, f, args...)\
do {\
struct v4l2_subdev *__sd; \
\
__v4l2_device_call_subdevs_p(v4l2_dev, __sd,\
!(grpid) || __sd->grp_id == (grpid), o, f , \
##args);\
} while (0)
As it currently doesn't return any error, this shouldn't break any driver
(as nobody expects an error code there). We'll need to review the bridge
drivers anyway, so that they'll return the error code from v4l_device_call().
Cheers,
Mauro.
--
To unsubscribe from this list: sen
Re: [RFCv4 PATCH 1/8] tuner-core: rename check_mode to supported_mode
Em 12-06-2011 14:27, Mauro Carvalho Chehab escreveu:
> Em 12-06-2011 13:07, Hans Verkuil escreveu:
>> On Sunday, June 12, 2011 16:37:55 Mauro Carvalho Chehab wrote:
>>> Em 12-06-2011 07:59, Hans Verkuil escreveu:
From: Hans Verkuil
The check_mode function checks whether a mode is supported. So calling it
supported_mode is more appropriate. In addition it returned either 0 or
-EINVAL which suggests that the -EINVAL error should be passed on. However,
that's not the case so change the return type to bool.
>>>
>>> I prefer to keep returning -EINVAL. This is the proper thing to do, and
>>> to return the result to the caller. A fixme should be added though, so,
>>> after someone add a subdev call that would properly handle the -EINVAL
>>> code for multiple tuners, the functions should return the error code
>>> instead of 0.
>>
>> No, you can't return -EINVAL here. It is the responsibility of the bridge
>> driver to determine whether there is e.g. a radio tuner. So if one of these
>> tuner subdevs is called with mode radio while it is a tv tuner, then that
>> is not an error, but instead it is a request that can safely be ignored
>> as not relevant for that tuner. It is up to the bridge driver to ensure
>> that a tuner is loaded that is actually valid for the radio mode.
>>
>> Subdev ops should only return errors when there is a real problem (e.g. i2c
>> errors) and should just return 0 if a request is not for them.
>>
>> That's why I posted these first two patches: these functions suggest that you
>> can return an error if the mode doesn't match when you really cannot.
>>
>> If I call v4l2_device_call_until_err() for e.g. s_frequency, then the error
>> that is returned should match a real error (e.g. an i2c error), not that one
>> of the tv tuners refused the radio mode. I know there is a radio tuner
>> somewhere,
>> otherwise there wouldn't be a /dev/radio node.
>>
>> I firmly believe that the RFCv4 series is correct and just needs an
>> additional
>> patch adding some documentation.
>>
>> That said, it would make sense to move the first three patches to the end
>> instead if you prefer. Since these are cleanups, not bug fixes like the
>> others.
>
>
> The errors at tuner should be propagated. If there's just one tuner, the error
> code should just be returned by the ioctl. But, if there are two tuners, if
> the bridge
> driver calls G_TUNER (or any other tuner subdev call) and both tuners return
> -EINVAL,
> then it needs to return -EINVAL to userspace. If just one returns -EINVAL,
> and the
> other tuner returns 0, then it should return 0. So, it is about the opposite
> behaviour
> implemented at v4l2_device_call_until_err().
>
> In order to implement the correct behaviour, the tuner driver should return
> -EINVAL if
> check_mode/set_mode fails. However, this breaks any bridge that may be using
> v4l2_device_call_until_err(). That's why the current code returns 0.
>
> The proper fix for it is:
>
> 1) create a call_all function that returns 0 if one of the subdevs
> returned 0,
> or returns an error code otherwise;
> 2) change all bridge calls to tuner stuff to the new call_all function;
> 3) return the check_mode/set_mode error to the bridge.
>
> One alternative for (1) would be to simply change the v4l2_device_call_all()
> to return 0 if
> one of the subdrivers returned 0. Something like (not tested):
>
Sorry, wrong logic. It should be, instead:
#define __v4l2_device_call_subdevs_p(v4l2_dev, sd, cond, o, f, args..$
({ \
long __rc = -ENOIOCTLCMD, __err = _rc; \
\
list_for_each_entry((sd), &(v4l2_dev)->subdevs, list) { \
if ((cond) && (sd)->ops->o && (sd)->ops->o->f) {\
__err = (sd)->ops->o->f((sd) , ##args); \
if (!_err) \
__rc = 0; \
} \
} \
(__rc == 0) ? 0 : __err;\
})
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv1 PATCH 7/7] tuner-core: s_tuner should not change tuner mode.
Em 12-06-2011 12:34, Andy Walls escreveu: > Devin Heitmueller wrote: > >> On Sun, Jun 12, 2011 at 9:44 AM, Andy Walls >> wrote: >>> BTW, the cx18-alsa module annoys me as a developer. PulseAudio holds >>> the device nodes open, pinning the cx18-alsa and cx18 modules in >> kernel. >>> When killed, PulseAudio respawns rapidly and reopens the nodes. >>> Unloading cx18 for development purposes is a real pain when the >>> cx18-alsa module exists. >> >> We've talked about this before, but something just feels wrong about >> this. I don't have this problem with other drivers that provide an >> "-alsa" module. For example, my ngene tree has four ALSA PCM devices >> and 16 mixer controls, yet PulseAudio doesn't keep the module in use. >> >> The more I think about this, the more I suspect this is just some sort >> of subtle bug in the cx18 ALSA driver where some resource is not being >> freed. >> >> Devin >> >> -- >> Devin J. Heitmueller - Kernel Labs >> http://www.kernellabs.com > > I'll recheck with my shiny new Fedora 15 install maybe later tonight. > > The only thing that springs to mind that PA may not like is no mixer > controls. Some basic code is there in cx18-alsa-mixer.c, but never > registered. > > Pactl does have some magic cmd to let go of the nodes, but I can never > remember it. This should do the job: # yum remove -y pulseaudio :) Cheers, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] [PATCH] tea575x: allow multiple opens
On Sunday, June 12, 2011 18:52:56 Takashi Iwai wrote:
> At Sat, 11 Jun 2011 15:36:39 +0200,
> Hans Verkuil wrote:
> >
> > On Saturday, June 11, 2011 15:28:59 Ondrej Zary wrote:
> > > Change locking to allow tea575x-radio device to be opened multiple times.
> >
> > Very nice!
> >
> > Signed-off-by: Hans Verkuil
>
> Hans, would you apply this and another one via v4l tree or shall I
> apply them via sound tree?
This is all V4L related, so I think Mauro can apply this patch and the
other patch for v3.1. No need to go through an intermediate tree of mine.
Regards,
Hans
>
>
> Takashi
>
> >
> > Regards,
> >
> > Hans
> >
> > > Signed-off-by: Ondrej Zary
> > >
> > > --- linux-2.6.39-rc2-/include/sound/tea575x-tuner.h 2011-06-11
> > > 15:21:50.0 +0200
> > > +++ linux-2.6.39-rc2/include/sound/tea575x-tuner.h2011-06-11
> > > 14:50:55.0 +0200
> > > @@ -49,7 +49,7 @@ struct snd_tea575x {
> > > bool tuned; /* tuned to a station */
> > > unsigned int val; /* hw value */
> > > unsigned long freq; /* frequency */
> > > - unsigned long in_use; /* set if the device is in use */
> > > + struct mutex mutex;
> > > struct snd_tea575x_ops *ops;
> > > void *private_data;
> > > u8 card[32];
> > > --- linux-2.6.39-rc2-/sound/i2c/other/tea575x-tuner.c 2011-06-11
> > > 15:21:50.0 +0200
> > > +++ linux-2.6.39-rc2/sound/i2c/other/tea575x-tuner.c 2011-06-11
> > > 14:57:28.0 +0200
> > > @@ -282,26 +282,9 @@ static int vidioc_s_input(struct file *f
> > > return 0;
> > > }
> > >
> > > -static int snd_tea575x_exclusive_open(struct file *file)
> > > -{
> > > - struct snd_tea575x *tea = video_drvdata(file);
> > > -
> > > - return test_and_set_bit(0, &tea->in_use) ? -EBUSY : 0;
> > > -}
> > > -
> > > -static int snd_tea575x_exclusive_release(struct file *file)
> > > -{
> > > - struct snd_tea575x *tea = video_drvdata(file);
> > > -
> > > - clear_bit(0, &tea->in_use);
> > > - return 0;
> > > -}
> > > -
> > > static const struct v4l2_file_operations tea575x_fops = {
> > > .owner = THIS_MODULE,
> > > - .open = snd_tea575x_exclusive_open,
> > > - .release= snd_tea575x_exclusive_release,
> > > - .ioctl = video_ioctl2,
> > > + .unlocked_ioctl = video_ioctl2,
> > > };
> > >
> > > static const struct v4l2_ioctl_ops tea575x_ioctl_ops = {
> > > @@ -340,13 +323,14 @@ int snd_tea575x_init(struct snd_tea575x
> > > if (snd_tea575x_read(tea) != 0x55AA)
> > > return -ENODEV;
> > >
> > > - tea->in_use = 0;
> > > tea->val = TEA575X_BIT_BAND_FM | TEA575X_BIT_SEARCH_10_40;
> > > tea->freq = 90500 * 16; /* 90.5Mhz default */
> > > snd_tea575x_set_freq(tea);
> > >
> > > tea->vd = tea575x_radio;
> > > video_set_drvdata(&tea->vd, tea);
> > > + mutex_init(&tea->mutex);
> > > + tea->vd.lock = &tea->mutex;
> > >
> > > v4l2_ctrl_handler_init(&tea->ctrl_handler, 1);
> > > tea->vd.ctrl_handler = &tea->ctrl_handler;
> > >
> > >
> > >
> > ___
> > Alsa-devel mailing list
> > alsa-de...@alsa-project.org
> > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> >
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
firmware segfault with 'Terratec Cinergy T USB XXS (HD)/ T3'
When I plugin that USB receiver into a docked ThinkPad T400, s2ram it and resume it, I got this line within /var/log/message : 2011-06-12T19:49:10.030+02:00 n22 kernel: firmware[29666]: segfault at 46 ip b770b0a8 sp bfb21560 error 4 in libc-2.12.2.so[b76af000+156000] 2011-06-12T19:49:10.000+02:00 n22 udevd-work[29659]: 'firmware --firmware=dvb-usb-dib0700-1.20.fw --devpath=/devices/pci:00/:00:1a.7/usb1/1-1/firmware/1-1' unexpected exit with status 0x008b Furthermore in the upper left corner a cursor is blinking and the system stays in this mode until I plug off the USB stick. Then after a while the KDE desktop is back. -- MfG/Sincerely Toralf Förster pgp finger print: 7B1A 07F4 EC82 0F90 D4C2 8936 872A E508 7DB6 9DA3 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv4 PATCH 1/8] tuner-core: rename check_mode to supported_mode
On Sunday, June 12, 2011 19:27:21 Mauro Carvalho Chehab wrote:
> Em 12-06-2011 13:07, Hans Verkuil escreveu:
> > On Sunday, June 12, 2011 16:37:55 Mauro Carvalho Chehab wrote:
> >> Em 12-06-2011 07:59, Hans Verkuil escreveu:
> >>> From: Hans Verkuil
> >>>
> >>> The check_mode function checks whether a mode is supported. So calling it
> >>> supported_mode is more appropriate. In addition it returned either 0 or
> >>> -EINVAL which suggests that the -EINVAL error should be passed on.
> >>> However,
> >>> that's not the case so change the return type to bool.
> >>
> >> I prefer to keep returning -EINVAL. This is the proper thing to do, and
> >> to return the result to the caller. A fixme should be added though, so,
> >> after someone add a subdev call that would properly handle the -EINVAL
> >> code for multiple tuners, the functions should return the error code
> >> instead of 0.
> >
> > No, you can't return -EINVAL here. It is the responsibility of the bridge
> > driver to determine whether there is e.g. a radio tuner. So if one of these
> > tuner subdevs is called with mode radio while it is a tv tuner, then that
> > is not an error, but instead it is a request that can safely be ignored
> > as not relevant for that tuner. It is up to the bridge driver to ensure
> > that a tuner is loaded that is actually valid for the radio mode.
> >
> > Subdev ops should only return errors when there is a real problem (e.g. i2c
> > errors) and should just return 0 if a request is not for them.
> >
> > That's why I posted these first two patches: these functions suggest that
> > you
> > can return an error if the mode doesn't match when you really cannot.
> >
> > If I call v4l2_device_call_until_err() for e.g. s_frequency, then the error
> > that is returned should match a real error (e.g. an i2c error), not that one
> > of the tv tuners refused the radio mode. I know there is a radio tuner
> > somewhere,
> > otherwise there wouldn't be a /dev/radio node.
> >
> > I firmly believe that the RFCv4 series is correct and just needs an
> > additional
> > patch adding some documentation.
> >
> > That said, it would make sense to move the first three patches to the end
> > instead if you prefer. Since these are cleanups, not bug fixes like the
> > others.
>
>
> The errors at tuner should be propagated. If there's just one tuner, the error
> code should just be returned by the ioctl. But, if there are two tuners, if
> the bridge
> driver calls G_TUNER (or any other tuner subdev call) and both tuners return
> -EINVAL,
> then it needs to return -EINVAL to userspace. If just one returns -EINVAL,
> and the
> other tuner returns 0, then it should return 0. So, it is about the opposite
> behaviour
> implemented at v4l2_device_call_until_err().
Sorry, but no, that's not true. You are trying to use the error codes from tuner
subdevs to determine whether none of the tuner subdevs support a certain tuner
mode.
E.g., you want to change something for a radio tuner and there are no radio
tuner
subdevs. But that's the job of the bridge driver to check. That has the
overview,
the lowly subdevs do not. The subdevs just filter the ops and check the mode to
see
if they should handle it and ignore it otherwise.
Only if they have to handle it will they return a possible error. The big
problem
with trying to use subdev errors codes for this is that you don't see the
difference
between a real error of a subdev (e.g. -EIO when an i2c access fails) and a
subdev
that returns -EINVAL just because it didn't understand the tuner mode.
So the bridge may return -EINVAL to the application instead of the real error,
which
is -EIO.
That's the whole principle behind the sub-device model: sub-devices do not know
about 'the world outside'. So if you pass RADIO mode to S_FREQUENCY and there
is no
radio tuner, then the bridge driver is the one that should detect that and
return
-EINVAL.
Actually, as mentioned before, it can also be done in video_ioctl2.c by checking
the tuner mode against the device node it's called on. But that requires
tightening
of the V4L2 spec first.
Regards,
Hans
> In order to implement the correct behaviour, the tuner driver should return
> -EINVAL if
> check_mode/set_mode fails. However, this breaks any bridge that may be using
> v4l2_device_call_until_err(). That's why the current code returns 0.
>
> The proper fix for it is:
>
> 1) create a call_all function that returns 0 if one of the subdevs
> returned 0,
> or returns an error code otherwise;
> 2) change all bridge calls to tuner stuff to the new call_all function;
> 3) return the check_mode/set_mode error to the bridge.
>
> One alternative for (1) would be to simply change the v4l2_device_call_all()
> to return 0 if
> one of the subdrivers returned 0. Something like (not tested):
>
>
> #define __v4l2_device_call_subdevs_p(v4l2_dev, sd, cond, o, f, args..$
> ({
Re: [PATCH] [media] rc-core support for Microsoft IR keyboard/mouse
On Jun 11, 2011, at 9:38 AM, Mauro Carvalho Chehab wrote:
> Em 03-06-2011 18:28, Jarod Wilson escreveu:
>> This is a custom IR protocol decoder, for the RC-6-ish protocol used by
>> the Microsoft Remote Keyboard.
>>
>> http://www.amazon.com/Microsoft-Remote-Keyboard-Windows-ZV1-4/dp/B000AOAAN8
>>
>> Its a standard keyboard with embedded thumb stick mouse pointer and
>> mouse buttons, along with a number of media keys. The media keys are
>> standard RC-6, identical to the signals from the stock MCE remotes, and
>> will be handled as such. The keyboard and mouse signals will be decoded
>> and delivered to the system by an input device registered specifically
>> by this driver.
>>
>> Successfully tested with an mceusb-driven receiver, but this should
>> actually work with any raw IR rc-core receiver.
>>
>> This work is inspired by lirc_mod_mce:
>>
>> http://mod-mce.sourceforge.net/
>>
>> The documentation there and code aided in understanding and decoding the
>> protocol, but the bulk of the code is actually borrowed more from the
>> existing in-kernel decoders than anything. I did recycle the keyboard
>> keycode table and a few defines from lirc_mod_mce though.
>>
>> Signed-off-by: Jarod Wilson
>> ---
>
> I did only a quick review, and everything looks fine for me. Just two
> comments:
>
>> +#if 0
>> +/* Adding this reference means two input devices are associated with
>> + * this rc-core device, which ir-keytable doesn't cope with yet */
>> +idev->dev.parent = &dev->dev;
>> +#endif
>
> Well, it was never tested with such config ;)
D'oh, I meant to mention that #if 0...
> Feel free to fix rc-core.
It wasn't actually rc-core that complained, it was ir-keytable that spit out
a message about not being able to handle an rc device with multiple input
devices, but the fix may well require both userspace and kernelspace changes...
>> +static unsigned char kbd_keycodes[256] = {
>> + 0, 0, 0, 0, 30, 48, 46, 32, 18, 33, 34, 35, 23,
>> 36, 37, 38,
>> + 50, 49, 24, 25, 16, 19, 31, 20, 22, 47, 17, 45, 21,
>> 44, 2, 3,
>> + 4, 5, 6, 7, 8, 9, 10, 11, 28, 1, 14, 15, 57,
>> 12, 13, 26,
>> + 27, 43, 43, 39, 40, 41, 51, 52, 53, 58, 59, 60, 61,
>> 62, 63, 64,
>> + 65, 66, 67, 68, 87, 88, 99, 70, 119, 110, 102, 104, 111,
>> 107, 109, 106,
>> +105, 108, 103, 69, 98, 55, 74, 78, 96, 79, 80, 81, 75,
>> 76, 77, 71,
>> + 72, 73, 82, 83, 86, 127, 116, 117, 183, 184, 185, 186, 187,
>> 188, 189, 190,
>> +191, 192, 193, 194, 134, 138, 130, 132, 128, 129, 131, 137, 133,
>> 135, 136, 113,
>> +115, 114, 0, 0, 0, 121, 0, 89, 93, 124, 92, 94, 95,
>> 0, 0, 0,
>> +122, 123, 90, 91, 85, 0, 0, 0, 0, 0, 0, 0, 0,
>> 0, 0, 0,
>> + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
>> 0, 0, 0,
>> + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
>> 0, 0, 0,
>> + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
>> 0, 0, 0,
>> + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
>> 0, 0, 0,
>> + 29, 42, 56, 125, 97, 54, 100, 126, 164, 166, 165, 163, 161,
>> 115, 114, 113,
>> +150, 158, 159, 128, 136, 177, 178, 176, 142, 152, 173, 140
>> +};
>
> This table looks weird to me: too much magic numbers there. Shouldn't
> the above be replaced by KEY_* definitions?
Yeah, probably. The above is basically the same convention as hid_keyboard in
drivers/hid/hid-input.c, but that doesn't mean we can't strive to do better. ;)
> PS.: I would like to have one of those keyboards, in order to test some
> things here,
> in special, for the xorg input/event proposal on my TODO list ;) Is it a
> cheap device?
> I may try to buy one the next time I would travel to US.
Looks like Amazon has them listed for less than $40USD right now, but they're
becoming increasingly hard to find, as I believe they're discontinued. Don't
know if I've ever seen them in a brick-n-mortar store anywhere. I should really
get one for myself, I have to send this one back to the guy who loaned it to me.
I could just make it a double order and stash one on the shelf for ya.
--
Jarod Wilson
ja...@wilsonet.com
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[cron job] v4l-dvb daily build: ERRORS
This message is generated daily by a cron job that builds v4l-dvb for the kernels and architectures in the list below. Results of the daily build of v4l-dvb: date:Sun Jun 12 19:00:33 CEST 2011 git hash:f49c454fe981d955d7c3d620f6baa04fb9876adc gcc version: i686-linux-gcc (GCC) 4.5.1 host hardware:x86_64 host os: 2.6.32.5 linux-git-armv5: WARNINGS linux-git-armv5-davinci: WARNINGS linux-git-armv5-ixp: WARNINGS linux-git-armv5-omap2: WARNINGS linux-git-i686: WARNINGS linux-git-m32r: OK linux-git-mips: WARNINGS linux-git-powerpc64: WARNINGS linux-git-x86_64: WARNINGS linux-2.6.31.12-i686: ERRORS linux-2.6.32.6-i686: WARNINGS linux-2.6.33-i686: WARNINGS linux-2.6.34-i686: WARNINGS linux-2.6.35.3-i686: WARNINGS linux-2.6.36-i686: ERRORS linux-2.6.37-i686: WARNINGS linux-2.6.38.2-i686: WARNINGS linux-2.6.39.1-i686: WARNINGS linux-2.6.31.12-x86_64: ERRORS linux-2.6.32.6-x86_64: WARNINGS linux-2.6.33-x86_64: WARNINGS linux-2.6.34-x86_64: WARNINGS linux-2.6.35.3-x86_64: WARNINGS linux-2.6.36-x86_64: ERRORS linux-2.6.37-x86_64: WARNINGS linux-2.6.38.2-x86_64: WARNINGS linux-2.6.39.1-x86_64: WARNINGS spec-git: ERRORS sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Sunday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Sunday.tar.bz2 The V4L-DVB specification failed to build, but the last compiled spec is here: http://www.xs4all.nl/~hverkuil/spec/media.html -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [alsa-devel] [PATCH] tea575x: allow multiple opens
At Sun, 12 Jun 2011 19:48:47 +0200,
Hans Verkuil wrote:
>
> On Sunday, June 12, 2011 18:52:56 Takashi Iwai wrote:
> > At Sat, 11 Jun 2011 15:36:39 +0200,
> > Hans Verkuil wrote:
> > >
> > > On Saturday, June 11, 2011 15:28:59 Ondrej Zary wrote:
> > > > Change locking to allow tea575x-radio device to be opened multiple
> > > > times.
> > >
> > > Very nice!
> > >
> > > Signed-off-by: Hans Verkuil
> >
> > Hans, would you apply this and another one via v4l tree or shall I
> > apply them via sound tree?
>
> This is all V4L related, so I think Mauro can apply this patch and the
> other patch for v3.1. No need to go through an intermediate tree of mine.
OK, good to know my work is shortened :)
thanks,
Takashi
>
> Regards,
>
> Hans
>
> >
> >
> > Takashi
> >
> > >
> > > Regards,
> > >
> > > Hans
> > >
> > > > Signed-off-by: Ondrej Zary
> > > >
> > > > --- linux-2.6.39-rc2-/include/sound/tea575x-tuner.h 2011-06-11
> > > > 15:21:50.0 +0200
> > > > +++ linux-2.6.39-rc2/include/sound/tea575x-tuner.h 2011-06-11
> > > > 14:50:55.0 +0200
> > > > @@ -49,7 +49,7 @@ struct snd_tea575x {
> > > > bool tuned; /* tuned to a station */
> > > > unsigned int val; /* hw value */
> > > > unsigned long freq; /* frequency */
> > > > - unsigned long in_use; /* set if the device is in use
> > > > */
> > > > + struct mutex mutex;
> > > > struct snd_tea575x_ops *ops;
> > > > void *private_data;
> > > > u8 card[32];
> > > > --- linux-2.6.39-rc2-/sound/i2c/other/tea575x-tuner.c 2011-06-11
> > > > 15:21:50.0 +0200
> > > > +++ linux-2.6.39-rc2/sound/i2c/other/tea575x-tuner.c2011-06-11
> > > > 14:57:28.0 +0200
> > > > @@ -282,26 +282,9 @@ static int vidioc_s_input(struct file *f
> > > > return 0;
> > > > }
> > > >
> > > > -static int snd_tea575x_exclusive_open(struct file *file)
> > > > -{
> > > > - struct snd_tea575x *tea = video_drvdata(file);
> > > > -
> > > > - return test_and_set_bit(0, &tea->in_use) ? -EBUSY : 0;
> > > > -}
> > > > -
> > > > -static int snd_tea575x_exclusive_release(struct file *file)
> > > > -{
> > > > - struct snd_tea575x *tea = video_drvdata(file);
> > > > -
> > > > - clear_bit(0, &tea->in_use);
> > > > - return 0;
> > > > -}
> > > > -
> > > > static const struct v4l2_file_operations tea575x_fops = {
> > > > .owner = THIS_MODULE,
> > > > - .open = snd_tea575x_exclusive_open,
> > > > - .release= snd_tea575x_exclusive_release,
> > > > - .ioctl = video_ioctl2,
> > > > + .unlocked_ioctl = video_ioctl2,
> > > > };
> > > >
> > > > static const struct v4l2_ioctl_ops tea575x_ioctl_ops = {
> > > > @@ -340,13 +323,14 @@ int snd_tea575x_init(struct snd_tea575x
> > > > if (snd_tea575x_read(tea) != 0x55AA)
> > > > return -ENODEV;
> > > >
> > > > - tea->in_use = 0;
> > > > tea->val = TEA575X_BIT_BAND_FM | TEA575X_BIT_SEARCH_10_40;
> > > > tea->freq = 90500 * 16; /* 90.5Mhz default */
> > > > snd_tea575x_set_freq(tea);
> > > >
> > > > tea->vd = tea575x_radio;
> > > > video_set_drvdata(&tea->vd, tea);
> > > > + mutex_init(&tea->mutex);
> > > > + tea->vd.lock = &tea->mutex;
> > > >
> > > > v4l2_ctrl_handler_init(&tea->ctrl_handler, 1);
> > > > tea->vd.ctrl_handler = &tea->ctrl_handler;
> > > >
> > > >
> > > >
> > > ___
> > > Alsa-devel mailing list
> > > alsa-de...@alsa-project.org
> > > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> > >
> >
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv4 PATCH 6/8] v4l2-ioctl.c: prefill tuner type for g_frequency and g/s_tuner.
On Sunday, June 12, 2011 19:08:03 Mauro Carvalho Chehab wrote: > > I think in the longer term we need to change the spec so that: > > > > 1) Opening a radio node no longer switches to radio mode. Instead, you need > > to > >call VIDIOC_S_FREQUENCY for that. > > 2) When VIDIOC_S_FREQUENCY the type field should match the video/radio node > > it > >is called on. So for /dev/radio type should be RADIO, for others it > > should be > >ANALOG_TV. Otherwise -EINVAL is called. > > > > So this might be a good feature removal for 3.2 or 3.3. > > I'm OK with that. How about this: diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt index 1a9446b..9df0e09 100644 --- a/Documentation/feature-removal-schedule.txt +++ b/Documentation/feature-removal-schedule.txt @@ -600,3 +600,25 @@ Why: Superseded by the UVCIOC_CTRL_QUERY ioctl. Who: Laurent Pinchart + +What: For VIDIOC_S_FREQUENCY the type field must match the device node's type. + If not, return -EINVAL. +When: 3.2 +Why: It makes no sense to switch the tuner to radio mode by calling + VIDIOC_S_FREQUENCY on a video node, or to switch the tuner to tv mode by + calling VIDIOC_S_FREQUENCY on a radio node. This is the first step of a + move to more consistent handling of tv and radio tuners. +Who: Hans Verkuil + + + +What: Opening a radio device node will no longer automatically switch the + tuner mode from tv to radio. +When: 3.3 +Why: Just opening a V4L device should not change the state of the hardware + like that. It's very unexpected and against the V4L spec. Instead, you + switch to radio mode by calling VIDIOC_S_FREQUENCY. This is the second + and last step of the move to consistent handling of tv and radio tuners. +Who: Hans Verkuil + + Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] [media] af9015: setup rc keytable for LC-Power LC-USB-DVBT
That's this tuner:
http://www.lc-power.de/index.php?id=146&L=1
The credit card sized remote more or less works if I set remote=4,
so I added the hash to get it autodetected. (`more or less' there
meaning sometimes buttons are `stuck on repeat', i.e. ir-keytable -t
keeps repeating the same scancode until i press another button.)
Signed-off-by: Juergen Lock
--- a/drivers/media/dvb/dvb-usb/af9015.c
+++ b/drivers/media/dvb/dvb-usb/af9015.c
@@ -735,6 +735,7 @@ static const struct af9015_rc_setup af90
{ 0xb8feb708, RC_MAP_MSI_DIGIVOX_II },
{ 0xa3703d00, RC_MAP_ALINK_DTU_M },
{ 0x9b7dc64e, RC_MAP_TOTAL_MEDIA_IN_HAND }, /* MYGICTV U718 */
+ { 0x5d49e3db, RC_MAP_DIGITTRADE }, /* LC-Power LC-USB-DVBT */
{ }
};
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Add support for PCTV452E.
The same thing happens when the devices are on different USB buses: Bus 002 Device 004: ID 0b48:300a TechnoTrend AG TT-connect S2-3650 CI Bus 001 Device 007: ID 0b48:3006 TechnoTrend AG TT-connect S-2400 DVB-S Bus 001 Device 004: ID 734c:5980 TBS Technologies China When the S2-3650 CI scans, the stream output from the TBS is gone. When it's done scanning / locking, the TBS continues to work fine. The S-2400 is not affected in any way, nor it affects the other receivers. It seems the issue is not relative to the devices being on the same USB bus, nor the kernel used. I've tried Debian's 2.6.32.5 and 2.6.38-bpo2 on a Debian Squeeze system. На 11.6.2011 г. 19:38 ч., Doychin Dokov написа: i've been using the patches in the latest media_tree for some hours - the S2-3650 CI seems to work. There's one thing that disturbs me, though - when it scans / locks on a frequency, another device on the same PC, using the same stb6100, gets stuck for a moment. Any ideas what might be the cause for that? The two devices do not share common RF input signal, but are on the same USB bus: Bus 001 Device 004: ID 734c:5980 TBS Technologies China Bus 001 Device 003: ID 0b48:300a TechnoTrend AG TT-connect S2-3650 CI The TBS device is a QBOX2 SI, which works with their official driver from their web-site. There's also a third DVB device in the system - a TT S-2400 (which is on the other USB bus, though), which does not exhibit any problems related with the first two devices, nor causes them to get stuck when it's scanning. I'll try to switch devices around and see if anything changes. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [media] af9015: setup rc keytable for LC-Power LC-USB-DVBT
I assume device uses vendor reference design USB ID (15a4:9016 or
15a4:9015)?
About the repeating bug you mention, are you using latest driver
version? I am not aware such bug. There have been this kind of incorrect
behaviour old driver versions which are using HID. It was coming from
wrong HID interval.
Also you can dump remote codes out when setting debug=2 to
dvb_usb_af9015 module.
regards
Antti
On 06/12/2011 11:25 PM, Juergen Lock wrote:
That's this tuner:
http://www.lc-power.de/index.php?id=146&L=1
The credit card sized remote more or less works if I set remote=4,
so I added the hash to get it autodetected. (`more or less' there
meaning sometimes buttons are `stuck on repeat', i.e. ir-keytable -t
keeps repeating the same scancode until i press another button.)
Signed-off-by: Juergen Lock
--- a/drivers/media/dvb/dvb-usb/af9015.c
+++ b/drivers/media/dvb/dvb-usb/af9015.c
@@ -735,6 +735,7 @@ static const struct af9015_rc_setup af90
{ 0xb8feb708, RC_MAP_MSI_DIGIVOX_II },
{ 0xa3703d00, RC_MAP_ALINK_DTU_M },
{ 0x9b7dc64e, RC_MAP_TOTAL_MEDIA_IN_HAND }, /* MYGICTV U718 */
+ { 0x5d49e3db, RC_MAP_DIGITTRADE }, /* LC-Power LC-USB-DVBT */
{ }
};
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
http://palosaari.fi/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Improving kernel -> userspace (usbfs) usb device hand off
On Sun, 12 Jun 2011, Hans de Goede wrote: > Hi, > > On 06/11/2011 06:19 PM, Theodore Kilgore wrote: > > > > > > On Sat, 11 Jun 2011, Hans de Goede wrote: > > > > > Hi, > > > > > > Given the many comments in this thread, I'm just > > > going reply to this one, and try to also answer any > > > other ones in this mail. > > > > > > As far as the dual mode camera is involved, I agree > > > that that should be fixed in the existing v4l2 > > > drivers + libgphoto. I think that Felipe's solution > > > to also handle the stillcam part in kernel space for > > > dual mode cameras (and add a libgphoto cam driver which > > > knows how to talk the new kernel API for this), is > > > the best solution. Unfortunately this will involve > > > quite a bit of work, but so be it. > > > > Hans, > > > > It appears to me that the solution ought to be at hand, actually. > > > > I was not aware of the recent changes in libusb, which I understand are > > supposed to allow a kernel driver to be hooked up again. > > > > To review the situation: > > > > 1. As of approximately 2 years ago, libusb already was so configured as to > > suspend the kernel module for a dual-mode device if a userspace-based > > program tried to claim the device. > > > > 2. At this point with the more recent versions of libusb (see the last > > message from yesterday, from Xiaofan Chen), we are supposed to be able to > > re-activate the kernel module for the device when it is relinquished by > > userspace. > > > > This ought to take care of the problems completely, provided that the new > > capabilities of libusb are actually used and called upon in libgphoto2. > > > > I have checked on what is happening, just now, on my own machine. I have > > libusb version 1.08 which ought to be recent enough. The advertised > > abilities did not work, however. Presumably, what is missing is on the > > other end of the problem, most likely in the functions in libgphoto2 which > > hook up a camera. That code would presumably need to call upon the new > > functionality of libusb. My currently installed version of libgphoto2 > > (from svn, but several months old) clearly does not contain the needed > > functionality. But it might have been put in recently and I did not > > notice. I guess that the first thing to do is to update my gphoto tree and > > then to see what happens. If things still don't work, then something needs > > to be updated and then things ought to work. > > > > I will try to see that something gets done about this. Thank you for > > raising the old issue of dual-mode devices yet again, and thanks to > > Xiaofan Chen for pointing out that the needed missing half of the > > functionality is supposed to exist now in libusb. That had escaped my > > attention. > > Actually libusb and libgphoto have been using the rebind orginal driver > functionality of the code for quite a while now, Oh? I can see that libusb is doing that, and I can also see that there is a "public" function for _unbinding_ a kernel driver, namely int usb_detach_kernel_driver_np() found in usb.h and it is used in libgphoto, as well. I am not sure that there is any corresponding rebind function which is public. Is it perhaps int usb_get_driver_np() ??? By context (looking at libgphoto2-port/usb/libusb.c) I would think that this function is not the rebind function, but is only checking whether or not there is any potential conflict with a kernel driver. If I am right, then where is the publicly exported rebind function, and where does it currently get used in libgphoto2? So frankly after my eagerness yesterday I do not see how it can easily be made to work, after all. unfortunately this > does not solve the problem, unless we somehow move to 1 central > coordinator for the device the user experience will stay subpar. > > Example, user downloads pictures from the camera using shotwell, > gthumb, fspot or whatever, keeps the app in question open and the app > in question keeps the gphoto2 device handle open. > > User wants to do some skyping with video chat, skype complains it > cannot find the device, since the kernel driver currently is unbound. > > -> Poor user experience. Poor user experience, or merely poor user? The user ought to know better. Of course, I do agree that there are lots of such people, and it is a good idea to try to put up warning signs. > > With having both functions in the kernel, the kernel could actually > allow skype to use the dual mode cameras as video source, and if > the user then were to switch to f-spot and try to import more photo's > then he will get an -ebusy in f-spot. If he finishes skyping and > then returns to f-spot everything will just continue working. > > This is the kind of "seamless" user experience I'm aiming for here. > > Regards, > > Hans Yes, I can see where you are coming from. But if the camera really will not let you run skype and fspot at the same time, which I do not believe it would allow on
Re: [PATCH] Add support for PCTV452E.
On 2.6.32.5 with s2-liplianin tree this is not observed, i.e. everything works as expected - the S2-3650 does not cause the TBS device to stutter. На 12.6.2011 г. 23:27 ч., Doychin Dokov написа: The same thing happens when the devices are on different USB buses: Bus 002 Device 004: ID 0b48:300a TechnoTrend AG TT-connect S2-3650 CI Bus 001 Device 007: ID 0b48:3006 TechnoTrend AG TT-connect S-2400 DVB-S Bus 001 Device 004: ID 734c:5980 TBS Technologies China When the S2-3650 CI scans, the stream output from the TBS is gone. When it's done scanning / locking, the TBS continues to work fine. The S-2400 is not affected in any way, nor it affects the other receivers. It seems the issue is not relative to the devices being on the same USB bus, nor the kernel used. I've tried Debian's 2.6.32.5 and 2.6.38-bpo2 on a Debian Squeeze system. На 11.6.2011 г. 19:38 ч., Doychin Dokov написа: i've been using the patches in the latest media_tree for some hours - the S2-3650 CI seems to work. There's one thing that disturbs me, though - when it scans / locks on a frequency, another device on the same PC, using the same stb6100, gets stuck for a moment. Any ideas what might be the cause for that? The two devices do not share common RF input signal, but are on the same USB bus: Bus 001 Device 004: ID 734c:5980 TBS Technologies China Bus 001 Device 003: ID 0b48:300a TechnoTrend AG TT-connect S2-3650 CI The TBS device is a QBOX2 SI, which works with their official driver from their web-site. There's also a third DVB device in the system - a TT S-2400 (which is on the other USB bus, though), which does not exhibit any problems related with the first two devices, nor causes them to get stuck when it's scanning. I'll try to switch devices around and see if anything changes. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Request: Change to /usr/share/dvb/dvb-t/au-Sydney_North_Shore
Hi, The entry for Channel 44 in Sydney needs to be modified in order to tune in TVS in Sydney. The following changes need to be made: (update to frequency) # D44 UHF35 #T 57850 7MHz 2/3 NONE QAM64 8k 1/32 NONE T 536625000 7MHz 2/3 NONE QAM64 8k 1/32 NONE This information was derived from http://www.tvs.org.au/get-involved/tune-in Steve Ollis "There are 10 types of people in this world - Those that can count in binary, and those that can't." -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv4 PATCH 6/8] v4l2-ioctl.c: prefill tuner type for g_frequency and g/s_tuner.
Em 12-06-2011 16:41, Hans Verkuil escreveu: > On Sunday, June 12, 2011 19:08:03 Mauro Carvalho Chehab wrote: >>> I think in the longer term we need to change the spec so that: >>> >>> 1) Opening a radio node no longer switches to radio mode. Instead, you need >>> to >>>call VIDIOC_S_FREQUENCY for that. >>> 2) When VIDIOC_S_FREQUENCY the type field should match the video/radio node >>> it >>>is called on. So for /dev/radio type should be RADIO, for others it >>> should be >>>ANALOG_TV. Otherwise -EINVAL is called. >>> >>> So this might be a good feature removal for 3.2 or 3.3. >> >> I'm OK with that. > > How about this: > > diff --git a/Documentation/feature-removal-schedule.txt > b/Documentation/feature-removal-schedule.txt > index 1a9446b..9df0e09 100644 > --- a/Documentation/feature-removal-schedule.txt > +++ b/Documentation/feature-removal-schedule.txt > @@ -600,3 +600,25 @@ Why: Superseded by the UVCIOC_CTRL_QUERY ioctl. > Who: Laurent Pinchart > > > + > +What:For VIDIOC_S_FREQUENCY the type field must match the device > node's type. > + If not, return -EINVAL. > +When:3.2 > +Why: It makes no sense to switch the tuner to radio mode by calling > + VIDIOC_S_FREQUENCY on a video node, or to switch the tuner to tv mode by > + calling VIDIOC_S_FREQUENCY on a radio node. This is the first step of a > + move to more consistent handling of tv and radio tuners. > +Who: Hans Verkuil > + > + > + > +What:Opening a radio device node will no longer automatically switch > the > + tuner mode from tv to radio. > +When:3.3 > +Why: Just opening a V4L device should not change the state of the hardware > + like that. It's very unexpected and against the V4L spec. Instead, you > + switch to radio mode by calling VIDIOC_S_FREQUENCY. This is the second > + and last step of the move to consistent handling of tv and radio tuners. > +Who: Hans Verkuil > + > + > > Regards, > > Hans Seems fine to me. Thanks! Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFCv4 PATCH 1/8] tuner-core: rename check_mode to supported_mode
Em 12-06-2011 15:09, Hans Verkuil escreveu: > On Sunday, June 12, 2011 19:27:21 Mauro Carvalho Chehab wrote: >> Em 12-06-2011 13:07, Hans Verkuil escreveu: >>> On Sunday, June 12, 2011 16:37:55 Mauro Carvalho Chehab wrote: Em 12-06-2011 07:59, Hans Verkuil escreveu: > From: Hans Verkuil > > The check_mode function checks whether a mode is supported. So calling it > supported_mode is more appropriate. In addition it returned either 0 or > -EINVAL which suggests that the -EINVAL error should be passed on. > However, > that's not the case so change the return type to bool. I prefer to keep returning -EINVAL. This is the proper thing to do, and to return the result to the caller. A fixme should be added though, so, after someone add a subdev call that would properly handle the -EINVAL code for multiple tuners, the functions should return the error code instead of 0. >>> >>> No, you can't return -EINVAL here. It is the responsibility of the bridge >>> driver to determine whether there is e.g. a radio tuner. So if one of these >>> tuner subdevs is called with mode radio while it is a tv tuner, then that >>> is not an error, but instead it is a request that can safely be ignored >>> as not relevant for that tuner. It is up to the bridge driver to ensure >>> that a tuner is loaded that is actually valid for the radio mode. >>> >>> Subdev ops should only return errors when there is a real problem (e.g. i2c >>> errors) and should just return 0 if a request is not for them. >>> >>> That's why I posted these first two patches: these functions suggest that >>> you >>> can return an error if the mode doesn't match when you really cannot. >>> >>> If I call v4l2_device_call_until_err() for e.g. s_frequency, then the error >>> that is returned should match a real error (e.g. an i2c error), not that one >>> of the tv tuners refused the radio mode. I know there is a radio tuner >>> somewhere, >>> otherwise there wouldn't be a /dev/radio node. >>> >>> I firmly believe that the RFCv4 series is correct and just needs an >>> additional >>> patch adding some documentation. >>> >>> That said, it would make sense to move the first three patches to the end >>> instead if you prefer. Since these are cleanups, not bug fixes like the >>> others. >> >> >> The errors at tuner should be propagated. If there's just one tuner, the >> error >> code should just be returned by the ioctl. But, if there are two tuners, if >> the bridge >> driver calls G_TUNER (or any other tuner subdev call) and both tuners return >> -EINVAL, >> then it needs to return -EINVAL to userspace. If just one returns -EINVAL, >> and the >> other tuner returns 0, then it should return 0. So, it is about the opposite >> behaviour >> implemented at v4l2_device_call_until_err(). > > Sorry, but no, that's not true. You are trying to use the error codes from > tuner > subdevs to determine whether none of the tuner subdevs support a certain > tuner mode. Not only that. There are some cases where the tuner driver may not bind for some reason. So, even if the bridge driver does support a certain mode, a call to G_TUNER may fail (for example, if tea5767 probe failed). Only with a proper return code, the bridge driver can detect if the driver found some issue. > E.g., you want to change something for a radio tuner and there are no radio > tuner > subdevs. But that's the job of the bridge driver to check. That has the > overview, > the lowly subdevs do not. The subdevs just filter the ops and check the mode > to see > if they should handle it and ignore it otherwise. > > Only if they have to handle it will they return a possible error. The big > problem > with trying to use subdev errors codes for this is that you don't see the > difference > between a real error of a subdev (e.g. -EIO when an i2c access fails) and a > subdev > that returns -EINVAL just because it didn't understand the tuner mode. > > So the bridge may return -EINVAL to the application instead of the real > error, which > is -EIO. An -EIO would also be discarded, as errors at v4l2_device_call_all() calls don't return anything. So, currently, the bridge has to assume that no errors happened and return 0. > That's the whole principle behind the sub-device model: sub-devices do not > know > about 'the world outside'. So if you pass RADIO mode to S_FREQUENCY and there > is no > radio tuner, then the bridge driver is the one that should detect that and > return > -EINVAL. > > Actually, as mentioned before, it can also be done in video_ioctl2.c by > checking > the tuner mode against the device node it's called on. But that requires > tightening > of the V4L2 spec first. Yes, video_ioctl2 (or, currently, the bridge driver) shouldn't allow an invalid operation. But if the call returns an error, this error should be propagated. Also, as I've explained before, even adding the invalid mode check inside video_ioctl, y
Re: [PATCH] [media] af9015: setup rc keytable for LC-Power LC-USB-DVBT
In article <4df52828.2070...@iki.fi> you write: >I assume device uses vendor reference design USB ID (15a4:9016 or >15a4:9015)? > Yeah 15a4:9016. >About the repeating bug you mention, are you using latest driver >version? I am not aware such bug. There have been this kind of incorrect >behaviour old driver versions which are using HID. It was coming from >wrong HID interval. > >Also you can dump remote codes out when setting debug=2 to >dvb_usb_af9015 module. That doesn't seem to work here so maybe my version is really too old to have that fix. (But the keytable patch should still apply I guess?) Thanx, :) Juergen -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [media] af9015: setup rc keytable for LC-Power LC-USB-DVBT
On 06/13/2011 01:15 AM, Juergen Lock wrote: About the repeating bug you mention, are you using latest driver version? I am not aware such bug. There have been this kind of incorrect behaviour old driver versions which are using HID. It was coming from wrong HID interval. Also you can dump remote codes out when setting debug=2 to dvb_usb_af9015 module. That doesn't seem to work here so maybe my version is really too old to have that fix. (But the keytable patch should still apply I guess?) Could you send af9015.c file you have I can check? Your patch is OK, but I want to know why it repeats. regards Antti -- http://palosaari.fi/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
"dvb_ca adaptor 0: PC card did not respond :(" with Technotrend S2-3200
Hi all, hope you can help me this one, because there's not a whole of info about similar problems to be found. I have a Technotrend S2-3200 with CI and on three different distros I get this "dvb_ca adaptor 0: PC card did not respond :( in /var/log/messages. I guess this is the reason why I cannot see encrypted channels on Linux. Unencrypted works fine. In Windows XP the cord works fine too. I tried Opensuse 11.4, the latest Mythbuntu and the latest Mythdora. Can anyone give any hints on how to go about this? thx!!! Bart -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Add support for PCTV452E.
I also confirmed it works with s2-liplianin on Debian's 2.6.38-bpo2 kernel without such problems. Another thing I've tested is to compile the media_tree with the stb6100, but the problem persist, so it seems it's not related to the component that both devices share. Any ideas what might be the reason for that, or at least where I should focus at searching? На 13.6.2011 г. 00:38 ч., Doychin Dokov написа: On 2.6.32.5 with s2-liplianin tree this is not observed, i.e. everything works as expected - the S2-3650 does not cause the TBS device to stutter. На 12.6.2011 г. 23:27 ч., Doychin Dokov написа: The same thing happens when the devices are on different USB buses: Bus 002 Device 004: ID 0b48:300a TechnoTrend AG TT-connect S2-3650 CI Bus 001 Device 007: ID 0b48:3006 TechnoTrend AG TT-connect S-2400 DVB-S Bus 001 Device 004: ID 734c:5980 TBS Technologies China When the S2-3650 CI scans, the stream output from the TBS is gone. When it's done scanning / locking, the TBS continues to work fine. The S-2400 is not affected in any way, nor it affects the other receivers. It seems the issue is not relative to the devices being on the same USB bus, nor the kernel used. I've tried Debian's 2.6.32.5 and 2.6.38-bpo2 on a Debian Squeeze system. На 11.6.2011 г. 19:38 ч., Doychin Dokov написа: i've been using the patches in the latest media_tree for some hours - the S2-3650 CI seems to work. There's one thing that disturbs me, though - when it scans / locks on a frequency, another device on the same PC, using the same stb6100, gets stuck for a moment. Any ideas what might be the cause for that? The two devices do not share common RF input signal, but are on the same USB bus: Bus 001 Device 004: ID 734c:5980 TBS Technologies China Bus 001 Device 003: ID 0b48:300a TechnoTrend AG TT-connect S2-3650 CI The TBS device is a QBOX2 SI, which works with their official driver from their web-site. There's also a third DVB device in the system - a TT S-2400 (which is on the other USB bus, though), which does not exhibit any problems related with the first two devices, nor causes them to get stuck when it's scanning. I'll try to switch devices around and see if anything changes. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [media] af9015: setup rc keytable for LC-Power LC-USB-DVBT
On Mon, Jun 13, 2011 at 01:24:54AM +0300, Antti Palosaari wrote: > On 06/13/2011 01:15 AM, Juergen Lock wrote: > >> About the repeating bug you mention, are you using latest driver > >> version? I am not aware such bug. There have been this kind of incorrect > >> behaviour old driver versions which are using HID. It was coming from > >> wrong HID interval. > >> > >> Also you can dump remote codes out when setting debug=2 to > >> dvb_usb_af9015 module. > > > > That doesn't seem to work here so maybe my version is really too old > > to have that fix. (But the keytable patch should still apply I guess?) > > Could you send af9015.c file you have I can check? > > Your patch is OK, but I want to know why it repeats. Sent off-list. Thanx, :) Juergen -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [media] af9015: setup rc keytable for LC-Power LC-USB-DVBT
On 06/13/2011 01:34 AM, Juergen Lock wrote: On Mon, Jun 13, 2011 at 01:24:54AM +0300, Antti Palosaari wrote: On 06/13/2011 01:15 AM, Juergen Lock wrote: About the repeating bug you mention, are you using latest driver version? I am not aware such bug. There have been this kind of incorrect behaviour old driver versions which are using HID. It was coming from wrong HID interval. Also you can dump remote codes out when setting debug=2 to dvb_usb_af9015 module. That doesn't seem to work here so maybe my version is really too old to have that fix. (But the keytable patch should still apply I guess?) Could you send af9015.c file you have I can check? Your patch is OK, but I want to know why it repeats. Sent off-list. It was latest version. Still mystery why it repeats... Have you unplugged that device after booting from Windows? I wonder if there is HID remote codes uploaded to device by Windows driver and then you have "warm" booted to Linux... Anyhow, I will take your patch and add it to the af9015 driver. regards Antti -- http://palosaari.fi/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [media] af9015: setup rc keytable for LC-Power LC-USB-DVBT
On Mon, Jun 13, 2011 at 01:50:54AM +0300, Antti Palosaari wrote: > On 06/13/2011 01:34 AM, Juergen Lock wrote: > > On Mon, Jun 13, 2011 at 01:24:54AM +0300, Antti Palosaari wrote: > >> On 06/13/2011 01:15 AM, Juergen Lock wrote: > About the repeating bug you mention, are you using latest driver > version? I am not aware such bug. There have been this kind of incorrect > behaviour old driver versions which are using HID. It was coming from > wrong HID interval. > > Also you can dump remote codes out when setting debug=2 to > dvb_usb_af9015 module. > >>> > >>>That doesn't seem to work here so maybe my version is really too old > >>> to have that fix. (But the keytable patch should still apply I guess?) > >> > >> Could you send af9015.c file you have I can check? > >> > >> Your patch is OK, but I want to know why it repeats. > > > > Sent off-list. > > It was latest version. Still mystery why it repeats... Have you > unplugged that device after booting from Windows? I wonder if there is > HID remote codes uploaded to device by Windows driver and then you have > "warm" booted to Linux... > Well at least I can't rule something like that out, will send details off-list. (Btw where is debug=2 to print remote events handled in that file? Or is that done somewhere else?) > Anyhow, I will take your patch and add it to the af9015 driver. Thanx! :) Juergen -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [media] af9015: setup rc keytable for LC-Power LC-USB-DVBT
On 06/13/2011 02:01 AM, Juergen Lock wrote: On Mon, Jun 13, 2011 at 01:50:54AM +0300, Antti Palosaari wrote: On 06/13/2011 01:34 AM, Juergen Lock wrote: On Mon, Jun 13, 2011 at 01:24:54AM +0300, Antti Palosaari wrote: On 06/13/2011 01:15 AM, Juergen Lock wrote: About the repeating bug you mention, are you using latest driver version? I am not aware such bug. There have been this kind of incorrect behaviour old driver versions which are using HID. It was coming from wrong HID interval. Also you can dump remote codes out when setting debug=2 to dvb_usb_af9015 module. That doesn't seem to work here so maybe my version is really too old to have that fix. (But the keytable patch should still apply I guess?) Could you send af9015.c file you have I can check? Your patch is OK, but I want to know why it repeats. Sent off-list. It was latest version. Still mystery why it repeats... Have you unplugged that device after booting from Windows? I wonder if there is HID remote codes uploaded to device by Windows driver and then you have "warm" booted to Linux... Well at least I can't rule something like that out, will send details off-list. (Btw where is debug=2 to print remote events handled in that file? Or is that done somewhere else?) Few words about AF9015 remote. Chipset implements HID remote (~keyboard) which is used normally. Driver uploads HID mappings (remote keycode & keyboard keycode) to the chipset memory and chipset then outputs remote events as HID without driver help. But there seems to be bug in chipset which sets HID polling interval too short. Due to that interval Linux HID starts repeating keycodes. There is some quirks added to the HID drivers for that which are mapped device USB ID. Quirk prints to log: "Afatech DVB-T 2: Fixing fullspeed to highspeed interval: 10 -> 7" Due to that bug and inflexible remote configuration of HID remote I implemented new way. Current code does not upload HID codes to chipset at all which makes HID remote as disabled. Instead, remote codes are read by polling directly from the chip memory. And it is very first time I hear this new method goes repeating loop. Drivers write to the system log. Typically it is called messages, message.log, syslog, etc. in /var/log/ directory. There is dmesg command which outputs same info. regards, Antti -- http://palosaari.fi/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [media] af9015: setup rc keytable for LC-Power LC-USB-DVBT
On Mon, Jun 13, 2011 at 02:46:10AM +0300, Antti Palosaari wrote: > On 06/13/2011 02:01 AM, Juergen Lock wrote: > > On Mon, Jun 13, 2011 at 01:50:54AM +0300, Antti Palosaari wrote: > >> On 06/13/2011 01:34 AM, Juergen Lock wrote: > >>> On Mon, Jun 13, 2011 at 01:24:54AM +0300, Antti Palosaari wrote: > On 06/13/2011 01:15 AM, Juergen Lock wrote: > >> About the repeating bug you mention, are you using latest driver > >> version? I am not aware such bug. There have been this kind of > >> incorrect > >> behaviour old driver versions which are using HID. It was coming from > >> wrong HID interval. > >> > >> Also you can dump remote codes out when setting debug=2 to > >> dvb_usb_af9015 module. > > > > That doesn't seem to work here so maybe my version is really too old > > to have that fix. (But the keytable patch should still apply I guess?) > > Could you send af9015.c file you have I can check? > > Your patch is OK, but I want to know why it repeats. > >>> > >>> Sent off-list. > >> > >> It was latest version. Still mystery why it repeats... Have you > >> unplugged that device after booting from Windows? I wonder if there is > >> HID remote codes uploaded to device by Windows driver and then you have > >> "warm" booted to Linux... > >> > > Well at least I can't rule something like that out, will send details > > off-list. (Btw where is debug=2 to print remote events handled in that > > file? Or is that done somewhere else?) > > Few words about AF9015 remote. Chipset implements HID remote (~keyboard) > which is used normally. Driver uploads HID mappings (remote keycode & > keyboard keycode) to the chipset memory and chipset then outputs remote > events as HID without driver help. But there seems to be bug in chipset > which sets HID polling interval too short. Due to that interval Linux > HID starts repeating keycodes. There is some quirks added to the HID > drivers for that which are mapped device USB ID. Quirk prints to log: > "Afatech DVB-T 2: Fixing fullspeed to highspeed interval: 10 -> 7" > > Due to that bug and inflexible remote configuration of HID remote I > implemented new way. Current code does not upload HID codes to chipset > at all which makes HID remote as disabled. Instead, remote codes are > read by polling directly from the chip memory. And it is very first time > I hear this new method goes repeating loop. > > Drivers write to the system log. Typically it is called messages, > message.log, syslog, etc. in /var/log/ directory. There is dmesg command > which outputs same info. Ah I needed CONFIG_DVB_USB_DEBUG. :) Here is what I see when it happens, first ir-keytable -t: [...] 1307924961.676720: event sync 1307924961.676723: event key down: KEY_STOP (0x0080) 1307924961.676725: event sync 1307924962.193037: event MSC: scancode = 0e 1307924962.712487: event MSC: scancode = 0e 1307924963.232184: event MSC: scancode = 0e 1307924964.791025: event MSC: scancode = 1a 1307924964.791034: event key up: KEY_STOP (0x0080) 1307924964.791037: event sync 1307924964.791040: event key down: KEY_PAUSE (0x0077) 1307924964.791042: event sync 1307924965.310734: event MSC: scancode = 1a 1307924966.867192: event MSC: scancode = 54 1307924966.867201: event key up: KEY_PAUSE (0x0077) 1307924966.867203: event sync 1307924966.867207: event key down: KEY_0 (0x000b) 1307924966.867209: event sync 1307924971.538731: event MSC: scancode = 02 1307924971.538741: event key up: KEY_0 (0x000b) 1307924971.538743: event sync 1307924971.538746: event key down: KEY_VOLUMEDOWN (0x0072) 1307924971.538748: event sync 1307924974.135096: event MSC: scancode = 40 1307924974.135104: event key up: KEY_VOLUMEDOWN (0x0072) 1307924974.135106: event sync 1307924974.135110: event key down: KEY_VOLUMEUP (0x0073) 1307924974.135112: event sync 1307924974.653804: event MSC: scancode = 40 1307924975.172874: event MSC: scancode = 40 1307924975.692828: event MSC: scancode = 40 1307924976.212909: event MSC: scancode = 40 1307924976.732230: event MSC: scancode = 40 1307924977.252420: event MSC: scancode = 40 1307924977.772252: event MSC: scancode = 40 1307924978.289195: event MSC: scancode = 40 1307924978.808266: event MSC: scancode = 40 1307924979.327347: event MSC: scancode = 40 1307924979.846548: event MSC: scancode = 40 1307924980.364610: event MSC: scancode = 40 1307924980.884690: event MSC: scancode = 40 1307924981.401760: event MSC: scancode = 40 1307924981.919843: event MSC: scancode = 40 1307924982.437907: event MSC: scancode = 40 1307924982.953985: event MSC: scancode = 40 1307924983.475070: event MSC: scancode = 40 1307924983.995011: event MSC: scancode = 40 1307924984.513089: event MSC: scancode = 40 1307924985.032154: event MSC: scancode = 40 1307924985.552225: event MSC: scancode = 40 1307924986.072310: event MSC: scancode = 40 1307924986.591383: event MSC: scancode = 40 1307924987.111452: event MSC: scancode = 40 1307924987.630544: event MSC: scancod
Re: Improving kernel -> userspace (usbfs) usb device hand off
On Mon, Jun 13, 2011 at 5:20 AM, Theodore Kilgore wrote: > On Sun, 12 Jun 2011, Hans de Goede wrote: >> Actually libusb and libgphoto have been using the rebind orginal driver >> functionality of the code for quite a while now, > > Oh? I can see that libusb is doing that, and I can also see that there is > a "public" function for _unbinding_ a kernel driver, namely > > int usb_detach_kernel_driver_np() > > found in usb.h > > and it is used in libgphoto, as well. > > I am not sure that there is any corresponding rebind function which is > public. Is it perhaps > > int usb_get_driver_np() > > ??? > > By context (looking at libgphoto2-port/usb/libusb.c) I would think that > this function is not the rebind function, but is only checking whether or > not there is any potential conflict with a kernel driver. If I am right, > then where is the publicly exported rebind function, and where does it > currently get used in libgphoto2? http://gphoto.svn.sourceforge.net/viewvc/gphoto/trunk/libgphoto2/libgphoto2_port/usb/libusb.c?revision=13652&view=markup The rebind happened under the function "static int gp_port_usb_close (GPPort *port)". Since libgphoto2 is still using libusb-0.1, the unbind is using usbfs IOCTL directly (USBDEVFS_CONNECT). > So frankly after my eagerness yesterday I do not see how it can easily be > made to work, after all. > >> unfortunately this >> does not solve the problem, unless we somehow move to 1 central >> coordinator for the device the user experience will stay subpar. Now I understand what Hans is saying. It will be a lot of work trying to sort out this issue in userspace. What can be the single central coordinator? A device manager applet listing the program or service which hold the device? >> Example, user downloads pictures from the camera using shotwell, >> gthumb, fspot or whatever, keeps the app in question open and the app >> in question keeps the gphoto2 device handle open. >> >> User wants to do some skyping with video chat, skype complains it >> cannot find the device, since the kernel driver currently is unbound. >> >> -> Poor user experience. > > Poor user experience, or merely poor user? The user ought to know better. > Of course, I do agree that there are lots of such people, and it is a good > idea to try to put up warning signs. It is difficult to call the users "poor users" in this case. Since they may not know that the other open program is holding the device. Some warning message may help, not "I can not find the device" though. It would be better to pinpoint which program is holding the device and then ask the user to close that program. I understand this is easily said than done... Similar experiences for Windows about the serial port, sometimes it is difficult for the user to know that some program or service are holding the serial port so that the other program or will fail or Windows complain that it is still open when you want to undock the computer. >> >> With having both functions in the kernel, the kernel could actually >> allow skype to use the dual mode cameras as video source, and if >> the user then were to switch to f-spot and try to import more photo's >> then he will get an -ebusy in f-spot. If he finishes skyping and >> then returns to f-spot everything will just continue working. >> >> This is the kind of "seamless" user experience I'm aiming for here. >> > Yes, I can see where you are coming from. But if the camera really will > not let you run skype and fspot at the same time, which I do not believe > it would allow on _any_ operating system, then each app should give an > error message which says it cannot be run unless and until the other app > has been closed. If that has to happen at the kernel level, then OK. > Yes. From what I read, to solve it in kernel or to solve it in user space are both a lot of work. Personally I tend to think to solve it in user space is more feasible. -- Xiaofan -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Libusb-devel] Improving kernel -> userspace (usbfs) usb device hand off
On Jun 12, 2011, at 7:03 PM, Xiaofan Chen wrote: On Mon, Jun 13, 2011 at 5:20 AM, Theodore Kilgore wrote: On Sun, 12 Jun 2011, Hans de Goede wrote: Actually libusb and libgphoto have been using the rebind orginal driver functionality of the code for quite a while now, Oh? I can see that libusb is doing that, and I can also see that there is a "public" function for _unbinding_ a kernel driver, namely int usb_detach_kernel_driver_np() found in usb.h and it is used in libgphoto, as well. I am not sure that there is any corresponding rebind function which is public. Is it perhaps int usb_get_driver_np() ??? By context (looking at libgphoto2-port/usb/libusb.c) I would think that this function is not the rebind function, but is only checking whether or not there is any potential conflict with a kernel driver. If I am right, then where is the publicly exported rebind function, and where does it currently get used in libgphoto2? http://gphoto.svn.sourceforge.net/viewvc/gphoto/trunk/libgphoto2/libgphoto2_port/usb/libusb.c?revision=13652&view=markup The rebind happened under the function "static int gp_port_usb_close (GPPort *port)". Since libgphoto2 is still using libusb-0.1, the unbind is using usbfs IOCTL directly (USBDEVFS_CONNECT). So frankly after my eagerness yesterday I do not see how it can easily be made to work, after all. unfortunately this does not solve the problem, unless we somehow move to 1 central coordinator for the device the user experience will stay subpar. Now I understand what Hans is saying. It will be a lot of work trying to sort out this issue in userspace. What can be the single central coordinator? A device manager applet listing the program or service which hold the device? Something like that. Have a user-space device allocation mechanism so that apps are not constrained by a particular interface semantic (i.e. not required to open /dev/video and have a kernel driver deliver pixels to the apps). Or hid that behind a library API and let instances of the library coordinate with each other; as long as an app uses the documented public library API to access a webcam, then everyone will play fair with each other. Look at the mess that this userspace/kernel driver issue has for code in an application like gphoto - that's insane that a photo manipulation app needs to know anything about userspace/kernel switching! Example, user downloads pictures from the camera using shotwell, gthumb, fspot or whatever, keeps the app in question open and the app in question keeps the gphoto2 device handle open. User wants to do some skyping with video chat, skype complains it cannot find the device, since the kernel driver currently is unbound. -> Poor user experience. Poor user experience, or merely poor user? The user ought to know better. Of course, I do agree that there are lots of such people, and it is a good idea to try to put up warning signs. It is difficult to call the users "poor users" in this case. Since they may not know that the other open program is holding the device. Some warning message may help, not "I can not find the device" though. It would be better to pinpoint which program is holding the device and then ask the user to close that program. I understand this is easily said than done... Similar experiences for Windows about the serial port, sometimes it is difficult for the user to know that some program or service are holding the serial port so that the other program or will fail or Windows complain that it is still open when you want to undock the computer. With having both functions in the kernel, the kernel could actually allow skype to use the dual mode cameras as video source, and if the user then were to switch to f-spot and try to import more photo's then he will get an -ebusy in f-spot. If he finishes skyping and then returns to f-spot everything will just continue working. This is the kind of "seamless" user experience I'm aiming for here. Yes, I can see where you are coming from. But if the camera really will not let you run skype and fspot at the same time, which I do not believe it would allow on _any_ operating system, then each app should give an error message which says it cannot be run unless and until the other app has been closed. If that has to happen at the kernel level, then OK. Yes. From what I read, to solve it in kernel or to solve it in user space are both a lot of work. Yes and a kernel-based solution locks you into a kernel-based webcam driver paradigm, or an even uglier loopback driver. Personally I tend to think to solve it in user space is more feasible. I agree. mike -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] OMAP_VOUT: Create separate file for VRFB related API's
Hi Vaibhav, On Friday 27 May 2011 12:31 PM, Taneja, Archit wrote: Introduce omap_vout_vrfb.c and omap_vout_vrfb.h, for all VRFB related API's, making OMAP_VOUT driver independent from VRFB. This is required for OMAP4 DSS, since OMAP4 doesn't have VRFB block. Added new enum vout_rotation_type and "rotation_type" member to omapvideo_info, this is initialized based on the arch type in omap_vout_probe. The rotation_type var is now used to choose between vrfb and non-vrfb calls. Any comments on this patch? Thanks, Archit -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/2] OMAP_VOUT: Create separate file for VRFB related API's
> -Original Message- > From: Taneja, Archit > Sent: Monday, June 13, 2011 10:17 AM > To: Hiremath, Vaibhav > Cc: linux-media@vger.kernel.org > Subject: Re: [PATCH 2/2] OMAP_VOUT: Create separate file for VRFB related > API's > > Hi Vaibhav, > > On Friday 27 May 2011 12:31 PM, Taneja, Archit wrote: > > Introduce omap_vout_vrfb.c and omap_vout_vrfb.h, for all VRFB related > API's, > > making OMAP_VOUT driver independent from VRFB. This is required for > OMAP4 DSS, > > since OMAP4 doesn't have VRFB block. > > > > Added new enum vout_rotation_type and "rotation_type" member to > omapvideo_info, > > this is initialized based on the arch type in omap_vout_probe. The > rotation_type > > var is now used to choose between vrfb and non-vrfb calls. > > Any comments on this patch? > Archit, Last week I had to park this due to some high priority issue, today I am going to validate these patches and will respond you. Code implementation point of view, this patch looks ok. And I believe you will incorporate my comments on first patch. Thanks, Vaibhav > > > Thanks, > Archit -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: vb2: about vb2_queue->queued_count
Hello, On Friday, June 10, 2011 6:29 PM Uwe Kleine-König wrote: > On Fri, Jun 10, 2011 at 01:50:37PM +0200, Marek Szyprowski wrote: > > Hello, > > > > On Wednesday, June 08, 2011 10:48 PM Uwe Kleine-König wrote: > > > > > I'm still debugging my new video overlay device driver. The current > > > problem is again when playing back a second video. > > > > > > After streamoff is called at the end of the first video, I disable the > > > overlay and call vb2_buffer_done on the last buffer. This is exited > > > early because vb->state == VB2_BUF_STATE_DEQUEUED. > > > This results in vb->vb2_queue->queued_count being 1. > > > > > > Now if the new video starts I call vb2_queue_init in > the .vidioc_reqbufs > > > callback on my queue (that still has queued_count == 1). After > > > vb2_queue_init returns queued_count is still 1 though q->queued_list is > > > reset to be empty. > > > > > > __vb2_queue_cancel has a similar problem, &q->queued_list is reset, but > > > queued_count is not. > > > > Thanks again for finding the bug. You are right, __vb2_queue_cancel > should > > reset queued_count too. I will post a patch soon. > IMHO vb2_queue_init should reset queued_count, too. Not sure if you just > skipped to mention it here vb2_queue_init assumes that the called allocated vb2_queue with kzalloc() or did memset(q, 0, sizeof(struct vb2_queue)), so it is not really required to explicitly set queued_count to zero. Best regards -- Marek Szyprowski Samsung Poland R&D Center -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: soc_camera_set_fmt in soc_camera_open
On Mon, 13 Jun 2011, Kassey Lee wrote: > On Fri, Jun 10, 2011 at 5:16 PM, Guennadi Liakhovetski < > g.liakhovet...@gmx.de> wrote: > > > On Fri, 10 Jun 2011, Kassey Lee wrote: > > > > > hi, Guennadi: > > > > > > in drivers/media/video/soc_camera.c > > > static int soc_camera_open(struct file *file) > > > > > > it will call soc_camera_set_fmt to configure the sensor and host > > controller. > > > for sensor, this means it will trigger download setting, this may take > > quite > > > time through i2c or SPI. > > > I complain about this, because after we open, request, s_param, S_FMT, > > > DQBUF, > > > in S_FMT, we will download the setting again. > > > > > > how do you think ? > > > > If it's a concern for you, you might consider moving most of your sensor > > set up from .s_(mbus_)fmt() to .s_stream(). Would that solve your problem? > > > > .s_stream can not pass the fmt info, because we need download different > setting depends on the format(UYVY, resolution, JPEG) Of course it cannot. You would have to store it in your .s_(mbus_)fmt() method and use during .s_stream(). > what I can figure out in open is power up sensor, why we need to .s_fmt() in > open ? The ideas behind this decision were to (1) make it simple for the drivers to comply with the specification's requirements to preserve the configuration across close() / open() calls, to be able to start streaming data directly after open() without any S_FMT ioctl()s, and to allow the hardware to be powered down when unused, and to (2) initialise internal format translation tables and other data. We could try to slightly optimise this by only calling soc_camera_set_fmt() during STREAMON _if_ no S_FMT has been called explicitly. But that would be a larger change, than just adjusting your driver. You could do another optimisation yourself too: if the new configuration is equal to the currently configured one, you don't issue any i2c commands. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] OMAP_VOUT: Create separate file for VRFB related API's
On Monday 13 June 2011 10:13 AM, Hiremath, Vaibhav wrote: -Original Message- From: Taneja, Archit Sent: Monday, June 13, 2011 10:17 AM To: Hiremath, Vaibhav Cc: linux-media@vger.kernel.org Subject: Re: [PATCH 2/2] OMAP_VOUT: Create separate file for VRFB related API's Hi Vaibhav, On Friday 27 May 2011 12:31 PM, Taneja, Archit wrote: Introduce omap_vout_vrfb.c and omap_vout_vrfb.h, for all VRFB related API's, making OMAP_VOUT driver independent from VRFB. This is required for OMAP4 DSS, since OMAP4 doesn't have VRFB block. Added new enum vout_rotation_type and "rotation_type" member to omapvideo_info, this is initialized based on the arch type in omap_vout_probe. The rotation_type var is now used to choose between vrfb and non-vrfb calls. Any comments on this patch? Archit, Last week I had to park this due to some high priority issue, today I am going to validate these patches and will respond you. Code implementation point of view, this patch looks ok. And I believe you will incorporate my comments on first patch. Oh okay, great. Thanks, Archit -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
