[Intel-gfx] [PATCH] drm/i915/adl_s: Add gmbus pin mapping

[Aditya Swarup]

On 2/10/21 3:54 AM, Anand Moon wrote:
> Add table to map the GMBUS pin pairs to GPIO registers and port to DDC
> mapping for ADL_S as per below Bspec.

Has this patch been tested on an ADLS system? Upstream CI AFAIK doesn't have
support for ADL-S. Also comments below..

> 
> Bspec:20124, 53597.
> 
> Cc: Aditya Swarup <aditya.swarup at intel.com>
> Cc: Matt Roper <matthew.d.roper at intel.com>
> Cc: Lucas De Marchi <lucas.demarchi at intel.com>
> Signed-off-by: Anand Moon <anandx.ram.moon at intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_gmbus.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_gmbus.c b/drivers/gpu/drm/i915/display/intel_gmbus.c
> index 0c952e1d720e..58b8e42d4f90 100644
> --- a/drivers/gpu/drm/i915/display/intel_gmbus.c
> +++ b/drivers/gpu/drm/i915/display/intel_gmbus.c
> @@ -52,6 +52,14 @@ static const struct gmbus_pin gmbus_pins[] = {
>  	[GMBUS_PIN_DPD] = { "dpd", GPIOF },
>  };
>  
> +static const struct gmbus_pin gmbus_pins_adls[] = {
> +	[GMBUS_PIN_1_BXT] = { "edp", GPIOA },

I am pretty sure that GMBUS_PIN_1_BXT should map to GPIOB(1) and not GPIOA(0) like what we have for ICP.

> +	[GMBUS_PIN_9_TC1_ICP] = { "tc1", GPIOD },
> +	[GMBUS_PIN_10_TC2_ICP] = { "tc2", GPIOE },
> +	[GMBUS_PIN_11_TC3_ICP] = { "tc3", GPIOF },
> +	[GMBUS_PIN_12_TC4_ICP] = { "tc4", GPIOG },
> +};
> +
>  static const struct gmbus_pin gmbus_pins_bdw[] = {
>  	[GMBUS_PIN_VGADDC] = { "vga", GPIOA },
>  	[GMBUS_PIN_DPC] = { "dpc", GPIOD },
> @@ -101,7 +109,9 @@ static const struct gmbus_pin gmbus_pins_dg1[] = {
>  static const struct gmbus_pin *get_gmbus_pin(struct drm_i915_private *dev_priv,
>  					     unsigned int pin)
>  {
> -	if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
> +	if (INTEL_PCH_TYPE(dev_priv) == PCH_ADP)

This check should be converted to IS_ALDERLAKE_S(dev_priv) instead of PCH check for PCH_ADP.
Unfortunately, we are reusing PCH_ADP for ADLS and ADLP which have different mappings but the same ADP PCH.
This will break ADLP. 

The PCH_ADP check for ADLS in intel_bios.c will also be changed in the ADLP patches
that are going to be submitted upstream.

So might as well do the correct thing now and change this to IS_ALDERLAKE_S(dev_priv).

> +		return &gmbus_pins_adls[pin];
> +	else if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
>  		return &gmbus_pins_dg1[pin];
>  	else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP)
>  		return &gmbus_pins_icp[pin];
> @@ -122,7 +132,9 @@ bool intel_gmbus_is_valid_pin(struct drm_i915_private *dev_priv,
>  {
>  	unsigned int size;
>  
> -	if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
> +	if (INTEL_PCH_TYPE(dev_priv) == PCH_ADP)

See comment above. Change to IS_ALDERLAKE_S(dev_priv)

Aditya Swarup

> +		size = ARRAY_SIZE(gmbus_pins_adls);
> +	else if (INTEL_PCH_TYPE(dev_priv) >= PCH_DG1)
>  		size = ARRAY_SIZE(gmbus_pins_dg1);
>  	else if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP)
>  		size = ARRAY_SIZE(gmbus_pins_icp);
>