Re: [PATCH v6 24/49] drm/i915/bxt: DDI Hotplug interrupt setup

Jani Nikula
2015-04-08T10:33:35Z

On Fri, 27 Mar 2015, Imre Deak <imre.deak@intel.com> wrote:
> From: Shashank Sharma <shashank.sharma@intel.com>
>
> In BXT, DDI hotplug control has been moved to CPU from PCH.
> This patch adds a new IRQ setup function for BXT which:
> 1. Checks which HPD ports are requested to be enabled by encoders.
> 2. Enables those ports in the hot plug control register.
> 3. Un-masks these port interrupts in the IMR register.
> 4. Enables these port interrupts in the IER register.
>
> V3: Kept the default HPD filter count to default (500 us) as per
> satheesh's comment
> v4: Remove unused HPD filter defines (Damien)
> v5: warn if trying to setup HPD on port A (imre)
> v6: fix order of definitions for register bitfields (Daniel)
>
> Reviewed-by: Satheeshakrishna M <satheeshakrishna.m@intel.com>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> (v4)
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
> drivers/gpu/drm/i915/i915_irq.c | 49 ++++++++++++++++++++++++++++++++++++++++-
> drivers/gpu/drm/i915/i915_reg.h | 25 ++++++++++++++++++++-
> 2 files changed, 72 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 14ecb4d..d09b389 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -88,6 +88,12 @@ static const u32 hpd_status_i915[HPD_NUM_PINS] = { /* i915 and valleyview are th
> [HPD_PORT_D] = PORTD_HOTPLUG_INT_STATUS
> };
>
> +/* BXT hpd list */
> +static const u32 hpd_bxt[] = {

You need to specify the size hpd_bxt[HPD_NUM_PINS] here to be defensive
about not going out of bounds.

> + [HPD_PORT_B] = BXT_DE_PORT_HP_DDIB,
> + [HPD_PORT_C] = BXT_DE_PORT_HP_DDIC
> +};
> +
> /* IIR can theoretically queue up two events. Be paranoid. */
> #define GEN8_IRQ_RESET_NDX(type, which) do { \
> I915_WRITE(GEN8_##type##_IMR(which), 0xffffffff); \
> @@ -3178,6 +3184,44 @@ static void ibx_hpd_irq_setup(struct drm_device *dev)
> I915_WRITE(PCH_PORT_HOTPLUG, hotplug);
> }
>
> +static void bxt_hpd_irq_setup(struct drm_device *dev)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct drm_mode_config *mode_config = &dev->mode_config;
> + struct intel_encoder *intel_encoder;
> + u32 hotplug_port = 0;
> + u32 hotplug_ctrl;
> +
> + /* Now, enable HPD */
> + list_for_each_entry(intel_encoder, &mode_config->encoder_list,
> + base.head) {

for_each_intel_encoder

> + if (dev_priv->hpd_stats[intel_encoder->hpd_pin].hpd_mark
> + == HPD_ENABLED)
> + hotplug_port |= hpd_bxt[intel_encoder->hpd_pin];
> + }
> +
> + /* Mask all HPD control bits */
> + hotplug_ctrl = I915_READ(BXT_HOTPLUG_CTL) & ~BXT_HOTPLUG_CTL_MASK;
> +
> + /* Enable requested port in hotplug control */
> + /* TODO: implement (short) HPD support on port A */
> + WARN_ON_ONCE(hotplug_port & BXT_DE_PORT_HP_DDIA);
> + if (hotplug_port & BXT_DE_PORT_HP_DDIB)
> + hotplug_ctrl |= BXT_DDIB_HPD_ENABLE;
> + if (hotplug_port & BXT_DE_PORT_HP_DDIC)
> + hotplug_ctrl |= BXT_DDIC_HPD_ENABLE;
> + I915_WRITE(BXT_HOTPLUG_CTL, hotplug_ctrl);
> +
> + /* Unmask DDI hotplug in IMR */
> + hotplug_ctrl = I915_READ(GEN8_DE_PORT_IMR) & ~hotplug_port;
> + I915_WRITE(GEN8_DE_PORT_IMR, hotplug_ctrl);
> +
> + /* Enable DDI hotplug in IER */
> + hotplug_ctrl = I915_READ(GEN8_DE_PORT_IER) | hotplug_port;
> + I915_WRITE(GEN8_DE_PORT_IER, hotplug_ctrl);
> + POSTING_READ(GEN8_DE_PORT_IER);
> +}
> +
> static void ibx_irq_postinstall(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -4298,7 +4342,10 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
> dev->driver->irq_uninstall = gen8_irq_uninstall;
> dev->driver->enable_vblank = gen8_enable_vblank;
> dev->driver->disable_vblank = gen8_disable_vblank;
> - dev_priv->display.hpd_irq_setup = ibx_hpd_irq_setup;
> + if (HAS_PCH_SPLIT(dev))
> + dev_priv->display.hpd_irq_setup = ibx_hpd_irq_setup;
> + else
> + dev_priv->display.hpd_irq_setup = bxt_hpd_irq_setup;
> } else if (HAS_PCH_SPLIT(dev)) {
> dev->driver->irq_handler = ironlake_irq_handler;
> dev->driver->irq_preinstall = ironlake_irq_reset;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 6a5ade6..a082d7d 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5240,10 +5240,16 @@ enum skl_disp_power_wells {
> #define GEN8_DE_PORT_IMR 0x44444
> #define GEN8_DE_PORT_IIR 0x44448
> #define GEN8_DE_PORT_IER 0x4444c
> -#define GEN8_PORT_DP_A_HOTPLUG (1 << 3)
> #define GEN9_AUX_CHANNEL_D (1 << 27)
> #define GEN9_AUX_CHANNEL_C (1 << 26)
> #define GEN9_AUX_CHANNEL_B (1 << 25)
> +#define BXT_DE_PORT_HP_DDIC (1 << 5)
> +#define BXT_DE_PORT_HP_DDIB (1 << 4)
> +#define BXT_DE_PORT_HP_DDIA (1 << 3)
> +#define BXT_DE_PORT_HOTPLUG_MASK (BXT_DE_PORT_HP_DDIA | \
> + BXT_DE_PORT_HP_DDIB | \
> + BXT_DE_PORT_HP_DDIC)
> +#define GEN8_PORT_DP_A_HOTPLUG (1 << 3)
> #define GEN8_AUX_CHANNEL_A (1 << 0)
>
> #define GEN8_DE_MISC_ISR 0x44460
> @@ -5257,6 +5263,23 @@ enum skl_disp_power_wells {
> #define GEN8_PCU_IIR 0x444e8
> #define GEN8_PCU_IER 0x444ec
>
> +/* BXT hotplug control */
> +#define BXT_HOTPLUG_CTL 0xC4030
> +#define BXT_DDIA_HPD_ENABLE (1 << 28)
> +#define BXT_DDIC_HPD_ENABLE (1 << 12)
> +#define BXT_DDIB_HPD_ENABLE (1 << 4)
> +#define BXT_HOTPLUG_CTL_MASK (BXT_DDIA_HPD_ENABLE | \
> + BXT_DDIB_HPD_ENABLE | \
> + BXT_DDIC_HPD_ENABLE)
> +
> +/* Hot plug status */
> +#define BXT_DDIA_HPD_STATUS (3 << 24)
> +#define BXT_DDIC_HPD_STATUS (3 << 8)
> +#define BXT_DDIB_HPD_STATUS (3 << 0)
> +#define BXT_HPD_STATUS_MASK (BXT_DDIA_HPD_STATUS | \
> + BXT_DDIB_HPD_STATUS | \
> + BXT_DDIC_HPD_STATUS)

I'd appreciate keeping the convention of having two spaces between
#define and the name for register contents (bits, masks, etc.).

> +
> #define ILK_DISPLAY_CHICKEN2 0x42004
> /* Required on all Ironlake and Sandybridge according to the B-Spec. */
> #define ILK_ELPIN_409_SELECT (1 << 25)
> --
> 2.1.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Jani Nikula, Intel Open Source Technology Center