[v2,8/9] media: dt-bindings: Add Intel Displayport RX IP

  The Intel Displayport RX IP is a part of the DisplayPort Intel FPGA IP
Core. It implements a DisplayPort 1.4 receiver capable of HBR3 video
capture and Multi-Stream Transport. The user guide can be found here:

https://www.intel.com/programmable/technical-pdfs/683273.pdf

Signed-off-by: Paweł Anikiel <panikiel@google.com>
---
 .../devicetree/bindings/media/intel,dprx.yaml | 160 ++++++++++++++++++
 1 file changed, 160 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/intel,dprx.yaml
  

On 21/02/2024 17:02, Paweł Anikiel wrote:
> The Intel Displayport RX IP is a part of the DisplayPort Intel FPGA IP
> Core. It implements a DisplayPort 1.4 receiver capable of HBR3 video
> capture and Multi-Stream Transport. The user guide can be found here:
> 
> https://www.intel.com/programmable/technical-pdfs/683273.pdf
> 
> Signed-off-by: Paweł Anikiel <panikiel@google.com>
> ---
>  .../devicetree/bindings/media/intel,dprx.yaml | 160 ++++++++++++++++++
>  1 file changed, 160 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/intel,dprx.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/intel,dprx.yaml b/Documentation/devicetree/bindings/media/intel,dprx.yaml
> new file mode 100644
> index 000000000000..31025f2d5dcd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/intel,dprx.yaml
> @@ -0,0 +1,160 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/intel,dprx.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Intel DisplayPort RX IP
> +
> +maintainers:
> +  - Paweł Anikiel <panikiel@google.com>
> +
> +description: |
> +  The Intel Displayport RX IP is a part of the DisplayPort Intel FPGA IP
> +  Core. It implements a DisplayPort 1.4 receiver capable of HBR3 video
> +  capture and Multi-Stream Transport.
> +
> +  The IP features a large number of configuration parameters, found at:
> +  https://www.intel.com/content/www/us/en/docs/programmable/683273/23-3-20-0-1/sink-parameters.html
> +
> +  The following parameters have to be enabled:
> +    - Support DisplayPort sink
> +    - Enable GPU control
> +  The following parameters' values have to be set in the devicetree:
> +    - RX maximum link rate
> +    - Maximum lane count
> +    - Support MST
> +    - Max stream count (only if Support MST is enabled)
> +
> +properties:
> +  compatible:
> +    const: intel,dprx-20.0.1
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  intel,max-link-rate:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Max link rate configuration parameter

Please do not duplicate property name in description. It's useless.
Instead explain what is this responsible for.

Why max-link-rate would differ for the same dprx-20.0.1? And why
standard properties cannot be used?

Same for all questions below.

> +
> +  intel,max-lane-count:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Max lane count configuration parameter
> +
> +  intel,multi-stream-support:
> +    type: boolean
> +    description: Multi-Stream Transport support configuration parameter
> +
> +  intel,max-stream-count:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Max stream count configuration parameter
> +
> +  port:
> +    $ref: /schemas/graph.yaml#/properties/port
> +    description: SST main link

I don't understand why you have both port and ports. Shouldn't this be
under ports?

> +
> +  ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +
> +    properties:
> +      port@0:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description: MST virtual channel 0 or SST main link
> +
> +      port@1:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description: MST virtual channel 1
> +
> +      port@2:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description: MST virtual channel 2
> +
> +      port@3:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description: MST virtual channel 3
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +allOf:
> +  - if:
> +      required:
> +        - intel,multi-stream-support
> +    then:
> +      required:
> +        - intel,max-stream-count
> +        - ports
> +    else:
> +      required:
> +        - port


Best regards,
Krzysztof
  

On Mon, Feb 26, 2024 at 10:13 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 21/02/2024 17:02, Paweł Anikiel wrote:
> > The Intel Displayport RX IP is a part of the DisplayPort Intel FPGA IP
> > Core. It implements a DisplayPort 1.4 receiver capable of HBR3 video
> > capture and Multi-Stream Transport. The user guide can be found here:
> >
> > https://www.intel.com/programmable/technical-pdfs/683273.pdf
> >
> > Signed-off-by: Paweł Anikiel <panikiel@google.com>
> > ---
> >  .../devicetree/bindings/media/intel,dprx.yaml | 160 ++++++++++++++++++
> >  1 file changed, 160 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/media/intel,dprx.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/media/intel,dprx.yaml b/Documentation/devicetree/bindings/media/intel,dprx.yaml
> > new file mode 100644
> > index 000000000000..31025f2d5dcd
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/intel,dprx.yaml
> > @@ -0,0 +1,160 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/intel,dprx.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Intel DisplayPort RX IP
> > +
> > +maintainers:
> > +  - Paweł Anikiel <panikiel@google.com>
> > +
> > +description: |
> > +  The Intel Displayport RX IP is a part of the DisplayPort Intel FPGA IP
> > +  Core. It implements a DisplayPort 1.4 receiver capable of HBR3 video
> > +  capture and Multi-Stream Transport.
> > +
> > +  The IP features a large number of configuration parameters, found at:
> > +  https://www.intel.com/content/www/us/en/docs/programmable/683273/23-3-20-0-1/sink-parameters.html
> > +
> > +  The following parameters have to be enabled:
> > +    - Support DisplayPort sink
> > +    - Enable GPU control
> > +  The following parameters' values have to be set in the devicetree:
> > +    - RX maximum link rate
> > +    - Maximum lane count
> > +    - Support MST
> > +    - Max stream count (only if Support MST is enabled)
> > +
> > +properties:
> > +  compatible:
> > +    const: intel,dprx-20.0.1
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  intel,max-link-rate:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: Max link rate configuration parameter
>
> Please do not duplicate property name in description. It's useless.
> Instead explain what is this responsible for.
>
> Why max-link-rate would differ for the same dprx-20.0.1? And why
> standard properties cannot be used?
>
> Same for all questions below.

These four properties are the IP configuration parameters mentioned in
the device description. When generating the IP core you can set these
parameters, which could make them differ for the same dprx-20.0.1.
They are documented in the user guide, for which I also put a link in
the description. Is that enough? Or should I also document these
parameters here?

>
> > +
> > +  intel,max-lane-count:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: Max lane count configuration parameter
> > +
> > +  intel,multi-stream-support:
> > +    type: boolean
> > +    description: Multi-Stream Transport support configuration parameter
> > +
> > +  intel,max-stream-count:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: Max stream count configuration parameter
> > +
> > +  port:
> > +    $ref: /schemas/graph.yaml#/properties/port
> > +    description: SST main link
>
> I don't understand why you have both port and ports. Shouldn't this be
> under ports?

