diff mbox series

[v2,21/21] drm/i915/mtl: Pin assignment for TypeC

Message ID 20230105125446.960504-22-mika.kahola@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/mtl: Add C10 and C20 phy support | expand

Commit Message

Mika Kahola Jan. 5, 2023, 12:54 p.m. UTC
From: Anusha Srivatsa <anusha.srivatsa@intel.com>

Unlike previous platforms that used PORT_TX_DFLEXDPSP
for max_lane calculation, MTL uses only PORT_TX_DFLEXPA1
from which the max_lanes has to be calculated.

Bspec: 50235, 65380
Cc: Mika Kahola <mika.kahola@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
Signed-off-by: Jose Roberto de Souza <jose.souza@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20221014124740.774835-21-mika.kahola@intel.com
---
 drivers/gpu/drm/i915/display/intel_tc.c | 30 +++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

Comments

Luca Coelho Jan. 26, 2023, 2:40 p.m. UTC | #1
On Thu, 2023-01-05 at 14:54 +0200, Mika Kahola wrote:
> From: Anusha Srivatsa <anusha.srivatsa@intel.com>
> 
> Unlike previous platforms that used PORT_TX_DFLEXDPSP
> for max_lane calculation, MTL uses only PORT_TX_DFLEXPA1
> from which the max_lanes has to be calculated.

"have to be calculated" or "the max_lanes value has..."


> Bspec: 50235, 65380
> Cc: Mika Kahola <mika.kahola@intel.com>

Shouldn't you have an s-o-b instead of Cc, since you rebased it and
sent it out?


> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Signed-off-by: Jose Roberto de Souza <jose.souza@intel.com>
> Link: https://patchwork.freedesktop.org/patch/msgid/20221014124740.774835-21-mika.kahola@intel.com
> ---
>  drivers/gpu/drm/i915/display/intel_tc.c | 30 +++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
> index de851fddb255..e7d5301fc35e 100644
> --- a/drivers/gpu/drm/i915/display/intel_tc.c
> +++ b/drivers/gpu/drm/i915/display/intel_tc.c
> @@ -15,6 +15,10 @@
>  #include "intel_mg_phy_regs.h"
>  #include "intel_tc.h"
>  
> +#define DP_PIN_ASSIGNMENT_C	0x3
> +#define DP_PIN_ASSIGNMENT_D	0x4
> +#define DP_PIN_ASSIGNMENT_E	0x5
> +
>  static const char *tc_port_mode_name(enum tc_port_mode mode)
>  {
>  	static const char * const names[] = {
> @@ -147,6 +151,29 @@ u32 intel_tc_port_get_pin_assignment_mask(struct intel_digital_port *dig_port)
>  	       DP_PIN_ASSIGNMENT_SHIFT(dig_port->tc_phy_fia_idx);
>  }
>  
> +static int mtl_tc_port_get_pin_assignment_mask(struct intel_digital_port *dig_port)

This is a bit of a misnomer, since the function doesn't return the pin
assignment mask.  It returns the max lane count.  I suggest
mtl_tc_port_get_max_lane_count(), like in the patch I sent internally.
;)

--
Cheers,
Luca.
Luca Coelho Jan. 26, 2023, 2:43 p.m. UTC | #2
On Thu, 2023-01-26 at 16:40 +0200, Luca Coelho wrote:
> On Thu, 2023-01-05 at 14:54 +0200, Mika Kahola wrote:
> > From: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > 
> > Unlike previous platforms that used PORT_TX_DFLEXDPSP
> > for max_lane calculation, MTL uses only PORT_TX_DFLEXPA1
> > from which the max_lanes has to be calculated.
> 
> "have to be calculated" or "the max_lanes value has..."
> 
> 
> > Bspec: 50235, 65380
> > Cc: Mika Kahola <mika.kahola@intel.com>
> 
> Shouldn't you have an s-o-b instead of Cc, since you rebased it and
> sent it out?
> 
> 
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: Matt Roper <matthew.d.roper@intel.com>
> > Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> > Signed-off-by: Jose Roberto de Souza <jose.souza@intel.com>
> > Link: https://patchwork.freedesktop.org/patch/msgid/20221014124740.774835-21-mika.kahola@intel.com
> > ---
> >  drivers/gpu/drm/i915/display/intel_tc.c | 30 +++++++++++++++++++++++++
> >  1 file changed, 30 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
> > index de851fddb255..e7d5301fc35e 100644
> > --- a/drivers/gpu/drm/i915/display/intel_tc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_tc.c
> > @@ -15,6 +15,10 @@
> >  #include "intel_mg_phy_regs.h"
> >  #include "intel_tc.h"
> >  
> > +#define DP_PIN_ASSIGNMENT_C	0x3
> > +#define DP_PIN_ASSIGNMENT_D	0x4
> > +#define DP_PIN_ASSIGNMENT_E	0x5

Oh, and I also think this should be defined close to the register
definition? These really are register-specific values and not really
related to TypeC itself.


> > +
> >  static const char *tc_port_mode_name(enum tc_port_mode mode)
> >  {
> >  	static const char * const names[] = {
> > @@ -147,6 +151,29 @@ u32 intel_tc_port_get_pin_assignment_mask(struct intel_digital_port *dig_port)
> >  	       DP_PIN_ASSIGNMENT_SHIFT(dig_port->tc_phy_fia_idx);
> >  }
> >  
> > +static int mtl_tc_port_get_pin_assignment_mask(struct intel_digital_port *dig_port)
> 
> This is a bit of a misnomer, since the function doesn't return the pin
> assignment mask.  It returns the max lane count.  I suggest
> mtl_tc_port_get_max_lane_count(), like in the patch I sent internally.
> ;)

--
Cheers,
Luca.
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_tc.c b/drivers/gpu/drm/i915/display/intel_tc.c
index de851fddb255..e7d5301fc35e 100644
--- a/drivers/gpu/drm/i915/display/intel_tc.c
+++ b/drivers/gpu/drm/i915/display/intel_tc.c
@@ -15,6 +15,10 @@ 
 #include "intel_mg_phy_regs.h"
 #include "intel_tc.h"
 
+#define DP_PIN_ASSIGNMENT_C	0x3
+#define DP_PIN_ASSIGNMENT_D	0x4
+#define DP_PIN_ASSIGNMENT_E	0x5
+
 static const char *tc_port_mode_name(enum tc_port_mode mode)
 {
 	static const char * const names[] = {
@@ -147,6 +151,29 @@  u32 intel_tc_port_get_pin_assignment_mask(struct intel_digital_port *dig_port)
 	       DP_PIN_ASSIGNMENT_SHIFT(dig_port->tc_phy_fia_idx);
 }
 
+static int mtl_tc_port_get_pin_assignment_mask(struct intel_digital_port *dig_port)
+{
+	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
+	intel_wakeref_t wakeref;
+	u32 pin_mask;
+
+	assert_tc_cold_blocked(dig_port);
+
+	with_intel_display_power(i915, POWER_DOMAIN_DISPLAY_CORE, wakeref)
+		pin_mask = intel_tc_port_get_pin_assignment_mask(dig_port);
+
+	switch(pin_mask) {
+	default:
+		MISSING_CASE(pin_mask);
+		fallthrough;
+	case DP_PIN_ASSIGNMENT_D:
+		return 2;
+	case DP_PIN_ASSIGNMENT_C:
+	case DP_PIN_ASSIGNMENT_E:
+		return 4;
+	}
+}
+
 int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port)
 {
 	struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
@@ -156,6 +183,9 @@  int intel_tc_port_fia_max_lane_count(struct intel_digital_port *dig_port)
 	if (dig_port->tc_mode != TC_PORT_DP_ALT)
 		return 4;
 
+	if (DISPLAY_VER(i915) >= 14)
+		return mtl_tc_port_get_pin_assignment_mask(dig_port);
+
 	assert_tc_cold_blocked(dig_port);
 
 	lane_mask = 0;