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@xxxxxxxxx>
> Cc: Matt Roper <matthew.d.roper@xxxxxxxxx>
> Cc: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> Signed-off-by: Anand Moon <anandx.ram.moon@xxxxxxxxx>
> ---
> 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);
>
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
