Em Tue, 04 Aug 2015 14:26:04 +0200
Hans Verkuil <hverkuil@xxxxxxxxx> escreveu:
>
>
> On 08/04/2015 01:41 PM, Mauro Carvalho Chehab wrote:
> > Due to the MC API proposed changes, we'll need to:
> > - have an unique object ID for all graph objects;
> > - be able to dynamically create/remove objects;
> > - be able to group objects;
> > - keep the object in memory until we stop use it.
> >
> > Due to that, create a struct media_graph_obj and put there the
> > common elements that all media objects will have in common.
> >
> > Signed-off-by: Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxxxx>
> >
> > diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> > index 0c003d817493..faead169fe32 100644
> > --- a/include/media/media-entity.h
> > +++ b/include/media/media-entity.h
> > @@ -27,11 +27,54 @@
> > #include <linux/kernel.h>
> > #include <linux/list.h>
> > #include <linux/media.h>
> > +#include <linux/kref.h>
> > +
> > +/* Enums used internally at the media controller to represent graphs */
> > +
> > +/**
> > + * enum media_graph_type - type of a graph element
> > + *
> > + * @MEDIA_GRAPH_ENTITY: Identify a media entity
> > + * @MEDIA_GRAPH_PAD: Identify a media PAD
> > + * @MEDIA_GRAPH_LINK: Identify a media link
> > + */
> > +enum media_graph_type {
> > + MEDIA_GRAPH_ENTITY,
> > + MEDIA_GRAPH_PAD,
> > + MEDIA_GRAPH_LINK,
> > +};
> > +
> > +
> > +/* Structs to represent the objects that belong to a media graph */
> > +
> > +/**
> > + * struct media_graph_obj - Define a graph object.
> > + *
> > + * @list: List of media graph objects
> > + * @obj_id: Non-zero object ID identifier. The ID should be unique
> > + * inside a media_device
> > + * @type: Type of the graph object
> > + * @mdev: Media device that contains the object
> > + * @kref: pointer to struct kref, used to avoid destroying the
> > + * object before stopping using it
> > + *
> > + * All elements on the media graph should have this struct embedded
> > + */
> > +struct media_graph_obj {
> > + struct list_head list;
> > + struct list_head group;
> > + u32 obj_id;
> > + enum media_graph_type type;
>
> I think that using the top 8 bits of the ID for the type and the lower 24 bits for
> the ID is more efficient. I proposed this in my RFC.
Oh, somehow I missed it. I'll read and comment it in a few.
>
> The IDs are still unique, but you don't need to keep track of the type since that's
> encoded in the ID. You'd need an array of MAX_TYPE atomics to get per-type unique IDs,
> but that's trivial.
That could be done, but I'm not sure about that. The code becomes a
little more complex. The interface will also be more complex. We're
already having enough complexity without that...
So, I would prefer to keep this in separate, at least for now. We
can latter change it (of course before finishing the final interface).
This is not a blocker to me, though.
Maybe, for now, I can add one define to get object type. This way, we can
easily replace the code latter to merge both ID and types into one u32.
Something like:
#define obj_type(gobj) ((gobj)->type)
> It also avoid the need for a connector flag since that would be
> a distinct type (although using the same media_entity struct).
Well, this can be done anyway, no matter if we encap type/ID into a
single u32 or not.
For me, a connector entity makes sense. It also makes sense, at least
at Kernel level, to have more than one interface type, but I'll discuss
that latter on, when I add the devnode interface at a new patch series.
> This is not a blocker for me, but I'd like to hear what others think.
>
> Otherwise this patch series looks OK.
Thanks for review!
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
