[PATCH v2 00/10] Add DSI display support for SA8775P target

[PATCH v2 00/10] Add DSI display support for SA8775P target

[Ayushi Makhija]

This series enables the support for DSI to DP bridge ports
(labled as DSI0 and DSI1) of the Qualcomm's SA8775P Ride platform.

SA8775P SoC has DSI controller v2.5.1 and DSI PHY v4.2.
The Ride platform is having ANX7625 DSI to DP bridge chip from Analogix.

---
This patch depends on following series:
https://lore.kernel.org/linux-arm-msm/20250127-dts-qcom-dsi-phy-clocks-v1-0-9d8ddbcb1c7f@linaro.org/
(ARM / arm64: dts: qcom: Use the header with DSI phy clock IDs)

Changes in v2: Fixed review comments from Rob, konard, Dmitry and  Krzysztof
    - Added additionalProperities in dsi and phy patternProperties in patch 3. [Rob's bot]
    - Updated example in qcom,sa8775p-mdss.yaml of patch 3:
        - Added port1 and port2 inside mdss0 ports.
        - Renamed dsi ports from mdss_dsi0_in to mdss0_dsi0_in and mdss_dsi1_in to mdss0_dsi1_in.
    - Updated the init load value for vdds supply of DSI PHY from
      150000uA to 48000uA as per chipset power grid in patch 4. [Dmitry]
    - Updated the init load value for vdda supply for DSI ctrl
      from 30100uA to 8300uA as per chipset power grid in patch 5. [Dmitry]
    - Rebase the series to use the header with DSI phy clock IDs to make code more
      readable in patch 6. [konard]
    - Added the interrupts-extended in patch 7. [konard]
    - Fixed the warning from DT checker against DT binding in patch 7. [Krzysztof]
    - Changed the connector node name from dsi0-connector to dp-dsi0-connector and dsi1-connector to dp-dsi1-connector
      respectively in patch 7. [Dmitry]
    - Added the vph_pwr for anx7625 vdda10, vdds18 and vdda33 supply to fix the warnings from DT checker in
      patch 7. [Rob's bot]
    - Addressed device tree comments in patch 7. [Konard]
    - Squash the DT patch 8 into DT patch 7. [Dmitry]
    - Added hpd_enable() and hpd_disable() bridge funcs in patch 9. [Dmitry]
    - Update hpd detection bridge op flags logic based on eDP connector in patch 10. [Dmitry]
    - Link to v1 : https://lore.kernel.org/linux-arm-msm/20250225121824.3869719-1-quic_amakhija@quicinc.com/

---

Ayushi Makhija (10):
  dt-bindings: display: msm-dsi-phy-7nm: document the SA8775P DSI PHY
  dt-bindings: msm: dsi-controller-main: document the SA8775P DSI CTRL
  dt-bindings: display: msm: document DSI controller and phy on SA8775P
  drm/msm/dsi: add DSI PHY configuration on SA8775P
  drm/msm/dsi: add DSI support for SA8775P
  arm64: dts: qcom: sa8775p: add Display Serial Interface device nodes
  arm64: dts: qcom: sa8775p-ride: add anx7625 DSI to DP bridge nodes
  drm/bridge: anx7625: enable HPD interrupts
  drm/bridge: anx7625: update bridge_ops and sink detect logic
  drm/bridge: anx7625: change the gpiod_set_value API

 .../display/msm/dsi-controller-main.yaml      |   1 +
 .../bindings/display/msm/dsi-phy-7nm.yaml     |   1 +
 .../display/msm/qcom,sa8775p-mdss.yaml        | 188 ++++++++++++++++
 arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi    | 208 +++++++++++++++++-
 arch/arm64/boot/dts/qcom/sa8775p.dtsi         | 186 +++++++++++++++-
 drivers/gpu/drm/bridge/analogix/anx7625.c     |  36 ++-
 drivers/gpu/drm/msm/dsi/dsi_cfg.c             |  18 ++
 drivers/gpu/drm/msm/dsi/dsi_cfg.h             |   1 +
 drivers/gpu/drm/msm/dsi/phy/dsi_phy.c         |   2 +
 drivers/gpu/drm/msm/dsi/phy/dsi_phy.h         |   1 +
 drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c     |  27 +++
 11 files changed, 657 insertions(+), 12 deletions(-)

-- 
2.34.1

[PATCH v2 01/10] dt-bindings: display: msm-dsi-phy-7nm: document the SA8775P DSI PHY

[Ayushi Makhija]

Document the DSI PHY on the SA8775P Platform.

Signed-off-by: Ayushi Makhija <quic_amakhija@quicinc.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 321470435e65..fd5728f3e89f 100644
--- a/Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.yaml
+++ b/Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.yaml
@@ -17,6 +17,7 @@ properties:
     enum:
       - qcom,dsi-phy-7nm
       - qcom,dsi-phy-7nm-8150
+      - qcom,sa8775p-dsi-phy-5nm
       - qcom,sc7280-dsi-phy-7nm
       - qcom,sm6375-dsi-phy-7nm
       - qcom,sm8350-dsi-phy-5nm
-- 
2.34.1

Re: [PATCH v2 01/10] dt-bindings: display: msm-dsi-phy-7nm: document the SA8775P DSI PHY

[Krzysztof Kozlowski]

On Tue, Mar 11, 2025 at 05:54:36PM +0530, Ayushi Makhija wrote:
> Document the DSI PHY on the SA8775P Platform.
> 
> Signed-off-by: Ayushi Makhija <quic_amakhija@quicinc.com>
> ---
>  Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.yaml | 1 +
>  1 file changed, 1 insertion(+)

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

[PATCH v2 02/10] dt-bindings: msm: dsi-controller-main: document the SA8775P DSI CTRL

[Ayushi Makhija]

Document the DSI CTRL on the SA8775P Platform.

Signed-off-by: Ayushi Makhija <quic_amakhija@quicinc.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 ffbd1dc9470e..3621e3f12b65 100644
--- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
+++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
@@ -23,6 +23,7 @@ properties:
               - qcom,msm8996-dsi-ctrl
               - qcom,msm8998-dsi-ctrl
               - qcom,qcm2290-dsi-ctrl
+              - qcom,sa8775p-dsi-ctrl
               - qcom,sc7180-dsi-ctrl
               - qcom,sc7280-dsi-ctrl
               - qcom,sdm660-dsi-ctrl
-- 
2.34.1

Re: [PATCH v2 02/10] dt-bindings: msm: dsi-controller-main: document the SA8775P DSI CTRL

[Krzysztof Kozlowski]

On Tue, Mar 11, 2025 at 05:54:37PM +0530, Ayushi Makhija wrote:
> diff --git a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> index ffbd1dc9470e..3621e3f12b65 100644
> --- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
> @@ -23,6 +23,7 @@ properties:
>                - qcom,msm8996-dsi-ctrl
>                - qcom,msm8998-dsi-ctrl
>                - qcom,qcm2290-dsi-ctrl
> +              - qcom,sa8775p-dsi-ctrl

That's incomplete, where is the rest of updates in the file? Not
needed?

Best regards,
Krzysztof

Re: [PATCH v2 02/10] dt-bindings: msm: dsi-controller-main: document the SA8775P DSI CTRL

[Ayushi Makhija]

On 3/12/2025 5:14 PM, Krzysztof Kozlowski wrote:
> On Tue, Mar 11, 2025 at 05:54:37PM +0530, Ayushi Makhija wrote:
>> diff --git a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
>> index ffbd1dc9470e..3621e3f12b65 100644
>> --- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
>> +++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
>> @@ -23,6 +23,7 @@ properties:
>>                - qcom,msm8996-dsi-ctrl
>>                - qcom,msm8998-dsi-ctrl
>>                - qcom,qcm2290-dsi-ctrl
>> +              - qcom,sa8775p-dsi-ctrl
> 
> That's incomplete, where is the rest of updates in the file? Not
> needed?
> 
> Best regards,
> Krzysztof
> 

Hi Krzysztof,

Thanks, for the review.

Each compatible string has a different set of clocks which are associated with it.

Sorry, I forget to add qcom,sa8775p-dsi-ctrl compatible based on the set of clocks which are associated with it.
Will add it in next patch.

--- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
+++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml
@@ -349,6 +349,7 @@ allOf:
         compatible:
           contains:
             enum:
+              - qcom,sa8775p-dsi-ctrl
               - qcom,sc7180-dsi-ctrl
               - qcom,sc7280-dsi-ctrl
               - qcom,sm6150-dsi-ctrl

The remaining content in the file will stay the same, no other changes are needed.

Thanks,
Ayushi

[PATCH v2 03/10] dt-bindings: display: msm: document DSI controller and phy on SA8775P

[Ayushi Makhija]

Document DSI controller and phy on SA8775P platform.

Signed-off-by: Ayushi Makhija <quic_amakhija@quicinc.com>
---
 .../display/msm/qcom,sa8775p-mdss.yaml        | 188 ++++++++++++++++++
 1 file changed, 188 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/msm/qcom,sa8775p-mdss.yaml b/Documentation/devicetree/bindings/display/msm/qcom,sa8775p-mdss.yaml
index a90a8b3f1a9e..628ca68871f4 100644
--- a/Documentation/devicetree/bindings/display/msm/qcom,sa8775p-mdss.yaml
+++ b/Documentation/devicetree/bindings/display/msm/qcom,sa8775p-mdss.yaml
@@ -52,6 +52,26 @@ patternProperties:
         items:
           - const: qcom,sa8775p-dp
 
+  "^dsi@[0-9a-f]+$":
+    type: object
+    additionalProperties: true
+
+    properties:
+      compatible:
+        items:
+          - const: qcom,sa8775p-dsi-ctrl
+          - const: qcom,mdss-dsi-ctrl
+
+  "^phy@[0-9a-f]+$":
+    type: object
+    additionalProperties: true
+
+    properties:
+      compatible:
+        enum:
+          - qcom,sa8775p-dsi-phy-5nm
+          - qcom,sa8775p-edp-phy
+
 required:
   - compatible
 
@@ -131,6 +151,20 @@ examples:
                         remote-endpoint = <&mdss0_dp0_in>;
                     };
                 };
+
+                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>;
+                    };
+                };
             };
 
             mdss0_mdp_opp_table: opp-table {
@@ -158,6 +192,160 @@ examples:
             };
         };
 
+        dsi@ae94000 {
+            compatible = "qcom,sa8775p-dsi-ctrl", "qcom,mdss-dsi-ctrl";
+            reg = <0x0ae94000 0x400>;
+            reg-names = "dsi_ctrl";
+
+            interrupt-parent = <&mdss>;
+            interrupts = <4>;
+
+            clocks = <&dispc_byte_clk>,
+                     <&dispcc_intf_clk>,
+                     <&dispcc_pclk>,
+                     <&dispcc_esc_clk>,
+                     <&dispcc_ahb_clk>,
+                     <&gcc_bus_clk>;
+            clock-names = "byte",
+                          "byte_intf",
+                          "pixel",
+                          "core",
+                          "iface",
+                          "bus";
+            assigned-clocks = <&dispcc_byte_clk>,
+                              <&dispcc_pclk>;
+            assigned-clock-parents = <&mdss0_dsi0_phy 0>, <&mdss0_dsi0_phy 1>;
+            phys = <&mdss0_dsi0_phy>;
+
+            operating-points-v2 = <&dsi0_opp_table>;
+            power-domains = <&rpmhpd SA8775P_MMCX>;
+
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            ports {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                port@0 {
+                    reg = <0>;
+                    mdss0_dsi0_in: endpoint {
+                        remote-endpoint = <&dpu_intf1_out>;
+                    };
+                };
+
+                port@1 {
+                    reg = <1>;
+                    mdss0_dsi0_out: endpoint { };
+                };
+            };
+
+            dsi0_opp_table: opp-table {
+                compatible = "operating-points-v2";
+
+                opp-358000000 {
+                    opp-hz = /bits/ 64 <358000000>;
+                    required-opps = <&rpmhpd_opp_svs_l1>;
+                };
+            };
+        };
+
+        mdss0_dsi0_phy: phy@ae94400 {
+            compatible = "qcom,sa8775p-dsi-phy-5nm";
+            reg = <0x0ae94400 0x200>,
+                  <0x0ae94600 0x280>,
+                  <0x0ae94900 0x27c>;
+            reg-names = "dsi_phy",
+                        "dsi_phy_lane",
+                        "dsi_pll";
+
+            #clock-cells = <1>;
+            #phy-cells = <0>;
+
+            clocks = <&dispcc_iface_clk>,
+                     <&rpmhcc_ref_clk>;
+            clock-names = "iface", "ref";
+
+            vdds-supply = <&vreg_dsi_supply>;
+        };
+
+        dsi@ae96000 {
+            compatible = "qcom,sa8775p-dsi-ctrl", "qcom,mdss-dsi-ctrl";
+            reg = <0x0ae96000 0x400>;
+            reg-names = "dsi_ctrl";
+
+            interrupt-parent = <&mdss>;
+            interrupts = <4>;
+
+            clocks = <&dispc_byte_clk>,
+                     <&dispcc_intf_clk>,
+                     <&dispcc_pclk>,
+                     <&dispcc_esc_clk>,
+                     <&dispcc_ahb_clk>,
+                     <&gcc_bus_clk>;
+            clock-names = "byte",
+                          "byte_intf",
+                          "pixel",
+                          "core",
+                          "iface",
+                          "bus";
+            assigned-clocks = <&dispcc_byte_clk>,
+                              <&dispcc_pclk>;
+            assigned-clock-parents = <&mdss0_dsi1_phy 0>, <&mdss0_dsi1_phy 1>;
+            phys = <&mdss0_dsi1_phy>;
+
+            operating-points-v2 = <&dsi1_opp_table>;
+            power-domains = <&rpmhpd SA8775P_MMCX>;
+
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            ports {
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                port@0 {
+                    reg = <0>;
+                    mdss0_dsi1_in: endpoint {
+                        remote-endpoint = <&dpu_intf2_out>;
+                    };
+                };
+
+                port@1 {
+                    reg = <1>;
+                    mdss0_dsi1_out: endpoint { };
+                };
+            };
+
+            dsi1_opp_table: opp-table {
+                compatible = "operating-points-v2";
+
+                opp-358000000 {
+                    opp-hz = /bits/ 64 <358000000>;
+                    required-opps = <&rpmhpd_opp_svs_l1>;
+                };
+            };
+        };
+
+        mdss0_dsi1_phy: phy@ae96400 {
+            compatible = "qcom,sa8775p-dsi-phy-5nm";
+            reg = <0x0ae96400 0x200>,
+                  <0x0ae96600 0x280>,
+                  <0x0ae96900 0x27c>;
+            reg-names = "dsi_phy",
+                        "dsi_phy_lane",
+                        "dsi_pll";
+
+            #clock-cells = <1>;
+            #phy-cells = <0>;
+
+            clocks = <&dispcc_iface_clk>,
+                     <&rpmhcc_ref_clk>;
+            clock-names = "iface", "ref";
+
+            vdds-supply = <&vreg_dsi_supply>;
+        };
+
         displayport-controller@af54000 {
             compatible = "qcom,sa8775p-dp";
 
-- 
2.34.1

Re: [PATCH v2 03/10] dt-bindings: display: msm: document DSI controller and phy on SA8775P

[Dmitry Baryshkov]

On Tue, Mar 11, 2025 at 05:54:38PM +0530, Ayushi Makhija wrote:
> Document DSI controller and phy on SA8775P platform.
> 
> Signed-off-by: Ayushi Makhija <quic_amakhija@quicinc.com>
> ---
>  .../display/msm/qcom,sa8775p-mdss.yaml        | 188 ++++++++++++++++++
>  1 file changed, 188 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/msm/qcom,sa8775p-mdss.yaml b/Documentation/devicetree/bindings/display/msm/qcom,sa8775p-mdss.yaml
> index a90a8b3f1a9e..628ca68871f4 100644
> --- a/Documentation/devicetree/bindings/display/msm/qcom,sa8775p-mdss.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/qcom,sa8775p-mdss.yaml
> @@ -52,6 +52,26 @@ patternProperties:
>          items:
>            - const: qcom,sa8775p-dp
>  
> +  "^dsi@[0-9a-f]+$":
> +    type: object
> +    additionalProperties: true
> +
> +    properties:
> +      compatible:
> +        items:
> +          - const: qcom,sa8775p-dsi-ctrl
> +          - const: qcom,mdss-dsi-ctrl
> +
> +  "^phy@[0-9a-f]+$":
> +    type: object
> +    additionalProperties: true
> +
> +    properties:
> +      compatible:
> +        enum:
> +          - qcom,sa8775p-dsi-phy-5nm
> +          - qcom,sa8775p-edp-phy

This does more than is written in the commit message: it also adds eDP
PHY as a possible child. Please expand the commit message to account for
that.

> +
>  required:
>    - compatible
>  
> @@ -131,6 +151,20 @@ examples:
>                          remote-endpoint = <&mdss0_dp0_in>;
>                      };
>                  };
> +
> +                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>;
> +                    };
> +                };
>              };
>  
>              mdss0_mdp_opp_table: opp-table {
> @@ -158,6 +192,160 @@ examples:
>              };
>          };
>  
> +        dsi@ae94000 {
> +            compatible = "qcom,sa8775p-dsi-ctrl", "qcom,mdss-dsi-ctrl";
> +            reg = <0x0ae94000 0x400>;
> +            reg-names = "dsi_ctrl";
> +
> +            interrupt-parent = <&mdss>;
> +            interrupts = <4>;
> +
> +            clocks = <&dispc_byte_clk>,
> +                     <&dispcc_intf_clk>,
> +                     <&dispcc_pclk>,
> +                     <&dispcc_esc_clk>,
> +                     <&dispcc_ahb_clk>,
> +                     <&gcc_bus_clk>;
> +            clock-names = "byte",
> +                          "byte_intf",
> +                          "pixel",
> +                          "core",
> +                          "iface",
> +                          "bus";
> +            assigned-clocks = <&dispcc_byte_clk>,
> +                              <&dispcc_pclk>;
> +            assigned-clock-parents = <&mdss0_dsi0_phy 0>, <&mdss0_dsi0_phy 1>;
> +            phys = <&mdss0_dsi0_phy>;
> +
> +            operating-points-v2 = <&dsi0_opp_table>;
> +            power-domains = <&rpmhpd SA8775P_MMCX>;
> +
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            ports {
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +
> +                port@0 {
> +                    reg = <0>;
> +                    mdss0_dsi0_in: endpoint {
> +                        remote-endpoint = <&dpu_intf1_out>;
> +                    };
> +                };
> +
> +                port@1 {
> +                    reg = <1>;
> +                    mdss0_dsi0_out: endpoint { };
> +                };
> +            };
> +
> +            dsi0_opp_table: opp-table {
> +                compatible = "operating-points-v2";
> +
> +                opp-358000000 {
> +                    opp-hz = /bits/ 64 <358000000>;
> +                    required-opps = <&rpmhpd_opp_svs_l1>;
> +                };
> +            };
> +        };
> +
> +        mdss0_dsi0_phy: phy@ae94400 {
> +            compatible = "qcom,sa8775p-dsi-phy-5nm";
> +            reg = <0x0ae94400 0x200>,
> +                  <0x0ae94600 0x280>,
> +                  <0x0ae94900 0x27c>;
> +            reg-names = "dsi_phy",
> +                        "dsi_phy_lane",
> +                        "dsi_pll";
> +
> +            #clock-cells = <1>;
> +            #phy-cells = <0>;
> +
> +            clocks = <&dispcc_iface_clk>,
> +                     <&rpmhcc_ref_clk>;
> +            clock-names = "iface", "ref";
> +
> +            vdds-supply = <&vreg_dsi_supply>;
> +        };
> +
> +        dsi@ae96000 {
> +            compatible = "qcom,sa8775p-dsi-ctrl", "qcom,mdss-dsi-ctrl";
> +            reg = <0x0ae96000 0x400>;
> +            reg-names = "dsi_ctrl";
> +
> +            interrupt-parent = <&mdss>;
> +            interrupts = <4>;
> +
> +            clocks = <&dispc_byte_clk>,
> +                     <&dispcc_intf_clk>,
> +                     <&dispcc_pclk>,
> +                     <&dispcc_esc_clk>,
> +                     <&dispcc_ahb_clk>,
> +                     <&gcc_bus_clk>;
> +            clock-names = "byte",
> +                          "byte_intf",
> +                          "pixel",
> +                          "core",
> +                          "iface",
> +                          "bus";
> +            assigned-clocks = <&dispcc_byte_clk>,
> +                              <&dispcc_pclk>;
> +            assigned-clock-parents = <&mdss0_dsi1_phy 0>, <&mdss0_dsi1_phy 1>;
> +            phys = <&mdss0_dsi1_phy>;
> +
> +            operating-points-v2 = <&dsi1_opp_table>;
> +            power-domains = <&rpmhpd SA8775P_MMCX>;
> +
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            ports {
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +
> +                port@0 {
> +                    reg = <0>;
> +                    mdss0_dsi1_in: endpoint {
> +                        remote-endpoint = <&dpu_intf2_out>;
> +                    };
> +                };
> +
> +                port@1 {
> +                    reg = <1>;
> +                    mdss0_dsi1_out: endpoint { };
> +                };
> +            };
> +
> +            dsi1_opp_table: opp-table {
> +                compatible = "operating-points-v2";
> +
> +                opp-358000000 {
> +                    opp-hz = /bits/ 64 <358000000>;
> +                    required-opps = <&rpmhpd_opp_svs_l1>;
> +                };
> +            };
> +        };
> +
> +        mdss0_dsi1_phy: phy@ae96400 {
> +            compatible = "qcom,sa8775p-dsi-phy-5nm";
> +            reg = <0x0ae96400 0x200>,
> +                  <0x0ae96600 0x280>,
> +                  <0x0ae96900 0x27c>;
> +            reg-names = "dsi_phy",
> +                        "dsi_phy_lane",
> +                        "dsi_pll";
> +
> +            #clock-cells = <1>;
> +            #phy-cells = <0>;
> +
> +            clocks = <&dispcc_iface_clk>,
> +                     <&rpmhcc_ref_clk>;
> +            clock-names = "iface", "ref";
> +
> +            vdds-supply = <&vreg_dsi_supply>;
> +        };
> +
>          displayport-controller@af54000 {
>              compatible = "qcom,sa8775p-dp";
>  
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH v2 03/10] dt-bindings: display: msm: document DSI controller and phy on SA8775P

[Krzysztof Kozlowski]

On Tue, Mar 11, 2025 at 05:54:38PM +0530, Ayushi Makhija wrote:
> Document DSI controller and phy on SA8775P platform.
> 
> Signed-off-by: Ayushi Makhija <quic_amakhija@quicinc.com>
> ---
>  .../display/msm/qcom,sa8775p-mdss.yaml        | 188 ++++++++++++++++++
>  1 file changed, 188 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/msm/qcom,sa8775p-mdss.yaml b/Documentation/devicetree/bindings/display/msm/qcom,sa8775p-mdss.yaml
> index a90a8b3f1a9e..628ca68871f4 100644
> --- a/Documentation/devicetree/bindings/display/msm/qcom,sa8775p-mdss.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/qcom,sa8775p-mdss.yaml
> @@ -52,6 +52,26 @@ patternProperties:
>          items:
>            - const: qcom,sa8775p-dp
>  
> +  "^dsi@[0-9a-f]+$":
> +    type: object
> +    additionalProperties: true
> +

Drop blank line

> +    properties:
> +      compatible:
> +        items:

contains

> +          - const: qcom,sa8775p-dsi-ctrl
> +          - const: qcom,mdss-dsi-ctrl

Drop fallback

Same comments further

Best regards,
Krzysztof

Re: [PATCH v2 03/10] dt-bindings: display: msm: document DSI controller and phy on SA8775P

[Ayushi Makhija]

On 3/12/2025 5:15 PM, Krzysztof Kozlowski wrote:
> On Tue, Mar 11, 2025 at 05:54:38PM +0530, Ayushi Makhija wrote:
>> Document DSI controller and phy on SA8775P platform.
>>
>> Signed-off-by: Ayushi Makhija <quic_amakhija@quicinc.com>
>> ---
>>  .../display/msm/qcom,sa8775p-mdss.yaml        | 188 ++++++++++++++++++
>>  1 file changed, 188 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/display/msm/qcom,sa8775p-mdss.yaml b/Documentation/devicetree/bindings/display/msm/qcom,sa8775p-mdss.yaml
>> index a90a8b3f1a9e..628ca68871f4 100644
>> --- a/Documentation/devicetree/bindings/display/msm/qcom,sa8775p-mdss.yaml
>> +++ b/Documentation/devicetree/bindings/display/msm/qcom,sa8775p-mdss.yaml
>> @@ -52,6 +52,26 @@ patternProperties:
>>          items:
>>            - const: qcom,sa8775p-dp
>>  
>> +  "^dsi@[0-9a-f]+$":
>> +    type: object
>> +    additionalProperties: true
>> +
> 
> Drop blank line
> 
>> +    properties:
>> +      compatible:
>> +        items:
> 
> contains
> 
>> +          - const: qcom,sa8775p-dsi-ctrl
>> +          - const: qcom,mdss-dsi-ctrl
> 
> Drop fallback
> 
> Same comments further
> 
> Best regards,
> Krzysztof
> 

Hi Krzysztof,

Thanks, for the review.

Will address the above comments in the next patch.

+  "^dsi@[0-9a-f]+$":
+    type: object
+    additionalProperties: true
+    properties:
+      compatible:
+        contains:
+          enum:
+            - qcom,sa8775p-dsi-ctrl
+            - qcom,mdss-dsi-ctrl
+
+  "^phy@[0-9a-f]+$":
+    type: object
+    additionalProperties: true
+    properties:
+      compatible:
+        contains:
+          enum:
+            - qcom,sa8775p-dsi-phy-5nm
+            - qcom,sa8775p-edp-phy

Thanks,
Ayushi

Re: [PATCH v2 03/10] dt-bindings: display: msm: document DSI controller and phy on SA8775P

[Ayushi Makhija]

On 3/12/2025 5:15 PM, Krzysztof Kozlowski wrote:
> On Tue, Mar 11, 2025 at 05:54:38PM +0530, Ayushi Makhija wrote:
>> Document DSI controller and phy on SA8775P platform.
>>
>> Signed-off-by: Ayushi Makhija <quic_amakhija@quicinc.com>
>> ---
>>  .../display/msm/qcom,sa8775p-mdss.yaml        | 188 ++++++++++++++++++
>>  1 file changed, 188 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/display/msm/qcom,sa8775p-mdss.yaml b/Documentation/devicetree/bindings/display/msm/qcom,sa8775p-mdss.yaml
>> index a90a8b3f1a9e..628ca68871f4 100644
>> --- a/Documentation/devicetree/bindings/display/msm/qcom,sa8775p-mdss.yaml
>> +++ b/Documentation/devicetree/bindings/display/msm/qcom,sa8775p-mdss.yaml
>> @@ -52,6 +52,26 @@ patternProperties:
>>          items:
>>            - const: qcom,sa8775p-dp
>>  
>> +  "^dsi@[0-9a-f]+$":
>> +    type: object
>> +    additionalProperties: true
>> +
> 
> Drop blank line
> 
>> +    properties:
>> +      compatible:
>> +        items:
> 
> contains
> 
>> +          - const: qcom,sa8775p-dsi-ctrl
>> +          - const: qcom,mdss-dsi-ctrl
> 
> Drop fallback
>
 
