| Message ID | 1362915635-5431-1-git-send-email-fschaefer.oss@googlemail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Am 10.03.2013 12:40, schrieb Frank Schäfer: > Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com> > --- > drivers/media/pci/bt8xx/bttv-driver.c | 22 +++++++++++----------- > 1 Datei geändert, 11 Zeilen hinzugefügt(+), 11 Zeilen entfernt(-) > > diff --git a/drivers/media/pci/bt8xx/bttv-driver.c b/drivers/media/pci/bt8xx/bttv-driver.c > index 8610b6a..2c09bc5 100644 > --- a/drivers/media/pci/bt8xx/bttv-driver.c > +++ b/drivers/media/pci/bt8xx/bttv-driver.c > @@ -992,21 +992,20 @@ static char *audio_modes[] = { > static int > audio_mux(struct bttv *btv, int input, int mute) > { > - int gpio_val, signal; > + int gpio_val, signal, mute_gpio; > struct v4l2_ctrl *ctrl; > > gpio_inout(bttv_tvcards[btv->c.type].gpiomask, > bttv_tvcards[btv->c.type].gpiomask); > signal = btread(BT848_DSTATUS) & BT848_DSTATUS_HLOC; > > - btv->mute = mute; > btv->audio = input; > > /* automute */ > - mute = mute || (btv->opt_automute && (!signal || !btv->users) > + mute_gpio = mute || (btv->opt_automute && (!signal || !btv->users) > && !btv->has_radio_tuner); > > - if (mute) > + if (mute_gpio) > gpio_val = bttv_tvcards[btv->c.type].gpiomute; > else > gpio_val = bttv_tvcards[btv->c.type].gpiomux[input]; > @@ -1022,7 +1021,7 @@ audio_mux(struct bttv *btv, int input, int mute) > } > > if (bttv_gpio) > - bttv_gpio_tracking(btv, audio_modes[mute ? 4 : input]); > + bttv_gpio_tracking(btv, audio_modes[mute_gpio ? 4 : input]); > if (in_interrupt()) > return 0; > > @@ -1031,7 +1030,7 @@ audio_mux(struct bttv *btv, int input, int mute) > > ctrl = v4l2_ctrl_find(btv->sd_msp34xx->ctrl_handler, V4L2_CID_AUDIO_MUTE); > if (ctrl) > - v4l2_ctrl_s_ctrl(ctrl, btv->mute); > + v4l2_ctrl_s_ctrl(ctrl, mute); > > /* Note: the inputs tuner/radio/extern/intern are translated > to msp routings. This assumes common behavior for all msp3400 > @@ -1080,7 +1079,7 @@ audio_mux(struct bttv *btv, int input, int mute) > ctrl = v4l2_ctrl_find(btv->sd_tvaudio->ctrl_handler, V4L2_CID_AUDIO_MUTE); > > if (ctrl) > - v4l2_ctrl_s_ctrl(ctrl, btv->mute); > + v4l2_ctrl_s_ctrl(ctrl, mute); > v4l2_subdev_call(btv->sd_tvaudio, audio, s_routing, > input, 0, 0); > } > @@ -1088,7 +1087,7 @@ audio_mux(struct bttv *btv, int input, int mute) > ctrl = v4l2_ctrl_find(btv->sd_tda7432->ctrl_handler, V4L2_CID_AUDIO_MUTE); > > if (ctrl) > - v4l2_ctrl_s_ctrl(ctrl, btv->mute); > + v4l2_ctrl_s_ctrl(ctrl, mute); > } > return 0; > } > @@ -1300,6 +1299,7 @@ static int bttv_s_ctrl(struct v4l2_ctrl *c) > break; > case V4L2_CID_AUDIO_MUTE: > audio_mute(btv, c->val); > + btv->mute = c->val; > break; > case V4L2_CID_AUDIO_VOLUME: > btv->volume_gpio(btv, c->val); > @@ -3062,8 +3062,7 @@ static int bttv_open(struct file *file) > sizeof(struct bttv_buffer), > fh, &btv->lock); > set_tvnorm(btv,btv->tvnorm); > - set_input(btv, btv->input, btv->tvnorm); > - > + set_input(btv, btv->input, btv->tvnorm); /* also (un)mutes audio */ > > /* The V4L2 spec requires one global set of cropping parameters > which only change on request. These are stored in btv->crop[1]. > @@ -3124,7 +3123,7 @@ static int bttv_release(struct file *file) > bttv_field_count(btv); > > if (!btv->users) > - audio_mute(btv, btv->mute); > + audio_mute(btv, 1); > Btw, what about the interaction between the video and the radio device node ? With the current code it is possible to open the video and the radio device node at the same time, so I wonder if we need an additional patch changing this to if (!btv->users && !btv->radio_user) Regards, Frank > v4l2_fh_del(&fh->fh); > v4l2_fh_exit(&fh->fh); > @@ -4209,6 +4208,7 @@ static int bttv_probe(struct pci_dev *dev, const struct pci_device_id *pci_id) > btv->std = V4L2_STD_PAL; > init_irqreg(btv); > v4l2_ctrl_handler_setup(hdl); > + audio_mute(btv, 1); > > if (hdl->error) { > result = hdl->error; -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun March 10 2013 12:40:34 Frank Schäfer wrote: > Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com> Could you describe exactly what bug is fixed? I tested it a fair amount when I created my bttv patches, so I'd like to know what I missed. Regards, Hans > --- > drivers/media/pci/bt8xx/bttv-driver.c | 22 +++++++++++----------- > 1 Datei geändert, 11 Zeilen hinzugefügt(+), 11 Zeilen entfernt(-) > > diff --git a/drivers/media/pci/bt8xx/bttv-driver.c b/drivers/media/pci/bt8xx/bttv-driver.c > index 8610b6a..2c09bc5 100644 > --- a/drivers/media/pci/bt8xx/bttv-driver.c > +++ b/drivers/media/pci/bt8xx/bttv-driver.c > @@ -992,21 +992,20 @@ static char *audio_modes[] = { > static int > audio_mux(struct bttv *btv, int input, int mute) > { > - int gpio_val, signal; > + int gpio_val, signal, mute_gpio; > struct v4l2_ctrl *ctrl; > > gpio_inout(bttv_tvcards[btv->c.type].gpiomask, > bttv_tvcards[btv->c.type].gpiomask); > signal = btread(BT848_DSTATUS) & BT848_DSTATUS_HLOC; > > - btv->mute = mute; > btv->audio = input; > > /* automute */ > - mute = mute || (btv->opt_automute && (!signal || !btv->users) > + mute_gpio = mute || (btv->opt_automute && (!signal || !btv->users) > && !btv->has_radio_tuner); > > - if (mute) > + if (mute_gpio) > gpio_val = bttv_tvcards[btv->c.type].gpiomute; > else > gpio_val = bttv_tvcards[btv->c.type].gpiomux[input]; > @@ -1022,7 +1021,7 @@ audio_mux(struct bttv *btv, int input, int mute) > } > > if (bttv_gpio) > - bttv_gpio_tracking(btv, audio_modes[mute ? 4 : input]); > + bttv_gpio_tracking(btv, audio_modes[mute_gpio ? 4 : input]); > if (in_interrupt()) > return 0; > > @@ -1031,7 +1030,7 @@ audio_mux(struct bttv *btv, int input, int mute) > > ctrl = v4l2_ctrl_find(btv->sd_msp34xx->ctrl_handler, V4L2_CID_AUDIO_MUTE); > if (ctrl) > - v4l2_ctrl_s_ctrl(ctrl, btv->mute); > + v4l2_ctrl_s_ctrl(ctrl, mute); > > /* Note: the inputs tuner/radio/extern/intern are translated > to msp routings. This assumes common behavior for all msp3400 > @@ -1080,7 +1079,7 @@ audio_mux(struct bttv *btv, int input, int mute) > ctrl = v4l2_ctrl_find(btv->sd_tvaudio->ctrl_handler, V4L2_CID_AUDIO_MUTE); > > if (ctrl) > - v4l2_ctrl_s_ctrl(ctrl, btv->mute); > + v4l2_ctrl_s_ctrl(ctrl, mute); > v4l2_subdev_call(btv->sd_tvaudio, audio, s_routing, > input, 0, 0); > } > @@ -1088,7 +1087,7 @@ audio_mux(struct bttv *btv, int input, int mute) > ctrl = v4l2_ctrl_find(btv->sd_tda7432->ctrl_handler, V4L2_CID_AUDIO_MUTE); > > if (ctrl) > - v4l2_ctrl_s_ctrl(ctrl, btv->mute); > + v4l2_ctrl_s_ctrl(ctrl, mute); > } > return 0; > } > @@ -1300,6 +1299,7 @@ static int bttv_s_ctrl(struct v4l2_ctrl *c) > break; > case V4L2_CID_AUDIO_MUTE: > audio_mute(btv, c->val); > + btv->mute = c->val; > break; > case V4L2_CID_AUDIO_VOLUME: > btv->volume_gpio(btv, c->val); > @@ -3062,8 +3062,7 @@ static int bttv_open(struct file *file) > sizeof(struct bttv_buffer), > fh, &btv->lock); > set_tvnorm(btv,btv->tvnorm); > - set_input(btv, btv->input, btv->tvnorm); > - > + set_input(btv, btv->input, btv->tvnorm); /* also (un)mutes audio */ > > /* The V4L2 spec requires one global set of cropping parameters > which only change on request. These are stored in btv->crop[1]. > @@ -3124,7 +3123,7 @@ static int bttv_release(struct file *file) > bttv_field_count(btv); > > if (!btv->users) > - audio_mute(btv, btv->mute); > + audio_mute(btv, 1); > > v4l2_fh_del(&fh->fh); > v4l2_fh_exit(&fh->fh); > @@ -4209,6 +4208,7 @@ static int bttv_probe(struct pci_dev *dev, const struct pci_device_id *pci_id) > btv->std = V4L2_STD_PAL; > init_irqreg(btv); > v4l2_ctrl_handler_setup(hdl); > + audio_mute(btv, 1); > > if (hdl->error) { > result = hdl->error; > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Am 10.03.2013 13:00, schrieb Hans Verkuil: > On Sun March 10 2013 12:40:34 Frank Schäfer wrote: >> Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com> > Could you describe exactly what bug is fixed? I tested it a fair amount > when I created my bttv patches, so I'd like to know what I missed. Yeah, sure. The whole thing is a bit tricky and I have to admit that the patch isn't self-explaining enough. The main issue it fixes is, that audio_mux() is called with the current value of the mute control instead of mute=1. So the device is muted on last close only if the mute control was 1. Fixing this needs changing some other parts of the code, too. What makes the patch bit complicated is the fact, that there are actually two types of "mute" handled in function audio_mux(): 1) gpio mute 2) subdevice muting The "automute" feature also does its best to make things complicating. And finally a follow-up fix is needed to avoid device unmuting already when the drivers probing function is called (should not happen before the first open). Hmm... sorry... I should really have split this into two patches. I will send an updated patch series with better explanations. Regards, Frank > > Regards, > > Hans > >> --- >> drivers/media/pci/bt8xx/bttv-driver.c | 22 +++++++++++----------- >> 1 Datei geändert, 11 Zeilen hinzugefügt(+), 11 Zeilen entfernt(-) >> >> diff --git a/drivers/media/pci/bt8xx/bttv-driver.c b/drivers/media/pci/bt8xx/bttv-driver.c >> index 8610b6a..2c09bc5 100644 >> --- a/drivers/media/pci/bt8xx/bttv-driver.c >> +++ b/drivers/media/pci/bt8xx/bttv-driver.c >> @@ -992,21 +992,20 @@ static char *audio_modes[] = { >> static int >> audio_mux(struct bttv *btv, int input, int mute) >> { >> - int gpio_val, signal; >> + int gpio_val, signal, mute_gpio; >> struct v4l2_ctrl *ctrl; >> >> gpio_inout(bttv_tvcards[btv->c.type].gpiomask, >> bttv_tvcards[btv->c.type].gpiomask); >> signal = btread(BT848_DSTATUS) & BT848_DSTATUS_HLOC; >> >> - btv->mute = mute; >> btv->audio = input; >> >> /* automute */ >> - mute = mute || (btv->opt_automute && (!signal || !btv->users) >> + mute_gpio = mute || (btv->opt_automute && (!signal || !btv->users) >> && !btv->has_radio_tuner); >> >> - if (mute) >> + if (mute_gpio) >> gpio_val = bttv_tvcards[btv->c.type].gpiomute; >> else >> gpio_val = bttv_tvcards[btv->c.type].gpiomux[input]; >> @@ -1022,7 +1021,7 @@ audio_mux(struct bttv *btv, int input, int mute) >> } >> >> if (bttv_gpio) >> - bttv_gpio_tracking(btv, audio_modes[mute ? 4 : input]); >> + bttv_gpio_tracking(btv, audio_modes[mute_gpio ? 4 : input]); >> if (in_interrupt()) >> return 0; >> >> @@ -1031,7 +1030,7 @@ audio_mux(struct bttv *btv, int input, int mute) >> >> ctrl = v4l2_ctrl_find(btv->sd_msp34xx->ctrl_handler, V4L2_CID_AUDIO_MUTE); >> if (ctrl) >> - v4l2_ctrl_s_ctrl(ctrl, btv->mute); >> + v4l2_ctrl_s_ctrl(ctrl, mute); >> >> /* Note: the inputs tuner/radio/extern/intern are translated >> to msp routings. This assumes common behavior for all msp3400 >> @@ -1080,7 +1079,7 @@ audio_mux(struct bttv *btv, int input, int mute) >> ctrl = v4l2_ctrl_find(btv->sd_tvaudio->ctrl_handler, V4L2_CID_AUDIO_MUTE); >> >> if (ctrl) >> - v4l2_ctrl_s_ctrl(ctrl, btv->mute); >> + v4l2_ctrl_s_ctrl(ctrl, mute); >> v4l2_subdev_call(btv->sd_tvaudio, audio, s_routing, >> input, 0, 0); >> } >> @@ -1088,7 +1087,7 @@ audio_mux(struct bttv *btv, int input, int mute) >> ctrl = v4l2_ctrl_find(btv->sd_tda7432->ctrl_handler, V4L2_CID_AUDIO_MUTE); >> >> if (ctrl) >> - v4l2_ctrl_s_ctrl(ctrl, btv->mute); >> + v4l2_ctrl_s_ctrl(ctrl, mute); >> } >> return 0; >> } >> @@ -1300,6 +1299,7 @@ static int bttv_s_ctrl(struct v4l2_ctrl *c) >> break; >> case V4L2_CID_AUDIO_MUTE: >> audio_mute(btv, c->val); >> + btv->mute = c->val; >> break; >> case V4L2_CID_AUDIO_VOLUME: >> btv->volume_gpio(btv, c->val); >> @@ -3062,8 +3062,7 @@ static int bttv_open(struct file *file) >> sizeof(struct bttv_buffer), >> fh, &btv->lock); >> set_tvnorm(btv,btv->tvnorm); >> - set_input(btv, btv->input, btv->tvnorm); >> - >> + set_input(btv, btv->input, btv->tvnorm); /* also (un)mutes audio */ >> >> /* The V4L2 spec requires one global set of cropping parameters >> which only change on request. These are stored in btv->crop[1]. >> @@ -3124,7 +3123,7 @@ static int bttv_release(struct file *file) >> bttv_field_count(btv); >> >> if (!btv->users) >> - audio_mute(btv, btv->mute); >> + audio_mute(btv, 1); >> >> v4l2_fh_del(&fh->fh); >> v4l2_fh_exit(&fh->fh); >> @@ -4209,6 +4208,7 @@ static int bttv_probe(struct pci_dev *dev, const struct pci_device_id *pci_id) >> btv->std = V4L2_STD_PAL; >> init_irqreg(btv); >> v4l2_ctrl_handler_setup(hdl); >> + audio_mute(btv, 1); >> >> if (hdl->error) { >> result = hdl->error; >> -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/media/pci/bt8xx/bttv-driver.c b/drivers/media/pci/bt8xx/bttv-driver.c index 8610b6a..2c09bc5 100644 --- a/drivers/media/pci/bt8xx/bttv-driver.c +++ b/drivers/media/pci/bt8xx/bttv-driver.c @@ -992,21 +992,20 @@ static char *audio_modes[] = { static int audio_mux(struct bttv *btv, int input, int mute) { - int gpio_val, signal; + int gpio_val, signal, mute_gpio; struct v4l2_ctrl *ctrl; gpio_inout(bttv_tvcards[btv->c.type].gpiomask, bttv_tvcards[btv->c.type].gpiomask); signal = btread(BT848_DSTATUS) & BT848_DSTATUS_HLOC; - btv->mute = mute; btv->audio = input; /* automute */ - mute = mute || (btv->opt_automute && (!signal || !btv->users) + mute_gpio = mute || (btv->opt_automute && (!signal || !btv->users) && !btv->has_radio_tuner); - if (mute) + if (mute_gpio) gpio_val = bttv_tvcards[btv->c.type].gpiomute; else gpio_val = bttv_tvcards[btv->c.type].gpiomux[input]; @@ -1022,7 +1021,7 @@ audio_mux(struct bttv *btv, int input, int mute) } if (bttv_gpio) - bttv_gpio_tracking(btv, audio_modes[mute ? 4 : input]); + bttv_gpio_tracking(btv, audio_modes[mute_gpio ? 4 : input]); if (in_interrupt()) return 0; @@ -1031,7 +1030,7 @@ audio_mux(struct bttv *btv, int input, int mute) ctrl = v4l2_ctrl_find(btv->sd_msp34xx->ctrl_handler, V4L2_CID_AUDIO_MUTE); if (ctrl) - v4l2_ctrl_s_ctrl(ctrl, btv->mute); + v4l2_ctrl_s_ctrl(ctrl, mute); /* Note: the inputs tuner/radio/extern/intern are translated to msp routings. This assumes common behavior for all msp3400 @@ -1080,7 +1079,7 @@ audio_mux(struct bttv *btv, int input, int mute) ctrl = v4l2_ctrl_find(btv->sd_tvaudio->ctrl_handler, V4L2_CID_AUDIO_MUTE); if (ctrl) - v4l2_ctrl_s_ctrl(ctrl, btv->mute); + v4l2_ctrl_s_ctrl(ctrl, mute); v4l2_subdev_call(btv->sd_tvaudio, audio, s_routing, input, 0, 0); } @@ -1088,7 +1087,7 @@ audio_mux(struct bttv *btv, int input, int mute) ctrl = v4l2_ctrl_find(btv->sd_tda7432->ctrl_handler, V4L2_CID_AUDIO_MUTE); if (ctrl) - v4l2_ctrl_s_ctrl(ctrl, btv->mute); + v4l2_ctrl_s_ctrl(ctrl, mute); } return 0; } @@ -1300,6 +1299,7 @@ static int bttv_s_ctrl(struct v4l2_ctrl *c) break; case V4L2_CID_AUDIO_MUTE: audio_mute(btv, c->val); + btv->mute = c->val; break; case V4L2_CID_AUDIO_VOLUME: btv->volume_gpio(btv, c->val); @@ -3062,8 +3062,7 @@ static int bttv_open(struct file *file) sizeof(struct bttv_buffer), fh, &btv->lock); set_tvnorm(btv,btv->tvnorm); - set_input(btv, btv->input, btv->tvnorm); - + set_input(btv, btv->input, btv->tvnorm); /* also (un)mutes audio */ /* The V4L2 spec requires one global set of cropping parameters which only change on request. These are stored in btv->crop[1]. @@ -3124,7 +3123,7 @@ static int bttv_release(struct file *file) bttv_field_count(btv); if (!btv->users) - audio_mute(btv, btv->mute); + audio_mute(btv, 1); v4l2_fh_del(&fh->fh); v4l2_fh_exit(&fh->fh); @@ -4209,6 +4208,7 @@ static int bttv_probe(struct pci_dev *dev, const struct pci_device_id *pci_id) btv->std = V4L2_STD_PAL; init_irqreg(btv); v4l2_ctrl_handler_setup(hdl); + audio_mute(btv, 1); if (hdl->error) { result = hdl->error;
Signed-off-by: Frank Schäfer <fschaefer.oss@googlemail.com> --- drivers/media/pci/bt8xx/bttv-driver.c | 22 +++++++++++----------- 1 Datei geändert, 11 Zeilen hinzugefügt(+), 11 Zeilen entfernt(-)