public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* Claude review: Add DSI display support for SC8280XP
  2026-02-25  5:45 [PATCH 0/5] " Pengyu Luo
@ 2026-02-27  4:04 ` Claude Code Review Bot
  0 siblings, 0 replies; 17+ messages in thread
From: Claude Code Review Bot @ 2026-02-27  4:04 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: Add DSI display support for SC8280XP
Author: Pengyu Luo <mitltlatltl@gmail.com>
Patches: 15
Reviewed: 2026-02-27T14:04:45.091186

---

This series adds DSI display support for the Qualcomm SC8280XP SoC across 5 patches: dt-bindings for the PHY and controller, MDSS binding documentation, the PHY driver table entry, and a large DTS patch adding 4 DSI controllers + 4 PHYs (2 per MDSS instance). The series is logically structured and follows existing patterns for similar Qualcomm SoCs.

However, there are several issues that need attention:

1. **Bug: Copy-paste error in mdss1_dsi0 clock parents** (Patch 5) - the byte clock parent incorrectly references `mdss1_dsi1_phy` instead of `mdss1_dsi0_phy`.
2. **Inconsistent PHY PLL register sizes** between mdss0 and mdss1 instances (0x27c vs 0x260).
3. **Patch numbering**: The last patch is labeled `[PATCH]` instead of `[PATCH 5/5]`.
4. **Known non-functional state**: The commit message acknowledges DSI won't work properly without dispcc fixes.

---

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v2 0/4] Add DSI display support for SC8280XP
@ 2026-02-28 10:19 Pengyu Luo
  2026-02-28 10:19 ` [PATCH v2 1/4] dt-bindings: display: msm-dsi-phy-7nm: Add SC8280XP Pengyu Luo
                   ` (4 more replies)
  0 siblings, 5 replies; 17+ messages in thread
From: Pengyu Luo @ 2026-02-28 10:19 UTC (permalink / raw)
  To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
	Sean Paul, Marijn Suijten, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
	Krishna Manikandan, Jonathan Marek
  Cc: linux-arm-msm, dri-devel, freedreno, devicetree, linux-kernel,
	Tianyu Gao, White Lewis, Pengyu Luo

Add DSI display support for SC8280XP.
---
Changes in v2:
- fallback to SA8775P compatible (Krzysztof, Konrad, Dmitry)
- fix DT styles[a newline between property and subnode, property order] (Konrad)
- use one dsi_opp_table and all dsi controllers reference it (Konrad)
- resize dsi_pll region to 0x280 (Konrad)
- update commit message
- Link to v1: https://lore.kernel.org/linux-arm-msm/20260225054525.6803-1-mitltlatltl@gmail.com

Pengyu Luo (4):
  dt-bindings: display: msm-dsi-phy-7nm: Add SC8280XP
  dt-bindings: display/msm: dsi-controller-main: Add SC8280XP
  dt-bindings: display: msm: Document DSI controller and DSI PHY on
    SC8280XP
  arm64: dts: qcom: sc8280xp: Add dsi nodes on SC8280XP

 .../display/msm/dsi-controller-main.yaml      |   1 +
 .../bindings/display/msm/dsi-phy-7nm.yaml     |   1 +
 .../display/msm/qcom,sc8280xp-mdss.yaml       |  30 ++
 arch/arm64/boot/dts/qcom/sc8280xp.dtsi        | 396 +++++++++++++++++-
 4 files changed, 420 insertions(+), 8 deletions(-)

-- 
2.53.0


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v2 1/4] dt-bindings: display: msm-dsi-phy-7nm: Add SC8280XP
  2026-02-28 10:19 [PATCH v2 0/4] Add DSI display support for SC8280XP Pengyu Luo
@ 2026-02-28 10:19 ` Pengyu Luo
  2026-03-03  4:28   ` Claude review: " Claude Code Review Bot
  2026-02-28 10:19 ` [PATCH v2 2/4] dt-bindings: display/msm: dsi-controller-main: " Pengyu Luo
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: Pengyu Luo @ 2026-02-28 10:19 UTC (permalink / raw)
  To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
	Sean Paul, Marijn Suijten, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
	Krishna Manikandan, Jonathan Marek
  Cc: linux-arm-msm, dri-devel, freedreno, devicetree, linux-kernel,
	Tianyu Gao, White Lewis, Pengyu Luo

Since SC8280XP and SA8775P have the same values for the DSI PHY
REVISION_ID registers, then we fallback to SA8775P compatible.

Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com>
---
 Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.yaml b/Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.yaml
index 9a9a6c4ab..532f37182 100644
--- a/Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.yaml
+++ b/Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.yaml
@@ -32,6 +32,7 @@ properties:
       - items:
           - enum:
               - qcom,qcs8300-dsi-phy-5nm
+              - qcom,sc8280xp-dsi-phy-5nm
           - const: qcom,sa8775p-dsi-phy-5nm
 
   reg:
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 2/4] dt-bindings: display/msm: dsi-controller-main: Add SC8280XP
  2026-02-28 10:19 [PATCH v2 0/4] Add DSI display support for SC8280XP Pengyu Luo
  2026-02-28 10:19 ` [PATCH v2 1/4] dt-bindings: display: msm-dsi-phy-7nm: Add SC8280XP Pengyu Luo
@ 2026-02-28 10:19 ` Pengyu Luo
  2026-03-03  4:28   ` Claude review: " Claude Code Review Bot
  2026-02-28 10:19 ` [PATCH v2 3/4] dt-bindings: display: msm: Document DSI controller and DSI PHY on SC8280XP Pengyu Luo
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: Pengyu Luo @ 2026-02-28 10:19 UTC (permalink / raw)
  To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
	Sean Paul, Marijn Suijten, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
	Krishna Manikandan, Jonathan Marek
  Cc: linux-arm-msm, dri-devel, freedreno, devicetree, linux-kernel,
	Tianyu Gao, White Lewis, Pengyu Luo

Since SC8280XP and SA8775P have the same DSI version(2.5.1), then we
fallback to SA8775P compatible.

Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com>
---
 .../devicetree/bindings/display/msm/dsi-controller-main.yaml     | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
index eb6d38dab..617dd110d 100644
--- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
+++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
@@ -49,6 +49,7 @@ properties:
       - items:
           - enum:
               - qcom,qcs8300-dsi-ctrl
