| Message ID | d328b90800498d10d908c67b05bd48bfb78162c0.1533138685.git.mchehab+samsung@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Better handle pads for tuning/decoder part of the devices | expand |
Hi Mauro, Thank you for the patch. Could you please CC me on patches touching the media controller core ? I can send a MAINTAINERS patch to make sure that gets handled automatically. On Wednesday, 1 August 2018 18:55:04 EEST Mauro Carvalho Chehab wrote: > Consumer devices are provided with a wide diferent range of types > supported by the same driver, allowing different configutations. > > In order to make easier to setup media controller links, "taint" > pads with the signal type it carries. > > While here, get rid of DEMOD_PAD_VBI_OUT, as the signal it carries > is actually the same as the normal video output. > > The difference happens at the video/VBI interface: > - for VBI, only the hidden lines are streamed; > - for video, the stream is usually cropped to hide the > vbi lines. > > Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org> > --- > drivers/media/dvb-frontends/au8522_decoder.c | 3 ++ > drivers/media/i2c/msp3400-driver.c | 2 ++ > drivers/media/i2c/saa7115.c | 2 ++ > drivers/media/i2c/tvp5150.c | 2 ++ > drivers/media/pci/saa7134/saa7134-core.c | 2 ++ > drivers/media/tuners/si2157.c | 3 ++ > drivers/media/usb/dvb-usb-v2/mxl111sf.c | 2 ++ > drivers/media/v4l2-core/tuner-core.c | 5 +++ > include/media/media-entity.h | 35 ++++++++++++++++++++ > 9 files changed, 56 insertions(+) [snip] > diff --git a/include/media/media-entity.h b/include/media/media-entity.h > index 3aa3d58d1d58..8bfbe6b59fa9 100644 > --- a/include/media/media-entity.h > +++ b/include/media/media-entity.h > @@ -155,6 +155,40 @@ struct media_link { > bool is_backlink; > }; > > +/** > + * struct media_pad_signal_type - type of the signal inside a media pad I'd say "carried by a media pad" instead of "inside a media pad". > + * > + * @PAD_SIGNAL_DEFAULT Shouldn't we use a MEDIA_PAD_ prefix ? > + * Default signal. Use this when all inputs or all outputs are > + * uniquely identified by the pad number. How about "Use this when the pad can carry a single signal type" ? I understand your formulation as meaning that all pads of the entity have to be of the default type, or none can be. > + * @PAD_SIGNAL_ANALOG This isn't a very good name given that PAD_SIGNAL_TV_CARRIERS is also analog. > + * The pad contains an analog signa. It can be Radio Frequency, s/signa/signal/ > + * Intermediate Frequency or baseband signal. > + * Tuner inputs, composite and s-video signals should use it. > + * On tuner sources, this is used for digital TV demodulators and for > + * IF-PLL demodulator like tda9887. > + * @PAD_SIGNAL_TV_CARRIERS > + * The pad contains analog signals carrying either a digital or an analog > + * modulated (or baseband) signal. As above, maybe "The pad carries either ...". > This is provided by tuner source > + * pads and used by analog TV standard decoders and by digital TV demods. > + * @PAD_SIGNAL_DV > + * Contains a digital video signal, with can be a bitstream of samples > + * taken from an analog TV video source. On such case, it usually > + * contains the VBI data on it. > + * @PAD_SIGNAL_AUDIO > + * Contains an Intermediate Frequency analog signal from an audio > + * sub-carrier or an audio bitstream. IF signals are provided by tuners > + * and consumed by audio AM/FM decoders. Bitstream audio is provided by s/ / / > + * an audio decoder. Generally speaking the types you propose here seem quite ad-hoc, without much coherency. For instance you split analog and digital video, but group all audio under a single type. It's also not very clear from the description how to handle analog video, as it could match both PAD_SIGNAL_ANALOG and PAD_SIGNAL_TV_CARRIERS. Both of those types also accept analog baseband signals. I think this should be reworked, it doesn't sound very usable except for the specific use case that this series tries to address. > + */ > +enum media_pad_signal_type { > + PAD_SIGNAL_DEFAULT = 0, > + PAD_SIGNAL_ANALOG, > + PAD_SIGNAL_TV_CARRIERS, > + PAD_SIGNAL_DV, > + PAD_SIGNAL_AUDIO, > +}; > + > /** > * struct media_pad - A media pad graph object. > * > @@ -169,6 +203,7 @@ struct media_pad { > struct media_gobj graph_obj; /* must be first field in struct */ > struct media_entity *entity; > u16 index; > + enum media_pad_signal_type sig_type; Missing kerneldoc and Documentation/ update ? It's important to document the use cases, and in particular whether the type should be static or can vary (as in whether a pad can carry different types over time). > unsigned long flags; > };
Em Wed, 26 Sep 2018 17:09:19 +0300 Laurent Pinchart <laurent.pinchart@ideasonboard.com> escreveu: > Hi Mauro, > > Thank you for the patch. > > Could you please CC me on patches touching the media controller core ? I can > send a MAINTAINERS patch to make sure that gets handled automatically. > > On Wednesday, 1 August 2018 18:55:04 EEST Mauro Carvalho Chehab wrote: > > Consumer devices are provided with a wide diferent range of types > > supported by the same driver, allowing different configutations. > > > > In order to make easier to setup media controller links, "taint" > > pads with the signal type it carries. > > > > While here, get rid of DEMOD_PAD_VBI_OUT, as the signal it carries > > is actually the same as the normal video output. > > > > The difference happens at the video/VBI interface: > > - for VBI, only the hidden lines are streamed; > > - for video, the stream is usually cropped to hide the > > vbi lines. > > > > Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org> > > --- > > drivers/media/dvb-frontends/au8522_decoder.c | 3 ++ > > drivers/media/i2c/msp3400-driver.c | 2 ++ > > drivers/media/i2c/saa7115.c | 2 ++ > > drivers/media/i2c/tvp5150.c | 2 ++ > > drivers/media/pci/saa7134/saa7134-core.c | 2 ++ > > drivers/media/tuners/si2157.c | 3 ++ > > drivers/media/usb/dvb-usb-v2/mxl111sf.c | 2 ++ > > drivers/media/v4l2-core/tuner-core.c | 5 +++ > > include/media/media-entity.h | 35 ++++++++++++++++++++ > > 9 files changed, 56 insertions(+) > > [snip] > > > diff --git a/include/media/media-entity.h b/include/media/media-entity.h > > index 3aa3d58d1d58..8bfbe6b59fa9 100644 > > --- a/include/media/media-entity.h > > +++ b/include/media/media-entity.h > > @@ -155,6 +155,40 @@ struct media_link { > > bool is_backlink; > > }; > > > > +/** > > + * struct media_pad_signal_type - type of the signal inside a media pad > > I'd say "carried by a media pad" instead of "inside a media pad". Ok. > > > + * > > + * @PAD_SIGNAL_DEFAULT > > Shouldn't we use a MEDIA_PAD_ prefix ? Ok. > > > + * Default signal. Use this when all inputs or all outputs are > > + * uniquely identified by the pad number. > > How about "Use this when the pad can carry a single signal type" ? I > understand your formulation as meaning that all pads of the entity have to be > of the default type, or none can be. Describing the MEDIA_PAD_SIGNAL_DEFAULT is not trivial. Basically, almost all pads at the subsystem are MEDIA_PAD_SIGNAL_DEFAULT. Only certain types of entities need to "taint" the pad (usually, the entities related to the tuning and audio/video decoding stages). In other words, the decision if it should either use MEDIA_PAD_SIGNAL_DEFAULT or not is per-entity (and not per-pad). Typically, it is needed when a certain entity function may have pads at the same direction with different meanings. A real example would be a digital tuner entity that has two pad sources: - pad source #0 - carries a RF signal; - pad source #1 - carries a digital audio signal. On this case, both pad sources should use a signal different than MEDIA_PAD_SIGNAL_DEFAULT. Writing it on a concise way is not a trivial task ;-) Maybe something like would work: "Default signal. Use this when there's no need to specify the signal carried inside a pad." > > > + * @PAD_SIGNAL_ANALOG > > This isn't a very good name given that PAD_SIGNAL_TV_CARRIERS is also analog. > > > + * The pad contains an analog signa. It can be Radio Frequency, > > s/signa/signal/ Ok (this is actually fixed already at the version applied). > > + * Intermediate Frequency or baseband signal. > > + * Tuner inputs, composite and s-video signals should use it. > > + * On tuner sources, this is used for digital TV demodulators and for > > + * IF-PLL demodulator like tda9887. > > + * @PAD_SIGNAL_TV_CARRIERS > > + * The pad contains analog signals carrying either a digital or an analog > > + * modulated (or baseband) signal. > > As above, maybe "The pad carries either ...". Replaced "contains" with "carries". > > This is provided by tuner source > > + * pads and used by analog TV standard decoders and by digital TV demods. > > + * @PAD_SIGNAL_DV > > + * Contains a digital video signal, with can be a bitstream of samples > > + * taken from an analog TV video source. On such case, it usually > > + * contains the VBI data on it. > > + * @PAD_SIGNAL_AUDIO > > + * Contains an Intermediate Frequency analog signal from an audio > > + * sub-carrier or an audio bitstream. IF signals are provided by tuners > > + * and consumed by audio AM/FM decoders. Bitstream audio is provided by > > s/ / / > > > + * an audio decoder. > > Generally speaking the types you propose here seem quite ad-hoc, without much > coherency. For instance you split analog and digital video, but group all > audio under a single type. It's also not very clear from the description how > to handle analog video, as it could match both PAD_SIGNAL_ANALOG and > PAD_SIGNAL_TV_CARRIERS. Both of those types also accept analog baseband > signals. I think this should be reworked, it doesn't sound very usable except > for the specific use case that this series tries to address. The goal here is to be able "taint" the signal to a certain group. It might have been called as "blue", "red", "green", but using a more coherent name for a group where the signal belongs make easier to add the right taints at the pads. With regards with PAD_SIGNAL_ANALOG versus PAD_SIGNAL_TV_CARRIERS, fully agreed. Actually, I noticed the same issue you pointed: it is not clear where to use PAD_SIGNAL_ANALOG or PAD_SIGNAL_TV_CARRIERS. So, I got rid of the latter at the second version of this patch: Date: Sat, 15 Sep 2018 17:14:17 -0300 Subject: [PATCH v2 02/14] media: v4l2: taint pads with the signal types for consumer devices After that, there are only 3 types of signal: PAD_SIGNAL_ANALOG - any kind of analog signal (either baseband or RF/IF) PAD_SIGNAL_DV - any kind of digital video signal PAD_SIGNAL_AUDIO - any kind of audio signal (either analog or digital) > > + */ > > +enum media_pad_signal_type { > > + PAD_SIGNAL_DEFAULT = 0, > > + PAD_SIGNAL_ANALOG, > > + PAD_SIGNAL_TV_CARRIERS, > > + PAD_SIGNAL_DV, > > + PAD_SIGNAL_AUDIO, > > +}; > > + > > /** > > * struct media_pad - A media pad graph object. > > * > > @@ -169,6 +203,7 @@ struct media_pad { > > struct media_gobj graph_obj; /* must be first field in struct */ > > struct media_entity *entity; > > u16 index; > > + enum media_pad_signal_type sig_type; > > Missing kerneldoc and Documentation/ update ? It's important to document the > use cases, and in particular whether the type should be static or can vary (as > in whether a pad can carry different types over time). I don't see any use cases where the type of the signal can vary over time. If we ever need that, then we would need another signal type (like MEDIA_PAD_DYNAMIC_AUDIO_OR_VIDEO). What about this? * @sig_type: Type of the signal that a media pad supports. This is used * when an entity function may have different kinds of signals * on different pads of the same direction (source/sink). For * example, if an entity has 2 pad sinks where one carries * an analog video while the other carries an audio signal. > > unsigned long flags; > > }; > > Thanks, Mauro media: improve media pad signal documentation There are some things at the media pad signal documentation that are not precise. Improve its documentation and rename from PAD_SIGNAL_* -> MEDIA_PAD_SIGNAL in order to better document its usage inside the drivers. Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org> diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c index b7171bf094fb..14e9b1db72a0 100644 --- a/drivers/media/dvb-core/dvbdev.c +++ b/drivers/media/dvb-core/dvbdev.c @@ -691,7 +691,7 @@ int dvb_create_media_graph(struct dvb_adapter *adap, false); } else { pad_sink = media_get_pad_index(tuner, true, - PAD_SIGNAL_ANALOG); + MEDIA_PAD_SIGNAL_ANALOG); if (pad_sink < 0) return -EINVAL; ret = media_create_pad_links(mdev, @@ -708,7 +708,7 @@ int dvb_create_media_graph(struct dvb_adapter *adap, if (ntuner && ndemod) { pad_source = media_get_pad_index(tuner, true, - PAD_SIGNAL_ANALOG); + MEDIA_PAD_SIGNAL_ANALOG); if (pad_source) return -EINVAL; ret = media_create_pad_links(mdev, diff --git a/drivers/media/dvb-frontends/au8522_decoder.c b/drivers/media/dvb-frontends/au8522_decoder.c index b2dd20ffd002..1f99e6238eb8 100644 --- a/drivers/media/dvb-frontends/au8522_decoder.c +++ b/drivers/media/dvb-frontends/au8522_decoder.c @@ -719,11 +719,11 @@ static int au8522_probe(struct i2c_client *client, #if defined(CONFIG_MEDIA_CONTROLLER) state->pads[AU8522_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK; - state->pads[AU8522_PAD_IF_INPUT].sig_type = PAD_SIGNAL_ANALOG; + state->pads[AU8522_PAD_IF_INPUT].sig_type = MEDIA_PAD_SIGNAL_ANALOG; state->pads[AU8522_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE; - state->pads[AU8522_PAD_VID_OUT].sig_type = PAD_SIGNAL_DV; + state->pads[AU8522_PAD_VID_OUT].sig_type = MEDIA_PAD_SIGNAL_DV; state->pads[AU8522_PAD_AUDIO_OUT].flags = MEDIA_PAD_FL_SOURCE; - state->pads[AU8522_PAD_AUDIO_OUT].sig_type = PAD_SIGNAL_AUDIO; + state->pads[AU8522_PAD_AUDIO_OUT].sig_type = MEDIA_PAD_SIGNAL_AUDIO; sd->entity.function = MEDIA_ENT_F_ATV_DECODER; ret = media_entity_pads_init(&sd->entity, ARRAY_SIZE(state->pads), diff --git a/drivers/media/i2c/msp3400-driver.c b/drivers/media/i2c/msp3400-driver.c index c63be01059b2..935dfec2b405 100644 --- a/drivers/media/i2c/msp3400-driver.c +++ b/drivers/media/i2c/msp3400-driver.c @@ -704,9 +704,9 @@ static int msp_probe(struct i2c_client *client, const struct i2c_device_id *id) #if defined(CONFIG_MEDIA_CONTROLLER) state->pads[MSP3400_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK; - state->pads[MSP3400_PAD_IF_INPUT].sig_type = PAD_SIGNAL_AUDIO; + state->pads[MSP3400_PAD_IF_INPUT].sig_type = MEDIA_PAD_SIGNAL_AUDIO; state->pads[MSP3400_PAD_OUT].flags = MEDIA_PAD_FL_SOURCE; - state->pads[MSP3400_PAD_OUT].sig_type = PAD_SIGNAL_AUDIO; + state->pads[MSP3400_PAD_OUT].sig_type = MEDIA_PAD_SIGNAL_AUDIO; sd->entity.function = MEDIA_ENT_F_IF_AUD_DECODER; diff --git a/drivers/media/i2c/saa7115.c b/drivers/media/i2c/saa7115.c index 6bc278aa31fc..d74065890a80 100644 --- a/drivers/media/i2c/saa7115.c +++ b/drivers/media/i2c/saa7115.c @@ -1841,9 +1841,9 @@ static int saa711x_probe(struct i2c_client *client, #if defined(CONFIG_MEDIA_CONTROLLER) state->pads[SAA711X_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK; - state->pads[SAA711X_PAD_IF_INPUT].sig_type = PAD_SIGNAL_ANALOG; + state->pads[SAA711X_PAD_IF_INPUT].sig_type = MEDIA_PAD_SIGNAL_ANALOG; state->pads[SAA711X_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE; - state->pads[SAA711X_PAD_VID_OUT].sig_type = PAD_SIGNAL_DV; + state->pads[SAA711X_PAD_VID_OUT].sig_type = MEDIA_PAD_SIGNAL_DV; sd->entity.function = MEDIA_ENT_F_ATV_DECODER; diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c index f5b234e4599d..45be245442e0 100644 --- a/drivers/media/i2c/tvp5150.c +++ b/drivers/media/i2c/tvp5150.c @@ -1738,9 +1738,9 @@ static int tvp5150_probe(struct i2c_client *c, #if defined(CONFIG_MEDIA_CONTROLLER) core->pads[TVP5150_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK; - core->pads[TVP5150_PAD_IF_INPUT].sig_type = PAD_SIGNAL_ANALOG; + core->pads[TVP5150_PAD_IF_INPUT].sig_type = MEDIA_PAD_SIGNAL_ANALOG; core->pads[TVP5150_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE; - core->pads[TVP5150_PAD_VID_OUT].sig_type = PAD_SIGNAL_DV; + core->pads[TVP5150_PAD_VID_OUT].sig_type = MEDIA_PAD_SIGNAL_DV; sd->entity.function = MEDIA_ENT_F_ATV_DECODER; diff --git a/drivers/media/pci/saa7134/saa7134-core.c b/drivers/media/pci/saa7134/saa7134-core.c index 8984b1bf57a5..f4933e5b4850 100644 --- a/drivers/media/pci/saa7134/saa7134-core.c +++ b/drivers/media/pci/saa7134/saa7134-core.c @@ -846,9 +846,9 @@ static void saa7134_create_entities(struct saa7134_dev *dev) if (!decoder) { dev->demod.name = "saa713x"; dev->demod_pad[SAA7134_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK; - dev->demod_pad[SAA7134_PAD_IF_INPUT].sig_type = PAD_SIGNAL_ANALOG; + dev->demod_pad[SAA7134_PAD_IF_INPUT].sig_type = MEDIA_PAD_SIGNAL_ANALOG; dev->demod_pad[SAA7134_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE; - dev->demod_pad[SAA7134_PAD_VID_OUT].sig_type = PAD_SIGNAL_DV; + dev->demod_pad[SAA7134_PAD_VID_OUT].sig_type = MEDIA_PAD_SIGNAL_DV; dev->demod.function = MEDIA_ENT_F_ATV_DECODER; ret = media_entity_pads_init(&dev->demod, SAA7134_NUM_PADS, diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c index d389f1fc237a..17992f9fab09 100644 --- a/drivers/media/tuners/si2157.c +++ b/drivers/media/tuners/si2157.c @@ -469,11 +469,11 @@ static int si2157_probe(struct i2c_client *client, dev->ent.function = MEDIA_ENT_F_TUNER; dev->pad[SI2157_PAD_RF_INPUT].flags = MEDIA_PAD_FL_SINK; - dev->pad[SI2157_PAD_RF_INPUT].sig_type = PAD_SIGNAL_ANALOG; + dev->pad[SI2157_PAD_RF_INPUT].sig_type = MEDIA_PAD_SIGNAL_ANALOG; dev->pad[SI2157_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE; - dev->pad[SI2157_PAD_VID_OUT].sig_type = PAD_SIGNAL_ANALOG; + dev->pad[SI2157_PAD_VID_OUT].sig_type = MEDIA_PAD_SIGNAL_ANALOG; dev->pad[SI2157_PAD_AUD_OUT].flags = MEDIA_PAD_FL_SOURCE; - dev->pad[SI2157_PAD_AUD_OUT].sig_type = PAD_SIGNAL_AUDIO; + dev->pad[SI2157_PAD_AUD_OUT].sig_type = MEDIA_PAD_SIGNAL_AUDIO; ret = media_entity_pads_init(&dev->ent, SI2157_NUM_PADS, &dev->pad[0]); diff --git a/drivers/media/usb/au0828/au0828-core.c b/drivers/media/usb/au0828/au0828-core.c index 1fdb1601dc65..484b620879c0 100644 --- a/drivers/media/usb/au0828/au0828-core.c +++ b/drivers/media/usb/au0828/au0828-core.c @@ -267,7 +267,7 @@ static void au0828_media_graph_notify(struct media_entity *new, create_link: if (decoder && mixer) { ret = media_get_pad_index(decoder, false, - PAD_SIGNAL_AUDIO); + MEDIA_PAD_SIGNAL_AUDIO); if (ret >= 0) ret = media_create_pad_link(decoder, ret, mixer, 0, diff --git a/drivers/media/usb/dvb-usb-v2/mxl111sf.c b/drivers/media/usb/dvb-usb-v2/mxl111sf.c index 85cdf593a9ad..1ebe04a3e7dd 100644 --- a/drivers/media/usb/dvb-usb-v2/mxl111sf.c +++ b/drivers/media/usb/dvb-usb-v2/mxl111sf.c @@ -893,9 +893,9 @@ static int mxl111sf_attach_tuner(struct dvb_usb_adapter *adap) state->tuner.function = MEDIA_ENT_F_TUNER; state->tuner.name = "mxl111sf tuner"; state->tuner_pads[MXL111SF_PAD_RF_INPUT].flags = MEDIA_PAD_FL_SINK; - state->tuner_pads[MXL111SF_PAD_RF_INPUT].sig_type = PAD_SIGNAL_ANALOG; + state->tuner_pads[MXL111SF_PAD_RF_INPUT].sig_type = MEDIA_PAD_SIGNAL_ANALOG; state->tuner_pads[MXL111SF_PAD_OUTPUT].flags = MEDIA_PAD_FL_SOURCE; - state->tuner_pads[MXL111SF_PAD_OUTPUT].sig_type = PAD_SIGNAL_ANALOG; + state->tuner_pads[MXL111SF_PAD_OUTPUT].sig_type = MEDIA_PAD_SIGNAL_ANALOG; ret = media_entity_pads_init(&state->tuner, MXL111SF_NUM_PADS, state->tuner_pads); diff --git a/drivers/media/v4l2-core/tuner-core.c b/drivers/media/v4l2-core/tuner-core.c index 03a340cb5a9b..061b13821367 100644 --- a/drivers/media/v4l2-core/tuner-core.c +++ b/drivers/media/v4l2-core/tuner-core.c @@ -732,20 +732,20 @@ static int tuner_probe(struct i2c_client *client, */ if (t->type == TUNER_TDA9887) { t->pad[IF_VID_DEC_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK; - t->pad[IF_VID_DEC_PAD_IF_INPUT].sig_type = PAD_SIGNAL_ANALOG; + t->pad[IF_VID_DEC_PAD_IF_INPUT].sig_type = MEDIA_PAD_SIGNAL_ANALOG; t->pad[IF_VID_DEC_PAD_OUT].flags = MEDIA_PAD_FL_SOURCE; - t->pad[IF_VID_DEC_PAD_OUT].sig_type = PAD_SIGNAL_ANALOG; + t->pad[IF_VID_DEC_PAD_OUT].sig_type = MEDIA_PAD_SIGNAL_ANALOG; ret = media_entity_pads_init(&t->sd.entity, IF_VID_DEC_PAD_NUM_PADS, &t->pad[0]); t->sd.entity.function = MEDIA_ENT_F_IF_VID_DECODER; } else { t->pad[TUNER_PAD_RF_INPUT].flags = MEDIA_PAD_FL_SINK; - t->pad[TUNER_PAD_RF_INPUT].sig_type = PAD_SIGNAL_ANALOG; + t->pad[TUNER_PAD_RF_INPUT].sig_type = MEDIA_PAD_SIGNAL_ANALOG; t->pad[TUNER_PAD_OUTPUT].flags = MEDIA_PAD_FL_SOURCE; - t->pad[TUNER_PAD_OUTPUT].sig_type = PAD_SIGNAL_ANALOG; + t->pad[TUNER_PAD_OUTPUT].sig_type = MEDIA_PAD_SIGNAL_ANALOG; t->pad[TUNER_PAD_AUD_OUT].flags = MEDIA_PAD_FL_SOURCE; - t->pad[TUNER_PAD_AUD_OUT].sig_type = PAD_SIGNAL_AUDIO; + t->pad[TUNER_PAD_AUD_OUT].sig_type = MEDIA_PAD_SIGNAL_AUDIO; ret = media_entity_pads_init(&t->sd.entity, TUNER_NUM_PADS, &t->pad[0]); t->sd.entity.function = MEDIA_ENT_F_TUNER; diff --git a/drivers/media/v4l2-core/v4l2-mc.c b/drivers/media/v4l2-core/v4l2-mc.c index 014a2a97cadd..f559e47cf8e8 100644 --- a/drivers/media/v4l2-core/v4l2-mc.c +++ b/drivers/media/v4l2-core/v4l2-mc.c @@ -106,9 +106,9 @@ int v4l2_mc_create_media_graph(struct media_device *mdev) if (tuner) { if (if_vid) { pad_source = media_get_pad_index(tuner, false, - PAD_SIGNAL_ANALOG); + MEDIA_PAD_SIGNAL_ANALOG); pad_sink = media_get_pad_index(if_vid, true, - PAD_SIGNAL_ANALOG); + MEDIA_PAD_SIGNAL_ANALOG); if (pad_source < 0 || pad_sink < 0) { dev_warn(mdev->dev, "Couldn't get tuner and/or PLL pad(s): (%d, %d)\n", pad_source, pad_sink); @@ -123,9 +123,9 @@ int v4l2_mc_create_media_graph(struct media_device *mdev) } pad_source = media_get_pad_index(if_vid, false, - PAD_SIGNAL_ANALOG); + MEDIA_PAD_SIGNAL_ANALOG); pad_sink = media_get_pad_index(decoder, true, - PAD_SIGNAL_ANALOG); + MEDIA_PAD_SIGNAL_ANALOG); if (pad_source < 0 || pad_sink < 0) { dev_warn(mdev->dev, "get decoder and/or PLL pad(s): (%d, %d)\n", pad_source, pad_sink); @@ -140,9 +140,9 @@ int v4l2_mc_create_media_graph(struct media_device *mdev) } } else { pad_source = media_get_pad_index(tuner, false, - PAD_SIGNAL_ANALOG); + MEDIA_PAD_SIGNAL_ANALOG); pad_sink = media_get_pad_index(decoder, true, - PAD_SIGNAL_ANALOG); + MEDIA_PAD_SIGNAL_ANALOG); if (pad_source < 0 || pad_sink < 0) { dev_warn(mdev->dev, "couldn't get tuner and/or decoder pad(s): (%d, %d)\n", pad_source, pad_sink); @@ -157,9 +157,9 @@ int v4l2_mc_create_media_graph(struct media_device *mdev) if (if_aud) { pad_source = media_get_pad_index(tuner, false, - PAD_SIGNAL_AUDIO); + MEDIA_PAD_SIGNAL_AUDIO); pad_sink = media_get_pad_index(if_aud, true, - PAD_SIGNAL_AUDIO); + MEDIA_PAD_SIGNAL_AUDIO); if (pad_source < 0 || pad_sink < 0) { dev_warn(mdev->dev, "couldn't get tuner and/or decoder pad(s) for audio: (%d, %d)\n", pad_source, pad_sink); @@ -180,7 +180,7 @@ int v4l2_mc_create_media_graph(struct media_device *mdev) /* Create demod to V4L, VBI and SDR radio links */ if (io_v4l) { - pad_source = media_get_pad_index(decoder, false, PAD_SIGNAL_DV); + pad_source = media_get_pad_index(decoder, false, MEDIA_PAD_SIGNAL_DV); if (pad_source < 0) { dev_warn(mdev->dev, "couldn't get decoder output pad for V4L I/O\n"); return -EINVAL; @@ -195,7 +195,7 @@ int v4l2_mc_create_media_graph(struct media_device *mdev) } if (io_swradio) { - pad_source = media_get_pad_index(decoder, false, PAD_SIGNAL_DV); + pad_source = media_get_pad_index(decoder, false, MEDIA_PAD_SIGNAL_DV); if (pad_source < 0) { dev_warn(mdev->dev, "couldn't get decoder output pad for SDR\n"); return -EINVAL; @@ -210,7 +210,7 @@ int v4l2_mc_create_media_graph(struct media_device *mdev) } if (io_vbi) { - pad_source = media_get_pad_index(decoder, false, PAD_SIGNAL_DV); + pad_source = media_get_pad_index(decoder, false, MEDIA_PAD_SIGNAL_DV); if (pad_source < 0) { dev_warn(mdev->dev, "couldn't get decoder output pad for VBI\n"); return -EINVAL; @@ -232,7 +232,7 @@ int v4l2_mc_create_media_graph(struct media_device *mdev) if (!tuner) continue; pad_sink = media_get_pad_index(tuner, true, - PAD_SIGNAL_ANALOG); + MEDIA_PAD_SIGNAL_ANALOG); if (pad_sink < 0) { dev_warn(mdev->dev, "couldn't get tuner analog pad sink\n"); return -EINVAL; @@ -244,7 +244,7 @@ int v4l2_mc_create_media_graph(struct media_device *mdev) case MEDIA_ENT_F_CONN_SVIDEO: case MEDIA_ENT_F_CONN_COMPOSITE: pad_sink = media_get_pad_index(decoder, true, - PAD_SIGNAL_ANALOG); + MEDIA_PAD_SIGNAL_ANALOG); if (pad_sink < 0) { dev_warn(mdev->dev, "couldn't get tuner analog pad sink\n"); return -EINVAL; diff --git a/include/media/media-entity.h b/include/media/media-entity.h index e5f6960d92f6..837f806593f5 100644 --- a/include/media/media-entity.h +++ b/include/media/media-entity.h @@ -156,31 +156,31 @@ struct media_link { }; /** - * enum media_pad_signal_type - type of the signal inside a media pad + * enum media_pad_signal_type - type of the signal carried by a media pad * - * @PAD_SIGNAL_DEFAULT: - * Default signal. Use this when all inputs or all outputs are - * uniquely identified by the pad number. - * @PAD_SIGNAL_ANALOG: - * The pad contains an analog signal. It can be Radio Frequency, + * @MEDIA_PAD_SIGNAL_DEFAULT: + * Default signal. Default signal. Use this when there's no need to + * specify the signal carried inside a pad. + * @MEDIA_PAD_SIGNAL_ANALOG: + * The pad carries an analog signal. It can be Radio Frequency, * Intermediate Frequency, a baseband signal or sub-cariers. * Tuner inputs, IF-PLL demodulators, composite and s-video signals * should use it. - * @PAD_SIGNAL_DV: - * Contains a digital video signal, with can be a bitstream of samples + * @MEDIA_PAD_SIGNAL_DV: + * Pad carries a digital video signal, with can be a bitstream of samples * taken from an analog TV video source. On such case, it usually * contains the VBI data on it. - * @PAD_SIGNAL_AUDIO: - * Contains an Intermediate Frequency analog signal from an audio + * @MEDIA_PAD_SIGNAL_AUDIO: + * Pad carries an Intermediate Frequency analog signal from an audio * sub-carrier or an audio bitstream. IF signals are provided by tuners * and consumed by audio AM/FM decoders. Bitstream audio is provided by * an audio decoder. */ enum media_pad_signal_type { - PAD_SIGNAL_DEFAULT = 0, - PAD_SIGNAL_ANALOG, - PAD_SIGNAL_DV, - PAD_SIGNAL_AUDIO, + MEDIA_PAD_SIGNAL_DEFAULT = 0, + MEDIA_PAD_SIGNAL_ANALOG, + MEDIA_PAD_SIGNAL_DV, + MEDIA_PAD_SIGNAL_AUDIO, }; /** @@ -189,7 +189,11 @@ enum media_pad_signal_type { * @graph_obj: Embedded structure containing the media object common data * @entity: Entity this pad belongs to * @index: Pad index in the entity pads array, numbered from 0 to n - * @sig_type: Type of the signal inside a media pad + * @sig_type: Type of the signal that a media pad supports. This is used + * when an entity function may have different kinds of signals + * on different pads of the same direction (source/sink). For + * example, if an entity has 2 pad sinks where one carries + * an analog video while the other carries an audio signal. * @flags: Pad flags, as defined in * :ref:`include/uapi/linux/media.h <media_header>` * (seek for ``MEDIA_PAD_FL_*``)
diff --git a/drivers/media/dvb-frontends/au8522_decoder.c b/drivers/media/dvb-frontends/au8522_decoder.c index 198dd2b6f326..583fdaa7339f 100644 --- a/drivers/media/dvb-frontends/au8522_decoder.c +++ b/drivers/media/dvb-frontends/au8522_decoder.c @@ -719,8 +719,11 @@ static int au8522_probe(struct i2c_client *client, #if defined(CONFIG_MEDIA_CONTROLLER) state->pads[DEMOD_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK; + state->pads[DEMOD_PAD_IF_INPUT].sig_type = PAD_SIGNAL_ANALOG; state->pads[DEMOD_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE; + state->pads[DEMOD_PAD_VID_OUT].sig_type = PAD_SIGNAL_DV; state->pads[DEMOD_PAD_AUDIO_OUT].flags = MEDIA_PAD_FL_SOURCE; + state->pads[DEMOD_PAD_AUDIO_OUT].sig_type = PAD_SIGNAL_AUDIO; sd->entity.function = MEDIA_ENT_F_ATV_DECODER; ret = media_entity_pads_init(&sd->entity, ARRAY_SIZE(state->pads), diff --git a/drivers/media/i2c/msp3400-driver.c b/drivers/media/i2c/msp3400-driver.c index 3db966db83eb..3b9c729fbd52 100644 --- a/drivers/media/i2c/msp3400-driver.c +++ b/drivers/media/i2c/msp3400-driver.c @@ -704,7 +704,9 @@ static int msp_probe(struct i2c_client *client, const struct i2c_device_id *id) #if defined(CONFIG_MEDIA_CONTROLLER) state->pads[IF_AUD_DEC_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK; + state->pads[IF_AUD_DEC_PAD_IF_INPUT].sig_type = PAD_SIGNAL_AUDIO; state->pads[IF_AUD_DEC_PAD_OUT].flags = MEDIA_PAD_FL_SOURCE; + state->pads[IF_AUD_DEC_PAD_OUT].sig_type = PAD_SIGNAL_AUDIO; sd->entity.function = MEDIA_ENT_F_IF_AUD_DECODER; diff --git a/drivers/media/i2c/saa7115.c b/drivers/media/i2c/saa7115.c index 4c72db58dfd2..8798a06c212f 100644 --- a/drivers/media/i2c/saa7115.c +++ b/drivers/media/i2c/saa7115.c @@ -1835,7 +1835,9 @@ static int saa711x_probe(struct i2c_client *client, #if defined(CONFIG_MEDIA_CONTROLLER) state->pads[DEMOD_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK; + state->pads[DEMOD_PAD_IF_INPUT].sig_type = PAD_SIGNAL_ANALOG; state->pads[DEMOD_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE; + state->pads[DEMOD_PAD_VID_OUT].sig_type = PAD_SIGNAL_DV; sd->entity.function = MEDIA_ENT_F_ATV_DECODER; diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c index 66235e10acdd..5037a03b5442 100644 --- a/drivers/media/i2c/tvp5150.c +++ b/drivers/media/i2c/tvp5150.c @@ -1500,7 +1500,9 @@ static int tvp5150_probe(struct i2c_client *c, #if defined(CONFIG_MEDIA_CONTROLLER) core->pads[DEMOD_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK; + core->pads[DEMOD_PAD_IF_INPUT].sig_type = PAD_SIGNAL_ANALOG; core->pads[DEMOD_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE; + core->pads[DEMOD_PAD_VID_OUT].sig_type = PAD_SIGNAL_DV; sd->entity.function = MEDIA_ENT_F_ATV_DECODER; diff --git a/drivers/media/pci/saa7134/saa7134-core.c b/drivers/media/pci/saa7134/saa7134-core.c index 267d143c3a48..c4e2df197bf9 100644 --- a/drivers/media/pci/saa7134/saa7134-core.c +++ b/drivers/media/pci/saa7134/saa7134-core.c @@ -846,7 +846,9 @@ static void saa7134_create_entities(struct saa7134_dev *dev) if (!decoder) { dev->demod.name = "saa713x"; dev->demod_pad[DEMOD_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK; + dev->demod_pad[DEMOD_PAD_IF_INPUT].sig_type = PAD_SIGNAL_ANALOG; dev->demod_pad[DEMOD_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE; + dev->demod_pad[DEMOD_PAD_VID_OUT].sig_type = PAD_SIGNAL_DV; dev->demod.function = MEDIA_ENT_F_ATV_DECODER; ret = media_entity_pads_init(&dev->demod, DEMOD_NUM_PADS, diff --git a/drivers/media/tuners/si2157.c b/drivers/media/tuners/si2157.c index 9e34d31d724d..d222912662d7 100644 --- a/drivers/media/tuners/si2157.c +++ b/drivers/media/tuners/si2157.c @@ -469,8 +469,11 @@ static int si2157_probe(struct i2c_client *client, dev->ent.function = MEDIA_ENT_F_TUNER; dev->pad[TUNER_PAD_RF_INPUT].flags = MEDIA_PAD_FL_SINK; + dev->pad[TUNER_PAD_RF_INPUT].sig_type = PAD_SIGNAL_ANALOG; dev->pad[TUNER_PAD_OUTPUT].flags = MEDIA_PAD_FL_SOURCE; + dev->pad[TUNER_PAD_OUTPUT].sig_type = PAD_SIGNAL_TV_CARRIERS; dev->pad[TUNER_PAD_AUD_OUT].flags = MEDIA_PAD_FL_SOURCE; + dev->pad[TUNER_PAD_AUD_OUT].sig_type = PAD_SIGNAL_AUDIO; ret = media_entity_pads_init(&dev->ent, TUNER_NUM_PADS, &dev->pad[0]); diff --git a/drivers/media/usb/dvb-usb-v2/mxl111sf.c b/drivers/media/usb/dvb-usb-v2/mxl111sf.c index 4713ba65e1c2..51a1b26199e6 100644 --- a/drivers/media/usb/dvb-usb-v2/mxl111sf.c +++ b/drivers/media/usb/dvb-usb-v2/mxl111sf.c @@ -893,7 +893,9 @@ static int mxl111sf_attach_tuner(struct dvb_usb_adapter *adap) state->tuner.function = MEDIA_ENT_F_TUNER; state->tuner.name = "mxl111sf tuner"; state->tuner_pads[TUNER_PAD_RF_INPUT].flags = MEDIA_PAD_FL_SINK; + state->tuner_pads[TUNER_PAD_RF_INPUT].sig_type = PAD_SIGNAL_ANALOG; state->tuner_pads[TUNER_PAD_OUTPUT].flags = MEDIA_PAD_FL_SOURCE; + state->tuner_pads[TUNER_PAD_OUTPUT].sig_type = PAD_SIGNAL_TV_CARRIERS; ret = media_entity_pads_init(&state->tuner, TUNER_NUM_PADS, state->tuner_pads); diff --git a/drivers/media/v4l2-core/tuner-core.c b/drivers/media/v4l2-core/tuner-core.c index 7f858c39753c..d4c32ccd0930 100644 --- a/drivers/media/v4l2-core/tuner-core.c +++ b/drivers/media/v4l2-core/tuner-core.c @@ -685,15 +685,20 @@ static int tuner_probe(struct i2c_client *client, */ if (t->type == TUNER_TDA9887) { t->pad[IF_VID_DEC_PAD_IF_INPUT].flags = MEDIA_PAD_FL_SINK; + t->pad[IF_VID_DEC_PAD_IF_INPUT].sig_type = PAD_SIGNAL_ANALOG; t->pad[IF_VID_DEC_PAD_OUT].flags = MEDIA_PAD_FL_SOURCE; + t->pad[IF_VID_DEC_PAD_OUT].sig_type = PAD_SIGNAL_DV; ret = media_entity_pads_init(&t->sd.entity, IF_VID_DEC_PAD_NUM_PADS, &t->pad[0]); t->sd.entity.function = MEDIA_ENT_F_IF_VID_DECODER; } else { t->pad[TUNER_PAD_RF_INPUT].flags = MEDIA_PAD_FL_SINK; + t->pad[TUNER_PAD_RF_INPUT].sig_type = PAD_SIGNAL_ANALOG; t->pad[TUNER_PAD_OUTPUT].flags = MEDIA_PAD_FL_SOURCE; + t->pad[TUNER_PAD_OUTPUT].sig_type = PAD_SIGNAL_TV_CARRIERS; t->pad[TUNER_PAD_AUD_OUT].flags = MEDIA_PAD_FL_SOURCE; + t->pad[TUNER_PAD_AUD_OUT].sig_type = PAD_SIGNAL_AUDIO; ret = media_entity_pads_init(&t->sd.entity, TUNER_NUM_PADS, &t->pad[0]); t->sd.entity.function = MEDIA_ENT_F_TUNER; diff --git a/include/media/media-entity.h b/include/media/media-entity.h index 3aa3d58d1d58..8bfbe6b59fa9 100644 --- a/include/media/media-entity.h +++ b/include/media/media-entity.h @@ -155,6 +155,40 @@ struct media_link { bool is_backlink; }; +/** + * struct media_pad_signal_type - type of the signal inside a media pad + * + * @PAD_SIGNAL_DEFAULT + * Default signal. Use this when all inputs or all outputs are + * uniquely identified by the pad number. + * @PAD_SIGNAL_ANALOG + * The pad contains an analog signa. It can be Radio Frequency, + * Intermediate Frequency or baseband signal. + * Tuner inputs, composite and s-video signals should use it. + * On tuner sources, this is used for digital TV demodulators and for + * IF-PLL demodulator like tda9887. + * @PAD_SIGNAL_TV_CARRIERS + * The pad contains analog signals carrying either a digital or an analog + * modulated (or baseband) signal. This is provided by tuner source + * pads and used by analog TV standard decoders and by digital TV demods. + * @PAD_SIGNAL_DV + * Contains a digital video signal, with can be a bitstream of samples + * taken from an analog TV video source. On such case, it usually + * contains the VBI data on it. + * @PAD_SIGNAL_AUDIO + * Contains an Intermediate Frequency analog signal from an audio + * sub-carrier or an audio bitstream. IF signals are provided by tuners + * and consumed by audio AM/FM decoders. Bitstream audio is provided by + * an audio decoder. + */ +enum media_pad_signal_type { + PAD_SIGNAL_DEFAULT = 0, + PAD_SIGNAL_ANALOG, + PAD_SIGNAL_TV_CARRIERS, + PAD_SIGNAL_DV, + PAD_SIGNAL_AUDIO, +}; + /** * struct media_pad - A media pad graph object. * @@ -169,6 +203,7 @@ struct media_pad { struct media_gobj graph_obj; /* must be first field in struct */ struct media_entity *entity; u16 index; + enum media_pad_signal_type sig_type; unsigned long flags; };
Consumer devices are provided with a wide diferent range of types supported by the same driver, allowing different configutations. In order to make easier to setup media controller links, "taint" pads with the signal type it carries. While here, get rid of DEMOD_PAD_VBI_OUT, as the signal it carries is actually the same as the normal video output. The difference happens at the video/VBI interface: - for VBI, only the hidden lines are streamed; - for video, the stream is usually cropped to hide the vbi lines. Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org> --- drivers/media/dvb-frontends/au8522_decoder.c | 3 ++ drivers/media/i2c/msp3400-driver.c | 2 ++ drivers/media/i2c/saa7115.c | 2 ++ drivers/media/i2c/tvp5150.c | 2 ++ drivers/media/pci/saa7134/saa7134-core.c | 2 ++ drivers/media/tuners/si2157.c | 3 ++ drivers/media/usb/dvb-usb-v2/mxl111sf.c | 2 ++ drivers/media/v4l2-core/tuner-core.c | 5 +++ include/media/media-entity.h | 35 ++++++++++++++++++++ 9 files changed, 56 insertions(+)