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

Imre Deak
2015-08-05T09:53:38Z

On Mon, 2015-07-27 at 11:02 +0530, Sonika Jindal wrote:
> WA for BXT A0/A1, where DDIB's HPD pin is swapped to DDIA, so enabling
> DDIA HPD pin in place of DDIB.
>
> 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)
> v3: Rebased on top of Imre's patchset for enabling HPD on PORT A.
> Added hpd_pin swapping for intel_dp_init_connector, setting encoder
> for PORT_A as per the WA in irq_port (Imre)
> v4: Dont enable interrupt for edp, also reframe the description (Siva)
>
> Signed-off-by: Sonika Jindal <sonika.jindal@intel.com>

This patch is a new version of 2/2 of your patchset at
http://lists.freedesktop.org/archives/intel-gfx/2015-July/071580.html

so this should've been sent either as a reply to the individual v1 patch
there, or just by resending the whole patchset. For now you could just
resend the whole patchset. Also please add the version info to the
subject with git format-patch --subject-prefix.

> ---
> drivers/gpu/drm/i915/intel_ddi.c | 11 ++++++++++-
> drivers/gpu/drm/i915/intel_dp.c | 4 ++++
> drivers/gpu/drm/i915/intel_hdmi.c | 9 ++++++++-
> 3 files changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index e2c6f73..8d7ffe0 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3225,7 +3225,16 @@ 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;
> + /* Dont enable interrupts for edp*/
> + else if (port != PORT_A)
> + dev_priv->hotplug.irq_port[port] = intel_dig_port;

Not enabling HPD on eDP is not a BXT specific change (even though it's
disabled now everywhere), so that part should be a separate patch and
ordered before patch 1/2. Also this isn't the proper place to disable
HPD handling; as long as the corresponding intel_encoder->hpd_pin is set
the relevant interrupt will get unmasked and hotplug_work will be
called. See bxt_hpd_irq_setup() and intel_hpd_irq_handler().

> }
>
> /* In theory we don't need the encoder->type check, but leave it just in
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index fcc64e5..71679ef 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5785,9 +5785,13 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
> switch (port) {
> case PORT_A:
> intel_encoder->hpd_pin = HPD_PORT_A;
> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
> + intel_encoder->hpd_pin = HPD_PORT_C;
> break;
> case PORT_B:
> intel_encoder->hpd_pin = HPD_PORT_B;
> + if (IS_BROXTON(dev_priv) && (INTEL_REVID(dev) < BXT_REVID_B0))
> + intel_encoder->hpd_pin = HPD_PORT_A;
> break;
> case PORT_C:
> intel_encoder->hpd_pin = HPD_PORT_C;
> 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))