Re: [RFCv4 11/21] media: v4l2_fh: add request entity field

[Alexandre Courbot]

On Wed, Feb 21, 2018 at 12:24 AM, Hans Verkuil  wrote:
> On 02/20/18 05:44, Alexandre Courbot wrote:
>> Allow drivers to assign a request entity to v4l2_fh. This will be useful
>> for request-aware ioctls to find out which request entity to use.
>>
>> Signed-off-by: Alexandre Courbot 
>> ---
>>  include/media/v4l2-fh.h | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/include/media/v4l2-fh.h b/include/media/v4l2-fh.h
>> index ea73fef8bdc0..f54cb319dd64 100644
>> --- a/include/media/v4l2-fh.h
>> +++ b/include/media/v4l2-fh.h
>> @@ -28,6 +28,7 @@
>>
>>  struct video_device;
>>  struct v4l2_ctrl_handler;
>> +struct media_request_entity;
>>
>>  /**
>>   * struct v4l2_fh - Describes a V4L2 file handler
>> @@ -43,6 +44,7 @@ struct v4l2_ctrl_handler;
>>   * @navailable: number of available events at @available list
>>   * @sequence: event sequence number
>>   * @m2m_ctx: pointer to  v4l2_m2m_ctx
>> + * @entity: the request entity this fh operates on behalf of
>>   */
>>  struct v4l2_fh {
>>   struct list_headlist;
>> @@ -60,6 +62,7 @@ struct v4l2_fh {
>>  #if IS_ENABLED(CONFIG_V4L2_MEM2MEM_DEV)
>>   struct v4l2_m2m_ctx *m2m_ctx;
>>  #endif
>> + struct media_request_entity *entity;
>
> The name 'media_request_entity' is very confusing.
>
> In the media controller API terminology an entity represents a piece
> of hardware with inputs and outputs (very rough description), but a
> request is not an entity. It may be associated with an entity, though.
>
> So calling this field 'entity' is also very misleading.

Note that this field is not a request though, it is a pointer to a
piece of hardware referenced by a request, which is closer to the MC
terminology. Or do you mean this should just be renamed
"request_entity"?

If we go all the way, the media_ prefix is also misleading - it
implies a dependency to the media controller framework, while there is
none (in this patchset at least).

However I thought that 'request' alone (instead of media_request) may
name-conflict with something else, and since 'media' is also the
umbrella term for anything under drivers/media it sounds fitting on
the other hand. Suggestions are welcome though.

>
> As with previous patches, I'll have to think about this and try and
> come up with better, less confusing names.

I will gladly take suggestions, have been trying to come with a better
name to reply to your comment above but could not find any. :)

Re: [RFCv4 11/21] media: v4l2_fh: add request entity field

[Hans Verkuil]

On 02/20/18 05:44, Alexandre Courbot wrote:
> Allow drivers to assign a request entity to v4l2_fh. This will be useful
> for request-aware ioctls to find out which request entity to use.
> 
> Signed-off-by: Alexandre Courbot 
> ---
>  include/media/v4l2-fh.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/include/media/v4l2-fh.h b/include/media/v4l2-fh.h
> index ea73fef8bdc0..f54cb319dd64 100644
> --- a/include/media/v4l2-fh.h
> +++ b/include/media/v4l2-fh.h
> @@ -28,6 +28,7 @@
>  
>  struct video_device;
>  struct v4l2_ctrl_handler;
> +struct media_request_entity;
>  
>  /**
>   * struct v4l2_fh - Describes a V4L2 file handler
> @@ -43,6 +44,7 @@ struct v4l2_ctrl_handler;
>   * @navailable: number of available events at @available list
>   * @sequence: event sequence number
>   * @m2m_ctx: pointer to  v4l2_m2m_ctx
> + * @entity: the request entity this fh operates on behalf of
>   */
>  struct v4l2_fh {
>   struct list_headlist;
> @@ -60,6 +62,7 @@ struct v4l2_fh {
>  #if IS_ENABLED(CONFIG_V4L2_MEM2MEM_DEV)
>   struct v4l2_m2m_ctx *m2m_ctx;
>  #endif
> + struct media_request_entity *entity;

The name 'media_request_entity' is very confusing.

In the media controller API terminology an entity represents a piece
of hardware with inputs and outputs (very rough description), but a
request is not an entity. It may be associated with an entity, though.

So calling this field 'entity' is also very misleading.

As with previous patches, I'll have to think about this and try and
come up with better, less confusing names.

Regards,

Hans

>  };
>  
>  /**
> 

[RFCv4 11/21] media: v4l2_fh: add request entity field

[Alexandre Courbot]

Allow drivers to assign a request entity to v4l2_fh. This will be useful
for request-aware ioctls to find out which request entity to use.

Signed-off-by: Alexandre Courbot 
---
 include/media/v4l2-fh.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/media/v4l2-fh.h b/include/media/v4l2-fh.h
index ea73fef8bdc0..f54cb319dd64 100644
--- a/include/media/v4l2-fh.h
+++ b/include/media/v4l2-fh.h
@@ -28,6 +28,7 @@
 
 struct video_device;
 struct v4l2_ctrl_handler;
+struct media_request_entity;
 
 /**
  * struct v4l2_fh - Describes a V4L2 file handler
@@ -43,6 +44,7 @@ struct v4l2_ctrl_handler;
  * @navailable: number of available events at @available list
  * @sequence: event sequence number
  * @m2m_ctx: pointer to  v4l2_m2m_ctx
+ * @entity: the request entity this fh operates on behalf of
  */
 struct v4l2_fh {
struct list_headlist;
@@ -60,6 +62,7 @@ struct v4l2_fh {
 #if IS_ENABLED(CONFIG_V4L2_MEM2MEM_DEV)
struct v4l2_m2m_ctx *m2m_ctx;
 #endif
+   struct media_request_entity *entity;
 };
 
 /**
-- 
2.16.1.291.g4437f3f132-goog