| Message ID | 20220625232513.522599-9-dmitry.baryshkov@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | dt-bindings: display/msm: rework MDSS and DPU bindings | expand |
On 27 June 2022 21:05:06 GMT+03:00, Rob Herring <robh@kernel.org> wrote: >On Sun, Jun 26, 2022 at 02:25:10AM +0300, Dmitry Baryshkov wrote: >> On SDM845 platforms DPU device tree node contains child object >> mdp-opp-table providing OPP table for the DPU. Add it to the list of >> properties to let sdm845.dtsi to validate. >> >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >> --- >> .../devicetree/bindings/display/msm/dpu-sdm845.yaml | 9 +++++++-- >> 1 file changed, 7 insertions(+), 2 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml b/Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml >> index 0dc16326bf8e..cc95adcf8f11 100644 >> --- a/Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml >> +++ b/Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml >> @@ -50,6 +50,10 @@ properties: >> maxItems: 1 >> >> operating-points-v2: true >> + >> + mdp-opp-table: > >Is there another kind of opp-table besides mdp? Node names should be >generic. Ack. Now if we rename the node to opp-table, should we define the old name with deprecated: true? > >> + $ref: /schemas/opp/opp-v2.yaml# >> + >> ports: >> $ref: /schemas/graph.yaml#/properties/ports >> description: | >> @@ -116,11 +120,12 @@ examples: >> <0x0aeb0000 0x2008>; >> reg-names = "mdp", "vbif"; >> >> - clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>, >> + clocks = <&gcc GCC_DISP_AXI_CLK>, >> + <&dispcc DISP_CC_MDSS_AHB_CLK>, > >What does the OPP table have to do with clocks? Adding a clock anywhere >but the end is an ABI break. I should split this to a separate patch. And, I must admit, this clock change has already landed. We did not think that it is an ABI break since we have clock-names here. > >> <&dispcc DISP_CC_MDSS_AXI_CLK>, >> <&dispcc DISP_CC_MDSS_MDP_CLK>, >> <&dispcc DISP_CC_MDSS_VSYNC_CLK>; >> - clock-names = "iface", "bus", "core", "vsync"; >> + clock-names = "gcc-bus", "iface", "bus", "core", "vsync"; >> >> interrupt-parent = <&mdss>; >> interrupts = <0>;
On 28/06/2022 22:23, Dmitry Baryshkov wrote: >>> + $ref: /schemas/opp/opp-v2.yaml# >>> + >>> ports: >>> $ref: /schemas/graph.yaml#/properties/ports >>> description: | >>> @@ -116,11 +120,12 @@ examples: >>> <0x0aeb0000 0x2008>; >>> reg-names = "mdp", "vbif"; >>> >>> - clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>, >>> + clocks = <&gcc GCC_DISP_AXI_CLK>, >>> + <&dispcc DISP_CC_MDSS_AHB_CLK>, >> >> What does the OPP table have to do with clocks? Adding a clock anywhere >> but the end is an ABI break. > > I should split this to a separate patch. And, I must admit, this clock change has already landed. We did not think that it is an ABI break since we have clock-names here. xxx-names are only a helper and order of items is always strict, thus any change in the order is always ABI break. Best regards, Krzysztof
On 29 June 2022 12:23:48 GMT+03:00, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: >On 28/06/2022 22:23, Dmitry Baryshkov wrote: > >>>> + $ref: /schemas/opp/opp-v2.yaml# >>>> + >>>> ports: >>>> $ref: /schemas/graph.yaml#/properties/ports >>>> description: | >>>> @@ -116,11 +120,12 @@ examples: >>>> <0x0aeb0000 0x2008>; >>>> reg-names = "mdp", "vbif"; >>>> >>>> - clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>, >>>> + clocks = <&gcc GCC_DISP_AXI_CLK>, >>>> + <&dispcc DISP_CC_MDSS_AHB_CLK>, >>> >>> What does the OPP table have to do with clocks? Adding a clock anywhere >>> but the end is an ABI break. >> >> I should split this to a separate patch. And, I must admit, this clock change has already landed. We did not think that it is an ABI break since we have clock-names here. > >xxx-names are only a helper and order of items is always strict, thus >any change in the order is always ABI break. Ack, we will keep this in mind. However in this case we have already made this change. So the question in his do we cope with it.
diff --git a/Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml b/Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml index 0dc16326bf8e..cc95adcf8f11 100644 --- a/Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml +++ b/Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml @@ -50,6 +50,10 @@ properties: maxItems: 1 operating-points-v2: true + + mdp-opp-table: + $ref: /schemas/opp/opp-v2.yaml# + ports: $ref: /schemas/graph.yaml#/properties/ports description: | @@ -116,11 +120,12 @@ examples: <0x0aeb0000 0x2008>; reg-names = "mdp", "vbif"; - clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>, + clocks = <&gcc GCC_DISP_AXI_CLK>, + <&dispcc DISP_CC_MDSS_AHB_CLK>, <&dispcc DISP_CC_MDSS_AXI_CLK>, <&dispcc DISP_CC_MDSS_MDP_CLK>, <&dispcc DISP_CC_MDSS_VSYNC_CLK>; - clock-names = "iface", "bus", "core", "vsync"; + clock-names = "gcc-bus", "iface", "bus", "core", "vsync"; interrupt-parent = <&mdss>; interrupts = <0>;
On SDM845 platforms DPU device tree node contains child object mdp-opp-table providing OPP table for the DPU. Add it to the list of properties to let sdm845.dtsi to validate. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- .../devicetree/bindings/display/msm/dpu-sdm845.yaml | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)