| Message ID | 1669852310-22360-2-git-send-email-quic_khsieh@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add data-lanes and link-frequencies to dp_out endpoint | expand |
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@quicinc.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 > + 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 > }; > }; > }; > 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 {
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@quicinc.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. > >> }; >> }; >> }; >> 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 { >
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@quicinc.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). > > >> >>> }; >>> }; >>> }; >>> 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 { >> >
On 11/30/2022 4:07 PM, 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@quicinc.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? there is no 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 >> + 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 > >> }; >> }; >> }; >> 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 { >
On 01/12/2022 19:34, Kuogee Hsieh wrote: > > On 11/30/2022 4:07 PM, 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@quicinc.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? > there is no old data-lanes property. There is one: $ grep -n data-lanes Documentation/devicetree/bindings/display/msm/dp-controller.yaml 82: data-lanes:
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@quicinc.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 { >>> >>
On 12/1/2022 9:49 AM, Dmitry Baryshkov wrote: > 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@quicinc.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. ok, in the case, we can specify link frequency (symbol rate), such as 81000000000 (8.1G hz), at dtsi to match link-frequencies cocumentation. then at parser, we have to divided by 10 to convert back to link rate and then divided by 1000 to convert to kb. is this work for you? > > >>>>> }; >>>>> }; >>>>> }; >>>>> 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 { >>>> >>> >
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 + + required: + - link-frequencies + - data-lanes + + additionalProperties: false + required: - compatible - reg @@ -158,6 +172,9 @@ examples: reg = <1>; endpoint { remote-endpoint = <&typec>; + data-lanes = <1 2>; + link-frequencies = /bits/ 64 <160000000 270000000 + 540000000 810000000>; }; }; }; 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>; }; &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>; }; &mdss_mdp {
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@quicinc.com> --- .../devicetree/bindings/display/msm/dp-controller.yaml | 17 +++++++++++++++++ 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(-)