2023-09-27 10:44:55

by Ming Qian

[permalink] [raw]
Subject: RE: [EXT] Re: [PATCH v4 2/2] dt-bindings: media: imx-jpeg: Assign slot for imx jpeg encoder/decoder

>From: Krzysztof Kozlowski <[email protected]>
>Sent: 2023年9月27日 16:06
>To: Ming Qian <[email protected]>; Mirela Rabulea (OSS)
><[email protected]>; [email protected]; [email protected]
>Cc: [email protected]; [email protected];
>[email protected]; [email protected]; [email protected];
>[email protected]; [email protected]; X.H. Bao
><[email protected]>; Eagle Zhou <[email protected]>; Tao Jiang
><[email protected]>; dl-linux-imx <[email protected]>;
>[email protected]; [email protected]; linux-
>[email protected]; [email protected]
>Subject: [EXT] Re: [PATCH v4 2/2] dt-bindings: media: imx-jpeg: Assign slot for
>imx jpeg encoder/decoder
>
>Caution: This is an external email. Please take care when clicking links or
>opening attachments. When in doubt, report the message using the 'Report
>this email' button
>
>
>On 26/09/2023 12:10, Ming Qian wrote:
>> There are total 4 slots available in the IP, and we only need to use
>> one slot in one os, assign a single slot, configure interrupt and
>> power domain only for 1 slot, not for the all 4 slots.
>>
>> Signed-off-by: Ming Qian <[email protected]>
>> ---
>> v4
>> - improve commit message
>> - don't make an ABI break
>> v3
>> - add vender prefix, change property slot to nxp,slot
>> - add type for property slot
>>
>> .../bindings/media/nxp,imx8-jpeg.yaml | 45 +++++++++----------
>> 1 file changed, 21 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..4bcfc815c894 100644
>> --- a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
>> +++ b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
>> @@ -32,19 +32,26 @@ 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 # VPUMIX
>> + maxItems: 2 # Wrapper and 1 slot
>> +
>> + nxp,slot:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description:
>> + Integer number of slot index used. There are 4 slots available in the IP,
>> + and driver can use a certain slot, it should have an associated interrupt
>> + and power-domain. In theory, it supports 4 os or vm. If not specified, 0
>> + is used by default.
>> + minimum: 0
>> + maximum: 3
>
>NAK, you still did not answer concerns why this is needed and justified.
>I keep asking and you keep ignoring...
>
>Best regards,
>Krzysztof

Hi Krzysztof,

> Nothing explains what is a slot and nothing explains why do you need this property.

I thought I had answered that question, but seems you don't agree with that explanation.
Would the following description be any better?

The slot is the IP used to share the jpeg engine across multiple VMS and os, there are 4 slots available in the IP. We can use one slot in linux. The slot index indicates which slot we choose. If not specified, 0 is used by default.



2023-09-28 07:22:59

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [EXT] Re: [PATCH v4 2/2] dt-bindings: media: imx-jpeg: Assign slot for imx jpeg encoder/decoder

On 27/09/2023 11:10, Ming Qian wrote:
>> From: Krzysztof Kozlowski <[email protected]>
>> Sent: 2023年9月27日 16:06
>> To: Ming Qian <[email protected]>; Mirela Rabulea (OSS)
>> <[email protected]>; [email protected]; [email protected]
>> Cc: [email protected]; [email protected];
>> [email protected]; [email protected]; [email protected];
>> [email protected]; [email protected]; X.H. Bao
>> <[email protected]>; Eagle Zhou <[email protected]>; Tao Jiang
>> <[email protected]>; dl-linux-imx <[email protected]>;
>> [email protected]; [email protected]; linux-
>> [email protected]; [email protected]
>> Subject: [EXT] Re: [PATCH v4 2/2] dt-bindings: media: imx-jpeg: Assign slot for
>> imx jpeg encoder/decoder
>>
>> Caution: This is an external email. Please take care when clicking links or
>> opening attachments. When in doubt, report the message using the 'Report
>> this email' button
>>
>>
>> On 26/09/2023 12:10, Ming Qian wrote:
>>> There are total 4 slots available in the IP, and we only need to use
>>> one slot in one os, assign a single slot, configure interrupt and
>>> power domain only for 1 slot, not for the all 4 slots.
>>>
>>> Signed-off-by: Ming Qian <[email protected]>
>>> ---
>>> v4
>>> - improve commit message
>>> - don't make an ABI break
>>> v3
>>> - add vender prefix, change property slot to nxp,slot
>>> - add type for property slot
>>>
>>> .../bindings/media/nxp,imx8-jpeg.yaml | 45 +++++++++----------
>>> 1 file changed, 21 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..4bcfc815c894 100644
>>> --- a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
>>> +++ b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
>>> @@ -32,19 +32,26 @@ 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 # VPUMIX
>>> + maxItems: 2 # Wrapper and 1 slot
>>> +
>>> + nxp,slot:
>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>> + description:
>>> + Integer number of slot index used. There are 4 slots available in the IP,
>>> + and driver can use a certain slot, it should have an associated interrupt
>>> + and power-domain. In theory, it supports 4 os or vm. If not specified, 0
>>> + is used by default.
>>> + minimum: 0
>>> + maximum: 3
>>
>> NAK, you still did not answer concerns why this is needed and justified.
>> I keep asking and you keep ignoring...
>>
>> Best regards,
>> Krzysztof
>
> Hi Krzysztof,
>
>> Nothing explains what is a slot and nothing explains why do you need this property.
>
> I thought I had answered that question, but seems you don't agree with that explanation.
> Would the following description be any better?

No, you didn't. I don't see any explanation in the commit msg. You just
say you have "4 slots".

Best regards,
Krzysztof

2023-09-28 08:24:38

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [EXT] Re: [PATCH v4 2/2] dt-bindings: media: imx-jpeg: Assign slot for imx jpeg encoder/decoder

On 27/09/2023 11:10, Ming Qian wrote:
>> From: Krzysztof Kozlowski <[email protected]>
>> Sent: 2023年9月27日 16:06
>> To: Ming Qian <[email protected]>; Mirela Rabulea (OSS)
>> <[email protected]>; [email protected]; [email protected]
>> Cc: [email protected]; [email protected];
>> [email protected]; [email protected]; [email protected];
>> [email protected]; [email protected]; X.H. Bao
>> <[email protected]>; Eagle Zhou <[email protected]>; Tao Jiang
>> <[email protected]>; dl-linux-imx <[email protected]>;
>> [email protected]; [email protected]; linux-
>> [email protected]; [email protected]
>> Subject: [EXT] Re: [PATCH v4 2/2] dt-bindings: media: imx-jpeg: Assign slot for
>> imx jpeg encoder/decoder
>>
>> Caution: This is an external email. Please take care when clicking links or
>> opening attachments. When in doubt, report the message using the 'Report
>> this email' button
>>
>>
>> On 26/09/2023 12:10, Ming Qian wrote:
>>> There are total 4 slots available in the IP, and we only need to use
>>> one slot in one os, assign a single slot, configure interrupt and
>>> power domain only for 1 slot, not for the all 4 slots.
>>>
>>> Signed-off-by: Ming Qian <[email protected]>
>>> ---
>>> v4
>>> - improve commit message
>>> - don't make an ABI break
>>> v3
>>> - add vender prefix, change property slot to nxp,slot
>>> - add type for property slot
>>>
>>> .../bindings/media/nxp,imx8-jpeg.yaml | 45 +++++++++----------
>>> 1 file changed, 21 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..4bcfc815c894 100644
>>> --- a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
>>> +++ b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
>>> @@ -32,19 +32,26 @@ 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 # VPUMIX
>>> + maxItems: 2 # Wrapper and 1 slot
>>> +
>>> + nxp,slot:
>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>> + description:
>>> + Integer number of slot index used. There are 4 slots available in the IP,
>>> + and driver can use a certain slot, it should have an associated interrupt
>>> + and power-domain. In theory, it supports 4 os or vm. If not specified, 0
>>> + is used by default.
>>> + minimum: 0
>>> + maximum: 3
>>
>> NAK, you still did not answer concerns why this is needed and justified.
>> I keep asking and you keep ignoring...
>>
>> Best regards,
>> Krzysztof
>
> Hi Krzysztof,
>
>> Nothing explains what is a slot and nothing explains why do you need this property.
>
> I thought I had answered that question, but seems you don't agree with that explanation.
> Would the following description be any better?

No, you didn't. I don't see any explanation in the commit msg. You just
say you have "4 slots".

Best regards,
Krzysztof

2023-09-28 15:42:57

by Ming Qian

[permalink] [raw]
Subject: RE: [EXT] Re: [PATCH v4 2/2] dt-bindings: media: imx-jpeg: Assign slot for imx jpeg encoder/decoder

>From: Krzysztof Kozlowski <[email protected]>
>Sent: 2023年9月28日 13:07
>To: Ming Qian <[email protected]>; Mirela Rabulea (OSS)
><[email protected]>; [email protected]; [email protected]
>Cc: [email protected]; [email protected];
>[email protected]; [email protected]; [email protected];
>[email protected]; [email protected]; X.H. Bao
><[email protected]>; Eagle Zhou <[email protected]>; Tao Jiang
><[email protected]>; dl-linux-imx <[email protected]>;
>[email protected]; [email protected]; linux-
>[email protected]; [email protected]
>Subject: Re: [EXT] Re: [PATCH v4 2/2] dt-bindings: media: imx-jpeg: Assign slot
>for imx jpeg encoder/decoder
>
>Caution: This is an external email. Please take care when clicking links or
>opening attachments. When in doubt, report the message using the 'Report
>this email' button
>
>
>On 27/09/2023 11:10, Ming Qian wrote:
>>> From: Krzysztof Kozlowski <[email protected]>
>>> Sent: 2023年9月27日 16:06
>>> To: Ming Qian <[email protected]>; Mirela Rabulea (OSS)
>>> <[email protected]>; [email protected];
>[email protected]
>>> Cc: [email protected]; [email protected];
>>> [email protected]; [email protected]; [email protected];
>>> [email protected]; [email protected]; X.H. Bao
>>> <[email protected]>; Eagle Zhou <[email protected]>; Tao Jiang
>>> <[email protected]>; dl-linux-imx <[email protected]>;
>>> [email protected]; [email protected]; linux-
>>> [email protected]; [email protected]
>>> Subject: [EXT] Re: [PATCH v4 2/2] dt-bindings: media: imx-jpeg:
>>> Assign slot for imx jpeg encoder/decoder
>>>
>>> Caution: This is an external email. Please take care when clicking
>>> links or opening attachments. When in doubt, report the message using
>>> the 'Report this email' button
>>>
>>>
>>> On 26/09/2023 12:10, Ming Qian wrote:
>>>> There are total 4 slots available in the IP, and we only need to use
>>>> one slot in one os, assign a single slot, configure interrupt and
>>>> power domain only for 1 slot, not for the all 4 slots.
>>>>
>>>> Signed-off-by: Ming Qian <[email protected]>
>>>> ---
>>>> v4
>>>> - improve commit message
>>>> - don't make an ABI break
>>>> v3
>>>> - add vender prefix, change property slot to nxp,slot
>>>> - add type for property slot
>>>>
>>>> .../bindings/media/nxp,imx8-jpeg.yaml | 45 +++++++++----------
>>>> 1 file changed, 21 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..4bcfc815c894 100644
>>>> --- a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
>>>> +++ b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
>>>> @@ -32,19 +32,26 @@ 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 # VPUMIX
>>>> + maxItems: 2 # Wrapper and 1 slot
>>>> +
>>>> + nxp,slot:
>>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>>> + description:
>>>> + Integer number of slot index used. There are 4 slots available in the IP,
>>>> + and driver can use a certain slot, it should have an associated
>interrupt
>>>> + and power-domain. In theory, it supports 4 os or vm. If not specified,
>0
>>>> + is used by default.
>>>> + minimum: 0
>>>> + maximum: 3
>>>
>>> NAK, you still did not answer concerns why this is needed and justified.
>>> I keep asking and you keep ignoring...
>>>
>>> Best regards,
>>> Krzysztof
>>
>> Hi Krzysztof,
>>
>>> Nothing explains what is a slot and nothing explains why do you need this
>property.
>>
>> I thought I had answered that question, but seems you don't agree with that
>explanation.
>> Would the following description be any better?
>
>No, you didn't. I don't see any explanation in the commit msg. You just say
>you have "4 slots".
>
>Best regards,
>Krzysztof

