| Message ID | 1456844246-18778-4-git-send-email-laurent.pinchart+renesas@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 03/01/16 15:57, Laurent Pinchart wrote: > Code that processes media entities can require knowledge of the > structure type that embeds a particular media entity instance in order > to cast the entity to the proper object type. This needs is shown by the > presence of the is_media_entity_v4l2_io and is_media_entity_v4l2_subdev > functions. > > The implementation of those two functions relies on the entity function > field, which is both a wrong and an inefficient design, without even > mentioning the maintenance issue involved in updating the functions > every time a new entity function is added. Fix this by adding add a type > field to the media entity structure to carry the information. > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> Acked-by: Hans Verkuil <hans.verkuil@cisco.com> Thanks! Hans > --- > drivers/media/v4l2-core/v4l2-dev.c | 1 + > drivers/media/v4l2-core/v4l2-subdev.c | 1 + > include/media/media-entity.h | 75 ++++++++++++++++++----------------- > 3 files changed, 40 insertions(+), 37 deletions(-) > > diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c > index d8e5994cccf1..7e766a92e3d9 100644 > --- a/drivers/media/v4l2-core/v4l2-dev.c > +++ b/drivers/media/v4l2-core/v4l2-dev.c > @@ -735,6 +735,7 @@ static int video_register_media_controller(struct video_device *vdev, int type) > if (!vdev->v4l2_dev->mdev) > return 0; > > + vdev->entity.type = MEDIA_ENTITY_TYPE_VIDEO_DEVICE; > vdev->entity.function = MEDIA_ENT_F_UNKNOWN; > > switch (type) { > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c > index d63083803144..bb6e79f14bb8 100644 > --- a/drivers/media/v4l2-core/v4l2-subdev.c > +++ b/drivers/media/v4l2-core/v4l2-subdev.c > @@ -584,6 +584,7 @@ void v4l2_subdev_init(struct v4l2_subdev *sd, const struct v4l2_subdev_ops *ops) > sd->host_priv = NULL; > #if defined(CONFIG_MEDIA_CONTROLLER) > sd->entity.name = sd->name; > + sd->entity.type = MEDIA_ENTITY_TYPE_V4L2_SUBDEV; > sd->entity.function = MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN; > #endif > } > diff --git a/include/media/media-entity.h b/include/media/media-entity.h > index d58e29d3f239..cbd37530f6b8 100644 > --- a/include/media/media-entity.h > +++ b/include/media/media-entity.h > @@ -187,10 +187,37 @@ struct media_entity_operations { > }; > > /** > + * enum media_entity_type - Media entity type > + * > + * @MEDIA_ENTITY_TYPE_MEDIA_ENTITY: > + * The entity is a struct media_entity instance. > + * @MEDIA_ENTITY_TYPE_VIDEO_DEVICE: > + * The entity is a struct video_device instance. > + * @MEDIA_ENTITY_TYPE_V4L2_SUBDEV: > + * The entity is a struct v4l2_subdev instance. > + * > + * The entity type identifies the type of the object instance that implements > + * the struct media_entity instance. This allows runtime type identification of > + * media entities and safe casting to the project object type. For instance, a > + * media entity structure instance embedded in a v4l2_subdev structure instance > + * will have the type MEDIA_ENTITY_TYPE_V4L2_SUBDEV and can safely be cast to a > + * v4l2_subdev structure using the container_of() macro. > + * > + * Media entities can be instantiated without creating any derived object type, > + * in which case their type will be MEDIA_ENTITY_TYPE_MEDIA_ENTITY. > + */ > +enum media_entity_type { > + MEDIA_ENTITY_TYPE_MEDIA_ENTITY, > + MEDIA_ENTITY_TYPE_VIDEO_DEVICE, > + MEDIA_ENTITY_TYPE_V4L2_SUBDEV, > +}; > + > +/** > * struct media_entity - A media entity graph object. > * > * @graph_obj: Embedded structure containing the media object common data. > * @name: Entity name. > + * @type: Type of the object that implements the media_entity. > * @function: Entity main function, as defined in uapi/media.h > * (MEDIA_ENT_F_*) > * @flags: Entity flags, as defined in uapi/media.h (MEDIA_ENT_FL_*) > @@ -219,6 +246,7 @@ struct media_entity_operations { > struct media_entity { > struct media_gobj graph_obj; /* must be first field in struct */ > const char *name; > + enum media_entity_type type; > u32 function; > unsigned long flags; > > @@ -328,56 +356,29 @@ static inline u32 media_gobj_gen_id(enum media_gobj_type type, u64 local_id) > } > > /** > - * is_media_entity_v4l2_io() - identify if the entity main function > - * is a V4L2 I/O > - * > + * is_media_entity_v4l2_io() - Check if the entity is a video_device > * @entity: pointer to entity > * > - * Return: true if the entity main function is one of the V4L2 I/O types > - * (video, VBI or SDR radio); false otherwise. > + * Return: true if the entity is an instance of a video_device object and can > + * safely be cast to a struct video_device using the container_of() macro, or > + * false otherwise. > */ > static inline bool is_media_entity_v4l2_io(struct media_entity *entity) > { > - if (!entity) > - return false; > - > - switch (entity->function) { > - case MEDIA_ENT_F_IO_V4L: > - case MEDIA_ENT_F_IO_VBI: > - case MEDIA_ENT_F_IO_SWRADIO: > - return true; > - default: > - return false; > - } > + return entity && entity->type == MEDIA_ENTITY_TYPE_VIDEO_DEVICE; > } > > /** > - * is_media_entity_v4l2_subdev - return true if the entity main function is > - * associated with the V4L2 API subdev usage > - * > + * is_media_entity_v4l2_subdev() - Check if the entity is a v4l2_subdev > * @entity: pointer to entity > * > - * This is an ancillary function used by subdev-based V4L2 drivers. > - * It checks if the entity function is one of functions used by a V4L2 subdev, > - * e. g. camera-relatef functions, analog TV decoder, TV tuner, V4L2 DSPs. > + * Return: true if the entity is an instance of a v4l2_subdev object and can > + * safely be cast to a struct v4l2_subdev using the container_of() macro, or > + * false otherwise. > */ > static inline bool is_media_entity_v4l2_subdev(struct media_entity *entity) > { > - if (!entity) > - return false; > - > - switch (entity->function) { > - case MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN: > - case MEDIA_ENT_F_CAM_SENSOR: > - case MEDIA_ENT_F_FLASH: > - case MEDIA_ENT_F_LENS: > - case MEDIA_ENT_F_ATV_DECODER: > - case MEDIA_ENT_F_TUNER: > - return true; > - > - default: > - return false; > - } > + return entity && entity->type == MEDIA_ENTITY_TYPE_V4L2_SUBDEV; > } > > /** > -- 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/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c index d8e5994cccf1..7e766a92e3d9 100644 --- a/drivers/media/v4l2-core/v4l2-dev.c +++ b/drivers/media/v4l2-core/v4l2-dev.c @@ -735,6 +735,7 @@ static int video_register_media_controller(struct video_device *vdev, int type) if (!vdev->v4l2_dev->mdev) return 0; + vdev->entity.type = MEDIA_ENTITY_TYPE_VIDEO_DEVICE; vdev->entity.function = MEDIA_ENT_F_UNKNOWN; switch (type) { diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c index d63083803144..bb6e79f14bb8 100644 --- a/drivers/media/v4l2-core/v4l2-subdev.c +++ b/drivers/media/v4l2-core/v4l2-subdev.c @@ -584,6 +584,7 @@ void v4l2_subdev_init(struct v4l2_subdev *sd, const struct v4l2_subdev_ops *ops) sd->host_priv = NULL; #if defined(CONFIG_MEDIA_CONTROLLER) sd->entity.name = sd->name; + sd->entity.type = MEDIA_ENTITY_TYPE_V4L2_SUBDEV; sd->entity.function = MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN; #endif } diff --git a/include/media/media-entity.h b/include/media/media-entity.h index d58e29d3f239..cbd37530f6b8 100644 --- a/include/media/media-entity.h +++ b/include/media/media-entity.h @@ -187,10 +187,37 @@ struct media_entity_operations { }; /** + * enum media_entity_type - Media entity type + * + * @MEDIA_ENTITY_TYPE_MEDIA_ENTITY: + * The entity is a struct media_entity instance. + * @MEDIA_ENTITY_TYPE_VIDEO_DEVICE: + * The entity is a struct video_device instance. + * @MEDIA_ENTITY_TYPE_V4L2_SUBDEV: + * The entity is a struct v4l2_subdev instance. + * + * The entity type identifies the type of the object instance that implements + * the struct media_entity instance. This allows runtime type identification of + * media entities and safe casting to the project object type. For instance, a + * media entity structure instance embedded in a v4l2_subdev structure instance + * will have the type MEDIA_ENTITY_TYPE_V4L2_SUBDEV and can safely be cast to a + * v4l2_subdev structure using the container_of() macro. + * + * Media entities can be instantiated without creating any derived object type, + * in which case their type will be MEDIA_ENTITY_TYPE_MEDIA_ENTITY. + */ +enum media_entity_type { + MEDIA_ENTITY_TYPE_MEDIA_ENTITY, + MEDIA_ENTITY_TYPE_VIDEO_DEVICE, + MEDIA_ENTITY_TYPE_V4L2_SUBDEV, +}; + +/** * struct media_entity - A media entity graph object. * * @graph_obj: Embedded structure containing the media object common data. * @name: Entity name. + * @type: Type of the object that implements the media_entity. * @function: Entity main function, as defined in uapi/media.h * (MEDIA_ENT_F_*) * @flags: Entity flags, as defined in uapi/media.h (MEDIA_ENT_FL_*) @@ -219,6 +246,7 @@ struct media_entity_operations { struct media_entity { struct media_gobj graph_obj; /* must be first field in struct */ const char *name; + enum media_entity_type type; u32 function; unsigned long flags; @@ -328,56 +356,29 @@ static inline u32 media_gobj_gen_id(enum media_gobj_type type, u64 local_id) } /** - * is_media_entity_v4l2_io() - identify if the entity main function - * is a V4L2 I/O - * + * is_media_entity_v4l2_io() - Check if the entity is a video_device * @entity: pointer to entity * - * Return: true if the entity main function is one of the V4L2 I/O types - * (video, VBI or SDR radio); false otherwise. + * Return: true if the entity is an instance of a video_device object and can + * safely be cast to a struct video_device using the container_of() macro, or + * false otherwise. */ static inline bool is_media_entity_v4l2_io(struct media_entity *entity) { - if (!entity) - return false; - - switch (entity->function) { - case MEDIA_ENT_F_IO_V4L: - case MEDIA_ENT_F_IO_VBI: - case MEDIA_ENT_F_IO_SWRADIO: - return true; - default: - return false; - } + return entity && entity->type == MEDIA_ENTITY_TYPE_VIDEO_DEVICE; } /** - * is_media_entity_v4l2_subdev - return true if the entity main function is - * associated with the V4L2 API subdev usage - * + * is_media_entity_v4l2_subdev() - Check if the entity is a v4l2_subdev * @entity: pointer to entity * - * This is an ancillary function used by subdev-based V4L2 drivers. - * It checks if the entity function is one of functions used by a V4L2 subdev, - * e. g. camera-relatef functions, analog TV decoder, TV tuner, V4L2 DSPs. + * Return: true if the entity is an instance of a v4l2_subdev object and can + * safely be cast to a struct v4l2_subdev using the container_of() macro, or + * false otherwise. */ static inline bool is_media_entity_v4l2_subdev(struct media_entity *entity) { - if (!entity) - return false; - - switch (entity->function) { - case MEDIA_ENT_F_V4L2_SUBDEV_UNKNOWN: - case MEDIA_ENT_F_CAM_SENSOR: - case MEDIA_ENT_F_FLASH: - case MEDIA_ENT_F_LENS: - case MEDIA_ENT_F_ATV_DECODER: - case MEDIA_ENT_F_TUNER: - return true; - - default: - return false; - } + return entity && entity->type == MEDIA_ENTITY_TYPE_V4L2_SUBDEV; } /**
Code that processes media entities can require knowledge of the structure type that embeds a particular media entity instance in order to cast the entity to the proper object type. This needs is shown by the presence of the is_media_entity_v4l2_io and is_media_entity_v4l2_subdev functions. The implementation of those two functions relies on the entity function field, which is both a wrong and an inefficient design, without even mentioning the maintenance issue involved in updating the functions every time a new entity function is added. Fix this by adding add a type field to the media entity structure to carry the information. Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> --- drivers/media/v4l2-core/v4l2-dev.c | 1 + drivers/media/v4l2-core/v4l2-subdev.c | 1 + include/media/media-entity.h | 75 ++++++++++++++++++----------------- 3 files changed, 40 insertions(+), 37 deletions(-)