Re: [PATCH 03/10] dt-bindings: display: imx: Add i.MX8qxp Display Controller pixel engine

[Liu Ying <victor.liu@nxp.com>]

On 07/08/2024, Krzysztof Kozlowski wrote:
> On 08/07/2024 08:47, Liu Ying wrote:
>> On 07/07/2024, Krzysztof Kozlowski wrote:
>>> On 05/07/2024 11:09, Liu Ying wrote:
>>>> i.MX8qxp Display Controller pixel engine consists of all processing units
>>>> that operate in the AXI bus clock domain.  Command sequencer and interrupt
>>>> controller of the Display Controller work with AXI bus clock, but they are
>>>> not in pixel engine.
>>>>
>>>> Signed-off-by: Liu Ying <victor.liu@nxp.com>
>>>> ---
>>>
>>>
>>>> +
>>>> +        extdst@56180a40 {
>>>> +            compatible = "fsl,imx8qxp-dc-extdst";
>>>> +            reg = <0x56180a40 0x7>, <0x56186000 0x400>;
>>>> +            reg-names = "pec", "cfg";
>>>> +            interrupt-parent = <&dc0_intc>;
>>>> +            interrupts = <12>, <13>, <14>;
>>>> +            interrupt-names = "shdload", "framecomplete", "seqcomplete";
>>>> +            fsl,dc-ed-id = <5>;
>>>> +        };
>>>> +
>>>> +        fetchwarp@56180a60 {
>>>> +            compatible = "fsl,imx8qxp-dc-fetchwarp";
>>>> +            reg = <0x56180a60 0x4>, <0x56186400 0x400>;
>>>
>>> Aha, one word for address range.
>>
>> Sorry, I don't get your idea here.
> 
> How many words are in the first IO address range?
> 
> One.

My bad, the address range should be corrected to
reg = <0x56180a60 0x10>, <0x56186400 0x190>;

Though only 4 words in the first IO address range.

> 
> That is not a separate device.

Each processing unit in pixel engine has two IO address
ranges - "pec" and "cfg".

"pec" stands for Pixel Engine Configuration where pixel
engine related stuff is controlled by 4 registers(4 words
in all) - LockUnlock, LockStatus, Dynamic and Status registers.
The first two provide lock and/or privilege protection to this
address block.  The last two control/report connection from/to
other processing units. 

"cfg" IO address range controls processing unit specific
functions.

As each processing unit has specific functions, it's not
unacceptable to take it as a separate device with it's own
IO address ranges.  However, frankly speaking, I'm ok with
the idea of not splitting the main display controller into
multiple devices, but taking it as one single device.

> 
>>
>>>
>>> Sorry, these are not separate devices.
>>
>> Hmm, again, Maxime suggested to use separate devices.
> 
> To some level you can create separate devices, but for one register?

There are two IO address ranges, so at least a few registers.
Especially, the "cfg" range kind of hints that they could be
taken as separate devices, as it controls specific functions. 

> 
> Best regards,
> Krzysztof
> 
> 

-- 
Regards,
Liu Ying