[Intel-gfx] [PATCH v3] drm/i915/gen11: Allow usage of all GPIO pins

Thu Aug 22 23:25:22 UTC 2019

On Fri, Aug 16, 2019 at 05:50:41PM -0700, Matt Roper wrote:
>Our pin mapping tables for ICP and MCC currently only list the standard
>GPIO pins used for various output ports.  Even through ICP's standard
>pin usage only utilizes pins 1, 2, and 9-12, and MCC's standard pin
>usage only uses pins 1, 2, and 9, these platforms do still have GPIO
>registers to address pins in the range 1-3 and 9-14.  OEM's may remap
>GPIO usage in non-standard ways (and provide the actual mapping via VBT
>settings), so we shouldn't exclude pins on these platforms just because
>they aren't part of the standard mappings.
>
>TGP's standard pin tables contains all the possible pins, so let's
>rename them to "icp" and use them for all PCH >= PCH_ICP.  This will
>prevent intel_gmbus_is_valid_pin from rejecting non-standard pin usage
>that an OEM specifies via the VBT.
>
>Note that this will cause pin 9 to be labeled as "tc1" instead of "dpc"
>in debug messages on platforms with the MCC PCH, but that may actually
>help avoid confusion since the text strings will now be the same on all
>gen11+ platforms instead of being different on just EHL.
>
>v2: Drop now-unused MCC_DDC_BUS_DDI_* names.
>
>v3: We want to compare against INTEL_PCH_TYPE, not INTEL_PCH_ID.
>
>Bspec: 8417
>Cc: José Roberto de Souza <jose.souza at intel.com>
>Cc: Lucas De Marchi <lucas.demarchi at intel.com>
>Cc: Vivek Kasireddy <vivek.kasireddy at intel.com>
>Cc: Jani Nikula <jani.nikula at intel.com>
>Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
>Reviewed-by: José Roberto de Souza <jose.souza at intel.com>

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

Can we also try to unify the tables/functions in gmbus.c, bios.c and hdmi.c?

Lucas De Marchi

>---
> drivers/gpu/drm/i915/display/intel_bios.c     | 23 +---------------
> drivers/gpu/drm/i915/display/intel_gmbus.c    | 27 ++-----------------
> drivers/gpu/drm/i915/display/intel_vbt_defs.h |  3 ---
> 3 files changed, 3 insertions(+), 50 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
>index b416b394b641..efb39f350b19 100644
>--- a/drivers/gpu/drm/i915/display/intel_bios.c
>+++ b/drivers/gpu/drm/i915/display/intel_bios.c
>@@ -1341,21 +1341,6 @@ static const u8 cnp_ddc_pin_map[] = {
> };
>
> static const u8 icp_ddc_pin_map[] = {
>-	[ICL_DDC_BUS_DDI_A] = GMBUS_PIN_1_BXT,
>-	[ICL_DDC_BUS_DDI_B] = GMBUS_PIN_2_BXT,
>-	[ICL_DDC_BUS_PORT_1] = GMBUS_PIN_9_TC1_ICP,
>-	[ICL_DDC_BUS_PORT_2] = GMBUS_PIN_10_TC2_ICP,
>-	[ICL_DDC_BUS_PORT_3] = GMBUS_PIN_11_TC3_ICP,
>-	[ICL_DDC_BUS_PORT_4] = GMBUS_PIN_12_TC4_ICP,
>-};
>-
>-static const u8 mcc_ddc_pin_map[] = {
>-	[MCC_DDC_BUS_DDI_A] = GMBUS_PIN_1_BXT,
>-	[MCC_DDC_BUS_DDI_B] = GMBUS_PIN_2_BXT,
>-	[MCC_DDC_BUS_DDI_C] = GMBUS_PIN_9_TC1_ICP,
>-};
>-
>-static const u8 tgp_ddc_pin_map[] = {
> 	[ICL_DDC_BUS_DDI_A] = GMBUS_PIN_1_BXT,
> 	[ICL_DDC_BUS_DDI_B] = GMBUS_PIN_2_BXT,
> 	[TGL_DDC_BUS_DDI_C] = GMBUS_PIN_3_BXT,
>@@ -1372,13 +1357,7 @@ static u8 map_ddc_pin(struct drm_i915_private *dev_priv, u8 vbt_pin)
> 	const u8 *ddc_pin_map;
> 	int n_entries;
>
>-	if (HAS_PCH_TGP(dev_priv)) {
>-		ddc_pin_map = tgp_ddc_pin_map;
>-		n_entries = ARRAY_SIZE(tgp_ddc_pin_map);
>-	} else if (HAS_PCH_MCC(dev_priv)) {
>-		ddc_pin_map = mcc_ddc_pin_map;
>-		n_entries = ARRAY_SIZE(mcc_ddc_pin_map);
>-	} else if (HAS_PCH_ICP(dev_priv)) {
>+	if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP) {
> 		ddc_pin_map = icp_ddc_pin_map;
> 		n_entries = ARRAY_SIZE(icp_ddc_pin_map);
> 	} else if (HAS_PCH_CNP(dev_priv)) {
>diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c b/drivers/gpu/drm/i915/display/intel_gmbus.c
>index 1e27b18aa3fc..d6775a005726 100644
>--- a/drivers/gpu/drm/i915/display/intel_gmbus.c
>+++ b/drivers/gpu/drm/i915/display/intel_gmbus.c
>@@ -80,21 +80,6 @@ static const struct gmbus_pin gmbus_pins_cnp[] = {
> };
>
> static const struct gmbus_pin gmbus_pins_icp[] = {
>-	[GMBUS_PIN_1_BXT] = { "dpa", GPIOB },
>-	[GMBUS_PIN_2_BXT] = { "dpb", GPIOC },
>-	[GMBUS_PIN_9_TC1_ICP] = { "tc1", GPIOJ },
>-	[GMBUS_PIN_10_TC2_ICP] = { "tc2", GPIOK },
>-	[GMBUS_PIN_11_TC3_ICP] = { "tc3", GPIOL },
>-	[GMBUS_PIN_12_TC4_ICP] = { "tc4", GPIOM },
>-};
>-
>-static const struct gmbus_pin gmbus_pins_mcc[] = {
>-	[GMBUS_PIN_1_BXT] = { "dpa", GPIOB },
>-	[GMBUS_PIN_2_BXT] = { "dpb", GPIOC },
>-	[GMBUS_PIN_9_TC1_ICP] = { "dpc", GPIOJ },
>-};
>-
>-static const struct gmbus_pin gmbus_pins_tgp[] = {
> 	[GMBUS_PIN_1_BXT] = { "dpa", GPIOB },
> 	[GMBUS_PIN_2_BXT] = { "dpb", GPIOC },
> 	[GMBUS_PIN_3_BXT] = { "dpc", GPIOD },
>@@ -110,11 +95,7 @@ static const struct gmbus_pin gmbus_pins_tgp[] = {
> static const struct gmbus_pin *get_gmbus_pin(struct drm_i915_private *dev_priv,
> 					     unsigned int pin)
> {
>-	if (HAS_PCH_TGP(dev_priv))
>-		return &gmbus_pins_tgp[pin];
>-	else if (HAS_PCH_MCC(dev_priv))
>-		return &gmbus_pins_mcc[pin];
>-	else if (HAS_PCH_ICP(dev_priv))
>+	if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP)
> 		return &gmbus_pins_icp[pin];
> 	else if (HAS_PCH_CNP(dev_priv))
> 		return &gmbus_pins_cnp[pin];
>@@ -133,11 +114,7 @@ bool intel_gmbus_is_valid_pin(struct drm_i915_private *dev_priv,
> {
> 	unsigned int size;
>
>-	if (HAS_PCH_TGP(dev_priv))
>-		size = ARRAY_SIZE(gmbus_pins_tgp);
>-	else if (HAS_PCH_MCC(dev_priv))
>-		size = ARRAY_SIZE(gmbus_pins_mcc);
>-	else if (HAS_PCH_ICP(dev_priv))
>+	if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP)
> 		size = ARRAY_SIZE(gmbus_pins_icp);
> 	else if (HAS_PCH_CNP(dev_priv))
> 		size = ARRAY_SIZE(gmbus_pins_cnp);
>diff --git a/drivers/gpu/drm/i915/display/intel_vbt_defs.h b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
>index 09cd37fb0b1c..dfcd156b5094 100644
>--- a/drivers/gpu/drm/i915/display/intel_vbt_defs.h
>+++ b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
>@@ -317,9 +317,6 @@ enum vbt_gmbus_ddi {
> 	ICL_DDC_BUS_PORT_4,
> 	TGL_DDC_BUS_PORT_5,
> 	TGL_DDC_BUS_PORT_6,
>-	MCC_DDC_BUS_DDI_A = 0x1,
>-	MCC_DDC_BUS_DDI_B,
>-	MCC_DDC_BUS_DDI_C = 0x4,
> };
>
> #define DP_AUX_A 0x40
>-- 
>2.20.1
>