Re: [PATCH 6/6] drm/i915/bxt: Fix irq_port for eDP

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Sep 10, 2015 at 01:07:30PM +0000, R, Durgadoss wrote:
> Hi Rodrigo,
> 
> I had to add this to get HDMI hotplug working on BXT.
> As you might already know, we have the HPD pins A & B
> swapped in BXT. And, we are using HPD_PORT_A as
> 'intel_encoder->hpd_pin' for HDMI on port B.

People keep saying that, but according to the spec it's not
even true. A and B are not swapped, but instead all the pins
are sort of shifted by one position.

HPD A = DDI0 / port B
HPD B = DDI1 / port C
HPD C = DDI2 / port A

If the code actually did the remapping in a way that matches
the spec, things would be a lot less confusing.

> 
> Without this, When an HPD on HDMI (port B) happens, the interrupt
> is handled as an eDP (port A) interrupt in 'intel_hpd_irq_handler',
> since hpd_pin for HDMI port B is set as PIN A.
> 
> Snippet:
> ---
> is_dig_port = intel_hpd_pin_to_port(i, &port) &&
>                                dev_priv->hotplug.irq_port[port];
> 
> ---
> 
> The issue occurs only when we have eDP + HDMI combination.
> MIPI + HDMI works well without this fix also.
> 
> And all these workarounds, are due to pin swap in
> BXT A0/A1. We have this fix enclosed with that check as well.
> 
> I tried to few other changes, but this one was less intrusive
> and easy to change (and notice the change).
> Kindly let us know if you have better ideas.
> 
> Thanks,
> Durga
> 
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Rodrigo Vivi
> Sent: Thursday, September 10, 2015 12:54 AM
> To: Jindal, Sonika; intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 6/6] drm/i915/bxt: Fix irq_port for eDP
> 
> Nak: I don't believe we need this... Actually I believe we need the opposite... we need to enable HPD on port A for eDP errors handling...
> 
> 
> 
> 
> On Fri, Sep 4, 2015 at 6:38 AM Sonika Jindal <sonika.jindal@intel.com> wrote:
> From: Durgadoss R <durgadoss.r@intel.com>
> 
> Currently, HDMI hotplug with eDP as local panel is failing
> because the HDMI hpd is detected as a long hpd for eDP; and is
> thus rightfully ignored. But, it should really be handled as
> an interrupt on port B for HDMI (due to BXT A1 platform having
> HPD pins A and B swapped). This patch sets the irq_port[PORT_A]
> to NULL in case eDP is on port A so that irq handler does not
> treat it as a 'dig_port' interrupt.
> 
> Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c |   15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 4823184..fec51df 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3218,15 +3218,20 @@ void intel_ddi_init(struct drm_device *dev, enum port port)
>                         goto err;
> 
>                 intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
> +               dev_priv->hotplug.irq_port[port] = intel_dig_port;
>                 /*
>                  * On BXT A0/A1, sw needs to activate DDIA HPD logic and
>                  * interrupts to check the external panel connection.
> +                * If eDP is connected on port A, set irq_port to NULL
> +                * so that we do not assume an interrupt here as a
> +                * 'dig_port' interrupt.
>                  */
> -               if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0)
> -                                        && port == PORT_B)
> -                       dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
> -               else
> -                       dev_priv->hotplug.irq_port[port] = intel_dig_port;
> +               if (IS_BROXTON(dev) && (INTEL_REVID(dev) < BXT_REVID_B0)) {
> +                       if (port == PORT_B)
> +                               dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
> +                       else if (intel_encoder->type == INTEL_OUTPUT_EDP)
> +                               dev_priv->hotplug.irq_port[port] = NULL;
> +               }
>         }
> 
>         /* In theory we don't need the encoder->type check, but leave it just in
> --
> 1.7.10.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx