Re: [PATCH v2 1/4] dt-bindings: display/msm: add core clock to the mdss bindings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
---
   .../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-common

The 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@xxxxxxxxx/
https://lore.kernel.org/all/09ed16e1-4af2-8fce-dab4-f6c0f09e688c@xxxxxxxxxx/

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


--
With best wishes
Dmitry




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux