On Fri 21 May 15:51 CDT 2021, Stephen Boyd wrote:

Quoting Bjorn Andersson (2021-05-21 09:00:29)

quoted

On Fri 21 May 05:27 CDT 2021, Krishna Manikandan wrote:

quoted

diff --git a/Documentation/devicetree/bindings/display/msm/dpu-sc7180.yaml b/Documentation/devicetree/bindings/display/msm/dpu-sc7180.yaml

[..]

quoted

+      ports:
+        $ref: /schemas/graph.yaml#/properties/ports
+        description: |
+          Contains the list of output ports from DPU device. These ports
+          connect to interfaces that are external to the DPU hardware,
+          such as DSI, DP etc. Each output port contains an endpoint that
+          describes how it is connected to an external interface.
+
+        properties:
+          port@0:
+            $ref: /schemas/graph.yaml#/properties/port
+            description: DPU_INTF1 (DSI1)
+
+          port@2:
+            $ref: /schemas/graph.yaml#/properties/port
+            description: DPU_INTF0 (DP)

Why is port@0 INTF1 and why is port@2 INTF0? In the binding you're
translating the two ports that are described are 0 and 1, representing
INTF1 and INTF2, or DSI1 and DSI2, respectively.

Further more, I have a need for somehow describing the pairing of 4 DP
INTFs (INTF 0, 3, 4 and 5) and how they are connected to the 3+1 DP+eDP
controllers.

Downstream this seems to be handled by adding cell-index to the DP
controllers and then matching that against the numbering in the driver's
INTF array. But rather than adding cell-index to map this, can't we
define that the port index is the INTF-number here?


This would obviously break compatibility with existing DTBs, but we
could start by doing it selectively for the new compatibles, fix up the
existing dts files and then drop the selective application after 1 or 2
LTS releases.

I requested that the existing DT not change a while ago when the DP
interface was being added to this binding. Is it possible to figure out
what interface it is that the port is for from the binding? It feels
like the problem is that the driver wants to look through the graph and
make connectors for each one, but it doesn't know what type of connector
to make.

Today there's a single priv->dp pointer which is initialized as the one
and only displayport controller component is bound.
_dpu_kms_set_encoder_mode() has no knowledge about which interface this
single controller is attached to, so dpu_encoder_setup_display() will
always just pick INTF_DP index 0.

So in its current form if your single DP port isn't sitting on the
platform's first DP INTF you need to hack dpu_hw_catalog and remove the
previous ones.

But with my desire to reuse the DP controller code for eDP, and the fact
that I have 3 DP controllers in my laptop we need something more.

But after considering my proposal further I realized that it is too
static for my use case anyway. SC8180x has INTF 0 and 4 are wired to
the DP controllers associated with the two primary USB ports and then
INTF 3 is dynamically switched between them for MST purposes.

So using port indices would prevent me from doing this dynamic dance.

Regards,
Bjorn