| Message ID | 2ddddaaaecbdbf624441793ca4c57e81530eaf05.1441559233.git.mchehab@osg.samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 09/06/2015 07:30 PM, Mauro Carvalho Chehab wrote: > Interface links are normally enabled, meaning that the interfaces are > bound to the entities. So, any ioctl send to the interface are reflected > at the entities managed by the interface. > > However, when a device is usage, other interfaces for the same hardware s/usage/used/ > could be decoupled from the entities linked to them, because the > hardware may have some parts busy. > > That's for example, what happens when an hybrid TV device is in usage. s/usage/use/ > If it is streaming analog TV or capturing signals from S-Video/Composite > connectors, typically the digital part of the hardware can't be used and > vice-versa. > > This is generally due to some internal hardware or firmware limitation, > that it is not easily mapped via data pipelines. > > What the Kernel drivers do internally is that they decouple the hardware > from the interface. So, all changes, if allowed, are done only at some > interface cache, but not physically changed at the hardware. > > The usage is similar to the usage of the MEDIA_LNK_FL_ENABLED on data > links. So, let's use the same flag to indicate if ether the interface s/ether/either/ > to entity link is bound/enabled or not. > > Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com> Acked-by: Hans Verkuil <hans.verkuil@cisco.com> > > diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c > index a8e7e2398f7a..5c4fb41060b4 100644 > --- a/drivers/media/dvb-core/dvbdev.c > +++ b/drivers/media/dvb-core/dvbdev.c > @@ -396,7 +396,8 @@ static void dvb_register_media_device(struct dvb_device *dvbdev, > if (!dvbdev->entity || !dvbdev->intf_devnode) > return; > > - media_create_intf_link(dvbdev->entity, &dvbdev->intf_devnode->intf, 0); > + media_create_intf_link(dvbdev->entity, &dvbdev->intf_devnode->intf, > + MEDIA_LNK_FL_ENABLED); > > #endif > } > @@ -583,20 +584,24 @@ void dvb_create_media_graph(struct dvb_adapter *adap) > /* Create indirect interface links for FE->tuner, DVR->demux and CA->ca */ > media_device_for_each_intf(intf, mdev) { > if (intf->type == MEDIA_INTF_T_DVB_CA && ca) > - media_create_intf_link(ca, intf, 0); > + media_create_intf_link(ca, intf, MEDIA_LNK_FL_ENABLED); > > if (intf->type == MEDIA_INTF_T_DVB_FE && tuner) > - media_create_intf_link(tuner, intf, 0); > + media_create_intf_link(tuner, intf, > + MEDIA_LNK_FL_ENABLED); > > if (intf->type == MEDIA_INTF_T_DVB_DVR && demux) > - media_create_intf_link(demux, intf, 0); > + media_create_intf_link(demux, intf, > + MEDIA_LNK_FL_ENABLED); > > media_device_for_each_entity(entity, mdev) { > if (entity->type == MEDIA_ENT_T_DVB_TSOUT) { > if (!strcmp(entity->name, DVR_TSOUT)) > - media_create_intf_link(entity, intf, 0); > + media_create_intf_link(entity, intf, > + MEDIA_LNK_FL_ENABLED); > if (!strcmp(entity->name, DEMUX_TSOUT)) > - media_create_intf_link(entity, intf, 0); > + media_create_intf_link(entity, intf, > + MEDIA_LNK_FL_ENABLED); > break; > } > } > diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c > index 07123dd569c4..8429da66754a 100644 > --- a/drivers/media/v4l2-core/v4l2-dev.c > +++ b/drivers/media/v4l2-core/v4l2-dev.c > @@ -788,7 +788,8 @@ static int video_register_media_controller(struct video_device *vdev, int type) > struct media_link *link; > > link = media_create_intf_link(&vdev->entity, > - &vdev->intf_devnode->intf, 0); > + &vdev->intf_devnode->intf, > + MEDIA_LNK_FL_ENABLED); > if (!link) { > media_devnode_remove(vdev->intf_devnode); > media_device_unregister_entity(&vdev->entity); > diff --git a/drivers/media/v4l2-core/v4l2-device.c b/drivers/media/v4l2-core/v4l2-device.c > index e788a085ba96..bb58d90fde5e 100644 > --- a/drivers/media/v4l2-core/v4l2-device.c > +++ b/drivers/media/v4l2-core/v4l2-device.c > @@ -256,7 +256,7 @@ int v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev) > > link = media_create_intf_link(&sd->entity, > &vdev->intf_devnode->intf, > - 0); > + MEDIA_LNK_FL_ENABLED); > if (!link) > goto clean_up; > } > -- 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
Hi Mauro, Thank you for the patch. On Sunday 06 September 2015 14:30:48 Mauro Carvalho Chehab wrote: > Interface links are normally enabled, meaning that the interfaces are > bound to the entities. So, any ioctl send to the interface are reflected s/send/sent/ > at the entities managed by the interface. > > However, when a device is usage, other interfaces for the same hardware s/usage/in use/ > could be decoupled from the entities linked to them, because the > hardware may have some parts busy. > > That's for example, what happens when an hybrid TV device is in usage. s/usage/use/ > If it is streaming analog TV or capturing signals from S-Video/Composite > connectors, typically the digital part of the hardware can't be used and > vice-versa. > > This is generally due to some internal hardware or firmware limitation, > that it is not easily mapped via data pipelines. > > What the Kernel drivers do internally is that they decouple the hardware > from the interface. So, all changes, if allowed, are done only at some > interface cache, but not physically changed at the hardware. > > The usage is similar to the usage of the MEDIA_LNK_FL_ENABLED on data > links. So, let's use the same flag to indicate if ether the interface s/ether/either/ > to entity link is bound/enabled or not. I believe we'll need to experiment with the interface links to see what's really needed there. As a general rule I'd like to avoid exposing too much information to userspace without a clear indication that the information is actually needed, as it's always easier to expose additional information later than to remove information already exposed. For this reason I'd like to see as a first step how we would do in userspace without making those links dynamic. If we then realize that we're lacking information we'll decide on the best course of action and on exactly what to expose and how to expose it, using concrete userspace use cases. > Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com> > > diff --git a/drivers/media/dvb-core/dvbdev.c > b/drivers/media/dvb-core/dvbdev.c index a8e7e2398f7a..5c4fb41060b4 100644 > --- a/drivers/media/dvb-core/dvbdev.c > +++ b/drivers/media/dvb-core/dvbdev.c > @@ -396,7 +396,8 @@ static void dvb_register_media_device(struct dvb_device > *dvbdev, if (!dvbdev->entity || !dvbdev->intf_devnode) > return; > > - media_create_intf_link(dvbdev->entity, &dvbdev->intf_devnode->intf, 0); > + media_create_intf_link(dvbdev->entity, &dvbdev->intf_devnode->intf, > + MEDIA_LNK_FL_ENABLED); > > #endif > } > @@ -583,20 +584,24 @@ void dvb_create_media_graph(struct dvb_adapter *adap) > /* Create indirect interface links for FE->tuner, DVR->demux and CA->ca */ > media_device_for_each_intf(intf, mdev) { > if (intf->type == MEDIA_INTF_T_DVB_CA && ca) > - media_create_intf_link(ca, intf, 0); > + media_create_intf_link(ca, intf, MEDIA_LNK_FL_ENABLED); > > if (intf->type == MEDIA_INTF_T_DVB_FE && tuner) > - media_create_intf_link(tuner, intf, 0); > + media_create_intf_link(tuner, intf, > + MEDIA_LNK_FL_ENABLED); > > if (intf->type == MEDIA_INTF_T_DVB_DVR && demux) > - media_create_intf_link(demux, intf, 0); > + media_create_intf_link(demux, intf, > + MEDIA_LNK_FL_ENABLED); > > media_device_for_each_entity(entity, mdev) { > if (entity->type == MEDIA_ENT_T_DVB_TSOUT) { > if (!strcmp(entity->name, DVR_TSOUT)) > - media_create_intf_link(entity, intf, 0); > + media_create_intf_link(entity, intf, > + MEDIA_LNK_FL_ENABLED); > if (!strcmp(entity->name, DEMUX_TSOUT)) > - media_create_intf_link(entity, intf, 0); > + media_create_intf_link(entity, intf, > + MEDIA_LNK_FL_ENABLED); > break; > } > } > diff --git a/drivers/media/v4l2-core/v4l2-dev.c > b/drivers/media/v4l2-core/v4l2-dev.c index 07123dd569c4..8429da66754a > 100644 > --- a/drivers/media/v4l2-core/v4l2-dev.c > +++ b/drivers/media/v4l2-core/v4l2-dev.c > @@ -788,7 +788,8 @@ static int video_register_media_controller(struct > video_device *vdev, int type) struct media_link *link; > > link = media_create_intf_link(&vdev->entity, > - &vdev->intf_devnode->intf, 0); > + &vdev->intf_devnode->intf, > + MEDIA_LNK_FL_ENABLED); > if (!link) { > media_devnode_remove(vdev->intf_devnode); > media_device_unregister_entity(&vdev->entity); > diff --git a/drivers/media/v4l2-core/v4l2-device.c > b/drivers/media/v4l2-core/v4l2-device.c index e788a085ba96..bb58d90fde5e > 100644 > --- a/drivers/media/v4l2-core/v4l2-device.c > +++ b/drivers/media/v4l2-core/v4l2-device.c > @@ -256,7 +256,7 @@ int v4l2_device_register_subdev_nodes(struct v4l2_device > *v4l2_dev) > > link = media_create_intf_link(&sd->entity, > &vdev->intf_devnode->intf, > - 0); > + MEDIA_LNK_FL_ENABLED); > if (!link) > goto clean_up; > }
Em Mon, 23 Nov 2015 21:46:53 +0200 Laurent Pinchart <laurent.pinchart@ideasonboard.com> escreveu: > Hi Mauro, > > Thank you for the patch. > > On Sunday 06 September 2015 14:30:48 Mauro Carvalho Chehab wrote: > > Interface links are normally enabled, meaning that the interfaces are > > bound to the entities. So, any ioctl send to the interface are reflected > > s/send/sent/ > > > at the entities managed by the interface. > > > > However, when a device is usage, other interfaces for the same hardware > > s/usage/in use/ > > > could be decoupled from the entities linked to them, because the > > hardware may have some parts busy. > > > > That's for example, what happens when an hybrid TV device is in usage. > > s/usage/use/ > > > If it is streaming analog TV or capturing signals from S-Video/Composite > > connectors, typically the digital part of the hardware can't be used and > > vice-versa. > > > > This is generally due to some internal hardware or firmware limitation, > > that it is not easily mapped via data pipelines. > > > > What the Kernel drivers do internally is that they decouple the hardware > > from the interface. So, all changes, if allowed, are done only at some > > interface cache, but not physically changed at the hardware. > > > > The usage is similar to the usage of the MEDIA_LNK_FL_ENABLED on data > > links. So, let's use the same flag to indicate if ether the interface > > s/ether/either/ > > > to entity link is bound/enabled or not. > > I believe we'll need to experiment with the interface links to see what's > really needed there. As a general rule I'd like to avoid exposing too much > information to userspace without a clear indication that the information is > actually needed, as it's always easier to expose additional information later > than to remove information already exposed. > > For this reason I'd like to see as a first step how we would do in userspace > without making those links dynamic. If we then realize that we're lacking > information we'll decide on the best course of action and on exactly what to > expose and how to expose it, using concrete userspace use cases. As discussed during MC workshop, the need of marking an interface link as active or not is because, on complex devices like hybrid TV ones, typically either the analog side of the digital side of the device is disabled. So, for example, if an hybrid device is working in digital mode, any change done via the V4L2 interfaces should not be applied to the hardware. The interface should either cache such changes to be applied the next time the device would switch to analog mode or return EBUSY. In other words, the link between the interface and the hardware entities are disabled. This is needed internally at the MC representation of the hardware (and one of the main reasons why we need MC on hybrid devices). Applications also need such information, as, except for a couple applications: - xawtv4, (with is not a popular one) - mythTV All other open source applications I'm aware of are either digital only or analog only and some radio applications even run as applets. So, the user may have more than one application running at the same time, being one active and the other would need to report that it cannot be activated because the device is busy. So, this change is actually needed. Anyway, I'm splitting this patch in two parts: one for DVB and another one for V4L, as I'm reordering some patches on the final rebase due to some troubles detected with KASAN and DEBUG_KMEMLEAK, and splitting it avoids merge conflicts. > > > Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com> > > > > diff --git a/drivers/media/dvb-core/dvbdev.c > > b/drivers/media/dvb-core/dvbdev.c index a8e7e2398f7a..5c4fb41060b4 100644 > > --- a/drivers/media/dvb-core/dvbdev.c > > +++ b/drivers/media/dvb-core/dvbdev.c > > @@ -396,7 +396,8 @@ static void dvb_register_media_device(struct dvb_device > > *dvbdev, if (!dvbdev->entity || !dvbdev->intf_devnode) > > return; > > > > - media_create_intf_link(dvbdev->entity, &dvbdev->intf_devnode->intf, 0); > > + media_create_intf_link(dvbdev->entity, &dvbdev->intf_devnode->intf, > > + MEDIA_LNK_FL_ENABLED); > > > > #endif > > } > > @@ -583,20 +584,24 @@ void dvb_create_media_graph(struct dvb_adapter *adap) > > /* Create indirect interface links for FE->tuner, DVR->demux and CA->ca > */ > > media_device_for_each_intf(intf, mdev) { > > if (intf->type == MEDIA_INTF_T_DVB_CA && ca) > > - media_create_intf_link(ca, intf, 0); > > + media_create_intf_link(ca, intf, MEDIA_LNK_FL_ENABLED); > > > > if (intf->type == MEDIA_INTF_T_DVB_FE && tuner) > > - media_create_intf_link(tuner, intf, 0); > > + media_create_intf_link(tuner, intf, > > + MEDIA_LNK_FL_ENABLED); > > > > if (intf->type == MEDIA_INTF_T_DVB_DVR && demux) > > - media_create_intf_link(demux, intf, 0); > > + media_create_intf_link(demux, intf, > > + MEDIA_LNK_FL_ENABLED); > > > > media_device_for_each_entity(entity, mdev) { > > if (entity->type == MEDIA_ENT_T_DVB_TSOUT) { > > if (!strcmp(entity->name, DVR_TSOUT)) > > - media_create_intf_link(entity, intf, 0); > > + media_create_intf_link(entity, intf, > > + MEDIA_LNK_FL_ENABLED); > > if (!strcmp(entity->name, DEMUX_TSOUT)) > > - media_create_intf_link(entity, intf, 0); > > + media_create_intf_link(entity, intf, > > + MEDIA_LNK_FL_ENABLED); > > break; > > } > > } > > diff --git a/drivers/media/v4l2-core/v4l2-dev.c > > b/drivers/media/v4l2-core/v4l2-dev.c index 07123dd569c4..8429da66754a > > 100644 > > --- a/drivers/media/v4l2-core/v4l2-dev.c > > +++ b/drivers/media/v4l2-core/v4l2-dev.c > > @@ -788,7 +788,8 @@ static int video_register_media_controller(struct > > video_device *vdev, int type) struct media_link *link; > > > > link = media_create_intf_link(&vdev->entity, > > - &vdev->intf_devnode->intf, 0); > > + &vdev->intf_devnode->intf, > > + MEDIA_LNK_FL_ENABLED); > > if (!link) { > > media_devnode_remove(vdev->intf_devnode); > > media_device_unregister_entity(&vdev->entity); > > diff --git a/drivers/media/v4l2-core/v4l2-device.c > > b/drivers/media/v4l2-core/v4l2-device.c index e788a085ba96..bb58d90fde5e > > 100644 > > --- a/drivers/media/v4l2-core/v4l2-device.c > > +++ b/drivers/media/v4l2-core/v4l2-device.c > > @@ -256,7 +256,7 @@ int v4l2_device_register_subdev_nodes(struct v4l2_device > > *v4l2_dev) > > > > link = media_create_intf_link(&sd->entity, > > &vdev->intf_devnode->intf, > > - 0); > > + MEDIA_LNK_FL_ENABLED); > > if (!link) > > goto clean_up; > > } > -- 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/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c index a8e7e2398f7a..5c4fb41060b4 100644 --- a/drivers/media/dvb-core/dvbdev.c +++ b/drivers/media/dvb-core/dvbdev.c @@ -396,7 +396,8 @@ static void dvb_register_media_device(struct dvb_device *dvbdev, if (!dvbdev->entity || !dvbdev->intf_devnode) return; - media_create_intf_link(dvbdev->entity, &dvbdev->intf_devnode->intf, 0); + media_create_intf_link(dvbdev->entity, &dvbdev->intf_devnode->intf, + MEDIA_LNK_FL_ENABLED); #endif } @@ -583,20 +584,24 @@ void dvb_create_media_graph(struct dvb_adapter *adap) /* Create indirect interface links for FE->tuner, DVR->demux and CA->ca */ media_device_for_each_intf(intf, mdev) { if (intf->type == MEDIA_INTF_T_DVB_CA && ca) - media_create_intf_link(ca, intf, 0); + media_create_intf_link(ca, intf, MEDIA_LNK_FL_ENABLED); if (intf->type == MEDIA_INTF_T_DVB_FE && tuner) - media_create_intf_link(tuner, intf, 0); + media_create_intf_link(tuner, intf, + MEDIA_LNK_FL_ENABLED); if (intf->type == MEDIA_INTF_T_DVB_DVR && demux) - media_create_intf_link(demux, intf, 0); + media_create_intf_link(demux, intf, + MEDIA_LNK_FL_ENABLED); media_device_for_each_entity(entity, mdev) { if (entity->type == MEDIA_ENT_T_DVB_TSOUT) { if (!strcmp(entity->name, DVR_TSOUT)) - media_create_intf_link(entity, intf, 0); + media_create_intf_link(entity, intf, + MEDIA_LNK_FL_ENABLED); if (!strcmp(entity->name, DEMUX_TSOUT)) - media_create_intf_link(entity, intf, 0); + media_create_intf_link(entity, intf, + MEDIA_LNK_FL_ENABLED); break; } } diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c index 07123dd569c4..8429da66754a 100644 --- a/drivers/media/v4l2-core/v4l2-dev.c +++ b/drivers/media/v4l2-core/v4l2-dev.c @@ -788,7 +788,8 @@ static int video_register_media_controller(struct video_device *vdev, int type) struct media_link *link; link = media_create_intf_link(&vdev->entity, - &vdev->intf_devnode->intf, 0); + &vdev->intf_devnode->intf, + MEDIA_LNK_FL_ENABLED); if (!link) { media_devnode_remove(vdev->intf_devnode); media_device_unregister_entity(&vdev->entity); diff --git a/drivers/media/v4l2-core/v4l2-device.c b/drivers/media/v4l2-core/v4l2-device.c index e788a085ba96..bb58d90fde5e 100644 --- a/drivers/media/v4l2-core/v4l2-device.c +++ b/drivers/media/v4l2-core/v4l2-device.c @@ -256,7 +256,7 @@ int v4l2_device_register_subdev_nodes(struct v4l2_device *v4l2_dev) link = media_create_intf_link(&sd->entity, &vdev->intf_devnode->intf, - 0); + MEDIA_LNK_FL_ENABLED); if (!link) goto clean_up; }
Interface links are normally enabled, meaning that the interfaces are bound to the entities. So, any ioctl send to the interface are reflected at the entities managed by the interface. However, when a device is usage, other interfaces for the same hardware could be decoupled from the entities linked to them, because the hardware may have some parts busy. That's for example, what happens when an hybrid TV device is in usage. If it is streaming analog TV or capturing signals from S-Video/Composite connectors, typically the digital part of the hardware can't be used and vice-versa. This is generally due to some internal hardware or firmware limitation, that it is not easily mapped via data pipelines. What the Kernel drivers do internally is that they decouple the hardware from the interface. So, all changes, if allowed, are done only at some interface cache, but not physically changed at the hardware. The usage is similar to the usage of the MEDIA_LNK_FL_ENABLED on data links. So, let's use the same flag to indicate if ether the interface to entity link is bound/enabled or not. Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>