Re: [PATCH v2 01/12] dt-bindings: media: qcom,venus: Add context bank subnodes to common schema

[Dmitry Baryshkov]

On Wed, Aug 05, 2026 at 12:37:06PM +0200, Krzysztof Kozlowski wrote:
> On 05/08/2026 09:21, Krzysztof Kozlowski wrote:
> > On Fri, Jul 31, 2026 at 11:52:16PM +0530, Vikash Garodia wrote:
> >> The VPU issues DMA through several SMMU streams, and the hardware does
> >> not give every stream the same addressable range. The non-pixel stream
> >> is restricted to use 0-600MB of IOVA space, while the pixel stream can
> >> address the full range:
> >>     +-----------------------------------------------------------+
> >>     | non-pixel stream addressable range (600 MB - 3.5 GB)      |
> >>     | 0x25800000 - 0xe0000000                                   |
> >>     +-----------------------------------------------------------+
> >>     | pixel stream addressable range (0 - 3.5 GB)               |
> >>     | 0x00000000 - 0xe0000000                                   |
> >>     +-----------------------------------------------------------+
> >> A single "iommus" property on the video-codec node puts every stream in
> >> one IOMMU domain sharing one IOVA allocator, so nothing keeps a
> >> non-pixel buffer inside the low 600 MB. Once an allocation lands below
> >> that boundary the hardware faults, which shows up as unhandled SMMU page
> >> faults and spontaneous reboots:
> >> https://gitlab.freedesktop.org/drm/msm/-/work_items/100
> >>
> >> Describe each stream as its own context bank subnode instead, so that
> >> each can be associated with the IOVA range its stream can actually
> >> reach. This limitation applies to every VPU generation, so add the
> >> subnodes to the common schema rather than to each SoC schema
> >> individually. "video-firmware" moves here from qcom,sc7180-venus.yaml
> >> for the same reason; it is the same kind of node and was already
> >> duplicated per-SoC.
> >> Adding the subnodes requires two supporting properties on the parent
> >> video-codec node:
> >> - '#address-cells' and '#size-cells', both fixed at 2. These do not
> >>    describe registers on the codec node. They set the cell widths
> >>    used when a reserved-memory node names one of these subnodes in an
> >>    "iommu-addresses" entry: of_translate_dma_region() reads the
> >>    address/size cell counts from the parent of the phandle target, not
> >>    from the reserved-memory node. Pinning both to 2 lets a subnode be
> >>    referenced with a full 64-bit IOVA base and length, and keeps the
> >>    encoding identical across SoCs, whose buses vary between 1 and 2
> >>    cells.
> >> - "dma-ranges", empty "dma-ranges" states the intended translation:
> >>    the subnode DMA address space maps 1:1 into the parent's, so an IOVA
> >>    reservation written against a subnode needs no offset applied.
> >>    of_translate_one() treats an empty "dma-ranges" as exactly that
> >>    identity mapping.
> >>
> >> The parent's "iommus" is kept as an alternative via "oneOf", so
> >> platforms that have not been converted to subnodes still validate. New
> >> platforms should use the subnode form.
> >>
> >> Fixes: 41661853ae8e ("arm64: dts: qcom: sm8550: add iris DT node")
> >> Cc: stable@xxxxxxxxxxxxxxx
> >> Tested-by: Daniel J Blueman <daniel@xxxxxxxxx>
> > 
> > Not a valid tag.
> > 
> > 
> > 
> > 
> >> Signed-off-by: Vikash Garodia <vikash.garodia@xxxxxxxxxxxxxxxx>
> >> ---
> >>  .../bindings/media/qcom,sc7180-venus.yaml          | 15 -------
> >>  .../bindings/media/qcom,venus-common.yaml          | 51 ++++++++++++++++++++++
> >>  2 files changed, 51 insertions(+), 15 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/media/qcom,sc7180-venus.yaml b/Documentation/devicetree/bindings/media/qcom,sc7180-venus.yaml
> >> index b21bed314848480b82153e49602f0b19e08e7335..bfd8b1ad473128c974bce84639cb0aff59d8c2cc 100644
> >> --- a/Documentation/devicetree/bindings/media/qcom,sc7180-venus.yaml
> >> +++ b/Documentation/devicetree/bindings/media/qcom,sc7180-venus.yaml
> >> @@ -91,21 +91,6 @@ properties:
> >>      deprecated: true
> >>      additionalProperties: false
> >>  
> >> -  video-firmware:
> >> -    type: object
> >> -    additionalProperties: false
> >> -
> >> -    description: |
> >> -      Firmware subnode is needed when the platform does not
> >> -      have TrustZone.
> >> -
> >> -    properties:
> >> -      iommus:
> >> -        maxItems: 1
> >> -
> >> -    required:
> >> -      - iommus
> >> -
> >>  required:
> >>    - compatible
> >>    - power-domain-names
> >> diff --git a/Documentation/devicetree/bindings/media/qcom,venus-common.yaml b/Documentation/devicetree/bindings/media/qcom,venus-common.yaml
> >> index 59a3fde846d2196ab1e4588eb396012ba6860712..ccae41bed757bf3d9518cd4afe4565f043b45166 100644
> >> --- a/Documentation/devicetree/bindings/media/qcom,venus-common.yaml
> >> +++ b/Documentation/devicetree/bindings/media/qcom,venus-common.yaml
> >> @@ -47,6 +47,50 @@ properties:
> >>      minItems: 1
> >>      maxItems: 4
> >>  
> >> +  dma-ranges: true
> >> +
> >> +  '#address-cells':
> >> +    const: 2
> >> +
> >> +  '#size-cells':
> >> +    const: 2
> > 
> > Above do not look valid. You do not describe the addressing of some
> > other device node (not a child) here. You describe that addressing in
> > that other device node's parent.
> 
> As we talked offline, these are actually needed for dma-ranges, but I am
> honestly confused whether we are representing this correct.

Vikash, you have been asked several times. Please keep the discussions
on the mailing list. Now there was a conversation which you can't
reference to anybody else having the same question.

> dma-ranges tell how this bus - so venus/iris - performs DMA translation
> in respective to parent. Address/size-cells are obviously also needed if
> this is a bus with addressing.
> 
> But there are no children with addressing, thus what sort of bus would
> it be?
> 
> It looks to me that having here both:
> 1. dma-ranges + address/size-cells
> 2. children without bus addressing
> is some sort of abuse of the DT syntax. It is allowed, but does not
> really represent hardware.
> 
> IOW, dma-ranges alone feels okay, although unusual, and it states proper
> DMA translation for this bus. If you add address/size-cells, it means
> this bus HAS addressing and thus YOU MUST use addressing.
> 
> If my understanding is correct, then solution would be to add addressing
> to the children (so unit address and "reg" property) or drop
> address/size-cells as Rob pointed out. [1]

Doesn't dma-ranges require address/size cells? In the end, how can you
specify the DMA address if the device doesn't have addressing at all (or
MMIO-style addressing)?

> 
> If kernel disagrees with the latter, then kernel is wrong, IMO.
> 
> [1] https://lore.kernel.org/all/20260716165434.GA290489-robh@xxxxxxxxxx/
> 
> 
> Best regards,
> Krzysztof

-- 
With best wishes
Dmitry