Hi Krzysztof,

I couldn't understand the meaning of "Drop fallback", could please elaborate it ?

Thanks,
Ayushi

Re: [PATCH v2 03/10] dt-bindings: display: msm: document DSI controller and phy on SA8775P

[Krzysztof Kozlowski]

On 08/04/2025 12:38, Ayushi Makhija wrote:
>>> +    properties:
>>> +      compatible:
>>> +        items:
>>
>> contains
>>
>>> +          - const: qcom,sa8775p-dsi-ctrl
>>> +          - const: qcom,mdss-dsi-ctrl
>>
>> Drop fallback
>>
>  
> Hi Krzysztof,
> 
> I couldn't understand the meaning of "Drop fallback", could please elaborate it ?
Look at SM8750 example on the lists. Keep only front compatible.

Best regards,
Krzysztof

Re: [PATCH v2 03/10] dt-bindings: display: msm: document DSI controller and phy on SA8775P

[Dmitry Baryshkov]

On Tue, Apr 08, 2025 at 01:03:53PM +0200, Krzysztof Kozlowski wrote:
> On 08/04/2025 12:38, Ayushi Makhija wrote:
> >>> +    properties:
> >>> +      compatible:
> >>> +        items:
> >>
> >> contains
> >>
> >>> +          - const: qcom,sa8775p-dsi-ctrl
> >>> +          - const: qcom,mdss-dsi-ctrl
> >>
> >> Drop fallback
> >>
> >  
> > Hi Krzysztof,
> > 
> > I couldn't understand the meaning of "Drop fallback", could please elaborate it ?
> Look at SM8750 example on the lists. Keep only front compatible.

Why?

-- 
With best wishes
Dmitry

Re: [PATCH v2 03/10] dt-bindings: display: msm: document DSI controller and phy on SA8775P

[Krzysztof Kozlowski]

On 08/04/2025 13:44, Dmitry Baryshkov wrote:
> On Tue, Apr 08, 2025 at 01:03:53PM +0200, Krzysztof Kozlowski wrote:
>> On 08/04/2025 12:38, Ayushi Makhija wrote:
>>>>> +    properties:
>>>>> +      compatible:
>>>>> +        items:
>>>>
>>>> contains
>>>>
>>>>> +          - const: qcom,sa8775p-dsi-ctrl
>>>>> +          - const: qcom,mdss-dsi-ctrl
>>>>
>>>> Drop fallback
>>>>
>>>  
>>> Hi Krzysztof,
>>>
>>> I couldn't understand the meaning of "Drop fallback", could please elaborate it ?
>> Look at SM8750 example on the lists. Keep only front compatible.
> 
> Why?

To make things simpler and shorter.

Best regards,
Krzysztof

Re: [PATCH v2 03/10] dt-bindings: display: msm: document DSI controller and phy on SA8775P

[Dmitry Baryshkov]

On 08/04/2025 21:42, Krzysztof Kozlowski wrote:
> On 08/04/2025 13:44, Dmitry Baryshkov wrote:
>> On Tue, Apr 08, 2025 at 01:03:53PM +0200, Krzysztof Kozlowski wrote:
>>> On 08/04/2025 12:38, Ayushi Makhija wrote:
>>>>>> +    properties:
>>>>>> +      compatible:
>>>>>> +        items:
>>>>>
>>>>> contains
>>>>>
>>>>>> +          - const: qcom,sa8775p-dsi-ctrl
>>>>>> +          - const: qcom,mdss-dsi-ctrl
>>>>>
>>>>> Drop fallback
>>>>>
>>>>   
>>>> Hi Krzysztof,
>>>>
>>>> I couldn't understand the meaning of "Drop fallback", could please elaborate it ?
>>> Look at SM8750 example on the lists. Keep only front compatible.
>>
>> Why?
> 
> To make things simpler and shorter.

I'd prefer consistency. Previous platforms use qcom,mdss-dsi-ctrl.


-- 
With best wishes
Dmitry

Re: [PATCH v2 03/10] dt-bindings: display: msm: document DSI controller and phy on SA8775P

[Krzysztof Kozlowski]

On 08/04/2025 22:26, Dmitry Baryshkov wrote:
>>>>>>> +          - const: qcom,sa8775p-dsi-ctrl
>>>>>>> +          - const: qcom,mdss-dsi-ctrl
>>>>>>
>>>>>> Drop fallback
>>>>>>
>>>>>   
>>>>> Hi Krzysztof,
>>>>>
>>>>> I couldn't understand the meaning of "Drop fallback", could please elaborate it ?
>>>> Look at SM8750 example on the lists. Keep only front compatible.
>>>
>>> Why?
>>
>> To make things simpler and shorter.
> 
> I'd prefer consistency. Previous platforms use qcom,mdss-dsi-ctrl.
Then you should have objected month(s) ago when Rob asked for dropping
fallback and since then we consistently drop it.

Best regards,
Krzysztof

Re: [PATCH v2 03/10] dt-bindings: display: msm: document DSI controller and phy on SA8775P

[Dmitry Baryshkov]

On 09/04/2025 09:07, Krzysztof Kozlowski wrote:
> On 08/04/2025 22:26, Dmitry Baryshkov wrote:
>>>>>>>> +          - const: qcom,sa8775p-dsi-ctrl
>>>>>>>> +          - const: qcom,mdss-dsi-ctrl
>>>>>>>
>>>>>>> Drop fallback
>>>>>>>
>>>>>>    
>>>>>> Hi Krzysztof,
>>>>>>
>>>>>> I couldn't understand the meaning of "Drop fallback", could please elaborate it ?
>>>>> Look at SM8750 example on the lists. Keep only front compatible.
>>>>
>>>> Why?
>>>
>>> To make things simpler and shorter.
>>
>> I'd prefer consistency. Previous platforms use qcom,mdss-dsi-ctrl.
> Then you should have objected month(s) ago when Rob asked for dropping
> fallback and since then we consistently drop it.

Well... It's still not merged. is it?
For SM8750 it kinda makes sense, because the clock handling is different 
from all other current platforms. For the existing devices... I'm not 
that sure.

> 
> Best regards,
> Krzysztof


-- 
With best wishes
Dmitry

Re: [PATCH v2 03/10] dt-bindings: display: msm: document DSI controller and phy on SA8775P

[Krzysztof Kozlowski]

On 09/04/2025 17:24, Dmitry Baryshkov wrote:
> On 09/04/2025 09:07, Krzysztof Kozlowski wrote:
>> On 08/04/2025 22:26, Dmitry Baryshkov wrote:
>>>>>>>>> +          - const: qcom,sa8775p-dsi-ctrl
>>>>>>>>> +          - const: qcom,mdss-dsi-ctrl
>>>>>>>>
>>>>>>>> Drop fallback
>>>>>>>>
>>>>>>>    
>>>>>>> Hi Krzysztof,
>>>>>>>
>>>>>>> I couldn't understand the meaning of "Drop fallback", could please elaborate it ?
>>>>>> Look at SM8750 example on the lists. Keep only front compatible.
>>>>>
>>>>> Why?
>>>>
>>>> To make things simpler and shorter.
>>>
>>> I'd prefer consistency. Previous platforms use qcom,mdss-dsi-ctrl.
>> Then you should have objected month(s) ago when Rob asked for dropping
>> fallback and since then we consistently drop it.
> 
> Well... It's still not merged. is it?
> For SM8750 it kinda makes sense, because the clock handling is different 
> from all other current platforms. For the existing devices... I'm not 
> that sure.
How does it differ? The clock handling does not matter - this is just
select of schema for the child node.

Best regards,
Krzysztof

Re: [PATCH v2 03/10] dt-bindings: display: msm: document DSI controller and phy on SA8775P

[Dmitry Baryshkov]

On Thu, Apr 10, 2025 at 08:08:17AM +0200, Krzysztof Kozlowski wrote:
> On 09/04/2025 17:24, Dmitry Baryshkov wrote:
> > On 09/04/2025 09:07, Krzysztof Kozlowski wrote:
> >> On 08/04/2025 22:26, Dmitry Baryshkov wrote:
> >>>>>>>>> +          - const: qcom,sa8775p-dsi-ctrl
> >>>>>>>>> +          - const: qcom,mdss-dsi-ctrl
> >>>>>>>>
> >>>>>>>> Drop fallback
> >>>>>>>>
> >>>>>>>    
> >>>>>>> Hi Krzysztof,
> >>>>>>>
> >>>>>>> I couldn't understand the meaning of "Drop fallback", could please elaborate it ?
> >>>>>> Look at SM8750 example on the lists. Keep only front compatible.
> >>>>>
> >>>>> Why?
> >>>>
> >>>> To make things simpler and shorter.
> >>>
> >>> I'd prefer consistency. Previous platforms use qcom,mdss-dsi-ctrl.
> >> Then you should have objected month(s) ago when Rob asked for dropping
> >> fallback and since then we consistently drop it.
> > 
> > Well... It's still not merged. is it?
> > For SM8750 it kinda makes sense, because the clock handling is different 
> > from all other current platforms. For the existing devices... I'm not 
> > that sure.
> How does it differ? The clock handling does not matter - this is just
> select of schema for the child node.

Ah.... I'm sorry, I misinterpreted the email. Yes, having only a single
compat here is perfectly fine. Please excuse me for the confusion.

-- 
With best wishes
Dmitry

Re: [PATCH v2 03/10] dt-bindings: display: msm: document DSI controller and phy on SA8775P

[Ayushi Makhija]

Hi Krzysztof/Dmitry,

On 4/10/2025 2:46 PM, Dmitry Baryshkov wrote:
> On Thu, Apr 10, 2025 at 08:08:17AM +0200, Krzysztof Kozlowski wrote:
>> On 09/04/2025 17:24, Dmitry Baryshkov wrote:
>>> On 09/04/2025 09:07, Krzysztof Kozlowski wrote:
>>>> On 08/04/2025 22:26, Dmitry Baryshkov wrote:
>>>>>>>>>>> +          - const: qcom,sa8775p-dsi-ctrl
>>>>>>>>>>> +          - const: qcom,mdss-dsi-ctrl
>>>>>>>>>>
>>>>>>>>>> Drop fallback
>>>>>>>>>>
>>>>>>>>>    
>>>>>>>>> Hi Krzysztof,
>>>>>>>>>
>>>>>>>>> I couldn't understand the meaning of "Drop fallback", could please elaborate it ?
>>>>>>>> Look at SM8750 example on the lists. Keep only front compatible.
>>>>>>>
>>>>>>> Why?
>>>>>>
>>>>>> To make things simpler and shorter.
>>>>>
>>>>> I'd prefer consistency. Previous platforms use qcom,mdss-dsi-ctrl.
>>>> Then you should have objected month(s) ago when Rob asked for dropping
>>>> fallback and since then we consistently drop it.
>>>
>>> Well... It's still not merged. is it?
>>> For SM8750 it kinda makes sense, because the clock handling is different 
>>> from all other current platforms. For the existing devices... I'm not 
>>> that sure.
>> How does it differ? The clock handling does not matter - this is just
>> select of schema for the child node.
> 
> Ah.... I'm sorry, I misinterpreted the email. Yes, having only a single
> compat here is perfectly fine. Please excuse me for the confusion.
> 

Thanks, for the review.

I will write single compatible string. Will update it in next patchset.

+  "^dsi@[0-9a-f]+$":
+    type: object
+    additionalProperties: true
+    properties:
+      compatible:
+        contains:
+          const: qcom,sa8775p-dsi-ctrl
+

Thanks,
Ayushi

[PATCH v2 04/10] drm/msm/dsi: add DSI PHY configuration on SA8775P

[Ayushi Makhija]

The SA8775P SoC uses the 5nm (v4.2) DSI PHY driver with
different enable regulator load.

Signed-off-by: Ayushi Makhija <quic_amakhija@quicinc.com>
---
 drivers/gpu/drm/msm/dsi/phy/dsi_phy.c     |  2 ++
 drivers/gpu/drm/msm/dsi/phy/dsi_phy.h     |  1 +
 drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c | 27 +++++++++++++++++++++++
 3 files changed, 30 insertions(+)

diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
index c0bcc6828963..92be08ac5f65 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
@@ -581,6 +581,8 @@ static const struct of_device_id dsi_phy_dt_match[] = {
 	  .data = &dsi_phy_7nm_cfgs },
 	{ .compatible = "qcom,dsi-phy-7nm-8150",
 	  .data = &dsi_phy_7nm_8150_cfgs },
