[PATCH 1/2] dt-bindings: display/msm/gpu: Narrow reg and reg-names for Adreno 610.0 and alike

[PATCH 1/2] dt-bindings: display/msm/gpu: Narrow reg and reg-names for Adreno 610.0 and alike

[Krzysztof Kozlowski]

DTS files for qcom,adreno-610.0 and qcom,adreno-07000200 contain only one
"reg" entry, not two, and the binding defines the second entry in
"reg-names" differently than top-level part, so just simplify it and
narrow to only one entry.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
 Documentation/devicetree/bindings/display/msm/gpu.yaml | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/msm/gpu.yaml b/Documentation/devicetree/bindings/display/msm/gpu.yaml
index 826aafdcc20b..1ae5faf2c867 100644
--- a/Documentation/devicetree/bindings/display/msm/gpu.yaml
+++ b/Documentation/devicetree/bindings/display/msm/gpu.yaml
@@ -378,11 +378,12 @@ allOf:
             - const: xo
               description: GPUCC clocksource clock
 
+        reg:
+          maxItems: 1
+
         reg-names:
-          minItems: 1
           items:
             - const: kgsl_3d0_reg_memory
-            - const: cx_dbgc
 
       required:
         - clocks
-- 
2.51.0

[PATCH 2/2] dt-bindings: display/msm/gpu: Drop redundant reg-names in one if:then:

[Krzysztof Kozlowski]

Top-level reg-names defines already proper order for "reg-names" with
minItems: 1, so no need to repeat it again in one of "if:then:" cases.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
 Documentation/devicetree/bindings/display/msm/gpu.yaml | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/msm/gpu.yaml b/Documentation/devicetree/bindings/display/msm/gpu.yaml
index 1ae5faf2c867..2e916309abce 100644
--- a/Documentation/devicetree/bindings/display/msm/gpu.yaml
+++ b/Documentation/devicetree/bindings/display/msm/gpu.yaml
@@ -402,13 +402,6 @@ allOf:
           clocks: false
           clock-names: false
 
-          reg-names:
-            minItems: 1
-            items:
-              - const: kgsl_3d0_reg_memory
-              - const: cx_mem
-              - const: cx_dbgc
-
 examples:
   - |
 
-- 
2.51.0

Re: [PATCH 1/2] dt-bindings: display/msm/gpu: Narrow reg and reg-names for Adreno 610.0 and alike

[Rob Clark]

On Sat, Dec 27, 2025 at 3:05 AM Krzysztof Kozlowski
<krzysztof.kozlowski@oss.qualcomm.com> wrote:
>
> DTS files for qcom,adreno-610.0 and qcom,adreno-07000200 contain only one
> "reg" entry, not two, and the binding defines the second entry in
> "reg-names" differently than top-level part, so just simplify it and
> narrow to only one entry.

I'll defer to Akhil about whether this is actually needed (vs just
incomplete gpu devcoredump support for certain GPUs).  In general
cx_dbgc is needed to capture state for gpu devcoredump state
snapshots, but not directly used in normal operations.  It seems
similar to the situation with mapping gpucc as part of gmu, ie. not
something the CPU normally deals with directly, but necessary to
capture crash state.

BR,
-R

> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> ---
>  Documentation/devicetree/bindings/display/msm/gpu.yaml | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/display/msm/gpu.yaml b/Documentation/devicetree/bindings/display/msm/gpu.yaml
> index 826aafdcc20b..1ae5faf2c867 100644
> --- a/Documentation/devicetree/bindings/display/msm/gpu.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/gpu.yaml
> @@ -378,11 +378,12 @@ allOf:
>              - const: xo
>                description: GPUCC clocksource clock
>
> +        reg:
> +          maxItems: 1
> +
>          reg-names:
> -          minItems: 1
>            items:
>              - const: kgsl_3d0_reg_memory
> -            - const: cx_dbgc
>
>        required:
>          - clocks
> --
> 2.51.0
>

Re: [PATCH 1/2] dt-bindings: display/msm/gpu: Narrow reg and reg-names for Adreno 610.0 and alike

[Krzysztof Kozlowski]

On 27/12/2025 23:01, Rob Clark wrote:
> On Sat, Dec 27, 2025 at 3:05 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@oss.qualcomm.com> wrote:
>>
>> DTS files for qcom,adreno-610.0 and qcom,adreno-07000200 contain only one
>> "reg" entry, not two, and the binding defines the second entry in
>> "reg-names" differently than top-level part, so just simplify it and
>> narrow to only one entry.
> 
> I'll defer to Akhil about whether this is actually needed (vs just
> incomplete gpu devcoredump support for certain GPUs).  In general
> cx_dbgc is needed to capture state for gpu devcoredump state
> snapshots, but not directly used in normal operations.  It seems
> similar to the situation with mapping gpucc as part of gmu, ie. not
> something the CPU normally deals with directly, but necessary to
> capture crash state.

I don't get why binding was added with cx_dbgc, but DTS not. Neither
binding nor DTS depends on actual usage, so I assume someone
intentionally did not want DTS to contain cx_dbgc and binding should
follow. Otherwise we should make the DTS complete and make the binding
strict (leading to warnings if DTS is not updated).

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: display/msm/gpu: Narrow reg and reg-names for Adreno 610.0 and alike

[Rob Clark]

On Sat, Dec 27, 2025 at 11:56 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 27/12/2025 23:01, Rob Clark wrote:
> > On Sat, Dec 27, 2025 at 3:05 AM Krzysztof Kozlowski
> > <krzysztof.kozlowski@oss.qualcomm.com> wrote:
> >>
> >> DTS files for qcom,adreno-610.0 and qcom,adreno-07000200 contain only one
> >> "reg" entry, not two, and the binding defines the second entry in
> >> "reg-names" differently than top-level part, so just simplify it and
> >> narrow to only one entry.
> >
> > I'll defer to Akhil about whether this is actually needed (vs just
> > incomplete gpu devcoredump support for certain GPUs).  In general
> > cx_dbgc is needed to capture state for gpu devcoredump state
> > snapshots, but not directly used in normal operations.  It seems
> > similar to the situation with mapping gpucc as part of gmu, ie. not
> > something the CPU normally deals with directly, but necessary to
> > capture crash state.
>
> I don't get why binding was added with cx_dbgc, but DTS not. Neither
> binding nor DTS depends on actual usage, so I assume someone
> intentionally did not want DTS to contain cx_dbgc and binding should
> follow. Otherwise we should make the DTS complete and make the binding
> strict (leading to warnings if DTS is not updated).

I'm not sure about the history.. but I can say that cx_dbgc is only
used for gpu state snapshot / devcoredump.  So it would be easy to not
notice if it were missing.

We have a similar slightly ugly thing where gpucc is included in the
gmu map.. only for devcoredump.  Maybe we need a different way to
handle these things that are only mapped for state capture?

BR,
-R

Re: [PATCH 1/2] dt-bindings: display/msm/gpu: Narrow reg and reg-names for Adreno 610.0 and alike

[Krzysztof Kozlowski]

