Re: [PATCH 7/9] drm/i915/display: add phy, vbt and ddi indexes

Lucas De Marchi <lucas.demarchi@xxxxxxxxx> · Mon, 23 Dec 2019 16:17:44 -0800

On Mon, Dec 23, 2019 at 04:10:44PM -0800, Matt Roper wrote:
On Mon, Dec 23, 2019 at 11:58:48AM -0800, Lucas De Marchi wrote:
Identify 3 possible cases in which the index numbers can be different
from the "port" and add them to the description-based ddi initialization
table.  This can be used in place of additional functions mapping from
on to the other.  Right now we already cover part of this by creating kind of
virtual phy numbering, but that comes with downsides:

a) there's not really a "phy numbering" in the spec, this is purely a
software thing; hardware uses whatever they want thinking mapping from
one to the other arbitrarily is easy in software.

b) currently the mapping occurs on "leaf" functions, making the decision
based on the platform.

With this new table the approach will be: the port as defined by the
enum port is purely a driver convention and won't be used anymore to
define the register offset or register bits. For that we have the other
3 indexes, identified as being possibly different from the current usage
of register bits: ddi, vbt and phy. The phy type is also added here,
meant to replace the checks for combo vs tc (although the helper
functions can remain so we may differentiate between, e.g. Dekel and MG
phys).

While at it, also give names to the ports so they can be easily
identified.

