Re: [Intel-gfx] [PATCH 6/6] drm/i915/bxt: Fix irq_port for eDP
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. 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, ) && 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-boun...@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.jin...@intel.com> wrote: From: Durgadoss R <durgados...@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 <durgados...@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
Re: [Intel-gfx] [PATCH 6/6] drm/i915/bxt: Fix irq_port for eDP
All this is because of the BXT A0/A1 workaround for HPD pins. As you can see, this is only done for BXT A0/A1 and can be removed when we have newer stable steppings. Durga can add more.. Regards, Sonika From: Rodrigo Vivi [mailto:rodrigo.v...@gmail.com] 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.jin...@intel.com<mailto:sonika.jin...@intel.com>> wrote: From: Durgadoss R <durgados...@intel.com<mailto:durgados...@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 <durgados...@intel.com<mailto:durgados...@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<mailto: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
Re: [Intel-gfx] [PATCH 6/6] drm/i915/bxt: Fix irq_port for eDP
On Thu, Sep 10, 2015 at 01:07:30PM +, 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, ) && >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-boun...@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.jin...@intel.com> wrote: > From: Durgadoss R <durgados...@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 <durgados...@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
Re: [Intel-gfx] [PATCH 6/6] drm/i915/bxt: Fix irq_port for eDP
On Thu, Sep 10, 2015 at 04:35:57PM +0300, Ville Syrjälä wrote: > On Thu, Sep 10, 2015 at 01:07:30PM +, 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. And we even have the logic to allow that remapping, so I guess the real job is to figure out where that's not done correctly. Not throwing more hacks on top. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
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 Jindalwrote: > From: Durgadoss R > > 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 > --- > 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
[Intel-gfx] [PATCH 6/6] drm/i915/bxt: Fix irq_port for eDP
From: Durgadoss RCurrently, 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 --- 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