Re: [Freedreno] [PATCH] MAINTAINERS: Update the URI for MSM DRM bugs
On Sat, Mar 4, 2023 at 3:27 PM Abhinav Kumar wrote: > > Update the URI for MSM DRM bugs for users to be able > to file bugs at a centralized location. > > Signed-off-by: Abhinav Kumar Acked-by: Rob Clark > --- > MAINTAINERS | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index a47d963af3b8..504138e294ef 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -6641,6 +6641,7 @@ L:linux-arm-...@vger.kernel.org > L: dri-de...@lists.freedesktop.org > L: freedreno@lists.freedesktop.org > S: Maintained > +B: https://gitlab.freedesktop.org/drm/msm/-/issues > T: git https://gitlab.freedesktop.org/drm/msm.git > F: Documentation/devicetree/bindings/display/msm/ > F: drivers/gpu/drm/msm/ > -- > 2.7.4 >
[Freedreno] [PATCH] MAINTAINERS: Update the URI for MSM DRM bugs
Update the URI for MSM DRM bugs for users to be able to file bugs at a centralized location. Signed-off-by: Abhinav Kumar --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index a47d963af3b8..504138e294ef 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6641,6 +6641,7 @@ L:linux-arm-...@vger.kernel.org L: dri-de...@lists.freedesktop.org L: freedreno@lists.freedesktop.org S: Maintained +B: https://gitlab.freedesktop.org/drm/msm/-/issues T: git https://gitlab.freedesktop.org/drm/msm.git F: Documentation/devicetree/bindings/display/msm/ F: drivers/gpu/drm/msm/ -- 2.7.4
Re: [Freedreno] [PATCH] dt-bindings: yamllint: Require a space after a comment '#'
On Fri, Mar 03, 2023 at 03:42:23PM -0600, Rob Herring wrote: > Enable yamllint to check the prefered commenting style of requiring a fwiiw: prefered -> preferred > space after a comment character '#'. Fix the cases in the tree which > have a warning with this enabled. Most cases just need a space after the > '#'. A couple of cases with comments which were not intended to be > comments are revealed. Those were in ti,sa2ul.yaml, ti,cal.yaml, and > brcm,bcmgenet.yaml. > > Signed-off-by: Rob Herring ...
Re: [Freedreno] [PATCH] dt-bindings: yamllint: Require a space after a comment '#'
On Fri, Mar 03, 2023 at 03:42:23PM -0600, Rob Herring wrote: > Enable yamllint to check the prefered commenting style of requiring a > space after a comment character '#'. Fix the cases in the tree which > have a warning with this enabled. Most cases just need a space after the > '#'. A couple of cases with comments which were not intended to be > comments are revealed. Those were in ti,sa2ul.yaml, ti,cal.yaml, and > brcm,bcmgenet.yaml. Acked-by: Mark Brown signature.asc Description: PGP signature
Re: [Freedreno] [PATCH v3 1/2] dt-bindings: display/msm: dsi-controller-main: Fix deprecated compatible
On 04/03/2023 17:45, Bryan O'Donoghue wrote:
On 04/03/2023 17:35, Konrad Dybcio wrote:
you'll see no error. However if you just do this
diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi
b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index 0733c2f4f3798..829fbe05b5713 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -1094,8 +1094,7 @@ mdp5_intf1_out: endpoint {
};
dsi0: dsi@1a98000 {
- compatible = "qcom,msm8916-dsi-ctrl",
- "qcom,mdss-dsi-ctrl";
+ compatible = "qcom,mdss-dsi-ctrl";
reg = <0x01a98000 0x25c>;
reg-names = "dsi_ctrl";
and run the same test you get
Yes, correct. It's valid but it's deprecated, so the bindings are
sane. Keep in mind there's an ABI-like aspect to this.
Konrad
The _driver_ will still accept "qcom,mdss-dsi-ctrl" which is ABI
compliant but, I don't see why the yaml should.
If you declare a new .dts with only "qcom,mdss-dsi-ctrl", that should
throw a yaml check error.
---
bod
Actually. I agree with you, I just dislike it.
- "qcom,mdss-dsi-ctrl" <- the driver will accept this
- "qcom,dsi-ctrl-6g-qcm2290" <- the driver will not accept this
bah
Reviewed-by: Bryan O'Donoghue
Re: [Freedreno] [PATCH v3 1/2] dt-bindings: display/msm: dsi-controller-main: Fix deprecated compatible
On 04/03/2023 17:35, Konrad Dybcio wrote:
you'll see no error. However if you just do this
diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi
b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index 0733c2f4f3798..829fbe05b5713 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -1094,8 +1094,7 @@ mdp5_intf1_out: endpoint {
};
dsi0: dsi@1a98000 {
- compatible = "qcom,msm8916-dsi-ctrl",
- "qcom,mdss-dsi-ctrl";
+ compatible = "qcom,mdss-dsi-ctrl";
reg = <0x01a98000 0x25c>;
reg-names = "dsi_ctrl";
and run the same test you get
Yes, correct. It's valid but it's deprecated, so the bindings are
sane. Keep in mind there's an ABI-like aspect to this.
Konrad
The _driver_ will still accept "qcom,mdss-dsi-ctrl" which is ABI
compliant but, I don't see why the yaml should.
If you declare a new .dts with only "qcom,mdss-dsi-ctrl", that should
throw a yaml check error.
---
bod
Re: [Freedreno] [PATCH v3 1/2] dt-bindings: display/msm: dsi-controller-main: Fix deprecated compatible
On 4.03.2023 17:59, Bryan O'Donoghue wrote:
> On 04/03/2023 15:55, Konrad Dybcio wrote:
>> The point of the previous cleanup was to disallow "qcom,mdss-dsi-ctrl"
>> alone. This however didn't quite work out and the property became
>> undocumented instead of deprecated. Fix that.
>>
>> Fixes: 0c0f65c6dd44 ("dt-bindings: msm: dsi-controller-main: Add compatible
>> strings for every current SoC")
>> Signed-off-by: Konrad Dybcio
>> ---
>> Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml | 2
>> +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git
>> a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
>> b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
>> index f195530ae964..d534451c8f7f 100644
>> --- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
>> +++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
>> @@ -35,7 +35,7 @@ properties:
>> - items:
>> - enum:
>> - qcom,dsi-ctrl-6g-qcm2290
>> - - const: qcom,mdss-dsi-ctrl
>> + - qcom,mdss-dsi-ctrl # This should always come with an
>> SoC-specific compatible
>> deprecated: true
>> reg:
>>
>
> This change would make compatible = "qcom,dsi-ctrl-6g-qcm2290",
> "qcom,mdss-dsi-ctrl"; break though
Intended, they were never supposed to go together, as at the time
before this patchset (and its stated dependency) the fallback
would not be sufficient, the driver wouldn't even probe.
>
> Take this example, I'm going to use 8916 because its easy.
>
> If we apply your change to dsi-controller-main.yaml
>
> diff --git
> a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> index e75a3efe4dace..e93c16431f0a1 100644
> --- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> @@ -34,7 +34,7 @@ properties:
> - items:
> - enum:
> - dsi-ctrl-6g-qcm2290
> - - const: qcom,mdss-dsi-ctrl
> + - qcom,mdss-dsi-ctrl
> deprecated: true
>
> reg:
>
> and then make 8916 == compatible = "qcom,dsi-ctrl-6g-qcm2290",
> "qcom,mdss-dsi-ctrl";
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi
> b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> index 0733c2f4f3798..7332b5f66a09d 100644
> --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> @@ -1094,7 +1094,7 @@ mdp5_intf1_out: endpoint {
> };
>
> dsi0: dsi@1a98000 {
> - compatible = "qcom,msm8916-dsi-ctrl",
> + compatible = "dsi-ctrl-6g-qcm2290",
> "qcom,mdss-dsi-ctrl";
> reg = <0x01a98000 0x25c>;
> reg-names = "dsi_ctrl";
>
> arch/arm64/boot/dts/qcom/apq8016-sbc.dtb: dsi@1a98000: compatible: 'oneOf'
> conditional failed, one must be fixed:
> ['dsi-ctrl-6g-qcm2290', 'qcom,mdss-dsi-ctrl'] is too long
>
>
> so compatible = "qcom,dsi-ctrl-6g-qcm2290", "qcom,mdss-dsi-ctrl"; is now
> invalid, not deprecated.
Intended
>
> This change also makes compatible = "qcom,dsi-ctrl-6g-qcm2290" or compatible
> = "qcom,mdss-dsi-ctrl" standalone valid compatible which is again not what we
> want.
-ish, it's marked as deprecated but it is valid.
>
> - enum:
> - qcom,dsi-ctrl-6g-qcm2290
> - qcom,mdss-dsi-ctrl
>
> means either "qcom,dsi-ctrl-6g-qcm2290" or "qcom,mdss-dsi-ctrl" are valid
> compat strings...
Correct
>
> As an example if you apply your change and then change the msm8916.dtsi to
> the below
>
> diff --git
> a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> index e75a3efe4dace..e93c16431f0a1 100644
> --- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> @@ -34,7 +34,7 @@ properties:
> - items:
> - enum:
> - dsi-ctrl-6g-qcm2290
> - - const: qcom,mdss-dsi-ctrl
> + - qcom,mdss-dsi-ctrl
> deprecated: true
>
> reg:
> diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi
> b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> index 0733c2f4f3798..829fbe05b5713 100644
> --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> @@ -1094,8 +1094,7 @@ mdp5_intf1_out: endpoint {
> };
>
> dsi0: dsi@1a98000 {
> - compatible = "qcom,msm8916-dsi-ctrl",
> - "qcom,mdss-dsi-ctrl";
> +
Re: [Freedreno] [PATCH v3 1/2] dt-bindings: display/msm: dsi-controller-main: Fix deprecated compatible
On 04/03/2023 15:55, Konrad Dybcio wrote:
The point of the previous cleanup was to disallow "qcom,mdss-dsi-ctrl"
alone. This however didn't quite work out and the property became
undocumented instead of deprecated. Fix that.
Fixes: 0c0f65c6dd44 ("dt-bindings: msm: dsi-controller-main: Add compatible strings
for every current SoC")
Signed-off-by: Konrad Dybcio
---
Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git
a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
index f195530ae964..d534451c8f7f 100644
--- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
+++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
@@ -35,7 +35,7 @@ properties:
- items:
- enum:
- qcom,dsi-ctrl-6g-qcm2290
- - const: qcom,mdss-dsi-ctrl
+ - qcom,mdss-dsi-ctrl # This should always come with an
SoC-specific compatible
deprecated: true
reg:
This change would make compatible = "qcom,dsi-ctrl-6g-qcm2290",
"qcom,mdss-dsi-ctrl"; break though
Take this example, I'm going to use 8916 because its easy.
If we apply your change to dsi-controller-main.yaml
diff --git
a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
index e75a3efe4dace..e93c16431f0a1 100644
--- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
+++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
@@ -34,7 +34,7 @@ properties:
- items:
- enum:
- dsi-ctrl-6g-qcm2290
- - const: qcom,mdss-dsi-ctrl
+ - qcom,mdss-dsi-ctrl
deprecated: true
reg:
and then make 8916 == compatible = "qcom,dsi-ctrl-6g-qcm2290",
"qcom,mdss-dsi-ctrl";
diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi
b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index 0733c2f4f3798..7332b5f66a09d 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -1094,7 +1094,7 @@ mdp5_intf1_out: endpoint {
};
dsi0: dsi@1a98000 {
- compatible = "qcom,msm8916-dsi-ctrl",
+ compatible = "dsi-ctrl-6g-qcm2290",
"qcom,mdss-dsi-ctrl";
reg = <0x01a98000 0x25c>;
reg-names = "dsi_ctrl";
arch/arm64/boot/dts/qcom/apq8016-sbc.dtb: dsi@1a98000: compatible:
'oneOf' conditional failed, one must be fixed:
['dsi-ctrl-6g-qcm2290', 'qcom,mdss-dsi-ctrl'] is too long
so compatible = "qcom,dsi-ctrl-6g-qcm2290", "qcom,mdss-dsi-ctrl"; is now
invalid, not deprecated.
This change also makes compatible = "qcom,dsi-ctrl-6g-qcm2290" or
compatible = "qcom,mdss-dsi-ctrl" standalone valid compatible which is
again not what we want.
- enum:
- qcom,dsi-ctrl-6g-qcm2290
- qcom,mdss-dsi-ctrl
means either "qcom,dsi-ctrl-6g-qcm2290" or "qcom,mdss-dsi-ctrl" are
valid compat strings...
As an example if you apply your change and then change the msm8916.dtsi
to the below
diff --git
a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
index e75a3efe4dace..e93c16431f0a1 100644
--- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
+++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
@@ -34,7 +34,7 @@ properties:
- items:
- enum:
- dsi-ctrl-6g-qcm2290
- - const: qcom,mdss-dsi-ctrl
+ - qcom,mdss-dsi-ctrl
deprecated: true
reg:
diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi
b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index 0733c2f4f3798..829fbe05b5713 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -1094,8 +1094,7 @@ mdp5_intf1_out: endpoint {
};
dsi0: dsi@1a98000 {
- compatible = "qcom,msm8916-dsi-ctrl",
-"qcom,mdss-dsi-ctrl";
+ compatible = "qcom,mdss-dsi-ctrl";
reg = <0x01a98000 0x25c>;
reg-names = "dsi_ctrl";
Then test it with
make O=$BUILDDIR DT_DOC_CHECKER=$DT_DOC_CHECKER
DT_EXTRACT_EX=$DT_EXTRACT_EX DT_MK_SCHEMA=$DT_MK_SCHEMA
DT_CHECKER=$DT_CHECKER CHECKER_FLAGS=-W=1 CHECK_DTBS=y qcom/apq8016-sbc.dtb
you'll see no error. However if you just do this
diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi
b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index 0733c2f4f3798..829fbe05b5713 100644
---
[Freedreno] [PATCH v3 2/2] dt-bindings: display: msm: sm6115-mdss: Fix DSI compatible
Since the DSI autodetection is bound to work correctly on 6115 now, switch to using the correct per-SoC + generic fallback compatible combo. Signed-off-by: Konrad Dybcio --- .../devicetree/bindings/display/msm/qcom,sm6115-mdss.yaml | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/display/msm/qcom,sm6115-mdss.yaml b/Documentation/devicetree/bindings/display/msm/qcom,sm6115-mdss.yaml index 2491cb100b33..605b1f654d78 100644 --- a/Documentation/devicetree/bindings/display/msm/qcom,sm6115-mdss.yaml +++ b/Documentation/devicetree/bindings/display/msm/qcom,sm6115-mdss.yaml @@ -40,7 +40,13 @@ patternProperties: type: object properties: compatible: -const: qcom,dsi-ctrl-6g-qcm2290 +oneOf: + - items: + - const: qcom,sm6115-dsi-ctrl + - const: qcom,mdss-dsi-ctrl + - description: Old binding, please don't use +deprecated: true +const: qcom,dsi-ctrl-6g-qcm2290 "^phy@[0-9a-f]+$": type: object -- 2.39.2
[Freedreno] [PATCH v3 1/2] dt-bindings: display/msm: dsi-controller-main: Fix deprecated compatible
The point of the previous cleanup was to disallow "qcom,mdss-dsi-ctrl"
alone. This however didn't quite work out and the property became
undocumented instead of deprecated. Fix that.
Fixes: 0c0f65c6dd44 ("dt-bindings: msm: dsi-controller-main: Add compatible
strings for every current SoC")
Signed-off-by: Konrad Dybcio
---
Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git
a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
index f195530ae964..d534451c8f7f 100644
--- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
+++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
@@ -35,7 +35,7 @@ properties:
- items:
- enum:
- qcom,dsi-ctrl-6g-qcm2290
- - const: qcom,mdss-dsi-ctrl
+ - qcom,mdss-dsi-ctrl # This should always come with an
SoC-specific compatible
deprecated: true
reg:
--
2.39.2
[Freedreno] [PATCH v3 0/2] Fix up Qualcomm DSI bindings
v2 -> v3: - Deprecate "qcom,mdss-dsi-ctrl" correctly instead of removing it [1/2] - Remove the note about separate driver logic, as it's gone now [2/2] Depends on: https://lore.kernel.org/linux-arm-msm/20230213121012.1768296-1-konrad.dyb...@linaro.org/ Link to v2: https://lore.kernel.org/linux-arm-msm/20230217111316.306241-1-konrad.dyb...@linaro.org/ Signed-off-by: Konrad Dybcio --- Konrad Dybcio (2): dt-bindings: display/msm: dsi-controller-main: Fix deprecated compatible dt-bindings: display: msm: sm6115-mdss: Fix DSI compatible .../devicetree/bindings/display/msm/dsi-controller-main.yaml | 2 +- .../devicetree/bindings/display/msm/qcom,sm6115-mdss.yaml | 8 +++- 2 files changed, 8 insertions(+), 2 deletions(-) --- base-commit: 9a33780f72f64c1cd151d84a9959f58a13b0c970 change-id: 20230304-topic-dsi_fixup-ecaf0bd3b767 Best regards, -- Konrad Dybcio
Re: [Freedreno] [PATCH v4 1/4] drm/msm/dpu: Move TE setup to prepare_for_kickoff()
On 2023-03-01 13:42:55, Abhinav Kumar wrote: > >>> Then, for some patch hygiene, starting here: > >>> > Depends on: "Implement tearcheck support on INTF block" [3] > > Changes in V3: > - Added function prototypes > - Reordered function definitions to make change more legible > - Removed prepare_commit() function from dpu_encoder_phys_cmd > > Changes in V4: > - Reworded commit message to be more specific > - Removed dpu_encoder_phys_cmd_is_ongoing_pptx() prototype > >>> > >>> ... until here: all this info belongs /below the cut/ outside of the > >>> messge that becomes part of the commit when this patch is applied to the > >>> tree. > >> > >> For DRM, I thought we are keeping the change log above the ? > >> Which means its allowed in the commit message. > > > > I hope not, seems unlikely to have different rules across kernel > > subsystems. The main point is that this changelog and dependency chain > > isn't of any value when the final patch is applied, regardless of > > whether it is "allowed". > > > > I looked at a recently posted change by Rob and change log is above the --- > > https://patchwork.kernel.org/project/dri-devel/patch/20230301185432.3010939-1-robdcl...@gmail.com/ > > So we will follow that. I hope that was in error, or no-one pointed it out to Rob. As said before there is no use to having this information in the applied patch, even the kernel guidelines state so: https://docs.kernel.org/process/submitting-patches.html Other comments relevant only to the moment or the maintainer, not suitable for the permanent changelog, should also go here. **A good example of such comments might be patch changelogs which describe what has changed between the v1 and v2 version of the patch.** **Please put this information after the --- line** which separates the changelog from the rest of the patch. The version information is not part of the changelog which gets committed to the git tree. It is additional information for the reviewers. If it’s placed above the commit tags, it needs manual interaction to remove it. If it is below the separator line, it gets automatically stripped off when applying the patch: > It should be first the intf TE series and then this one. You can go > ahead and post your v2, we will rebase on top of yours. Sounds good; though as said before I'm extremely short on time making it hard to actively commit to this, especially as the catalog changes are really hard to juggle between various "local" branches to test on the many (Sony) devices we are working on. As usual, a preview of v2 is still available at: https://github.com/SoMainline/linux/commits/marijn/dpu And I will do my best to get the last comments worked out. - Marijn
