Re: [PATCH V3 4/5] dt-bindings: media: ti: vpe: Add support for Video Input Port

[Krzysztof Kozlowski]

On 13/10/2025 08:21, Yemike Abhilash Chandra wrote:
>>> +examples:
>>> +  - |
>>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>>> +    #include <dt-bindings/interrupt-controller/irq.h>
>>> +
>>> +    vip1: video@48970000 {
>>> +        compatible = "ti,dra7-vip";
>>> +        reg = <0x48970000 0x114>,
>>> +              <0x48975500 0xD8>,
>>> +              <0x48975700 0x18>,
>>> +              <0x48975800 0x80>,
>>> +              <0x48975a00 0xD8>,
>>> +              <0x48975c00 0x18>,
>>> +              <0x48975d00 0x80>,
>>
>> Are you really, REALLY sure these are separate address spaces? Some
>> people thing that gaps means address space finishes...
>>
>>
>> How does your datasheet define these?
>>
> 
> These are not separate address spaces. The datasheet defines them as a
> single address region: VIP1 (0x4897_0000 –> 0x4897_FFFF, 64 KiB).
> 
> We created three common libraries sc.c (scalar), csc.c (color space 
> converter),
> and vpdma.c, which are used by both VPE and VIP drivers. The helper 
> functions
> in these libraries were originally written in a way that assumes sc0, csc0,
> sc1, and csc1 are separate address spaces. Since VPE has already been
> upstreamed using this approach. I have tried to use the same kind of 
> approach.
> But I now understand that, this might not be correct to define these as 
> separate
> address spaces.


Your previous driver choices should not matter. You are describing here
hardware, not drivers. Especially not previous drivers.

> 
> One solution would be to rewrite these helpers and update both VPE and
> VIP accordingly, but changing these helpers now may risk breaking backward
> compatibility for VPE.

Don't care. Describe properly hardware.

> 
> Alternatively, we could make the VIP driver standalone by avoiding the 
> use of these
> helpers. I was able to achieve this for csc.c and sc.c, but for vpdma.c, 
> I had to
> export a specific function from vpdma.c, since the VIP driver no longer 
> relies on the
> helpers provided by vpdma.c (In the previous approach the helper would 
> call this function)

Again, describe hardware, not drivers.


Best regards,
Krzysztof