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

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

On Tue, Aug 12, 2025 at 10:51:57AM +0300, Laurent Pinchart wrote:
> On Tue, Aug 12, 2025 at 08:13:39AM +0200, Andrzej Pietrasiewicz wrote:
> > 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?
> 
> I don't think this is needed, I believe it can be dropped.
> 
> I'll send a new version (of this patch only, I don't want to respin the
> whole series and spam everybody).

The series got merged before I could send a new version of this patch.
Sorry about that. The new order of the variables doesn't really make a
difference as far as I can see, so I won't send a patch to revert it. If
there's an issue I'm missing that requires a fix, please let me know.

> > >   
> > >   	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)

-- 
Regards,

Laurent Pinchart