| Message ID | d14f3141901856eaed358ab049f4a3aac8fe4863.1457711514.git.mchehab@osg.samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 03/11/2016 08:55 AM, Mauro Carvalho Chehab wrote: > For au0828_enable_source() to work, the tuner links should be > disabled and the tuner/decoder should be cached at au0828 struct. hmm. are you sure about needing to cache decoder in au0828 struct. It gets cached in au0828_card_analog_fe_setup() which is called from au0828_card_setup() - this step happens before au0828_media_device_register() #ifdef CONFIG_MEDIA_CONTROLLER if (sd) dev->decoder = &sd->entity; #endif thanks, -- Shuah > > Fixes: 9822f4173f84 ('[media] au0828: use v4l2_mc_create_media_graph()') > Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com> > --- > drivers/media/usb/au0828/au0828-core.c | 42 ++++++++++++++++++++++------------ > 1 file changed, 27 insertions(+), 15 deletions(-) > > diff --git a/drivers/media/usb/au0828/au0828-core.c b/drivers/media/usb/au0828/au0828-core.c > index 5dc82e8c8670..ecfa18939663 100644 > --- a/drivers/media/usb/au0828/au0828-core.c > +++ b/drivers/media/usb/au0828/au0828-core.c > @@ -456,7 +456,8 @@ static int au0828_media_device_register(struct au0828_dev *dev, > { > #ifdef CONFIG_MEDIA_CONTROLLER > int ret; > - struct media_entity *entity, *demod = NULL, *tuner = NULL; > + struct media_entity *entity, *demod = NULL; > + struct media_link *link; > > if (!dev->media_dev) > return 0; > @@ -482,26 +483,37 @@ static int au0828_media_device_register(struct au0828_dev *dev, > } > > /* > - * Find tuner and demod to disable the link between > - * the two to avoid disable step when tuner is requested > - * by video or audio. Note that this step can't be done > - * until dvb graph is created during dvb register. > + * Find tuner, decoder and demod. > + * > + * The tuner and decoder should be cached, as they'll be used by > + * au0828_enable_source. > + * > + * It also needs to disable the link between tuner and > + * decoder/demod, to avoid disable step when tuner is requested > + * by video or audio. Note that this step can't be done until dvb > + * graph is created during dvb register. > */ > media_device_for_each_entity(entity, dev->media_dev) { > - if (entity->function == MEDIA_ENT_F_DTV_DEMOD) > + switch (entity->function) { > + case MEDIA_ENT_F_TUNER: > + dev->tuner = entity; > + break; > + case MEDIA_ENT_F_ATV_DECODER: > + dev->decoder = entity; > + break; > + case MEDIA_ENT_F_DTV_DEMOD: > demod = entity; > - else if (entity->function == MEDIA_ENT_F_TUNER) > - tuner = entity; > + break; > + } > } > - /* Disable link between tuner and demod */ > - if (tuner && demod) { > - struct media_link *link; > > - list_for_each_entry(link, &demod->links, list) { > - if (link->sink->entity == demod && > - link->source->entity == tuner) { > + /* Disable link between tuner->demod and/or tuner->decoder */ > + if (dev->tuner) { > + list_for_each_entry(link, &dev->tuner->links, list) { > + if (demod && link->sink->entity == demod) > + media_entity_setup_link(link, 0); > + if (dev->decoder && link->sink->entity == dev->decoder) > media_entity_setup_link(link, 0); > - } > } > } > >
Em Fri, 11 Mar 2016 09:27:40 -0700 Shuah Khan <shuahkh@osg.samsung.com> escreveu: > On 03/11/2016 08:55 AM, Mauro Carvalho Chehab wrote: > > For au0828_enable_source() to work, the tuner links should be > > disabled and the tuner/decoder should be cached at au0828 struct. > > hmm. are you sure about needing to cache decoder in au0828 struct. > It gets cached in au0828_card_analog_fe_setup() which is called > from au0828_card_setup() - this step happens before > au0828_media_device_register() > > #ifdef CONFIG_MEDIA_CONTROLLER > if (sd) > dev->decoder = &sd->entity; > #endif I haven't check it. Yet, I guess the best would be to put those caches all at au0828_media_device_register(). This way, it would be easier to remember removing them, once we move au0828_enable_source to the core. (my plan is to take a look on it after the merge window, doing some tests on more complex drivers, like em28xx and saa7134). Regards, Mauro -- 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 03/11/2016 09:34 AM, Mauro Carvalho Chehab wrote: > Em Fri, 11 Mar 2016 09:27:40 -0700 > Shuah Khan <shuahkh@osg.samsung.com> escreveu: > >> On 03/11/2016 08:55 AM, Mauro Carvalho Chehab wrote: >>> For au0828_enable_source() to work, the tuner links should be >>> disabled and the tuner/decoder should be cached at au0828 struct. >> >> hmm. are you sure about needing to cache decoder in au0828 struct. >> It gets cached in au0828_card_analog_fe_setup() which is called >> from au0828_card_setup() - this step happens before >> au0828_media_device_register() >> >> #ifdef CONFIG_MEDIA_CONTROLLER >> if (sd) >> dev->decoder = &sd->entity; >> #endif > > I haven't check it. Yet, I guess the best would be to put those > caches all at au0828_media_device_register(). This way, it would be > easier to remember removing them, once we move au0828_enable_source > to the core. Yes. Consolidating caches in one place is a good idea. One thing we have to keep in mind when we do that is the !MEDIA_CONTROLLER cases. The above cached value could be used even when MEDIA_CONTROLLER isn't enabled. > > (my plan is to take a look on it after the merge window, doing some > tests on more complex drivers, like em28xx and saa7134). Sounds good. thanks, -- Shuah
On 03/11/2016 09:52 AM, Shuah Khan wrote: > On 03/11/2016 09:34 AM, Mauro Carvalho Chehab wrote: >> Em Fri, 11 Mar 2016 09:27:40 -0700 >> Shuah Khan <shuahkh@osg.samsung.com> escreveu: >> >>> On 03/11/2016 08:55 AM, Mauro Carvalho Chehab wrote: >>>> For au0828_enable_source() to work, the tuner links should be >>>> disabled and the tuner/decoder should be cached at au0828 struct. >>> >>> hmm. are you sure about needing to cache decoder in au0828 struct. >>> It gets cached in au0828_card_analog_fe_setup() which is called >>> from au0828_card_setup() - this step happens before >>> au0828_media_device_register() >>> >>> #ifdef CONFIG_MEDIA_CONTROLLER >>> if (sd) >>> dev->decoder = &sd->entity; >>> #endif >> >> I haven't check it. Yet, I guess the best would be to put those >> caches all at au0828_media_device_register(). This way, it would be >> easier to remember removing them, once we move au0828_enable_source >> to the core. > > Yes. Consolidating caches in one place is a good idea. One thing we > have to keep in mind when we do that is the !MEDIA_CONTROLLER cases. > The above cached value could be used even when MEDIA_CONTROLLER isn't > enabled. Never mind - the second comment isn't valid! We are setting this in ifdef CONFIG_MEDIA_CONTROLLER :) thanks, -- Shuah
diff --git a/drivers/media/usb/au0828/au0828-core.c b/drivers/media/usb/au0828/au0828-core.c index 5dc82e8c8670..ecfa18939663 100644 --- a/drivers/media/usb/au0828/au0828-core.c +++ b/drivers/media/usb/au0828/au0828-core.c @@ -456,7 +456,8 @@ static int au0828_media_device_register(struct au0828_dev *dev, { #ifdef CONFIG_MEDIA_CONTROLLER int ret; - struct media_entity *entity, *demod = NULL, *tuner = NULL; + struct media_entity *entity, *demod = NULL; + struct media_link *link; if (!dev->media_dev) return 0; @@ -482,26 +483,37 @@ static int au0828_media_device_register(struct au0828_dev *dev, } /* - * Find tuner and demod to disable the link between - * the two to avoid disable step when tuner is requested - * by video or audio. Note that this step can't be done - * until dvb graph is created during dvb register. + * Find tuner, decoder and demod. + * + * The tuner and decoder should be cached, as they'll be used by + * au0828_enable_source. + * + * It also needs to disable the link between tuner and + * decoder/demod, to avoid disable step when tuner is requested + * by video or audio. Note that this step can't be done until dvb + * graph is created during dvb register. */ media_device_for_each_entity(entity, dev->media_dev) { - if (entity->function == MEDIA_ENT_F_DTV_DEMOD) + switch (entity->function) { + case MEDIA_ENT_F_TUNER: + dev->tuner = entity; + break; + case MEDIA_ENT_F_ATV_DECODER: + dev->decoder = entity; + break; + case MEDIA_ENT_F_DTV_DEMOD: demod = entity; - else if (entity->function == MEDIA_ENT_F_TUNER) - tuner = entity; + break; + } } - /* Disable link between tuner and demod */ - if (tuner && demod) { - struct media_link *link; - list_for_each_entry(link, &demod->links, list) { - if (link->sink->entity == demod && - link->source->entity == tuner) { + /* Disable link between tuner->demod and/or tuner->decoder */ + if (dev->tuner) { + list_for_each_entry(link, &dev->tuner->links, list) { + if (demod && link->sink->entity == demod) + media_entity_setup_link(link, 0); + if (dev->decoder && link->sink->entity == dev->decoder) media_entity_setup_link(link, 0); - } } }
For au0828_enable_source() to work, the tuner links should be disabled and the tuner/decoder should be cached at au0828 struct. Fixes: 9822f4173f84 ('[media] au0828: use v4l2_mc_create_media_graph()') Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com> --- drivers/media/usb/au0828/au0828-core.c | 42 ++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 15 deletions(-)