Re: [PATCH v2 13/27] media: v4l2-subdev: Introduce VIDIOC_SUBDEV_BIND_CONTEXT

[John Cox]

> Introduce a new ioctl in V4L2 subdev to allocate a subdevice context
> and associate it with a media device context.
>
> The ioctl handler calls the entity ops to let the driver allocate a
> new context and initialize the subdev state there contained.
>
> The new subdevice context is bound to the media device context
> associated with the file descriptor provided by userspace as the new
> ioctl argument.
>
> The newly allocated context is stored in the v4l2_subdev_fh handle
> that represent the open file handle on which the ioctl has been called.
>
> Signed-off-by: Jacopo Mondi <jacopo.mondi@xxxxxxxxxxxxxxxx>
> ---
>  drivers/media/v4l2-core/v4l2-subdev.c | 59 +++++++++++++++++++++++++++++++++++
>  include/uapi/linux/v4l2-subdev.h      | 11 +++++++
>  2 files changed, 70 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index 7307f57439499c8d5360c89f492944828ac23973..300f84317623dd082a4cd2caec97057f972e82a3 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -607,6 +607,46 @@ subdev_ioctl_get_state(struct v4l2_subdev *sd, struct v4l2_subdev_fh *subdev_fh,
>                              v4l2_subdev_get_unlocked_active_state(sd);
>  }
>
> +static int subdev_do_bind_context(struct v4l2_subdev *sd,
> +                                 struct v4l2_subdev_context **context,
> +                                 struct media_device_context *mdev_context)
> +{
> +       static struct lock_class_key key;
> +       struct v4l2_subdev_state *state;
> +       int ret;
> +
> +       ret = sd->entity.ops->alloc_context(&sd->entity,
> +                                           (struct media_entity_context **)
> +                                           context);
> +       if (ret)
> +               return ret;
> +
> +       state = __v4l2_subdev_state_alloc(sd, "context->state->lock", &key);
> +       if (IS_ERR(state)) {
> +               ret = PTR_ERR(state);
> +               goto err_put_context;
> +       }
> +       (*context)->state = state;
> +
> +       /*
> +        * Bind the newly created video device context to the media device
> +        * context identified by the file descriptor.
> +        */
> +       ret = media_device_bind_context(mdev_context,
> +                                       (struct media_entity_context *)
> +                                       *context);
> +       if (ret)
> +               goto err_free_state;
> +
> +       return 0;
> +
> +err_free_state:
> +       __v4l2_subdev_state_free((*context)->state);
> +err_put_context:
> +       v4l2_subdev_context_put(*context);
> +       return ret;
> +}
> +
>  static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
>                             struct v4l2_subdev_state *state)
>  {
> @@ -1089,6 +1129,25 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
>                 return 0;
>         }
>
> +       case VIDIOC_SUBDEV_BIND_CONTEXT: {
> +               struct v4l2_subdev_bind_context *c = arg;
> +               struct media_device_context *mdev_context;
> +               int ret;
> +
> +               if (!sd->entity.ops || !sd->entity.ops->alloc_context ||
> +                   !sd->entity.ops->destroy_context)
> +                       return -ENOTTY;
> +
> +               mdev_context = media_device_context_get_from_fd(c->context_fd);
> +               if (!mdev_context)
> +                       return -EINVAL;
> +
> +               ret = subdev_do_bind_context(sd, &subdev_fh->context,
> +                                            mdev_context);
> +               media_device_context_put(mdev_context);
> +               return ret;
> +       }
> +
I think you also need a v4l2_subdev_context_put in subdev_close to
free up the bound
context.

Regards

John Cox

>         case VIDIOC_SUBDEV_G_CLIENT_CAP: {
>                 struct v4l2_subdev_client_capability *client_cap = arg;
>
> diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h
> index 2347e266cf7516b4073c1edd43b97a3ddddb183b..6184cc0153a9dd9fbfa6729e1b6127ba4a961395 100644
> --- a/include/uapi/linux/v4l2-subdev.h
> +++ b/include/uapi/linux/v4l2-subdev.h
> @@ -243,6 +243,16 @@ struct v4l2_subdev_routing {
>         __u32 reserved[11];
>  };
>
> +/**
> + * struct v4l2_subdev_bind_context - Subdev context information
> + *
> + * @context_fd: The file descriptor of the media_device instance the subdevice
> + *             has to be bound to
> + */
> +struct v4l2_subdev_bind_context {
> +       __u32 context_fd;
> +};
> +
>  /*
>   * The client is aware of streams. Setting this flag enables the use of 'stream'
>   * fields (referring to the stream number) with various ioctls. If this is not
> @@ -285,6 +295,7 @@ struct v4l2_subdev_client_capability {
>  #define VIDIOC_SUBDEV_S_SELECTION              _IOWR('V', 62, struct v4l2_subdev_selection)
>  #define VIDIOC_SUBDEV_G_ROUTING                        _IOWR('V', 38, struct v4l2_subdev_routing)
>  #define VIDIOC_SUBDEV_S_ROUTING                        _IOWR('V', 39, struct v4l2_subdev_routing)
> +#define VIDIOC_SUBDEV_BIND_CONTEXT             _IOWR('V', 50, struct v4l2_subdev_bind_context)
>  #define VIDIOC_SUBDEV_G_CLIENT_CAP             _IOR('V',  101, struct v4l2_subdev_client_capability)
>  #define VIDIOC_SUBDEV_S_CLIENT_CAP             _IOWR('V',  102, struct v4l2_subdev_client_capability)
>
>
> --
> 2.49.0
>
>