[PATCH v2 0/4] drm/msm/mdss: enable access to hw_rev from MDSS driver

[PATCH v2 0/4] drm/msm/mdss: enable access to hw_rev from MDSS driver

[Dmitry Baryshkov]

On newer platforms mdss can read hw_rev register. On older platforms the
mdss driver can not read hw_rev register (to determine the platform).
Add optional 'core' clock that enables access to MDSS registers before
MDP5/DPU device is bound (and enabled the core clock).

Dmitry Baryshkov (4):
  dt-bindings: display/msm: add core clock to the mdss bindings
  drm/msm/mdss: enable optional core clock for MDP5 MDSS
  drm/msm/mdss: check for core clk before accessing HW_REV
  drm/msm/mdss: move is_mdp5 condition to msm_mdss_init

 .../bindings/display/msm/qcom,mdss.yaml       | 34 ++++++++++++++-----
 drivers/gpu/drm/msm/msm_mdss.c                | 31 ++++++++++++-----
 2 files changed, 48 insertions(+), 17 deletions(-)

-- 
2.39.0

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

[Dmitry Baryshkov]

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
-      - 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
-- 
2.39.0

[PATCH v2 2/4] drm/msm/mdss: enable optional core clock for MDP5 MDSS

[Dmitry Baryshkov]

Enable (optional) core (MDP_CLK) clock that allows accessing HW_REV
registers during the platform init.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/msm_mdss.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
index f9ef5085041d..c3364f85c148 100644
--- a/drivers/gpu/drm/msm/msm_mdss.c
+++ b/drivers/gpu/drm/msm/msm_mdss.c
@@ -355,7 +355,7 @@ static int msm_mdss_reset(struct device *dev)
 /*
  * MDP5 MDSS uses at most three specified clocks.
  */
-#define MDP5_MDSS_NUM_CLOCKS 3
+#define MDP5_MDSS_NUM_CLOCKS 4
 static int mdp5_mdss_parse_clock(struct platform_device *pdev, struct clk_bulk_data **clocks)
 {
 	struct clk_bulk_data *bulk;
@@ -372,6 +372,7 @@ static int mdp5_mdss_parse_clock(struct platform_device *pdev, struct clk_bulk_d
 	bulk[num_clocks++].id = "iface";
 	bulk[num_clocks++].id = "bus";
 	bulk[num_clocks++].id = "vsync";
+	bulk[num_clocks++].id = "core"; /* for hw_rev access */
 
 	ret = devm_clk_bulk_get_optional(&pdev->dev, num_clocks, bulk);
 	if (ret)
-- 
2.39.0

[PATCH v2 3/4] drm/msm/mdss: check for core clk before accessing HW_REV

[Dmitry Baryshkov]

Rather than checking whether the platform is an mdp5 or dpu platform,
check if the MDP_CLK is provided or not before trying to access HW_REV
(and skip reading the registers if the clock is not provided by the DT).

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/msm_mdss.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
index c3364f85c148..ce554f2c2e02 100644
--- a/drivers/gpu/drm/msm/msm_mdss.c
+++ b/drivers/gpu/drm/msm/msm_mdss.c
@@ -35,7 +35,6 @@ struct msm_mdss {
 	void __iomem *mmio;
 	struct clk_bulk_data *clocks;
 	size_t num_clocks;
-	bool is_mdp5;
 	struct {
 		unsigned long enabled_mask;
 		struct irq_domain *domain;
@@ -230,6 +229,19 @@ static void msm_mdss_setup_ubwc_dec_40(struct msm_mdss *msm_mdss,
 	}
 }
 
+static bool msm_mdss_has_clock(struct msm_mdss *msm_mdss, const char *name)
+{
+	unsigned int i;
+
+	for (i = 0; i < msm_mdss->num_clocks; i++) {
+		if (!strcmp(msm_mdss->clocks[i].id, name) &&
+		    msm_mdss->clocks[i].clk)
+			return true;
+	}
+
+	return false;
+}
+
 static int msm_mdss_enable(struct msm_mdss *msm_mdss)
 {
 	int ret;
@@ -249,10 +261,11 @@ static int msm_mdss_enable(struct msm_mdss *msm_mdss)
 	}
 
 	/*
-	 * HW_REV requires MDSS_MDP_CLK, which is not enabled by the mdss on
-	 * mdp5 hardware. Skip reading it for now.
+	 * HW_REV requires MDSS_MDP_CLK, which is not used for MDSS device in
+	 * older device trees. Skip accessing registers if the clock is not
+	 * present.
 	 */
-	if (msm_mdss->is_mdp5)
+	if (!msm_mdss_has_clock(msm_mdss, "core"))
 		return 0;
 
 	hw_rev = readl_relaxed(msm_mdss->mmio + HW_REV);
@@ -419,7 +432,6 @@ static struct msm_mdss *msm_mdss_init(struct platform_device *pdev, bool is_mdp5
 		return ERR_PTR(ret);
 	}
 	msm_mdss->num_clocks = ret;
-	msm_mdss->is_mdp5 = is_mdp5;
 
 	msm_mdss->dev = &pdev->dev;
 
-- 
2.39.0

[PATCH v2 4/4] drm/msm/mdss: move is_mdp5 condition to msm_mdss_init

[Dmitry Baryshkov]

Move is_mdp5 check to a more logical place, to the msm_mdss_init(),
rather than getting it in the mdss_probe() and passing it then as an
argument.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/msm_mdss.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
index ce554f2c2e02..c2ef50c3101b 100644
--- a/drivers/gpu/drm/msm/msm_mdss.c
+++ b/drivers/gpu/drm/msm/msm_mdss.c
@@ -396,8 +396,9 @@ static int mdp5_mdss_parse_clock(struct platform_device *pdev, struct clk_bulk_d
 	return num_clocks;
 }
 
-static struct msm_mdss *msm_mdss_init(struct platform_device *pdev, bool is_mdp5)
+static struct msm_mdss *msm_mdss_init(struct platform_device *pdev)
 {
+	bool is_mdp5 = of_device_is_compatible(pdev->dev.of_node, "qcom,mdss");
 	struct msm_mdss *msm_mdss;
 	int ret;
 	int irq;
@@ -494,11 +495,10 @@ static const struct dev_pm_ops mdss_pm_ops = {
 static int mdss_probe(struct platform_device *pdev)
 {
 	struct msm_mdss *mdss;
-	bool is_mdp5 = of_device_is_compatible(pdev->dev.of_node, "qcom,mdss");
 	struct device *dev = &pdev->dev;
 	int ret;
 
-	mdss = msm_mdss_init(pdev, is_mdp5);
+	mdss = msm_mdss_init(pdev);
 	if (IS_ERR(mdss))
 		return PTR_ERR(mdss);
 
-- 
2.39.0

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

[Krzysztof Kozlowski]

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.




> -      - 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?

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?

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.



Best regards,
Krzysztof

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

[Dmitry Baryshkov]

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.

-- 
With best wishes
Dmitry

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

[Krzysztof Kozlowski]

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.

> 
>>
>> 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.

Best regards,
Krzysztof

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

[Dmitry Baryshkov]

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-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@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
> 

-- 
With best wishes
Dmitry

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

[Dmitry Baryshkov]

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/

-- 
With best wishes
Dmitry

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

[Rob Herring]

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 the
following warnings. Consider if they are expected or the schema is
incorrect. 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.org


mdss@1a00000: mdp@1a01000:compatible:0: 'qcom,mdp5' was expected
	arch/arm64/boot/dts/qcom/sdm632-fairphone-fp3.dtb

mdss@1a00000: mdp@1a01000:compatible: ['qcom,msm8953-mdp5', 'qcom,mdp5'] is too long
	arch/arm64/boot/dts/qcom/sdm632-fairphone-fp3.dtb

mdss@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.dtb
	arch/arm64/boot/dts/qcom/apq8096-ifc6640.dtb
	arch/arm64/boot/dts/qcom/msm8996-mtp.dtb
	arch/arm64/boot/dts/qcom/msm8996-oneplus3.dtb
	arch/arm64/boot/dts/qcom/msm8996-oneplus3t.dtb
	arch/arm64/boot/dts/qcom/msm8996pro-xiaomi-natrium.dtb
	arch/arm64/boot/dts/qcom/msm8996pro-xiaomi-scorpio.dtb
	arch/arm64/boot/dts/qcom/msm8996-sony-xperia-tone-dora.dtb
	arch/arm64/boot/dts/qcom/msm8996-sony-xperia-tone-kagura.dtb
	arch/arm64/boot/dts/qcom/msm8996-sony-xperia-tone-keyaki.dtb
	arch/arm64/boot/dts/qcom/msm8996-xiaomi-gemini.dtb