drm/msm: Misc patches

Assign drm_atomic_helper_crtc_set_property helper to MDP4 and MDP5 crtcs' set_property ops. This replaces the custom funcs that returned an error even for standard crtc properties.

Signed-off-by Archit Taneja <[email protected]>

---
 drivers/gpu/drm/msm/mdp/mdp4/mdp4_crtc.c | 9 +--------
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c | 9 +--------
 2 files changed, 2 insertions(+), 16 deletions(-)

drivers/gpu/drm/msm/mdp/mdp4/mdp4_crtc.c +0 -0

index 28df397..4b95d6c 100644

drivers/gpu/drm/msm/mdp/mdp4/mdp4_crtc.c +1 -3

-}
-
 #define CURSOR_WIDTH 64
 #define CURSOR_HEIGHT 64
 

@@ -499,7 +492,7 @@ static const struct drm_crtc_funcs mdp4_crtc_funcs = {
 	.set_config = drm_atomic_helper_set_config,
 	.destroy = mdp4_crtc_destroy,
 	.page_flip = drm_atomic_helper_page_flip,
-	.set_property = mdp4_crtc_set_property,
+	.set_property = drm_atomic_helper_crtc_set_property,
 	.cursor_set = mdp4_crtc_cursor_set,
 	.cursor_move = mdp4_crtc_cursor_move,
 	.reset = drm_atomic_helper_crtc_reset,

drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c +1 -8

index 20cee5c..9a15205 100644
@@ -468,13 +468,6 @@ static void mdp5_crtc_atomic_flush(struct drm_crtc *crtc,
 	request_pending(crtc, PENDING_FLIP);
 }
 
-static int mdp5_crtc_set_property(struct drm_crtc *crtc,
-		struct drm_property *property, uint64_t val)
-{
-	// XXX
-	return -EINVAL;
-}
-
 static void get_roi(struct drm_crtc *crtc, uint32_t *roi_w, uint32_t *roi_h)
 {
 	struct mdp5_crtc *mdp5_crtc = to_mdp5_crtc(crtc);

@@ -625,7 +618,7 @@ static const struct drm_crtc_funcs mdp5_crtc_funcs = {
 	.set_config = drm_atomic_helper_set_config,
 	.destroy = mdp5_crtc_destroy,
 	.page_flip = drm_atomic_helper_page_flip,
-	.set_property = mdp5_crtc_set_property,
+	.set_property = drm_atomic_helper_crtc_set_property,
 	.reset = drm_atomic_helper_crtc_reset,
 	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,

Signature · 7 lines

The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

The DSI driver is currently unaware of how the DSI clock and data pins are mapped to the logical lanes provided by the DSI controller.

Use the generic 'lanes' DT binding provided for DSI lanes (used for DSI in bindings/display/ti/ti,omap4-dss.txt) to get the desired mapping.

The MSM DSI controller is restricted in terms of what all mappings it can support. The lane polarity is fixed for all the lanes, the clock lanes are fixed, and the data lanes can be swapped among each other only for a few combinations. Apply these restrictions when we parse the DT data.

Cc [email protected]

Cc Rob Herring <[email protected]>

Cc Tomi Valkeinen <[email protected]>

Signed-off-by Archit Taneja <[email protected]>

---
 .../devicetree/bindings/display/msm/dsi.txt        |  26 +++-
 drivers/gpu/drm/msm/dsi/dsi_host.c                 | 146 ++++++++++++++++++---
 2 files changed, 149 insertions(+), 23 deletions(-)

Documentation/devicetree/bindings/display/msm/dsi.txt +0 -2

index e7423be..f0d8b6f 100644
@@ -44,9 +44,28 @@ Optional properties:
 - pinctrl-names: the pin control state names; should contain "default"
 - pinctrl-0: the default pinctrl state (active)
 - pinctrl-n: the "sleep" pinctrl state
-- port: DSI controller output port. This contains one endpoint subnode, with its
-  remote-endpoint set to the phandle of the connected panel's endpoint.

Quoted 36 lines · Archit Taneja

The DSI driver is currently unaware of how the DSI clock and data pins are mapped to the logical lanes provided by the DSI controller.

Use the generic 'lanes' DT binding provided for DSI lanes (used for DSI in bindings/display/ti/ti,omap4-dss.txt) to get the desired mapping.

The MSM DSI controller is restricted in terms of what all mappings it can support. The lane polarity is fixed for all the lanes, the clock lanes are fixed, and the data lanes can be swapped among each other only for a few combinations. Apply these restrictions when we parse the DT data.

Cc [email protected]

Cc Rob Herring <[email protected]>

Cc Tomi Valkeinen <[email protected]>

Signed-off-by Archit Taneja <[email protected]>

---
  .../devicetree/bindings/display/msm/dsi.txt        |  26 +++-
  drivers/gpu/drm/msm/dsi/dsi_host.c                 | 146 ++++++++++++++++++---
  2 files changed, 149 insertions(+), 23 deletions(-)

Documentation/devicetree/bindings/display/msm/dsi.txt +3 -3

index e7423be..f0d8b6f 100644
@@ -44,9 +44,28 @@ Optional properties:
  - pinctrl-names: the pin control state names; should contain "default"
  - pinctrl-0: the default pinctrl state (active)
  - pinctrl-n: the "sleep" pinctrl state
-- port: DSI controller output port. This contains one endpoint subnode, with its
-  remote-endpoint set to the phandle of the connected panel's endpoint.
-  See Documentation/devicetree/bindings/graph.txt for device graph info.
+- port: DSI controller output port, containing one endpoint subnode.
+
+  DSI Endpoint properties:

Documentation/devicetree/bindings/display/msm/dsi.txt Optional properties: · line 55

Quoted 6 lines

Documentation/devicetree/bindings/display/msm/dsi.txt +6 -0

+  - remote-endpoint: set to phandle of the connected panel's endpoint.
+    See Documentation/devicetree/bindings/graph.txt for device graph info.
+  - lanes: list of pin numbers for the DSI lanes: CLKp, CLKn, DATA0p, DATA0n,
+    DATA1p, DATA1n, ...
+    This provides a physical to logical mapping of the DSI lanes. The CLKp and
+    CLKn pins have to be mapped to pins 0 and 1. For data lanes, there are only

Then why describe the clk pins?

Quoted 4 lines

Documentation/devicetree/bindings/display/msm/dsi.txt +3 -0

+    a limited number of physical to logical mappings possible:
+
+     "0123": Logic 0->Phys 0; Logic 1->Phys 1; Logic 2->Phys 2; Logic 3->Phys 3;

Documentation/devicetree/bindings/display/msm/dsi.txt Optional properties: · line 68

Quoted 10 lines

Documentation/devicetree/bindings/display/msm/dsi.txt +10 -0

+     "3012": Logic 3->Phys 0; Logic 0->Phys 1; Logic 1->Phys 2; Logic 2->Phys 3;
+     "2301": Logic 2->Phys 0; Logic 3->Phys 1; Logic 0->Phys 2; Logic 1->Phys 3;
+     "1230": Logic 1->Phys 0; Logic 2->Phys 1; Logic 3->Phys 2; Logic 0->Phys 3;
+     "0321": Logic 0->Phys 0; Logic 3->Phys 1; Logic 2->Phys 2; Logic 1->Phys 3;
+     "1032": Logic 1->Phys 0; Logic 0->Phys 1; Logic 3->Phys 2; Logic 2->Phys 3;
+     "2103": Logic 2->Phys 0; Logic 1->Phys 1; Logic 0->Phys 2; Logic 3->Phys 3;
+     "3210": Logic 3->Phys 0; Logic 2->Phys 1; Logic 1->Phys 2; Logic 0->Phys 3;
+
+     Here, a "3012" mapping will be represented by:
+     lanes = <0 1 8 9 2 3 4 5 6 7>;

I'm lost here. What does 8 mean for example. The index represents?

Quoted 47 lines · Rob Herring

Quoted 45 lines · Archit Taneja

The DSI driver is currently unaware of how the DSI clock and data pins are mapped to the logical lanes provided by the DSI controller.

Use the generic 'lanes' DT binding provided for DSI lanes (used for DSI in bindings/display/ti/ti,omap4-dss.txt) to get the desired mapping.

The MSM DSI controller is restricted in terms of what all mappings it can support. The lane polarity is fixed for all the lanes, the clock lanes are fixed, and the data lanes can be swapped among each other only for a few combinations. Apply these restrictions when we parse the DT data.

Cc [email protected]

Cc Rob Herring <[email protected]>

Cc Tomi Valkeinen <[email protected]>

Signed-off-by Archit Taneja <[email protected]>

---
  .../devicetree/bindings/display/msm/dsi.txt        |  26 +++-
  drivers/gpu/drm/msm/dsi/dsi_host.c                 | 146
++++++++++++++++++---
  2 files changed, 149 insertions(+), 23 deletions(-)

Documentation/devicetree/bindings/display/msm/dsi.txt +0 -1

index e7423be..f0d8b6f 100644
@@ -44,9 +44,28 @@ Optional properties:
  - pinctrl-names: the pin control state names; should contain "default"
  - pinctrl-0: the default pinctrl state (active)
  - pinctrl-n: the "sleep" pinctrl state
-- port: DSI controller output port. This contains one endpoint subnode,

with its

Documentation/devicetree/bindings/display/msm/dsi.txt +5 -2

-  remote-endpoint set to the phandle of the connected panel's endpoint.
-  See Documentation/devicetree/bindings/graph.txt for device graph info.
+- port: DSI controller output port, containing one endpoint subnode.
+
+  DSI Endpoint properties:
+  - remote-endpoint: set to phandle of the connected panel's endpoint.
+    See Documentation/devicetree/bindings/graph.txt for device graph

info.
+ - lanes: list of pin numbers for the DSI lanes: CLKp, CLKn, DATA0p, DATA0n,

Quoted 8 lines

Quoted 5 lines

Documentation/devicetree/bindings/display/msm/dsi.txt +2 -0

+    DATA1p, DATA1n, ...
+    This provides a physical to logical mapping of the DSI lanes. The

CLKp and
+ CLKn pins have to be mapped to pins 0 and 1. For data lanes, there
are only

Then why describe the clk pins?

I was trying to use a DT binding that is already used for DSI hosts in TI SoCs.

SoCs give different flexibility over how we map the physical lines with the actual data/clock lanes in the DSI controller. From what I know, this flexibility varies. For example, TI SoCs lets us move clock lanes too, and swap polarities.

If we want all DSI host controllers to use a common binding to describe lanes, we'd need to go with the most flexible one, and the driver restricts it to the subsets that we support.

Quoted 17 lines

Quoted 17 lines

Documentation/devicetree/bindings/display/msm/dsi.txt +3 -0

+    a limited number of physical to logical mappings possible:
+
+     "0123": Logic 0->Phys 0; Logic 1->Phys 1; Logic 2->Phys 2; Logic

3->Phys 3;
+ "3012": Logic 3->Phys 0; Logic 0->Phys 1; Logic 1->Phys 2; Logic
2->Phys 3;
+ "2301": Logic 2->Phys 0; Logic 3->Phys 1; Logic 0->Phys 2; Logic
1->Phys 3;
+ "1230": Logic 1->Phys 0; Logic 2->Phys 1; Logic 3->Phys 2; Logic
0->Phys 3;
+ "0321": Logic 0->Phys 0; Logic 3->Phys 1; Logic 2->Phys 2; Logic
1->Phys 3;
+ "1032": Logic 1->Phys 0; Logic 0->Phys 1; Logic 3->Phys 2; Logic
2->Phys 3;
+ "2103": Logic 2->Phys 0; Logic 1->Phys 1; Logic 0->Phys 2; Logic
3->Phys 3;

Quoted 8 lines

Documentation/devicetree/bindings/display/msm/dsi.txt Optional properties: · line 68

Quoted 5 lines

+ "3210": Logic 3->Phys 0; Logic 2->Phys 1; Logic 1->Phys 2; Logic
0->Phys 3;

Documentation/devicetree/bindings/display/msm/dsi.txt +3 -0

+
+     Here, a "3012" mapping will be represented by:
+     lanes = <0 1 8 9 2 3 4 5 6 7>;

I'm lost here. What does 8 mean for example. The index represents?

The numbers here describe the logical lanes. I.e, the way in which the DSI controller pushes out data to the PHY. The ordering is as follows:

0 - CLKp
1 - CLKn
2 - DATA0p
3 - DATA0n
4 - DATA1p
5 - DATA1n
6 - DATA2p
7 - DATA2n
8 - DATA3p
9 - DATA3n

The indices corresponding to these values represent the actual physical pins. The index 0 will always represent the pin CLKp, index 8 will always represent the physical pin DATA3p. This is something we would want to keep fixed across all SoCs.

The configuration in the example would provide the following mapping:

L: CLKp CLKn DATA3p DATA3n ATA0p DATA0n DATA1p DATA1n DATA2p DATA2n

P: CLKp CLKn DATA0p DATA0n DATA1p DATA1n DATA2p DATA2n DATA3p DATA3n

L represents the logical lanes, and P represents the physical lanes. L is what we provide in the DT property, and P are what the indices represent. Here, the 3rd logical lane is mapped on to the 0th physical data lane. The 0th logical lane to the 1st physical data lane and so on.

Quoted 3 lines

If we want all DSI host controllers to use a common binding to describe lanes, we'd need to go with the most flexible one, and the driver restricts it to the subsets that we support.

Quoted 26 lines

Quoted 24 lines

Quoted 18 lines

Documentation/devicetree/bindings/display/msm/dsi.txt +3 -0

+    a limited number of physical to logical mappings possible:
+
+     "0123": Logic 0->Phys 0; Logic 1->Phys 1; Logic 2->Phys 2; Logic

3->Phys 3;
+ "3012": Logic 3->Phys 0; Logic 0->Phys 1; Logic 1->Phys 2; Logic
2->Phys 3;
+ "2301": Logic 2->Phys 0; Logic 3->Phys 1; Logic 0->Phys 2; Logic
1->Phys 3;
+ "1230": Logic 1->Phys 0; Logic 2->Phys 1; Logic 3->Phys 2; Logic
0->Phys 3;
+ "0321": Logic 0->Phys 0; Logic 3->Phys 1; Logic 2->Phys 2; Logic
1->Phys 3;
+ "1032": Logic 1->Phys 0; Logic 0->Phys 1; Logic 3->Phys 2; Logic
2->Phys 3;
+ "2103": Logic 2->Phys 0; Logic 1->Phys 1; Logic 0->Phys 2; Logic
3->Phys 3;
+ "3210": Logic 3->Phys 0; Logic 2->Phys 1; Logic 1->Phys 2; Logic
0->Phys 3;

Documentation/devicetree/bindings/display/msm/dsi.txt Optional properties: · line 68

Quoted 3 lines

Documentation/devicetree/bindings/display/msm/dsi.txt +3 -0

+
+     Here, a "3012" mapping will be represented by:
+     lanes = <0 1 8 9 2 3 4 5 6 7>;

I'm lost here. What does 8 mean for example. The index represents?

Quoted 2 lines

The numbers here describe the logical lanes. I.e, the way in which the DSI controller pushes out data to the PHY. The ordering is as follows:

Okay, so the confusing part is the description is all about lanes (0-3) but the binding (index and value) is all about pin/signal numbers. Either simplify the binding to be lanes or describe the binding in terms of pins.

Quoted 3 lines · Rob Herring

Quoted 3 lines

If we want all DSI host controllers to use a common binding to describe lanes, we'd need to go with the most flexible one, and the driver restricts it to the subsets that we support.

True, but I wonder if that's necessary. The lane property for the SoC should be read by the SoC specific driver, right? So the DT property can be anything. I'm not sure if there's ever a reason for a generic code to observe the DSI lane setup.

That said, if we do have a common binding, it's perhaps easier for people to understand the setups for different SoCs.

Quoted 26 lines

Quoted 26 lines

Quoted 24 lines

Quoted 18 lines

Documentation/devicetree/bindings/display/msm/dsi.txt +3 -0

+    a limited number of physical to logical mappings possible:
+
+     "0123": Logic 0->Phys 0; Logic 1->Phys 1; Logic 2->Phys 2; Logic

3->Phys 3;
+ "3012": Logic 3->Phys 0; Logic 0->Phys 1; Logic 1->Phys 2; Logic
2->Phys 3;
+ "2301": Logic 2->Phys 0; Logic 3->Phys 1; Logic 0->Phys 2; Logic
1->Phys 3;
+ "1230": Logic 1->Phys 0; Logic 2->Phys 1; Logic 3->Phys 2; Logic
0->Phys 3;
+ "0321": Logic 0->Phys 0; Logic 3->Phys 1; Logic 2->Phys 2; Logic
1->Phys 3;
+ "1032": Logic 1->Phys 0; Logic 0->Phys 1; Logic 3->Phys 2; Logic
2->Phys 3;
+ "2103": Logic 2->Phys 0; Logic 1->Phys 1; Logic 0->Phys 2; Logic
3->Phys 3;
+ "3210": Logic 3->Phys 0; Logic 2->Phys 1; Logic 1->Phys 2; Logic
0->Phys 3;

Documentation/devicetree/bindings/display/msm/dsi.txt Optional properties: · line 68

Quoted 3 lines

Documentation/devicetree/bindings/display/msm/dsi.txt +3 -0

+
+     Here, a "3012" mapping will be represented by:
+     lanes = <0 1 8 9 2 3 4 5 6 7>;

I'm lost here. What does 8 mean for example. The index represents?

Quoted 2 lines

Quoted 2 lines

The numbers here describe the logical lanes. I.e, the way in which the DSI controller pushes out data to the PHY. The ordering is as follows:

If I read you right, I think that's vice versa. At least the OMAP version. The OMAP doc says:

"- lanes: list of pin numbers for the DSI lanes: CLK+, CLK-, DATA0+, DATA0-, DATA1+, DATA1-, ..."

This means that the first value in the array is the pin number for CLK+. In other words, CLK+ wire going to the panel/encoder is connected to pin number X.

What the pin number means is SoC specific. CLK+ could be connected to, say, SoC pin number 123, and then you'd enter 123 as the first value. On OMAP we use DSI block specific pin numbers, so they start at 0.

Quoted 5 lines

Okay, so the confusing part is the description is all about lanes (0-3) but the binding (index and value) is all about pin/signal numbers. Either simplify the binding to be lanes or describe the binding in terms of pins.

Perhaps "lane-pins" or something would be more descriptive. If we want to make the description common, I could change the OMAP implementation to accept the new property name too.

But, I'm not sure if a common description helps much. Any thoughts?

Quoted 11 lines · Tomi Valkeinen

Quoted 3 lines · Rob Herring

Quoted 3 lines

If we want all DSI host controllers to use a common binding to describe lanes, we'd need to go with the most flexible one, and the driver restricts it to the subsets that we support.

True, but I wonder if that's necessary. The lane property for the SoC should be read by the SoC specific driver, right? So the DT property can be anything. I'm not sure if there's ever a reason for a generic code to observe the DSI lane setup.

Yeah, it is very SoC specific.

The only place where it might matter is if a panel/bridge ever needs to know what pins implement what lanes on the platform. A common binding there might help us keep the panel driver generic. Although, this need itself is a bit hypothetical.

Quoted 10 lines · Archit Taneja

Quoted 7 lines · Tomi Valkeinen

Quoted 3 lines · Rob Herring

Quoted 3 lines

If we want all DSI host controllers to use a common binding to describe lanes, we'd need to go with the most flexible one, and the driver restricts it to the subsets that we support.

Quoted 6 lines

Quoted 4 lines

True, but I wonder if that's necessary. The lane property for the SoC should be read by the SoC specific driver, right? So the DT property can be anything. I'm not sure if there's ever a reason for a generic code to observe the DSI lane setup.

Yeah, it is very SoC specific.

Quoted 4 lines

The only place where it might matter is if a panel/bridge ever needs to know what pins implement what lanes on the platform. A common binding there might help us keep the panel driver generic. Although, this need itself is a bit hypothetical.

My opinion here is that if the panel/bridge needs to know something about the DSI lanes/pins, we should have that data in the panel's/bridge's endpoint data.

So if both SoC and the DSI peripheral need complex DSI pin/lane setup, you might have very similar data on both sides. There's possibly some duplication there, but I think it keeps things much simpler.

For example, if the SoC needs OMAP style DSI pin data, and the DSI peripheral needs to know the amount of DSI lanes used (but nothing else), you might think it's nice if the DSI peripheral would peek at the SoC side data, finding out about the DSI lanes.

But I think in that case you should just add a "num-lanes" property to the DSI peripheral. The DT data for a device should be private to the driver handling the device, except for some special cases like following the graph.

Quoted 13 lines · Tomi Valkeinen

Quoted 10 lines · Archit Taneja

Quoted 7 lines · Tomi Valkeinen

Quoted 3 lines · Rob Herring

Quoted 3 lines

If we want all DSI host controllers to use a common binding to describe lanes, we'd need to go with the most flexible one, and the driver restricts it to the subsets that we support.

Quoted 6 lines

Quoted 6 lines

Quoted 4 lines

True, but I wonder if that's necessary. The lane property for the SoC should be read by the SoC specific driver, right? So the DT property can be anything. I'm not sure if there's ever a reason for a generic code to observe the DSI lane setup.

Yeah, it is very SoC specific.

Agreed.