Devicetree
 help / color / mirror / Atom feed
From: Krzysztof Kozlowski <krzk@kernel.org>
To: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
Cc: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>,
	Abhinav Kumar <abhinav.kumar@linux.dev>,
	Bryan O'Donoghue <bod@kernel.org>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Bjorn Andersson <andersson@kernel.org>,
	Konrad Dybcio <konradybcio@kernel.org>,
	Stanimir Varbanov <stanimir.varbanov@linaro.org>,
	Neil Armstrong <neil.armstrong@linaro.org>,
	Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>,
	Bryan O'Donoghue <bryan.odonoghue@linaro.org>,
	Stephan Gerhold <stephan.gerhold@linaro.org>,
	linux-media@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>,
	stable@vger.kernel.org, Daniel J Blueman <daniel@quora.org>
Subject: Re: [PATCH v2 01/12] dt-bindings: media: qcom,venus: Add context bank subnodes to common schema
Date: Wed, 5 Aug 2026 12:37:06 +0200	[thread overview]
Message-ID: <7a546bc6-a2b4-457d-ac27-05fc1cfe17f5@kernel.org> (raw)
In-Reply-To: <20260805-dark-voracious-jacamar-6aae4c@quoll>

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@vger.kernel.org
>> Tested-by: Daniel J Blueman <daniel@quora.org>
> 
> Not a valid tag.
> 
> 
> 
> 
>> Signed-off-by: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
>> ---
>>  .../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.

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]

If kernel disagrees with the latter, then kernel is wrong, IMO.

[1] https://lore.kernel.org/all/20260716165434.GA290489-robh@kernel.org/


Best regards,
Krzysztof

  reply	other threads:[~2026-08-05 10:37 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-31 18:22 [PATCH v2 00/12] media: iris: Migrate iommus to iris sub nodes Vikash Garodia
2026-07-31 18:22 ` [PATCH v2 01/12] dt-bindings: media: qcom,venus: Add context bank subnodes to common schema Vikash Garodia
2026-07-31 18:36   ` sashiko-bot
2026-08-01 10:06   ` Bryan O'Donoghue
2026-08-06  5:21     ` Vikash Garodia
2026-08-06  7:54       ` Bryan O'Donoghue
2026-08-06  7:57         ` Vikash Garodia
2026-08-06  9:40           ` Bryan O'Donoghue
2026-08-06 10:07             ` Bryan O'Donoghue
2026-08-06 10:14               ` Krzysztof Kozlowski
2026-08-06 10:24                 ` Bryan O'Donoghue
2026-08-06 10:32                   ` Krzysztof Kozlowski
2026-08-06 10:50                     ` Bryan O'Donoghue
2026-08-06 11:14                       ` Dmitry Baryshkov
2026-08-06 11:19                         ` Bryan O'Donoghue
2026-08-06 12:20                         ` Vikash Garodia
2026-08-05  7:21   ` Krzysztof Kozlowski
2026-08-05 10:37     ` Krzysztof Kozlowski [this message]
2026-08-06  8:46       ` Dmitry Baryshkov
2026-08-06  9:11         ` Krzysztof Kozlowski
2026-08-06  9:20           ` Krzysztof Kozlowski
2026-08-06  9:22           ` Dmitry Baryshkov
2026-08-06  9:36             ` Krzysztof Kozlowski
2026-08-06 10:35               ` Dmitry Baryshkov
2026-08-06 11:56                 ` Vikash Garodia
2026-08-06 12:10                   ` Dmitry Baryshkov
2026-08-06  8:49   ` Dmitry Baryshkov
2026-08-06  9:52     ` Vikash Garodia
2026-08-06 10:38       ` Dmitry Baryshkov
2026-07-31 18:22 ` [PATCH v2 02/12] dt-bindings: media: qcom,sm8550-iris: Add context bank subnodes Vikash Garodia
2026-07-31 18:31   ` sashiko-bot
2026-07-31 20:36   ` Rob Herring (Arm)
2026-08-06  8:51     ` Dmitry Baryshkov
2026-08-05  7:21   ` Krzysztof Kozlowski
2026-07-31 18:22 ` [PATCH v2 03/12] media: iris: Add platform hooks for context bank device setup Vikash Garodia
2026-07-31 18:36   ` sashiko-bot
2026-08-06  8:53   ` Dmitry Baryshkov
2026-07-31 18:22 ` [PATCH v2 04/12] media: iris: Add helper to create a context bank device Vikash Garodia
2026-08-06  8:57   ` Dmitry Baryshkov
2026-08-06 10:08     ` Vikash Garodia
2026-08-06 10:18       ` Krzysztof Kozlowski
2026-07-31 18:22 ` [PATCH v2 05/12] media: iris: Route buffers to the matching " Vikash Garodia
2026-07-31 18:57   ` sashiko-bot
2026-08-06  9:03   ` Dmitry Baryshkov
2026-07-31 18:22 ` [PATCH v2 06/12] media: iris: Skip DMA mask setup when the core device has no IOMMU Vikash Garodia
2026-07-31 18:43   ` sashiko-bot
2026-07-31 18:22 ` [PATCH v2 07/12] media: iris: Create pixel and non-pixel context banks on VPU3x Vikash Garodia
2026-07-31 18:41   ` sashiko-bot
2026-08-06  9:07   ` Dmitry Baryshkov
2026-08-06 10:06     ` Vikash Garodia
2026-08-06 10:53       ` Dmitry Baryshkov
2026-07-31 18:22 ` [PATCH v2 08/12] arm64: dts: qcom: hamoa: Add Iris context bank subnodes Vikash Garodia
2026-07-31 18:22 ` [PATCH v2 09/12] arm64: dts: qcom: sm8550: " Vikash Garodia
2026-07-31 18:38   ` sashiko-bot
2026-07-31 18:22 ` [PATCH v2 10/12] arm64: dts: qcom: lemans: " Vikash Garodia
2026-07-31 18:22 ` [PATCH v2 11/12] arm64: dts: qcom: monaco: " Vikash Garodia
2026-07-31 18:22 ` [PATCH v2 12/12] arm64: dts: qcom: sm8650: " Vikash Garodia

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7a546bc6-a2b4-457d-ac27-05fc1cfe17f5@kernel.org \
    --to=krzk@kernel.org \
    --cc=abhinav.kumar@linux.dev \
    --cc=andersson@kernel.org \
    --cc=bod@kernel.org \
    --cc=bryan.odonoghue@linaro.org \
    --cc=conor+dt@kernel.org \
    --cc=daniel@quora.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dikshita.agarwal@oss.qualcomm.com \
    --cc=dmitry.baryshkov@oss.qualcomm.com \
    --cc=konrad.dybcio@oss.qualcomm.com \
    --cc=konradybcio@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=neil.armstrong@linaro.org \
    --cc=robh@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=stanimir.varbanov@linaro.org \
    --cc=stephan.gerhold@linaro.org \
    --cc=vikash.garodia@oss.qualcomm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox