[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <50820e7b-b302-4f7f-baf9-778f3db6cfff@quicinc.com>
Date: Tue, 22 Apr 2025 19:46:02 -0700
From: Abhinav Kumar <quic_abhinavk@...cinc.com>
To: Krzysztof Kozlowski <krzk@...nel.org>, Rob Clark <robdclark@...il.com>,
Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
Sean Paul <sean@...rly.run>,
Marijn Suijten <marijn.suijten@...ainline.org>,
David Airlie
<airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Maarten Lankhorst
<maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Kuogee Hsieh <quic_khsieh@...cinc.com>,
Mahadevan <quic_mahap@...cinc.com>
CC: <linux-arm-msm@...r.kernel.org>, <dri-devel@...ts.freedesktop.org>,
<freedreno@...ts.freedesktop.org>, <devicetree@...r.kernel.org>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 3/4] dt-bindings: display/msm: add stream 1 pixel clock
binding
Hi Krzysztof
On 12/3/2024 12:04 AM, Krzysztof Kozlowski wrote:
> On 03/12/2024 04:31, Abhinav Kumar wrote:
>> On some chipsets the display port controller can support more
>
> Which chipsets?
>
From the current list of chipsets which support DP, the following can
support more than one stream.
qcom,sa8775p-dp
qcom,sc7280-dp
qcom,sc8180x-dp
qcom,sc8280xp-dp
qcom,sm8350-dp
qcom,sm8650-dp
qcom,sm8550-dp
qcom,sm8450-dp
qcom,sm8250-dp
qcom,sm8150-dp
So do you also want all of these to be added in the same if block as
qcom,sa8775p-dp?
>> than one pixel stream (multi-stream transport). To support MST
>> on such chipsets, add the binding for stream 1 pixel clock for
>> display port controller. Since this mode is not supported on all
>> chipsets, add exception rules and min/max items to clearly mark
>> which chipsets support only SST mode (single stream) and which ones
>> support MST.
>>
>> Signed-off-by: Abhinav Kumar <quic_abhinavk@...cinc.com>
>> ---
>> .../bindings/display/msm/dp-controller.yaml | 32 ++++++++++++++++++++++
>> .../bindings/display/msm/qcom,sa8775p-mdss.yaml | 9 ++++--
>> 2 files changed, 38 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
>> index 9fe2bf0484d8..650d19e58277 100644
>> --- a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
>> +++ b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
>> @@ -50,30 +50,38 @@ properties:
>> maxItems: 1
>>
>> clocks:
>> + minItems: 5
>> items:
>> - description: AHB clock to enable register access
>> - description: Display Port AUX clock
>> - description: Display Port Link clock
>> - description: Link interface clock between DP and PHY
>> - description: Display Port stream 0 Pixel clock
>> + - description: Display Port stream 1 Pixel clock
>>
>> clock-names:
>> + minItems: 5
>> items:
>> - const: core_iface
>> - const: core_aux
>> - const: ctrl_link
>> - const: ctrl_link_iface
>> - const: stream_pixel
>> + - const: stream_1_pixel
>>
>> assigned-clocks:
>> + minItems: 2
>> items:
>> - description: link clock source
>> - description: stream 0 pixel clock source
>> + - description: stream 1 pixel clock source
>>
>> assigned-clock-parents:
>> + minItems: 2
>> items:
>> - description: Link clock PLL output provided by PHY block
>> - description: Stream 0 pixel clock PLL output provided by PHY block
>> + - description: Stream 1 pixel clock PLL output provided by PHY block
>>
>> phys:
>> maxItems: 1
>> @@ -175,6 +183,30 @@ allOf:
>> required:
>> - "#sound-dai-cells"
>>
>
> Missing if: narrowing this to 5 items for other devices.
>
OR would an else be better?
+ else:
+ properties:
+ clocks:
+ maxItems: 5
+ clock-names:
+ items:
+ - const: core_iface
+ - const: core_aux
+ - const: ctrl_link
+ - const: ctrl_link_iface
+ - const: stream_pixel
>> + - if:
>> + properties:
>> + compatible:
>> + contains:
>> + enum:
>> + - qcom,sa8775p-dp
>> +
>> + then:
>> + properties:
>> + clocks:
>
> Missing minItems, otherwise it is pointless.
>
I thought that since I have already specified the minItems as 5
in the clocks in the section above, I need to specify only the maxItems
here?
clocks:
+ minItems: 5
items:
- description: AHB clock to enable register access
- description: Display Port AUX clock
- description: Display Port Link clock
- description: Link interface clock between DP and PHY
- description: Display Port stream 0 Pixel clock
+ - description: Display Port stream 1 Pixel clock
>> + maxItems: 6
>> + clock-names:
>> + items:
>> + - const: core_iface
>> + - const: core_aux
>> + - const: ctrl_link
>> + - const: ctrl_link_iface
>> + - const: stream_pixel
>> + - const: stream_1_pixel
>> + assigned-clocks:
>> + maxItems: 3
>
> Missing minItems... or just drop, it's not accurate or not even correct.
> I can assign 4 clocks, why not? Or rather: why do you stop users from
> assigning 4 clocks?
>
Sure, I can drop this.
>
>> + assigned-clock-parents:
>> + maxItems: 3
>> +
>> additionalProperties: false
>
>
>
> Best regards,
> Krzysztof
Powered by blists - more mailing lists