+              - qcom,sc8280xp-dsi-ctrl
           - const: qcom,sa8775p-dsi-ctrl
           - const: qcom,mdss-dsi-ctrl
       - enum:
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 3/4] dt-bindings: display: msm: Document DSI controller and DSI PHY on SC8280XP
  2026-02-28 10:19 [PATCH v2 0/4] Add DSI display support for SC8280XP Pengyu Luo
  2026-02-28 10:19 ` [PATCH v2 1/4] dt-bindings: display: msm-dsi-phy-7nm: Add SC8280XP Pengyu Luo
  2026-02-28 10:19 ` [PATCH v2 2/4] dt-bindings: display/msm: dsi-controller-main: " Pengyu Luo
@ 2026-02-28 10:19 ` Pengyu Luo
  2026-03-03  4:28   ` Claude review: " Claude Code Review Bot
  2026-02-28 10:19 ` [PATCH v2 4/4] arm64: dts: qcom: sc8280xp: Add dsi nodes " Pengyu Luo
  2026-03-03  4:28 ` Claude review: Add DSI display support for SC8280XP Claude Code Review Bot
  4 siblings, 1 reply; 17+ messages in thread
From: Pengyu Luo @ 2026-02-28 10:19 UTC (permalink / raw)
  To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
	Sean Paul, Marijn Suijten, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
	Krishna Manikandan, Jonathan Marek
  Cc: linux-arm-msm, dri-devel, freedreno, devicetree, linux-kernel,
	Tianyu Gao, White Lewis, Pengyu Luo

Document DSI controller and DSI phy on SC8280XP platform.

Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com>
---
 .../display/msm/qcom,sc8280xp-mdss.yaml       | 30 +++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/msm/qcom,sc8280xp-mdss.yaml b/Documentation/devicetree/bindings/display/msm/qcom,sc8280xp-mdss.yaml
index af79406e1..a710cc84e 100644
--- a/Documentation/devicetree/bindings/display/msm/qcom,sc8280xp-mdss.yaml
+++ b/Documentation/devicetree/bindings/display/msm/qcom,sc8280xp-mdss.yaml
@@ -50,6 +50,22 @@ patternProperties:
           - qcom,sc8280xp-dp
           - qcom,sc8280xp-edp
 
+  "^dsi@[0-9a-f]+$":
+    type: object
+    additionalProperties: true
+    properties:
+      compatible:
+        contains:
+          const: qcom,sc8280xp-dsi-ctrl
+
+  "^phy@[0-9a-f]+$":
+    type: object
+    additionalProperties: true
+    properties:
+      compatible:
+        contains:
+          const: qcom,sc8280xp-dsi-phy-5nm
+
 unevaluatedProperties: false
 
 examples:
@@ -129,6 +145,20 @@ examples:
                     };
                 };
 
