Re: [Intel-gfx] [PATCH 11/21] drm/i915/adl_s: Add adl-s ddc pin mapping

From: Matt Roper <matthew.d.roper@intel.com>
To: Aditya Swarup <aditya.swarup@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>,
	intel-gfx@lists.freedesktop.org,
	Lucas De Marchi <lucas.demarchi@intel.com>
Subject: Re: [Intel-gfx] [PATCH 11/21] drm/i915/adl_s: Add adl-s ddc pin mapping
Date: Thu, 19 Nov 2020 16:33:57 -0800	[thread overview]
Message-ID: <20201120003357.GD2099117@mdroper-desk1.amr.corp.intel.com> (raw)
In-Reply-To: <20201117185029.22078-12-aditya.swarup@intel.com>

On Tue, Nov 17, 2020 at 10:50:19AM -0800, Aditya Swarup wrote:
> ADL-S requires TC pins to set up ddc for Combo PHY B, C, D and E.
> Combo PHY A still uses the old ddc pin mapping.
> 
> From VBT, ddc pin info suggests the following mapping:
> VBT 			               DRIVER
> DDI B->ddc_pin=2 should translate to PORT_D->0x9
> DDI C->ddc_pin=3 should translate to PORT_E->0xa
> DDI D->ddc_pin=4 should translate to PORT_F->0xb
> DDI E->ddc_pin=5 should translate to PORT_G->0xc
> 
> Adding pin map to facilitate this translation as we cannot use existing
> icl ddc pin map due to conflict with DDI B and DDI C info.
> 
> Bspec:20124
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: Aditya Swarup <aditya.swarup@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_bios.c     | 13 +++++++++++-
>  drivers/gpu/drm/i915/display/intel_hdmi.c     | 20 ++++++++++++++++++-
>  drivers/gpu/drm/i915/display/intel_vbt_defs.h |  4 ++++
>  3 files changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> index 4cc949b228f2..716a15f87632 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -1623,12 +1623,23 @@ static const u8 icp_ddc_pin_map[] = {
>  	[TGL_DDC_BUS_PORT_6] = GMBUS_PIN_14_TC6_TGP,
>  };
>  
> +static const u8 adls_ddc_pin_map[] = {
> +	[ICL_DDC_BUS_DDI_A] = GMBUS_PIN_1_BXT,
> +	[ADLS_DDC_BUS_PORT_TC1] = GMBUS_PIN_9_TC1_ICP,
> +	[ADLS_DDC_BUS_PORT_TC2] = GMBUS_PIN_10_TC2_ICP,
> +	[ADLS_DDC_BUS_PORT_TC3] = GMBUS_PIN_11_TC3_ICP,
> +	[ADLS_DDC_BUS_PORT_TC4] = GMBUS_PIN_12_TC4_ICP,
> +};
> +
>  static u8 map_ddc_pin(struct drm_i915_private *dev_priv, u8 vbt_pin)
>  {
>  	const u8 *ddc_pin_map;
>  	int n_entries;
>  
> -	if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1) {
> +	if (IS_ALDERLAKE_S(dev_priv)) {

As I mentioned on an earlier patch, these kind of conditions should
probably just be HAS_PCH_ADP().

It's possible that in the future we'll run into a pairing of ADP with a
different GPU that should be handled in a special manner (like we have
with RKL+TGP differing from TGL+TGP), but the first platform should
still stay the "default" for conditions if that happens.

Alternatively if we wind up with another variant of ADP in the future
that behaves differently in general, then presumably it would also have
a different device ID and we could just treat them separately as
HAS_PCH_ADP_S() vs HAS_PCH_ADP_X() or whatever.

But we can cross that bridge if/when we get to it.  For now it's
probably cleanest to just make this condition (and others like it later)
be HAS_PCH_ADP().

Matt

> +		ddc_pin_map = adls_ddc_pin_map;
> +		n_entries = ARRAY_SIZE(adls_ddc_pin_map);
> +	} else if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1) {
>  		return vbt_pin;
>  	} else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP) {
>  		ddc_pin_map = icp_ddc_pin_map;
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index f90838bc74fb..8fdf0623d663 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -3145,6 +3145,22 @@ static u8 dg1_port_to_ddc_pin(struct drm_i915_private *dev_priv, enum port port)
>  	return intel_port_to_phy(dev_priv, port) + 1;
>  }
>  
> +static u8 adls_port_to_ddc_pin(struct drm_i915_private *dev_priv, enum port port)
> +{
> +	enum phy phy = intel_port_to_phy(dev_priv, port);
> +
> +	WARN_ON(port == PORT_B || port == PORT_C);
> +
> +	/*
> +	 * Pin mapping for ADL-S requires TC pins for all combo phy outputs
> +	 * except first combo output.
> +	 */
> +	if (IS_ALDERLAKE_S(dev_priv) && phy >= PHY_B)
> +		return GMBUS_PIN_9_TC1_ICP + phy - PHY_B;
> +
> +	return GMBUS_PIN_1_BXT + phy;
> +}
> +
>  static u8 g4x_port_to_ddc_pin(struct drm_i915_private *dev_priv,
>  			      enum port port)
>  {
> @@ -3182,7 +3198,9 @@ static u8 intel_hdmi_ddc_pin(struct intel_encoder *encoder)
>  		return ddc_pin;
>  	}
>  
> -	if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
> +	if (IS_ALDERLAKE_S(dev_priv))
> +		ddc_pin = adls_port_to_ddc_pin(dev_priv, port);
> +	else if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
>  		ddc_pin = dg1_port_to_ddc_pin(dev_priv, port);
>  	else if (IS_ROCKETLAKE(dev_priv))
>  		ddc_pin = rkl_port_to_ddc_pin(dev_priv, port);
> diff --git a/drivers/gpu/drm/i915/display/intel_vbt_defs.h b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> index 49b4b5fca941..32d1b4f05760 100644
> --- a/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> +++ b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> @@ -325,6 +325,10 @@ enum vbt_gmbus_ddi {
>  	ICL_DDC_BUS_PORT_4,
>  	TGL_DDC_BUS_PORT_5,
>  	TGL_DDC_BUS_PORT_6,
> +	ADLS_DDC_BUS_PORT_TC1 = 0x2,
> +	ADLS_DDC_BUS_PORT_TC2,
> +	ADLS_DDC_BUS_PORT_TC3,
> +	ADLS_DDC_BUS_PORT_TC4
>  };
>  
>  #define DP_AUX_A 0x40
> -- 
> 2.27.0
> 

-- 
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