Re: [PATCH 3/4] dt-bindings: display/msm: add stream 1 pixel clock binding

[Krzysztof Kozlowski <krzk@kernel.org>]

On 23/04/2025 04:46, Abhinav Kumar wrote:
> 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?

That was talk in 2024. Entire context is gone if you reply after three
months. I do not have even that emails in my inbox anymore.

Probably I expected commit msg to mention at least some, so everyone
knows which chipsets are affected here and one can verify the statements
from commit msg.

> 
>>> 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@quicinc.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?

Usually not, although depends how this binding is written.


> 
> +    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?

No, you need explicit constraints here.



Best regards,
Krzysztof