I put both so that you can use the shorter port property when the
device only has one port (i.e. no MST support). It would work fine
without it. If you think that's unnecessary, I can remove it (and use
the ports property even if there is only one).
  

On 26/02/2024 11:59, Paweł Anikiel wrote:
>>> +properties:
>>> +  compatible:
>>> +    const: intel,dprx-20.0.1
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +  interrupts:
>>> +    maxItems: 1
>>> +
>>> +  intel,max-link-rate:
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    description: Max link rate configuration parameter
>>
>> Please do not duplicate property name in description. It's useless.
>> Instead explain what is this responsible for.
>>
>> Why max-link-rate would differ for the same dprx-20.0.1? And why
>> standard properties cannot be used?
>>
>> Same for all questions below.
> 
> These four properties are the IP configuration parameters mentioned in
> the device description. When generating the IP core you can set these
> parameters, which could make them differ for the same dprx-20.0.1.
> They are documented in the user guide, for which I also put a link in
> the description. Is that enough? Or should I also document these
> parameters here?

user-guide is something for users, like user-space programmers or
end-users. I would never open it to look for any information related to
hardware.

Anyway, external resources are a no-go. We have it clearly in submitting
patches:

https://elixir.bootlin.com/linux/v6.8-rc6/source/Documentation/process/submitting-patches.rst#L130

> 
>>
>>> +
>>> +  intel,max-lane-count:
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    description: Max lane count configuration parameter
>>> +
>>> +  intel,multi-stream-support:
>>> +    type: boolean
>>> +    description: Multi-Stream Transport support configuration parameter
>>> +
>>> +  intel,max-stream-count:
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    description: Max stream count configuration parameter
>>> +
>>> +  port:
>>> +    $ref: /schemas/graph.yaml#/properties/port
>>> +    description: SST main link
>>
>> I don't understand why you have both port and ports. Shouldn't this be
>> under ports?
> 
> I put both so that you can use the shorter port property when the
> device only has one port (i.e. no MST support). It would work fine
> without it. If you think that's unnecessary, I can remove it (and use
> the ports property even if there is only one).

No, it is fine, but then you need allOf: which will restrict to only one
of them: either port or ports.

Best regards,
Krzysztof
  

On Mon, Feb 26, 2024 at 1:06 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 26/02/2024 11:59, Paweł Anikiel wrote:
> >>> +properties:
> >>> +  compatible:
> >>> +    const: intel,dprx-20.0.1
> >>> +
> >>> +  reg:
> >>> +    maxItems: 1
> >>> +
> >>> +  interrupts:
> >>> +    maxItems: 1
> >>> +
> >>> +  intel,max-link-rate:
> >>> +    $ref: /schemas/types.yaml#/definitions/uint32
> >>> +    description: Max link rate configuration parameter
> >>
> >> Please do not duplicate property name in description. It's useless.
> >> Instead explain what is this responsible for.
> >>
> >> Why max-link-rate would differ for the same dprx-20.0.1? And why
> >> standard properties cannot be used?
> >>
> >> Same for all questions below.
> >
> > These four properties are the IP configuration parameters mentioned in
> > the device description. When generating the IP core you can set these
> > parameters, which could make them differ for the same dprx-20.0.1.
> > They are documented in the user guide, for which I also put a link in
> > the description. Is that enough? Or should I also document these
> > parameters here?
>
> user-guide is something for users, like user-space programmers or
> end-users. I would never open it to look for any information related to
> hardware.
>
> Anyway, external resources are a no-go. We have it clearly in submitting
> patches:
>
> https://elixir.bootlin.com/linux/v6.8-rc6/source/Documentation/process/submitting-patches.rst#L130

Okay, I will describe these properties in the bindings as well.

>
> >
> >>
> >>> +
> >>> +  intel,max-lane-count:
> >>> +    $ref: /schemas/types.yaml#/definitions/uint32
> >>> +    description: Max lane count configuration parameter
> >>> +
> >>> +  intel,multi-stream-support:
> >>> +    type: boolean
> >>> +    description: Multi-Stream Transport support configuration parameter
> >>> +
> >>> +  intel,max-stream-count:
> >>> +    $ref: /schemas/types.yaml#/definitions/uint32
> >>> +    description: Max stream count configuration parameter
> >>> +
> >>> +  port:
> >>> +    $ref: /schemas/graph.yaml#/properties/port
> >>> +    description: SST main link
> >>
> >> I don't understand why you have both port and ports. Shouldn't this be
> >> under ports?
> >
> > I put both so that you can use the shorter port property when the
> > device only has one port (i.e. no MST support). It would work fine
> > without it. If you think that's unnecessary, I can remove it (and use
> > the ports property even if there is only one).
>
> No, it is fine, but then you need allOf: which will restrict to only one
> of them: either port or ports.

There already is an allOf below that says that ports is required for
MST support and port is required otherwise. Isn't this enough?
  

On 26/02/2024 13:43, Paweł Anikiel wrote:
>>>>> +  intel,max-stream-count:
>>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>>>> +    description: Max stream count configuration parameter
>>>>> +
>>>>> +  port:
>>>>> +    $ref: /schemas/graph.yaml#/properties/port
>>>>> +    description: SST main link
>>>>
>>>> I don't understand why you have both port and ports. Shouldn't this be
>>>> under ports?
>>>
>>> I put both so that you can use the shorter port property when the
>>> device only has one port (i.e. no MST support). It would work fine
>>> without it. If you think that's unnecessary, I can remove it (and use
>>> the ports property even if there is only one).
>>
>> No, it is fine, but then you need allOf: which will restrict to only one
>> of them: either port or ports.
> 
> There already is an allOf below that says that ports is required for
> MST support and port is required otherwise. Isn't this enough?

Add both port and ports and see if it is enough.

Best regards,
Krzysztof
  

On Mon, Feb 26, 2024 at 6:29 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 26/02/2024 13:43, Paweł Anikiel wrote:
> >>>>> +  intel,max-stream-count:
> >>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
> >>>>> +    description: Max stream count configuration parameter
> >>>>> +
> >>>>> +  port:
> >>>>> +    $ref: /schemas/graph.yaml#/properties/port
> >>>>> +    description: SST main link
> >>>>
> >>>> I don't understand why you have both port and ports. Shouldn't this be
> >>>> under ports?
> >>>
> >>> I put both so that you can use the shorter port property when the
> >>> device only has one port (i.e. no MST support). It would work fine
> >>> without it. If you think that's unnecessary, I can remove it (and use
> >>> the ports property even if there is only one).
> >>
> >> No, it is fine, but then you need allOf: which will restrict to only one
> >> of them: either port or ports.
> >
> > There already is an allOf below that says that ports is required for
> > MST support and port is required otherwise. Isn't this enough?
>
> Add both port and ports and see if it is enough.

Ok, I see. I tried and this seems to work:

oneOf:
  - required:
      - port
  - required:
      - ports

And that would make the if/else with port and ports below not needed.
What do you think?
  

On Tue, Feb 27, 2024 at 02:11:27PM +0100, Paweł Anikiel wrote:
> On Mon, Feb 26, 2024 at 6:29 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
> >
> > On 26/02/2024 13:43, Paweł Anikiel wrote:
> > >>>>> +  intel,max-stream-count:
> > >>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
> > >>>>> +    description: Max stream count configuration parameter
> > >>>>> +
> > >>>>> +  port:
> > >>>>> +    $ref: /schemas/graph.yaml#/properties/port
> > >>>>> +    description: SST main link
> > >>>>
> > >>>> I don't understand why you have both port and ports. Shouldn't this be
> > >>>> under ports?
> > >>>
> > >>> I put both so that you can use the shorter port property when the
> > >>> device only has one port (i.e. no MST support). It would work fine
> > >>> without it. If you think that's unnecessary, I can remove it (and use
> > >>> the ports property even if there is only one).
> > >>
> > >> No, it is fine, but then you need allOf: which will restrict to only one
> > >> of them: either port or ports.
> > >
> > > There already is an allOf below that says that ports is required for
> > > MST support and port is required otherwise. Isn't this enough?
> >
> > Add both port and ports and see if it is enough.
> 
> Ok, I see. I tried and this seems to work:
> 
> oneOf:
>   - required:
>       - port
>   - required:
>       - ports
> 
> And that would make the if/else with port and ports below not needed.
> What do you think?

Just always use 'ports' rather than complicate things.

Rob
  

On Mon, Feb 26, 2024 at 11:59:42AM +0100, Paweł Anikiel wrote:
> On Mon, Feb 26, 2024 at 10:13 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
> >
> > On 21/02/2024 17:02, Paweł Anikiel wrote:
> > > The Intel Displayport RX IP is a part of the DisplayPort Intel FPGA IP
> > > Core. It implements a DisplayPort 1.4 receiver capable of HBR3 video
> > > capture and Multi-Stream Transport. The user guide can be found here:
> > >
> > > https://www.intel.com/programmable/technical-pdfs/683273.pdf
> > >
> > > Signed-off-by: Paweł Anikiel <panikiel@google.com>
> > > ---
> > >  .../devicetree/bindings/media/intel,dprx.yaml | 160 ++++++++++++++++++
> > >  1 file changed, 160 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/media/intel,dprx.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/media/intel,dprx.yaml b/Documentation/devicetree/bindings/media/intel,dprx.yaml
> > > new file mode 100644
> > > index 000000000000..31025f2d5dcd
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/media/intel,dprx.yaml
> > > @@ -0,0 +1,160 @@
> > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/media/intel,dprx.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Intel DisplayPort RX IP
> > > +
> > > +maintainers:
> > > +  - Paweł Anikiel <panikiel@google.com>
> > > +
> > > +description: |
> > > +  The Intel Displayport RX IP is a part of the DisplayPort Intel FPGA IP
> > > +  Core. It implements a DisplayPort 1.4 receiver capable of HBR3 video
> > > +  capture and Multi-Stream Transport.
> > > +
> > > +  The IP features a large number of configuration parameters, found at:
> > > +  https://www.intel.com/content/www/us/en/docs/programmable/683273/23-3-20-0-1/sink-parameters.html
> > > +
> > > +  The following parameters have to be enabled:
> > > +    - Support DisplayPort sink
> > > +    - Enable GPU control
> > > +  The following parameters' values have to be set in the devicetree:
> > > +    - RX maximum link rate
> > > +    - Maximum lane count
> > > +    - Support MST
> > > +    - Max stream count (only if Support MST is enabled)
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: intel,dprx-20.0.1
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  interrupts:
> > > +    maxItems: 1
> > > +
> > > +  intel,max-link-rate:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +    description: Max link rate configuration parameter
> >
> > Please do not duplicate property name in description. It's useless.
> > Instead explain what is this responsible for.
> >
> > Why max-link-rate would differ for the same dprx-20.0.1? And why
> > standard properties cannot be used?
> >
> > Same for all questions below.
> 
> These four properties are the IP configuration parameters mentioned in
> the device description. When generating the IP core you can set these
> parameters, which could make them differ for the same dprx-20.0.1.
> They are documented in the user guide, for which I also put a link in
> the description. Is that enough? Or should I also document these
> parameters here?

Use the standard properties: link-frequencies and data-lanes. Those go 
under the port(s) because they are inheritly per logical link.

Rob
  

On Tue, Feb 27, 2024 at 3:29 PM Rob Herring <robh@kernel.org> wrote:
>
> On Mon, Feb 26, 2024 at 11:59:42AM +0100, Paweł Anikiel wrote:
> > On Mon, Feb 26, 2024 at 10:13 AM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> > >
> > > On 21/02/2024 17:02, Paweł Anikiel wrote:
> > > > The Intel Displayport RX IP is a part of the DisplayPort Intel FPGA IP
> > > > Core. It implements a DisplayPort 1.4 receiver capable of HBR3 video
> > > > capture and Multi-Stream Transport. The user guide can be found here:
> > > >
> > > > https://www.intel.com/programmable/technical-pdfs/683273.pdf
> > > >
> > > > Signed-off-by: Paweł Anikiel <panikiel@google.com>
> > > > ---
> > > >  .../devicetree/bindings/media/intel,dprx.yaml | 160 ++++++++++++++++++
> > > >  1 file changed, 160 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/media/intel,dprx.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/media/intel,dprx.yaml b/Documentation/devicetree/bindings/media/intel,dprx.yaml
> > > > new file mode 100644
> > > > index 000000000000..31025f2d5dcd
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/media/intel,dprx.yaml
> > > > @@ -0,0 +1,160 @@
> > > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/media/intel,dprx.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Intel DisplayPort RX IP
> > > > +
> > > > +maintainers:
> > > > +  - Paweł Anikiel <panikiel@google.com>
> > > > +
> > > > +description: |
> > > > +  The Intel Displayport RX IP is a part of the DisplayPort Intel FPGA IP
> > > > +  Core. It implements a DisplayPort 1.4 receiver capable of HBR3 video
> > > > +  capture and Multi-Stream Transport.
> > > > +
> > > > +  The IP features a large number of configuration parameters, found at:
> > > > +  https://www.intel.com/content/www/us/en/docs/programmable/683273/23-3-20-0-1/sink-parameters.html
> > > > +
> > > > +  The following parameters have to be enabled:
> > > > +    - Support DisplayPort sink
> > > > +    - Enable GPU control
> > > > +  The following parameters' values have to be set in the devicetree:
> > > > +    - RX maximum link rate
> > > > +    - Maximum lane count
> > > > +    - Support MST
> > > > +    - Max stream count (only if Support MST is enabled)
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    const: intel,dprx-20.0.1
> > > > +
> > > > +  reg:
> > > > +    maxItems: 1
> > > > +
> > > > +  interrupts:
> > > > +    maxItems: 1
> > > > +
> > > > +  intel,max-link-rate:
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > +    description: Max link rate configuration parameter
> > >
> > > Please do not duplicate property name in description. It's useless.
> > > Instead explain what is this responsible for.
> > >
> > > Why max-link-rate would differ for the same dprx-20.0.1? And why
> > > standard properties cannot be used?
> > >
> > > Same for all questions below.
> >
> > These four properties are the IP configuration parameters mentioned in
> > the device description. When generating the IP core you can set these
> > parameters, which could make them differ for the same dprx-20.0.1.
> > They are documented in the user guide, for which I also put a link in
> > the description. Is that enough? Or should I also document these
> > parameters here?
>
> Use the standard properties: link-frequencies and data-lanes. Those go
> under the port(s) because they are inheritly per logical link.

The DP receiver has one input interface (a deserialized DP stream),
and up to four output interfaces (the decoded video streams). The "max
link rate" and "max lane count" parameters only describe the input
interface to the receiver. However, the port(s) I am using here are
for the output streams. They are not affected by those parameters, so
I don't think these properties should go under the output port(s).

The receiver doesn't have an input port in the DT, because there isn't
any controllable entity on the other side - the deserializer doesn't
have any software interface. Since these standard properties
(link-frequencies and data-lanes) are only defined in
video-interfaces.yaml (which IIUC describes a graph endpoint), I can't
use them directly in the device node.

Do you see a way to use these standard properties here?
  

On 28/02/2024 12:05, Paweł Anikiel wrote:
> On Tue, Feb 27, 2024 at 3:29 PM Rob Herring <robh@kernel.org> wrote:
>>
>> On Mon, Feb 26, 2024 at 11:59:42AM +0100, Paweł Anikiel wrote:
>>> On Mon, Feb 26, 2024 at 10:13 AM Krzysztof Kozlowski
>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>>
>>>> On 21/02/2024 17:02, Paweł Anikiel wrote:
>>>>> The Intel Displayport RX IP is a part of the DisplayPort Intel FPGA IP
>>>>> Core. It implements a DisplayPort 1.4 receiver capable of HBR3 video
>>>>> capture and Multi-Stream Transport. The user guide can be found here:
>>>>>
>>>>> https://www.intel.com/programmable/technical-pdfs/683273.pdf
>>>>>
>>>>> Signed-off-by: Paweł Anikiel <panikiel@google.com>
>>>>> ---
>>>>>  .../devicetree/bindings/media/intel,dprx.yaml | 160 ++++++++++++++++++
>>>>>  1 file changed, 160 insertions(+)
>>>>>  create mode 100644 Documentation/devicetree/bindings/media/intel,dprx.yaml
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/media/intel,dprx.yaml b/Documentation/devicetree/bindings/media/intel,dprx.yaml
>>>>> new file mode 100644
>>>>> index 000000000000..31025f2d5dcd
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/media/intel,dprx.yaml
>>>>> @@ -0,0 +1,160 @@
>>>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>>>> +%YAML 1.2
>>>>> +---
>>>>> +$id: http://devicetree.org/schemas/media/intel,dprx.yaml#
>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>> +
>>>>> +title: Intel DisplayPort RX IP
>>>>> +
>>>>> +maintainers:
>>>>> +  - Paweł Anikiel <panikiel@google.com>
>>>>> +
>>>>> +description: |
>>>>> +  The Intel Displayport RX IP is a part of the DisplayPort Intel FPGA IP
>>>>> +  Core. It implements a DisplayPort 1.4 receiver capable of HBR3 video
>>>>> +  capture and Multi-Stream Transport.
>>>>> +
>>>>> +  The IP features a large number of configuration parameters, found at:
>>>>> +  https://www.intel.com/content/www/us/en/docs/programmable/683273/23-3-20-0-1/sink-parameters.html
>>>>> +
>>>>> +  The following parameters have to be enabled:
>>>>> +    - Support DisplayPort sink
>>>>> +    - Enable GPU control
>>>>> +  The following parameters' values have to be set in the devicetree:
>>>>> +    - RX maximum link rate
>>>>> +    - Maximum lane count
>>>>> +    - Support MST
>>>>> +    - Max stream count (only if Support MST is enabled)
>>>>> +
>>>>> +properties:
>>>>> +  compatible:
>>>>> +    const: intel,dprx-20.0.1
>>>>> +
>>>>> +  reg:
>>>>> +    maxItems: 1
>>>>> +
>>>>> +  interrupts:
>>>>> +    maxItems: 1
>>>>> +
>>>>> +  intel,max-link-rate:
>>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>>>> +    description: Max link rate configuration parameter
>>>>
>>>> Please do not duplicate property name in description. It's useless.
>>>> Instead explain what is this responsible for.
>>>>
>>>> Why max-link-rate would differ for the same dprx-20.0.1? And why
>>>> standard properties cannot be used?
>>>>
>>>> Same for all questions below.
>>>
>>> These four properties are the IP configuration parameters mentioned in
>>> the device description. When generating the IP core you can set these
>>> parameters, which could make them differ for the same dprx-20.0.1.
>>> They are documented in the user guide, for which I also put a link in
>>> the description. Is that enough? Or should I also document these
>>> parameters here?
>>
>> Use the standard properties: link-frequencies and data-lanes. Those go
>> under the port(s) because they are inheritly per logical link.
> 
> The DP receiver has one input interface (a deserialized DP stream),
> and up to four output interfaces (the decoded video streams). The "max
> link rate" and "max lane count" parameters only describe the input
> interface to the receiver. However, the port(s) I am using here are
> for the output streams. They are not affected by those parameters, so
> I don't think these properties should go under the output port(s).
> 
> The receiver doesn't have an input port in the DT, because there isn't
> any controllable entity on the other side - the deserializer doesn't
> have any software interface. Since these standard properties
> (link-frequencies and data-lanes) are only defined in
> video-interfaces.yaml (which IIUC describes a graph endpoint), I can't
> use them directly in the device node.

DT describes the hardware, so where does the input come? From something,
right? Regardless if you have a driver or not. There is dp-connector
binding, if this is physical port.

> 
> Do you see a way to use these standard properties here?

Best regards,
Krzysztof
  

On Wed, Feb 28, 2024 at 1:18 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 28/02/2024 12:05, Paweł Anikiel wrote:
> > On Tue, Feb 27, 2024 at 3:29 PM Rob Herring <robh@kernel.org> wrote:
> >>
> >> On Mon, Feb 26, 2024 at 11:59:42AM +0100, Paweł Anikiel wrote:
> >>> On Mon, Feb 26, 2024 at 10:13 AM Krzysztof Kozlowski
> >>> <krzysztof.kozlowski@linaro.org> wrote:
> >>>>
> >>>> On 21/02/2024 17:02, Paweł Anikiel wrote:
> >>>>> The Intel Displayport RX IP is a part of the DisplayPort Intel FPGA IP
> >>>>> Core. It implements a DisplayPort 1.4 receiver capable of HBR3 video
> >>>>> capture and Multi-Stream Transport. The user guide can be found here:
> >>>>>
> >>>>> https://www.intel.com/programmable/technical-pdfs/683273.pdf
> >>>>>
> >>>>> Signed-off-by: Paweł Anikiel <panikiel@google.com>
> >>>>> ---
> >>>>>  .../devicetree/bindings/media/intel,dprx.yaml | 160 ++++++++++++++++++
> >>>>>  1 file changed, 160 insertions(+)
> >>>>>  create mode 100644 Documentation/devicetree/bindings/media/intel,dprx.yaml
> >>>>>
> >>>>> diff --git a/Documentation/devicetree/bindings/media/intel,dprx.yaml b/Documentation/devicetree/bindings/media/intel,dprx.yaml
> >>>>> new file mode 100644
> >>>>> index 000000000000..31025f2d5dcd
> >>>>> --- /dev/null
> >>>>> +++ b/Documentation/devicetree/bindings/media/intel,dprx.yaml
> >>>>> @@ -0,0 +1,160 @@
> >>>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> >>>>> +%YAML 1.2
> >>>>> +---
> >>>>> +$id: http://devicetree.org/schemas/media/intel,dprx.yaml#
> >>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>>>> +
> >>>>> +title: Intel DisplayPort RX IP
> >>>>> +
> >>>>> +maintainers:
> >>>>> +  - Paweł Anikiel <panikiel@google.com>
> >>>>> +
> >>>>> +description: |
> >>>>> +  The Intel Displayport RX IP is a part of the DisplayPort Intel FPGA IP
> >>>>> +  Core. It implements a DisplayPort 1.4 receiver capable of HBR3 video
> >>>>> +  capture and Multi-Stream Transport.
> >>>>> +
> >>>>> +  The IP features a large number of configuration parameters, found at:
> >>>>> +  https://www.intel.com/content/www/us/en/docs/programmable/683273/23-3-20-0-1/sink-parameters.html
> >>>>> +
> >>>>> +  The following parameters have to be enabled:
> >>>>> +    - Support DisplayPort sink
> >>>>> +    - Enable GPU control
> >>>>> +  The following parameters' values have to be set in the devicetree:
> >>>>> +    - RX maximum link rate
> >>>>> +    - Maximum lane count
> >>>>> +    - Support MST
> >>>>> +    - Max stream count (only if Support MST is enabled)
> >>>>> +
> >>>>> +properties:
> >>>>> +  compatible:
> >>>>> +    const: intel,dprx-20.0.1
> >>>>> +
> >>>>> +  reg:
> >>>>> +    maxItems: 1
> >>>>> +
> >>>>> +  interrupts:
> >>>>> +    maxItems: 1
> >>>>> +
> >>>>> +  intel,max-link-rate:
> >>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
> >>>>> +    description: Max link rate configuration parameter
> >>>>
> >>>> Please do not duplicate property name in description. It's useless.
> >>>> Instead explain what is this responsible for.
> >>>>
> >>>> Why max-link-rate would differ for the same dprx-20.0.1? And why
> >>>> standard properties cannot be used?
> >>>>
> >>>> Same for all questions below.
> >>>
> >>> These four properties are the IP configuration parameters mentioned in
> >>> the device description. When generating the IP core you can set these
> >>> parameters, which could make them differ for the same dprx-20.0.1.
> >>> They are documented in the user guide, for which I also put a link in
> >>> the description. Is that enough? Or should I also document these
> >>> parameters here?
> >>
> >> Use the standard properties: link-frequencies and data-lanes. Those go
> >> under the port(s) because they are inheritly per logical link.
> >
> > The DP receiver has one input interface (a deserialized DP stream),
> > and up to four output interfaces (the decoded video streams). The "max
> > link rate" and "max lane count" parameters only describe the input
> > interface to the receiver. However, the port(s) I am using here are
> > for the output streams. They are not affected by those parameters, so
> > I don't think these properties should go under the output port(s).
> >
> > The receiver doesn't have an input port in the DT, because there isn't
> > any controllable entity on the other side - the deserializer doesn't
> > have any software interface. Since these standard properties
> > (link-frequencies and data-lanes) are only defined in
> > video-interfaces.yaml (which IIUC describes a graph endpoint), I can't
> > use them directly in the device node.
>
> DT describes the hardware, so where does the input come? From something,
> right? Regardless if you have a driver or not. There is dp-connector
> binding, if this is physical port.

Yes, it is a physical port. I agree adding a DT node for the physical
DP input connector would let us add link-frequencies to the input port
of the receiver.

However, dp-connector seems to be a binding for an output port - it's
under schemas/display/connector, and DP_PWR can be a power supply only
for an output port (looking at the dp-pwr-supply property). Also, the
driver for this binding is a DRM bridge driver (display-connector.c)
which would not be compatible with a v4l2 (sub)device.
  

On Wed, Feb 28, 2024 at 02:09:33PM +0100, Paweł Anikiel wrote:
> On Wed, Feb 28, 2024 at 1:18 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
> >
> > On 28/02/2024 12:05, Paweł Anikiel wrote:
> > > On Tue, Feb 27, 2024 at 3:29 PM Rob Herring <robh@kernel.org> wrote:
> > >>
> > >> On Mon, Feb 26, 2024 at 11:59:42AM +0100, Paweł Anikiel wrote:
> > >>> On Mon, Feb 26, 2024 at 10:13 AM Krzysztof Kozlowski
> > >>> <krzysztof.kozlowski@linaro.org> wrote:
> > >>>>
> > >>>> On 21/02/2024 17:02, Paweł Anikiel wrote:
> > >>>>> The Intel Displayport RX IP is a part of the DisplayPort Intel FPGA IP
> > >>>>> Core. It implements a DisplayPort 1.4 receiver capable of HBR3 video
> > >>>>> capture and Multi-Stream Transport. The user guide can be found here:
> > >>>>>
> > >>>>> https://www.intel.com/programmable/technical-pdfs/683273.pdf
> > >>>>>
> > >>>>> Signed-off-by: Paweł Anikiel <panikiel@google.com>
> > >>>>> ---
> > >>>>>  .../devicetree/bindings/media/intel,dprx.yaml | 160 ++++++++++++++++++
> > >>>>>  1 file changed, 160 insertions(+)
> > >>>>>  create mode 100644 Documentation/devicetree/bindings/media/intel,dprx.yaml
> > >>>>>
> > >>>>> diff --git a/Documentation/devicetree/bindings/media/intel,dprx.yaml b/Documentation/devicetree/bindings/media/intel,dprx.yaml
> > >>>>> new file mode 100644
> > >>>>> index 000000000000..31025f2d5dcd
> > >>>>> --- /dev/null
> > >>>>> +++ b/Documentation/devicetree/bindings/media/intel,dprx.yaml
> > >>>>> @@ -0,0 +1,160 @@
> > >>>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > >>>>> +%YAML 1.2
> > >>>>> +---
> > >>>>> +$id: http://devicetree.org/schemas/media/intel,dprx.yaml#
> > >>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > >>>>> +
> > >>>>> +title: Intel DisplayPort RX IP
> > >>>>> +
> > >>>>> +maintainers:
> > >>>>> +  - Paweł Anikiel <panikiel@google.com>
> > >>>>> +
> > >>>>> +description: |
> > >>>>> +  The Intel Displayport RX IP is a part of the DisplayPort Intel FPGA IP
> > >>>>> +  Core. It implements a DisplayPort 1.4 receiver capable of HBR3 video
> > >>>>> +  capture and Multi-Stream Transport.
> > >>>>> +
> > >>>>> +  The IP features a large number of configuration parameters, found at:
> > >>>>> +  https://www.intel.com/content/www/us/en/docs/programmable/683273/23-3-20-0-1/sink-parameters.html
> > >>>>> +
> > >>>>> +  The following parameters have to be enabled:
> > >>>>> +    - Support DisplayPort sink
> > >>>>> +    - Enable GPU control
> > >>>>> +  The following parameters' values have to be set in the devicetree:
> > >>>>> +    - RX maximum link rate
> > >>>>> +    - Maximum lane count
> > >>>>> +    - Support MST
> > >>>>> +    - Max stream count (only if Support MST is enabled)
> > >>>>> +
> > >>>>> +properties:
> > >>>>> +  compatible:
> > >>>>> +    const: intel,dprx-20.0.1
> > >>>>> +
> > >>>>> +  reg:
> > >>>>> +    maxItems: 1
> > >>>>> +
> > >>>>> +  interrupts:
> > >>>>> +    maxItems: 1
> > >>>>> +
> > >>>>> +  intel,max-link-rate:
> > >>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
> > >>>>> +    description: Max link rate configuration parameter
> > >>>>
> > >>>> Please do not duplicate property name in description. It's useless.
> > >>>> Instead explain what is this responsible for.
> > >>>>
> > >>>> Why max-link-rate would differ for the same dprx-20.0.1? And why
> > >>>> standard properties cannot be used?
> > >>>>
> > >>>> Same for all questions below.
> > >>>
> > >>> These four properties are the IP configuration parameters mentioned in
> > >>> the device description. When generating the IP core you can set these
> > >>> parameters, which could make them differ for the same dprx-20.0.1.
> > >>> They are documented in the user guide, for which I also put a link in
> > >>> the description. Is that enough? Or should I also document these
> > >>> parameters here?
> > >>
> > >> Use the standard properties: link-frequencies and data-lanes. Those go
> > >> under the port(s) because they are inheritly per logical link.
> > >
> > > The DP receiver has one input interface (a deserialized DP stream),
> > > and up to four output interfaces (the decoded video streams). The "max
> > > link rate" and "max lane count" parameters only describe the input
> > > interface to the receiver. However, the port(s) I am using here are
> > > for the output streams. They are not affected by those parameters, so
> > > I don't think these properties should go under the output port(s).
> > >
> > > The receiver doesn't have an input port in the DT, because there isn't
> > > any controllable entity on the other side - the deserializer doesn't
> > > have any software interface. Since these standard properties
> > > (link-frequencies and data-lanes) are only defined in
> > > video-interfaces.yaml (which IIUC describes a graph endpoint), I can't
> > > use them directly in the device node.
> >
> > DT describes the hardware, so where does the input come? From something,
> > right? Regardless if you have a driver or not. There is dp-connector
> > binding, if this is physical port.
> 
> Yes, it is a physical port. I agree adding a DT node for the physical
> DP input connector would let us add link-frequencies to the input port
> of the receiver.
> 
> However, dp-connector seems to be a binding for an output port - it's
> under schemas/display/connector, and DP_PWR can be a power supply only
> for an output port (looking at the dp-pwr-supply property). Also, the
> driver for this binding is a DRM bridge driver (display-connector.c)
> which would not be compatible with a v4l2 (sub)device.

So then we should add 'dp-input-connector' because they are different. 
When we haven't defined connectors, properties of the connector have 
been shoved in whatever node is associated with a connector like you 
have done. That works for a while, but then becomes unmanageable. DP on 
USB-C connectors for example.

OTOH, maybe your use here is niche enough to not be worth the trouble. 
Depends if we see the need for video input connectors in general.

Rob
  

On Wed, Feb 28, 2024 at 7:10 PM Rob Herring <robh@kernel.org> wrote:
>
> On Wed, Feb 28, 2024 at 02:09:33PM +0100, Paweł Anikiel wrote:
> > On Wed, Feb 28, 2024 at 1:18 PM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> > >
> > > On 28/02/2024 12:05, Paweł Anikiel wrote:
> > > > On Tue, Feb 27, 2024 at 3:29 PM Rob Herring <robh@kernel.org> wrote:
> > > >>
> > > >> On Mon, Feb 26, 2024 at 11:59:42AM +0100, Paweł Anikiel wrote:
> > > >>> On Mon, Feb 26, 2024 at 10:13 AM Krzysztof Kozlowski
> > > >>> <krzysztof.kozlowski@linaro.org> wrote:
> > > >>>>
> > > >>>> On 21/02/2024 17:02, Paweł Anikiel wrote:
> > > >>>>> The Intel Displayport RX IP is a part of the DisplayPort Intel FPGA IP
> > > >>>>> Core. It implements a DisplayPort 1.4 receiver capable of HBR3 video
> > > >>>>> capture and Multi-Stream Transport. The user guide can be found here:
> > > >>>>>
> > > >>>>> https://www.intel.com/programmable/technical-pdfs/683273.pdf
> > > >>>>>
> > > >>>>> Signed-off-by: Paweł Anikiel <panikiel@google.com>
> > > >>>>> ---
> > > >>>>>  .../devicetree/bindings/media/intel,dprx.yaml | 160 ++++++++++++++++++
> > > >>>>>  1 file changed, 160 insertions(+)
> > > >>>>>  create mode 100644 Documentation/devicetree/bindings/media/intel,dprx.yaml
> > > >>>>>
> > > >>>>> diff --git a/Documentation/devicetree/bindings/media/intel,dprx.yaml b/Documentation/devicetree/bindings/media/intel,dprx.yaml
> > > >>>>> new file mode 100644
> > > >>>>> index 000000000000..31025f2d5dcd
> > > >>>>> --- /dev/null
> > > >>>>> +++ b/Documentation/devicetree/bindings/media/intel,dprx.yaml
> > > >>>>> @@ -0,0 +1,160 @@
> > > >>>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > > >>>>> +%YAML 1.2
> > > >>>>> +---
> > > >>>>> +$id: http://devicetree.org/schemas/media/intel,dprx.yaml#
> > > >>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > >>>>> +
> > > >>>>> +title: Intel DisplayPort RX IP
> > > >>>>> +
> > > >>>>> +maintainers:
> > > >>>>> +  - Paweł Anikiel <panikiel@google.com>
> > > >>>>> +
> > > >>>>> +description: |
> > > >>>>> +  The Intel Displayport RX IP is a part of the DisplayPort Intel FPGA IP
> > > >>>>> +  Core. It implements a DisplayPort 1.4 receiver capable of HBR3 video
> > > >>>>> +  capture and Multi-Stream Transport.
> > > >>>>> +
> > > >>>>> +  The IP features a large number of configuration parameters, found at:
> > > >>>>> +  https://www.intel.com/content/www/us/en/docs/programmable/683273/23-3-20-0-1/sink-parameters.html
> > > >>>>> +
> > > >>>>> +  The following parameters have to be enabled:
> > > >>>>> +    - Support DisplayPort sink
> > > >>>>> +    - Enable GPU control
> > > >>>>> +  The following parameters' values have to be set in the devicetree:
> > > >>>>> +    - RX maximum link rate
> > > >>>>> +    - Maximum lane count
> > > >>>>> +    - Support MST
> > > >>>>> +    - Max stream count (only if Support MST is enabled)
> > > >>>>> +
> > > >>>>> +properties:
> > > >>>>> +  compatible:
> > > >>>>> +    const: intel,dprx-20.0.1
> > > >>>>> +
> > > >>>>> +  reg:
> > > >>>>> +    maxItems: 1
> > > >>>>> +
> > > >>>>> +  interrupts:
> > > >>>>> +    maxItems: 1
> > > >>>>> +
> > > >>>>> +  intel,max-link-rate:
> > > >>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
> > > >>>>> +    description: Max link rate configuration parameter
> > > >>>>
> > > >>>> Please do not duplicate property name in description. It's useless.
> > > >>>> Instead explain what is this responsible for.
> > > >>>>
> > > >>>> Why max-link-rate would differ for the same dprx-20.0.1? And why
> > > >>>> standard properties cannot be used?
> > > >>>>
> > > >>>> Same for all questions below.
> > > >>>
> > > >>> These four properties are the IP configuration parameters mentioned in
> > > >>> the device description. When generating the IP core you can set these
> > > >>> parameters, which could make them differ for the same dprx-20.0.1.
> > > >>> They are documented in the user guide, for which I also put a link in
> > > >>> the description. Is that enough? Or should I also document these
> > > >>> parameters here?
> > > >>
> > > >> Use the standard properties: link-frequencies and data-lanes. Those go
> > > >> under the port(s) because they are inheritly per logical link.
> > > >
> > > > The DP receiver has one input interface (a deserialized DP stream),
> > > > and up to four output interfaces (the decoded video streams). The "max
> > > > link rate" and "max lane count" parameters only describe the input
> > > > interface to the receiver. However, the port(s) I am using here are
> > > > for the output streams. They are not affected by those parameters, so
> > > > I don't think these properties should go under the output port(s).
> > > >
> > > > The receiver doesn't have an input port in the DT, because there isn't
> > > > any controllable entity on the other side - the deserializer doesn't
> > > > have any software interface. Since these standard properties
> > > > (link-frequencies and data-lanes) are only defined in
> > > > video-interfaces.yaml (which IIUC describes a graph endpoint), I can't
> > > > use them directly in the device node.
> > >
> > > DT describes the hardware, so where does the input come? From something,
> > > right? Regardless if you have a driver or not. There is dp-connector
> > > binding, if this is physical port.
> >
> > Yes, it is a physical port. I agree adding a DT node for the physical
> > DP input connector would let us add link-frequencies to the input port
> > of the receiver.
> >
> > However, dp-connector seems to be a binding for an output port - it's
> > under schemas/display/connector, and DP_PWR can be a power supply only
> > for an output port (looking at the dp-pwr-supply property). Also, the
> > driver for this binding is a DRM bridge driver (display-connector.c)
> > which would not be compatible with a v4l2 (sub)device.
>
> So then we should add 'dp-input-connector' because they are different.
> When we haven't defined connectors, properties of the connector have
> been shoved in whatever node is associated with a connector like you
> have done. That works for a while, but then becomes unmanageable. DP on
> USB-C connectors for example.
>
> OTOH, maybe your use here is niche enough to not be worth the trouble.
> Depends if we see the need for video input connectors in general.

My use case is a dedicated hardware that runs DP tests of an external
DUT. I can't think of another scenario where we'd need an input DP
port. IMO this is pretty niche, but I'll leave the decision to you
  

