Re: [PATCH v3 56/76] media: s5p-jpeg: Access v4l2_fh from file

[Andrzej Pietrasiewicz <andrzejtp2010@gmail.com>]

Hi Laurent & Jacopo,

Thanks for taking care of this, this generally looks good to me,
but please see inline.

W dniu 10.08.2025 o 03:30, Laurent Pinchart pisze:
> From: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> 
> The v4l2_fh associated with an open file handle is now guaranteed
> to be available in file->private_data, initialised by v4l2_fh_add().
> 
> Access the v4l2_fh, and from there the driver-specific structure,
> from the file * in all ioctl handlers.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> Reviewed-by: Andrzej Pietrasiewicz <andrzejtp2010@gmail.com>
> Co-developed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
> Changes since v1:
> 
> - Update file-to-ctx macro due to removal of fh-to-ctx macro
> ---
>   .../platform/samsung/s5p-jpeg/jpeg-core.c     | 27 ++++++++-----------
>   1 file changed, 11 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/media/platform/samsung/s5p-jpeg/jpeg-core.c b/drivers/media/platform/samsung/s5p-jpeg/jpeg-core.c
> index 65f256db4c76..81792f7f8b16 100644
> --- a/drivers/media/platform/samsung/s5p-jpeg/jpeg-core.c
> +++ b/drivers/media/platform/samsung/s5p-jpeg/jpeg-core.c
> @@ -580,14 +580,9 @@ static inline struct s5p_jpeg_ctx *ctrl_to_ctx(struct v4l2_ctrl *c)
>   	return container_of(c->handler, struct s5p_jpeg_ctx, ctrl_handler);
>   }
>   
> -static inline struct s5p_jpeg_ctx *fh_to_ctx(struct v4l2_fh *fh)
> -{
> -	return container_of(fh, struct s5p_jpeg_ctx, fh);
> -}
> -
>   static inline struct s5p_jpeg_ctx *file_to_ctx(struct file *filp)
>   {
> -	return fh_to_ctx(file_to_v4l2_fh(filp));
> +	return container_of(file_to_v4l2_fh(filp), struct s5p_jpeg_ctx, fh);
>   }
>   
>   static int s5p_jpeg_to_user_subsampling(struct s5p_jpeg_ctx *ctx)
> @@ -1015,8 +1010,8 @@ static int s5p_jpeg_open(struct file *file)
>   
>   static int s5p_jpeg_release(struct file *file)
>   {
> -	struct s5p_jpeg *jpeg = video_drvdata(file);
>   	struct s5p_jpeg_ctx *ctx = file_to_ctx(file);
> +	struct s5p_jpeg *jpeg = video_drvdata(file);

What is the purpose of this change?

Regards,

Andrzej

>   
>   	mutex_lock(&jpeg->lock);
>   	v4l2_m2m_ctx_release(ctx->fh.m2m_ctx);
> @@ -1253,7 +1248,7 @@ static bool s5p_jpeg_parse_hdr(struct s5p_jpeg_q_data *result,
>   static int s5p_jpeg_querycap(struct file *file, void *priv,
>   			   struct v4l2_capability *cap)
>   {
> -	struct s5p_jpeg_ctx *ctx = fh_to_ctx(priv);
> +	struct s5p_jpeg_ctx *ctx = file_to_ctx(file);
>   
>   	if (ctx->mode == S5P_JPEG_ENCODE) {
>   		strscpy(cap->driver, S5P_JPEG_M2M_NAME,
> @@ -1301,7 +1296,7 @@ static int enum_fmt(struct s5p_jpeg_ctx *ctx,
>   static int s5p_jpeg_enum_fmt_vid_cap(struct file *file, void *priv,
>   				   struct v4l2_fmtdesc *f)
>   {
> -	struct s5p_jpeg_ctx *ctx = fh_to_ctx(priv);
> +	struct s5p_jpeg_ctx *ctx = file_to_ctx(file);
>   
>   	if (ctx->mode == S5P_JPEG_ENCODE)
>   		return enum_fmt(ctx, sjpeg_formats, SJPEG_NUM_FORMATS, f,
> @@ -1314,7 +1309,7 @@ static int s5p_jpeg_enum_fmt_vid_cap(struct file *file, void *priv,
>   static int s5p_jpeg_enum_fmt_vid_out(struct file *file, void *priv,
>   				   struct v4l2_fmtdesc *f)
>   {
> -	struct s5p_jpeg_ctx *ctx = fh_to_ctx(priv);
> +	struct s5p_jpeg_ctx *ctx = file_to_ctx(file);
>   
>   	if (ctx->mode == S5P_JPEG_ENCODE)
>   		return enum_fmt(ctx, sjpeg_formats, SJPEG_NUM_FORMATS, f,
> @@ -1340,7 +1335,7 @@ static int s5p_jpeg_g_fmt(struct file *file, void *priv, struct v4l2_format *f)
>   	struct vb2_queue *vq;
>   	struct s5p_jpeg_q_data *q_data = NULL;
>   	struct v4l2_pix_format *pix = &f->fmt.pix;
> -	struct s5p_jpeg_ctx *ct = fh_to_ctx(priv);
> +	struct s5p_jpeg_ctx *ct = file_to_ctx(file);
>   
>   	vq = v4l2_m2m_get_vq(ct->fh.m2m_ctx, f->type);
>   	if (!vq)
> @@ -1480,7 +1475,7 @@ static int vidioc_try_fmt(struct v4l2_format *f, struct s5p_jpeg_fmt *fmt,
>   static int s5p_jpeg_try_fmt_vid_cap(struct file *file, void *priv,
>   				  struct v4l2_format *f)
>   {
> -	struct s5p_jpeg_ctx *ctx = fh_to_ctx(priv);
> +	struct s5p_jpeg_ctx *ctx = file_to_ctx(file);
>   	struct v4l2_pix_format *pix = &f->fmt.pix;
>   	struct s5p_jpeg_fmt *fmt;
>   	int ret;
> @@ -1539,7 +1534,7 @@ static int s5p_jpeg_try_fmt_vid_cap(struct file *file, void *priv,
>   static int s5p_jpeg_try_fmt_vid_out(struct file *file, void *priv,
>   				  struct v4l2_format *f)
>   {
> -	struct s5p_jpeg_ctx *ctx = fh_to_ctx(priv);
> +	struct s5p_jpeg_ctx *ctx = file_to_ctx(file);
>   	struct s5p_jpeg_fmt *fmt;
>   
>   	fmt = s5p_jpeg_find_format(ctx, f->fmt.pix.pixelformat,
> @@ -1686,7 +1681,7 @@ static int s5p_jpeg_s_fmt_vid_cap(struct file *file, void *priv,
>   	if (ret)
>   		return ret;
>   
> -	return s5p_jpeg_s_fmt(fh_to_ctx(priv), f);
> +	return s5p_jpeg_s_fmt(file_to_ctx(file), f);
>   }
>   
>   static int s5p_jpeg_s_fmt_vid_out(struct file *file, void *priv,
> @@ -1698,7 +1693,7 @@ static int s5p_jpeg_s_fmt_vid_out(struct file *file, void *priv,
>   	if (ret)
>   		return ret;
>   
> -	return s5p_jpeg_s_fmt(fh_to_ctx(priv), f);
> +	return s5p_jpeg_s_fmt(file_to_ctx(file), f);
>   }
>   
>   static int s5p_jpeg_subscribe_event(struct v4l2_fh *fh,
> @@ -1795,7 +1790,7 @@ static int exynos3250_jpeg_try_crop(struct s5p_jpeg_ctx *ctx,
>   static int s5p_jpeg_g_selection(struct file *file, void *priv,
>   			 struct v4l2_selection *s)
>   {
> -	struct s5p_jpeg_ctx *ctx = fh_to_ctx(priv);
> +	struct s5p_jpeg_ctx *ctx = file_to_ctx(file);
>   
>   	if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT &&
>   	    s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)