diff mbox series

[3/5] drm/i915: Add missing AUX channel H & I support

Message ID 20191025230623.27829-4-matthew.d.roper@intel.com (mailing list archive)
State New, archived
Headers show
Series DP AUX updates | expand

Commit Message

Matt Roper Oct. 25, 2019, 11:06 p.m. UTC
TGL's extra ports also bring extra AUX channels.

Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/display/intel_bios.c     |  6 ++++
 drivers/gpu/drm/i915/display/intel_display.c  | 36 +++++--------------
 drivers/gpu/drm/i915/display/intel_display.h  |  2 ++
 drivers/gpu/drm/i915/display/intel_dp.c       |  4 +++
 drivers/gpu/drm/i915/display/intel_vbt_defs.h |  2 ++
 5 files changed, 22 insertions(+), 28 deletions(-)

Comments

Matt Roper Oct. 28, 2019, 2:57 p.m. UTC | #1
On Fri, Oct 25, 2019 at 04:13:40PM -0700, Lucas De Marchi wrote:
> On Fri, Oct 25, 2019 at 04:06:21PM -0700, Matt Roper wrote:
> > TGL's extra ports also bring extra AUX channels.
> > 
> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> > ---
> > drivers/gpu/drm/i915/display/intel_bios.c     |  6 ++++
> > drivers/gpu/drm/i915/display/intel_display.c  | 36 +++++--------------
> > drivers/gpu/drm/i915/display/intel_display.h  |  2 ++
> > drivers/gpu/drm/i915/display/intel_dp.c       |  4 +++
> > drivers/gpu/drm/i915/display/intel_vbt_defs.h |  2 ++
> > 5 files changed, 22 insertions(+), 28 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> > index fe302338b7fd..3867b41338a7 100644
> > --- a/drivers/gpu/drm/i915/display/intel_bios.c
> > +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> > @@ -2339,6 +2339,12 @@ enum aux_ch intel_bios_port_aux_ch(struct drm_i915_private *dev_priv,
> > 	case DP_AUX_G:
> > 		aux_ch = AUX_CH_G;
> > 		break;
> > +	case DP_AUX_H:
> > +		aux_ch = AUX_CH_H;
> > +		break;
> > +	case DP_AUX_I:
> > +		aux_ch = AUX_CH_I;
> > +		break;
> 
> I'd rather drop H/I from all other places since we are not using them.

I'm not sure I understand what you mean here.  My understanding was that
OEM's can use non-standard association of port<->aux that's dependent on
their board design.  If we don't honor their desired AUX setting here in
the VBT (or handle it elsewhere in the driver) don't we risk having
display fail on some platforms?


Matt

> 
> Lucas De Marchi
> 
> 
> > 	default:
> > 		MISSING_CASE(info->alternate_aux_channel);
> > 		aux_ch = AUX_CH_A;
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index cbf9cf30050c..e45ed0c07d0d 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -6847,39 +6847,19 @@ intel_aux_power_domain(struct intel_digital_port *dig_port)
> > 
> > 	if (intel_phy_is_tc(dev_priv, phy) &&
> > 	    dig_port->tc_mode == TC_PORT_TBT_ALT) {
> > -		switch (dig_port->aux_ch) {
> > -		case AUX_CH_C:
> > -			return POWER_DOMAIN_AUX_C_TBT;
> > -		case AUX_CH_D:
> > -			return POWER_DOMAIN_AUX_D_TBT;
> > -		case AUX_CH_E:
> > -			return POWER_DOMAIN_AUX_E_TBT;
> > -		case AUX_CH_F:
> > -			return POWER_DOMAIN_AUX_F_TBT;
> > -		case AUX_CH_G:
> > -			return POWER_DOMAIN_AUX_G_TBT;
> > -		default:
> > +		if (dig_port->aux_ch >= AUX_CH_C &&
> > +		    dig_port->aux_ch <= AUX_CH_I) {
> > +			return POWER_DOMAIN_AUX_C_TBT + dig_port->aux_ch -
> > +				AUX_CH_C;
> > +		} else {
> > 			MISSING_CASE(dig_port->aux_ch);
> > 			return POWER_DOMAIN_AUX_C_TBT;
> > 		}
> > 	}
> > 
> > -	switch (dig_port->aux_ch) {
> > -	case AUX_CH_A:
> > -		return POWER_DOMAIN_AUX_A;
> > -	case AUX_CH_B:
> > -		return POWER_DOMAIN_AUX_B;
> > -	case AUX_CH_C:
> > -		return POWER_DOMAIN_AUX_C;
> > -	case AUX_CH_D:
> > -		return POWER_DOMAIN_AUX_D;
> > -	case AUX_CH_E:
> > -		return POWER_DOMAIN_AUX_E;
> > -	case AUX_CH_F:
> > -		return POWER_DOMAIN_AUX_F;
> > -	case AUX_CH_G:
> > -		return POWER_DOMAIN_AUX_G;
> > -	default:
> > +	if (dig_port->aux_ch <= AUX_CH_I) {
> > +		return POWER_DOMAIN_AUX_A + dig_port->aux_ch;
> > +	} else {
> > 		MISSING_CASE(dig_port->aux_ch);
> > 		return POWER_DOMAIN_AUX_A;
> > 	}
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> > index ca7ca2804d8b..9ccaae41a8ad 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display.h
> > @@ -275,6 +275,8 @@ enum aux_ch {
> > 	AUX_CH_E, /* ICL+ */
> > 	AUX_CH_F,
> > 	AUX_CH_G,
> > +	AUX_CH_H,
> > +	AUX_CH_I,
> > };
> > 
> > #define aux_ch_name(a) ((a) + 'A')
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > index 86989ec25bc6..65bab46f7b43 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > @@ -1667,6 +1667,8 @@ static i915_reg_t skl_aux_ctl_reg(struct intel_dp *intel_dp)
> > 	case AUX_CH_E:
> > 	case AUX_CH_F:
> > 	case AUX_CH_G:
> > +	case AUX_CH_H:
> > +	case AUX_CH_I:
> > 		return DP_AUX_CH_CTL(aux_ch);
> > 	default:
> > 		MISSING_CASE(aux_ch);
> > @@ -1688,6 +1690,8 @@ static i915_reg_t skl_aux_data_reg(struct intel_dp *intel_dp, int index)
> > 	case AUX_CH_E:
> > 	case AUX_CH_F:
> > 	case AUX_CH_G:
> > +	case AUX_CH_H:
> > +	case AUX_CH_I:
> > 		return DP_AUX_CH_DATA(aux_ch, index);
> > 	default:
> > 		MISSING_CASE(aux_ch);
> > diff --git a/drivers/gpu/drm/i915/display/intel_vbt_defs.h b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> > index e7057f53866a..49caa066061d 100644
> > --- a/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> > +++ b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> > @@ -329,6 +329,8 @@ enum vbt_gmbus_ddi {
> > #define DP_AUX_E 0x50
> > #define DP_AUX_F 0x60
> > #define DP_AUX_G 0x70
> > +#define DP_AUX_H 0x80
> > +#define DP_AUX_I 0x90
> > 
> > #define VBT_DP_MAX_LINK_RATE_HBR3	0
> > #define VBT_DP_MAX_LINK_RATE_HBR2	1
> > -- 
> > 2.21.0
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Lucas De Marchi Oct. 29, 2019, 5:59 p.m. UTC | #2
On Mon, Oct 28, 2019 at 07:57:12AM -0700, Matt Roper wrote:
>On Fri, Oct 25, 2019 at 04:13:40PM -0700, Lucas De Marchi wrote:
>> On Fri, Oct 25, 2019 at 04:06:21PM -0700, Matt Roper wrote:
>> > TGL's extra ports also bring extra AUX channels.
>> >
>> > Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
>> > ---
>> > drivers/gpu/drm/i915/display/intel_bios.c     |  6 ++++
>> > drivers/gpu/drm/i915/display/intel_display.c  | 36 +++++--------------
>> > drivers/gpu/drm/i915/display/intel_display.h  |  2 ++
>> > drivers/gpu/drm/i915/display/intel_dp.c       |  4 +++
>> > drivers/gpu/drm/i915/display/intel_vbt_defs.h |  2 ++
>> > 5 files changed, 22 insertions(+), 28 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
>> > index fe302338b7fd..3867b41338a7 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_bios.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_bios.c
>> > @@ -2339,6 +2339,12 @@ enum aux_ch intel_bios_port_aux_ch(struct drm_i915_private *dev_priv,
>> > 	case DP_AUX_G:
>> > 		aux_ch = AUX_CH_G;
>> > 		break;
>> > +	case DP_AUX_H:
>> > +		aux_ch = AUX_CH_H;
>> > +		break;
>> > +	case DP_AUX_I:
>> > +		aux_ch = AUX_CH_I;
>> > +		break;
>>
>> I'd rather drop H/I from all other places since we are not using them.
>
>I'm not sure I understand what you mean here.  My understanding was that
>OEM's can use non-standard association of port<->aux that's dependent on
>their board design.  If we don't honor their desired AUX setting here in
>the VBT (or handle it elsewhere in the driver) don't we risk having
>display fail on some platforms?

the pins are not exposed on any soc that we can test... that's why I'd
rather drop them from the other places. We could then bring them back if
needed later.

No strong opinion though. If you decide to go with this patch, then it's


Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>


Lucas De Marchi

>
>
>Matt
>
>>
>> Lucas De Marchi
>>
>>
>> > 	default:
>> > 		MISSING_CASE(info->alternate_aux_channel);
>> > 		aux_ch = AUX_CH_A;
>> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>> > index cbf9cf30050c..e45ed0c07d0d 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_display.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> > @@ -6847,39 +6847,19 @@ intel_aux_power_domain(struct intel_digital_port *dig_port)
>> >
>> > 	if (intel_phy_is_tc(dev_priv, phy) &&
>> > 	    dig_port->tc_mode == TC_PORT_TBT_ALT) {
>> > -		switch (dig_port->aux_ch) {
>> > -		case AUX_CH_C:
>> > -			return POWER_DOMAIN_AUX_C_TBT;
>> > -		case AUX_CH_D:
>> > -			return POWER_DOMAIN_AUX_D_TBT;
>> > -		case AUX_CH_E:
>> > -			return POWER_DOMAIN_AUX_E_TBT;
>> > -		case AUX_CH_F:
>> > -			return POWER_DOMAIN_AUX_F_TBT;
>> > -		case AUX_CH_G:
>> > -			return POWER_DOMAIN_AUX_G_TBT;
>> > -		default:
>> > +		if (dig_port->aux_ch >= AUX_CH_C &&
>> > +		    dig_port->aux_ch <= AUX_CH_I) {
>> > +			return POWER_DOMAIN_AUX_C_TBT + dig_port->aux_ch -
>> > +				AUX_CH_C;
>> > +		} else {
>> > 			MISSING_CASE(dig_port->aux_ch);
>> > 			return POWER_DOMAIN_AUX_C_TBT;
>> > 		}
>> > 	}
>> >
>> > -	switch (dig_port->aux_ch) {
>> > -	case AUX_CH_A:
>> > -		return POWER_DOMAIN_AUX_A;
>> > -	case AUX_CH_B:
>> > -		return POWER_DOMAIN_AUX_B;
>> > -	case AUX_CH_C:
>> > -		return POWER_DOMAIN_AUX_C;
>> > -	case AUX_CH_D:
>> > -		return POWER_DOMAIN_AUX_D;
>> > -	case AUX_CH_E:
>> > -		return POWER_DOMAIN_AUX_E;
>> > -	case AUX_CH_F:
>> > -		return POWER_DOMAIN_AUX_F;
>> > -	case AUX_CH_G:
>> > -		return POWER_DOMAIN_AUX_G;
>> > -	default:
>> > +	if (dig_port->aux_ch <= AUX_CH_I) {
>> > +		return POWER_DOMAIN_AUX_A + dig_port->aux_ch;
>> > +	} else {
>> > 		MISSING_CASE(dig_port->aux_ch);
>> > 		return POWER_DOMAIN_AUX_A;
>> > 	}
>> > diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
>> > index ca7ca2804d8b..9ccaae41a8ad 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_display.h
>> > +++ b/drivers/gpu/drm/i915/display/intel_display.h
>> > @@ -275,6 +275,8 @@ enum aux_ch {
>> > 	AUX_CH_E, /* ICL+ */
>> > 	AUX_CH_F,
>> > 	AUX_CH_G,
>> > +	AUX_CH_H,
>> > +	AUX_CH_I,
>> > };
>> >
>> > #define aux_ch_name(a) ((a) + 'A')
>> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>> > index 86989ec25bc6..65bab46f7b43 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> > @@ -1667,6 +1667,8 @@ static i915_reg_t skl_aux_ctl_reg(struct intel_dp *intel_dp)
>> > 	case AUX_CH_E:
>> > 	case AUX_CH_F:
>> > 	case AUX_CH_G:
>> > +	case AUX_CH_H:
>> > +	case AUX_CH_I:
>> > 		return DP_AUX_CH_CTL(aux_ch);
>> > 	default:
>> > 		MISSING_CASE(aux_ch);
>> > @@ -1688,6 +1690,8 @@ static i915_reg_t skl_aux_data_reg(struct intel_dp *intel_dp, int index)
>> > 	case AUX_CH_E:
>> > 	case AUX_CH_F:
>> > 	case AUX_CH_G:
>> > +	case AUX_CH_H:
>> > +	case AUX_CH_I:
>> > 		return DP_AUX_CH_DATA(aux_ch, index);
>> > 	default:
>> > 		MISSING_CASE(aux_ch);
>> > diff --git a/drivers/gpu/drm/i915/display/intel_vbt_defs.h b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
>> > index e7057f53866a..49caa066061d 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_vbt_defs.h
>> > +++ b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
>> > @@ -329,6 +329,8 @@ enum vbt_gmbus_ddi {
>> > #define DP_AUX_E 0x50
>> > #define DP_AUX_F 0x60
>> > #define DP_AUX_G 0x70
>> > +#define DP_AUX_H 0x80
>> > +#define DP_AUX_I 0x90
>> >
>> > #define VBT_DP_MAX_LINK_RATE_HBR3	0
>> > #define VBT_DP_MAX_LINK_RATE_HBR2	1
>> > --
>> > 2.21.0
>> >
>> > _______________________________________________
>> > Intel-gfx mailing list
>> > Intel-gfx@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>-- 
>Matt Roper
>Graphics Software Engineer
>VTT-OSGC Platform Enablement
>Intel Corporation
>(916) 356-2795
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
index fe302338b7fd..3867b41338a7 100644
--- a/drivers/gpu/drm/i915/display/intel_bios.c
+++ b/drivers/gpu/drm/i915/display/intel_bios.c
@@ -2339,6 +2339,12 @@  enum aux_ch intel_bios_port_aux_ch(struct drm_i915_private *dev_priv,
 	case DP_AUX_G:
 		aux_ch = AUX_CH_G;
 		break;
+	case DP_AUX_H:
+		aux_ch = AUX_CH_H;
+		break;
+	case DP_AUX_I:
+		aux_ch = AUX_CH_I;
+		break;
 	default:
 		MISSING_CASE(info->alternate_aux_channel);
 		aux_ch = AUX_CH_A;
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index cbf9cf30050c..e45ed0c07d0d 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -6847,39 +6847,19 @@  intel_aux_power_domain(struct intel_digital_port *dig_port)
 
 	if (intel_phy_is_tc(dev_priv, phy) &&
 	    dig_port->tc_mode == TC_PORT_TBT_ALT) {
-		switch (dig_port->aux_ch) {
-		case AUX_CH_C:
-			return POWER_DOMAIN_AUX_C_TBT;
-		case AUX_CH_D:
-			return POWER_DOMAIN_AUX_D_TBT;
-		case AUX_CH_E:
-			return POWER_DOMAIN_AUX_E_TBT;
-		case AUX_CH_F:
-			return POWER_DOMAIN_AUX_F_TBT;
-		case AUX_CH_G:
-			return POWER_DOMAIN_AUX_G_TBT;
-		default:
+		if (dig_port->aux_ch >= AUX_CH_C &&
+		    dig_port->aux_ch <= AUX_CH_I) {
+			return POWER_DOMAIN_AUX_C_TBT + dig_port->aux_ch -
+				AUX_CH_C;
+		} else {
 			MISSING_CASE(dig_port->aux_ch);
 			return POWER_DOMAIN_AUX_C_TBT;
 		}
 	}
 
-	switch (dig_port->aux_ch) {
-	case AUX_CH_A:
-		return POWER_DOMAIN_AUX_A;
-	case AUX_CH_B:
-		return POWER_DOMAIN_AUX_B;
-	case AUX_CH_C:
-		return POWER_DOMAIN_AUX_C;
-	case AUX_CH_D:
-		return POWER_DOMAIN_AUX_D;
-	case AUX_CH_E:
-		return POWER_DOMAIN_AUX_E;
-	case AUX_CH_F:
-		return POWER_DOMAIN_AUX_F;
-	case AUX_CH_G:
-		return POWER_DOMAIN_AUX_G;
-	default:
+	if (dig_port->aux_ch <= AUX_CH_I) {
+		return POWER_DOMAIN_AUX_A + dig_port->aux_ch;
+	} else {
 		MISSING_CASE(dig_port->aux_ch);
 		return POWER_DOMAIN_AUX_A;
 	}
diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
index ca7ca2804d8b..9ccaae41a8ad 100644
--- a/drivers/gpu/drm/i915/display/intel_display.h
+++ b/drivers/gpu/drm/i915/display/intel_display.h
@@ -275,6 +275,8 @@  enum aux_ch {
 	AUX_CH_E, /* ICL+ */
 	AUX_CH_F,
 	AUX_CH_G,
+	AUX_CH_H,
+	AUX_CH_I,
 };
 
 #define aux_ch_name(a) ((a) + 'A')
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 86989ec25bc6..65bab46f7b43 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -1667,6 +1667,8 @@  static i915_reg_t skl_aux_ctl_reg(struct intel_dp *intel_dp)
 	case AUX_CH_E:
 	case AUX_CH_F:
 	case AUX_CH_G:
+	case AUX_CH_H:
+	case AUX_CH_I:
 		return DP_AUX_CH_CTL(aux_ch);
 	default:
 		MISSING_CASE(aux_ch);
@@ -1688,6 +1690,8 @@  static i915_reg_t skl_aux_data_reg(struct intel_dp *intel_dp, int index)
 	case AUX_CH_E:
 	case AUX_CH_F:
 	case AUX_CH_G:
+	case AUX_CH_H:
+	case AUX_CH_I:
 		return DP_AUX_CH_DATA(aux_ch, index);
 	default:
 		MISSING_CASE(aux_ch);
diff --git a/drivers/gpu/drm/i915/display/intel_vbt_defs.h b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
index e7057f53866a..49caa066061d 100644
--- a/drivers/gpu/drm/i915/display/intel_vbt_defs.h
+++ b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
@@ -329,6 +329,8 @@  enum vbt_gmbus_ddi {
 #define DP_AUX_E 0x50
 #define DP_AUX_F 0x60
 #define DP_AUX_G 0x70
+#define DP_AUX_H 0x80
+#define DP_AUX_I 0x90
 
 #define VBT_DP_MAX_LINK_RATE_HBR3	0
 #define VBT_DP_MAX_LINK_RATE_HBR2	1