Em 15-01-2012 22:16, Mauro Carvalho Chehab escreveu: > Em 15-01-2012 19:47, Antti Palosaari escreveu: >> On 01/15/2012 11:08 PM, Mauro Carvalho Chehab wrote: >>> There was a bug at the error code handling on dvb-fe-tool: basically, if it can't open >>> a device, it were using a NULL pointer. It was likely fixed by this commit: >>> >>> http://git.linuxtv.org/v4l-utils.git/commit/1f669eed5433d17df4d8fb1fa43d2886f99d3991 >> >> That bug was fixed as I tested. >> >> But could you tell why dvb-fe-tool --set-delsys=DVBC/ANNEX_A calls get_frontent() ? > > That's what happens here, at the application level: > > $ strace dvb-fe-tool -d DVBC/ANNEX_A > > open("/dev/dvb/adapter0/frontend0", O_RDWR) = 3 > ioctl(3, FE_GET_INFO, 0xb070c4) = 0 > fstat(1, {st_mode=S_IFCHR|0620, st_rdev=makedev(136, 2), ...}) = 0 > mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f37922be000 > write(1, "Device DRXK DVB-C DVB-T (/dev/dv"..., 68Device DRXK DVB-C DVB-T (/dev/dvb/adapter0/frontend0) capabilities: > ) = 68 > write(1, "\tCAN_FEC_1_2 CAN_FEC_2_3 CAN_FEC"..., 245 CAN_FEC_1_2 CAN_FEC_2_3 CAN_FEC_3_4 CAN_FEC_5_6 CAN_FEC_7_8 CAN_FEC_AUTO CAN_GUARD_INTERVAL_AUTO CAN_HIERARCHY_AUTO CAN_INVERSION_AUTO CAN_MUTE_TS CAN_QAM_16 CAN_QAM_32 CAN_QAM_64 CAN_QAM_128 CAN_QAM_256 CAN_RECOVER CAN_TRANSMISSION_MODE_AUTO > ) = 245 > ioctl(3, FE_GET_PROPERTY, 0x7fff326ce310) = 0 > write(1, "DVB API Version 5.5, Current v5 "..., 54DVB API Version 5.5, Current v5 delivery system: DVBT > ) = 54 > ioctl(3, FE_GET_PROPERTY, 0x7fff326ce310) = 0 > write(1, "Supported delivery systems: DVBC"..., 62Supported delivery systems: DVBC/ANNEX_A DVBC/ANNEX_C [DVBT] > ) = 62 > write(1, "Changing delivery system to: DVB"..., 42Changing delivery system to: DVBC/ANNEX_A > ) = 42 > ioctl(3, FE_SET_PROPERTY, 0x7fff326ce340) = 0 > close(3) = 0 > exit_group(0) = ? > > > The first FE_GET_PROPERTY reads the DVB API version and the current delivery > system: > > parms->dvb_prop[0].cmd = DTV_API_VERSION; > parms->dvb_prop[1].cmd = DTV_DELIVERY_SYSTEM; > > dtv_prop.num = 2; > dtv_prop.props = parms->dvb_prop; > > /* Detect a DVBv3 device */ > if (ioctl(fd, FE_GET_PROPERTY, &dtv_prop) == -1) { > parms->dvb_prop[0].u.data = 0x300; > parms->dvb_prop[1].u.data = SYS_UNDEFINED; > } > parms->version = parms->dvb_prop[0].u.data; > parms->current_sys = parms->dvb_prop[1].u.data; > > The second FE_GET_PROPERTY is used only if DVB API v5.5 or upper is detected, > and does: > > parms->dvb_prop[0].cmd = DTV_ENUM_DELSYS; > parms->n_props = 1; > dtv_prop.num = 1; > dtv_prop.props = parms->dvb_prop; > if (ioctl(fd, FE_GET_PROPERTY, &dtv_prop) == -1) { > perror("FE_GET_PROPERTY"); > dvb_v5_free(parms); > close(fd); > return NULL; > } > > Both were called inside dvb_fe_open(). > > The FE_SET_PROPERTY changes the property inside the DVBv5 cache, > at dvb_set_sys(): > > dvb_prop[0].cmd = DTV_DELIVERY_SYSTEM; > dvb_prop[0].u.data = sys; > prop.num = 1; > prop.props = dvb_prop; > > if (ioctl(parms->fd, FE_SET_PROPERTY, &prop) == -1) { > perror("Set delivery system"); > return errno; > } > > The FE_SET_PROPERTY doesn't call a DTV_TUNE, so it shouldn't be calling the > set frontend methods inside the driver. > > So, from the userspace applications standpoint, I'm not seeing anything wrong. > >> That will cause this kind of calls in demod driver: >> init() >> get_frontend() >> get_frontend() >> sleep() >> >> My guess is that it resolves current delivery system. But as demod is usually sleeping (not tuned) at that phase it does not know frontend settings asked, like modulation etc. In case of cxd2820r those are available after set_frontend() call. I think I will add check and return -EINVAL in that case. > > > What seems to be happening at dvb-core/dvb_frontend.h is due to this code: > > if(cmd == FE_GET_PROPERTY) { > ... > /* > * Fills the cache out struct with the cache contents, plus > * the data retrieved from get_frontend. > */ > dtv_get_frontend(fe, NULL); > for (i = 0; i < tvps->num; i++) { > err = dtv_property_process_get(fe, c, tvp + i, file); > if (err < 0) > goto out; > (tvp + i)->result = err; > } > > E. g. even if the FE_GET_PROPERTY is only reading the DVB version and calling > DTV_ENUM_DELSYS, it is calling the dtv_get_frontend() to retrieve more data. > > What it can be done is to do something like: > > static bool need_get_frontend() > { > ... > for (i = 0; i < tvps->num; i++) > ... > } > > if (need_get_frontend(tvps)) > dtv_get_frontend(fe, NULL); > for (i = 0; i < tvps->num; i++) { > err = dtv_property_process_get(fe, c, tvp + i, file); > if (err < 0) > goto out; > (tvp + i)->result = err; > } > > And add some logic inside need_get_frontend() to return false if the > FE_GET_PROPERTY only wants static info, like DTV_ENUM_DELSYS, DTV_VERSION > and DTV_DELIVERY_SYSTEM. The enclosed patch should do the trick - [PATCH RFC] Don't call get_frontend() if not needed If the frontend is in idle state, or a FE_GET_PROPERTY is called for reading the enumsys, api version or delivery system, don't call the frontend, as it is not needed. Signed-off-by: Mauro Carvalho Chehab <mchehab@xxxxxxxxxx> PS.: There's one extra printk here for test purposes. Of course, this should be removed at the final version. diff --git a/drivers/media/dvb/dvb-core/dvb_frontend.c b/drivers/media/dvb/dvb-core/dvb_frontend.c index f5fa7aa..3c80c92 100644 --- a/drivers/media/dvb/dvb-core/dvb_frontend.c +++ b/drivers/media/dvb/dvb-core/dvb_frontend.c @@ -1234,6 +1234,8 @@ static int dtv_get_frontend(struct dvb_frontend *fe, { int r; +printk("%s()\n", __func__); + if (fe->ops.get_frontend) { r = fe->ops.get_frontend(fe); if (unlikely(r < 0)) @@ -1739,12 +1741,35 @@ static int dvb_frontend_ioctl(struct file *file, return err; } +static bool need_get_frontend_call(struct dtv_properties *tvps, + struct dtv_property *tvp) +{ + int i; + + /* + * If the DTV command is just informational or cache read, + * don't bother to call the frontend to handle. + */ + for (i = 0; i < tvps->num; i++) { + switch(tvp->cmd) { + case DTV_ENUM_DELSYS: + case DTV_DELIVERY_SYSTEM: + case DTV_API_VERSION: + break; + default: + return true; + } + } + return false; +} + static int dvb_frontend_ioctl_properties(struct file *file, unsigned int cmd, void *parg) { struct dvb_device *dvbdev = file->private_data; struct dvb_frontend *fe = dvbdev->priv; struct dtv_frontend_properties *c = &fe->dtv_property_cache; + struct dvb_frontend_private *fepriv = fe->frontend_priv; int err = 0; struct dtv_properties *tvps = NULL; @@ -1812,7 +1837,8 @@ static int dvb_frontend_ioctl_properties(struct file *file, * Fills the cache out struct with the cache contents, plus * the data retrieved from get_frontend. */ - dtv_get_frontend(fe, NULL); + if (need_get_frontend_call(tvps, tvp) && fepriv->state != FESTATE_IDLE) + dtv_get_frontend(fe, NULL); for (i = 0; i < tvps->num; i++) { err = dtv_property_process_get(fe, c, tvp + i, file); if (err < 0) -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
