Re: [PATCH v5] drm/i915: add bxt gmbus support

Imre Deak
2015-04-09T11:12:17Z

On Wed, 2015-04-01 at 10:58 +0300, Jani Nikula wrote:
> For BXT gmbus is pulled from PCH to CPU. From implementation point of
> view only pin pair configuration will change. The existing
> implementation supports all platforms previous to GEN8 and also SKL. But
> for BXT pin pair configuration is completely different than SKL or other
> previous GEN's. This patch introduces the new pin pair configuration
> structure specific to BXT and also ensures every real gmbus port has a
> gpio pin.
>
> v3 by Jani: with the platform independent prep work in place, the bxt
> enabling reduces to a fairly trivial patch. Credits are due Sunil for
> giving me the ideas (with his patches) what the platform independent
> parts should look like.
>
> v4: Fix intel_hdmi_init_connector() for bxt. Abstract gmbus_pin access
> more. s/GPU/PCH/ in commit message.
>
> v5: Rebase.
>
> Issue: VIZ-3574
> Signed-off-by: A.Sunil Kamath <sunil.kamath@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 3 +++
> drivers/gpu/drm/i915/intel_hdmi.c | 14 +++++++++++---
> drivers/gpu/drm/i915/intel_i2c.c | 30 +++++++++++++++++++++++++++---
> 3 files changed, 41 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index b134fa39c5b8..a2889013088f 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1797,6 +1797,9 @@ enum skl_disp_power_wells {
> #define GMBUS_PIN_DPB 5 /* SDVO, HDMIB */
> #define GMBUS_PIN_DPD 6 /* HDMID */
> #define GMBUS_PIN_RESERVED 7 /* 7 reserved */
> +#define GMBUS_PIN_1_BXT 1
> +#define GMBUS_PIN_2_BXT 2
> +#define GMBUS_PIN_3_BXT 3

I would have called these GMBUS_PIN_DDI_{B,C,A}_BXT to have a more
descriptive name (and since the pin->port mapping is fixed). Either way
the patch looks ok:
Reviewed-by: Imre Deak <imre.deak@intel.com>

> #define GMBUS_NUM_PINS 7 /* including 0 */
> #define GMBUS1 0x5104 /* command/status */
> #define GMBUS_SW_CLR_INT (1<<31)
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 26222e6c1ff3..587bab6e5f60 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1681,15 +1681,23 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
>
> switch (port) {
> case PORT_B:
> - intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
> + if (IS_BROXTON(dev_priv))
> + intel_hdmi->ddc_bus = GMBUS_PIN_1_BXT;
> + else
> + intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
> intel_encoder->hpd_pin = HPD_PORT_B;
> break;
> case PORT_C:
> - intel_hdmi->ddc_bus = GMBUS_PIN_DPC;
> + if (IS_BROXTON(dev_priv))
> + intel_hdmi->ddc_bus = GMBUS_PIN_2_BXT;
> + else
> + intel_hdmi->ddc_bus = GMBUS_PIN_DPC;
> intel_encoder->hpd_pin = HPD_PORT_C;
> break;
> case PORT_D:
> - if (IS_CHERRYVIEW(dev))
> + if (WARN_ON(IS_BROXTON(dev_priv)))
> + intel_hdmi->ddc_bus = GMBUS_PIN_DISABLED;
> + else if (IS_CHERRYVIEW(dev_priv))
> intel_hdmi->ddc_bus = GMBUS_PIN_DPD_CHV;
> else
> intel_hdmi->ddc_bus = GMBUS_PIN_DPD;
> diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
> index ec9cc8cf642e..cadbc17d2775 100644
> --- a/drivers/gpu/drm/i915/intel_i2c.c
> +++ b/drivers/gpu/drm/i915/intel_i2c.c
> @@ -49,10 +49,33 @@ static const struct gmbus_pin gmbus_pins[] = {
> [GMBUS_PIN_DPD] = { "dpd", GPIOF },
> };
>
> +static const struct gmbus_pin gmbus_pins_bxt[] = {
> + [GMBUS_PIN_1_BXT] = { "dpb", PCH_GPIOB },
> + [GMBUS_PIN_2_BXT] = { "dpc", PCH_GPIOC },
> + [GMBUS_PIN_3_BXT] = { "misc", PCH_GPIOD },
> +};
> +
> +/* pin is expected to be valid */
> +static const struct gmbus_pin *get_gmbus_pin(struct drm_i915_private *dev_priv,
> + unsigned int pin)
> +{
> + if (IS_BROXTON(dev_priv))
> + return &gmbus_pins_bxt[pin];
> + else
> + return &gmbus_pins[pin];
> +}
> +
> bool intel_gmbus_is_valid_pin(struct drm_i915_private *dev_priv,
> unsigned int pin)
> {
> - return pin < ARRAY_SIZE(gmbus_pins) && gmbus_pins[pin].reg;
> + unsigned int size;
> +
> + if (IS_BROXTON(dev_priv))
> + size = ARRAY_SIZE(gmbus_pins_bxt);
> + else
> + size = ARRAY_SIZE(gmbus_pins);
> +
> + return pin < size && get_gmbus_pin(dev_priv, pin)->reg;
> }
>
> /* Intel GPIO access functions */
> @@ -196,7 +219,8 @@ intel_gpio_setup(struct intel_gmbus *bus, unsigned int pin)
>
> algo = &bus->bit_algo;
>
> - bus->gpio_reg = dev_priv->gpio_mmio_base + gmbus_pins[pin].reg;
> + bus->gpio_reg = dev_priv->gpio_mmio_base +
> + get_gmbus_pin(dev_priv, pin)->reg;
>
> bus->adapter.algo_data = algo;
> algo->setsda = set_data;
> @@ -550,7 +574,7 @@ int intel_setup_gmbus(struct drm_device *dev)
> snprintf(bus->adapter.name,
> sizeof(bus->adapter.name),
> "i915 gmbus %s",
> - gmbus_pins[pin].name);
> + get_gmbus_pin(dev_priv, pin)->name);
>
> bus->adapter.dev.parent = &dev->pdev->dev;
> bus->dev_priv = dev_priv;