Re: [PATCH v2 9/9] ARM: dts: chameleonv3: Add video device nodes

From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
To: "Paweł Anikiel" <panikiel@google.com>
Cc: airlied@gmail.com, akpm@linux-foundation.org,
	conor+dt@kernel.org, daniel@ffwll.ch, dinguyen@kernel.org,
	hverkuil-cisco@xs4all.nl, krzysztof.kozlowski+dt@linaro.org,
	maarten.lankhorst@linux.intel.com, mchehab@kernel.org,
	mripard@kernel.org, robh+dt@kernel.org, tzimmermann@suse.de,
	devicetree@vger.kernel.org, dri-devel@lists.freedesktop.org,
	linux-kernel@vger.kernel.org, linux-media@vger.kernel.org,
	chromeos-krk-upstreaming@google.com, ribalda@chromium.org
Subject: Re: [PATCH v2 9/9] ARM: dts: chameleonv3: Add video device nodes
Date: Mon, 26 Feb 2024 18:30:16 +0100	[thread overview]
Message-ID: <e210b318-dcd7-4c0e-b08e-e1c4da1a8cd9@linaro.org> (raw)
In-Reply-To: <CAM5zL5pz0K5ro4-UjiYojM4h9Lqo_af5ZmH1FoZ_ajde_3+Dcg@mail.gmail.com>

On 26/02/2024 13:27, Paweł Anikiel wrote:
> On Mon, Feb 26, 2024 at 1:07 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 26/02/2024 12:09, Paweł Anikiel wrote:
>>> On Mon, Feb 26, 2024 at 10:15 AM Krzysztof Kozlowski
>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>>
>>>> On 21/02/2024 17:02, Paweł Anikiel wrote:
>>>>> Add device nodes for the video system present on the Chameleon v3.
>>>>> It consists of six framebuffers and two Intel Displayport receivers.
>>>>>
>>>>> Signed-off-by: Paweł Anikiel <panikiel@google.com>
>>>>> ---
>>>>
>>>> ...
>>>>
>>>>> +             dprx_sst: dp-receiver@c0064000 {
>>>>> +                     compatible = "intel,dprx-20.0.1";
>>>>> +                     reg = <0xc0064000 0x800>;
>>>>> +                     interrupt-parent = <&dprx_sst_irq>;
>>>>> +                     interrupts = <0 IRQ_TYPE_EDGE_RISING>;
>>>>> +                     intel,max-link-rate = <0x1e>;
>>>>
>>>> Rate is not in hex! Rate is in Hz, at least usually...
>>>>
>>>> Fix your bindings...
>>>
>>> This is the DisplayPort link rate, for which the allowed values are
>>> 8.1 Gbps, 5.4 Gbps, 2.7 Gbps, or 1.62 Gbps. The standard way to encode
>>> them (used in the DisplayPort DPCD registers and this device's
>>
>> Then it is in bps or some other units:
>>
>> https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml
>>
>>> configuration) is by multiples of 0.27Gbps. This value (AFAIK) is
>>> usually represented in hex, so 8.1Gbps would be 0x1e.
>>
>> No, the value is represented in logical units. Frequency in Hz. Rate in
>> bps/kbps/etc. Voltage in volts.
> 
> Okay, thanks for the info. So if I understand correctly, the max link
> rate should be represented in bps in the devicetree, and then be

or kbps

> converted to the per 0.27Gbps value by the driver?

If driver needs some register-based value, then yes.

> 
> One problem is that the values here are too large to be represented in
> bps (since the datatype is uint32). Can the property be in Mbps
> instead?

Can be. You can submit a patch to dtschema (patch to DT spec list or
github pull request) adding '-mbps' as well.

Best regards,
Krzysztof