Re: [PATCH 4/5] media: entity: Add support for ancillary links

[Daniel Scally <djrscally@gmail.com>]

Hi Sakari

On 14/12/2021 21:25, Sakari Ailus wrote:
> Hi Daniel,
>
> On Mon, Dec 13, 2021 at 11:28:48PM +0000, Daniel Scally wrote:
>> Add functions to create and destroy ancillary links, so that they
>> don't need to be manually created by users.
>>
>> Signed-off-by: Daniel Scally <djrscally@gmail.com>
>> ---
>> Changes since the rfc:
>>
>> 	- (Laurent) Set gobj0 and gobj1 directly instead of the other union
>> 	members
>> 	- (Laurent) Added MEDIA_LNK_FL_IMMUTABLE to the kerneldoc for the new
>> 	create function
>>
>>  drivers/media/mc/mc-entity.c | 30 ++++++++++++++++++++++++++++++
>>  include/media/media-entity.h | 29 +++++++++++++++++++++++++++++
>>  2 files changed, 59 insertions(+)
>>
>> diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
>> index aeddc3f6310e..4e39e100ea03 100644
>> --- a/drivers/media/mc/mc-entity.c
>> +++ b/drivers/media/mc/mc-entity.c
>> @@ -1052,3 +1052,33 @@ void media_remove_intf_links(struct media_interface *intf)
>>  	mutex_unlock(&mdev->graph_mutex);
>>  }
>>  EXPORT_SYMBOL_GPL(media_remove_intf_links);
>> +
>> +struct media_link *media_create_ancillary_link(struct media_entity *primary,
>> +					       struct media_entity *ancillary,
>> +					       u32 flags)
>> +{
>> +	struct media_link *link;
>> +
>> +	link = media_add_link(&primary->links);
>> +	if (!link)
>> +		return ERR_PTR(-ENOMEM);
>> +
>> +	link->gobj0 = &primary->graph_obj;
>> +	link->gobj1 = &ancillary->graph_obj;
>> +	link->flags = flags | MEDIA_LNK_FL_ANCILLARY_LINK;
>> +
>> +	/* Initialize graph object embedded at the new link */
>> +	media_gobj_create(primary->graph_obj.mdev, MEDIA_GRAPH_LINK,
>> +			  &link->graph_obj);
>> +
>> +	return link;
>> +}
>> +EXPORT_SYMBOL_GPL(media_create_ancillary_link);
>> +
>> +void media_remove_ancillary_link(struct media_link *link)
>> +{
>> +	list_del(&link->list);
>> +	media_gobj_destroy(&link->graph_obj);
>> +	kfree(link);
>> +}
>> +EXPORT_SYMBOL_GPL(media_remove_ancillary_link);
>> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
>> index fea489f03d57..f7b1738cef88 100644
>> --- a/include/media/media-entity.h
>> +++ b/include/media/media-entity.h
>> @@ -1104,6 +1104,35 @@ void media_remove_intf_links(struct media_interface *intf);
>>   * it will issue a call to @operation\(@entity, @args\).
>>   */
>>  
>> +/**
>> + * media_create_ancillary_link() - creates a link between two entities
>> + *
>> + * @primary:	pointer to the primary &media_entity
>> + * @ancillary:	pointer to the ancillary &media_entity
>> + * @flags:	Link flags, as defined in
>> + *		:ref:`include/uapi/linux/media.h <media_header>`
>> + *		( seek for ``MEDIA_LNK_FL_*``)
>> + *
>> + *
>> + * Valid values for flags:
>> + *
>> + * %MEDIA_LNK_FL_ENABLED
>> + *   Indicates that the two entities are connected pieces of hardware that form
>> + *   a single logical unit.
>> + *
>> + *   A typical example is a camera lens being linked to the sensor that it is
>> + *   supporting.
>> + *
>> + * %MEDIA_LNK_FL_IMMUTABLE
>> + *   Indicates that the link enabled state can't be modified at runtime. If
>> + *   %MEDIA_LNK_FL_IMMUTABLE is set, then %MEDIA_LNK_FL_ENABLED must also be
>> + *   set, since an immutable link is always enabled.
> What's the use case for both of the flags?
>
> I know the flags are there but what will they mean in practice for
> ancillary links?


Within the kernel, I don't think they have any effect now (without patch
#3 of this series the graph iteration would have tried to walk them). I
mostly intended that they would be set so that future userspace users
wouldn't be able to flag them as disabled.

>
>> + */
>> +struct media_link *
>> +media_create_ancillary_link(struct media_entity *primary,
>> +			    struct media_entity *ancillary,
>> +			    u32 flags);
>> +
>>  #define media_entity_call(entity, operation, args...)			\
>>  	(((entity)->ops && (entity)->ops->operation) ?			\
>>  	 (entity)->ops->operation((entity) , ##args) : -ENOIOCTLCMD)