[PATCH] media: imx-jpeg: Add support for descriptor allocation from SRAM

[PATCH] media: imx-jpeg: Add support for descriptor allocation from SRAM

[Marek Vasut]

Add support for optional allocation of bitstream descriptors from SRAM
instead of DRAM. In case the encoder/decoder DT node contains 'sram'
property which points to 'mmio-sram', the driver will attempt to use
the SRAM instead of DRAM for descriptor allocation, which might improve
performance.

This however helps on i.MX95 with sporadic SLOTn_STATUS IMG_RD_ERR bit 11
being triggered during JPEG encoding. The following pipeline triggers the
problem when descriptors get allocated from DRAM, the pipeline often hangs
after a few seconds and the encoder driver indicates "timeout, cancel it" :

gst-launch-1.0 videotestsrc ! video/x-raw,width=256,height=256,format=YUY2 ! \
               queue ! v4l2jpegenc ! queue ! fakesink

Signed-off-by: Marek Vasut <marek.vasut@mailbox.org>
---
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Ming Qian <ming.qian@oss.nxp.com>
Cc: Mirela Rabulea <mirela.rabulea@nxp.com>
Cc: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: imx@lists.linux.dev
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-media@vger.kernel.org
---
 .../media/platform/nxp/imx-jpeg/mxc-jpeg.c    | 69 +++++++++++--------
 .../media/platform/nxp/imx-jpeg/mxc-jpeg.h    |  1 +
 2 files changed, 42 insertions(+), 28 deletions(-)

diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
index aef1d6473eb8d..0095c2182ed39 100644
--- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
+++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
@@ -44,6 +44,7 @@
 #include <linux/module.h>
 #include <linux/io.h>
 #include <linux/clk.h>
+#include <linux/genalloc.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
@@ -783,32 +784,40 @@ static int mxc_get_free_slot(struct mxc_jpeg_slot_data *slot_data)
 	return -1;
 }
 
+static void mxc_jpeg_free(struct mxc_jpeg_dev *jpeg, size_t size, void *addr, dma_addr_t handle)
+{
+	if (jpeg->sram_pool)
+		gen_pool_free(jpeg->sram_pool, (unsigned long)addr, size);
+	else
+		dma_free_coherent(jpeg->dev, size, addr, handle);
+}
+
 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);
+	mxc_jpeg_free(jpeg, 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);
+	mxc_jpeg_free(jpeg, 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);
+	mxc_jpeg_free(jpeg, 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;
 
-	dma_free_coherent(jpeg->dev, jpeg->slot_data.cfg_dec_size,
-			  jpeg->slot_data.cfg_dec_vaddr,
-			  jpeg->slot_data.cfg_dec_daddr);
+	mxc_jpeg_free(jpeg, jpeg->slot_data.cfg_dec_size,
+		      jpeg->slot_data.cfg_dec_vaddr,
+		      jpeg->slot_data.cfg_dec_daddr);
 	jpeg->slot_data.cfg_dec_size = 0;
 	jpeg->slot_data.cfg_dec_vaddr = NULL;
 	jpeg->slot_data.cfg_dec_daddr = 0;
@@ -816,6 +825,14 @@ static void mxc_jpeg_free_slot_data(struct mxc_jpeg_dev *jpeg)
 	jpeg->slot_data.used = false;
 }
 
+static struct mxc_jpeg_desc *mxc_jpeg_alloc(struct mxc_jpeg_dev *jpeg, size_t size, dma_addr_t *handle)
+{
+	if (jpeg->sram_pool)
+		return gen_pool_dma_zalloc(jpeg->sram_pool, size, handle);
+	else
+		return dma_alloc_coherent(jpeg->dev, size, handle, GFP_ATOMIC);
+}
+
 static bool mxc_jpeg_alloc_slot_data(struct mxc_jpeg_dev *jpeg)
 {
 	struct mxc_jpeg_desc *desc;
@@ -826,37 +843,29 @@ static bool mxc_jpeg_alloc_slot_data(struct mxc_jpeg_dev *jpeg)
 		goto skip_alloc; /* already allocated, reuse it */
 
 	/* allocate descriptor for decoding/encoding phase */
-	desc = dma_alloc_coherent(jpeg->dev,
-				  sizeof(struct mxc_jpeg_desc),
-				  &jpeg->slot_data.desc_handle,
-				  GFP_ATOMIC);
+	desc = mxc_jpeg_alloc(jpeg, sizeof(struct mxc_jpeg_desc),
+			      &jpeg->slot_data.desc_handle);
 	if (!desc)
 		goto err;
 	jpeg->slot_data.desc = desc;
 
 	/* allocate descriptor for configuration phase (encoder only) */
-	cfg_desc = dma_alloc_coherent(jpeg->dev,
-				      sizeof(struct mxc_jpeg_desc),
-				      &jpeg->slot_data.cfg_desc_handle,
-				      GFP_ATOMIC);
+	cfg_desc = mxc_jpeg_alloc(jpeg, sizeof(struct mxc_jpeg_desc),
+				  &jpeg->slot_data.cfg_desc_handle);
 	if (!cfg_desc)
 		goto err;
 	jpeg->slot_data.cfg_desc = cfg_desc;
 
 	/* allocate configuration stream */
-	cfg_stm = dma_alloc_coherent(jpeg->dev,
-				     MXC_JPEG_MAX_CFG_STREAM,
-				     &jpeg->slot_data.cfg_stream_handle,
-				     GFP_ATOMIC);
+	cfg_stm = mxc_jpeg_alloc(jpeg, MXC_JPEG_MAX_CFG_STREAM,
+				 &jpeg->slot_data.cfg_stream_handle);
 	if (!cfg_stm)
 		goto err;
 	jpeg->slot_data.cfg_stream_vaddr = cfg_stm;
 
 	jpeg->slot_data.cfg_dec_size = MXC_JPEG_PATTERN_WIDTH * MXC_JPEG_PATTERN_HEIGHT * 2;
-	jpeg->slot_data.cfg_dec_vaddr = dma_alloc_coherent(jpeg->dev,
-							   jpeg->slot_data.cfg_dec_size,
-							   &jpeg->slot_data.cfg_dec_daddr,
-							   GFP_ATOMIC);
+	jpeg->slot_data.cfg_dec_vaddr = mxc_jpeg_alloc(jpeg, jpeg->slot_data.cfg_dec_size,
+						       &jpeg->slot_data.cfg_dec_daddr);
 	if (!jpeg->slot_data.cfg_dec_vaddr)
 		goto err;
 
@@ -2902,6 +2911,10 @@ static int mxc_jpeg_probe(struct platform_device *pdev)
 	jpeg->dev = dev;
 	jpeg->mode = mode;
 
+	/* SRAM pool is optional */
+	jpeg->sram_pool = of_gen_pool_get(pdev->dev.of_node, "sram", 0);
+	dev_info(dev, "Using DMA descriptor pool in %cRAM\n", jpeg->sram_pool ? 'S' : 'D');
+
 	/* Get clocks */
 	ret = devm_clk_bulk_get_all(&pdev->dev, &jpeg->clks);
 	if (ret < 0) {
diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h
index 7f0910fc9b47e..311f2f2ac519f 100644
--- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h
+++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h
@@ -142,6 +142,7 @@ struct mxc_jpeg_dev {
 	int				num_domains;
 	struct device			**pd_dev;
 	struct device_link		**pd_link;
+	struct gen_pool			*sram_pool;
 };
 
 /**
-- 
2.50.1

Re: [PATCH] media: imx-jpeg: Add support for descriptor allocation from SRAM

[Frank Li]

On Wed, Aug 20, 2025 at 06:29:53PM +0200, Marek Vasut wrote:
> Add support for optional allocation of bitstream descriptors from SRAM
> instead of DRAM. In case the encoder/decoder DT node contains 'sram'
> property which points to 'mmio-sram', the driver will attempt to use
> the SRAM instead of DRAM for descriptor allocation, which might improve
> performance.
>
> This however helps on i.MX95 with sporadic SLOTn_STATUS IMG_RD_ERR bit 11
> being triggered during JPEG encoding. The following pipeline triggers the
> problem when descriptors get allocated from DRAM, the pipeline often hangs
> after a few seconds and the encoder driver indicates "timeout, cancel it" :

Do you know why this happen if descriptor is in DRAM? why sram help this
case?

>
> gst-launch-1.0 videotestsrc ! video/x-raw,width=256,height=256,format=YUY2 ! \
>                queue ! v4l2jpegenc ! queue ! fakesink
>
> Signed-off-by: Marek Vasut <marek.vasut@mailbox.org>
> ---
...
> -							   GFP_ATOMIC);
> +	jpeg->slot_data.cfg_dec_vaddr = mxc_jpeg_alloc(jpeg, jpeg->slot_data.cfg_dec_size,
> +						       &jpeg->slot_data.cfg_dec_daddr);
>  	if (!jpeg->slot_data.cfg_dec_vaddr)
>  		goto err;
>
> @@ -2902,6 +2911,10 @@ static int mxc_jpeg_probe(struct platform_device *pdev)
>  	jpeg->dev = dev;
>  	jpeg->mode = mode;
>
> +	/* SRAM pool is optional */
> +	jpeg->sram_pool = of_gen_pool_get(pdev->dev.of_node, "sram", 0);

Need update binding doc nxp,imx8-jpeg.yaml

Frank

> +	dev_info(dev, "Using DMA descriptor pool in %cRAM\n", jpeg->sram_pool ? 'S' : 'D');
> +
>  	/* Get clocks */
>  	ret = devm_clk_bulk_get_all(&pdev->dev, &jpeg->clks);
>  	if (ret < 0) {
> diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h
> index 7f0910fc9b47e..311f2f2ac519f 100644
> --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h
> +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h
> @@ -142,6 +142,7 @@ struct mxc_jpeg_dev {
>  	int				num_domains;
>  	struct device			**pd_dev;
>  	struct device_link		**pd_link;
> +	struct gen_pool			*sram_pool;
>  };
>
>  /**
> --
> 2.50.1
>

Re: [PATCH] media: imx-jpeg: Add support for descriptor allocation from SRAM

[Marek Vasut]

On 8/20/25 8:17 PM, Frank Li wrote:
> On Wed, Aug 20, 2025 at 06:29:53PM +0200, Marek Vasut wrote:
>> Add support for optional allocation of bitstream descriptors from SRAM
>> instead of DRAM. In case the encoder/decoder DT node contains 'sram'
>> property which points to 'mmio-sram', the driver will attempt to use
>> the SRAM instead of DRAM for descriptor allocation, which might improve
>> performance.
>>
>> This however helps on i.MX95 with sporadic SLOTn_STATUS IMG_RD_ERR bit 11
>> being triggered during JPEG encoding. The following pipeline triggers the
>> problem when descriptors get allocated from DRAM, the pipeline often hangs
>> after a few seconds and the encoder driver indicates "timeout, cancel it" :
> 
> Do you know why this happen if descriptor is in DRAM? why sram help this
> case?

No. Did NXP ever observe this ?

It seems to me the encode never received much testing, considering the 
severe bugs that showed up in there.

>> gst-launch-1.0 videotestsrc ! video/x-raw,width=256,height=256,format=YUY2 ! \
>>                 queue ! v4l2jpegenc ! queue ! fakesink
>>
>> Signed-off-by: Marek Vasut <marek.vasut@mailbox.org>
>> ---
> ...
>> -							   GFP_ATOMIC);
>> +	jpeg->slot_data.cfg_dec_vaddr = mxc_jpeg_alloc(jpeg, jpeg->slot_data.cfg_dec_size,
>> +						       &jpeg->slot_data.cfg_dec_daddr);
>>   	if (!jpeg->slot_data.cfg_dec_vaddr)
>>   		goto err;
>>
>> @@ -2902,6 +2911,10 @@ static int mxc_jpeg_probe(struct platform_device *pdev)
>>   	jpeg->dev = dev;
>>   	jpeg->mode = mode;
>>
>> +	/* SRAM pool is optional */
>> +	jpeg->sram_pool = of_gen_pool_get(pdev->dev.of_node, "sram", 0);
> 
> Need update binding doc nxp,imx8-jpeg.yaml
Sure

Re: [PATCH] media: imx-jpeg: Add support for descriptor allocation from SRAM

[Ming Qian(OSS)]

Hi Marek,

On 8/21/2025 12:29 AM, Marek Vasut wrote:
> [You don't often get email from marek.vasut@mailbox.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> Add support for optional allocation of bitstream descriptors from SRAM
> instead of DRAM. In case the encoder/decoder DT node contains 'sram'
> property which points to 'mmio-sram', the driver will attempt to use
> the SRAM instead of DRAM for descriptor allocation, which might improve
> performance.
> 
> This however helps on i.MX95 with sporadic SLOTn_STATUS IMG_RD_ERR bit 11
> being triggered during JPEG encoding. The following pipeline triggers the
> problem when descriptors get allocated from DRAM, the pipeline often hangs
> after a few seconds and the encoder driver indicates "timeout, cancel it" :

It's a hardware bug in i.MX95 A0, and the i.MX95 B0 has fixed this issue.

Using sram instead of dram only improves timing, but doesn't completely 
circumvent the hardware bug

Regards,
Ming

> 
> gst-launch-1.0 videotestsrc ! video/x-raw,width=256,height=256,format=YUY2 ! \
>                 queue ! v4l2jpegenc ! queue ! fakesink
> 
> Signed-off-by: Marek Vasut <marek.vasut@mailbox.org>
> ---
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> Cc: Ming Qian <ming.qian@oss.nxp.com>
> Cc: Mirela Rabulea <mirela.rabulea@nxp.com>
> Cc: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: imx@lists.linux.dev
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-media@vger.kernel.org
> ---
>   .../media/platform/nxp/imx-jpeg/mxc-jpeg.c    | 69 +++++++++++--------
>   .../media/platform/nxp/imx-jpeg/mxc-jpeg.h    |  1 +
>   2 files changed, 42 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> index aef1d6473eb8d..0095c2182ed39 100644
> --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> @@ -44,6 +44,7 @@
>   #include <linux/module.h>
>   #include <linux/io.h>
>   #include <linux/clk.h>
> +#include <linux/genalloc.h>
>   #include <linux/of_platform.h>
>   #include <linux/platform_device.h>
>   #include <linux/slab.h>
> @@ -783,32 +784,40 @@ static int mxc_get_free_slot(struct mxc_jpeg_slot_data *slot_data)
>          return -1;
>   }
> 
> +static void mxc_jpeg_free(struct mxc_jpeg_dev *jpeg, size_t size, void *addr, dma_addr_t handle)
> +{
> +       if (jpeg->sram_pool)
> +               gen_pool_free(jpeg->sram_pool, (unsigned long)addr, size);
> +       else
> +               dma_free_coherent(jpeg->dev, size, addr, handle);
> +}
> +
>   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);
> +       mxc_jpeg_free(jpeg, 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);
> +       mxc_jpeg_free(jpeg, 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);
> +       mxc_jpeg_free(jpeg, 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;
> 
> -       dma_free_coherent(jpeg->dev, jpeg->slot_data.cfg_dec_size,
> -                         jpeg->slot_data.cfg_dec_vaddr,
> -                         jpeg->slot_data.cfg_dec_daddr);
> +       mxc_jpeg_free(jpeg, jpeg->slot_data.cfg_dec_size,
> +                     jpeg->slot_data.cfg_dec_vaddr,
> +                     jpeg->slot_data.cfg_dec_daddr);
>          jpeg->slot_data.cfg_dec_size = 0;
>          jpeg->slot_data.cfg_dec_vaddr = NULL;
>          jpeg->slot_data.cfg_dec_daddr = 0;
> @@ -816,6 +825,14 @@ static void mxc_jpeg_free_slot_data(struct mxc_jpeg_dev *jpeg)
>          jpeg->slot_data.used = false;
>   }
> 
> +static struct mxc_jpeg_desc *mxc_jpeg_alloc(struct mxc_jpeg_dev *jpeg, size_t size, dma_addr_t *handle)
> +{
> +       if (jpeg->sram_pool)
> +               return gen_pool_dma_zalloc(jpeg->sram_pool, size, handle);
> +       else
> +               return dma_alloc_coherent(jpeg->dev, size, handle, GFP_ATOMIC);
> +}
> +
>   static bool mxc_jpeg_alloc_slot_data(struct mxc_jpeg_dev *jpeg)
>   {
>          struct mxc_jpeg_desc *desc;
> @@ -826,37 +843,29 @@ static bool mxc_jpeg_alloc_slot_data(struct mxc_jpeg_dev *jpeg)
>                  goto skip_alloc; /* already allocated, reuse it */
> 
>          /* allocate descriptor for decoding/encoding phase */
> -       desc = dma_alloc_coherent(jpeg->dev,
> -                                 sizeof(struct mxc_jpeg_desc),
> -                                 &jpeg->slot_data.desc_handle,
> -                                 GFP_ATOMIC);
> +       desc = mxc_jpeg_alloc(jpeg, sizeof(struct mxc_jpeg_desc),
> +                             &jpeg->slot_data.desc_handle);
>          if (!desc)
>                  goto err;
>          jpeg->slot_data.desc = desc;
> 
>          /* allocate descriptor for configuration phase (encoder only) */
> -       cfg_desc = dma_alloc_coherent(jpeg->dev,
> -                                     sizeof(struct mxc_jpeg_desc),
> -                                     &jpeg->slot_data.cfg_desc_handle,
> -                                     GFP_ATOMIC);
> +       cfg_desc = mxc_jpeg_alloc(jpeg, sizeof(struct mxc_jpeg_desc),
> +                                 &jpeg->slot_data.cfg_desc_handle);
>          if (!cfg_desc)
>                  goto err;
>          jpeg->slot_data.cfg_desc = cfg_desc;
> 
>          /* allocate configuration stream */
> -       cfg_stm = dma_alloc_coherent(jpeg->dev,
> -                                    MXC_JPEG_MAX_CFG_STREAM,
> -                                    &jpeg->slot_data.cfg_stream_handle,
> -                                    GFP_ATOMIC);
> +       cfg_stm = mxc_jpeg_alloc(jpeg, MXC_JPEG_MAX_CFG_STREAM,
> +                                &jpeg->slot_data.cfg_stream_handle);
>          if (!cfg_stm)
>                  goto err;
>          jpeg->slot_data.cfg_stream_vaddr = cfg_stm;
> 
>          jpeg->slot_data.cfg_dec_size = MXC_JPEG_PATTERN_WIDTH * MXC_JPEG_PATTERN_HEIGHT * 2;
> -       jpeg->slot_data.cfg_dec_vaddr = dma_alloc_coherent(jpeg->dev,
> -                                                          jpeg->slot_data.cfg_dec_size,
> -                                                          &jpeg->slot_data.cfg_dec_daddr,
> -                                                          GFP_ATOMIC);
> +       jpeg->slot_data.cfg_dec_vaddr = mxc_jpeg_alloc(jpeg, jpeg->slot_data.cfg_dec_size,
> +                                                      &jpeg->slot_data.cfg_dec_daddr);
>          if (!jpeg->slot_data.cfg_dec_vaddr)
>                  goto err;
> 
> @@ -2902,6 +2911,10 @@ static int mxc_jpeg_probe(struct platform_device *pdev)
>          jpeg->dev = dev;
>          jpeg->mode = mode;
> 
> +       /* SRAM pool is optional */
> +       jpeg->sram_pool = of_gen_pool_get(pdev->dev.of_node, "sram", 0);
> +       dev_info(dev, "Using DMA descriptor pool in %cRAM\n", jpeg->sram_pool ? 'S' : 'D');
> +
>          /* Get clocks */
>          ret = devm_clk_bulk_get_all(&pdev->dev, &jpeg->clks);
>          if (ret < 0) {
> diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h
> index 7f0910fc9b47e..311f2f2ac519f 100644
> --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h
> +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h
> @@ -142,6 +142,7 @@ struct mxc_jpeg_dev {
>          int                             num_domains;
>          struct device                   **pd_dev;
>          struct device_link              **pd_link;
> +       struct gen_pool                 *sram_pool;
>   };
> 
>   /**
> --
> 2.50.1
> 

Re: [PATCH] media: imx-jpeg: Add support for descriptor allocation from SRAM

[Marek Vasut]

On 8/21/25 8:28 AM, Ming Qian(OSS) wrote:
> Hi Marek,

Hello Ming,

> On 8/21/2025 12:29 AM, Marek Vasut wrote:
>> [You don't often get email from marek.vasut@mailbox.org. Learn why 
>> this is important at https://aka.ms/LearnAboutSenderIdentification ]
>>
>> Add support for optional allocation of bitstream descriptors from SRAM
>> instead of DRAM. In case the encoder/decoder DT node contains 'sram'
>> property which points to 'mmio-sram', the driver will attempt to use
>> the SRAM instead of DRAM for descriptor allocation, which might improve
>> performance.
>>
>> This however helps on i.MX95 with sporadic SLOTn_STATUS IMG_RD_ERR bit 11
>> being triggered during JPEG encoding. The following pipeline triggers the
>> problem when descriptors get allocated from DRAM, the pipeline often 
>> hangs
>> after a few seconds and the encoder driver indicates "timeout, cancel 
>> it" :
> 
> It's a hardware bug in i.MX95 A0, and the i.MX95 B0 has fixed this issue.

Ahh, this helps, thank you.

> Using sram instead of dram only improves timing, but doesn't completely 
> circumvent the hardware bug
Does it make sense to upstream this patch anyway ?

Re: [PATCH] media: imx-jpeg: Add support for descriptor allocation from SRAM

[Ming Qian(OSS)]

Hi Marek,

On 8/21/2025 5:03 PM, Marek Vasut wrote:
> On 8/21/25 8:28 AM, Ming Qian(OSS) wrote:
>> Hi Marek,
> 
> Hello Ming,
> 
>> On 8/21/2025 12:29 AM, Marek Vasut wrote:
>>> [You don't often get email from marek.vasut@mailbox.org. Learn why 
>>> this is important at https://aka.ms/LearnAboutSenderIdentification ]
>>>
>>> Add support for optional allocation of bitstream descriptors from SRAM
>>> instead of DRAM. In case the encoder/decoder DT node contains 'sram'
>>> property which points to 'mmio-sram', the driver will attempt to use
>>> the SRAM instead of DRAM for descriptor allocation, which might improve
>>> performance.
>>>
>>> This however helps on i.MX95 with sporadic SLOTn_STATUS IMG_RD_ERR 
>>> bit 11
>>> being triggered during JPEG encoding. The following pipeline triggers 
>>> the
>>> problem when descriptors get allocated from DRAM, the pipeline often 
>>> hangs
>>> after a few seconds and the encoder driver indicates "timeout, cancel 
>>> it" :
>>
>> It's a hardware bug in i.MX95 A0, and the i.MX95 B0 has fixed this issue.
> 
> Ahh, this helps, thank you.
> 
>> Using sram instead of dram only improves timing, but doesn't 
>> completely circumvent the hardware bug
> Does it make sense to upstream this patch anyway ?

I think this patch is helpful.

But so far, we don't have any SRAM resources for jpeg.
The i.MX95 does have sram, but it is very limited, and it has been
assigned to arm_scmi and VPU.

This patch makes the jpeg can support SRAM. Maybe in some scenarios, we
can assign the SRAM to jpeg instead of VPU.

So for me, I welcome this patch.

Regards,
Ming

Re: [PATCH] media: imx-jpeg: Add support for descriptor allocation from SRAM

[Marek Vasut]

On 8/21/25 11:42 AM, Ming Qian(OSS) wrote:

Hello Ming,

>>> Using sram instead of dram only improves timing, but doesn't 
>>> completely circumvent the hardware bug
>> Does it make sense to upstream this patch anyway ?
> 
> I think this patch is helpful.
> 
> But so far, we don't have any SRAM resources for jpeg.
> The i.MX95 does have sram, but it is very limited, and it has been
> assigned to arm_scmi and VPU.

I found out the SM can work fine with only 192 kiB of SRAM instead of 
256 kiB of SRAM. That frees up 64 kiB of SRAM for NS world. Each JPEG IP 
needs 20544 Bytes for descriptors, which fits into those freed up 64 kiB 
of SRAM.

The alternative would be to handle those AXI read errors and restart the 
encoding. I always observe these errors in the CONFIG phase of encoding. 
Do you happen to have any details of this issue you could share ? Do you 
know whether the problem occurs only during CONFIG phase, or also during 
image read phase ?

If the problem occurs only during CONFIG phase of encoding, it would be 
easy to retrigger the CONFIG I think. If the problem occurs also during 
image read, that might need some more creative fix.

> This patch makes the jpeg can support SRAM. Maybe in some scenarios, we
> can assign the SRAM to jpeg instead of VPU.
> 
> So for me, I welcome this patch.

Thank you

-- 
Best regards,
Marek Vasut

Re: [PATCH] media: imx-jpeg: Add support for descriptor allocation from SRAM

[Ming Qian(OSS)]

On 8/21/2025 8:01 PM, Marek Vasut wrote:
> On 8/21/25 11:42 AM, Ming Qian(OSS) wrote:
> 
> Hello Ming,
> 
>>>> Using sram instead of dram only improves timing, but doesn't 
>>>> completely circumvent the hardware bug
>>> Does it make sense to upstream this patch anyway ?
>>
>> I think this patch is helpful.
>>
>> But so far, we don't have any SRAM resources for jpeg.
>> The i.MX95 does have sram, but it is very limited, and it has been
>> assigned to arm_scmi and VPU.
> 
> I found out the SM can work fine with only 192 kiB of SRAM instead of 
> 256 kiB of SRAM. That frees up 64 kiB of SRAM for NS world. Each JPEG IP 
> needs 20544 Bytes for descriptors, which fits into those freed up 64 kiB 
> of SRAM.
> 
> The alternative would be to handle those AXI read errors and restart the 
> encoding. I always observe these errors in the CONFIG phase of encoding. 
> Do you happen to have any details of this issue you could share ? Do you 
> know whether the problem occurs only during CONFIG phase, or also during 
> image read phase ?
> 
> If the problem occurs only during CONFIG phase of encoding, it would be 
> easy to retrigger the CONFIG I think. If the problem occurs also during 
> image read, that might need some more creative fix.
> 

This is a hardware bug that affects not only the jpeg encoder but also
the jpeg decoder.

This issue occurs under conditions of high backpressure on the AXI bus,
meaning the latency on the write/response channel is high, while the
read channel latency remains low.

When error occurs during CONFIG phase of encoder, it will trigger a
interrupt. But in other phase, there is no interrupt, then the running
job is stalled until timeout.

Regards,
Ming

>> This patch makes the jpeg can support SRAM. Maybe in some scenarios, we
>> can assign the SRAM to jpeg instead of VPU.
>>
>> So for me, I welcome this patch.
> 
> Thank you
>