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

[Jani Nikula <jani.nikula@intel.com>]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx