[PATCH v10 0/3] Mediatek MT8189 JPEG support

[PATCH v10 0/3] Mediatek MT8189 JPEG support

[Jianhua Lin]

This series is based on linux-next tag next-20260707.

This series depends on commit 7560349ee0d9 ("media: mediatek: jpeg:
support 34bits"), which introduced the `support_34bit` field in struct
mtk_jpeg_variant. That commit has already been merged via the media
tree and is present in linux-next as of next-20260707.

Changes compared with v9:
- Patches 1/3 & 2/3 (dt-bindings: decoder & encoder):
  - Explicitly set `mediatek,larb: false` in the `else` branch of
    the `allOf` block for older SoCs to prevent incorrect configuration
    and ensure backward compatibility.

Changes compared with v8:
- Patches 1/3 & 2/3 (dt-bindings: decoder & encoder):
  - Expand the description of the `mediatek,larb` property to explicitly
    explain its necessity for accurately describing the hardware topology
    (SMI LARB connection for memory bandwidth arbitration), independent
    of the IOMMU driver's implicit handling.

Changes compared with v7:
- Patches 1/3 (dt-bindings: decoder):
  - In the allOf constraints, only keep maxItems: 1 in the then 
    branch and minItems: 2 in the else branch; remove duplicated
    constraints already defined at the top level

Changes compared with v6:
- Patches 1/3 (dt-bindings: decoder):
  update the existing `allOf` condition for mediatek,mt8189-jpgdec to
  make the 'mediatek,larb' property strictly required for MT8189 SoC.
- Patches 2/3 (dt-bindings: encoder):
  Add an `allOf` condition to enforce that the `mediatek,larb` property
  is strictly required when the compatible string contains
  mediatek,mt8189-jpgenc.

Changes compared with v5:
- Patches 1/3 (dt-bindings: decoder):
  - Drop top-level minItems/maxItems for clock-names per Krzysztof's
    review.
  - Refine allOf block to strictly enforce clock constraints.

Changes compared with v4:
- Refines the device tree bindings for JPEG decoder and encoder.
  - Patches 1/3 (dt-bindings: decoder):
    Moved the standalone compatible string mediatek,mt8189-jpgdec
    into the first oneOf entry along with mt2701 and mt8173, as
    suggested by Rob Herring. This correctly groups all independent
    ICs and removes the redundant items wrapper.
  - Patches 2/3 (dt-bindings: encoder):
    Applied the same logic suggested by Rob Herring to the encoder
    binding. Restructured the compatible property to clearly
    distinguish between the standalone IC (mediatek,mt8189-jpgenc)
    and the ICs that must fallback to mediatek,mtk-jpgenc.

Changes compared with v3:
- The v4 is resending the cover-letter, because the v3 cover-letter was
  not sent successfully.

Changes compared with v2:
- Dropped the dts patch (arm64: dts: mt8188: update JPEG encoder/decoder
  compatible) as it belongs to a different tree/series.
- Patches 1/3 (dt-bindings: decoder):
  - Changed the MT8189 compatible to be a standalone `const` instead of
    an `enum`.
  - Added an `allOf` block with conditional checks to enforce the single
    clock ("jpgdec") requirement for MT8189, while preserving the
    two-clock requirement for older SoCs.
  - Updated commit message to reflect the schema structure changes and
    hardware differences.
- Patches 2/3 (dt-bindings: encoder):
  - Changed the MT8189 compatible to be a standalone `const` instead of
    an `enum` inside the `items` list, as it does not fallback to
    "mediatek,mtk-jpgenc" due to 34-bit IOVA requirements.
  - Updated commit message to explain the standalone compatible design.
- Patches 3/3 (media: mediatek: jpeg):
  - Refined commit message for better clarity regarding 34-bit IOVA and
    single clock configuration.

Changes compared with v1:
- Patches 1/4:
  - Updating commit message
- Patches 2/4, 3/4: 
  - Updating commit message
  - Adjusted property descriptions acorrding to hardware requirements
  - Improved formatting for better readability and consistency
- Patches 4/4:
  - Updating commit message

Jianhua Lin (3):
  dt-bindings: media: mediatek-jpeg-decoder: add MT8189 compatible
    string
  dt-bindings: media: mediatek-jpeg-encoder: add MT8189 compatible
    string
  media: mediatek: jpeg: add compatible for MT8189 SoC

 .../bindings/media/mediatek-jpeg-decoder.yaml | 50 ++++++++++++++++---
 .../bindings/media/mediatek-jpeg-encoder.yaml | 38 +++++++++++---
 .../platform/mediatek/jpeg/mtk_jpeg_core.c    | 44 ++++++++++++++++
 3 files changed, 118 insertions(+), 14 deletions(-)

-- 
2.45.2

[PATCH v10 1/3] dt-bindings: media: mediatek-jpeg-decoder: add MT8189 compatible string

[Jianhua Lin]

Add the compatible string for the JPEG decoder block found in the
MediaTek MT8189 SoC.

Compared to previous generation ICs, the MT8189 JPEG decoder requires
34-bit IOVA address space support and only needs a single clock
("jpgdec") instead of two. Therefore, it is added as a standalone
compatible string without falling back to older SoCs.

Update the binding schema to include the new compatible string and add
an `allOf` block with conditional checks. This enforces the single clock
requirement for MT8189 while preserving the two-clock requirement
("jpgdec-smi", "jpgdec") for older SoCs.

Signed-off-by: Jianhua Lin <jianhua.lin@mediatek.com>
---
 .../bindings/media/mediatek-jpeg-decoder.yaml | 50 ++++++++++++++++---
 1 file changed, 42 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/mediatek-jpeg-decoder.yaml b/Documentation/devicetree/bindings/media/mediatek-jpeg-decoder.yaml
index a4aacd3eb189..654360c19c48 100644
--- a/Documentation/devicetree/bindings/media/mediatek-jpeg-decoder.yaml
+++ b/Documentation/devicetree/bindings/media/mediatek-jpeg-decoder.yaml
@@ -15,10 +15,10 @@ description: |-
 properties:
   compatible:
     oneOf:
-      - items:
-          - enum:
-              - mediatek,mt8173-jpgdec
-              - mediatek,mt2701-jpgdec
+      - enum:
+          - mediatek,mt2701-jpgdec
+          - mediatek,mt8173-jpgdec
+          - mediatek,mt8189-jpgdec
       - items:
           - enum:
               - mediatek,mt7623-jpgdec
@@ -32,13 +32,16 @@ properties:
     maxItems: 1
 
   clocks:
+    minItems: 1
     maxItems: 2
-    minItems: 2
 
   clock-names:
-    items:
-      - const: jpgdec-smi
-      - const: jpgdec
+    oneOf:
+      - items:
+          - const: jpgdec
+      - items:
+          - const: jpgdec-smi
+          - const: jpgdec
 
   power-domains:
     maxItems: 1
@@ -51,6 +54,15 @@ properties:
       Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml for details.
       Ports are according to the HW.
 
+  mediatek,larb:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: |
+      A phandle to the local arbiter (SMI LARB) node. This explicitly
+      describes the hardware topology where the JPEG block is connected
+      to a dedicated SMI LARB for memory bandwidth arbitration. It is
+      required to accurately represent the hardware connection independently
+      of the IOMMU.
+
 required:
   - compatible
   - reg
@@ -60,6 +72,28 @@ required:
   - power-domains
   - iommus
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: mediatek,mt8189-jpgdec
+    then:
+      properties:
+        clocks:
+          maxItems: 1
+        clock-names:
+          maxItems: 1
+      required:
+        - mediatek,larb
+    else:
+      properties:
+        clocks:
+          minItems: 2
+        clock-names:
+          minItems: 2
+        mediatek,larb: false
+
 additionalProperties: false
 
 examples:
-- 
2.45.2

[PATCH v10 2/3] dt-bindings: media: mediatek-jpeg-encoder: add MT8189 compatible string

[Jianhua Lin]

Add the compatible string for the JPEG encoder block found in the
MediaTek MT8189 SoC.

Unlike some previous SoCs, the MT8189 JPEG encoder requires 34-bit IOVA
address space support. Therefore, it is added as a standalone compatible
string without falling back to the generic "mediatek,mtk-jpgenc" to
ensure the driver applies the correct hardware-specific configurations.

Signed-off-by: Jianhua Lin <jianhua.lin@mediatek.com>
---
 .../bindings/media/mediatek-jpeg-encoder.yaml | 38 ++++++++++++++++---
 1 file changed, 32 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.yaml b/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.yaml
index 5b15f8977f67..d2f035715a2e 100644
--- a/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.yaml
+++ b/Documentation/devicetree/bindings/media/mediatek-jpeg-encoder.yaml
@@ -14,13 +14,17 @@ description: |-
 
 properties:
   compatible:
-    items:
+    oneOf:
       - enum:
-          - mediatek,mt2701-jpgenc
-          - mediatek,mt8183-jpgenc
-          - mediatek,mt8186-jpgenc
-          - mediatek,mt8188-jpgenc
-      - const: mediatek,mtk-jpgenc
+          - mediatek,mt8189-jpgenc
+      - items:
+          - enum:
+              - mediatek,mt2701-jpgenc
+              - mediatek,mt8183-jpgenc
+              - mediatek,mt8186-jpgenc
+              - mediatek,mt8188-jpgenc
+          - const: mediatek,mtk-jpgenc
+
   reg:
     maxItems: 1
 
@@ -45,6 +49,15 @@ properties:
       Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml for details.
       Ports are according to the HW.
 
+  mediatek,larb:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: |
+      A phandle to the local arbiter (SMI LARB) node. This explicitly
+      describes the hardware topology where the JPEG block is connected
+      to a dedicated SMI LARB for memory bandwidth arbitration. It is
+      required to accurately represent the hardware connection independently
+      of the IOMMU.
+
 required:
   - compatible
   - reg
@@ -54,6 +67,19 @@ required:
   - power-domains
   - iommus
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: mediatek,mt8189-jpgenc
+    then:
+      required:
+        - mediatek,larb
+    else:
+      properties:
+        mediatek,larb: false
+
 additionalProperties: false
 
 examples:
-- 
2.45.2

[PATCH v10 3/3] media: mediatek: jpeg: add compatible for MT8189 SoC

[Jianhua Lin]

Compared to the previous generation ICs, the MT8189 uses a 34-bit IOVA
address space (16GB) and requires a single clock configuration.

Therefore, add new compatible strings ("mediatek,mt8189-jpgenc" and
"mediatek,mt8189-jpgdec") along with their specific driver data to
support the JPEG encoder and decoder of the MT8189 SoC.

Signed-off-by: Jianhua Lin <jianhua.lin@mediatek.com>
---
 .../platform/mediatek/jpeg/mtk_jpeg_core.c    | 44 +++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
index d147ec483081..14f2991a4053 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
@@ -1868,6 +1868,10 @@ static struct clk_bulk_data mt8173_jpeg_dec_clocks[] = {
 	{ .id = "jpgdec" },
 };
 
+static struct clk_bulk_data mtk_jpeg_dec_clocks[] = {
+	{ .id = "jpgdec" },
+};
+
 static const struct mtk_jpeg_variant mt8173_jpeg_drvdata = {
 	.clks = mt8173_jpeg_dec_clocks,
 	.num_clks = ARRAY_SIZE(mt8173_jpeg_dec_clocks),
@@ -1899,6 +1903,38 @@ static const struct mtk_jpeg_variant mtk_jpeg_drvdata = {
 	.multi_core = false,
 };
 
+static const struct mtk_jpeg_variant mtk8189_jpegenc_drvdata = {
+	.clks = mtk_jpeg_clocks,
+	.num_clks = ARRAY_SIZE(mtk_jpeg_clocks),
+	.formats = mtk_jpeg_enc_formats,
+	.num_formats = MTK_JPEG_ENC_NUM_FORMATS,
+	.qops = &mtk_jpeg_enc_qops,
+	.irq_handler = mtk_jpeg_enc_irq,
+	.hw_reset = mtk_jpeg_enc_reset,
+	.m2m_ops = &mtk_jpeg_enc_m2m_ops,
+	.dev_name = "mtk-jpeg-enc",
+	.ioctl_ops = &mtk_jpeg_enc_ioctl_ops,
+	.out_q_default_fourcc = V4L2_PIX_FMT_YUYV,
+	.cap_q_default_fourcc = V4L2_PIX_FMT_JPEG,
+	.support_34bit = true,
+};
+
+static const struct mtk_jpeg_variant mtk8189_jpegdec_drvdata = {
+	.clks = mtk_jpeg_dec_clocks,
+	.num_clks = ARRAY_SIZE(mtk_jpeg_dec_clocks),
+	.formats = mtk_jpeg_dec_formats,
+	.num_formats = MTK_JPEG_DEC_NUM_FORMATS,
+	.qops = &mtk_jpeg_dec_qops,
+	.irq_handler = mtk_jpeg_dec_irq,
+	.hw_reset = mtk_jpeg_dec_reset,
+	.m2m_ops = &mtk_jpeg_dec_m2m_ops,
+	.dev_name = "mtk-jpeg-dec",
+	.ioctl_ops = &mtk_jpeg_dec_ioctl_ops,
+	.out_q_default_fourcc = V4L2_PIX_FMT_JPEG,
+	.cap_q_default_fourcc = V4L2_PIX_FMT_YUV420M,
+	.support_34bit = true,
+};
+
 static struct mtk_jpeg_variant mtk8195_jpegenc_drvdata = {
 	.formats = mtk_jpeg_enc_formats,
 	.num_formats = MTK_JPEG_ENC_NUM_FORMATS,
@@ -1938,6 +1974,14 @@ static const struct of_device_id mtk_jpeg_match[] = {
 		.compatible = "mediatek,mtk-jpgenc",
 		.data = &mtk_jpeg_drvdata,
 	},
+	{
+		.compatible = "mediatek,mt8189-jpgenc",
+		.data = &mtk8189_jpegenc_drvdata,
+	},
+	{
+		.compatible = "mediatek,mt8189-jpgdec",
+		.data = &mtk8189_jpegdec_drvdata,
+	},
 	{
 		.compatible = "mediatek,mt8195-jpgenc",
 		.data = &mtk8195_jpegenc_drvdata,
-- 
2.45.2

Re: [PATCH v10 3/3] media: mediatek: jpeg: add compatible for MT8189 SoC

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

New issues:
- [Medium] Missing DMA Mask Configuration for 34-bit IOVA support.

Pre-existing issues:
- [High] Use-After-Free when unbound while a V4L2 file descriptor is held open.
- [High] Use-After-Free due to uncanceled job timeout delayed work during unbind.
- [High] NULL Pointer Dereference due to interrupt request before memory-to-memory context initialization.
- [High] DMA Address Calculation Bug for 34-bit stall address extension.
--

--- Patch [3]: [PATCH v10 3/3] media: mediatek: jpeg: add compatible for MT8189 SoC ---
commit 3c5efcbd1b6f6e5dab41b7f01500bcc550eb3c09
Author: Jianhua Lin <jianhua.lin@mediatek.com>

media: mediatek: jpeg: add compatible for MT8189 SoC

This commit adds new compatible strings and specific driver data to support
the JPEG encoder and decoder on the MT8189 SoC, which uses a 34-bit IOVA
address space.

> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> index d147ec4830811..14f2991a40535 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c

[ ... ]

> @@ -1899,6 +1903,38 @@ static const struct mtk_jpeg_variant mtk_jpeg_drvdata = {
>  	.multi_core = false,
>  };
>  
> +static const struct mtk_jpeg_variant mtk8189_jpegenc_drvdata = {
> +	.clks = mtk_jpeg_clocks,
> +	.num_clks = ARRAY_SIZE(mtk_jpeg_clocks),
> +	.formats = mtk_jpeg_enc_formats,
> +	.num_formats = MTK_JPEG_ENC_NUM_FORMATS,
> +	.qops = &mtk_jpeg_enc_qops,
> +	.irq_handler = mtk_jpeg_enc_irq,
> +	.hw_reset = mtk_jpeg_enc_reset,
> +	.m2m_ops = &mtk_jpeg_enc_m2m_ops,
> +	.dev_name = "mtk-jpeg-enc",
> +	.ioctl_ops = &mtk_jpeg_enc_ioctl_ops,
> +	.out_q_default_fourcc = V4L2_PIX_FMT_YUYV,
> +	.cap_q_default_fourcc = V4L2_PIX_FMT_JPEG,
> +	.support_34bit = true,

[Severity: Medium]
Will the 34-bit addressing hardware capability be silently disabled by the
kernel's DMA API?

The driver sets .support_34bit = true for the new MT8189 variants, intending
to allow the hardware to address up to 16GB.

However, the driver never calls dma_set_mask_and_coherent() or dma_set_mask()
in the probe sequence. Without this call, the kernel's DMA APIs assume the
default 32-bit mask (4GB) for platform devices.

Does the driver need to configure the DMA mask during initialization when
support_34bit is enabled?

[Severity: High]
This is a pre-existing issue, but does enabling .support_34bit = true here
activate a buggy DMA address calculation path in mtk_jpeg_set_enc_dst()?

Looking at drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c, the 34-bit
extension logic correctly calculates the base 2-bit extension:

    addr_ext = FIELD_PREP(MTK_JPEG_ADDR_MASK, upper_32_bits(dma_addr));

But for the stall address, it erroneously writes addr_ext + size:

    writel(addr_ext + size, base + JPEG_ENC_STALL_ADDR0_EXT);

Since size is the byte size of the buffer (e.g., 0x20000), wouldn't this write
an improperly large value into a register meant for the upper 2 bits of the
address, rather than calculating upper_32_bits(dma_addr + size) & 3?

Could this lead to memory corruption or IOMMU faults by incorrectly setting
the upper address bits for the hardware DMA?

> +};
> +
> +static const struct mtk_jpeg_variant mtk8189_jpegdec_drvdata = {
> +	.clks = mtk_jpeg_dec_clocks,
> +	.num_clks = ARRAY_SIZE(mtk_jpeg_dec_clocks),
> +	.formats = mtk_jpeg_dec_formats,
> +	.num_formats = MTK_JPEG_DEC_NUM_FORMATS,
> +	.qops = &mtk_jpeg_dec_qops,
> +	.irq_handler = mtk_jpeg_dec_irq,
> +	.hw_reset = mtk_jpeg_dec_reset,
> +	.m2m_ops = &mtk_jpeg_dec_m2m_ops,
> +	.dev_name = "mtk-jpeg-dec",
> +	.ioctl_ops = &mtk_jpeg_dec_ioctl_ops,
> +	.out_q_default_fourcc = V4L2_PIX_FMT_JPEG,
> +	.cap_q_default_fourcc = V4L2_PIX_FMT_YUV420M,
> +	.support_34bit = true,
> +};

[ ... ]

> @@ -1938,6 +1974,14 @@ static const struct of_device_id mtk_jpeg_match[] = {
>  		.compatible = "mediatek,mtk-jpgenc",
>  		.data = &mtk_jpeg_drvdata,
>  	},
> +	{
> +		.compatible = "mediatek,mt8189-jpgenc",
> +		.data = &mtk8189_jpegenc_drvdata,
> +	},

[Severity: High]
This isn't a bug introduced by this patch, but is there a use-after-free
if the device is unbound while a userspace application holds the V4L2
character device file descriptor open?

The struct mtk_jpeg_dev pointer is allocated via devm_kzalloc() and its
lifetime is tied to the platform device. In mtk_jpeg_remove(), the driver
unregisters the V4L2 device and explicitly releases m2m_dev:

mtk_jpeg_remove() {
    ...
    v4l2_m2m_release(jpeg->m2m_dev);
    v4l2_device_unregister(&jpeg->v4l2_dev);
}

The core structure is freed immediately by devres when remove() returns.

When the application later closes the file, mtk_jpeg_release() is called and
accesses the devres-freed jpeg structure:

mtk_jpeg_release() {
    struct mtk_jpeg_dev *jpeg = video_drvdata(file);
    ...
    mutex_lock(&jpeg->lock);
    ...
}

Should the driver wait for file operations to complete or manage the lifetime
of the jpeg structure differently?

[Severity: High]
This isn't a bug introduced by this patch, but does unbinding the device while
a hardware job is in flight cause a use-after-free via the job timeout delayed
work?

mtk_jpeg_single_core_init() initializes a delayed work jpeg->job_timeout_work.
When hardware processing starts, this work is scheduled.

However, mtk_jpeg_remove() fails to call cancel_delayed_work_sync() for
jpeg->job_timeout_work.

If the device is unbound before the timeout expires, the struct mtk_jpeg_dev
will be freed by devres. When the timer eventually expires,
mtk_jpeg_job_timeout_work() will dereference the freed jpeg pointer.

Should mtk_jpeg_remove() cancel this work before returning?

[Severity: High]
This isn't a bug introduced by this patch, but is there a risk of a NULL
pointer dereference if a hardware interrupt fires immediately after the IRQ
is requested?

In mtk_jpeg_single_core_init(), the driver calls devm_request_irq() to
register and enable the IRQ handler.

However, jpeg->m2m_dev is not initialized until mtk_jpeg_probe() calls
v4l2_m2m_init() later in the execution flow.

If the bootloader leaves the hardware in an active or errored state, the
interrupt could fire instantly. mtk_jpeg_enc_irq() or mtk_jpeg_dec_irq() will
invoke mtk_jpeg_enc_done(), which attempts to fetch the current context:

    ctx = v4l2_m2m_get_curr_priv(jpeg->m2m_dev);

Since jpeg->m2m_dev is still NULL at this point, will this dereference a NULL
pointer?

> +	{
> +		.compatible = "mediatek,mt8189-jpgdec",
> +		.data = &mtk8189_jpegdec_drvdata,
> +	},
>  	{
>  		.compatible = "mediatek,mt8195-jpgenc",
>  		.data = &mtk8195_jpegenc_drvdata,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720062829.7343-1-jianhua.lin@mediatek.com?part=3