[v2,1/4] dt-bindings: display/msm: add core clock to the mdss bindings (2024)

Add (optional) core clock to the mdss bindings to let the MDSS driveraccess harware registers before MDP driver probes.Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>--- .../bindings/display/msm/qcom,mdss.yaml | 34 ++++++++++++++----- 1 file changed, 26 insertions(+), 8 deletions(-)

On 09/01/2023 01:52, Dmitry Baryshkov wrote:> Add (optional) core clock to the mdss bindings to let the MDSS driver> access harware registers before MDP driver probes.> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>> ---> .../bindings/display/msm/qcom,mdss.yaml | 34 ++++++++++++++-----> 1 file changed, 26 insertions(+), 8 deletions(-)> > diff --git a/Documentation/devicetree/bindings/display/msm/qcom,mdss.yaml b/Documentation/devicetree/bindings/display/msm/qcom,mdss.yaml> index ba0460268731..0647fc5a7d94 100644> --- a/Documentation/devicetree/bindings/display/msm/qcom,mdss.yaml> +++ b/Documentation/devicetree/bindings/display/msm/qcom,mdss.yaml> @@ -45,17 +45,11 @@ properties:> > clocks:> minItems: 1> - items:> - - description: Display abh clock> - - description: Display axi clockNot related to this patch, but it is a bit surprising to see AXI clockoptional.> - - description: Display vsync clock> + maxItems: 4> > clock-names:> minItems: 1> - items:> - - const: iface> - - const: bus> - - const: vsync> + maxItems: 4> > "#address-cells":> const: 1> @@ -69,6 +63,30 @@ properties:> items:> - description: MDSS_CORE reset> > +oneOf:> + - properties:> + clocks:> + minItems: 3> + maxItems: 4> +> + clock-names:> + minItems: 3> + items:> + - const: iface> + - const: busBTW, sc7180-mdss uses here ahb name and calls it "AHB clock from dispcc".SM8250 won't match here either. Maybe this should be reworked to specifylimits here but not the names and actual clocks? IOW, drop entire oneOf?There were a lot, a lot of changes to MDSS/DPU bindings recently, so Iam really loosing track what is done where and when.There are also few separate patchsets from you on the lists. Could theybe combined into one cleanup?I understand that sometimes new cleanup is needed after old cleanupfinished (I had the same with pinctrl), so it is not a complain.Another problem (and this time I complain) is that several of yourpatchsets were sent, discussed and then without any notice applied. Nomessage that a patchset was applied to some tree. Look:https://lore.kernel.org/all/20221124001708.25720-2-a39.skl@gmail.com/https://lore.kernel.org/all/09ed16e1-4af2-8fce-dab4-f6c0f09e688c@linaro.org/Nothing. Silent application. If you are the maintainer which picks upthe patch, please always, always send message that they are applied.Patchwork does it automatically, b4 can do it easily as well. If you useother tools - use other tools for sending it. Otherwise things arediscussed on mailing lists, receive several comments and there is nevera resubmit but instead they show in the tree.Best regards,Krzysztof

On 09/01/2023 12:35, Krzysztof Kozlowski wrote:> On 09/01/2023 01:52, Dmitry Baryshkov wrote:>> Add (optional) core clock to the mdss bindings to let the MDSS driver>> access harware registers before MDP driver probes.>>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>>> --->> .../bindings/display/msm/qcom,mdss.yaml | 34 ++++++++++++++----->> 1 file changed, 26 insertions(+), 8 deletions(-)>>>> diff --git a/Documentation/devicetree/bindings/display/msm/qcom,mdss.yaml b/Documentation/devicetree/bindings/display/msm/qcom,mdss.yaml>> index ba0460268731..0647fc5a7d94 100644>> --- a/Documentation/devicetree/bindings/display/msm/qcom,mdss.yaml>> +++ b/Documentation/devicetree/bindings/display/msm/qcom,mdss.yaml>> @@ -45,17 +45,11 @@ properties:>> >> clocks:>> minItems: 1>> - items:>> - - description: Display abh clock>> - - description: Display axi clock> > Not related to this patch, but it is a bit surprising to see AXI clock> optional.Hmm, There is one defined downstream. Probably we should fix that (but yes, it's a separate issue).>> - - description: Display vsync clock>> + maxItems: 4>> >> clock-names:>> minItems: 1>> - items:>> - - const: iface>> - - const: bus>> - - const: vsync>> + maxItems: 4>> >> "#address-cells":>> const: 1>> @@ -69,6 +63,30 @@ properties:>> items:>> - description: MDSS_CORE reset>> >> +oneOf:>> + - properties:>> + clocks:>> + minItems: 3>> + maxItems: 4>> +>> + clock-names:>> + minItems: 3>> + items:>> + - const: iface>> + - const: bus> > BTW, sc7180-mdss uses here ahb name and calls it "AHB clock from dispcc".> > SM8250 won't match here either. Maybe this should be reworked to specify> limits here but not the names and actual clocks? IOW, drop entire oneOf?SC7180 and SM8250 use platform-specific bindings (qcom,sc7180-mdss.yaml and qcom,sm8250-mdss.yaml). This file is used only for older platforms (msm8916, msm8996, etc).> > There were a lot, a lot of changes to MDSS/DPU bindings recently, so I> am really loosing track what is done where and when.> > There are also few separate patchsets from you on the lists. Could they> be combined into one cleanup?Ack, I'll merge them into a single patchset.> I understand that sometimes new cleanup is needed after old cleanup> finished (I had the same with pinctrl), so it is not a complain.> > Another problem (and this time I complain) is that several of your> patchsets were sent, discussed and then without any notice applied. No> message that a patchset was applied to some tree. Look:> > https://lore.kernel.org/all/20221124001708.25720-2-a39.skl@gmail.com/> https://lore.kernel.org/all/09ed16e1-4af2-8fce-dab4-f6c0f09e688c@linaro.org/> > Nothing. Silent application. If you are the maintainer which picks up> the patch, please always, always send message that they are applied.> Patchwork does it automatically, b4 can do it easily as well. If you use> other tools - use other tools for sending it. Otherwise things are> discussed on mailing lists, receive several comments and there is never> a resubmit but instead they show in the tree.Unfortunately freedreno uses patchwork-fdo, which doesn't send notifications. And the fdo fork is not supported by b4. I checked what would be necessary to enable support in b4. Unfortunately several API changes would be necessary. So this is a long process. But we are open to any suggestions on how to improve the process. Currently all three maintainers (Rob, Abhinav and me) keep the patch status in the patchwork, but that's all.

On 09/01/2023 11:51, Dmitry Baryshkov wrote:> On 09/01/2023 12:35, Krzysztof Kozlowski wrote:>> On 09/01/2023 01:52, Dmitry Baryshkov wrote:>>> Add (optional) core clock to the mdss bindings to let the MDSS driver>>> access harware registers before MDP driver probes.>>>>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>>>> --->>> .../bindings/display/msm/qcom,mdss.yaml | 34 ++++++++++++++----->>> 1 file changed, 26 insertions(+), 8 deletions(-)>>>>>> diff --git a/Documentation/devicetree/bindings/display/msm/qcom,mdss.yaml b/Documentation/devicetree/bindings/display/msm/qcom,mdss.yaml>>> index ba0460268731..0647fc5a7d94 100644>>> --- a/Documentation/devicetree/bindings/display/msm/qcom,mdss.yaml>>> +++ b/Documentation/devicetree/bindings/display/msm/qcom,mdss.yaml>>> @@ -45,17 +45,11 @@ properties:>>> >>> clocks:>>> minItems: 1>>> - items:>>> - - description: Display abh clock>>> - - description: Display axi clock>>>> Not related to this patch, but it is a bit surprising to see AXI clock>> optional.> > Hmm, There is one defined downstream. Probably we should fix that (but > yes, it's a separate issue).> >>> - - description: Display vsync clock>>> + maxItems: 4>>> >>> clock-names:>>> minItems: 1>>> - items:>>> - - const: iface>>> - - const: bus>>> - - const: vsync>>> + maxItems: 4>>> >>> "#address-cells":>>> const: 1>>> @@ -69,6 +63,30 @@ properties:>>> items:>>> - description: MDSS_CORE reset>>> >>> +oneOf:>>> + - properties:>>> + clocks:>>> + minItems: 3>>> + maxItems: 4>>> +>>> + clock-names:>>> + minItems: 3>>> + items:>>> + - const: iface>>> + - const: bus>>>> BTW, sc7180-mdss uses here ahb name and calls it "AHB clock from dispcc".>>>> SM8250 won't match here either. Maybe this should be reworked to specify>> limits here but not the names and actual clocks? IOW, drop entire oneOf?> > SC7180 and SM8250 use platform-specific bindings (qcom,sc7180-mdss.yaml > and qcom,sm8250-mdss.yaml). This file is used only for older platforms > (msm8916, msm8996, etc).Ah, right. It's a bit confusing to have bindings split into files:1. mdss-common2. mdss3. device specificbut I guess fixing this would be another chunk of work.> >>>> There were a lot, a lot of changes to MDSS/DPU bindings recently, so I>> am really loosing track what is done where and when.>>>> There are also few separate patchsets from you on the lists. Could they>> be combined into one cleanup?> > Ack, I'll merge them into a single patchset.> >> I understand that sometimes new cleanup is needed after old cleanup>> finished (I had the same with pinctrl), so it is not a complain.>>>> Another problem (and this time I complain) is that several of your>> patchsets were sent, discussed and then without any notice applied. No>> message that a patchset was applied to some tree. Look:>>>> https://lore.kernel.org/all/20221124001708.25720-2-a39.skl@gmail.com/>> https://lore.kernel.org/all/09ed16e1-4af2-8fce-dab4-f6c0f09e688c@linaro.org/>>>> Nothing. Silent application. If you are the maintainer which picks up>> the patch, please always, always send message that they are applied.>> Patchwork does it automatically, b4 can do it easily as well. If you use>> other tools - use other tools for sending it. Otherwise things are>> discussed on mailing lists, receive several comments and there is never>> a resubmit but instead they show in the tree.> > Unfortunately freedreno uses patchwork-fdo, which doesn't send > notifications. And the fdo fork is not supported by b4. I checked what > would be necessary to enable support in b4. Unfortunately several API > changes would be necessary. So this is a long process. But we are open > to any suggestions on how to improve the process. Currently all three > maintainers (Rob, Abhinav and me) keep the patch status in the > patchwork, but that's all.And how other freedesktop.org patchwork users notify? Manually or isthere some hook? I notice only Exynos DRM where maintainer sends manual"Applied" messages.Best regards,Krzysztof

On 09/01/2023 13:20, Krzysztof Kozlowski wrote:> On 09/01/2023 11:51, Dmitry Baryshkov wrote:>> On 09/01/2023 12:35, Krzysztof Kozlowski wrote:>>> On 09/01/2023 01:52, Dmitry Baryshkov wrote:>>>> Add (optional) core clock to the mdss bindings to let the MDSS driver>>>> access harware registers before MDP driver probes.>>>>>>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>>>>> --->>>> .../bindings/display/msm/qcom,mdss.yaml | 34 ++++++++++++++----->>>> 1 file changed, 26 insertions(+), 8 deletions(-)>>>>>>>> diff --git a/Documentation/devicetree/bindings/display/msm/qcom,mdss.yaml b/Documentation/devicetree/bindings/display/msm/qcom,mdss.yaml>>>> index ba0460268731..0647fc5a7d94 100644>>>> --- a/Documentation/devicetree/bindings/display/msm/qcom,mdss.yaml>>>> +++ b/Documentation/devicetree/bindings/display/msm/qcom,mdss.yaml>>>> @@ -45,17 +45,11 @@ properties:>>>> >>>> clocks:>>>> minItems: 1>>>> - items:>>>> - - description: Display abh clock>>>> - - description: Display axi clock>>>>>> Not related to this patch, but it is a bit surprising to see AXI clock>>> optional.>>>> Hmm, There is one defined downstream. Probably we should fix that (but>> yes, it's a separate issue).>>>>>> - - description: Display vsync clock>>>> + maxItems: 4>>>> >>>> clock-names:>>>> minItems: 1>>>> - items:>>>> - - const: iface>>>> - - const: bus>>>> - - const: vsync>>>> + maxItems: 4>>>> >>>> "#address-cells":>>>> const: 1>>>> @@ -69,6 +63,30 @@ properties:>>>> items:>>>> - description: MDSS_CORE reset>>>> >>>> +oneOf:>>>> + - properties:>>>> + clocks:>>>> + minItems: 3>>>> + maxItems: 4>>>> +>>>> + clock-names:>>>> + minItems: 3>>>> + items:>>>> + - const: iface>>>> + - const: bus>>>>>> BTW, sc7180-mdss uses here ahb name and calls it "AHB clock from dispcc".>>>>>> SM8250 won't match here either. Maybe this should be reworked to specify>>> limits here but not the names and actual clocks? IOW, drop entire oneOf?>>>> SC7180 and SM8250 use platform-specific bindings (qcom,sc7180-mdss.yaml>> and qcom,sm8250-mdss.yaml). This file is used only for older platforms>> (msm8916, msm8996, etc).> > Ah, right. It's a bit confusing to have bindings split into files:> 1. mdss-common> 2. mdss> 3. device specific> > but I guess fixing this would be another chunk of work.It comes from the history of display devices on Qualcomm platforms. Older platforms used single compatible entry: qcom,mdss (and qcom,mdp5 for the corresponding MDP/DPU device). Then at the sdm845 point there was a change: per-SoC compatibles for both MDSS and DPU. But older devices still have the qcom,mdss compat string. Moreover this change also introduced a shift in the DT (some properties were moved from MDP to the MDSS device, e.g. interconnects and iommus). So the mdss-common lists common properties of new-style bindings, but it is not applicable to old platforms.Thus we ended up in a situation where we have:- qcom,mdss for old devices. Maybe it better be renamed to qcom,mdss-other?- qcom,SoC-mdss + mdss-commonThe same situation applies to the MDP/DPU:- qcom,mdp5- qcom,SoC-dpu + dpu-common> >>>>>>>> There were a lot, a lot of changes to MDSS/DPU bindings recently, so I>>> am really loosing track what is done where and when.>>>>>> There are also few separate patchsets from you on the lists. Could they>>> be combined into one cleanup?>>>> Ack, I'll merge them into a single patchset.>>>>> I understand that sometimes new cleanup is needed after old cleanup>>> finished (I had the same with pinctrl), so it is not a complain.>>>>>> Another problem (and this time I complain) is that several of your>>> patchsets were sent, discussed and then without any notice applied. No>>> message that a patchset was applied to some tree. Look:>>>>>> https://lore.kernel.org/all/20221124001708.25720-2-a39.skl@gmail.com/>>> https://lore.kernel.org/all/09ed16e1-4af2-8fce-dab4-f6c0f09e688c@linaro.org/>>>>>> Nothing. Silent application. If you are the maintainer which picks up>>> the patch, please always, always send message that they are applied.>>> Patchwork does it automatically, b4 can do it easily as well. If you use>>> other tools - use other tools for sending it. Otherwise things are>>> discussed on mailing lists, receive several comments and there is never>>> a resubmit but instead they show in the tree.>>>> Unfortunately freedreno uses patchwork-fdo, which doesn't send>> notifications. And the fdo fork is not supported by b4. I checked what>> would be necessary to enable support in b4. Unfortunately several API>> changes would be necessary. So this is a long process. But we are open>> to any suggestions on how to improve the process. Currently all three>> maintainers (Rob, Abhinav and me) keep the patch status in the>> patchwork, but that's all.> > And how other freedesktop.org patchwork users notify? Manually or is> there some hook? I notice only Exynos DRM where maintainer sends manual> "Applied" messages.I fear there are no other good options. I picked up maintenance practices from Rob, who doesn't send `applied' message. Maybe it's something to change.> > Best regards,> Krzysztof>

On 09/01/2023 12:51, Dmitry Baryshkov wrote:> On 09/01/2023 12:35, Krzysztof Kozlowski wrote:>> On 09/01/2023 01:52, Dmitry Baryshkov wrote:[skipped]>>>> There were a lot, a lot of changes to MDSS/DPU bindings recently, so I>> am really loosing track what is done where and when.>>>> There are also few separate patchsets from you on the lists. Could they>> be combined into one cleanup?> > Ack, I'll merge them into a single patchset.I gave it some thought... Putting small non-mdss/mdp fixes aside, there are three series, which I'm trying to track:- MDP5 schema conversion + per-SoC compatibles for MDP5 [1] no dependencies- mdss/mdp/dpu -> display-subsystem, display-controller rename [2] depends on [1]- mdp5 core clock support [3] depends on [1]I think I'll target on merging [1] and [2] first, since they are purely DT + schema changes. If you prefer I can squash them for the next iteration or I can keep them separate.For [3] I'll wait for first two to be in the proper shape, since it also contributes driver changes (and I don't want to have additional interlocks here).[1] https://lore.kernel.org/linux-arm-msm/20230109050152.316606-1-dmitry.baryshkov@linaro.org/[2] https://lore.kernel.org/linux-arm-msm/20230109051402.317577-1-dmitry.baryshkov@linaro.org/[3] https://lore.kernel.org/linux-arm-msm/20230109005209.247356-1-dmitry.baryshkov@linaro.org/

On Mon, 09 Jan 2023 02:52:06 +0200, Dmitry Baryshkov wrote:> Add (optional) core clock to the mdss bindings to let the MDSS driver> access harware registers before MDP driver probes.> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>> ---> .../bindings/display/msm/qcom,mdss.yaml | 34 ++++++++++++++-----> 1 file changed, 26 insertions(+), 8 deletions(-)> Running 'make dtbs_check' with the schema in this patch gives thefollowing warnings. Consider if they are expected or the schema isincorrect. These may not be new warnings.Note that it is not yet a requirement to have 0 warnings for dtbs_check.This will change in the future.Full log is available here: https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230109005209.247356-2-dmitry.baryshkov@linaro.orgmdss@1a00000: mdp@1a01000:compatible:0: 'qcom,mdp5' was expectedarch/arm64/boot/dts/qcom/sdm632-fairphone-fp3.dtbmdss@1a00000: mdp@1a01000:compatible: ['qcom,msm8953-mdp5', 'qcom,mdp5'] is too longarch/arm64/boot/dts/qcom/sdm632-fairphone-fp3.dtbmdss@900000: phy@9a0600:compatible:0: 'qcom,hdmi-phy-8996' is not one of ['qcom,dsi-phy-14nm', 'qcom,dsi-phy-14nm-660', 'qcom,dsi-phy-14nm-8953', 'qcom,dsi-phy-20nm', 'qcom,dsi-phy-28nm-hpm', 'qcom,dsi-phy-28nm-lp']arch/arm64/boot/dts/qcom/apq8096-db820c.dtbarch/arm64/boot/dts/qcom/apq8096-ifc6640.dtbarch/arm64/boot/dts/qcom/msm8996-mtp.dtbarch/arm64/boot/dts/qcom/msm8996-oneplus3.dtbarch/arm64/boot/dts/qcom/msm8996-oneplus3t.dtbarch/arm64/boot/dts/qcom/msm8996pro-xiaomi-natrium.dtbarch/arm64/boot/dts/qcom/msm8996pro-xiaomi-scorpio.dtbarch/arm64/boot/dts/qcom/msm8996-sony-xperia-tone-dora.dtbarch/arm64/boot/dts/qcom/msm8996-sony-xperia-tone-kagura.dtbarch/arm64/boot/dts/qcom/msm8996-sony-xperia-tone-keyaki.dtbarch/arm64/boot/dts/qcom/msm8996-xiaomi-gemini.dtb

Patch

diff --git a/Documentation/devicetree/bindings/display/msm/qcom,mdss.yaml b/Documentation/devicetree/bindings/display/msm/qcom,mdss.yamlindex ba0460268731..0647fc5a7d94 100644--- a/Documentation/devicetree/bindings/display/msm/qcom,mdss.yaml+++ b/Documentation/devicetree/bindings/display/msm/qcom,mdss.yaml@@ -45,17 +45,11 @@  properties: clocks: minItems: 1- items:- - description: Display abh clock- - description: Display axi clock- - description: Display vsync clock+ maxItems: 4 clock-names: minItems: 1- items:- - const: iface- - const: bus- - const: vsync+ maxItems: 4 "#address-cells": const: 1@@ -69,6 +63,30 @@  properties: items: - description: MDSS_CORE reset +oneOf:+ - properties:+ clocks:+ minItems: 3+ maxItems: 4++ clock-names:+ minItems: 3+ items:+ - const: iface+ - const: bus+ - const: vsync+ - const: core+ - properties:+ clocks:+ minItems: 1+ maxItems: 2++ clock-names:+ minItems: 1+ items:+ - const: iface+ - const: core+ required: - compatible - reg