On Thu, Feb 29, 2024 at 11:25:41AM +0100, Paweł Anikiel wrote:
> On Wed, Feb 28, 2024 at 7:10 PM Rob Herring <robh@kernel.org> wrote:
> >
> > On Wed, Feb 28, 2024 at 02:09:33PM +0100, Paweł Anikiel wrote:
> > > On Wed, Feb 28, 2024 at 1:18 PM Krzysztof Kozlowski
> > > <krzysztof.kozlowski@linaro.org> wrote:
> > > >
> > > > On 28/02/2024 12:05, Paweł Anikiel wrote:
> > > > > On Tue, Feb 27, 2024 at 3:29 PM Rob Herring <robh@kernel.org> wrote:
> > > > >>
> > > > >> On Mon, Feb 26, 2024 at 11:59:42AM +0100, Paweł Anikiel wrote:
> > > > >>> On Mon, Feb 26, 2024 at 10:13 AM Krzysztof Kozlowski
> > > > >>> <krzysztof.kozlowski@linaro.org> wrote:
> > > > >>>>
> > > > >>>> On 21/02/2024 17:02, Paweł Anikiel wrote:
> > > > >>>>> The Intel Displayport RX IP is a part of the DisplayPort Intel FPGA IP
> > > > >>>>> Core. It implements a DisplayPort 1.4 receiver capable of HBR3 video
> > > > >>>>> capture and Multi-Stream Transport. The user guide can be found here:
> > > > >>>>>
> > > > >>>>> https://www.intel.com/programmable/technical-pdfs/683273.pdf
> > > > >>>>>
> > > > >>>>> Signed-off-by: Paweł Anikiel <panikiel@google.com>
> > > > >>>>> ---
> > > > >>>>>  .../devicetree/bindings/media/intel,dprx.yaml | 160 ++++++++++++++++++
> > > > >>>>>  1 file changed, 160 insertions(+)
> > > > >>>>>  create mode 100644 Documentation/devicetree/bindings/media/intel,dprx.yaml
> > > > >>>>>
> > > > >>>>> diff --git a/Documentation/devicetree/bindings/media/intel,dprx.yaml b/Documentation/devicetree/bindings/media/intel,dprx.yaml
> > > > >>>>> new file mode 100644
> > > > >>>>> index 000000000000..31025f2d5dcd
> > > > >>>>> --- /dev/null
> > > > >>>>> +++ b/Documentation/devicetree/bindings/media/intel,dprx.yaml
> > > > >>>>> @@ -0,0 +1,160 @@
> > > > >>>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > > > >>>>> +%YAML 1.2
> > > > >>>>> +---
> > > > >>>>> +$id: http://devicetree.org/schemas/media/intel,dprx.yaml#
> > > > >>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > >>>>> +
> > > > >>>>> +title: Intel DisplayPort RX IP
> > > > >>>>> +
> > > > >>>>> +maintainers:
> > > > >>>>> +  - Paweł Anikiel <panikiel@google.com>
> > > > >>>>> +
> > > > >>>>> +description: |
> > > > >>>>> +  The Intel Displayport RX IP is a part of the DisplayPort Intel FPGA IP
> > > > >>>>> +  Core. It implements a DisplayPort 1.4 receiver capable of HBR3 video
> > > > >>>>> +  capture and Multi-Stream Transport.
> > > > >>>>> +
> > > > >>>>> +  The IP features a large number of configuration parameters, found at:
> > > > >>>>> +  https://www.intel.com/content/www/us/en/docs/programmable/683273/23-3-20-0-1/sink-parameters.html
> > > > >>>>> +
> > > > >>>>> +  The following parameters have to be enabled:
> > > > >>>>> +    - Support DisplayPort sink
> > > > >>>>> +    - Enable GPU control
> > > > >>>>> +  The following parameters' values have to be set in the devicetree:
> > > > >>>>> +    - RX maximum link rate
> > > > >>>>> +    - Maximum lane count
> > > > >>>>> +    - Support MST
> > > > >>>>> +    - Max stream count (only if Support MST is enabled)
> > > > >>>>> +
> > > > >>>>> +properties:
> > > > >>>>> +  compatible:
> > > > >>>>> +    const: intel,dprx-20.0.1
> > > > >>>>> +
> > > > >>>>> +  reg:
> > > > >>>>> +    maxItems: 1
> > > > >>>>> +
> > > > >>>>> +  interrupts:
> > > > >>>>> +    maxItems: 1
> > > > >>>>> +
> > > > >>>>> +  intel,max-link-rate:
> > > > >>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
> > > > >>>>> +    description: Max link rate configuration parameter
> > > > >>>>
> > > > >>>> Please do not duplicate property name in description. It's useless.
> > > > >>>> Instead explain what is this responsible for.
> > > > >>>>
> > > > >>>> Why max-link-rate would differ for the same dprx-20.0.1? And why
> > > > >>>> standard properties cannot be used?
> > > > >>>>
> > > > >>>> Same for all questions below.
> > > > >>>
> > > > >>> These four properties are the IP configuration parameters mentioned in
> > > > >>> the device description. When generating the IP core you can set these
> > > > >>> parameters, which could make them differ for the same dprx-20.0.1.
> > > > >>> They are documented in the user guide, for which I also put a link in
> > > > >>> the description. Is that enough? Or should I also document these
> > > > >>> parameters here?
> > > > >>
> > > > >> Use the standard properties: link-frequencies and data-lanes. Those go
> > > > >> under the port(s) because they are inheritly per logical link.
> > > > >
> > > > > The DP receiver has one input interface (a deserialized DP stream),
> > > > > and up to four output interfaces (the decoded video streams). The "max
> > > > > link rate" and "max lane count" parameters only describe the input
> > > > > interface to the receiver. However, the port(s) I am using here are
> > > > > for the output streams. They are not affected by those parameters, so
> > > > > I don't think these properties should go under the output port(s).
> > > > >
> > > > > The receiver doesn't have an input port in the DT, because there isn't
> > > > > any controllable entity on the other side - the deserializer doesn't
> > > > > have any software interface. Since these standard properties
> > > > > (link-frequencies and data-lanes) are only defined in
> > > > > video-interfaces.yaml (which IIUC describes a graph endpoint), I can't
> > > > > use them directly in the device node.
> > > >
> > > > DT describes the hardware, so where does the input come? From something,
> > > > right? Regardless if you have a driver or not. There is dp-connector
> > > > binding, if this is physical port.
> > >
> > > Yes, it is a physical port. I agree adding a DT node for the physical
> > > DP input connector would let us add link-frequencies to the input port
> > > of the receiver.
> > >
> > > However, dp-connector seems to be a binding for an output port - it's
> > > under schemas/display/connector, and DP_PWR can be a power supply only
> > > for an output port (looking at the dp-pwr-supply property). Also, the
> > > driver for this binding is a DRM bridge driver (display-connector.c)
> > > which would not be compatible with a v4l2 (sub)device.
> >
> > So then we should add 'dp-input-connector' because they are different.
> > When we haven't defined connectors, properties of the connector have
> > been shoved in whatever node is associated with a connector like you
> > have done. That works for a while, but then becomes unmanageable. DP on
> > USB-C connectors for example.
> >
> > OTOH, maybe your use here is niche enough to not be worth the trouble.
> > Depends if we see the need for video input connectors in general.
> 
> My use case is a dedicated hardware that runs DP tests of an external
> DUT. I can't think of another scenario where we'd need an input DP
> port. IMO this is pretty niche, but I'll leave the decision to you

Your device is niche, but a video capture/in device is not that niche. 
After all, Smart TVs run Linux. They have video in connectors. Maybe not 
DP though. It's conceivable someone could make a "Smart Monitor" I 
suppose.

Rob