+                port@1 {
+                    reg = <1>;
+                    dpu_intf1_out: endpoint {
+                        remote-endpoint = <&mdss0_dsi0_in>;
+                    };
+                };
+
+                port@2 {
+                    reg = <2>;
+                    dpu_intf2_out: endpoint {
+                        remote-endpoint = <&mdss0_dsi1_in>;
+                    };
+                };
+
                 port@4 {
                     reg = <4>;
                     endpoint {
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 4/4] arm64: dts: qcom: sc8280xp: Add dsi nodes on SC8280XP
  2026-02-28 10:19 [PATCH v2 0/4] Add DSI display support for SC8280XP Pengyu Luo
                   ` (2 preceding siblings ...)
  2026-02-28 10:19 ` [PATCH v2 3/4] dt-bindings: display: msm: Document DSI controller and DSI PHY on SC8280XP Pengyu Luo
@ 2026-02-28 10:19 ` Pengyu Luo
  2026-02-28 13:31   ` Dmitry Baryshkov
  2026-03-03  4:28   ` Claude review: " Claude Code Review Bot
  2026-03-03  4:28 ` Claude review: Add DSI display support for SC8280XP Claude Code Review Bot
  4 siblings, 2 replies; 17+ messages in thread
From: Pengyu Luo @ 2026-02-28 10:19 UTC (permalink / raw)
  To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
	Sean Paul, Marijn Suijten, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
	Krishna Manikandan, Jonathan Marek
  Cc: linux-arm-msm, dri-devel, freedreno, devicetree, linux-kernel,
	Tianyu Gao, White Lewis, Pengyu Luo

The DT configuration follows other Samsung 5nm-based Qualcomm SOCs,
utilizing the same register layouts and clock structures.

However, DSI won't work properly for now until we submit dispcc fixes.
And some DSC enabled panels require DPU timing calculation fixes too.
(hdisplay / width timing round errors cause the fifo error)

Co-developed-by: Tianyu Gao <gty0622@gmail.com>
Signed-off-by: Tianyu Gao <gty0622@gmail.com>
Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com>
Tested-by: White Lewis <liu224806@gmail.com> # HUAWEI Gaokun3
---
 arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 396 ++++++++++++++++++++++++-
 1 file changed, 388 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
index 706eb1309..79021267d 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
@@ -5,6 +5,7 @@
  */
 
 #include <dt-bindings/clock/qcom,dispcc-sc8280xp.h>
+#include <dt-bindings/clock/qcom,dsi-phy-28nm.h>
 #include <dt-bindings/clock/qcom,gcc-sc8280xp.h>
 #include <dt-bindings/clock/qcom,gpucc-sc8280xp.h>
 #include <dt-bindings/clock/qcom,rpmh.h>
@@ -4652,13 +4653,31 @@ ports {
 
 					port@0 {
 						reg = <0>;
+
 						mdss0_intf0_out: endpoint {
 							remote-endpoint = <&mdss0_dp0_in>;
 						};
 					};
 
+					port@1 {
+						reg = <1>;
+
+						mdss0_intf1_out: endpoint {
+							remote-endpoint = <&mdss0_dsi0_in>;
+						};
+					};
+
+					port@2 {
+						reg = <2>;
+
+						mdss0_intf2_out: endpoint {
+							remote-endpoint = <&mdss0_dsi1_in>;
+						};
+					};
+
 					port@4 {
 						reg = <4>;
+
 						mdss0_intf4_out: endpoint {
 							remote-endpoint = <&mdss0_dp1_in>;
 						};
@@ -4666,6 +4685,7 @@ mdss0_intf4_out: endpoint {
 
 					port@5 {
 						reg = <5>;
+
 						mdss0_intf5_out: endpoint {
 							remote-endpoint = <&mdss0_dp3_in>;
 						};
@@ -4673,6 +4693,7 @@ mdss0_intf5_out: endpoint {
 
 					port@6 {
 						reg = <6>;
+
 						mdss0_intf6_out: endpoint {
 							remote-endpoint = <&mdss0_dp2_in>;
 						};
@@ -4791,6 +4812,185 @@ opp-810000000 {
 				};
 			};
 
+			mdss0_dsi0: dsi@ae94000 {
+				compatible = "qcom,sc8280xp-dsi-ctrl",
+					     "qcom,sa8775p-dsi-ctrl",
+					     "qcom,mdss-dsi-ctrl";
+				reg = <0 0x0ae94000 0 0x400>;
+				reg-names = "dsi_ctrl";
+
+				interrupt-parent = <&mdss0>;
+				interrupts = <4>;
+
+				clocks = <&dispcc0 DISP_CC_MDSS_BYTE0_CLK>,
+					 <&dispcc0 DISP_CC_MDSS_BYTE0_INTF_CLK>,
+					 <&dispcc0 DISP_CC_MDSS_PCLK0_CLK>,
+					 <&dispcc0 DISP_CC_MDSS_ESC0_CLK>,
+					 <&dispcc0 DISP_CC_MDSS_AHB_CLK>,
+					 <&gcc GCC_DISP_HF_AXI_CLK>;
+				clock-names = "byte",
+					      "byte_intf",
+					      "pixel",
+					      "core",
+					      "iface",
+					      "bus";
+
+				assigned-clocks = <&dispcc0 DISP_CC_MDSS_BYTE0_CLK_SRC>,
+						  <&dispcc0 DISP_CC_MDSS_PCLK0_CLK_SRC>;
+				assigned-clock-parents = <&mdss0_dsi0_phy DSI_BYTE_PLL_CLK>,
+							 <&mdss0_dsi0_phy DSI_PIXEL_PLL_CLK>;
+
+				operating-points-v2 = <&dsi_opp_table>;
+				power-domains = <&rpmhpd SC8280XP_MMCX>;
+
+				phys = <&mdss0_dsi0_phy>;
+				phy-names = "dsi";
+
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				status = "disabled";
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					port@0 {
+						reg = <0>;
+
+						mdss0_dsi0_in: endpoint {
+							remote-endpoint = <&mdss0_intf1_out>;
+						};
+					};
+
+					port@1 {
+						reg = <1>;
+
+						mdss0_dsi0_out: endpoint {
+						};
+					};
+				};
+
+				dsi_opp_table: opp-table {
+					compatible = "operating-points-v2";
+
+					opp-187500000 {
+						opp-hz = /bits/ 64 <187500000>;
+						required-opps = <&rpmhpd_opp_low_svs>;
+					};
+
+					opp-300000000 {
+						opp-hz = /bits/ 64 <300000000>;
+						required-opps = <&rpmhpd_opp_svs>;
+					};
+
+					opp-358000000 {
+						opp-hz = /bits/ 64 <358000000>;
+						required-opps = <&rpmhpd_opp_svs_l1>;
+					};
+				};
+			};
+
+			mdss0_dsi0_phy: phy@ae94400 {
+				compatible = "qcom,sc8280xp-dsi-phy-5nm"
+					     "qcom,sa8775p-dsi-phy-5nm";
+				reg = <0 0x0ae94400 0 0x200>,
+				      <0 0x0ae94600 0 0x280>,
+				      <0 0x0ae94900 0 0x280>;
+				reg-names = "dsi_phy",
+					    "dsi_phy_lane",
+					    "dsi_pll";
+
+				#clock-cells = <1>;
+				#phy-cells = <0>;
+
+				clocks = <&dispcc0 DISP_CC_MDSS_AHB_CLK>,
+					 <&rpmhcc RPMH_CXO_CLK>;
+				clock-names = "iface", "ref";
+
+				status = "disabled";
+			};
+
+			mdss0_dsi1: dsi@ae96000 {
+				compatible = "qcom,sc8280xp-dsi-ctrl",
+					     "qcom,sa8775p-dsi-ctrl",
+					     "qcom,mdss-dsi-ctrl";
+				reg = <0 0x0ae96000 0 0x400>;
+				reg-names = "dsi_ctrl";
+
+				interrupt-parent = <&mdss0>;
+				interrupts = <5>;
+
+				clocks = <&dispcc0 DISP_CC_MDSS_BYTE1_CLK>,
+					 <&dispcc0 DISP_CC_MDSS_BYTE1_INTF_CLK>,
+					 <&dispcc0 DISP_CC_MDSS_PCLK1_CLK>,
+					 <&dispcc0 DISP_CC_MDSS_ESC1_CLK>,
+					 <&dispcc0 DISP_CC_MDSS_AHB_CLK>,
+					 <&gcc GCC_DISP_HF_AXI_CLK>;
+				clock-names = "byte",
+					      "byte_intf",
+					      "pixel",
+					      "core",
+					      "iface",
+					      "bus";
+
+				assigned-clocks = <&dispcc0 DISP_CC_MDSS_BYTE1_CLK_SRC>,
+						  <&dispcc0 DISP_CC_MDSS_PCLK1_CLK_SRC>;
+				assigned-clock-parents = <&mdss0_dsi1_phy DSI_BYTE_PLL_CLK>,
+							 <&mdss0_dsi1_phy DSI_PIXEL_PLL_CLK>;
+
+				operating-points-v2 = <&dsi_opp_table>;
+				power-domains = <&rpmhpd SC8280XP_MMCX>;
+
+				phys = <&mdss0_dsi1_phy>;
+				phy-names = "dsi";
+
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				status = "disabled";
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					port@0 {
+						reg = <0>;
+
+						mdss0_dsi1_in: endpoint {
+							remote-endpoint = <&mdss0_intf2_out>;
+						};
+					};
+
+					port@1 {
+						reg = <1>;
+
+						mdss0_dsi1_out: endpoint {
+						};
+					};
+				};
+			};
+
+			mdss0_dsi1_phy: phy@ae96400 {
+				compatible = "qcom,sc8280xp-dsi-phy-5nm"
+					     "qcom,sa8775p-dsi-phy-5nm";
+				reg = <0 0x0ae96400 0 0x200>,
+				      <0 0x0ae96600 0 0x280>,
+				      <0 0x0ae96900 0 0x280>;
+				reg-names = "dsi_phy",
+					    "dsi_phy_lane",
+					    "dsi_pll";
+
+				#clock-cells = <1>;
+				#phy-cells = <0>;
+
+				clocks = <&dispcc0 DISP_CC_MDSS_AHB_CLK>,
+					 <&rpmhcc RPMH_CXO_CLK>;
+				clock-names = "iface", "ref";
+
+				status = "disabled";
+			};
+
 			mdss0_dp1: displayport-controller@ae98000 {
 				compatible = "qcom,sc8280xp-dp";
 				reg = <0 0xae98000 0 0x200>,
@@ -5080,10 +5280,10 @@ dispcc0: clock-controller@af00000 {
 				 <&mdss0_dp2_phy 1>,
 				 <&mdss0_dp3_phy 0>,
 				 <&mdss0_dp3_phy 1>,
-				 <0>,
-				 <0>,
-				 <0>,
-				 <0>;
+				 <&mdss0_dsi0_phy DSI_BYTE_PLL_CLK>,
+				 <&mdss0_dsi0_phy DSI_PIXEL_PLL_CLK>,
+				 <&mdss0_dsi1_phy DSI_BYTE_PLL_CLK>,
+				 <&mdss0_dsi1_phy DSI_PIXEL_PLL_CLK>;
 			power-domains = <&rpmhpd SC8280XP_MMCX>;
 
 			#clock-cells = <1>;
@@ -6011,13 +6211,31 @@ ports {
 
 					port@0 {
 						reg = <0>;
+
 						mdss1_intf0_out: endpoint {
 							remote-endpoint = <&mdss1_dp0_in>;
 						};
 					};
 
+					port@1 {
+						reg = <1>;
+
+						mdss1_intf1_out: endpoint {
+							remote-endpoint = <&mdss1_dsi0_in>;
+						};
+					};
+
+					port@2 {
+						reg = <2>;
+
+						mdss1_intf2_out: endpoint {
+							remote-endpoint = <&mdss1_dsi1_in>;
+						};
+					};
+
 					port@4 {
 						reg = <4>;
+
 						mdss1_intf4_out: endpoint {
 							remote-endpoint = <&mdss1_dp1_in>;
 						};
@@ -6025,6 +6243,7 @@ mdss1_intf4_out: endpoint {
 
 					port@5 {
 						reg = <5>;
+
 						mdss1_intf5_out: endpoint {
 							remote-endpoint = <&mdss1_dp3_in>;
 						};
@@ -6032,6 +6251,7 @@ mdss1_intf5_out: endpoint {
 
 					port@6 {
 						reg = <6>;
+
 						mdss1_intf6_out: endpoint {
 							remote-endpoint = <&mdss1_dp2_in>;
 						};
@@ -6147,6 +6367,166 @@ opp-810000000 {
 				};
 			};
 
+			mdss1_dsi0: dsi@22094000 {
+				compatible = "qcom,sc8280xp-dsi-ctrl",
+					     "qcom,sa8775p-dsi-ctrl",
+					     "qcom,mdss-dsi-ctrl";
+				reg = <0 0x22094000 0 0x400>;
+				reg-names = "dsi_ctrl";
+
+				interrupt-parent = <&mdss1>;
+				interrupts = <4>;
+
+				clocks = <&dispcc1 DISP_CC_MDSS_BYTE0_CLK>,
+					 <&dispcc1 DISP_CC_MDSS_BYTE0_INTF_CLK>,
+					 <&dispcc1 DISP_CC_MDSS_PCLK0_CLK>,
+					 <&dispcc1 DISP_CC_MDSS_ESC0_CLK>,
+					 <&dispcc1 DISP_CC_MDSS_AHB_CLK>,
+					 <&gcc GCC_DISP_HF_AXI_CLK>;
+				clock-names = "byte",
+					      "byte_intf",
+					      "pixel",
+					      "core",
+					      "iface",
+					      "bus";
+
+				assigned-clocks = <&dispcc1 DISP_CC_MDSS_BYTE0_CLK_SRC>,
+						  <&dispcc1 DISP_CC_MDSS_PCLK0_CLK_SRC>;
+				assigned-clock-parents = <&mdss1_dsi1_phy DSI_BYTE_PLL_CLK>,
+							 <&mdss1_dsi0_phy DSI_PIXEL_PLL_CLK>;
+
+				operating-points-v2 = <&dsi_opp_table>;
+				power-domains = <&rpmhpd SC8280XP_MMCX>;
+
+				phys = <&mdss1_dsi0_phy>;
+				phy-names = "dsi";
+
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				status = "disabled";
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					port@0 {
+						reg = <0>;
+
+						mdss1_dsi0_in: endpoint {
+							remote-endpoint = <&mdss1_intf1_out>;
+						};
+					};
+
+					port@1 {
+						reg = <1>;
+
+						mdss1_dsi0_out: endpoint {
+						};
+					};
+				};
+			};
+
+			mdss1_dsi0_phy: phy@22094400 {
+				compatible = "qcom,sc8280xp-dsi-phy-5nm"
+					     "qcom,sa8775p-dsi-phy-5nm";
+				reg = <0 0x22094400 0 0x200>,
+				      <0 0x22094600 0 0x280>,
+				      <0 0x22094900 0 0x280>;
+				reg-names = "dsi_phy",
+					    "dsi_phy_lane",
+					    "dsi_pll";
+
+				#clock-cells = <1>;
+				#phy-cells = <0>;
+
+				clocks = <&dispcc1 DISP_CC_MDSS_AHB_CLK>,
+					 <&rpmhcc RPMH_CXO_CLK>;
+				clock-names = "iface", "ref";
+
+				status = "disabled";
+			};
+
+			mdss1_dsi1: dsi@22096000 {
+				compatible = "qcom,sc8280xp-dsi-ctrl",
+					     "qcom,sa8775p-dsi-ctrl",
+					     "qcom,mdss-dsi-ctrl";
+				reg = <0 0x22096000 0 0x400>;
+				reg-names = "dsi_ctrl";
+
+				interrupt-parent = <&mdss1>;
+				interrupts = <5>;
+
+				clocks = <&dispcc1 DISP_CC_MDSS_BYTE1_CLK>,
+					 <&dispcc1 DISP_CC_MDSS_BYTE1_INTF_CLK>,
+					 <&dispcc1 DISP_CC_MDSS_PCLK1_CLK>,
+					 <&dispcc1 DISP_CC_MDSS_ESC1_CLK>,
+					 <&dispcc1 DISP_CC_MDSS_AHB_CLK>,
+					 <&gcc GCC_DISP_HF_AXI_CLK>;
+				clock-names = "byte",
+					      "byte_intf",
+					      "pixel",
+					      "core",
+					      "iface",
+					      "bus";
+
+				assigned-clocks = <&dispcc1 DISP_CC_MDSS_BYTE1_CLK_SRC>,
+						  <&dispcc1 DISP_CC_MDSS_PCLK1_CLK_SRC>;
+				assigned-clock-parents = <&mdss1_dsi1_phy DSI_BYTE_PLL_CLK>,
+							 <&mdss1_dsi1_phy DSI_PIXEL_PLL_CLK>;
+
+				operating-points-v2 = <&dsi_opp_table>;
+				power-domains = <&rpmhpd SC8280XP_MMCX>;
+
+				phys = <&mdss1_dsi1_phy>;
+				phy-names = "dsi";
+
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				status = "disabled";
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					port@0 {
+						reg = <0>;
+
+						mdss1_dsi1_in: endpoint {
+							remote-endpoint = <&mdss1_intf2_out>;
+						};
+					};
+
+					port@1 {
+						reg = <1>;
+
+						mdss1_dsi1_out: endpoint {
+						};
+					};
+				};
+			};
+
+			mdss1_dsi1_phy: phy@22096400 {
+				compatible = "qcom,sc8280xp-dsi-phy-5nm"
+					     "qcom,sa8775p-dsi-phy-5nm";
+				reg = <0 0x22096400 0 0x200>,
+				      <0 0x22096600 0 0x280>,
+				      <0 0x22096900 0 0x280>;
+				reg-names = "dsi_phy",
+					    "dsi_phy_lane",
+					    "dsi_pll";
+
+				#clock-cells = <1>;
+				#phy-cells = <0>;
+
+				clocks = <&dispcc1 DISP_CC_MDSS_AHB_CLK>,
+					 <&rpmhcc RPMH_CXO_CLK>;
+				clock-names = "iface", "ref";
+
+				status = "disabled";
+			};
+
 			mdss1_dp1: displayport-controller@22098000 {
 				compatible = "qcom,sc8280xp-dp";
 				reg = <0 0x22098000 0 0x200>,
@@ -6434,10 +6814,10 @@ dispcc1: clock-controller@22100000 {
 				 <&mdss1_dp2_phy 1>,
 				 <&mdss1_dp3_phy 0>,
 				 <&mdss1_dp3_phy 1>,
-				 <0>,
-				 <0>,
-				 <0>,
-				 <0>;
+				 <&mdss1_dsi0_phy DSI_BYTE_PLL_CLK>,
+				 <&mdss1_dsi0_phy DSI_PIXEL_PLL_CLK>,
+				 <&mdss1_dsi1_phy DSI_BYTE_PLL_CLK>,
+				 <&mdss1_dsi1_phy DSI_PIXEL_PLL_CLK>;
 			power-domains = <&rpmhpd SC8280XP_MMCX>;
 
 			#clock-cells = <1>;
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 4/4] arm64: dts: qcom: sc8280xp: Add dsi nodes on SC8280XP
  2026-02-28 10:19 ` [PATCH v2 4/4] arm64: dts: qcom: sc8280xp: Add dsi nodes " Pengyu Luo
@ 2026-02-28 13:31   ` Dmitry Baryshkov
  2026-02-28 13:57     ` Pengyu Luo
  2026-03-03  4:28   ` Claude review: " Claude Code Review Bot
  1 sibling, 1 reply; 17+ messages in thread
From: Dmitry Baryshkov @ 2026-02-28 13:31 UTC (permalink / raw)
  To: Pengyu Luo
  Cc: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
	Sean Paul, Marijn Suijten, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
	Krishna Manikandan, Jonathan Marek, linux-arm-msm, dri-devel,
	freedreno, devicetree, linux-kernel, Tianyu Gao, White Lewis

On Sat, Feb 28, 2026 at 06:19:07PM +0800, Pengyu Luo wrote:
> The DT configuration follows other Samsung 5nm-based Qualcomm SOCs,
> utilizing the same register layouts and clock structures.
> 
> However, DSI won't work properly for now until we submit dispcc fixes.
> And some DSC enabled panels require DPU timing calculation fixes too.
> (hdisplay / width timing round errors cause the fifo error)

- Please squash refgen patch into this one.
- Please post at least the dispcc fixes.

> 
> Co-developed-by: Tianyu Gao <gty0622@gmail.com>
> Signed-off-by: Tianyu Gao <gty0622@gmail.com>
> Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com>
> Tested-by: White Lewis <liu224806@gmail.com> # HUAWEI Gaokun3
> ---
>  arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 396 ++++++++++++++++++++++++-
>  1 file changed, 388 insertions(+), 8 deletions(-)
> 

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 4/4] arm64: dts: qcom: sc8280xp: Add dsi nodes on SC8280XP
  2026-02-28 13:31   ` Dmitry Baryshkov
@ 2026-02-28 13:57     ` Pengyu Luo
  2026-02-28 14:23       ` Dmitry Baryshkov
  0 siblings, 1 reply; 17+ messages in thread
From: Pengyu Luo @ 2026-02-28 13:57 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
	Sean Paul, Marijn Suijten, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
	Krishna Manikandan, Jonathan Marek, linux-arm-msm, dri-devel,
	freedreno, devicetree, linux-kernel, Tianyu Gao, White Lewis

On Sat, Feb 28, 2026 at 9:32 PM Dmitry Baryshkov
<dmitry.baryshkov@oss.qualcomm.com> wrote:
>
> On Sat, Feb 28, 2026 at 06:19:07PM +0800, Pengyu Luo wrote:
> > The DT configuration follows other Samsung 5nm-based Qualcomm SOCs,
> > utilizing the same register layouts and clock structures.
> >
> > However, DSI won't work properly for now until we submit dispcc fixes.
> > And some DSC enabled panels require DPU timing calculation fixes too.
> > (hdisplay / width timing round errors cause the fifo error)
>
> - Please squash refgen patch into this one.

Ack

> - Please post at least the dispcc fixes.
>

Yes, it will come later with the panel driver. Lewis provided the
patch but it was generated by Claude AI. The patch itself is not
complicated, it only removes CLK_SET_RATE_PARENT from
byte_div_clk_src dividers. But I need to find some time to analyze the
dsi phy clock diagram and reword the commit log.

Best wishes,
Pengyu

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 4/4] arm64: dts: qcom: sc8280xp: Add dsi nodes on SC8280XP
  2026-02-28 13:57     ` Pengyu Luo
@ 2026-02-28 14:23       ` Dmitry Baryshkov
  2026-03-01  3:49         ` Pengyu Luo
  0 siblings, 1 reply; 17+ messages in thread
From: Dmitry Baryshkov @ 2026-02-28 14:23 UTC (permalink / raw)
  To: Pengyu Luo
  Cc: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
	Sean Paul, Marijn Suijten, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
	Krishna Manikandan, Jonathan Marek, linux-arm-msm, dri-devel,
	freedreno, devicetree, linux-kernel, Tianyu Gao, White Lewis

On Sat, Feb 28, 2026 at 09:57:19PM +0800, Pengyu Luo wrote:
> On Sat, Feb 28, 2026 at 9:32 PM Dmitry Baryshkov
> <dmitry.baryshkov@oss.qualcomm.com> wrote:
> >
> > On Sat, Feb 28, 2026 at 06:19:07PM +0800, Pengyu Luo wrote:
> > > The DT configuration follows other Samsung 5nm-based Qualcomm SOCs,
> > > utilizing the same register layouts and clock structures.
> > >
> > > However, DSI won't work properly for now until we submit dispcc fixes.
> > > And some DSC enabled panels require DPU timing calculation fixes too.
> > > (hdisplay / width timing round errors cause the fifo error)
> >
> > - Please squash refgen patch into this one.
> 
> Ack
> 
> > - Please post at least the dispcc fixes.
> >
> 
> Yes, it will come later with the panel driver. Lewis provided the
> patch but it was generated by Claude AI. The patch itself is not
> complicated, it only removes CLK_SET_RATE_PARENT from
> byte_div_clk_src dividers. But I need to find some time to analyze the
> dsi phy clock diagram and reword the commit log.

Krzysztof wrote a perfect description in the commit  b8501febdc51 ("clk:
qcom: dispcc-sm8750: Drop incorrect CLK_SET_RATE_PARENT on byte intf
parent").

> 
> Best wishes,
> Pengyu

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 4/4] arm64: dts: qcom: sc8280xp: Add dsi nodes on SC8280XP
  2026-02-28 14:23       ` Dmitry Baryshkov
@ 2026-03-01  3:49         ` Pengyu Luo
  0 siblings, 0 replies; 17+ messages in thread
From: Pengyu Luo @ 2026-03-01  3:49 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
	Sean Paul, Marijn Suijten, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson, Konrad Dybcio,
	Krishna Manikandan, Jonathan Marek, linux-arm-msm, dri-devel,
	freedreno, devicetree, linux-kernel, Tianyu Gao, White Lewis

On Sat, Feb 28, 2026 at 10:23 PM Dmitry Baryshkov
<dmitry.baryshkov@oss.qualcomm.com> wrote:
>
> On Sat, Feb 28, 2026 at 09:57:19PM +0800, Pengyu Luo wrote:
> > On Sat, Feb 28, 2026 at 9:32 PM Dmitry Baryshkov
> > <dmitry.baryshkov@oss.qualcomm.com> wrote:
> > >
> > > On Sat, Feb 28, 2026 at 06:19:07PM +0800, Pengyu Luo wrote:
> > > > The DT configuration follows other Samsung 5nm-based Qualcomm SOCs,
> > > > utilizing the same register layouts and clock structures.
> > > >
> > > > However, DSI won't work properly for now until we submit dispcc fixes.
> > > > And some DSC enabled panels require DPU timing calculation fixes too.
> > > > (hdisplay / width timing round errors cause the fifo error)
> > >
> > > - Please squash refgen patch into this one.
> >
> > Ack
> >
> > > - Please post at least the dispcc fixes.
> > >
> >
> > Yes, it will come later with the panel driver. Lewis provided the
> > patch but it was generated by Claude AI. The patch itself is not
> > complicated, it only removes CLK_SET_RATE_PARENT from
> > byte_div_clk_src dividers. But I need to find some time to analyze the
> > dsi phy clock diagram and reword the commit log.
>
> Krzysztof wrote a perfect description in the commit  b8501febdc51 ("clk:
> qcom: dispcc-sm8750: Drop incorrect CLK_SET_RATE_PARENT on byte intf
> parent").
>

Thanks, that is very helpful. It's indeed the same problem.I'll post
it in the next two or three days.

Best wishes,
Pengyu

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Claude review: Add DSI display support for SC8280XP
  2026-02-28 14:17 [PATCH v3 0/4] " Pengyu Luo
@ 2026-03-03  4:16 ` Claude Code Review Bot
  0 siblings, 0 replies; 17+ messages in thread
From: Claude Code Review Bot @ 2026-03-03  4:16 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: Add DSI display support for SC8280XP
Author: Pengyu Luo <mitltlatltl@gmail.com>
Patches: 10
Reviewed: 2026-03-03T14:16:43.200820

---

This is a v3 patch series by Pengyu Luo adding DSI display support for the Qualcomm SC8280XP platform. The series is well-structured: two small dt-bindings patches to register the new compatibles with fallback to SA8775P, one patch documenting the DSI/PHY nodes in the SC8280XP MDSS binding schema, and one large DTS patch adding the actual DSI controller and PHY nodes for both MDSS instances (mdss0 and mdss1).

The v3 changelog addresses prior review feedback (adding refgen supply, fallback compatibles, shared OPP table, resized PLL regions, style fixes).

**However, there are two bugs in patch 4 that must be fixed before merging:**
1. **Missing commas in PHY compatible strings** (4 instances) — will cause DTS compilation failures.
2. **Copy-paste error in mdss1_dsi0 assigned-clock-parents** — byte clock parent references the wrong PHY.

---

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Claude review: Add DSI display support for SC8280XP
  2026-02-28 10:19 [PATCH v2 0/4] Add DSI display support for SC8280XP Pengyu Luo
                   ` (3 preceding siblings ...)
  2026-02-28 10:19 ` [PATCH v2 4/4] arm64: dts: qcom: sc8280xp: Add dsi nodes " Pengyu Luo
@ 2026-03-03  4:28 ` Claude Code Review Bot
  4 siblings, 0 replies; 17+ messages in thread
From: Claude Code Review Bot @ 2026-03-03  4:28 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: Add DSI display support for SC8280XP
Author: Pengyu Luo <mitltlatltl@gmail.com>
Patches: 9
Reviewed: 2026-03-03T14:28:31.215188

---

This v2 series adds DSI display support for the Qualcomm SC8280XP platform (4 patches: two dt-binding compatible additions, one MDSS binding update, and the main DTS patch). The approach of falling back to SA8775P compatibles for both the DSI controller and PHY is sensible since the hardware is reportedly the same.

However, there is a **critical syntax error** (missing comma in all PHY compatible strings) and a **likely copy-paste bug** in clock parent assignments for one of the MDSS1 DSI controllers. These must be fixed before the series can be merged.

---

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Claude review: dt-bindings: display: msm-dsi-phy-7nm: Add SC8280XP
  2026-02-28 10:19 ` [PATCH v2 1/4] dt-bindings: display: msm-dsi-phy-7nm: Add SC8280XP Pengyu Luo
@ 2026-03-03  4:28   ` Claude Code Review Bot
  0 siblings, 0 replies; 17+ messages in thread
From: Claude Code Review Bot @ 2026-03-03  4:28 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Status: Looks good**

Straightforward one-line addition. The SC8280XP DSI PHY compatible falls back to SA8775P, which is the correct pattern given they share the same PHY REVISION_ID registers.

```diff
+              - qcom,sc8280xp-dsi-phy-5nm
           - const: qcom,sa8775p-dsi-phy-5nm
```

The commit message clearly explains the rationale for the fallback. No issues.

---

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Claude review: dt-bindings: display/msm: dsi-controller-main: Add SC8280XP
  2026-02-28 10:19 ` [PATCH v2 2/4] dt-bindings: display/msm: dsi-controller-main: " Pengyu Luo
@ 2026-03-03  4:28   ` Claude Code Review Bot
  0 siblings, 0 replies; 17+ messages in thread
From: Claude Code Review Bot @ 2026-03-03  4:28 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Status: Looks good**

Another straightforward one-line addition. SC8280XP DSI controller falls back to SA8775P compatible (same DSI version 2.5.1).

```diff
+              - qcom,sc8280xp-dsi-ctrl
           - const: qcom,sa8775p-dsi-ctrl
           - const: qcom,mdss-dsi-ctrl
```

No issues.

---

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Claude review: dt-bindings: display: msm: Document DSI controller and DSI PHY on SC8280XP
  2026-02-28 10:19 ` [PATCH v2 3/4] dt-bindings: display: msm: Document DSI controller and DSI PHY on SC8280XP Pengyu Luo
@ 2026-03-03  4:28   ` Claude Code Review Bot
  0 siblings, 0 replies; 17+ messages in thread
From: Claude Code Review Bot @ 2026-03-03  4:28 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Status: Looks good with a minor note**

Adds `patternProperties` for DSI controller and PHY child nodes to the sc8280xp-mdss binding, plus extends the example with DPU output port endpoints for DSI0 and DSI1.

The patternProperties look correctly structured:
```yaml
+  "^dsi@[0-9a-f]+$":
+    type: object
+    additionalProperties: true
+    properties:
+      compatible:
+        contains:
+          const: qcom,sc8280xp-dsi-ctrl
```

The example additions add `port@1` and `port@2` for the DPU output ports connecting to DSI0 and DSI1 respectively, which is consistent with the actual DTS in patch 4. The example endpoint labels (`dpu_intf1_out`, `dpu_intf2_out`, `mdss0_dsi0_in`, `mdss0_dsi1_in`) match the convention used in the DTS.

**Minor**: The example could be more complete by including skeleton DSI/PHY child nodes to match the new `patternProperties`, but this isn't strictly required for binding validation.

---

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Claude review: arm64: dts: qcom: sc8280xp: Add dsi nodes on SC8280XP
  2026-02-28 10:19 ` [PATCH v2 4/4] arm64: dts: qcom: sc8280xp: Add dsi nodes " Pengyu Luo
  2026-02-28 13:31   ` Dmitry Baryshkov
@ 2026-03-03  4:28   ` Claude Code Review Bot
  1 sibling, 0 replies; 17+ messages in thread
From: Claude Code Review Bot @ 2026-03-03  4:28 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Status: Needs fixes - has critical errors**

This is the main patch adding DSI controller and PHY nodes for both MDSS instances (mdss0 and mdss1), each with two DSI controllers and two DSI PHYs (4 controllers + 4 PHYs total). It also wires up the dispcc clock sources properly.

#### Bug 1 (Critical): Missing comma in all PHY compatible strings

All four PHY nodes have a missing comma between the two compatible strings:

```dts
+			mdss0_dsi0_phy: phy@ae94400 {
+				compatible = "qcom,sc8280xp-dsi-phy-5nm"
+					     "qcom,sa8775p-dsi-phy-5nm";
```

This appears identically in `mdss0_dsi0_phy`, `mdss0_dsi1_phy`, `mdss1_dsi0_phy`, and `mdss1_dsi1_phy`. Without the comma, dtc will concatenate the two strings into a single nonsensical compatible string `"qcom,sc8280xp-dsi-phy-5nmqcom,sa8775p-dsi-phy-5nm"`, which won't match any driver.

It should be:
```dts
				compatible = "qcom,sc8280xp-dsi-phy-5nm",
					     "qcom,sa8775p-dsi-phy-5nm";
```

Note: the DSI controller nodes have the comma correctly:
```dts
+				compatible = "qcom,sc8280xp-dsi-ctrl",
+					     "qcom,sa8775p-dsi-ctrl",
+					     "qcom,mdss-dsi-ctrl";
```

So this is clearly an oversight in the PHY nodes only.

#### Bug 2 (Likely copy-paste error): mdss1_dsi0 assigned-clock-parents mismatch

The `mdss1_dsi0` node has inconsistent clock parent PHY references:

```dts
+			mdss1_dsi0: dsi@22094000 {
+				...
+				assigned-clock-parents = <&mdss1_dsi1_phy DSI_BYTE_PLL_CLK>,
+							 <&mdss1_dsi0_phy DSI_PIXEL_PLL_CLK>;
```

The byte clock parent references `mdss1_dsi1_phy` (DSI **1** PHY) while the pixel clock parent references `mdss1_dsi0_phy` (DSI **0** PHY). Comparing with the other three controllers:

- `mdss0_dsi0`: both from `mdss0_dsi0_phy` -- consistent
- `mdss0_dsi1`: both from `mdss0_dsi1_phy` -- consistent
- `mdss1_dsi0`: byte from `mdss1_dsi1_phy`, pixel from `mdss1_dsi0_phy` -- **inconsistent**
- `mdss1_dsi1`: both from `mdss1_dsi1_phy` -- consistent

This looks like a copy-paste error. It should likely be:
```dts
				assigned-clock-parents = <&mdss1_dsi0_phy DSI_BYTE_PLL_CLK>,
							 <&mdss1_dsi0_phy DSI_PIXEL_PLL_CLK>;
```

#### Observation: 28nm clock header for 5nm PHY

```diff
+#include <dt-bindings/clock/qcom,dsi-phy-28nm.h>
```

This is technically correct -- `DSI_BYTE_PLL_CLK` and `DSI_PIXEL_PLL_CLK` are generic clock index defines that live in the 28nm header and are reused across all PHY generations. Other recent SoC dtsi files (SA8775P, etc.) do the same. But it could confuse future readers given the PHY is 5nm.

#### Observation: Shared OPP table placement

The `dsi_opp_table` is defined inside the `mdss0_dsi0` node but referenced by all four DSI controllers via phandle. This works functionally but is slightly unusual -- typically shared tables are placed at a common parent level. This was apparently requested by Konrad in v1 review, so it's intentional.

#### Observation: Style-only blank line additions

The patch adds blank lines after `reg = <N>;` in several existing port nodes (port@0, port@4, port@5, port@6 in both MDSS instances). This improves style consistency with the new port nodes but mixes style cleanup with functional changes. Likely acceptable since it was probably requested in v1 review.

#### Overall patch 4 structure

The DSI controller nodes are well-structured with correct interrupt numbers (4 for DSI0, 5 for DSI1), correct register addresses, proper clock assignments, and correct endpoint wiring. The dispcc clock source replacements from `<0>` stubs to actual PHY clock references are correct and necessary.

---

**Summary of required fixes:**
1. Add missing comma in all 4 PHY compatible strings
2. Fix `mdss1_dsi0` assigned-clock-parents to consistently reference `mdss1_dsi0_phy`

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Claude review: Add DSI display support for SC8280XP
  2026-03-08  6:48 [PATCH v4 0/4] " Pengyu Luo
@ 2026-03-08 21:56 ` Claude Code Review Bot
  0 siblings, 0 replies; 17+ messages in thread
From: Claude Code Review Bot @ 2026-03-08 21:56 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: Add DSI display support for SC8280XP
Author: Pengyu Luo <mitltlatltl@gmail.com>
Patches: 5
Reviewed: 2026-03-09T07:56:45.643189

---

This is a clean, well-structured v4 series adding DSI display support for the SC8280XP SoC. It follows standard Qualcomm DT patterns: first adding binding compatibles (patches 1-2), then documenting the MDSS schema (patch 3), and finally the DTS nodes (patch 4). The series correctly falls back to SA8775P compatibles for both the DSI controller and PHY. All three binding patches already carry `Reviewed-by` from Krzysztof Kozlowski, and patch 4 has reviews from Dmitry Baryshkov and Konrad Dybcio.

**There is one likely copy-paste bug in patch 4** in the `mdss1_dsi0` node's `assigned-clock-parents` that should be addressed before merging.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2026-03-08 21:56 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-28 10:19 [PATCH v2 0/4] Add DSI display support for SC8280XP Pengyu Luo
2026-02-28 10:19 ` [PATCH v2 1/4] dt-bindings: display: msm-dsi-phy-7nm: Add SC8280XP Pengyu Luo
2026-03-03  4:28   ` Claude review: " Claude Code Review Bot
2026-02-28 10:19 ` [PATCH v2 2/4] dt-bindings: display/msm: dsi-controller-main: " Pengyu Luo
2026-03-03  4:28   ` Claude review: " Claude Code Review Bot
2026-02-28 10:19 ` [PATCH v2 3/4] dt-bindings: display: msm: Document DSI controller and DSI PHY on SC8280XP Pengyu Luo
2026-03-03  4:28   ` Claude review: " Claude Code Review Bot
2026-02-28 10:19 ` [PATCH v2 4/4] arm64: dts: qcom: sc8280xp: Add dsi nodes " Pengyu Luo
2026-02-28 13:31   ` Dmitry Baryshkov
2026-02-28 13:57     ` Pengyu Luo
2026-02-28 14:23       ` Dmitry Baryshkov
2026-03-01  3:49         ` Pengyu Luo
2026-03-03  4:28   ` Claude review: " Claude Code Review Bot
2026-03-03  4:28 ` Claude review: Add DSI display support for SC8280XP Claude Code Review Bot
  -- strict thread matches above, loose matches on Subject: below --
2026-03-08  6:48 [PATCH v4 0/4] " Pengyu Luo
2026-03-08 21:56 ` Claude review: " Claude Code Review Bot
2026-02-28 14:17 [PATCH v3 0/4] " Pengyu Luo
2026-03-03  4:16 ` Claude review: " Claude Code Review Bot
2026-02-25  5:45 [PATCH 0/5] " Pengyu Luo
2026-02-27  4:04 ` Claude review: " Claude Code Review Bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox