On Mon, Jul 13, 2015 at 02:10:09PM +0530, Sonika Jindal wrote:
> As per bspec, on BXT A0/A1, sw needs to activate DDIA HPD logic
> and interrupts to check the external panel connection.
> And remove the redundant comment.
>
> v2: Remove redundant IS_BROXTON check, Add comment about port C not
> connected, and rephrase the commit message to include only what we
> are doing here (Imre)
> v3: Add comment about the WA, move 'hpd' initialization outside for
> loop (Siva)
> Also, remove few redundant comments from hpd handler (me)
>
> Signed-off-by: Sonika Jindal <sonika.jindal@xxxxxxxxx>
Can't we do this in two steps:
- Wire up port A hpd in a generic way.
- Add wa for bxt to use port A hpd in the various encoder setup functions
where we assign intel_encoder->hpd_pin.
Currently that switchover is spread all through low-level functions, which
makes this a bit confusion. Imo low-level code shouldn't treat hpd A as
anything but hpd A since that's just confusing. And we already have the
infrastructure for encoders to ask for any hpd pin they want really.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_irq.c | 46 ++++++++++++++++++++++++++-------------
> 1 file changed, 31 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index a897f68..13cabca 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -88,7 +88,14 @@ static const u32 hpd_status_i915[HPD_NUM_PINS] = {
> [HPD_PORT_D] = PORTD_HOTPLUG_INT_STATUS
> };
>
> -/* BXT hpd list */
> +/*
> + * On BXT A0/A1, sw needs to activate DDIA HPD logic and interrupts to check
> + * the external panel connection. Port C is not connected on bxt A0/A1
> + */
> +static const u32 hpd_bxt_a0[HPD_NUM_PINS] = {
> + [HPD_PORT_B] = BXT_DE_PORT_HP_DDIA
> +};
> +
> static const u32 hpd_bxt[HPD_NUM_PINS] = {
> [HPD_PORT_B] = BXT_DE_PORT_HP_DDIB,
> [HPD_PORT_C] = BXT_DE_PORT_HP_DDIC
> @@ -1967,6 +1974,7 @@ static void bxt_hpd_handler(struct drm_device *dev, uint32_t iir_status)
> struct drm_i915_private *dev_priv = dev->dev_private;
> u32 hp_control, hp_trigger;
> u32 pin_mask, long_mask;
> + const u32 *hpd;
>
> /* Get the status */
> hp_trigger = iir_status & BXT_DE_PORT_HOTPLUG_MASK;
> @@ -1981,7 +1989,12 @@ static void bxt_hpd_handler(struct drm_device *dev, uint32_t iir_status)
> /* Clear sticky bits in hpd status */
> I915_WRITE(BXT_HOTPLUG_CTL, hp_control);
>
> - pch_get_hpd_pins(&pin_mask, &long_mask, hp_trigger, hp_control, hpd_bxt);
> + if (INTEL_REVID(dev) < BXT_REVID_B0)
> + hpd = hpd_bxt_a0;
> + else
> + hpd = hpd_bxt;
> +
> + pch_get_hpd_pins(&pin_mask, &long_mask, hp_trigger, hp_control, hpd);
> intel_hpd_irq_handler(dev, pin_mask, long_mask);
> }
>
> @@ -3021,31 +3034,34 @@ static void bxt_hpd_irq_setup(struct drm_device *dev)
> struct intel_encoder *intel_encoder;
> u32 hotplug_port = 0;
> u32 hotplug_ctrl;
> + const u32 *hpd;
>
> - /* Now, enable HPD */
> - for_each_intel_encoder(dev, intel_encoder) {
> + if (INTEL_REVID(dev) < BXT_REVID_B0)
> + hpd = hpd_bxt_a0;
> + else
> + hpd = hpd_bxt;
> +
> + for_each_intel_encoder(dev, intel_encoder)
> if (dev_priv->hotplug.stats[intel_encoder->hpd_pin].state
> == HPD_ENABLED)
> - hotplug_port |= hpd_bxt[intel_encoder->hpd_pin];
> - }
> + hotplug_port |= hpd[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;
> + if (INTEL_REVID(dev) < BXT_REVID_B0 && (hotplug_port & BXT_DE_PORT_HP_DDIA))
> + hotplug_ctrl |= BXT_DDIA_HPD_ENABLE;
> + else {
> + 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);
> --
> 1.7.10.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
