[PATCH v5 0/3] media: imx-jpeg: Assign a single slot

[PATCH v5 0/3] media: imx-jpeg: Assign a single slot

[Ming Qian (OSS)]

From: Ming Qian <ming.qian@nxp.com>

This IP includes a jpeg wrapper and a jpeg engine, the wrapper is
working on descriptor based manner. It supports up to 4 slots, each slot
can have its own chained descriptors. Host won't configure the engine
directly, but fill some descriptors to encode or decode one jpeg
picture. Then configure the descriptors to certain slot register. The
jpeg wrapper will schedule between different slots. When some slot is
finished, the slot interrupt will be triggered. The purpose of slot is
that engine can be shared across multiple VMS and os.

Currently, power domains and interrupts are enabled for all 4 slots, but
only one slot is used. There is no benefit in using more that one slot
from within the same OS, as the slots are scheduled in round-robin
manner and not executed in parallel.

Use the property "nxp,slot" to assign a single slot, and just expose the
parts of the h/w for the assigned slot. For example, only put slot 1's
power-domains entry in the DT when slot 1 is assigned. If not specified,
0 is used by default.

v5
- improve commit message
- improve property description in bindings document
- update patch based on linux-next next-20231017 (DTS patch already upstream)

v4
- improve commit message
- drop line making the property required, to avoid ABI break

v3
- add vender prefix, change property slot to nxp,slot
- add type for property slot in bindings document

v2
- add a new property in bindings document

Ming Qian (3):
  dt-bindings: media: imx-jpeg: Assign slot for imx jpeg encoder/decoder
  arm64: dts: imx8-ss-img: Add vendor prefix in slot property
  media: imx-jpeg: Add vendor prefix in slot property

 .../bindings/media/nxp,imx8-jpeg.yaml         | 46 +++++++++----------
 .../arm64/boot/dts/freescale/imx8-ss-img.dtsi |  4 +-
 .../media/platform/nxp/imx-jpeg/mxc-jpeg.c    |  2 +-
 3 files changed, 25 insertions(+), 27 deletions(-)


base-commit: 4d5ab2376ec576af173e5eac3887ed0b51bd8566
prerequisite-patch-id: ecdb5f6c6975c1c8f10a56ef7fdb96d9f7d16720
-- 
2.38.1

[PATCH v5 1/3] dt-bindings: media: imx-jpeg: Assign slot for imx jpeg encoder/decoder

[Ming Qian (OSS)]

From: Ming Qian <ming.qian@nxp.com>

This IP includes a jpeg wrapper and a jpeg engine, the wrapper is
working on descriptor based manner. It supports up to 4 slots, each slot
can have its own chained descriptors. Host won't configure the engine
directly, but fill some descriptors to encode or decode one jpeg
picture. Then configure the descriptors to certain slot register. The
jpeg wrapper will schedule between different slots. When some slot is
finished, the slot interrupt will be triggered. The purpose of slot is
that engine can be shared across multiple VMS and os.

Currently, power domains and interrupts are enabled for all 4 slots, but
only one slot is used. There is no benefit in using more that one slot
from within the same OS, as the slots are scheduled in round-robin
manner and not executed in parallel.

Use the property "nxp,slot" to assign a single slot, and just expose the
parts of the h/w for the assigned slot. For example, only put slot 1's
power-domains entry in the DT when slot 1 is assigned. If not specified,
0 is used by default.

Signed-off-by: Ming Qian <ming.qian@nxp.com>
---
v5
- improve commit message
- improve property description

v4
- improve commit message
- drop line making the property required, to avoid ABI break

v3
- add vender prefix, change property slot to nxp,slot
- add type for property slot

v2
- add a new property in bindings document

 .../bindings/media/nxp,imx8-jpeg.yaml         | 46 +++++++++----------
 1 file changed, 22 insertions(+), 24 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
index 3d9d1db37040..0961856bdcab 100644
--- a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
+++ b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
@@ -32,19 +32,27 @@ properties:
     maxItems: 1
 
   interrupts:
-    description: |
-      There are 4 slots available in the IP, which the driver may use
-      If a certain slot is used, it should have an associated interrupt
-      The interrupt with index i is assumed to be for slot i
-    minItems: 1               # At least one slot is needed by the driver
-    maxItems: 4               # The IP has 4 slots available for use
+    description:
+      Interrupt number for slot
+    maxItems: 1
 
   power-domains:
     description:
       List of phandle and PM domain specifier as documented in
       Documentation/devicetree/bindings/power/power_domain.txt
-    minItems: 2               # Wrapper and 1 slot
-    maxItems: 5               # Wrapper and 4 slots
+    minItems: 1               # Mixed power domain
+    maxItems: 2               # Wrapper and 1 slot
+
+  nxp,slot:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Integer number of slot index used. This IP includes a jpeg wrapper, the
+      wrapper is working on descriptor based manner. It supports up to 4 slots,
+      each slot can have its own chained descriptors. The purpose is to share
+      the jpeg engine across multiple VMS and os. We use this property to
+      assign a single slot. If not specified, 0 is used by default.
+    minimum: 0
+    maximum: 3
 
 required:
   - compatible
@@ -62,28 +70,18 @@ examples:
     jpegdec: jpegdec@58400000 {
         compatible = "nxp,imx8qxp-jpgdec";
         reg = <0x58400000 0x00050000 >;
-        interrupts = <GIC_SPI 309 IRQ_TYPE_LEVEL_HIGH>,
-                     <GIC_SPI 310 IRQ_TYPE_LEVEL_HIGH>,
-                     <GIC_SPI 311 IRQ_TYPE_LEVEL_HIGH>,
-                     <GIC_SPI 312 IRQ_TYPE_LEVEL_HIGH>;
+        interrupts = <GIC_SPI 309 IRQ_TYPE_LEVEL_HIGH>;
         power-domains = <&pd IMX_SC_R_MJPEG_DEC_MP>,
-                        <&pd IMX_SC_R_MJPEG_DEC_S0>,
-                        <&pd IMX_SC_R_MJPEG_DEC_S1>,
-                        <&pd IMX_SC_R_MJPEG_DEC_S2>,
-                        <&pd IMX_SC_R_MJPEG_DEC_S3>;
+                        <&pd IMX_SC_R_MJPEG_DEC_S0>;
+        nxp,slot = <0>;
     };
 
     jpegenc: jpegenc@58450000 {
         compatible = "nxp,imx8qm-jpgenc", "nxp,imx8qxp-jpgenc";
         reg = <0x58450000 0x00050000 >;
-        interrupts = <GIC_SPI 305 IRQ_TYPE_LEVEL_HIGH>,
-                     <GIC_SPI 306 IRQ_TYPE_LEVEL_HIGH>,
-                     <GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>,
-                     <GIC_SPI 308 IRQ_TYPE_LEVEL_HIGH>;
+        interrupts = <GIC_SPI 305 IRQ_TYPE_LEVEL_HIGH>;
         power-domains = <&pd IMX_SC_R_MJPEG_ENC_MP>,
-                        <&pd IMX_SC_R_MJPEG_ENC_S0>,
-                        <&pd IMX_SC_R_MJPEG_ENC_S1>,
-                        <&pd IMX_SC_R_MJPEG_ENC_S2>,
-                        <&pd IMX_SC_R_MJPEG_ENC_S3>;
+                        <&pd IMX_SC_R_MJPEG_ENC_S0>;
+        nxp,slot = <0>;
     };
 ...
-- 
2.38.1

[PATCH v5 2/3] arm64: dts: imx8-ss-img: Add vendor prefix in slot property

[Ming Qian (OSS)]

From: Ming Qian <ming.qian@nxp.com>

The slot property isn't generic property, add vendor prefix. Change the
property name to nxp,slot.

This IP includes a jpeg wrapper and a jpeg engine, the wrapper is
working on descriptor based manner. It supports up to 4 slots, each slot
can have its own chained descriptors. Host won't configure the engine
directly, but fill some descriptors to encode or decode one jpeg
picture. Then configure the descriptors to certain slot register. The
jpeg wrapper will schedule between different slots. When some slot is
finished, the slot interrupt will be triggered. The purpose of slot is
that engine can be shared across multiple VMS and os.

Currently, power domains and interrupts are enabled for all 4 slots, but
only one slot is used. There is no benefit in using more that one slot
from within the same OS, as the slots are scheduled in round-robin
manner and not executed in parallel.

Use the property "nxp,slot" to assign a single slot, and just expose the
parts of the h/w for the assigned slot. For example, only put slot 1's
power-domains entry in the DT when slot 1 is assigned. If not specified,
0 is used by default.

Fixes: 5943de495b6d ("arm64: dts: imx8-ss-img: Assign slot for imx jpeg encoder/decoder")
Signed-off-by: Ming Qian <ming.qian@nxp.com>
---
v5
- improve commit message
- update patch based on commit 4d5ab2376ec5

v4
- improve commit message

v3
- add vender prefix, change property slot to nxp,slot

v2
- add a new property in bindings document

 arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
