diff mbox series

[v4,1/4] media: imx-jpeg: Move mxc_jpeg_free_slot_data() ahead

Message ID 20250411074347.809-2-ming.qian@oss.nxp.com (mailing list archive)
State New
Headers show
Series media: imx-jpeg: Fix some motion-jpeg decoding | expand

Commit Message

Ming Qian(OSS) April 11, 2025, 7:43 a.m. UTC
From: Ming Qian <ming.qian@oss.nxp.com>

Move function mxc_jpeg_free_slot_data() above mxc_jpeg_alloc_slot_data()
allowing to call that function during allocation failures.
No functional changes are made.

Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
---
 .../media/platform/nxp/imx-jpeg/mxc-jpeg.c    | 46 +++++++++++--------
 1 file changed, 26 insertions(+), 20 deletions(-)

Comments

Frank Li April 11, 2025, 2:36 p.m. UTC | #1
On Fri, Apr 11, 2025 at 03:43:40PM +0800, ming.qian@oss.nxp.com wrote:
> From: Ming Qian <ming.qian@oss.nxp.com>
>
> Move function mxc_jpeg_free_slot_data() above mxc_jpeg_alloc_slot_data()
> allowing to call that function during allocation failures.
> No functional changes are made.
>
> Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
> Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> ---
>  .../media/platform/nxp/imx-jpeg/mxc-jpeg.c    | 46 +++++++++++--------
>  1 file changed, 26 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> index 0e6ee997284b..b2f7e9ad1885 100644
> --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> @@ -752,6 +752,32 @@ static int mxc_get_free_slot(struct mxc_jpeg_slot_data *slot_data)
>  	return -1;
>  }
>
> +static void mxc_jpeg_free_slot_data(struct mxc_jpeg_dev *jpeg)
> +{
> +	/* free descriptor for decoding/encoding phase */
> +	dma_free_coherent(jpeg->dev, sizeof(struct mxc_jpeg_desc),
> +			  jpeg->slot_data.desc,
> +			  jpeg->slot_data.desc_handle);
> +	jpeg->slot_data.desc = NULL;
> +	jpeg->slot_data.desc_handle = 0;

You add above two lines, it is not simple move function. Move above two
line change to next patch.

Frank

> +
> +	/* free descriptor for encoder configuration phase / decoder DHT */
> +	dma_free_coherent(jpeg->dev, sizeof(struct mxc_jpeg_desc),
> +			  jpeg->slot_data.cfg_desc,
> +			  jpeg->slot_data.cfg_desc_handle);
> +	jpeg->slot_data.cfg_desc_handle = 0;
> +	jpeg->slot_data.cfg_desc = NULL;

The same here.

> +
> +	/* free configuration stream */
> +	dma_free_coherent(jpeg->dev, MXC_JPEG_MAX_CFG_STREAM,
> +			  jpeg->slot_data.cfg_stream_vaddr,
> +			  jpeg->slot_data.cfg_stream_handle);
> +	jpeg->slot_data.cfg_stream_vaddr = NULL;
> +	jpeg->slot_data.cfg_stream_handle = 0;

The same here.

> +
> +	jpeg->slot_data.used = false;
> +}
> +
>  static bool mxc_jpeg_alloc_slot_data(struct mxc_jpeg_dev *jpeg)
>  {
>  	struct mxc_jpeg_desc *desc;
> @@ -798,26 +824,6 @@ static bool mxc_jpeg_alloc_slot_data(struct mxc_jpeg_dev *jpeg)
>  	return false;
>  }
>
> -static void mxc_jpeg_free_slot_data(struct mxc_jpeg_dev *jpeg)
> -{
> -	/* free descriptor for decoding/encoding phase */
> -	dma_free_coherent(jpeg->dev, sizeof(struct mxc_jpeg_desc),
> -			  jpeg->slot_data.desc,
> -			  jpeg->slot_data.desc_handle);
> -
> -	/* free descriptor for encoder configuration phase / decoder DHT */
> -	dma_free_coherent(jpeg->dev, sizeof(struct mxc_jpeg_desc),
> -			  jpeg->slot_data.cfg_desc,
> -			  jpeg->slot_data.cfg_desc_handle);
> -
> -	/* free configuration stream */
> -	dma_free_coherent(jpeg->dev, MXC_JPEG_MAX_CFG_STREAM,
> -			  jpeg->slot_data.cfg_stream_vaddr,
> -			  jpeg->slot_data.cfg_stream_handle);
> -
> -	jpeg->slot_data.used = false;
> -}
> -
>  static void mxc_jpeg_check_and_set_last_buffer(struct mxc_jpeg_ctx *ctx,
>  					       struct vb2_v4l2_buffer *src_buf,
>  					       struct vb2_v4l2_buffer *dst_buf)
> --
> 2.43.0-rc1
>
Nicolas Dufresne April 11, 2025, 6:47 p.m. UTC | #2
Le vendredi 11 avril 2025 à 10:36 -0400, Frank Li a écrit :
> On Fri, Apr 11, 2025 at 03:43:40PM +0800, ming.qian@oss.nxp.com wrote:
> > From: Ming Qian <ming.qian@oss.nxp.com>
> > 
> > Move function mxc_jpeg_free_slot_data() above mxc_jpeg_alloc_slot_data()
> > allowing to call that function during allocation failures.
> > No functional changes are made.
> > 
> > Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
> > Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> > ---
> >  .../media/platform/nxp/imx-jpeg/mxc-jpeg.c    | 46 +++++++++++--------
> >  1 file changed, 26 insertions(+), 20 deletions(-)
> > 
> > diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> > index 0e6ee997284b..b2f7e9ad1885 100644
> > --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> > +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> > @@ -752,6 +752,32 @@ static int mxc_get_free_slot(struct mxc_jpeg_slot_data *slot_data)
> >  	return -1;
> >  }
> > 
> > +static void mxc_jpeg_free_slot_data(struct mxc_jpeg_dev *jpeg)
> > +{
> > +	/* free descriptor for decoding/encoding phase */
> > +	dma_free_coherent(jpeg->dev, sizeof(struct mxc_jpeg_desc),
> > +			  jpeg->slot_data.desc,
> > +			  jpeg->slot_data.desc_handle);
> > +	jpeg->slot_data.desc = NULL;
> > +	jpeg->slot_data.desc_handle = 0;
> 
> You add above two lines, it is not simple move function. Move above two
> line change to next patch.

What about just making this its own commit, give you the chance to
write down that your making that function safe to be called multiple
times ?

Nicolas

> 
> Frank
> 
> > +
> > +	/* free descriptor for encoder configuration phase / decoder DHT */
> > +	dma_free_coherent(jpeg->dev, sizeof(struct mxc_jpeg_desc),
> > +			  jpeg->slot_data.cfg_desc,
> > +			  jpeg->slot_data.cfg_desc_handle);
> > +	jpeg->slot_data.cfg_desc_handle = 0;
> > +	jpeg->slot_data.cfg_desc = NULL;
> 
> The same here.
> 
> > +
> > +	/* free configuration stream */
> > +	dma_free_coherent(jpeg->dev, MXC_JPEG_MAX_CFG_STREAM,
> > +			  jpeg->slot_data.cfg_stream_vaddr,
> > +			  jpeg->slot_data.cfg_stream_handle);
> > +	jpeg->slot_data.cfg_stream_vaddr = NULL;
> > +	jpeg->slot_data.cfg_stream_handle = 0;
> 
> The same here.
> 
> > +
> > +	jpeg->slot_data.used = false;
> > +}
> > +
> >  static bool mxc_jpeg_alloc_slot_data(struct mxc_jpeg_dev *jpeg)
> >  {
> >  	struct mxc_jpeg_desc *desc;
> > @@ -798,26 +824,6 @@ static bool mxc_jpeg_alloc_slot_data(struct mxc_jpeg_dev *jpeg)
> >  	return false;
> >  }
> > 
> > -static void mxc_jpeg_free_slot_data(struct mxc_jpeg_dev *jpeg)
> > -{
> > -	/* free descriptor for decoding/encoding phase */
> > -	dma_free_coherent(jpeg->dev, sizeof(struct mxc_jpeg_desc),
> > -			  jpeg->slot_data.desc,
> > -			  jpeg->slot_data.desc_handle);
> > -
> > -	/* free descriptor for encoder configuration phase / decoder DHT */
> > -	dma_free_coherent(jpeg->dev, sizeof(struct mxc_jpeg_desc),
> > -			  jpeg->slot_data.cfg_desc,
> > -			  jpeg->slot_data.cfg_desc_handle);
> > -
> > -	/* free configuration stream */
> > -	dma_free_coherent(jpeg->dev, MXC_JPEG_MAX_CFG_STREAM,
> > -			  jpeg->slot_data.cfg_stream_vaddr,
> > -			  jpeg->slot_data.cfg_stream_handle);
> > -
> > -	jpeg->slot_data.used = false;
> > -}
> > -
> >  static void mxc_jpeg_check_and_set_last_buffer(struct mxc_jpeg_ctx *ctx,
> >  					       struct vb2_v4l2_buffer *src_buf,
> >  					       struct vb2_v4l2_buffer *dst_buf)
> > --
> > 2.43.0-rc1
> >
diff mbox series

Patch

diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
index 0e6ee997284b..b2f7e9ad1885 100644
--- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
+++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
@@ -752,6 +752,32 @@  static int mxc_get_free_slot(struct mxc_jpeg_slot_data *slot_data)
 	return -1;
 }
 
+static void mxc_jpeg_free_slot_data(struct mxc_jpeg_dev *jpeg)
+{
+	/* free descriptor for decoding/encoding phase */
+	dma_free_coherent(jpeg->dev, sizeof(struct mxc_jpeg_desc),
+			  jpeg->slot_data.desc,
+			  jpeg->slot_data.desc_handle);
+	jpeg->slot_data.desc = NULL;
+	jpeg->slot_data.desc_handle = 0;
+
+	/* free descriptor for encoder configuration phase / decoder DHT */
+	dma_free_coherent(jpeg->dev, sizeof(struct mxc_jpeg_desc),
+			  jpeg->slot_data.cfg_desc,
+			  jpeg->slot_data.cfg_desc_handle);
+	jpeg->slot_data.cfg_desc_handle = 0;
+	jpeg->slot_data.cfg_desc = NULL;
+
+	/* free configuration stream */
+	dma_free_coherent(jpeg->dev, MXC_JPEG_MAX_CFG_STREAM,
+			  jpeg->slot_data.cfg_stream_vaddr,
+			  jpeg->slot_data.cfg_stream_handle);
+	jpeg->slot_data.cfg_stream_vaddr = NULL;
+	jpeg->slot_data.cfg_stream_handle = 0;
+
+	jpeg->slot_data.used = false;
+}
+
 static bool mxc_jpeg_alloc_slot_data(struct mxc_jpeg_dev *jpeg)
 {
 	struct mxc_jpeg_desc *desc;
@@ -798,26 +824,6 @@  static bool mxc_jpeg_alloc_slot_data(struct mxc_jpeg_dev *jpeg)
 	return false;
 }
 
-static void mxc_jpeg_free_slot_data(struct mxc_jpeg_dev *jpeg)
-{
-	/* free descriptor for decoding/encoding phase */
-	dma_free_coherent(jpeg->dev, sizeof(struct mxc_jpeg_desc),
-			  jpeg->slot_data.desc,
-			  jpeg->slot_data.desc_handle);
-
-	/* free descriptor for encoder configuration phase / decoder DHT */
-	dma_free_coherent(jpeg->dev, sizeof(struct mxc_jpeg_desc),
-			  jpeg->slot_data.cfg_desc,
-			  jpeg->slot_data.cfg_desc_handle);
-
-	/* free configuration stream */
-	dma_free_coherent(jpeg->dev, MXC_JPEG_MAX_CFG_STREAM,
-			  jpeg->slot_data.cfg_stream_vaddr,
-			  jpeg->slot_data.cfg_stream_handle);
-
-	jpeg->slot_data.used = false;
-}
-
 static void mxc_jpeg_check_and_set_last_buffer(struct mxc_jpeg_ctx *ctx,
 					       struct vb2_v4l2_buffer *src_buf,
 					       struct vb2_v4l2_buffer *dst_buf)