Hi Krzysztof,

I check the our document again, it does not give a standard definition too. I'll explain it as I understand it. This IP includes a jpeg wrapper, the wrapper is working on descriptor based manner. It support up to 4 slots, each slots can have its own chained descriptors. The purpose is to share the jpeg engine across multiple VMS and os, It's a bit like vm partitioning. We just want to assign 1 slot to linux, Then, theoretically, it can support up to 3 VMS.
The slot index indicates which slot we choose, driver will use the slot index to configure. If not specified, 0 is used by default.
Currently driver only use the slot 0 by default, but it enable the power-domain and request irq to all the 4 slots. there has been no real negative impact, as we have no practical application to use other slots yet. But it does cause trouble when we want to use another slot in a vm.

Best regards,
Ming

2023-09-28 16:55:20

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [EXT] Re: [PATCH v4 2/2] dt-bindings: media: imx-jpeg: Assign slot for imx jpeg encoder/decoder

On Wed, Sep 27, 2023 at 09:10:52AM +0000, Ming Qian wrote:
> >From: Krzysztof Kozlowski <[email protected]>
> >
> >On 26/09/2023 12:10, Ming Qian wrote:
> >> There are total 4 slots available in the IP, and we only need to use
> >> one slot in one os, assign a single slot, configure interrupt and
> >> power domain only for 1 slot, not for the all 4 slots.
> >>
> >> Signed-off-by: Ming Qian <[email protected]>
> >> ---
> >> v4
> >> - improve commit message
> >> - don't make an ABI break
> >> v3
> >> - add vender prefix, change property slot to nxp,slot
> >> - add type for property slot
> >>
> >> .../bindings/media/nxp,imx8-jpeg.yaml | 45 +++++++++----------
> >> 1 file changed, 21 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..4bcfc815c894 100644
> >> --- a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
> >> +++ b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
> >> @@ -32,19 +32,26 @@ 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 # VPUMIX
> >> + maxItems: 2 # Wrapper and 1 slot
> >> +
> >> + nxp,slot:
> >> + $ref: /schemas/types.yaml#/definitions/uint32
> >> + description:
> >> + Integer number of slot index used. There are 4 slots available in the IP,
> >> + and driver can use a certain slot, it should have an associated interrupt
> >> + and power-domain. In theory, it supports 4 os or vm. If not specified, 0
> >> + is used by default.
> >> + minimum: 0
> >> + maximum: 3
> >
> >NAK, you still did not answer concerns why this is needed and justified.
> >I keep asking and you keep ignoring...
> >
> >Best regards,
> >Krzysztof
>
> Hi Krzysztof,
>
> > Nothing explains what is a slot and nothing explains why do you need this property.
>
> I thought I had answered that question, but seems you don't agree with that explanation.
> Would the following description be any better?
>
> The slot is the IP used to share the jpeg engine across multiple VMS
> and os, there are 4 slots available in the IP. We can use one slot in
> linux. The slot index indicates which slot we choose. If not
> specified, 0 is used by default.

If you have to modify the DT to update 'nxp,slot', then you can modify
the DT to 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.

As far as formatting your emails, talk to your NXP colleagues. They've
figured it out. If you've configured git-send-email successfully, then
using mutt should be possible. You can use 'b4' to get the thread mbox,
and read it in mutt.

Rob

2023-09-29 09:30:18

by Ming Qian

[permalink] [raw]
Subject: RE: [EXT] Re: [PATCH v4 2/2] dt-bindings: media: imx-jpeg: Assign slot for imx jpeg encoder/decoder

>From: Rob Herring <[email protected]>
>Sent: 2023??9??29?? 0:47
>To: Ming Qian <[email protected]>
>Cc: Krzysztof Kozlowski <[email protected]>; Mirela Rabulea (OSS)
><[email protected]>; [email protected];
>[email protected]; [email protected];
>[email protected]; [email protected]; [email protected];
>[email protected]; [email protected]; X.H. Bao
><[email protected]>; Eagle Zhou <[email protected]>; Tao Jiang
><[email protected]>; dl-linux-imx <[email protected]>;
>[email protected]; [email protected]; linux-
>[email protected]; [email protected]
>Subject: Re: [EXT] Re: [PATCH v4 2/2] dt-bindings: media: imx-jpeg: Assign slot
>for imx jpeg encoder/decoder
>
>Caution: This is an external email. Please take care when clicking links or
>opening attachments. When in doubt, report the message using the 'Report
>this email' button
>
>
>On Wed, Sep 27, 2023 at 09:10:52AM +0000, Ming Qian wrote:
>> >From: Krzysztof Kozlowski <[email protected]>
>> >
>> >On 26/09/2023 12:10, Ming Qian wrote:
>> >> There are total 4 slots available in the IP, and we only need to
>> >> use one slot in one os, assign a single slot, configure interrupt
>> >> and power domain only for 1 slot, not for the all 4 slots.
>> >>
>> >> Signed-off-by: Ming Qian <[email protected]>
>> >> ---
>> >> v4
>> >> - improve commit message
>> >> - don't make an ABI break
>> >> v3
>> >> - add vender prefix, change property slot to nxp,slot
>> >> - add type for property slot
>> >>
>> >> .../bindings/media/nxp,imx8-jpeg.yaml | 45 +++++++++----------
>> >> 1 file changed, 21 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..4bcfc815c894 100644
>> >> --- a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
>> >> +++ b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
>> >> @@ -32,19 +32,26 @@ 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 # VPUMIX
>> >> + maxItems: 2 # Wrapper and 1 slot
>> >> +
>> >> + nxp,slot:
>> >> + $ref: /schemas/types.yaml#/definitions/uint32
>> >> + description:
>> >> + Integer number of slot index used. There are 4 slots available in the
>IP,
>> >> + and driver can use a certain slot, it should have an associated
>interrupt
>> >> + and power-domain. In theory, it supports 4 os or vm. If not specified,
>0
>> >> + is used by default.
>> >> + minimum: 0
>> >> + maximum: 3
>> >
>> >NAK, you still did not answer concerns why this is needed and justified.
>> >I keep asking and you keep ignoring...
>> >
>> >Best regards,
>> >Krzysztof
>>
>> Hi Krzysztof,
>>
>> > Nothing explains what is a slot and nothing explains why do you need this
>property.
>>
>> I thought I had answered that question, but seems you don't agree with that
>explanation.
>> Would the following description be any better?
>>
>> The slot is the IP used to share the jpeg engine across multiple VMS
>> and os, there are 4 slots available in the IP. We can use one slot in
>> linux. The slot index indicates which slot we choose. If not
>> specified, 0 is used by default.
>

Hi Rob

>If you have to modify the DT to update 'nxp,slot', then you can modify the DT
>to 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.
>

That's what this patch is trying to do.
What should I do next?
Prepare the next version patch and modify the commit message after reaching an agreement?

>As far as formatting your emails, talk to your NXP colleagues. They've figured it
>out. If you've configured git-send-email successfully, then using mutt should
>be possible. You can use 'b4' to get the thread mbox, and read it in mutt.
>
>Rob

I'll talk with someone in nxp, and I'll try to use mutt.
Thanks for your advice.

Best regards,
Ming

2023-09-30 17:02:49

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [EXT] Re: [PATCH v4 2/2] dt-bindings: media: imx-jpeg: Assign slot for imx jpeg encoder/decoder

On 28/09/2023 11:42, Ming Qian wrote:
>> From: Krzysztof Kozlowski <[email protected]>
>> Sent: 2023年9月28日 13:07
>> To: Ming Qian <[email protected]>; Mirela Rabulea (OSS)
>> <[email protected]>; [email protected]; [email protected]
>> Cc: [email protected]; [email protected];
>> [email protected]; [email protected]; [email protected];
>> [email protected]; [email protected]; X.H. Bao
>> <[email protected]>; Eagle Zhou <[email protected]>; Tao Jiang
>> <[email protected]>; dl-linux-imx <[email protected]>;
>> [email protected]; [email protected]; linux-
>> [email protected]; [email protected]
>> Subject: Re: [EXT] Re: [PATCH v4 2/2] dt-bindings: media: imx-jpeg: Assign slot
>> for imx jpeg encoder/decoder
>>
>> Caution: This is an external email. Please take care when clicking links or
>> opening attachments. When in doubt, report the message using the 'Report
>> this email' button
>>
>>
>> On 27/09/2023 11:10, Ming Qian wrote:
>>>> From: Krzysztof Kozlowski <[email protected]>
>>>> Sent: 2023年9月27日 16:06
>>>> To: Ming Qian <[email protected]>; Mirela Rabulea (OSS)
>>>> <[email protected]>; [email protected];
>> [email protected]
>>>> Cc: [email protected]; [email protected];
>>>> [email protected]; [email protected]; [email protected];
>>>> [email protected]; [email protected]; X.H. Bao
>>>> <[email protected]>; Eagle Zhou <[email protected]>; Tao Jiang
>>>> <[email protected]>; dl-linux-imx <[email protected]>;
>>>> [email protected]; [email protected]; linux-
>>>> [email protected]; [email protected]
>>>> Subject: [EXT] Re: [PATCH v4 2/2] dt-bindings: media: imx-jpeg:
>>>> Assign slot for imx jpeg encoder/decoder
>>>>
>>>> Caution: This is an external email. Please take care when clicking
>>>> links or opening attachments. When in doubt, report the message using
>>>> the 'Report this email' button
>>>>
>>>>
>>>> On 26/09/2023 12:10, Ming Qian wrote:
>>>>> There are total 4 slots available in the IP, and we only need to use
>>>>> one slot in one os, assign a single slot, configure interrupt and
>>>>> power domain only for 1 slot, not for the all 4 slots.
>>>>>
>>>>> Signed-off-by: Ming Qian <[email protected]>
>>>>> ---
>>>>> v4
>>>>> - improve commit message
>>>>> - don't make an ABI break
>>>>> v3
>>>>> - add vender prefix, change property slot to nxp,slot
>>>>> - add type for property slot
>>>>>
>>>>> .../bindings/media/nxp,imx8-jpeg.yaml | 45 +++++++++----------
>>>>> 1 file changed, 21 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..4bcfc815c894 100644
>>>>> --- a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
>>>>> +++ b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
>>>>> @@ -32,19 +32,26 @@ 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 # VPUMIX
>>>>> + maxItems: 2 # Wrapper and 1 slot
>>>>> +
>>>>> + nxp,slot:
>>>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>>>> + description:
>>>>> + Integer number of slot index used. There are 4 slots available in the IP,
>>>>> + and driver can use a certain slot, it should have an associated
>> interrupt
>>>>> + and power-domain. In theory, it supports 4 os or vm. If not specified,
>> 0
>>>>> + is used by default.
>>>>> + minimum: 0
>>>>> + maximum: 3
>>>>
>>>> NAK, you still did not answer concerns why this is needed and justified.
>>>> I keep asking and you keep ignoring...
>>>>
>>>> Best regards,
>>>> Krzysztof
>>>
>>> Hi Krzysztof,
>>>
>>>> Nothing explains what is a slot and nothing explains why do you need this
>> property.
>>>
>>> I thought I had answered that question, but seems you don't agree with that
>> explanation.
>>> Would the following description be any better?
>>
>> No, you didn't. I don't see any explanation in the commit msg. You just say
>> you have "4 slots".
>>
>> Best regards,
>> Krzysztof
>
> Hi Krzysztof,
>
> I check the our document again, it does not give a standard definition too. I'll explain it as I understand it. This IP includes a jpeg wrapper, the wrapper is working on descriptor based manner. It support up to 4 slots, each slots can have its own chained descriptors. The purpose is to share the jpeg engine across multiple VMS and os, It's a bit like vm partitioning. We just want to assign 1 slot to linux, Then, theoretically, it can support up to 3 VMS.
> The slot index indicates which slot we choose, driver will use the slot index to configure. If not specified, 0 is used by default.
> Currently driver only use the slot 0 by default, but it enable the power-domain and request irq to all the 4 slots. there has been no real negative impact, as we have no practical application to use other slots yet. But it does cause trouble when we want to use another slot in a vm.

Which parts of hardware are common and which are shared?

Best regards,
Krzysztof

2023-10-02 04:51:57

by Ming Qian

[permalink] [raw]
Subject: RE: [EXT] Re: [PATCH v4 2/2] dt-bindings: media: imx-jpeg: Assign slot for imx jpeg encoder/decoder

>From: Krzysztof Kozlowski <[email protected]>
>Sent: 2023年9月30日 22:40
>To: Ming Qian <[email protected]>; Mirela Rabulea (OSS)
><[email protected]>; [email protected]; [email protected]
>Cc: [email protected]; [email protected];
>[email protected]; [email protected]; [email protected];
>[email protected]; [email protected]; X.H. Bao
><[email protected]>; Eagle Zhou <[email protected]>; Tao Jiang
><[email protected]>; dl-linux-imx <[email protected]>;
>[email protected]; [email protected]; linux-
>[email protected]; [email protected]
>Subject: Re: [EXT] Re: [PATCH v4 2/2] dt-bindings: media: imx-jpeg: Assign slot
>for imx jpeg encoder/decoder
>
>Caution: This is an external email. Please take care when clicking links or
>opening attachments. When in doubt, report the message using the 'Report
>this email' button
>
>
>On 28/09/2023 11:42, Ming Qian wrote:
>>> From: Krzysztof Kozlowski <[email protected]>
>>> Sent: 2023年9月28日 13:07
>>> To: Ming Qian <[email protected]>; Mirela Rabulea (OSS)
>>> <[email protected]>; [email protected];
>[email protected]
>>> Cc: [email protected]; [email protected];
>>> [email protected]; [email protected]; [email protected];
>>> [email protected]; [email protected]; X.H. Bao
>>> <[email protected]>; Eagle Zhou <[email protected]>; Tao Jiang
>>> <[email protected]>; dl-linux-imx <[email protected]>;
>>> [email protected]; [email protected]; linux-
>>> [email protected]; [email protected]
>>> Subject: Re: [EXT] Re: [PATCH v4 2/2] dt-bindings: media: imx-jpeg:
>>> Assign slot for imx jpeg encoder/decoder
>>>
>>> Caution: This is an external email. Please take care when clicking
>>> links or opening attachments. When in doubt, report the message using
>>> the 'Report this email' button
>>>
>>>
>>> On 27/09/2023 11:10, Ming Qian wrote:
>>>>> From: Krzysztof Kozlowski <[email protected]>
>>>>> Sent: 2023年9月27日 16:06
>>>>> To: Ming Qian <[email protected]>; Mirela Rabulea (OSS)
>>>>> <[email protected]>; [email protected];
>>> [email protected]
>>>>> Cc: [email protected]; [email protected];
>>>>> [email protected]; [email protected];
>>>>> [email protected]; [email protected]; [email protected];
>>>>> X.H. Bao <[email protected]>; Eagle Zhou <[email protected]>;
>>>>> Tao Jiang <[email protected]>; dl-linux-imx <[email protected]>;
>>>>> [email protected]; [email protected]; linux-
>>>>> [email protected]; [email protected]
>>>>> Subject: [EXT] Re: [PATCH v4 2/2] dt-bindings: media: imx-jpeg:
>>>>> Assign slot for imx jpeg encoder/decoder
>>>>>
>>>>> Caution: This is an external email. Please take care when clicking
>>>>> links or opening attachments. When in doubt, report the message
>>>>> using the 'Report this email' button
>>>>>
>>>>>
>>>>> On 26/09/2023 12:10, Ming Qian wrote:
>>>>>> There are total 4 slots available in the IP, and we only need to
>>>>>> use one slot in one os, assign a single slot, configure interrupt
>>>>>> and power domain only for 1 slot, not for the all 4 slots.
>>>>>>
>>>>>> Signed-off-by: Ming Qian <[email protected]>
>>>>>> ---
>>>>>> v4
>>>>>> - improve commit message
>>>>>> - don't make an ABI break
>>>>>> v3
>>>>>> - add vender prefix, change property slot to nxp,slot
>>>>>> - add type for property slot
>>>>>>
>>>>>> .../bindings/media/nxp,imx8-jpeg.yaml | 45 +++++++++----------
>>>>>> 1 file changed, 21 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..4bcfc815c894 100644
>>>>>> --- a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
>>>>>> +++ b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
>>>>>> @@ -32,19 +32,26 @@ 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 # VPUMIX
>>>>>> + maxItems: 2 # Wrapper and 1 slot
>>>>>> +
>>>>>> + nxp,slot:
>>>>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>>>>> + description:
>>>>>> + Integer number of slot index used. There are 4 slots available in the
>IP,
>>>>>> + and driver can use a certain slot, it should have an
>>>>>> + associated
>>> interrupt
>>>>>> + and power-domain. In theory, it supports 4 os or vm. If not
>>>>>> + specified,
>>> 0
>>>>>> + is used by default.
>>>>>> + minimum: 0
>>>>>> + maximum: 3
>>>>>
>>>>> NAK, you still did not answer concerns why this is needed and justified.
>>>>> I keep asking and you keep ignoring...
>>>>>
>>>>> Best regards,
>>>>> Krzysztof
>>>>
>>>> Hi Krzysztof,
>>>>
>>>>> Nothing explains what is a slot and nothing explains why do you
>>>>> need this
>>> property.
>>>>
>>>> I thought I had answered that question, but seems you don't agree
>>>> with that
>>> explanation.
>>>> Would the following description be any better?
>>>
>>> No, you didn't. I don't see any explanation in the commit msg. You
>>> just say you have "4 slots".
>>>
>>> Best regards,
>>> Krzysztof
>>
>> Hi Krzysztof,
>>
>> I check the our document again, it does not give a standard definition too. I'll
>explain it as I understand it. This IP includes a jpeg wrapper, the wrapper is
>working on descriptor based manner. It support up to 4 slots, each slots can
>have its own chained descriptors. The purpose is to share the jpeg engine
>across multiple VMS and os, It's a bit like vm partitioning. We just want to
>assign 1 slot to linux, Then, theoretically, it can support up to 3 VMS.
>> The slot index indicates which slot we choose, driver will use the slot index
>to configure. If not specified, 0 is used by default.
>> Currently driver only use the slot 0 by default, but it enable the power-
>domain and request irq to all the 4 slots. there has been no real negative
>impact, as we have no practical application to use other slots yet. But it does
>cause trouble when we want to use another slot in a vm.
>
>Which parts of hardware are common and which are shared?
>
>Best regards,
>Krzysztof

Hi Krzysztof,
The jpeg codec engine are shared, but we don't configure the engine directly.
We can fill some descriptors to encode or decode one jpeg frame. 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.

Best regards,
MIng