index e7783cc2d830..8ba4b0260a6b 100644
--- a/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
@@ -27,7 +27,7 @@ jpegdec: jpegdec@58400000 {
 		assigned-clock-rates = <200000000>, <200000000>;
 		power-domains = <&pd IMX_SC_R_MJPEG_DEC_MP>,
 				<&pd IMX_SC_R_MJPEG_DEC_S0>;
-		slot = <0>;
+		nxp,slot = <0>;
 	};
 
 	jpegenc: jpegenc@58450000 {
@@ -41,7 +41,7 @@ jpegenc: jpegenc@58450000 {
 		assigned-clock-rates = <200000000>, <200000000>;
 		power-domains = <&pd IMX_SC_R_MJPEG_ENC_MP>,
 				<&pd IMX_SC_R_MJPEG_ENC_S0>;
-		slot = <0>;
+		nxp,slot = <0>;
 	};
 
 	img_jpeg_dec_lpcg: clock-controller@585d0000 {
-- 
2.38.1

[PATCH v5 3/3] media: imx-jpeg: Add vendor prefix in slot property

[Ming Qian (OSS)]

From: Ming Qian <ming.qian@nxp.com>

The slot property isn't generic property, add vendor prefix. Change the
property name to nxp,slot.

Fixes: 53ebeea50599 ("media: imx-jpeg: Support to assign slot for encoder/decoder")
Signed-off-by: Ming Qian <ming.qian@nxp.com>
---
v5
- add vender prefix, change property slot to nxp,slot

v4
- nothing changed here, just modify the bindings and dts

v3
- nothing changed here, just modify the bindings and dts

v2
- nothing changed here, just modify the bindings and dts

 drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
index 64112b63298c..f8b99a292ad1 100644
--- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
+++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
@@ -2762,7 +2762,7 @@ static int mxc_jpeg_probe(struct platform_device *pdev)
 	if (IS_ERR(jpeg->base_reg))
 		return PTR_ERR(jpeg->base_reg);
 
-	ret = of_property_read_u32_index(pdev->dev.of_node, "slot", 0, &jpeg->slot_data.slot);
+	ret = of_property_read_u32_index(pdev->dev.of_node, "nxp,slot", 0, &jpeg->slot_data.slot);
 	if (ret)
 		jpeg->slot_data.slot = 0;
 	dev_info(&pdev->dev, "choose slot %d\n", jpeg->slot_data.slot);
-- 
2.38.1

Re: [PATCH v5 3/3] media: imx-jpeg: Add vendor prefix in slot property

[Krzysztof Kozlowski]

On 19/10/2023 04:48, Ming Qian (OSS) wrote:
> From: Ming Qian <ming.qian@nxp.com>
> 
> The slot property isn't generic property, add vendor prefix. Change the
> property name to nxp,slot.
> 
> Fixes: 53ebeea50599 ("media: imx-jpeg: Support to assign slot for encoder/decoder")
> Signed-off-by: Ming Qian <ming.qian@nxp.com>
> ---
> v5
> - add vender prefix, change property slot to nxp,slot
> 
> v4
> - nothing changed here, just modify the bindings and dts
> 
> v3
> - nothing changed here, just modify the bindings and dts
> 
> v2
> - nothing changed here, just modify the bindings and dts
> 
>  drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> index 64112b63298c..f8b99a292ad1 100644
> --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> @@ -2762,7 +2762,7 @@ static int mxc_jpeg_probe(struct platform_device *pdev)
>  	if (IS_ERR(jpeg->base_reg))
>  		return PTR_ERR(jpeg->base_reg);
>  
> -	ret = of_property_read_u32_index(pdev->dev.of_node, "slot", 0, &jpeg->slot_data.slot);

How did it appear in the code and in the DTS without bindings...

Best regards,
Krzysztof

Re: [PATCH v5 1/3] dt-bindings: media: imx-jpeg: Assign slot for imx jpeg encoder/decoder

[Krzysztof Kozlowski]

On 19/10/2023 04:48, Ming Qian (OSS) wrote:
> From: Ming Qian <ming.qian@nxp.com>
> 
> This IP includes a jpeg wrapper and a jpeg engine, the wrapper is
> working on descriptor based manner. It supports up to 4 slots, each slot
> can have its own chained descriptors. Host won't configure the engine
> directly, but fill some descriptors to encode or decode one jpeg
> picture. Then configure the descriptors to certain slot register. The
> jpeg wrapper will schedule between different slots. When some slot is
> finished, the slot interrupt will be triggered. The purpose of slot is
> that engine can be shared across multiple VMS and os.
> 
> Currently, power domains and interrupts are enabled for all 4 slots, but
> only one slot is used. There is no benefit in using more that one slot
> from within the same OS, as the slots are scheduled in round-robin
> manner and not executed in parallel.
> 
> Use the property "nxp,slot" to assign a single slot, and just expose the
> parts of the h/w for the assigned slot. For example, only put slot 1's
> power-domains entry in the DT when slot 1 is assigned. If not specified,
> 0 is used by default.
> 
> Signed-off-by: Ming Qian <ming.qian@nxp.com>
> ---
> v5
> - improve commit message
> - improve property description
> 
> v4
> - improve commit message
> - drop line making the property required, to avoid ABI break
> 
> v3
> - add vender prefix, change property slot to nxp,slot
> - add type for property slot
> 
> v2
> - add a new property in bindings document
> 
>  .../bindings/media/nxp,imx8-jpeg.yaml         | 46 +++++++++----------
>  1 file changed, 22 insertions(+), 24 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
> index 3d9d1db37040..0961856bdcab 100644
> --- a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
> +++ b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
> @@ -32,19 +32,27 @@ properties:
>      maxItems: 1
>  
>    interrupts:
> -    description: |
> -      There are 4 slots available in the IP, which the driver may use
> -      If a certain slot is used, it should have an associated interrupt
> -      The interrupt with index i is assumed to be for slot i
> -    minItems: 1               # At least one slot is needed by the driver
> -    maxItems: 4               # The IP has 4 slots available for use
> +    description:
> +      Interrupt number for slot
> +    maxItems: 1

The device still has four interrupts, so we should allow up to four of
them. One given OS might want to use two or all four slots.


>  
>    power-domains:
>      description:
>        List of phandle and PM domain specifier as documented in
>        Documentation/devicetree/bindings/power/power_domain.txt
> -    minItems: 2               # Wrapper and 1 slot
> -    maxItems: 5               # Wrapper and 4 slots
> +    minItems: 1               # Mixed power domain
> +    maxItems: 2               # Wrapper and 1 slot
> +
> +  nxp,slot:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Integer number of slot index used. This IP includes a jpeg wrapper, the
> +      wrapper is working on descriptor based manner. It supports up to 4 slots,
> +      each slot can have its own chained descriptors. The purpose is to share
> +      the jpeg engine across multiple VMS and os. We use this property to
> +      assign a single slot. If not specified, 0 is used by default.
> +    minimum: 0
> +    maximum: 3

default: 0



Best regards,
Krzysztof

Re: [PATCH v5 3/3] media: imx-jpeg: Add vendor prefix in slot property

[ming qian]

Hi Krzysztof,

>> From: Ming Qian <ming.qian@nxp.com>
>>
>> The slot property isn't generic property, add vendor prefix. Change the
>> property name to nxp,slot.
>>
>> Fixes: 53ebeea50599 ("media: imx-jpeg: Support to assign slot for encoder/decoder")
>> Signed-off-by: Ming Qian <ming.qian@nxp.com>
>> ---
>> v5
>> - add vender prefix, change property slot to nxp,slot
>>
>> v4
>> - nothing changed here, just modify the bindings and dts
>>
>> v3
>> - nothing changed here, just modify the bindings and dts
>>
>> v2
>> - nothing changed here, just modify the bindings and dts
>>
>>   drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>> index 64112b63298c..f8b99a292ad1 100644
>> --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>> +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>> @@ -2762,7 +2762,7 @@ static int mxc_jpeg_probe(struct platform_device *pdev)
>>   	if (IS_ERR(jpeg->base_reg))
>>   		return PTR_ERR(jpeg->base_reg);
>>   
>> -	ret = of_property_read_u32_index(pdev->dev.of_node, "slot", 0, &jpeg->slot_data.slot);
> 
> How did it appear in the code and in the DTS without bindings...
> 
> Best regards,
> Krzysztof
> 

In the beginning, I only made the patch set of code and dts, the code
patch can work without dts patch due to the default value. The code
patch is merged to media_tree, but mark the dts patch as ´Not
Applicable´. After that, I sent the dts patch again, but maintainer
reminds me that I need to change the binding too. So I made this patch
set. And I guess the dts patch was merged by mistake.

best regards,
Ming

Re: [PATCH v5 1/3] dt-bindings: media: imx-jpeg: Assign slot for imx jpeg encoder/decoder

[ming qian]

Hi Krzysztof,

>> From: Ming Qian <ming.qian@nxp.com>
>>
>> This IP includes a jpeg wrapper and a jpeg engine, the wrapper is
>> working on descriptor based manner. It supports up to 4 slots, each slot
>> can have its own chained descriptors. Host won't configure the engine
>> directly, but fill some descriptors to encode or decode one jpeg
>> picture. Then configure the descriptors to certain slot register. The
>> jpeg wrapper will schedule between different slots. When some slot is
>> finished, the slot interrupt will be triggered. The purpose of slot is
>> that engine can be shared across multiple VMS and os.
>>
>> Currently, power domains and interrupts are enabled for all 4 slots, but
>> only one slot is used. There is no benefit in using more that one slot
>> from within the same OS, as the slots are scheduled in round-robin
>> manner and not executed in parallel.
>>
>> Use the property "nxp,slot" to assign a single slot, and just expose the
>> parts of the h/w for the assigned slot. For example, only put slot 1's
>> power-domains entry in the DT when slot 1 is assigned. If not specified,
>> 0 is used by default.
>>
>> Signed-off-by: Ming Qian <ming.qian@nxp.com>
>> ---
>> v5
>> - improve commit message
>> - improve property description
>>
>> v4
>> - improve commit message
>> - drop line making the property required, to avoid ABI break
>>
>> v3
>> - add vender prefix, change property slot to nxp,slot
>> - add type for property slot
>>
>> v2
>> - add a new property in bindings document
>>
>>   .../bindings/media/nxp,imx8-jpeg.yaml         | 46 +++++++++----------
>>   1 file changed, 22 insertions(+), 24 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
>> index 3d9d1db37040..0961856bdcab 100644
>> --- a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
>> +++ b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
>> @@ -32,19 +32,27 @@ properties:
>>       maxItems: 1
>>   
>>     interrupts:
>> -    description: |
>> -      There are 4 slots available in the IP, which the driver may use
>> -      If a certain slot is used, it should have an associated interrupt
>> -      The interrupt with index i is assumed to be for slot i
>> -    minItems: 1               # At least one slot is needed by the driver
>> -    maxItems: 4               # The IP has 4 slots available for use
>> +    description:
>> +      Interrupt number for slot
>> +    maxItems: 1
> 
> The device still has four interrupts, so we should allow up to four of
> them. One given OS might want to use two or all four slots.
> 
> 
Got it, I will fix it in v6 patch.

>>   
>>     power-domains:
>>       description:
>>         List of phandle and PM domain specifier as documented in
>>         Documentation/devicetree/bindings/power/power_domain.txt
>> -    minItems: 2               # Wrapper and 1 slot
>> -    maxItems: 5               # Wrapper and 4 slots
>> +    minItems: 1               # Mixed power domain
>> +    maxItems: 2               # Wrapper and 1 slot
>> +
>> +  nxp,slot:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    description:
>> +      Integer number of slot index used. This IP includes a jpeg wrapper, the
>> +      wrapper is working on descriptor based manner. It supports up to 4 slots,
>> +      each slot can have its own chained descriptors. The purpose is to share
>> +      the jpeg engine across multiple VMS and os. We use this property to
>> +      assign a single slot. If not specified, 0 is used by default.
>> +    minimum: 0
>> +    maximum: 3
> 
> default: 0
> 
Got it.

Best regards,
Ming
> 
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v5 1/3] dt-bindings: media: imx-jpeg: Assign slot for imx jpeg encoder/decoder

[ming qian]

Hi Krzysztof,

> 
>>> From: Ming Qian <ming.qian@nxp.com>
>>>
>>> This IP includes a jpeg wrapper and a jpeg engine, the wrapper is
>>> working on descriptor based manner. It supports up to 4 slots, each slot
>>> can have its own chained descriptors. Host won't configure the engine
>>> directly, but fill some descriptors to encode or decode one jpeg
>>> picture. Then configure the descriptors to certain slot register. The
>>> jpeg wrapper will schedule between different slots. When some slot is
>>> finished, the slot interrupt will be triggered. The purpose of slot is
>>> that engine can be shared across multiple VMS and os.
>>>
>>> Currently, power domains and interrupts are enabled for all 4 slots, but
>>> only one slot is used. There is no benefit in using more that one slot
>>> from within the same OS, as the slots are scheduled in round-robin
>>> manner and not executed in parallel.
>>>
>>> Use the property "nxp,slot" to assign a single slot, and just expose the
>>> parts of the h/w for the assigned slot. For example, only put slot 1's
>>> power-domains entry in the DT when slot 1 is assigned. If not specified,
>>> 0 is used by default.
>>>
>>> Signed-off-by: Ming Qian <ming.qian@nxp.com>
>>> ---
>>> v5
>>> - improve commit message
>>> - improve property description
>>>
>>> v4
>>> - improve commit message
>>> - drop line making the property required, to avoid ABI break
>>>
>>> v3
>>> - add vender prefix, change property slot to nxp,slot
>>> - add type for property slot
>>>
>>> v2
>>> - add a new property in bindings document
>>>
>>>   .../bindings/media/nxp,imx8-jpeg.yaml         | 46 +++++++++----------
>>>   1 file changed, 22 insertions(+), 24 deletions(-)
>>>
>>> diff --git 
>>> a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml 
>>> b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
>>> index 3d9d1db37040..0961856bdcab 100644
>>> --- a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
>>> +++ b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
>>> @@ -32,19 +32,27 @@ properties:
>>>       maxItems: 1
>>>     interrupts:
>>> -    description: |
>>> -      There are 4 slots available in the IP, which the driver may use
>>> -      If a certain slot is used, it should have an associated interrupt
>>> -      The interrupt with index i is assumed to be for slot i
>>> -    minItems: 1               # At least one slot is needed by the 
>>> driver
>>> -    maxItems: 4               # The IP has 4 slots available for use
>>> +    description:
>>> +      Interrupt number for slot
>>> +    maxItems: 1
>>
>> The device still has four interrupts, so we should allow up to four of
>> them. One given OS might want to use two or all four slots.
>>
>>
> Got it, I will fix it in v6 patch.
> 

We made an internal discussion about this, current approach of the
bindings implies that one dts node is for one slot only, and the slot
property is integer, not a list of slots. Whoever wants to use 2 or more
slots, should put more nodes in the dts, one for each slot. Therefor,
there is no point in allowing more than one interrupt in one node. Or,
if we do, for the sake of allowing one OS to use more slots within the
same dts node, we must also allow more power domains, in any case, we do
not have multiple slots functionality in the driver anymore.

Best regards,
Ming

>>>     power-domains:
>>>       description:
>>>         List of phandle and PM domain specifier as documented in
>>>         Documentation/devicetree/bindings/power/power_domain.txt
>>> -    minItems: 2               # Wrapper and 1 slot
>>> -    maxItems: 5               # Wrapper and 4 slots
>>> +    minItems: 1               # Mixed power domain
>>> +    maxItems: 2               # Wrapper and 1 slot
>>> +
>>> +  nxp,slot:
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    description:
>>> +      Integer number of slot index used. This IP includes a jpeg 
>>> wrapper, the
>>> +      wrapper is working on descriptor based manner. It supports up 
>>> to 4 slots,
>>> +      each slot can have its own chained descriptors. The purpose is 
>>> to share
>>> +      the jpeg engine across multiple VMS and os. We use this 
>>> property to
>>> +      assign a single slot. If not specified, 0 is used by default.
>>> +    minimum: 0
>>> +    maximum: 3
>>
>> default: 0
>>
> Got it.
> 
> Best regards,
> Ming
>>
>>
>> Best regards,
>> Krzysztof
>>

Re: [PATCH v5 3/3] media: imx-jpeg: Add vendor prefix in slot property

[Krzysztof Kozlowski]

On 19/10/2023 09:41, ming qian wrote:
>>> diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>>> index 64112b63298c..f8b99a292ad1 100644
>>> --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>>> +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>>> @@ -2762,7 +2762,7 @@ static int mxc_jpeg_probe(struct platform_device *pdev)
>>>   	if (IS_ERR(jpeg->base_reg))
>>>   		return PTR_ERR(jpeg->base_reg);
>>>   
>>> -	ret = of_property_read_u32_index(pdev->dev.of_node, "slot", 0, &jpeg->slot_data.slot);
>>
>> How did it appear in the code and in the DTS without bindings...
>>
>> Best regards,
>> Krzysztof
>>
> 
> In the beginning, I only made the patch set of code and dts, the code
> patch can work without dts patch due to the default value. The code
> patch is merged to media_tree, but mark the dts patch as ´Not
> Applicable´. After that, I sent the dts patch again, but maintainer
> reminds me that I need to change the binding too. So I made this patch
> set. And I guess the dts patch was merged by mistake.

The code was merged by mistake. You cannot add of_property() ABI to the
Linux without documenting it via bindings. Don't send such patches.

Best regards,
Krzysztof

Re: [PATCH v5 1/3] dt-bindings: media: imx-jpeg: Assign slot for imx jpeg encoder/decoder

[Krzysztof Kozlowski]

On 20/10/2023 07:38, ming qian wrote:
> Hi Krzysztof,
> 
>>
>>>> From: Ming Qian <ming.qian@nxp.com>
>>>>
>>>> This IP includes a jpeg wrapper and a jpeg engine, the wrapper is
>>>> working on descriptor based manner. It supports up to 4 slots, each slot
>>>> can have its own chained descriptors. Host won't configure the engine
>>>> directly, but fill some descriptors to encode or decode one jpeg
>>>> picture. Then configure the descriptors to certain slot register. The
>>>> jpeg wrapper will schedule between different slots. When some slot is
>>>> finished, the slot interrupt will be triggered. The purpose of slot is
>>>> that engine can be shared across multiple VMS and os.
>>>>
>>>> Currently, power domains and interrupts are enabled for all 4 slots, but
>>>> only one slot is used. There is no benefit in using more that one slot
>>>> from within the same OS, as the slots are scheduled in round-robin
>>>> manner and not executed in parallel.
>>>>
>>>> Use the property "nxp,slot" to assign a single slot, and just expose the
>>>> parts of the h/w for the assigned slot. For example, only put slot 1's
>>>> power-domains entry in the DT when slot 1 is assigned. If not specified,
>>>> 0 is used by default.
>>>>
>>>> Signed-off-by: Ming Qian <ming.qian@nxp.com>
>>>> ---
>>>> v5
>>>> - improve commit message
>>>> - improve property description
>>>>
>>>> v4
>>>> - improve commit message
>>>> - drop line making the property required, to avoid ABI break
>>>>
>>>> v3
>>>> - add vender prefix, change property slot to nxp,slot
>>>> - add type for property slot
>>>>
>>>> v2
>>>> - add a new property in bindings document
>>>>
>>>>   .../bindings/media/nxp,imx8-jpeg.yaml         | 46 +++++++++----------
>>>>   1 file changed, 22 insertions(+), 24 deletions(-)
>>>>
>>>> diff --git 
>>>> a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml 
>>>> b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
>>>> index 3d9d1db37040..0961856bdcab 100644
>>>> --- a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
>>>> +++ b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
>>>> @@ -32,19 +32,27 @@ properties:
>>>>       maxItems: 1
>>>>     interrupts:
>>>> -    description: |
>>>> -      There are 4 slots available in the IP, which the driver may use
>>>> -      If a certain slot is used, it should have an associated interrupt
>>>> -      The interrupt with index i is assumed to be for slot i
>>>> -    minItems: 1               # At least one slot is needed by the 
>>>> driver
>>>> -    maxItems: 4               # The IP has 4 slots available for use
>>>> +    description:
>>>> +      Interrupt number for slot
>>>> +    maxItems: 1
>>>
>>> The device still has four interrupts, so we should allow up to four of
>>> them. One given OS might want to use two or all four slots.
>>>
>>>
>> Got it, I will fix it in v6 patch.
>>
> 
> We made an internal discussion about this, current approach of the

How does it help me? Why private discussion should matter to us?

> bindings implies that one dts node is for one slot only, and the slot
> property is integer, not a list of slots. Whoever wants to use 2 or more
> slots, should put more nodes in the dts, one for each slot. Therefor,

Really? Under the same IO address? Please show how it works and then run
`dtbs_check W=1`..

> there is no point in allowing more than one interrupt in one node. Or,
> if we do, for the sake of allowing one OS to use more slots within the
> same dts node, we must also allow more power domains, in any case, we do

power-domains, interrupts, everything.

> not have multiple slots functionality in the driver anymore.

Why your driver should limit bindings? My driver supports multiple slots...

Best regards,
Krzysztof