On 28/12/2025 15:59, Rob Clark wrote:
> On Sat, Dec 27, 2025 at 11:56 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On 27/12/2025 23:01, Rob Clark wrote:
>>> On Sat, Dec 27, 2025 at 3:05 AM Krzysztof Kozlowski
>>> <krzysztof.kozlowski@oss.qualcomm.com> wrote:
>>>>
>>>> DTS files for qcom,adreno-610.0 and qcom,adreno-07000200 contain only one
>>>> "reg" entry, not two, and the binding defines the second entry in
>>>> "reg-names" differently than top-level part, so just simplify it and
>>>> narrow to only one entry.
>>>
>>> I'll defer to Akhil about whether this is actually needed (vs just
>>> incomplete gpu devcoredump support for certain GPUs).  In general
>>> cx_dbgc is needed to capture state for gpu devcoredump state
>>> snapshots, but not directly used in normal operations.  It seems
>>> similar to the situation with mapping gpucc as part of gmu, ie. not
>>> something the CPU normally deals with directly, but necessary to
>>> capture crash state.
>>
>> I don't get why binding was added with cx_dbgc, but DTS not. Neither
>> binding nor DTS depends on actual usage, so I assume someone
>> intentionally did not want DTS to contain cx_dbgc and binding should
>> follow. Otherwise we should make the DTS complete and make the binding
>> strict (leading to warnings if DTS is not updated).
> 
> I'm not sure about the history.. but I can say that cx_dbgc is only
> used for gpu state snapshot / devcoredump.  So it would be easy to not
> notice if it were missing.
> 
> We have a similar slightly ugly thing where gpucc is included in the
> gmu map.. only for devcoredump.  Maybe we need a different way to
> handle these things that are only mapped for state capture?

No. Either hardware has it or not. If hardware has it, then both DTS and
binding should have it. If people decided that DTS should not have it
(for whatever reason), then apparently that's the desired hardware
description and let's remove it from the binding to match the ABI.

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: display/msm/gpu: Narrow reg and reg-names for Adreno 610.0 and alike

[Konrad Dybcio]

On 12/29/25 8:23 AM, Krzysztof Kozlowski wrote:
> On 28/12/2025 15:59, Rob Clark wrote:
>> On Sat, Dec 27, 2025 at 11:56 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>>
>>> On 27/12/2025 23:01, Rob Clark wrote:
>>>> On Sat, Dec 27, 2025 at 3:05 AM Krzysztof Kozlowski
>>>> <krzysztof.kozlowski@oss.qualcomm.com> wrote:
>>>>>
>>>>> DTS files for qcom,adreno-610.0 and qcom,adreno-07000200 contain only one
>>>>> "reg" entry, not two, and the binding defines the second entry in
>>>>> "reg-names" differently than top-level part, so just simplify it and
>>>>> narrow to only one entry.
>>>>
>>>> I'll defer to Akhil about whether this is actually needed (vs just
>>>> incomplete gpu devcoredump support for certain GPUs).  In general
>>>> cx_dbgc is needed to capture state for gpu devcoredump state
>>>> snapshots, but not directly used in normal operations.  It seems
>>>> similar to the situation with mapping gpucc as part of gmu, ie. not
>>>> something the CPU normally deals with directly, but necessary to
>>>> capture crash state.
>>>
>>> I don't get why binding was added with cx_dbgc, but DTS not. Neither
>>> binding nor DTS depends on actual usage, so I assume someone
>>> intentionally did not want DTS to contain cx_dbgc and binding should
>>> follow. Otherwise we should make the DTS complete and make the binding
>>> strict (leading to warnings if DTS is not updated).
>>
>> I'm not sure about the history.. but I can say that cx_dbgc is only
>> used for gpu state snapshot / devcoredump.  So it would be easy to not
>> notice if it were missing.
>>
>> We have a similar slightly ugly thing where gpucc is included in the
>> gmu map.. only for devcoredump.  Maybe we need a different way to
>> handle these things that are only mapped for state capture?
> 
> No. Either hardware has it or not. If hardware has it, then both DTS and
> binding should have it. If people decided that DTS should not have it
> (for whatever reason), then apparently that's the desired hardware
> description and let's remove it from the binding to match the ABI.

I don't recall why it was never added. It's

<0x0 0x05961000 0x0 0x800>

for both 6115 and 2290 though. I'll send a patch to fix that up.
It seems like (at a glance) that there shouldn't be much of an issue
with the crashdumper, but I'm not super sure either..

Konrad