Signed-off-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
---
 drivers/gpu/drm/i915/display/intel_display.c  | 54 +++++++++----------
 drivers/gpu/drm/i915/display/intel_display.h  |  7 +++
 .../drm/i915/display/intel_display_types.h    |  5 ++
 3 files changed, 39 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index ad85cf75c815..219f180fa395 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -16277,14 +16277,14 @@ static bool ddi_is_port_present(struct drm_i915_private *i915,
 static const struct intel_output tgl_output = {
 	.dsi_init = icl_dsi_init,
 	.ddi_ports = {
-		{ .port = PORT_A },
-		{ .port = PORT_B },
-		{ .port = PORT_D },
-		{ .port = PORT_E },
-		{ .port = PORT_F },
-		{ .port = PORT_G },
-		{ .port = PORT_H },
-		{ .port = PORT_I },
+		{ .name = "DDI A",   .port = PORT_A, .phy_type = PHY_TYPE_COMBO, .ddi_idx = 0x0, .phy_idx = 0x0, .vbt_idx = 0x0, },
+		{ .name = "DDI B",   .port = PORT_B, .phy_type = PHY_TYPE_COMBO, .ddi_idx = 0x1, .phy_idx = 0x1, .vbt_idx = 0x1, },
+		{ .name = "DDI TC1", .port = PORT_D, .phy_type = PHY_TYPE_TC,    .ddi_idx = 0x3, .phy_idx = 0x0, .vbt_idx = 0x2, },
+		{ .name = "DDI TC2", .port = PORT_E, .phy_type = PHY_TYPE_TC,    .ddi_idx = 0x4, .phy_idx = 0x1, .vbt_idx = 0x3, },
+		{ .name = "DDI TC3", .port = PORT_F, .phy_type = PHY_TYPE_TC,    .ddi_idx = 0x5, .phy_idx = 0x2, .vbt_idx = 0x4, },
+		{ .name = "DDI TC4", .port = PORT_G, .phy_type = PHY_TYPE_TC,    .ddi_idx = 0x6, .phy_idx = 0x3, .vbt_idx = 0x5, },
+		{ .name = "DDI TC5", .port = PORT_H, .phy_type = PHY_TYPE_TC,    .ddi_idx = 0x7, .phy_idx = 0x4, .vbt_idx = 0x6, },
+		{ .name = "DDI TC6", .port = PORT_I, .phy_type = PHY_TYPE_TC,    .ddi_idx = 0x8, .phy_idx = 0x5, .vbt_idx = 0x7, },
 		{ .port = PORT_NONE }
 	}
 };
@@ -16293,12 +16293,12 @@ static const struct intel_output icl_output = {
 	.dsi_init = icl_dsi_init,
 	.is_port_present = icl_is_port_present,
 	.ddi_ports = {
-		{ .port = PORT_A },
-		{ .port = PORT_B },
-		{ .port = PORT_C },
-		{ .port = PORT_D },
-		{ .port = PORT_E },
-		{ .port = PORT_F },
+		{ .name = "DDI A",   .port = PORT_A, .phy_type = PHY_TYPE_COMBO, .ddi_idx = 0x0, .phy_idx = 0x0, .vbt_idx = 0x0, },
+		{ .name = "DDI B",   .port = PORT_B, .phy_type = PHY_TYPE_COMBO, .ddi_idx = 0x1, .phy_idx = 0x1, .vbt_idx = 0x1, },
+		{ .name = "DDI TC1", .port = PORT_C, .phy_type = PHY_TYPE_TC,    .ddi_idx = 0x2, .phy_idx = 0x0, .vbt_idx = 0x2, },
+		{ .name = "DDI TC2", .port = PORT_D, .phy_type = PHY_TYPE_TC,    .ddi_idx = 0x3, .phy_idx = 0x1, .vbt_idx = 0x3, },
+		{ .name = "DDI TC3", .port = PORT_E, .phy_type = PHY_TYPE_TC,    .ddi_idx = 0x4, .phy_idx = 0x2, .vbt_idx = 0x4, },
+		{ .name = "DDI TC4", .port = PORT_F, .phy_type = PHY_TYPE_TC,    .ddi_idx = 0x5, .phy_idx = 0x3, .vbt_idx = 0x5, },
 		{ .port = PORT_NONE }
 	}
 };
@@ -16306,10 +16306,10 @@ static const struct intel_output icl_output = {
 static const struct intel_output ehl_output = {
 	.dsi_init = icl_dsi_init,
 	.ddi_ports = {
-		{ .port = PORT_A },
-		{ .port = PORT_B },
-		{ .port = PORT_C },
-		{ .port = PORT_D },
+		{ .name = "DDI A",   .port = PORT_A, .phy_type = PHY_TYPE_COMBO, .ddi_idx = 0x0, .phy_idx = 0x0, .vbt_idx = 0x0, },
+		{ .name = "DDI B",   .port = PORT_B, .phy_type = PHY_TYPE_COMBO, .ddi_idx = 0x1, .phy_idx = 0x1, .vbt_idx = 0x1, },
+		{ .name = "DDI C",   .port = PORT_C, .phy_type = PHY_TYPE_COMBO, .ddi_idx = 0x2, .phy_idx = 0x2, .vbt_idx = 0x2, },
+		{ .name = "DDI D",   .port = PORT_D, .phy_type = PHY_TYPE_COMBO, .ddi_idx = 0x3, .phy_idx = 0x0, .vbt_idx = 0x3, },
 		{ .port = PORT_NONE }
 	}
 };
@@ -16317,9 +16317,9 @@ static const struct intel_output ehl_output = {
 static const struct intel_output gen9lp_output = {
 	.dsi_init = vlv_dsi_init,
 	.ddi_ports = {
-		{ .port = PORT_A },
-		{ .port = PORT_B },
-		{ .port = PORT_C },
+		{ .name = "DDI A", .port = PORT_A, .ddi_idx = 0x0, .phy_idx = 0x0, .vbt_idx = 0x0 },
+		{ .name = "DDI B", .port = PORT_B, .ddi_idx = 0x1, .phy_idx = 0x1, .vbt_idx = 0x1 },
+		{ .name = "DDI C", .port = PORT_C, .ddi_idx = 0x2, .phy_idx = 0x2, .vbt_idx = 0x2 },
 		{ .port = PORT_NONE }
 	},
 };
@@ -16327,12 +16327,12 @@ static const struct intel_output gen9lp_output = {
 static const struct intel_output ddi_output = {
 	.is_port_present = ddi_is_port_present,
 	.ddi_ports = {
-		{ .port = PORT_A },
-		{ .port = PORT_B },
-		{ .port = PORT_C },
-		{ .port = PORT_D },
-		{ .port = PORT_E },
-		{ .port = PORT_F },
+		{ .name = "DDI A", .port = PORT_A, .ddi_idx = 0x0, .phy_idx = 0x0, .vbt_idx = 0x0 },
+		{ .name = "DDI B", .port = PORT_B, .ddi_idx = 0x1, .phy_idx = 0x1, .vbt_idx = 0x1 },
+		{ .name = "DDI C", .port = PORT_C, .ddi_idx = 0x2, .phy_idx = 0x2, .vbt_idx = 0x2 },
+		{ .name = "DDI D", .port = PORT_D, .ddi_idx = 0x3, .phy_idx = 0x3, .vbt_idx = 0x3 },
+		{ .name = "DDI E", .port = PORT_E, .ddi_idx = 0x4, .phy_idx = 0x4, .vbt_idx = 0x4 },
+		{ .name = "DDI F", .port = PORT_F, .ddi_idx = 0x5, .phy_idx = 0x5, .vbt_idx = 0x5 },
 		{ .port = PORT_NONE }
 	}
 };
diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
index 05d68bd393dc..db19e5eee248 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -303,6 +303,13 @@ enum phy {
 	I915_MAX_PHYS
 };

+enum phy_type {
+	PHY_TYPE_NONE = 0,
+
+	PHY_TYPE_COMBO,
+	PHY_TYPE_TC,
+};

Why not go ahead and actually specify the true phy type in the enum?

       enum phy_type {
               PHY_TYPE_NONE = 0,

               PHY_TYPE_COMBO,
               PHY_TYPE_MG,
               PHY_TYPE_DKL,
               PHY_TYPE_FOO,
               PHY_TYPE_BAR,
       };

and then once your series is complete the helpers can figure out whether
something is "type c" with something like
  type == MG || type == DKL || type == BAR

yeah, I thought about that but I couldn't find a compelling reason to
differentiate DKL and MG.  There's only a single place where it could be
useful, but I didn't convert enough of the driver to make sure it
actually is.

that's a good option indeed, I will think about it while doing the rest
of the conversion if people agree with the general idea on this series.

thanks
Lucas De Marchi



Matt

+
 #define phy_name(a) ((a) + 'A')

 enum phy_fia {
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 4d2f4ee35812..23a885895803 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1377,7 +1377,12 @@ struct intel_dp_mst_encoder {
 };

 struct intel_ddi_port_info {
+	const char *name;
 	enum port port;
+	s8 phy_type;
+	u8 ddi_idx;
+	u8 phy_idx;
+	u8 vbt_idx;
 };

 static inline enum dpio_channel
--
2.24.0


--
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx






