Re: [PATCH v2 03/27] media: media-entity: Introduce media_entity_context

[Michael Riesch <michael.riesch@collabora.com>]

Hi Jacopo,

Thanks for your patch. Looks good to me, but found a few typos.

On 7/24/25 16:10, Jacopo Mondi wrote:
> Introduce the 'struct media_entity_context' type, which serves for
> reference counting and introduce two new media entity operations to
> allow drivers to allocate and free a media entity context.
> 
> The newly introduced type will be used as a base type for the
> device context types (video_device_context and v4l2_subdevice_context)
> that will be introduced in the next patches.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---
>  drivers/media/mc/mc-entity.c |  46 ++++++++++++++++++++
>  include/media/media-entity.h | 101 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 147 insertions(+)
> 
> diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
> index 045590905582054c46656e20463271b1f93fa6b4..b4a9f0a0aa7353d7a3333f20903980956b3df4a7 100644
> --- a/drivers/media/mc/mc-entity.c
> +++ b/drivers/media/mc/mc-entity.c
> @@ -1673,3 +1673,49 @@ struct media_link *__media_entity_next_link(struct media_entity *entity,
>  	return NULL;
>  }
>  EXPORT_SYMBOL_GPL(__media_entity_next_link);
> +
> +static void media_entity_release_context(struct kref *refcount)
> +{
> +	struct media_entity_context *ctx =
> +		container_of(refcount, struct media_entity_context, refcount);
> +
> +	ctx->entity->ops->destroy_context(ctx);
> +}
> +
> +struct media_entity_context *
> +media_entity_context_get(struct media_entity_context *ctx)
> +{
> +	if (!ctx)
> +		return ERR_PTR(-EINVAL);
> +
> +	kref_get(&ctx->refcount);
> +
> +	return ctx;
> +}
> +EXPORT_SYMBOL_GPL(media_entity_context_get);
> +
> +void media_entity_context_put(struct media_entity_context *ctx)
> +{
> +	if (!ctx)
> +		return;
> +
> +	kref_put(&ctx->refcount, media_entity_release_context);
> +}
> +EXPORT_SYMBOL_GPL(media_entity_context_put);
> +
> +void media_entity_init_context(struct media_entity *entity,
> +			       struct media_entity_context *ctx)
> +{
> +	if (!ctx)
> +		return;
> +
> +	ctx->entity = entity;
> +	kref_init(&ctx->refcount);
> +	INIT_LIST_HEAD(&ctx->list);
> +}
> +EXPORT_SYMBOL_GPL(media_entity_init_context);
> +
> +void media_entity_cleanup_context(struct media_entity_context *ctx)
> +{
> +}
> +EXPORT_SYMBOL_GPL(media_entity_cleanup_context);
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index 64cf590b11343f68a456c5870ca2f32917c122f9..32298fe8a18c6ee3c1dbcff9ef869548904417a7 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -15,6 +15,7 @@
>  #include <linux/bug.h>
>  #include <linux/container_of.h>
>  #include <linux/fwnode.h>
> +#include <linux/kref.h>
>  #include <linux/list.h>
>  #include <linux/media.h>
>  #include <linux/minmax.h>
> @@ -248,6 +249,37 @@ struct media_pad {
>  	struct media_pipeline *pipe;
>  };
>  
> +/**
> + * struct media_entity_context - A media entity execution context
> + * @mdev_context: The media device context this media entity is bound to.
> + *		  The field is initialized when the entity is bound to a media
> + *		  device context.
> + * @entity: The media entity this context belongs to
> + * @refcount: The kref reference counter
> + * list: The list entry to link the entity context in the media device context
> + *
> + * This type represent the 'base class' used to implement execution context for
> + * video device contexts and subdevice contexts. Those types embedds an instance

Typo 'embedds' -> 'embed'?

> + * of 'struct media_entity_context' as their first member, allowing the MC core
> + * to implement type polymorphism and handle video device and subdevice contexts
> + * transparently.
> + *
> + * The main function of this type is to provide reference counting for the
> + * 'dervived' device context types. The video device and subdevice core

Typo 'dervived' -> 'derived'?

> + * populates the 'context_release' function pointer that implement specific
> + * clean-up operations, similar to what a 'virtual destructor' would do in C++.
> + *
> + * Drivers are not expected to use this type directly, but only the MC core
> + * will.
> + */
> +struct media_device_context;
> +struct media_entity_context {
> +	struct media_device_context *mdev_context;
> +	struct media_entity *entity;
> +	struct kref refcount;
> +	struct list_head list;
> +};
> +
>  /**
>   * struct media_entity_operations - Media entity operations
>   * @get_fwnode_pad:	Return the pad number based on a fwnode endpoint or
> @@ -269,6 +301,15 @@ struct media_pad {
>   *			media_entity_has_pad_interdep().
>   *			Optional: If the operation isn't implemented all pads
>   *			will be considered as interdependent.
> + * @alloc_context:	Allocate a media entity context. Drivers are allowed to
> + *			sub-class the entity context type by defining a driver
> + *			specific type that embeds an instance of either a
> + *			video_device_context or subdevice_context as first
> + *			member, and allocate the size of a driver-specific type
> + *			in the implementation of this operation. Returns 0 for
> + *			success, or an error code < 0 otherwise.
> + * @destroy_context:	Release a media entity context previously allocated by
> + *			the driver.
>   *
>   * .. note::
>   *
> @@ -284,6 +325,9 @@ struct media_entity_operations {
>  	int (*link_validate)(struct media_link *link);
>  	bool (*has_pad_interdep)(struct media_entity *entity, unsigned int pad0,
>  				 unsigned int pad1);
> +	int (*alloc_context)(struct media_entity *entity,
> +			     struct media_entity_context **context);
> +	void (*destroy_context)(struct media_entity_context *context);
>  };
>  
>  /**
> @@ -1448,3 +1492,60 @@ struct media_link *__media_entity_next_link(struct media_entity *entity,
>  					     MEDIA_LNK_FL_DATA_LINK))
>  
>  #endif
> +
> +/**
> + * media_entity_context_get - Increase the media entity context reference count
> + *			      and return a reference to it
> + *
> + * @ctx: the media entity context
> + *
> + * Increase the media entity context reference count. The reference count
> + * is increased by the V4L2 core when:
> + *
> + * * a new context is allocated when bounding a media entity to a media device
> + *   context (by kref_init())
> + * * the media pipeline the context is part of starts streaming
> + *
> + * The entity context gets automatically decreased by the V4L2 core when:
> + *
> + * * a context is unbound
> + * * the pipeline stops streaming
> + */
> +struct media_entity_context *
> +media_entity_context_get(struct media_entity_context *ctx);
> +
> +/**
> + * media_entity_context_put - Decrease the media entity context reference count
> + *
> + * @ctx: the media entity context
> + *
> + * Decrease the media entity context reference count. The reference count
> + * is decreased by the V4L2 core when:
> + *
> + * * the file handle the context is associated with is closed
> + * * the media pipeline the context is part of is stopped
> + */
> +void media_entity_context_put(struct media_entity_context *ctx);
> +
> +/**
> + * media_entity_init_context - Initialize the media entity context
> + *
> + * @entity: the media entity this context belongs to
> + * @ctx: the media entity context
> + *
> + * Initialize the media entity context by initializing the kref reference
> + * counter. The intended caller of this function are the video device context
> + * and subdevic context initialize functions.

Typo "subdevic" -> "subdevice"

> + */
> +void media_entity_init_context(struct media_entity *entity,
> +			       struct media_entity_context *ctx);
> +
> +/**
> + * media_entity_cleanup_context - Cleanup the media entity context
> + *
> + * @ctx: the media entity context
> + *
> + * Cleanup the media entity context. The intended caller of this function are
> + * the video device and subdevice context cleanup functions.
> + */
> +void media_entity_cleanup_context(struct media_entity_context *ctx);
> 

Best regards,
Michael