Re: [PATCH] drm/i915/bxt: WA for swapped HPD pins in A stepping

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 7/18/2015 5:17 AM, Imre Deak wrote:
On Fri, 2015-07-17 at 13:47 +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 DDIC HPD
logic for edp panel.

v2: For DP, irq_port is used to determine the encoder instead of
hpd_pin and removing the edp HPD logic because port A HPD is not
present(Imre)

Signed-off-by: Sonika Jindal <sonika.jindal@xxxxxxxxx>
---
  drivers/gpu/drm/i915/intel_ddi.c  |   10 +++++++++-
  drivers/gpu/drm/i915/intel_hdmi.c |    9 ++++++++-
  2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index e2c6f73..777e3a3 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -3225,7 +3225,15 @@ 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 (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0)
+					 && port == PORT_B)
+			dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;

This happens to work but is confusing. irq_port[PORT_A] will be set here
already and the above will simply overwrite it without explanation. I
would also handle the port == PORT_A case and not set irq_port for it.

The same swapping for hpd_pin is missing from intel_dp_init_connector().

+		else
+			dev_priv->hotplug.irq_port[port] = intel_dig_port;
  	}

  	/* In theory we don't need the encoder->type check, but leave it just in
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 70bad5b..94fa716 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1973,7 +1973,14 @@ void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
  			intel_hdmi->ddc_bus = GMBUS_PIN_1_BXT;
  		else
  			intel_hdmi->ddc_bus = GMBUS_PIN_DPB;
-		intel_encoder->hpd_pin = HPD_PORT_B;
+		/*
+		 * On BXT A0/A1, sw needs to activate DDIA HPD logic and
+		 * interrupts to check the external panel connection.
+		 */
+		if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
+			intel_encoder->hpd_pin = HPD_PORT_A;
+		else
+			intel_encoder->hpd_pin = HPD_PORT_B;
  		break;
  	case PORT_C:
  		if (IS_BROXTON(dev_priv))

As I earlier pointed out with the above approach, you need to add
support for HPD events on the HPD_PORT_A pin. If you look at the
for_each_hpd_pin() macro and intel_hpd_irq_handler()/is_dig_port you'll
notice that any interrupt event on the HPD_PORT_A pin will be ignored
now.
Hmm :(. For now, we can fix for_each_hpd_pin and add a check in intel_hpd_pin_to_port to return PORT_B for pin 0 if A0/A1. Then we can skip the check in intel_ddi_init. But this again will look very confusing.

So two options:
1. We move back to the older approach where we just use another hpd ports array in i915_irq.c
2. Go with current approach.

I feel option 1 is more clean and less prone to further issues.
What do you suggest?

Regards,
Sonika

--Imre

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux