[RFC] Media controller entity information ioctl [was "Re: [patch] suggestion for media framework"]

[RFC] Media controller entity information ioctl [was "Re: [patch] suggestion for media framework"]

[Laurent Pinchart]

Hi Oleksiy,

Thank you for the patch.

[CC'ing linux-media]

On Sunday 03 June 2012 19:17:06 Oleksij Rempel wrote:
> Hi Laurent,
> 
> in attachment is a suggestion patch for media framework and a test
> program which use this patch.
> 
> Suddenly we still didn't solved the problem with finding of XU. You
> know, the proper way to find them is guid (i do not need to explain this
> :)). Since uvc devices starting to have more and complicated XUs, media
> api is probably proper way to go - how you suggested.
> 
> On the wiki of TexasInstruments i found some code examples, how they use
> this api. And it looks like there is some desing differences between
> OMPA drivers and UVC. It is easy to find proper entity name for omap
> devices just by: "(!strcmp(entity[index].name, "OMAP3 ISP CCDC"))".
> We can't do the same for UVC, current names are just "Extension %u". We
> can put guid instead, but it will looks ugly and not really informative.
> This is why i added new struct uvc_ext.
> 
> If you do not agree with this patch, it will be good if you proved other
> solution. This problem need to be solved.

The patch goes in the right direction, in that I think the media controller 
API is the proper way to solve this problem. However, extending the 
media_entity_desc structure with information about all possible kinds of 
entities will not scale, especially given that an entity may need to expose 
information related to multiple types (for instance an XU need to expose its 
GUID, but also subdev-related information if it has a device node).

I've been thinking about adding a new ioctl to the media controller API for 
some time now, to report advanced static information about entities.

The idea is that each entity would be allowed to report an arbitrary number of 
static items. Items would have a type (for which we would likely need some 
kind of central registry, possible with driver-specific types), a length and 
data. The items would be static (registered an initialization time) and 
aggregated in a single buffer that would be read in one go through a new 
ioctl.

One important benefit of such an API would be to be able to report more than 
one entity type per subdev using entity type items. Many entities serve 
several purpose, for instance a sensor can integrate a flash controller. This 
can't be reported with the current API, as subdevs have a single type. By 
having several entity type items we could fix this issue.

Details remain to be drafted, but I'd like a feedback on the general approach.

-- 
Regards,

Laurent Pinchart

Re: [RFC] Media controller entity information ioctl [was "Re: [patch] suggestion for media framework"]

[Hans Verkuil]

On Mon June 4 2012 16:02:12 Laurent Pinchart wrote:
> Hi Oleksiy,
> 
> Thank you for the patch.
> 
> [CC'ing linux-media]
> 
> On Sunday 03 June 2012 19:17:06 Oleksij Rempel wrote:
> > Hi Laurent,
> > 
> > in attachment is a suggestion patch for media framework and a test
> > program which use this patch.
> > 
> > Suddenly we still didn't solved the problem with finding of XU. You
> > know, the proper way to find them is guid (i do not need to explain this
> > :)). Since uvc devices starting to have more and complicated XUs, media
> > api is probably proper way to go - how you suggested.
> > 
> > On the wiki of TexasInstruments i found some code examples, how they use
> > this api. And it looks like there is some desing differences between
> > OMPA drivers and UVC. It is easy to find proper entity name for omap
> > devices just by: "(!strcmp(entity[index].name, "OMAP3 ISP CCDC"))".
> > We can't do the same for UVC, current names are just "Extension %u". We
> > can put guid instead, but it will looks ugly and not really informative.
> > This is why i added new struct uvc_ext.
> > 
> > If you do not agree with this patch, it will be good if you proved other
> > solution. This problem need to be solved.
> 
> The patch goes in the right direction, in that I think the media controller 
> API is the proper way to solve this problem. However, extending the 
> media_entity_desc structure with information about all possible kinds of 
> entities will not scale, especially given that an entity may need to expose 
> information related to multiple types (for instance an XU need to expose its 
> GUID, but also subdev-related information if it has a device node).
> 
> I've been thinking about adding a new ioctl to the media controller API for 
> some time now, to report advanced static information about entities.
> 
> The idea is that each entity would be allowed to report an arbitrary number of 
> static items. Items would have a type (for which we would likely need some 
> kind of central registry, possible with driver-specific types), a length and 
> data. The items would be static (registered an initialization time) and 
> aggregated in a single buffer that would be read in one go through a new 
> ioctl.

And since it is static the media_entity_desc struct can tell the caller how many
of these items there are, and use that information to retrieve them.

> One important benefit of such an API would be to be able to report more than 
> one entity type per subdev using entity type items. Many entities serve 
> several purpose, for instance a sensor can integrate a flash controller. This 
> can't be reported with the current API, as subdevs have a single type. By 
> having several entity type items we could fix this issue.
> 
> Details remain to be drafted, but I'd like a feedback on the general approach.

Sound good. We discussed this before, and I agree that these seems to be the
right approach.

Regards,

	Hans

Re: [RFC] Media controller entity information ioctl [was "Re: [patch] suggestion for media framework"]

[Oleksij Rempel]

On 04.06.2012 16:02, Laurent Pinchart wrote:
> Hi Oleksiy,
> 
> Thank you for the patch.
> 
> [CC'ing linux-media]
> 
> On Sunday 03 June 2012 19:17:06 Oleksij Rempel wrote:
>> Hi Laurent,
>>
>> in attachment is a suggestion patch for media framework and a test
>> program which use this patch.
>>
>> Suddenly we still didn't solved the problem with finding of XU. You
>> know, the proper way to find them is guid (i do not need to explain this
>> :)). Since uvc devices starting to have more and complicated XUs, media
>> api is probably proper way to go - how you suggested.
>>
>> On the wiki of TexasInstruments i found some code examples, how they use
>> this api. And it looks like there is some desing differences between
>> OMPA drivers and UVC. It is easy to find proper entity name for omap
>> devices just by: "(!strcmp(entity[index].name, "OMAP3 ISP CCDC"))".
>> We can't do the same for UVC, current names are just "Extension %u". We
>> can put guid instead, but it will looks ugly and not really informative.
>> This is why i added new struct uvc_ext.
>>
>> If you do not agree with this patch, it will be good if you proved other
>> solution. This problem need to be solved.
> 
> The patch goes in the right direction, in that I think the media controller 
> API is the proper way to solve this problem. However, extending the 
> media_entity_desc structure with information about all possible kinds of 
> entities will not scale, especially given that an entity may need to expose 
> information related to multiple types (for instance an XU need to expose its 
> GUID, but also subdev-related information if it has a device node).
> 
> I've been thinking about adding a new ioctl to the media controller API for 
> some time now, to report advanced static information about entities.
> 
> The idea is that each entity would be allowed to report an arbitrary number of 
> static items. Items would have a type (for which we would likely need some 
> kind of central registry, possible with driver-specific types), a length and 
> data. The items would be static (registered an initialization time) and 
> aggregated in a single buffer that would be read in one go through a new 
> ioctl.
> 
> One important benefit of such an API would be to be able to report more than 
> one entity type per subdev using entity type items. Many entities serve 
> several purpose, for instance a sensor can integrate a flash controller. This 
> can't be reported with the current API, as subdevs have a single type. By 
> having several entity type items we could fix this issue.
> 
> Details remain to be drafted, but I'd like a feedback on the general approach.

Currently i can talk only about uvc :)
Normally entity sub/type can be created by driver and device where
documentation is available. For uvc it is not the case. If we get some
comlicated uvc device with decumentation about XU, we will need
userspace library to map all needed entities. But if you wont to make it
static, created on driver init, then we will need some kind of plugin
interface for uvcvideo.... (this sounds scary ;)) or remapable entities.

Then one more question:
for the case with uvcvideo + usb-audio, one of driver may reset complete
device if it get some timeout or error. Make it sense to use media api
to avoid it? For example uvcvideo set enitity state to busy, and if
audio need to reset it and handler is not usb-audio for busy device,
then it should notify handler...


-- 
Regards,
Oleksij

Re: [RFC] Media controller entity information ioctl [was "Re: [patch] suggestion for media framework"]

[Sakari Ailus]

Hi Laurent,

Laurent Pinchart wrote:
> Hi Oleksiy,
> 
> Thank you for the patch.
> 
> [CC'ing linux-media]
> 
> On Sunday 03 June 2012 19:17:06 Oleksij Rempel wrote:
>> Hi Laurent,
>>
>> in attachment is a suggestion patch for media framework and a test
>> program which use this patch.
>>
>> Suddenly we still didn't solved the problem with finding of XU. You
>> know, the proper way to find them is guid (i do not need to explain this
>> :)). Since uvc devices starting to have more and complicated XUs, media
>> api is probably proper way to go - how you suggested.
>>
>> On the wiki of TexasInstruments i found some code examples, how they use
>> this api. And it looks like there is some desing differences between
>> OMPA drivers and UVC. It is easy to find proper entity name for omap
>> devices just by: "(!strcmp(entity[index].name, "OMAP3 ISP CCDC"))".
>> We can't do the same for UVC, current names are just "Extension %u". We
>> can put guid instead, but it will looks ugly and not really informative.
>> This is why i added new struct uvc_ext.
>>
>> If you do not agree with this patch, it will be good if you proved other
>> solution. This problem need to be solved.
> 
> The patch goes in the right direction, in that I think the media controller 
> API is the proper way to solve this problem. However, extending the 
> media_entity_desc structure with information about all possible kinds of 
> entities will not scale, especially given that an entity may need to expose 
> information related to multiple types (for instance an XU need to expose its 
> GUID, but also subdev-related information if it has a device node).
> 
> I've been thinking about adding a new ioctl to the media controller API for 
> some time now, to report advanced static information about entities.
> 
> The idea is that each entity would be allowed to report an arbitrary number of 
> static items. Items would have a type (for which we would likely need some 
> kind of central registry, possible with driver-specific types), a length and 
> data. The items would be static (registered an initialization time) and 
> aggregated in a single buffer that would be read in one go through a new 
> ioctl.
> 
> One important benefit of such an API would be to be able to report more than 
> one entity type per subdev using entity type items. Many entities serve 
> several purpose, for instance a sensor can integrate a flash controller. This 
> can't be reported with the current API, as subdevs have a single type. By 
> having several entity type items we could fix this issue.

I welcome this idea!

Another example of information that's missing currently is the lack of
bus information for the entities: it's next to impossible for the user
space to learn which i2c device a subdev is related to. At the same time
we could deprecate the media_entity_desc.type field.

Providing entity bus information as part of entity enumeration would
resolve this issue.

Btw. do you think a new IOCTL is really required? Why not to just add a
pointer for additional data the user may provide to the driver to fill
up? There's plenty of room in the struct for a pointer and perhaps a
size field.

Cheers,

-- 
Sakari Ailus
sakari.ailus@iki.fi

Re: [linux-uvc-devel] [RFC] Media controller entity information ioctl [was "Re: [patch] suggestion for media framework"]

[Laurent Pinchart]

Hi Robert,

On Monday 04 June 2012 10:11:33 Robert Krakora wrote:
> When you say "static" you mean items that are "well known" by the system by
> reading a registry at initialization?

By static I mean items that are initialized at driver initialization time and 
not modified afterwards. I don't think we should support adding/removing items 
at runtime, at least in the first version.

> When a new device exposes functionality that necessitates the creation of a
> new "static" item then how does the registry get updated to reflect this or
> am I misunderstanding?

Item types should be defined in a kernel header and documented. If a driver 
needs a new item types, the driver developer should add the new type to the 
header and document it.

-- 
Regards,

Laurent Pinchart

Re: [linux-uvc-devel] [RFC] Media controller entity information ioctl [was "Re: [patch] suggestion for media framework"]

[Oleksij Rempel]

On 08.06.2012 04:46, Laurent Pinchart wrote:
> Hi Robert,
> 
> On Monday 04 June 2012 10:11:33 Robert Krakora wrote:
>> When you say "static" you mean items that are "well known" by the system by
>> reading a registry at initialization?
> 
> By static I mean items that are initialized at driver initialization time and 
> not modified afterwards. I don't think we should support adding/removing items 
> at runtime, at least in the first version.
> 
>> When a new device exposes functionality that necessitates the creation of a
>> new "static" item then how does the registry get updated to reflect this or
>> am I misunderstanding?
> 
> Item types should be defined in a kernel header and documented. If a driver 
> needs a new item types, the driver developer should add the new type to the 
> header and document it.

Hi Laurent,

what is your progress on this issue?

I was able to make video stream on my webcam work more stable by setting
"snd_usb_audio ignore_ctl_error=1". I think it is one of cases where
media framework should help.

-- 
Regards,
Oleksij

Re: [linux-uvc-devel] [RFC] Media controller entity information ioctl [was "Re: [patch] suggestion for media framework"]

[Laurent Pinchart]

Hi,

On Thursday 21 June 2012 19:11:14 Oleksij Rempel wrote:
> On 08.06.2012 04:46, Laurent Pinchart wrote:
> > On Monday 04 June 2012 10:11:33 Robert Krakora wrote:
> >> When you say "static" you mean items that are "well known" by the system
> >> by reading a registry at initialization?
> > 
> > By static I mean items that are initialized at driver initialization time
> > and not modified afterwards. I don't think we should support
> > adding/removing items at runtime, at least in the first version.
> > 
> >> When a new device exposes functionality that necessitates the creation of
> >> a new "static" item then how does the registry get updated to reflect
> >> this or am I misunderstanding?
> > 
> > Item types should be defined in a kernel header and documented. If a
> > driver needs a new item types, the driver developer should add the new
> > type to the header and document it.
> 
> Hi Laurent,
> 
> what is your progress on this issue?

None unfortunately. I won't have time to work on this before July at the 
earliest.

> I was able to make video stream on my webcam work more stable by setting
> "snd_usb_audio ignore_ctl_error=1". I think it is one of cases where
> media framework should help.

-- 
Regards,

Laurent Pinchart