[Intel-gfx] [PATCH 08/12] drm/i915: Fix the IBX transcoder B workarounds
Jesse Barnes
jbarnes at virtuousgeek.org
Thu May 21 12:23:21 PDT 2015
On 05/05/2015 07:17 AM, ville.syrjala at linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> Currently the IBX transcoder B workarounds are not working correctly.
> Well, the HDMI one seems to be working somewhat, but the DP one is
> definitely busted.
>
> After a bit of experimentation it looks like the best way to make this
> work is first disable the port on transcoder B, and then re-enable it
> transcoder A, and immediately disable it again.
>
> We can also clean up the code by noting that we can't be called without
> a valid crtc. And also note that port A on ILK does not need the
> workaround, so let's check for that one too.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 37 ++++++++++++++++-------------
> drivers/gpu/drm/i915/intel_hdmi.c | 50 ++++++++++++++++++---------------------
> drivers/gpu/drm/i915/intel_sdvo.c | 41 +++++++++++++-------------------
> 3 files changed, 60 insertions(+), 68 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 17b006c..3401cde 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3821,6 +3821,7 @@ static void
> intel_dp_link_down(struct intel_dp *intel_dp)
> {
> struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> + struct intel_crtc *crtc = to_intel_crtc(intel_dig_port->base.base.crtc);
> enum port port = intel_dig_port->port;
> struct drm_device *dev = intel_dig_port->base.base.dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -3837,34 +3838,38 @@ intel_dp_link_down(struct intel_dp *intel_dp)
> if ((IS_GEN7(dev) && port == PORT_A) ||
> (HAS_PCH_CPT(dev) && port != PORT_A)) {
> DP &= ~DP_LINK_TRAIN_MASK_CPT;
> - I915_WRITE(intel_dp->output_reg, DP | DP_LINK_TRAIN_PAT_IDLE_CPT);
> + DP |= DP_LINK_TRAIN_PAT_IDLE_CPT;
> } else {
> if (IS_CHERRYVIEW(dev))
> DP &= ~DP_LINK_TRAIN_MASK_CHV;
> else
> DP &= ~DP_LINK_TRAIN_MASK;
> - I915_WRITE(intel_dp->output_reg, DP | DP_LINK_TRAIN_PAT_IDLE);
> + DP |= DP_LINK_TRAIN_PAT_IDLE;
> }
> + I915_WRITE(intel_dp->output_reg, DP);
> POSTING_READ(intel_dp->output_reg);
>
> - if (HAS_PCH_IBX(dev) &&
> - I915_READ(intel_dp->output_reg) & DP_PIPEB_SELECT) {
> - /* Hardware workaround: leaving our transcoder select
> - * set to transcoder B while it's off will prevent the
> - * corresponding HDMI output on transcoder A.
> - *
> - * Combine this with another hardware workaround:
> - * transcoder select bit can only be cleared while the
> - * port is enabled.
> - */
> - DP &= ~DP_PIPEB_SELECT;
> + DP &= ~(DP_PORT_EN | DP_AUDIO_OUTPUT_ENABLE);
> + I915_WRITE(intel_dp->output_reg, DP);
> + POSTING_READ(intel_dp->output_reg);
> +
> + /*
> + * HW workaround for IBX, we need to move the port
> + * to transcoder A after disabling it to allow the
> + * matching HDMI port to be enabled on transcoder A.
> + */
> + if (HAS_PCH_IBX(dev) && crtc->pipe == PIPE_B && port != PORT_A) {
> + /* always enable with pattern 1 (as per spec) */
> + DP &= ~(DP_PIPEB_SELECT | DP_LINK_TRAIN_MASK);
> + DP |= DP_PORT_EN | DP_LINK_TRAIN_PAT_1;
> + I915_WRITE(intel_dp->output_reg, DP);
> + POSTING_READ(intel_dp->output_reg);
> +
> + DP &= ~DP_PORT_EN;
> I915_WRITE(intel_dp->output_reg, DP);
> POSTING_READ(intel_dp->output_reg);
> }
>
> - DP &= ~DP_AUDIO_OUTPUT_ENABLE;
> - I915_WRITE(intel_dp->output_reg, DP & ~DP_PORT_EN);
> - POSTING_READ(intel_dp->output_reg);
> msleep(intel_dp->panel_power_down_delay);
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 308015e..9b9a69e 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1096,39 +1096,13 @@ static void intel_disable_hdmi(struct intel_encoder *encoder)
> struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
> struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
> u32 temp;
> - u32 enable_bits = SDVO_ENABLE | SDVO_AUDIO_ENABLE;
>
> if (crtc->config->has_audio)
> intel_audio_codec_disable(encoder);
>
> temp = I915_READ(intel_hdmi->hdmi_reg);
>
> - /* HW workaround for IBX, we need to move the port to transcoder A
> - * before disabling it. */
> - if (HAS_PCH_IBX(dev)) {
> - struct drm_crtc *crtc = encoder->base.crtc;
> - int pipe = crtc ? to_intel_crtc(crtc)->pipe : -1;
> -
> - if (temp & SDVO_PIPE_B_SELECT) {
> - temp &= ~SDVO_PIPE_B_SELECT;
> - I915_WRITE(intel_hdmi->hdmi_reg, temp);
> - POSTING_READ(intel_hdmi->hdmi_reg);
> -
> - /* Again we need to write this twice. */
> - I915_WRITE(intel_hdmi->hdmi_reg, temp);
> - POSTING_READ(intel_hdmi->hdmi_reg);
> -
> - /* Transcoder selection bits only update
> - * effectively on vblank. */
> - if (crtc)
> - intel_wait_for_vblank(dev, pipe);
> - else
> - msleep(50);
> - }
> - }
> -
> - temp &= ~enable_bits;
> -
> + temp &= ~(SDVO_ENABLE | SDVO_AUDIO_ENABLE);
> I915_WRITE(intel_hdmi->hdmi_reg, temp);
> POSTING_READ(intel_hdmi->hdmi_reg);
>
> @@ -1136,6 +1110,28 @@ static void intel_disable_hdmi(struct intel_encoder *encoder)
> chv_powergate_phy_lanes(encoder, 0xf);
>
> intel_hdmi->set_infoframes(&encoder->base, false, NULL);
> +
> + /*
> + * HW workaround for IBX, we need to move the port
> + * to transcoder A after disabling it to allow the
> + * matching DP port to be enabled on transcoder A.
> + */
> + if (HAS_PCH_IBX(dev) && crtc->pipe == PIPE_B) {
> + temp &= ~SDVO_PIPE_B_SELECT;
> + temp |= SDVO_ENABLE;
> + /*
> + * HW workaround, need to write this twice for issue
> + * that may result in first write getting masked.
> + */
> + I915_WRITE(intel_hdmi->hdmi_reg, temp);
> + POSTING_READ(intel_hdmi->hdmi_reg);
> + I915_WRITE(intel_hdmi->hdmi_reg, temp);
> + POSTING_READ(intel_hdmi->hdmi_reg);
> +
> + temp &= ~SDVO_ENABLE;
> + I915_WRITE(intel_hdmi->hdmi_reg, temp);
> + POSTING_READ(intel_hdmi->hdmi_reg);
> + }
> }
>
> static int hdmi_portclock_limit(struct intel_hdmi *hdmi, bool respect_dvi_limit)
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index e3e9c98..4a87691 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -1437,6 +1437,7 @@ static void intel_disable_sdvo(struct intel_encoder *encoder)
> {
> struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
> struct intel_sdvo *intel_sdvo = to_sdvo(encoder);
> + struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
> u32 temp;
>
> intel_sdvo_set_active_outputs(intel_sdvo, 0);
> @@ -1445,32 +1446,22 @@ static void intel_disable_sdvo(struct intel_encoder *encoder)
> DRM_MODE_DPMS_OFF);
>
> temp = I915_READ(intel_sdvo->sdvo_reg);
> - if ((temp & SDVO_ENABLE) != 0) {
> - /* HW workaround for IBX, we need to move the port to
> - * transcoder A before disabling it. */
> - if (HAS_PCH_IBX(encoder->base.dev)) {
> - struct drm_crtc *crtc = encoder->base.crtc;
> - int pipe = crtc ? to_intel_crtc(crtc)->pipe : -1;
> -
> - if (temp & SDVO_PIPE_B_SELECT) {
> - temp &= ~SDVO_PIPE_B_SELECT;
> - I915_WRITE(intel_sdvo->sdvo_reg, temp);
> - POSTING_READ(intel_sdvo->sdvo_reg);
> -
> - /* Again we need to write this twice. */
> - I915_WRITE(intel_sdvo->sdvo_reg, temp);
> - POSTING_READ(intel_sdvo->sdvo_reg);
> -
> - /* Transcoder selection bits only update
> - * effectively on vblank. */
> - if (crtc)
> - intel_wait_for_vblank(encoder->base.dev, pipe);
> - else
> - msleep(50);
> - }
> - }
>
> - intel_sdvo_write_sdvox(intel_sdvo, temp & ~SDVO_ENABLE);
> + temp &= ~SDVO_ENABLE;
> + intel_sdvo_write_sdvox(intel_sdvo, temp);
> +
> + /*
> + * HW workaround for IBX, we need to move the port
> + * to transcoder A after disabling it to allow the
> + * matching DP port to be enabled on transcoder A.
> + */
> + if (HAS_PCH_IBX(dev_priv) && crtc->pipe == PIPE_B) {
> + temp &= ~SDVO_PIPE_B_SELECT;
> + temp |= SDVO_ENABLE;
> + intel_sdvo_write_sdvox(intel_sdvo, temp);
> +
> + temp &= ~SDVO_ENABLE;
> + intel_sdvo_write_sdvox(intel_sdvo, temp);
> }
> }
>
>
Cool, again, testing wins.
Reviewed-by: Jesse Barnes <jbarnes at virtuousgeek.org>
More information about the Intel-gfx
mailing list