| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 1 Dec 2022 19:49:56 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Kuogee Hsieh <quic_khsieh@...cinc.com>,
dri-devel@...ts.freedesktop.org, robdclark@...il.com,
sean@...rly.run, swboyd@...omium.org, dianders@...omium.org,
vkoul@...nel.org, daniel@...ll.ch, airlied@...ux.ie,
agross@...nel.org, bjorn.andersson@...aro.org
Cc: quic_abhinavk@...cinc.com, quic_sbillaka@...cinc.com,
freedreno@...ts.freedesktop.org, linux-arm-msm@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6 1/4] arm64: dts: qcom: add data-lanes and
link-freuencies into dp_out endpoint
On 01/12/2022 19:32, Kuogee Hsieh wrote:
>
> On 11/30/2022 4:21 PM, Dmitry Baryshkov wrote:
>> On 01/12/2022 02:07, Dmitry Baryshkov wrote:
>>> On 01/12/2022 01:51, Kuogee Hsieh wrote:
>>>> Move data-lanes property from mdss_dp node to dp_out endpoint. Also
>>>> add link-frequencies property into dp_out endpoint as well. The last
>>>> frequency specified at link-frequencies will be the max link rate
>>>> supported by DP.
>>>>
>>>> Changes in v5:
>>>> -- revert changes at sc7180.dtsi and sc7280.dtsi
>>>> -- add &dp_out to sc7180-trogdor.dtsi and sc7280-herobrine.dtsi
>>>>
>>>> Changes in v6:
>>>> -- add data-lanes and link-frequencies to yaml
>>>>
>>>> Signed-off-by: Kuogee Hsieh <quic_khsieh@...cinc.com>
>>>> ---
>>>> .../devicetree/bindings/display/msm/dp-controller.yaml | 17
>>>> +++++++++++++++++
>>>
>>> Separate patch. Also you didn't check the get_maintainers output, so
>>> required parties were not included into the distribution.
>>>
>>> Also as you'd check the get_maintainers output, please fix other
>>> email addresses too.
>>>
>>>> arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 6 +++++-
>>>> arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi | 6 +++++-
>>>> 3 files changed, 27 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git
>>>> a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
>>>> b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
>>>> index 94bc6e1..af70343 100644
>>>> --- a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
>>>> +++ b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
>>>> @@ -90,6 +90,20 @@ properties:
>>>> $ref: /schemas/graph.yaml#/properties/port
>>>> description: Output endpoint of the controller
>>>> + properties:
>>>> + endpoint:
>>>> + $ref: /schemas/media/video-interfaces.yaml#
>>>> +
>>>> + properties:
>>>> + link-frequencies: true
>>>> + data-lanes: true
>>>
>>> No. Use $ref for both of them.
>>>
>>>> +
>>>> + required:
>>>> + - link-frequencies
>>>> + - data-lanes
>>>
>>> No, they are not required.
>>>
>>>> +
>>>> + additionalProperties: false
>>>> +
>>>
>>> deprecation of old data-lanes property?
>>>
>>>> required:
>>>> - compatible
>>>> - reg
>>>> @@ -158,6 +172,9 @@ examples:
>>>> reg = <1>;
>>>> endpoint {
>>>> remote-endpoint = <&typec>;
>>>> + data-lanes = <1 2>;
>>>> + link-frequencies = /bits/ 64 <160000000 270000000
>>
>> s/1600/1620
>>
>>>> + 540000000 810000000>;
>>>
>>> I guess the number of zeroes is wrong here. This is 160 MHz ... 810
>>> Mhz, rather than 1.6 GHz ... 8.1 GHz
>>
>> Ok, I was wrong here. The old code definitely defaults to 570
>> mega-something. Now I'd really like to read your description for the
>> link-frequencies property, because the
>> phy_configure_opts_dp::link_rate is clearly specified in Mb/s and it
>> takes a fixed set of values from 1.62 Gb/s up to 8.1 Gb/s.
>>
>> I think the drm_dp_bw_code_to_link_rate() function is incorrect by
>> itself, as it multiplies with 27000 (27 Mbps) rather than 270000 (0.27
>> Gbps) as required by the standard. So first, we should fix the
>> function, then all the rates would become logical.
>
> no, drm_dp_bw_code_to_link_rate() is correct and should not be changes
> since it impact to other dp drivers too.
>
> 0.27Gbps/lane is specified at DP spec.
>
> DP use 8b/10b coding rule (10 bits symbol contains 8 bits data).
At least it should get documentation that it returns Kylo-bytes per second.
But, getting back to link-frequencies. The documentation clearly says
that it should be allowed data bus _frequencies_. And frequencies are
measured in Hz, not in bits/sec or bytes/sec.
>>>> };
>>>> };
>>>> };
>>>> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
>>>> b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
>>>> index 754d2d6..39f0844 100644
>>>> --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
>>>> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
>>>> @@ -812,7 +812,11 @@ hp_i2c: &i2c9 {
>>>> status = "okay";
>>>> pinctrl-names = "default";
>>>> pinctrl-0 = <&dp_hot_plug_det>;
>>>> - data-lanes = <0 1>;
>>>> +};
>>>> +
>>>> +&dp_out {
>>>> + data-lanes = <0 1>;
>>>> + link-frequencies = /bits/ 64 <160000000 270000000 540000000>;
>>>
>>> Same comment here.
>>>
>>>> };
>>>> &pm6150_adc {
>>>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
>>>> b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
>>>> index 93e39fc..b7c343d 100644
>>>> --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
>>>> +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
>>>> @@ -440,7 +440,11 @@ ap_i2c_tpm: &i2c14 {
>>>> status = "okay";
>>>> pinctrl-names = "default";
>>>> pinctrl-0 = <&dp_hot_plug_det>;
>>>> - data-lanes = <0 1>;
>>>> +};
>>>> +
>>>> +&dp_out {
>>>> + data-lanes = <0 1>;
>>>> + link-frequencies = /bits/ 64 <160000000 270000000 540000000
>>>> 810000000>;
>>>
>>> And here.
>>>
>>>> };
>>>> &mdss_mdp {
>>>
>>
--
With best wishes
Dmitry
Powered by blists - more mailing lists