+	{ .compatible = "qcom,sa8775p-dsi-phy-5nm",
+	  .data = &dsi_phy_5nm_8775p_cfgs },
 	{ .compatible = "qcom,sc7280-dsi-phy-7nm",
 	  .data = &dsi_phy_7nm_7280_cfgs },
 	{ .compatible = "qcom,sm6375-dsi-phy-7nm",
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
index 8985818bb2e0..22f23a5b9fa0 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
@@ -58,6 +58,7 @@ extern const struct msm_dsi_phy_cfg dsi_phy_7nm_8150_cfgs;
 extern const struct msm_dsi_phy_cfg dsi_phy_7nm_7280_cfgs;
 extern const struct msm_dsi_phy_cfg dsi_phy_5nm_8350_cfgs;
 extern const struct msm_dsi_phy_cfg dsi_phy_5nm_8450_cfgs;
+extern const struct msm_dsi_phy_cfg dsi_phy_5nm_8775p_cfgs;
 extern const struct msm_dsi_phy_cfg dsi_phy_4nm_8550_cfgs;
 extern const struct msm_dsi_phy_cfg dsi_phy_4nm_8650_cfgs;
 
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
index 798168180c1a..598ea183c746 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
@@ -1146,6 +1146,10 @@ static const struct regulator_bulk_data dsi_phy_7nm_37750uA_regulators[] = {
 	{ .supply = "vdds", .init_load_uA = 37550 },
 };
 
+static const struct regulator_bulk_data dsi_phy_7nm_48000uA_regulators[] = {
+	{ .supply = "vdds", .init_load_uA = 48000 },
+};
+
 static const struct regulator_bulk_data dsi_phy_7nm_98000uA_regulators[] = {
 	{ .supply = "vdds", .init_load_uA = 98000 },
 };
@@ -1288,6 +1292,29 @@ const struct msm_dsi_phy_cfg dsi_phy_5nm_8450_cfgs = {
 	.quirks = DSI_PHY_7NM_QUIRK_V4_3,
 };
 
+const struct msm_dsi_phy_cfg dsi_phy_5nm_8775p_cfgs = {
+	.has_phy_lane = true,
+	.regulator_data = dsi_phy_7nm_48000uA_regulators,
+	.num_regulators = ARRAY_SIZE(dsi_phy_7nm_48000uA_regulators),
+	.ops = {
+		.enable = dsi_7nm_phy_enable,
+		.disable = dsi_7nm_phy_disable,
+		.pll_init = dsi_pll_7nm_init,
+		.save_pll_state = dsi_7nm_pll_save_state,
+		.restore_pll_state = dsi_7nm_pll_restore_state,
+		.set_continuous_clock = dsi_7nm_set_continuous_clock,
+		},
+	.min_pll_rate = 600000000UL,
+#ifdef CONFIG_64BIT
+	.max_pll_rate = 5000000000UL,
+#else
+	.max_pll_rate = ULONG_MAX,
+#endif
+	.io_start = { 0xae94400, 0xae96400 },
+	.num_dsi_phy = 2,
+	.quirks = DSI_PHY_7NM_QUIRK_V4_2,
+};
+
 const struct msm_dsi_phy_cfg dsi_phy_4nm_8550_cfgs = {
 	.has_phy_lane = true,
 	.regulator_data = dsi_phy_7nm_98400uA_regulators,
-- 
2.34.1

Re: [PATCH v2 04/10] drm/msm/dsi: add DSI PHY configuration on SA8775P

[Dmitry Baryshkov]

On Tue, Mar 11, 2025 at 05:54:39PM +0530, Ayushi Makhija wrote:
> The SA8775P SoC uses the 5nm (v4.2) DSI PHY driver with
> different enable regulator load.
> 
> Signed-off-by: Ayushi Makhija <quic_amakhija@quicinc.com>
> ---
>  drivers/gpu/drm/msm/dsi/phy/dsi_phy.c     |  2 ++
>  drivers/gpu/drm/msm/dsi/phy/dsi_phy.h     |  1 +
>  drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c | 27 +++++++++++++++++++++++
>  3 files changed, 30 insertions(+)
> 

Reviewed-by: Dmitry Baryshkov <lumag@kernel.org>

-- 
With best wishes
Dmitry

[PATCH v2 05/10] drm/msm/dsi: add DSI support for SA8775P

[Ayushi Makhija]

Add DSI Controller v2.5.1 support for SA8775P SoC.

Signed-off-by: Ayushi Makhija <quic_amakhija@quicinc.com>
---
 drivers/gpu/drm/msm/dsi/dsi_cfg.c | 18 ++++++++++++++++++
 drivers/gpu/drm/msm/dsi/dsi_cfg.h |  1 +
 2 files changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.c b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
index 7754dcec33d0..7675558ae2e5 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_cfg.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
@@ -221,6 +221,22 @@ static const struct msm_dsi_config sc7280_dsi_cfg = {
 	},
 };
 
+static const struct regulator_bulk_data sa8775p_dsi_regulators[] = {
+	{ .supply = "vdda", .init_load_uA = 8300 },    /* 1.2 V */
+	{ .supply = "refgen" },
+};
+
+static const struct msm_dsi_config sa8775p_dsi_cfg = {
+	.io_offset = DSI_6G_REG_SHIFT,
+	.regulator_data = sa8775p_dsi_regulators,
+	.num_regulators = ARRAY_SIZE(sa8775p_dsi_regulators),
+	.bus_clk_names = dsi_v2_4_clk_names,
+	.num_bus_clks = ARRAY_SIZE(dsi_v2_4_clk_names),
+	.io_start = {
+		{ 0xae94000, 0xae96000 },
+	},
+};
+
 static const struct msm_dsi_host_cfg_ops msm_dsi_v2_host_ops = {
 	.link_clk_set_rate = dsi_link_clk_set_rate_v2,
 	.link_clk_enable = dsi_link_clk_enable_v2,
@@ -294,6 +310,8 @@ static const struct msm_dsi_cfg_handler dsi_cfg_handlers[] = {
 		&sdm845_dsi_cfg, &msm_dsi_6g_v2_host_ops},
 	{MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V2_5_0,
 		&sc7280_dsi_cfg, &msm_dsi_6g_v2_host_ops},
+	{MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V2_5_1,
+		&sa8775p_dsi_cfg, &msm_dsi_6g_v2_host_ops},
 	{MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V2_6_0,
 		&sdm845_dsi_cfg, &msm_dsi_6g_v2_host_ops},
 	{MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V2_7_0,
diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.h b/drivers/gpu/drm/msm/dsi/dsi_cfg.h
index 120cb65164c1..65b0705fac0e 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_cfg.h
+++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.h
@@ -27,6 +27,7 @@
 #define MSM_DSI_6G_VER_MINOR_V2_4_0	0x20040000
 #define MSM_DSI_6G_VER_MINOR_V2_4_1	0x20040001
 #define MSM_DSI_6G_VER_MINOR_V2_5_0	0x20050000
+#define MSM_DSI_6G_VER_MINOR_V2_5_1	0x20050001
 #define MSM_DSI_6G_VER_MINOR_V2_6_0	0x20060000
 #define MSM_DSI_6G_VER_MINOR_V2_7_0	0x20070000
 #define MSM_DSI_6G_VER_MINOR_V2_8_0	0x20080000
-- 
2.34.1

Re: [PATCH v2 05/10] drm/msm/dsi: add DSI support for SA8775P

[Dmitry Baryshkov]

On Tue, Mar 11, 2025 at 05:54:40PM +0530, Ayushi Makhija wrote:
> Add DSI Controller v2.5.1 support for SA8775P SoC.
> 
> Signed-off-by: Ayushi Makhija <quic_amakhija@quicinc.com>
> ---
>  drivers/gpu/drm/msm/dsi/dsi_cfg.c | 18 ++++++++++++++++++
>  drivers/gpu/drm/msm/dsi/dsi_cfg.h |  1 +
>  2 files changed, 19 insertions(+)
> 

Reviewed-by: Dmitry Baryshkov <lumag@kernel.org>

-- 
With best wishes
Dmitry

[PATCH v2 06/10] arm64: dts: qcom: sa8775p: add Display Serial Interface device nodes

[Ayushi Makhija]

Add device tree nodes for the DSI0 and DSI1 controllers
with their corresponding PHYs found on Qualcomm SA8775P SoC.

Signed-off-by: Ayushi Makhija <quic_amakhija@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sa8775p.dtsi | 186 +++++++++++++++++++++++++-
 1 file changed, 185 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
index 3394ae2d1300..edc5130f4bae 100644
--- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi
+++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
@@ -6,6 +6,7 @@
 
 #include <dt-bindings/interconnect/qcom,icc.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/clock/qcom,dsi-phy-28nm.h>
 #include <dt-bindings/clock/qcom,rpmh.h>
 #include <dt-bindings/clock/qcom,sa8775p-dispcc.h>
 #include <dt-bindings/clock/qcom,sa8775p-gcc.h>
@@ -3890,6 +3891,22 @@ dpu_intf4_out: endpoint {
 							remote-endpoint = <&mdss0_dp1_in>;
 						};
 					};
+
+					port@2 {
+						reg = <2>;
+
+						dpu_intf1_out: endpoint {
+							remote-endpoint = <&mdss0_dsi0_in>;
+						};
+					};
+
+					port@3 {
+						reg = <3>;
+
+						dpu_intf2_out: endpoint {
+							remote-endpoint = <&mdss0_dsi1_in>;
+						};
+					};
 				};
 
 				mdss0_mdp_opp_table: opp-table {
@@ -3917,6 +3934,170 @@ opp-650000000 {
 				};
 			};
 
+			mdss0_dsi0: dsi@ae94000 {
+				compatible = "qcom,sa8775p-dsi-ctrl", "qcom,mdss-dsi-ctrl";
+				reg = <0x0 0x0ae94000 0x0 0x400>;
+				reg-names = "dsi_ctrl";
+
+				interrupt-parent = <&mdss0>;
+				interrupts = <4>;
+
+				clocks = <&dispcc0 MDSS_DISP_CC_MDSS_BYTE0_CLK>,
+					 <&dispcc0 MDSS_DISP_CC_MDSS_BYTE0_INTF_CLK>,
+					 <&dispcc0 MDSS_DISP_CC_MDSS_PCLK0_CLK>,
+					 <&dispcc0 MDSS_DISP_CC_MDSS_ESC0_CLK>,
+					 <&dispcc0 MDSS_DISP_CC_MDSS_AHB_CLK>,
+					 <&gcc GCC_DISP_HF_AXI_CLK>;
+				clock-names = "byte",
+					      "byte_intf",
+					      "pixel",
+					      "core",
+					      "iface",
+					      "bus";
+				assigned-clocks = <&dispcc0 MDSS_DISP_CC_MDSS_BYTE0_CLK_SRC>,
+						  <&dispcc0 MDSS_DISP_CC_MDSS_PCLK0_CLK_SRC>;
+				assigned-clock-parents = <&mdss0_dsi0_phy DSI_BYTE_PLL_CLK>,
+							 <&mdss0_dsi0_phy DSI_PIXEL_PLL_CLK>;
+				phys = <&mdss0_dsi0_phy>;
+
+				operating-points-v2 = <&dsi0_opp_table>;
+				power-domains = <&rpmhpd SA8775P_MMCX>;
+
+				#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 = <&dpu_intf1_out>;
+						};
+					};
+
+					port@1 {
+						reg = <1>;
+
+						mdss0_dsi0_out: endpoint{ };
+					};
+				};
+
+				dsi0_opp_table: opp-table {
+					compatible = "operating-points-v2";
+
+					opp-358000000 {
+						opp-hz = /bits/ 64 <358000000>;
+						required-opps = <&rpmhpd_opp_svs_l1>;
+					};
+				};
+			};
+
+			mdss0_dsi0_phy: phy@ae94400 {
+				compatible = "qcom,sa8775p-dsi-phy-5nm";
+				reg = <0x0 0x0ae94400 0x0 0x200>,
+				      <0x0 0x0ae94600 0x0 0x280>,
+				      <0x0 0x0ae94900 0x0 0x27c>;
+				reg-names = "dsi_phy",
+					    "dsi_phy_lane",
+					    "dsi_pll";
+
+				#clock-cells = <1>;
+				#phy-cells = <0>;
+
+				clocks = <&dispcc0 MDSS_DISP_CC_MDSS_AHB_CLK>,
+					 <&rpmhcc RPMH_CXO_CLK>;
+				clock-names = "iface", "ref";
+
+				status = "disabled";
+			};
+
+			mdss0_dsi1: dsi@ae96000 {
+				compatible = "qcom,sa8775p-dsi-ctrl", "qcom,mdss-dsi-ctrl";
+				reg = <0x0 0x0ae96000 0x0 0x400>;
+				reg-names = "dsi_ctrl";
+
+				interrupt-parent = <&mdss0>;
+				interrupts = <5>;
+
+				clocks = <&dispcc0 MDSS_DISP_CC_MDSS_BYTE1_CLK>,
+					 <&dispcc0 MDSS_DISP_CC_MDSS_BYTE1_INTF_CLK>,
+					 <&dispcc0 MDSS_DISP_CC_MDSS_PCLK1_CLK>,
+					 <&dispcc0 MDSS_DISP_CC_MDSS_ESC1_CLK>,
+					 <&dispcc0 MDSS_DISP_CC_MDSS_AHB_CLK>,
+					 <&gcc GCC_DISP_HF_AXI_CLK>;
+				clock-names = "byte",
+					      "byte_intf",
+					      "pixel",
+					      "core",
+					      "iface",
+					      "bus";
+				assigned-clocks = <&dispcc0 MDSS_DISP_CC_MDSS_BYTE1_CLK_SRC>,
+						  <&dispcc0 MDSS_DISP_CC_MDSS_PCLK1_CLK_SRC>;
+				assigned-clock-parents = <&mdss0_dsi1_phy DSI_BYTE_PLL_CLK>,
+							 <&mdss0_dsi1_phy DSI_PIXEL_PLL_CLK>;
+				phys = <&mdss0_dsi1_phy>;
+
+				operating-points-v2 = <&dsi1_opp_table>;
+				power-domains = <&rpmhpd SA8775P_MMCX>;
+
+				#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 = <&dpu_intf2_out>;
+						};
+					};
+
+					port@1 {
+						reg = <1>;
+
+						mdss0_dsi1_out: endpoint { };
+					};
+				};
+
+				dsi1_opp_table: opp-table {
+					compatible = "operating-points-v2";
+
+					opp-358000000 {
+						opp-hz = /bits/ 64 <358000000>;
+						required-opps = <&rpmhpd_opp_svs_l1>;
+					};
+				};
+			};
+
+			mdss0_dsi1_phy: phy@ae96400 {
+				compatible = "qcom,sa8775p-dsi-phy-5nm";
+				reg = <0x0 0x0ae96400 0x0 0x200>,
+				      <0x0 0x0ae96600 0x0 0x280>,
+				      <0x0 0x0ae96900 0x0 0x27c>;
+				reg-names = "dsi_phy",
+					    "dsi_phy_lane",
+					    "dsi_pll";
+
+				#clock-cells = <1>;
+				#phy-cells = <0>;
+
+				clocks = <&dispcc0 MDSS_DISP_CC_MDSS_AHB_CLK>,
+					 <&rpmhcc RPMH_CXO_CLK>;
+				clock-names = "iface", "ref";
+
+				status = "disabled";
+			};
+
 			mdss0_dp0_phy: phy@aec2a00 {
 				compatible = "qcom,sa8775p-edp-phy";
 
@@ -4123,7 +4304,10 @@ dispcc0: clock-controller@af00000 {
 				 <&sleep_clk>,
 				 <&mdss0_dp0_phy 0>, <&mdss0_dp0_phy 1>,
 				 <&mdss0_dp1_phy 0>, <&mdss0_dp1_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 SA8775P_MMCX>;
 			#clock-cells = <1>;
 			#reset-cells = <1>;
-- 
2.34.1

Re: [PATCH v2 06/10] arm64: dts: qcom: sa8775p: add Display Serial Interface device nodes

[Dmitry Baryshkov]

On Tue, Mar 11, 2025 at 05:54:41PM +0530, Ayushi Makhija wrote:
> Add device tree nodes for the DSI0 and DSI1 controllers
> with their corresponding PHYs found on Qualcomm SA8775P SoC.
> 
> Signed-off-by: Ayushi Makhija <quic_amakhija@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/sa8775p.dtsi | 186 +++++++++++++++++++++++++-
>  1 file changed, 185 insertions(+), 1 deletion(-)
> 

Reviewed-by: Dmitry Baryshkov <lumag@kernel.org>

-- 
With best wishes
Dmitry

[PATCH v2 07/10] arm64: dts: qcom: sa8775p-ride: add anx7625 DSI to DP bridge nodes

[Ayushi Makhija]

Add anx7625 DSI to DP bridge device nodes.

Signed-off-by: Ayushi Makhija <quic_amakhija@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi | 208 ++++++++++++++++++++-
 1 file changed, 207 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
index 175f8b1e3b2d..77d86c1d8aa6 100644
--- a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
+++ b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
@@ -28,6 +28,13 @@ chosen {
 		stdout-path = "serial0:115200n8";
 	};
 
+	vph_pwr: vph-pwr-regulator {
+		compatible = "regulator-fixed";
+		regulator-name = "vph_pwr";
+		regulator-always-on;
+		regulator-boot-on;
+	};
+
 	vreg_conn_1p8: vreg_conn_1p8 {
 		compatible = "regulator-fixed";
 		regulator-name = "vreg_conn_1p8";
@@ -128,6 +135,30 @@ dp1_connector_in: endpoint {
 			};
 		};
 	};
+
+	dp-dsi0-connector {
+		compatible = "dp-connector";
+		label = "DSI0";
+		type = "full-size";
+
+		port {
+			dp_dsi0_connector_in: endpoint {
+				remote-endpoint = <&anx7625_1_out>;
+			};
+		};
+	};
+
+	dp-dsi1-connector {
+		compatible = "dp-connector";
+		label = "DSI1";
+		type = "full-size";
+
+		port {
+			dp_dsi1_connector_in: endpoint {
+				remote-endpoint = <&anx7625_2_out>;
+			};
+		};
+	};
 };
 
 &apps_rsc {
@@ -517,9 +548,135 @@ &i2c11 {
 
 &i2c18 {
 	clock-frequency = <400000>;
-	pinctrl-0 = <&qup_i2c18_default>;
+	pinctrl-0 = <&qup_i2c18_default>,
+		    <&io_expander_intr_active>,
+		    <&io_expander_reset_active>;
 	pinctrl-names = "default";
+
 	status = "okay";
+
+	io_expander: gpio@74 {
+		compatible = "ti,tca9539";
+		reg = <0x74>;
+		interrupts-extended = <&tlmm 98 IRQ_TYPE_EDGE_BOTH>;
+		gpio-controller;
+		#gpio-cells = <2>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+
+		gpio2-hog {
+			gpio-hog;
+			gpios = <2 GPIO_ACTIVE_HIGH>;
+			input;
+			line-name = "dsi0_int_pin";
+		};
+
+		gpio3-hog {
+			gpio-hog;
+			gpios = <3 GPIO_ACTIVE_LOW>;
+			output-high;
+			line-name = "dsi0_cbl_det_pin";
+		};
+
+		gpio10-hog {
+			gpio-hog;
+			gpios = <10 GPIO_ACTIVE_HIGH>;
+			input;
+			line-name = "dsi1_int_pin";
+		};
+
+		gpio11-hog {
+			gpio-hog;
+			gpios = <11 GPIO_ACTIVE_LOW>;
+			output-high;
+			line-name = "dsi1_cbl_det_pin";
+		};
+	};
+
+	i2c-mux@70 {
+		compatible = "nxp,pca9543";
+		#address-cells = <1>;
+
+		#size-cells = <0>;
+		reg = <0x70>;
+
+		i2c@0 {
+			reg = <0>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			anx_bridge1: anx7625@58 {
+				compatible = "analogix,anx7625";
+				reg = <0x58>;
+				interrupts-extended = <&io_expander 2 IRQ_TYPE_EDGE_FALLING>;
+				enable-gpios = <&io_expander 1 GPIO_ACTIVE_HIGH>;
+				reset-gpios = <&io_expander 0 GPIO_ACTIVE_HIGH>;
+				vdd10-supply = <&vph_pwr>;
+				vdd18-supply = <&vph_pwr>;
+				vdd33-supply = <&vph_pwr>;
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					dsi2dp_bridge_1_in: port@0 {
+						reg = <0>;
+
+						anx7625_1_in: endpoint {
+							remote-endpoint = <&mdss0_dsi0_out>;
+						};
+					};
+
+					dsi2dp_bridge_1_out: port@1 {
+						reg = <1>;
+
+						anx7625_1_out: endpoint {
+							remote-endpoint = <&dp_dsi0_connector_in>;
+						};
+					};
+				};
+			};
+		};
+
+		i2c@1 {
+			reg = <1>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			anx_bridge2: anx7625@58 {
+				compatible = "analogix,anx7625";
+				reg = <0x58>;
+				interrupts-extended = <&io_expander 10 IRQ_TYPE_EDGE_FALLING>;
+				enable-gpios = <&io_expander 9 GPIO_ACTIVE_HIGH>;
+				reset-gpios = <&io_expander 8 GPIO_ACTIVE_HIGH>;
+				vdd10-supply = <&vph_pwr>;
+				vdd18-supply = <&vph_pwr>;
+				vdd33-supply = <&vph_pwr>;
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					dsi2dp_bridge_2_in: port@0 {
+						reg = <0>;
+
+						anx7625_2_in: endpoint {
+							remote-endpoint = <&mdss0_dsi1_out>;
+						};
+					};
+
+					dsi2dp_bridge_2_out: port@1 {
+						reg = <1>;
+
+						anx7625_2_out: endpoint {
+							remote-endpoint = <&dp_dsi1_connector_in>;
+						};
+					};
+				};
+			};
+		};
+	};
+
 };
 
 &mdss0 {
@@ -566,6 +723,40 @@ &mdss0_dp1_phy {
 	status = "okay";
 };
 
+&mdss0_dsi0 {
+	vdda-supply = <&vreg_l1c>;
+
+	status = "okay";
+};
+
+&mdss0_dsi0_out {
+	data-lanes = <0 1 2 3>;
+	remote-endpoint = <&anx7625_1_in>;
+};
+
+&mdss0_dsi0_phy {
+	vdds-supply = <&vreg_l4a>;
+
+	status = "okay";
+};
+
+&mdss0_dsi1 {
+	vdda-supply = <&vreg_l1c>;
+
+	status = "okay";
+};
+
+&mdss0_dsi1_out {
+	data-lanes = <0 1 2 3>;
+	remote-endpoint = <&anx7625_2_in>;
+};
+
+&mdss0_dsi1_phy {
+	vdds-supply = <&vreg_l4a>;
+
+	status = "okay";
+};
+
 &pmm8654au_0_gpios {
 	gpio-line-names = "DS_EN",
 			  "POFF_COMPLETE",
@@ -714,6 +905,21 @@ ethernet0_mdio: ethernet0-mdio-pins {
 		};
 	};
 
+	io_expander_intr_active: io-expander-intr-active-state {
+		pins = "gpio98";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+	};
+
+	io_expander_reset_active: io-expander-reset-active-state {
+		pins = "gpio97";
+		function = "gpio";
+		drive-strength = <2>;
+		bias-disable;
+		output-high;
+	};
+
 	qup_uart10_default: qup-uart10-state {
 		pins = "gpio46", "gpio47";
 		function = "qup1_se3";
-- 
2.34.1

Re: [PATCH v2 07/10] arm64: dts: qcom: sa8775p-ride: add anx7625 DSI to DP bridge nodes

[Krzysztof Kozlowski]

On Tue, Mar 11, 2025 at 05:54:42PM +0530, Ayushi Makhija wrote:
> Add anx7625 DSI to DP bridge device nodes.
> 
> Signed-off-by: Ayushi Makhija <quic_amakhija@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi | 208 ++++++++++++++++++++-
>  1 file changed, 207 insertions(+), 1 deletion(-)
>

So you just gave up after one comment? Context of every email should be
trimmed, so if it is not trimmed means something is still there. I know
there are reviewers who respond with huge unrelated context, but that's
just disrespectful to our time and don't take it as normal.

<form letter>
This is a friendly reminder during the review process.

It seems my or other reviewer's previous comments were not fully
addressed. Maybe the feedback got lost between the quotes, maybe you
just forgot to apply it. Please go back to the previous discussion and
either implement all requested changes or keep discussing them.

Thank you.
</form letter>

Re: [PATCH v2 07/10] arm64: dts: qcom: sa8775p-ride: add anx7625 DSI to DP bridge nodes

[Ayushi Makhija]

On 3/12/2025 5:18 PM, Krzysztof Kozlowski wrote:
> On Tue, Mar 11, 2025 at 05:54:42PM +0530, Ayushi Makhija wrote:
>> Add anx7625 DSI to DP bridge device nodes.
>>
>> Signed-off-by: Ayushi Makhija <quic_amakhija@quicinc.com>
>> ---
>>  arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi | 208 ++++++++++++++++++++-
>>  1 file changed, 207 insertions(+), 1 deletion(-)
>>
> 
> So you just gave up after one comment? Context of every email should be
> trimmed, so if it is not trimmed means something is still there. I know
> there are reviewers who respond with huge unrelated context, but that's
> just disrespectful to our time and don't take it as normal.
> 
> <form letter>
> This is a friendly reminder during the review process.
> 
> It seems my or other reviewer's previous comments were not fully
> addressed. Maybe the feedback got lost between the quotes, maybe you
> just forgot to apply it. Please go back to the previous discussion and
> either implement all requested changes or keep discussing them.
> 
> Thank you.
> </form letter>
> 

Hi Krzysztof,

Thanks, for the review.

I apologize for any confusion or oversight regarding the recent review comments.
Thank you for your patience and understanding. I value your time and feedback and will work to improve the review process.

Below are the comments on the patch 7 and patch 8 of the version 1 of the series, that I have addressed in version 2 of patch 7 of the series.
Let me know, If I did some mistake or if you have any other suggestions.

Comments from Konard:

comment 1

> -	pinctrl-0 = <&qup_i2c18_default>;
> +	pinctrl-0 = <&qup_i2c18_default>,
> +			<&io_expander_intr_active>,
> +			<&io_expander_reset_active>;

Please align the '<'s

comment 2

> +		interrupt-parent = <&tlmm>;
> +		interrupts = <98 IRQ_TYPE_EDGE_BOTH>;

use interrupts-extended, here and below

These above two comments were from the konard in patch 7 in version 1 of the series.
I have addressed both the above comments in the version 2 of patch 7 of the series.



Comments from Krzysztof:

comment 1

> +
> +		dsi0_int_pin: gpio2_cfg {
No underscores, see DTS coding style.

I have corrected the above comment in the version 2 of patch 7 of the series.

comment 2

> +
> +			anx_bridge_1: anx7625@58 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

In this I have changed the node name as anx_bridge1 : anx7625@58.
Let me know, if I did some mistake or you have any other suggestion over the node name.

I have took the reference from below:
linux/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi at 629c635eafbaf18260c8083360745c71674640d2 · torvalds/linux · GitHub

comment 3

> +				enable-gpios = <&io_expander 1 0>;
> +				reset-gpios = <&io_expander 0 0>;
Use proper defines.

For this above comment,  I have changed above lines into below lines in patch 7 of version 2 of the series.

> +				enable-gpios = <&io_expander 1 GPIO_ACTIVE_HIGH>;
> +				reset-gpios = <&io_expander 0 GPIO_ACTIVE_HIGH>;

comment 4

> +
> +			anx_bridge_2: anx7625@58 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

In this I have changed the node name as anx_bridge2 : anx7625@58.
Let me know, if I did some mistake or you have any other suggestion over the node name.

I have took the reference from below:
linux/arch/arm64/boot/dts/mediatek/mt8183-kukui-jacuzzi.dtsi at 629c635eafbaf18260c8083360745c71674640d2 · torvalds/linux · GitHub

comment 5

And as Rob's bot pointed out: insufficient testing. :(
Please be 100% sure everything is tested before you post new version.
You shouldn't use reviewers for the job of tools, that's quite waste of
our time.

Fixed the  above warning from DT checker against DT binding in patch 7 of version 2 of the series.


Comments from Dmitry:

comment 1

Missing dp-connector devices. Please add them together with the bridges. 

comment 2

Please squash into the previous patch. It doesn't make a lot of sense separately.

These both above commented from Dmitry I have addressed in the version 2 of patch 7 of the series.
I have squash patch 8 into patch 7 of version 1 into patch 7 of version 2 of the series.


Thanks,
Ayushi

Re: [PATCH v2 07/10] arm64: dts: qcom: sa8775p-ride: add anx7625 DSI to DP bridge nodes

[Ayushi Makhija]

> These both above commented from Dmitry I have addressed in the version 2 of patch 7 of the series.
> I have squash patch 8 into patch 7 of version 1 into patch 7 of version 2 of the series.
> 
> 
> Thanks,
> Ayushi

Hi Krzysztof,

I hope this message finds you well. I wanted to follow up on the reply I sent. Your feedback is invaluable to us, and we would greatly appreciate any further insights or comments you might have.

Thanks,
Ayushi

Re: [PATCH v2 07/10] arm64: dts: qcom: sa8775p-ride: add anx7625 DSI to DP bridge nodes

[Dmitry Baryshkov]

On Fri, Mar 28, 2025 at 03:13:57PM +0530, Ayushi Makhija wrote:
> > These both above commented from Dmitry I have addressed in the version 2 of patch 7 of the series.
> > I have squash patch 8 into patch 7 of version 1 into patch 7 of version 2 of the series.
> > 
> > 
> > Thanks,
> > Ayushi
> 
> Hi Krzysztof,
> 
> I hope this message finds you well. I wanted to follow up on the reply I sent. Your feedback is invaluable to us, and we would greatly appreciate any further insights or comments you might have.
> 

Granted the lack of response, please make sure that you've addressed all
the comments and proceed with the next iteration of the patchset.

-- 
With best wishes
Dmitry

Re: [PATCH v2 07/10] arm64: dts: qcom: sa8775p-ride: add anx7625 DSI to DP bridge nodes

[Krzysztof Kozlowski]

On 28/03/2025 13:45, Dmitry Baryshkov wrote:
> On Fri, Mar 28, 2025 at 03:13:57PM +0530, Ayushi Makhija wrote:
>>> These both above commented from Dmitry I have addressed in the version 2 of patch 7 of the series.
>>> I have squash patch 8 into patch 7 of version 1 into patch 7 of version 2 of the series.
>>>
>>>
>>> Thanks,
>>> Ayushi
>>
>> Hi Krzysztof,
>>
>> I hope this message finds you well. I wanted to follow up on the reply I sent. Your feedback is invaluable to us, and we would greatly appreciate any further insights or comments you might have.
>>
> 
> Granted the lack of response, please make sure that you've addressed all
> the comments and proceed with the next iteration of the patchset.

Just to clarify, I did not plan to respond here, because email style
which tries to respond to my comments is unreadable. It's impossible to
find what is quote, what is the comment and what is the response.

I expected inline responses to the original emails and detailed changelog.

Best regards,
Krzysztof

Re: [PATCH v2 07/10] arm64: dts: qcom: sa8775p-ride: add anx7625 DSI to DP bridge nodes

[Dmitry Baryshkov]

On Fri, 28 Mar 2025 at 16:22, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 28/03/2025 13:45, Dmitry Baryshkov wrote:
> > On Fri, Mar 28, 2025 at 03:13:57PM +0530, Ayushi Makhija wrote:
> >>> These both above commented from Dmitry I have addressed in the version 2 of patch 7 of the series.
> >>> I have squash patch 8 into patch 7 of version 1 into patch 7 of version 2 of the series.
> >>>
> >>>
> >>> Thanks,
> >>> Ayushi
> >>
> >> Hi Krzysztof,
> >>
> >> I hope this message finds you well. I wanted to follow up on the reply I sent. Your feedback is invaluable to us, and we would greatly appreciate any further insights or comments you might have.
> >>
> >
> > Granted the lack of response, please make sure that you've addressed all
> > the comments and proceed with the next iteration of the patchset.
>
> Just to clarify, I did not plan to respond here, because email style
> which tries to respond to my comments is unreadable. It's impossible to
> find what is quote, what is the comment and what is the response.
>
> I expected inline responses to the original emails and detailed changelog.

Works for me. I'd say, let's get the next revision and check if it
resolves your comments or we have more comments.

-- 
With best wishes
Dmitry

Re: [PATCH v2 07/10] arm64: dts: qcom: sa8775p-ride: add anx7625 DSI to DP bridge nodes

[Krzysztof Kozlowski]

On 13/03/2025 13:10, Ayushi Makhija wrote:

...

> 
>> +
>> +			anx_bridge_1: anx7625@58 {
> 
> Node names should be generic. See also an explanation and list of
> examples (not exhaustive) in DT specification:
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> 
> In this I have changed the node name as anx_bridge1 : anx7625@58.

Except that it is difficult to understand what is what, let's recap.

Original code was:
	anx_bridge_1: anx7625@58 {

You said you changed it to:

	anx_bridge1 : anx7625@58.

and now I give my offer: I offer to buy a beer (or tee/coffee/juice) to
anyone who will spot the difference(s) between these two node names,
IOW, tell me what changed here.

Best regards,
Krzysztof

Re: [PATCH v2 07/10] arm64: dts: qcom: sa8775p-ride: add anx7625 DSI to DP bridge nodes

[Dmitry Baryshkov]

On Fri, Mar 28, 2025 at 03:28:20PM +0100, Krzysztof Kozlowski wrote:
> On 13/03/2025 13:10, Ayushi Makhija wrote:
> 
> ...
> 
> > 
> >> +
> >> +			anx_bridge_1: anx7625@58 {
> > 
> > Node names should be generic. See also an explanation and list of
> > examples (not exhaustive) in DT specification:
> > https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> > 
> > In this I have changed the node name as anx_bridge1 : anx7625@58.
> 
> Except that it is difficult to understand what is what, let's recap.
> 
> Original code was:
> 	anx_bridge_1: anx7625@58 {
> 
> You said you changed it to:
> 
> 	anx_bridge1 : anx7625@58.
> 
> and now I give my offer: I offer to buy a beer (or tee/coffee/juice) to
> anyone who will spot the difference(s) between these two node names,
> IOW, tell me what changed here.

Ayushi: mediatek DT are a bad example here. Please use bridge@58 as node
name.

> 
> Best regards,
> Krzysztof

-- 
With best wishes
Dmitry

Re: [PATCH v2 07/10] arm64: dts: qcom: sa8775p-ride: add anx7625 DSI to DP bridge nodes

[Ayushi Makhija]

> Ayushi: mediatek DT are a bad example here. Please use bridge@58 as node
> name.

Hi Dmitry,
Thanks for the review.

Previously, I was referencing the mediatek DT, I will be using bridge@58 as node name. Will upload the change in next patch.

Thanks,
Ayushi

[PATCH v2 08/10] drm/bridge: anx7625: enable HPD interrupts

[Ayushi Makhija]

When device enters the suspend state, it prevents
HPD interrupts from occurring. To address this,
add an additional PM runtime vote in hpd_enable().
This vote is removed in hpd_disable().

Signed-off-by: Ayushi Makhija <quic_amakhija@quicinc.com>
---
 drivers/gpu/drm/bridge/analogix/anx7625.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
index 4be34d5c7a3b..764da1c1dc11 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -2474,6 +2474,22 @@ static const struct drm_edid *anx7625_bridge_edid_read(struct drm_bridge *bridge
 	return anx7625_edid_read(ctx);
 }
 
+static void anx7625_bridge_hpd_enable(struct drm_bridge *bridge)
+{
+	struct anx7625_data *ctx = bridge_to_anx7625(bridge);
+	struct device *dev = ctx->dev;
+
+	pm_runtime_get_sync(dev);
+}
+
+static void anx7625_bridge_hpd_disable(struct drm_bridge *bridge)
+{
+	struct anx7625_data *ctx = bridge_to_anx7625(bridge);
+	struct device *dev = ctx->dev;
+
+	pm_runtime_put_sync(dev);
+}
+
 static const struct drm_bridge_funcs anx7625_bridge_funcs = {
 	.attach = anx7625_bridge_attach,
 	.detach = anx7625_bridge_detach,
@@ -2487,6 +2503,8 @@ static const struct drm_bridge_funcs anx7625_bridge_funcs = {
 	.atomic_reset = drm_atomic_helper_bridge_reset,
 	.detect = anx7625_bridge_detect,
 	.edid_read = anx7625_bridge_edid_read,
+	.hpd_enable = anx7625_bridge_hpd_enable,
+	.hpd_disable = anx7625_bridge_hpd_disable,
 };
 
 static int anx7625_register_i2c_dummy_clients(struct anx7625_data *ctx,
-- 
2.34.1

Re: [PATCH v2 08/10] drm/bridge: anx7625: enable HPD interrupts

[Dmitry Baryshkov]

On Tue, Mar 11, 2025 at 05:54:43PM +0530, Ayushi Makhija wrote:
> When device enters the suspend state, it prevents
> HPD interrupts from occurring. To address this,
> add an additional PM runtime vote in hpd_enable().
> This vote is removed in hpd_disable().

Is it really enough to toggle the HPD interrupts? Is there any kind of
programming that should be moved to .hpd_enable() too (so that by
default the bridge doesn't generate HPD interrupts)?

> 
> Signed-off-by: Ayushi Makhija <quic_amakhija@quicinc.com>
> ---
>  drivers/gpu/drm/bridge/analogix/anx7625.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
> index 4be34d5c7a3b..764da1c1dc11 100644
> --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> @@ -2474,6 +2474,22 @@ static const struct drm_edid *anx7625_bridge_edid_read(struct drm_bridge *bridge
>  	return anx7625_edid_read(ctx);
>  }
>  
> +static void anx7625_bridge_hpd_enable(struct drm_bridge *bridge)
> +{
> +	struct anx7625_data *ctx = bridge_to_anx7625(bridge);
> +	struct device *dev = ctx->dev;
> +
> +	pm_runtime_get_sync(dev);
> +}
> +
> +static void anx7625_bridge_hpd_disable(struct drm_bridge *bridge)
> +{
> +	struct anx7625_data *ctx = bridge_to_anx7625(bridge);
> +	struct device *dev = ctx->dev;
> +
> +	pm_runtime_put_sync(dev);
> +}
> +
>  static const struct drm_bridge_funcs anx7625_bridge_funcs = {
>  	.attach = anx7625_bridge_attach,
>  	.detach = anx7625_bridge_detach,
> @@ -2487,6 +2503,8 @@ static const struct drm_bridge_funcs anx7625_bridge_funcs = {
>  	.atomic_reset = drm_atomic_helper_bridge_reset,
>  	.detect = anx7625_bridge_detect,
>  	.edid_read = anx7625_bridge_edid_read,
> +	.hpd_enable = anx7625_bridge_hpd_enable,
> +	.hpd_disable = anx7625_bridge_hpd_disable,
>  };
>  
>  static int anx7625_register_i2c_dummy_clients(struct anx7625_data *ctx,
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH v2 08/10] drm/bridge: anx7625: enable HPD interrupts

[Ayushi Makhija]

On 3/11/2025 9:09 PM, Dmitry Baryshkov wrote:
> On Tue, Mar 11, 2025 at 05:54:43PM +0530, Ayushi Makhija wrote:
>> When device enters the suspend state, it prevents
>> HPD interrupts from occurring. To address this,
>> add an additional PM runtime vote in hpd_enable().
>> This vote is removed in hpd_disable().
> 
> Is it really enough to toggle the HPD interrupts? Is there any kind of
> programming that should be moved to .hpd_enable() too (so that by
> default the bridge doesn't generate HPD interrupts)?
> 

Hi Dmirty,

I couldn't find the ANX7625 bridge driver datasheet, where all the registers information are present.

As per my understanding, we have anx7625_hpd_timer_config(), where debounce timer registers are getting set, which help to manage the detection and stability of the HPD signal.

anx7625_hpd_timer_config() is getting called from anx7625_runtime_pm_resume().

anx7625_runtime_pm_resume
   anx7625_power_on_init
       anx7625_ocm_loading_check
           anx7625_disable_pd_protocol
              anx7625_hpd_timer_config

So, I think HPD programming is already taken care in anx7625_hpd_timer_config(). anx7625_runtime_pm_resume() is getting called for both eDP and DP configuration. 

If you suggest then, I can move the anx7625_hpd_timer_config() from anx7625_disable_pd_protocol() to anx7625_bridge_hpd_enable().

Thanks,
Ayushi

Re: [PATCH v2 08/10] drm/bridge: anx7625: enable HPD interrupts

[Dmitry Baryshkov]

On Fri, Mar 21, 2025 at 02:36:21AM +0530, Ayushi Makhija wrote:
> On 3/11/2025 9:09 PM, Dmitry Baryshkov wrote:
> > On Tue, Mar 11, 2025 at 05:54:43PM +0530, Ayushi Makhija wrote:
> >> When device enters the suspend state, it prevents
> >> HPD interrupts from occurring. To address this,
> >> add an additional PM runtime vote in hpd_enable().
> >> This vote is removed in hpd_disable().
> > 
> > Is it really enough to toggle the HPD interrupts? Is there any kind of
> > programming that should be moved to .hpd_enable() too (so that by
> > default the bridge doesn't generate HPD interrupts)?
> > 
> 
> Hi Dmirty,
> 
> I couldn't find the ANX7625 bridge driver datasheet, where all the registers information are present.
> 
> As per my understanding, we have anx7625_hpd_timer_config(), where debounce timer registers are getting set, which help to manage the detection and stability of the HPD signal.
> 
> anx7625_hpd_timer_config() is getting called from anx7625_runtime_pm_resume().
> 
> anx7625_runtime_pm_resume
>    anx7625_power_on_init
>        anx7625_ocm_loading_check
>            anx7625_disable_pd_protocol
>               anx7625_hpd_timer_config
> 
> So, I think HPD programming is already taken care in anx7625_hpd_timer_config(). anx7625_runtime_pm_resume() is getting called for both eDP and DP configuration. 
> 
> If you suggest then, I can move the anx7625_hpd_timer_config() from anx7625_disable_pd_protocol() to anx7625_bridge_hpd_enable().

This might result in HPD signal being generated after invalid or
improper timings. If we can't get a feedback from Analogix on how to
control HPD generation, then it's better to leave it as is.

-- 
With best wishes
Dmitry

[PATCH v2 09/10] drm/bridge: anx7625: update bridge_ops and sink detect logic

[Ayushi Makhija]

The anx7625_link_bridge() checks if a device is not a panel
bridge and add DRM_BRIDGE_OP_HPD and DRM_BRIDGE_OP_DETECT to
the bridge operations. However, on port 1 of the anx7625
bridge, any device added is always treated as a panel
bridge, preventing connector_detect function from being
called. To resolve this, instead of just checking if it is a
panel bridge, verify the type of panel bridge
whether it is a DisplayPort or eDP panel. If the panel
bridge is not of the eDP type, add DRM_BRIDGE_OP_HPD and
DRM_BRIDGE_OP_DETECT to the bridge operations.

In the anx7625_sink_detect(), the device is checked to see
if it is a panel bridge, and it always sends a "connected"
status to the connector. When adding the DP port on port 1 of the
anx7625, it incorrectly treats it as a panel bridge and sends an
always "connected" status. Instead of checking the status on the
panel bridge, it's better to check the hpd_status for connectors
like DisplayPort. This way, it verifies the hpd_status variable
before sending the status to the connector.

Signed-off-by: Ayushi Makhija <quic_amakhija@quicinc.com>
---
 drivers/gpu/drm/bridge/analogix/anx7625.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
index 764da1c1dc11..ad99ad19653f 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -1814,9 +1814,6 @@ static enum drm_connector_status anx7625_sink_detect(struct anx7625_data *ctx)
 
 	DRM_DEV_DEBUG_DRIVER(dev, "sink detect\n");
 
-	if (ctx->pdata.panel_bridge)
-		return connector_status_connected;
-
 	return ctx->hpd_status ? connector_status_connected :
 				     connector_status_disconnected;
 }
@@ -2608,9 +2605,10 @@ static int anx7625_link_bridge(struct drm_dp_aux *aux)
 	platform->bridge.of_node = dev->of_node;
 	if (!anx7625_of_panel_on_aux_bus(dev))
 		platform->bridge.ops |= DRM_BRIDGE_OP_EDID;
-	if (!platform->pdata.panel_bridge)
-		platform->bridge.ops |= DRM_BRIDGE_OP_HPD |
-					DRM_BRIDGE_OP_DETECT;
+	if (!platform->pdata.panel_bridge ||
+	    platform->pdata.panel_bridge->type != DRM_MODE_CONNECTOR_eDP) {
+		platform->bridge.ops |= DRM_BRIDGE_OP_HPD | DRM_BRIDGE_OP_DETECT;
+	}
 	platform->bridge.type = platform->pdata.panel_bridge ?
 				    DRM_MODE_CONNECTOR_eDP :
 				    DRM_MODE_CONNECTOR_DisplayPort;
-- 
2.34.1

Re: [PATCH v2 09/10] drm/bridge: anx7625: update bridge_ops and sink detect logic

[Dmitry Baryshkov]

On Tue, Mar 11, 2025 at 05:54:44PM +0530, Ayushi Makhija wrote:
> The anx7625_link_bridge() checks if a device is not a panel
> bridge and add DRM_BRIDGE_OP_HPD and DRM_BRIDGE_OP_DETECT to
> the bridge operations. However, on port 1 of the anx7625
> bridge, any device added is always treated as a panel
> bridge, preventing connector_detect function from being
> called. To resolve this, instead of just checking if it is a
> panel bridge, verify the type of panel bridge
> whether it is a DisplayPort or eDP panel. If the panel
> bridge is not of the eDP type, add DRM_BRIDGE_OP_HPD and
> DRM_BRIDGE_OP_DETECT to the bridge operations.

Are/were there any devices using anx7625, eDP panel _and_ not using the
AUX bus? It would be better to use the precence of the 'aux' node to
determine whether it is an eDP or a DP configuration.

> 
> In the anx7625_sink_detect(), the device is checked to see
> if it is a panel bridge, and it always sends a "connected"
> status to the connector. When adding the DP port on port 1 of the
> anx7625, it incorrectly treats it as a panel bridge and sends an
> always "connected" status. Instead of checking the status on the
> panel bridge, it's better to check the hpd_status for connectors
> like DisplayPort. This way, it verifies the hpd_status variable
> before sending the status to the connector.
> 
> Signed-off-by: Ayushi Makhija <quic_amakhija@quicinc.com>
> ---
>  drivers/gpu/drm/bridge/analogix/anx7625.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
> index 764da1c1dc11..ad99ad19653f 100644
> --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> @@ -1814,9 +1814,6 @@ static enum drm_connector_status anx7625_sink_detect(struct anx7625_data *ctx)
>  
>  	DRM_DEV_DEBUG_DRIVER(dev, "sink detect\n");
>  
> -	if (ctx->pdata.panel_bridge)
> -		return connector_status_connected;
> -
>  	return ctx->hpd_status ? connector_status_connected :
>  				     connector_status_disconnected;
>  }
> @@ -2608,9 +2605,10 @@ static int anx7625_link_bridge(struct drm_dp_aux *aux)
>  	platform->bridge.of_node = dev->of_node;
>  	if (!anx7625_of_panel_on_aux_bus(dev))
>  		platform->bridge.ops |= DRM_BRIDGE_OP_EDID;
> -	if (!platform->pdata.panel_bridge)
> -		platform->bridge.ops |= DRM_BRIDGE_OP_HPD |
> -					DRM_BRIDGE_OP_DETECT;
> +	if (!platform->pdata.panel_bridge ||
> +	    platform->pdata.panel_bridge->type != DRM_MODE_CONNECTOR_eDP) {
> +		platform->bridge.ops |= DRM_BRIDGE_OP_HPD | DRM_BRIDGE_OP_DETECT;
> +	}
>  	platform->bridge.type = platform->pdata.panel_bridge ?
>  				    DRM_MODE_CONNECTOR_eDP :
>  				    DRM_MODE_CONNECTOR_DisplayPort;
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH v2 09/10] drm/bridge: anx7625: update bridge_ops and sink detect logic

[Ayushi Makhija]

On 3/11/2025 9:11 PM, Dmitry Baryshkov wrote:
> On Tue, Mar 11, 2025 at 05:54:44PM +0530, Ayushi Makhija wrote:
>> The anx7625_link_bridge() checks if a device is not a panel
>> bridge and add DRM_BRIDGE_OP_HPD and DRM_BRIDGE_OP_DETECT to
>> the bridge operations. However, on port 1 of the anx7625
>> bridge, any device added is always treated as a panel
>> bridge, preventing connector_detect function from being
>> called. To resolve this, instead of just checking if it is a
>> panel bridge, verify the type of panel bridge
>> whether it is a DisplayPort or eDP panel. If the panel
>> bridge is not of the eDP type, add DRM_BRIDGE_OP_HPD and
>> DRM_BRIDGE_OP_DETECT to the bridge operations.
> 
> Are/were there any devices using anx7625, eDP panel _and_ not using the
> AUX bus? It would be better to use the precence of the 'aux' node to
> determine whether it is an eDP or a DP configuration.
> 
>>
>> In the anx7625_sink_detect(), the device is checked to see
>> if it is a panel bridge, and it always sends a "connected"
>> status to the connector. When adding the DP port on port 1 of the
>> anx7625, it incorrectly treats it as a panel bridge and sends an
>> always "connected" status. Instead of checking the status on the
>> panel bridge, it's better to check the hpd_status for connectors
>> like DisplayPort. This way, it verifies the hpd_status variable
>> before sending the status to the connector.
>>
>> Signed-off-by: Ayushi Makhija <quic_amakhija@quicinc.com>
>> ---
>>  drivers/gpu/drm/bridge/analogix/anx7625.c | 10 ++++------
>>  1 file changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
>> index 764da1c1dc11..ad99ad19653f 100644
>> --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
>> +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
>> @@ -1814,9 +1814,6 @@ static enum drm_connector_status anx7625_sink_detect(struct anx7625_data *ctx)
>>  
>>  	DRM_DEV_DEBUG_DRIVER(dev, "sink detect\n");
>>  
>> -	if (ctx->pdata.panel_bridge)
>> -		return connector_status_connected;
>> -
>>  	return ctx->hpd_status ? connector_status_connected :
>>  				     connector_status_disconnected;
>>  }
>> @@ -2608,9 +2605,10 @@ static int anx7625_link_bridge(struct drm_dp_aux *aux)
>>  	platform->bridge.of_node = dev->of_node;
>>  	if (!anx7625_of_panel_on_aux_bus(dev))
>>  		platform->bridge.ops |= DRM_BRIDGE_OP_EDID;
>> -	if (!platform->pdata.panel_bridge)
>> -		platform->bridge.ops |= DRM_BRIDGE_OP_HPD |
>> -					DRM_BRIDGE_OP_DETECT;
>> +	if (!platform->pdata.panel_bridge ||
>> +	    platform->pdata.panel_bridge->type != DRM_MODE_CONNECTOR_eDP) {
>> +		platform->bridge.ops |= DRM_BRIDGE_OP_HPD | DRM_BRIDGE_OP_DETECT;
>> +	}

Hi Dmitry,

Thanks, for the review.

Yes, it is better to check the presence of the 'aux' node for eDP or DP configuration.
Will change it in next patch.

-	if (!platform->pdata.panel_bridge)
-		platform->bridge.ops |= DRM_BRIDGE_OP_HPD |
-					DRM_BRIDGE_OP_DETECT;
+	if (!platform->pdata.panel_bridge || !anx7625_of_panel_on_aux_bus(dev)) {
+		platform->bridge.ops |= DRM_BRIDGE_OP_HPD | DRM_BRIDGE_OP_DETECT;
+	}

Thanks,
Ayushi

Re: [PATCH v2 09/10] drm/bridge: anx7625: update bridge_ops and sink detect logic

[Dmitry Baryshkov]

On Wed, 12 Mar 2025 at 11:47, Ayushi Makhija <quic_amakhija@quicinc.com> wrote:
>
> On 3/11/2025 9:11 PM, Dmitry Baryshkov wrote:
> > On Tue, Mar 11, 2025 at 05:54:44PM +0530, Ayushi Makhija wrote:
> >> The anx7625_link_bridge() checks if a device is not a panel
> >> bridge and add DRM_BRIDGE_OP_HPD and DRM_BRIDGE_OP_DETECT to
> >> the bridge operations. However, on port 1 of the anx7625
> >> bridge, any device added is always treated as a panel
> >> bridge, preventing connector_detect function from being
> >> called. To resolve this, instead of just checking if it is a
> >> panel bridge, verify the type of panel bridge
> >> whether it is a DisplayPort or eDP panel. If the panel
> >> bridge is not of the eDP type, add DRM_BRIDGE_OP_HPD and
> >> DRM_BRIDGE_OP_DETECT to the bridge operations.
> >
> > Are/were there any devices using anx7625, eDP panel _and_ not using the
> > AUX bus? It would be better to use the precence of the 'aux' node to
> > determine whether it is an eDP or a DP configuration.
> >
> >>
> >> In the anx7625_sink_detect(), the device is checked to see
> >> if it is a panel bridge, and it always sends a "connected"
> >> status to the connector. When adding the DP port on port 1 of the
> >> anx7625, it incorrectly treats it as a panel bridge and sends an
> >> always "connected" status. Instead of checking the status on the
> >> panel bridge, it's better to check the hpd_status for connectors
> >> like DisplayPort. This way, it verifies the hpd_status variable
> >> before sending the status to the connector.
> >>
> >> Signed-off-by: Ayushi Makhija <quic_amakhija@quicinc.com>
> >> ---
> >>  drivers/gpu/drm/bridge/analogix/anx7625.c | 10 ++++------
> >>  1 file changed, 4 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
> >> index 764da1c1dc11..ad99ad19653f 100644
> >> --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> >> +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> >> @@ -1814,9 +1814,6 @@ static enum drm_connector_status anx7625_sink_detect(struct anx7625_data *ctx)
> >>
> >>      DRM_DEV_DEBUG_DRIVER(dev, "sink detect\n");
> >>
> >> -    if (ctx->pdata.panel_bridge)
> >> -            return connector_status_connected;
> >> -
> >>      return ctx->hpd_status ? connector_status_connected :
> >>                                   connector_status_disconnected;
> >>  }
> >> @@ -2608,9 +2605,10 @@ static int anx7625_link_bridge(struct drm_dp_aux *aux)
> >>      platform->bridge.of_node = dev->of_node;
> >>      if (!anx7625_of_panel_on_aux_bus(dev))
> >>              platform->bridge.ops |= DRM_BRIDGE_OP_EDID;
> >> -    if (!platform->pdata.panel_bridge)
> >> -            platform->bridge.ops |= DRM_BRIDGE_OP_HPD |
> >> -                                    DRM_BRIDGE_OP_DETECT;
> >> +    if (!platform->pdata.panel_bridge ||
> >> +        platform->pdata.panel_bridge->type != DRM_MODE_CONNECTOR_eDP) {
> >> +            platform->bridge.ops |= DRM_BRIDGE_OP_HPD | DRM_BRIDGE_OP_DETECT;
> >> +    }
>
> Hi Dmitry,
>
> Thanks, for the review.
>
> Yes, it is better to check the presence of the 'aux' node for eDP or DP configuration.
> Will change it in next patch.
>
> -       if (!platform->pdata.panel_bridge)
> -               platform->bridge.ops |= DRM_BRIDGE_OP_HPD |
> -                                       DRM_BRIDGE_OP_DETECT;
> +       if (!platform->pdata.panel_bridge || !anx7625_of_panel_on_aux_bus(dev)) {

This is incorrect, if I'm not mistaken, please doublecheck it. I'd
suggest following msm_dp_display_get_connector_type() (feel free to
extract that to a helper function).


> +               platform->bridge.ops |= DRM_BRIDGE_OP_HPD | DRM_BRIDGE_OP_DETECT;
> +       }
>
> Thanks,
> Ayushi



-- 
With best wishes
Dmitry

Re: [PATCH v2 09/10] drm/bridge: anx7625: update bridge_ops and sink detect logic

[Ayushi Makhija]

On 3/12/2025 4:33 PM, Dmitry Baryshkov wrote:
> On Wed, 12 Mar 2025 at 11:47, Ayushi Makhija <quic_amakhija@quicinc.com> wrote:
>>
>> On 3/11/2025 9:11 PM, Dmitry Baryshkov wrote:
>>> On Tue, Mar 11, 2025 at 05:54:44PM +0530, Ayushi Makhija wrote:
>>>> The anx7625_link_bridge() checks if a device is not a panel
>>>> bridge and add DRM_BRIDGE_OP_HPD and DRM_BRIDGE_OP_DETECT to
>>>> the bridge operations. However, on port 1 of the anx7625
>>>> bridge, any device added is always treated as a panel
>>>> bridge, preventing connector_detect function from being
>>>> called. To resolve this, instead of just checking if it is a
>>>> panel bridge, verify the type of panel bridge
>>>> whether it is a DisplayPort or eDP panel. If the panel
>>>> bridge is not of the eDP type, add DRM_BRIDGE_OP_HPD and
>>>> DRM_BRIDGE_OP_DETECT to the bridge operations.
>>>
>>> Are/were there any devices using anx7625, eDP panel _and_ not using the
>>> AUX bus? It would be better to use the precence of the 'aux' node to
>>> determine whether it is an eDP or a DP configuration.
>>>
>>>>
>>>> In the anx7625_sink_detect(), the device is checked to see
>>>> if it is a panel bridge, and it always sends a "connected"
>>>> status to the connector. When adding the DP port on port 1 of the
>>>> anx7625, it incorrectly treats it as a panel bridge and sends an
>>>> always "connected" status. Instead of checking the status on the
>>>> panel bridge, it's better to check the hpd_status for connectors
>>>> like DisplayPort. This way, it verifies the hpd_status variable
>>>> before sending the status to the connector.
>>>>
>>>> Signed-off-by: Ayushi Makhija <quic_amakhija@quicinc.com>
>>>> ---
>>>>  drivers/gpu/drm/bridge/analogix/anx7625.c | 10 ++++------
>>>>  1 file changed, 4 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
>>>> index 764da1c1dc11..ad99ad19653f 100644
>>>> --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
>>>> +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
>>>> @@ -1814,9 +1814,6 @@ static enum drm_connector_status anx7625_sink_detect(struct anx7625_data *ctx)
>>>>
>>>>      DRM_DEV_DEBUG_DRIVER(dev, "sink detect\n");
>>>>
>>>> -    if (ctx->pdata.panel_bridge)
>>>> -            return connector_status_connected;
>>>> -
>>>>      return ctx->hpd_status ? connector_status_connected :
>>>>                                   connector_status_disconnected;
>>>>  }
>>>> @@ -2608,9 +2605,10 @@ static int anx7625_link_bridge(struct drm_dp_aux *aux)
>>>>      platform->bridge.of_node = dev->of_node;
>>>>      if (!anx7625_of_panel_on_aux_bus(dev))
>>>>              platform->bridge.ops |= DRM_BRIDGE_OP_EDID;
>>>> -    if (!platform->pdata.panel_bridge)
>>>> -            platform->bridge.ops |= DRM_BRIDGE_OP_HPD |
>>>> -                                    DRM_BRIDGE_OP_DETECT;
>>>> +    if (!platform->pdata.panel_bridge ||
>>>> +        platform->pdata.panel_bridge->type != DRM_MODE_CONNECTOR_eDP) {
>>>> +            platform->bridge.ops |= DRM_BRIDGE_OP_HPD | DRM_BRIDGE_OP_DETECT;
>>>> +    }
>>
>> Hi Dmitry,
>>
>> Thanks, for the review.
>>
>> Yes, it is better to check the presence of the 'aux' node for eDP or DP configuration.
>> Will change it in next patch.
>>
>> -       if (!platform->pdata.panel_bridge)
>> -               platform->bridge.ops |= DRM_BRIDGE_OP_HPD |
>> -                                       DRM_BRIDGE_OP_DETECT;
>> +       if (!platform->pdata.panel_bridge || !anx7625_of_panel_on_aux_bus(dev)) {
> 
> This is incorrect, if I'm not mistaken, please doublecheck it. I'd
> suggest following msm_dp_display_get_connector_type() (feel free to
> extract that to a helper function).
> 

Hi Dmirty,

Thanks, for the review.

If we see definition of anx7625_of_panel_on_aux_bus() it is doing the same
thing as msm_dp_display_get_connector_type().

static bool anx7625_of_panel_on_aux_bus(struct device *dev)
{
        struct device_node *bus, *panel;

        bus = of_get_child_by_name(dev->of_node, "aux-bus");

        // if aux-bus is not there it will return the false.
        if (!bus)
                return false;

        panel = of_get_child_by_name(bus, "panel");
        of_node_put(bus);

        // if panel is not there it will return the false.
        if (!panel)
                return false;
        of_node_put(panel);

        return true;
}

Above function will return true in case of eDP and false in case of DP.
So we can use anx7625_of_panel_on_aux_bus() to check whether it
is DP or eDP configuration based on aux.

I don't think so we need extract msm_dp_display_get_connector_type() to check the eDP or DP configuration based on aux.
Let me know, if I am missing anything.

Thanks,
Ayushi

Re: [PATCH v2 09/10] drm/bridge: anx7625: update bridge_ops and sink detect logic

[Dmitry Baryshkov]

On Thu, Mar 13, 2025 at 12:14:04PM +0530, Ayushi Makhija wrote:
> 
> 
> On 3/12/2025 4:33 PM, Dmitry Baryshkov wrote:
> > On Wed, 12 Mar 2025 at 11:47, Ayushi Makhija <quic_amakhija@quicinc.com> wrote:
> >>
> >> On 3/11/2025 9:11 PM, Dmitry Baryshkov wrote:
> >>> On Tue, Mar 11, 2025 at 05:54:44PM +0530, Ayushi Makhija wrote:
> >>>> The anx7625_link_bridge() checks if a device is not a panel
> >>>> bridge and add DRM_BRIDGE_OP_HPD and DRM_BRIDGE_OP_DETECT to
> >>>> the bridge operations. However, on port 1 of the anx7625
> >>>> bridge, any device added is always treated as a panel
> >>>> bridge, preventing connector_detect function from being
> >>>> called. To resolve this, instead of just checking if it is a
> >>>> panel bridge, verify the type of panel bridge
> >>>> whether it is a DisplayPort or eDP panel. If the panel
> >>>> bridge is not of the eDP type, add DRM_BRIDGE_OP_HPD and
> >>>> DRM_BRIDGE_OP_DETECT to the bridge operations.
> >>>
> >>> Are/were there any devices using anx7625, eDP panel _and_ not using the
> >>> AUX bus? It would be better to use the precence of the 'aux' node to
> >>> determine whether it is an eDP or a DP configuration.
> >>>
> >>>>
> >>>> In the anx7625_sink_detect(), the device is checked to see
> >>>> if it is a panel bridge, and it always sends a "connected"
> >>>> status to the connector. When adding the DP port on port 1 of the
> >>>> anx7625, it incorrectly treats it as a panel bridge and sends an
> >>>> always "connected" status. Instead of checking the status on the
> >>>> panel bridge, it's better to check the hpd_status for connectors
> >>>> like DisplayPort. This way, it verifies the hpd_status variable
> >>>> before sending the status to the connector.
> >>>>
> >>>> Signed-off-by: Ayushi Makhija <quic_amakhija@quicinc.com>
> >>>> ---
> >>>>  drivers/gpu/drm/bridge/analogix/anx7625.c | 10 ++++------
> >>>>  1 file changed, 4 insertions(+), 6 deletions(-)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
> >>>> index 764da1c1dc11..ad99ad19653f 100644
> >>>> --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> >>>> +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
> >>>> @@ -1814,9 +1814,6 @@ static enum drm_connector_status anx7625_sink_detect(struct anx7625_data *ctx)
> >>>>
> >>>>      DRM_DEV_DEBUG_DRIVER(dev, "sink detect\n");
> >>>>
> >>>> -    if (ctx->pdata.panel_bridge)
> >>>> -            return connector_status_connected;
> >>>> -
> >>>>      return ctx->hpd_status ? connector_status_connected :
> >>>>                                   connector_status_disconnected;
> >>>>  }
> >>>> @@ -2608,9 +2605,10 @@ static int anx7625_link_bridge(struct drm_dp_aux *aux)
> >>>>      platform->bridge.of_node = dev->of_node;
> >>>>      if (!anx7625_of_panel_on_aux_bus(dev))
> >>>>              platform->bridge.ops |= DRM_BRIDGE_OP_EDID;
> >>>> -    if (!platform->pdata.panel_bridge)
> >>>> -            platform->bridge.ops |= DRM_BRIDGE_OP_HPD |
> >>>> -                                    DRM_BRIDGE_OP_DETECT;
> >>>> +    if (!platform->pdata.panel_bridge ||
> >>>> +        platform->pdata.panel_bridge->type != DRM_MODE_CONNECTOR_eDP) {
> >>>> +            platform->bridge.ops |= DRM_BRIDGE_OP_HPD | DRM_BRIDGE_OP_DETECT;
> >>>> +    }
> >>
> >> Hi Dmitry,
> >>
> >> Thanks, for the review.
> >>
> >> Yes, it is better to check the presence of the 'aux' node for eDP or DP configuration.
> >> Will change it in next patch.
> >>
> >> -       if (!platform->pdata.panel_bridge)
> >> -               platform->bridge.ops |= DRM_BRIDGE_OP_HPD |
> >> -                                       DRM_BRIDGE_OP_DETECT;
> >> +       if (!platform->pdata.panel_bridge || !anx7625_of_panel_on_aux_bus(dev)) {
> > 
> > This is incorrect, if I'm not mistaken, please doublecheck it. I'd
> > suggest following msm_dp_display_get_connector_type() (feel free to
> > extract that to a helper function).
> > 
> 
> Hi Dmirty,
> 
> Thanks, for the review.
> 
> If we see definition of anx7625_of_panel_on_aux_bus() it is doing the same
> thing as msm_dp_display_get_connector_type().

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

Please excuse my confusion.

> 
> static bool anx7625_of_panel_on_aux_bus(struct device *dev)
> {
>         struct device_node *bus, *panel;
> 
>         bus = of_get_child_by_name(dev->of_node, "aux-bus");
> 
>         // if aux-bus is not there it will return the false.
>         if (!bus)
>                 return false;
> 
>         panel = of_get_child_by_name(bus, "panel");
>         of_node_put(bus);
> 
>         // if panel is not there it will return the false.
>         if (!panel)
>                 return false;
>         of_node_put(panel);
> 
>         return true;
> }
> 
> Above function will return true in case of eDP and false in case of DP.
> So we can use anx7625_of_panel_on_aux_bus() to check whether it
> is DP or eDP configuration based on aux.
> 
> I don't think so we need extract msm_dp_display_get_connector_type() to check the eDP or DP configuration based on aux.
> Let me know, if I am missing anything.
> 
> Thanks,
> Ayushi
> 

-- 
With best wishes
Dmitry

[PATCH v2 10/10] drm/bridge: anx7625: change the gpiod_set_value API

[Ayushi Makhija]

Use gpiod_set_value_cansleep() instead of gpiod_set_value()
to fix the below call trace in the boot log:

[    5.690534] Call trace:
[    5.690536]  gpiod_set_value+0x40/0xa4
[    5.690540]  anx7625_runtime_pm_resume+0xa0/0x324 [anx7625]
[    5.690545]  __rpm_callback+0x48/0x1d8
[    5.690549]  rpm_callback+0x6c/0x78

Certain GPIO controllers require access via message-based buses
such as I2C or SPI, which may cause the GPIOs to enter a sleep
state. Therefore, use the gpiod_set_value_cansleep().

Signed-off-by: Ayushi Makhija <quic_amakhija@quicinc.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/bridge/analogix/anx7625.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
index ad99ad19653f..310c9b598fbf 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -1257,10 +1257,10 @@ static void anx7625_power_on(struct anx7625_data *ctx)
 	usleep_range(11000, 12000);
 
 	/* Power on pin enable */
-	gpiod_set_value(ctx->pdata.gpio_p_on, 1);
+	gpiod_set_value_cansleep(ctx->pdata.gpio_p_on, 1);
 	usleep_range(10000, 11000);
 	/* Power reset pin enable */
-	gpiod_set_value(ctx->pdata.gpio_reset, 1);
+	gpiod_set_value_cansleep(ctx->pdata.gpio_reset, 1);
 	usleep_range(10000, 11000);
 
 	DRM_DEV_DEBUG_DRIVER(dev, "power on !\n");
@@ -1280,9 +1280,9 @@ static void anx7625_power_standby(struct anx7625_data *ctx)
 		return;
 	}
 
-	gpiod_set_value(ctx->pdata.gpio_reset, 0);
+	gpiod_set_value_cansleep(ctx->pdata.gpio_reset, 0);
 	usleep_range(1000, 1100);
-	gpiod_set_value(ctx->pdata.gpio_p_on, 0);
+	gpiod_set_value_cansleep(ctx->pdata.gpio_p_on, 0);
 	usleep_range(1000, 1100);
 
 	ret = regulator_bulk_disable(ARRAY_SIZE(ctx->pdata.supplies),
-- 
2.34.1