From patchwork Sat Oct 27 13:58:40 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 1655751 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by patchwork2.kernel.org (Postfix) with ESMTP id F37D5DFAC4 for ; Sat, 27 Oct 2012 13:58:59 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 95A499E877 for ; Sat, 27 Oct 2012 06:58:59 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-ea0-f177.google.com (mail-ea0-f177.google.com [209.85.215.177]) by gabe.freedesktop.org (Postfix) with ESMTP id CDBAD9E872 for ; Sat, 27 Oct 2012 06:58:46 -0700 (PDT) Received: by mail-ea0-f177.google.com with SMTP id n13so1189095eaa.36 for ; Sat, 27 Oct 2012 06:58:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=from:to:cc:subject:date:message-id:x-mailer:in-reply-to:references; bh=Efbdiy2tu847xgPMemTImwt8bKziOqzuRlvhUaWYJRo=; b=E49bLq0VzfH5vASCRCpc+m8DXxMTsI4FcFkit+jQ1JKUoswOFz0oL8gFPLTI3VB1FU M944iUD/LrACjtqdrn+mhgsK4AfDwSMwu9m1ZUjosGPgpWldqIyOmknxSy2nUD+XUnix YHtKdq37kVTOR+WGbJ4eAT8PTp4herIG7+8s4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=from:to:cc:subject:date:message-id:x-mailer:in-reply-to:references :x-gm-message-state; bh=Efbdiy2tu847xgPMemTImwt8bKziOqzuRlvhUaWYJRo=; b=YzqH3Oao36GU3/P5UbxT+c2nS19ygYQtGSpgsHFPmiHylwkWlo7n/aXVd2yqyFdSiH q2CJdgB9NVhxvL+Pgc8kNG7AYGrzwvGCKmZ57jCjirXaE1cF0LqMQUpoTm60bf99tRt2 Jhs5n+78PmAFdL+9QN+8O/QifOr1Z4zdFMNrTSigsQoit9j0WXjiaowXdQO95/3Igh3a 71fvT49EG6sDDwpyawfIJNmu+P9kBK7auZFT/KOF9giGHrrIaM/FzKC8TMrx5I3uLnmt 7HcGXpXfpOaWLn+t+/zh7J2B/W+NAivKlP+f7/uRklwhCamrfoncA0yJtmiSQmKjP+az 7cZg== Received: by 10.14.179.69 with SMTP id g45mr33282934eem.42.1351346325662; Sat, 27 Oct 2012 06:58:45 -0700 (PDT) Received: from bremse.ffwll.local (178-83-130-250.dynamic.hispeed.ch. [178.83.130.250]) by mx.google.com with ESMTPS id z43sm9455378een.16.2012.10.27.06.58.44 (version=TLSv1/SSLv3 cipher=OTHER); Sat, 27 Oct 2012 06:58:45 -0700 (PDT) From: Daniel Vetter To: Intel Graphics Development Date: Sat, 27 Oct 2012 15:58:40 +0200 Message-Id: <1351346320-32740-1-git-send-email-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 1.7.11.4 In-Reply-To: <1351241899-7870-10-git-send-email-daniel.vetter@ffwll.ch> References: <1351241899-7870-10-git-send-email-daniel.vetter@ffwll.ch> X-Gm-Message-State: ALoCoQmgL73igGfk7i7q3KvHjakFn9dA0fWh/h2+yU4XzDwUEv82Mb9TZEd87xFACXap9joFVqQT Cc: Daniel Vetter Subject: [Intel-gfx] [PATCH] drm/i915: check fdi B/C lane sharing constraint X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: intel-gfx-bounces+patchwork-intel-gfx=patchwork.kernel.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+patchwork-intel-gfx=patchwork.kernel.org@lists.freedesktop.org And properly toggle the chicken bit in the pch to enable/disable fdi C rx. If we don't set this bit correctly, the rx gets confused in link training, which can result in an fdi link that silently fails to train the link (since the corresponding register reports success). Note that both fdi link B and C can suffer when this bit is not set correctly. The code as-is has a few deficiencies: - We presume all pipes use the pch which is not the case for cpu edp. - We don't bother with disabling both pipes when we could make things work, e.g. when pipe B switched from 4 to 2 lanes due to a mode change, we don't bother updating the w/a bit. - It's ugly. All of these are because we compute ->fdi_lanes way too late, when we're already setting up individual pipes. We need to have this information in ->modeset_global_resources already, to set things up correctly. But that is a much larger reorg of the code. Note that we actually hit the 2 lanes limit in practice rather quickly: Even though the 1920x1200 mode native mode of my screen fits into 2 lanes, it needs 3 lanes for the 1920x1080 (since that somehow has much more blanking ...). Not obeying this restriction seems to results in cute-looking digital noise. v2: Only ever clear the chicken bit when both pipes are off. v3: Use the new ->modeset_global_resources callback. v4: Move the WARNs to the right place. Oh how I hate hacks. v5: Fix spelling, noticed by Paulo Zanoni. Signed-off-by: Daniel Vetter Reviewed-by: Paulo Zanoni --- drivers/gpu/drm/i915/i915_reg.h | 5 +- drivers/gpu/drm/i915/intel_display.c | 122 +++++++++++++++++++++++++++++++++-- 2 files changed, 121 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 84b09ee..f681d01 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -3810,8 +3810,9 @@ #define SOUTH_CHICKEN1 0xc2000 #define FDIA_PHASE_SYNC_SHIFT_OVR 19 #define FDIA_PHASE_SYNC_SHIFT_EN 18 -#define FDI_PHASE_SYNC_OVR(pipe) (1<<(FDIA_PHASE_SYNC_SHIFT_OVR - ((pipe) * 2))) -#define FDI_PHASE_SYNC_EN(pipe) (1<<(FDIA_PHASE_SYNC_SHIFT_EN - ((pipe) * 2))) +#define FDI_PHASE_SYNC_OVR(pipe) (1<<(FDIA_PHASE_SYNC_SHIFT_OVR - ((pipe) * 2))) +#define FDI_PHASE_SYNC_EN(pipe) (1<<(FDIA_PHASE_SYNC_SHIFT_EN - ((pipe) * 2))) +#define FDI_BC_BIFURCATION_SELECT (1 << 12) #define SOUTH_CHICKEN2 0xc2004 #define DPLS_EDP_PPS_FIX_DIS (1<<0) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 43ac99b..befc06c 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2324,6 +2324,29 @@ static void cpt_phase_pointer_enable(struct drm_device *dev, int pipe) POSTING_READ(SOUTH_CHICKEN1); } +static void ivb_modeset_global_resources(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + struct intel_crtc *pipe_B_crtc = + to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_B]); + struct intel_crtc *pipe_C_crtc = + to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_C]); + uint32_t temp; + + /* When everything is off disable fdi C so that we could enable fdi B + * with all lanes. XXX: This misses the case where a pipe is not using + * any pch resources and so doesn't need any fdi lanes. */ + if (!pipe_B_crtc->base.enabled && !pipe_C_crtc->base.enabled) { + WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE); + WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE); + + temp = I915_READ(SOUTH_CHICKEN1); + temp &= ~FDI_BC_BIFURCATION_SELECT; + DRM_DEBUG_KMS("disabling fdi C rx\n"); + I915_WRITE(SOUTH_CHICKEN1, temp); + } +} + /* The FDI link training functions for ILK/Ibexpeak. */ static void ironlake_fdi_link_train(struct drm_crtc *crtc) { @@ -2580,6 +2603,9 @@ static void ivb_manual_fdi_link_train(struct drm_crtc *crtc) POSTING_READ(reg); udelay(150); + DRM_DEBUG_KMS("FDI_RX_IIR before link train 0x%x\n", + I915_READ(FDI_RX_IIR(pipe))); + /* enable CPU FDI TX and PCH FDI RX */ reg = FDI_TX_CTL(pipe); temp = I915_READ(reg); @@ -2625,7 +2651,7 @@ static void ivb_manual_fdi_link_train(struct drm_crtc *crtc) if (temp & FDI_RX_BIT_LOCK || (I915_READ(reg) & FDI_RX_BIT_LOCK)) { I915_WRITE(reg, temp | FDI_RX_BIT_LOCK); - DRM_DEBUG_KMS("FDI train 1 done.\n"); + DRM_DEBUG_KMS("FDI train 1 done, level %i.\n", i); break; } } @@ -2666,7 +2692,7 @@ static void ivb_manual_fdi_link_train(struct drm_crtc *crtc) if (temp & FDI_RX_SYMBOL_LOCK) { I915_WRITE(reg, temp | FDI_RX_SYMBOL_LOCK); - DRM_DEBUG_KMS("FDI train 2 done.\n"); + DRM_DEBUG_KMS("FDI train 2 done, level %i.\n", i); break; } } @@ -4878,6 +4904,88 @@ static bool ironlake_compute_clocks(struct drm_crtc *crtc, return true; } +static void cpt_enable_fdi_bc_bifurcation(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + uint32_t temp; + + temp = I915_READ(SOUTH_CHICKEN1); + if (temp & FDI_BC_BIFURCATION_SELECT) + return; + + WARN_ON(I915_READ(FDI_RX_CTL(PIPE_B)) & FDI_RX_ENABLE); + WARN_ON(I915_READ(FDI_RX_CTL(PIPE_C)) & FDI_RX_ENABLE); + + temp |= FDI_BC_BIFURCATION_SELECT; + DRM_DEBUG_KMS("enabling fdi C rx\n"); + I915_WRITE(SOUTH_CHICKEN1, temp); + POSTING_READ(SOUTH_CHICKEN1); +} + +static bool ironlake_check_fdi_lanes(struct intel_crtc *intel_crtc) +{ + struct drm_device *dev = intel_crtc->base.dev; + struct drm_i915_private *dev_priv = dev->dev_private; + struct intel_crtc *pipe_B_crtc = + to_intel_crtc(dev_priv->pipe_to_crtc_mapping[PIPE_B]); + + DRM_DEBUG_KMS("checking fdi config on pipe %i, lanes %i\n", + intel_crtc->pipe, intel_crtc->fdi_lanes); + if (intel_crtc->fdi_lanes > 4) { + DRM_DEBUG_KMS("invalid fdi lane config on pipe %i: %i lanes\n", + intel_crtc->pipe, intel_crtc->fdi_lanes); + /* Clamp lanes to avoid programming the hw with bogus values. */ + intel_crtc->fdi_lanes = 4; + + return false; + } + + if (dev_priv->num_pipe == 2) + return true; + + switch (intel_crtc->pipe) { + case PIPE_A: + return true; + case PIPE_B: + if (dev_priv->pipe_to_crtc_mapping[PIPE_C]->enabled && + intel_crtc->fdi_lanes > 2) { + DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %i: %i lanes\n", + intel_crtc->pipe, intel_crtc->fdi_lanes); + /* Clamp lanes to avoid programming the hw with bogus values. */ + intel_crtc->fdi_lanes = 2; + + return false; + } + + if (intel_crtc->fdi_lanes > 2) + WARN_ON(I915_READ(SOUTH_CHICKEN1) & FDI_BC_BIFURCATION_SELECT); + else + cpt_enable_fdi_bc_bifurcation(dev); + + return true; + case PIPE_C: + if (!pipe_B_crtc->base.enabled || pipe_B_crtc->fdi_lanes <= 2) { + if (intel_crtc->fdi_lanes > 2) { + DRM_DEBUG_KMS("invalid shared fdi lane config on pipe %i: %i lanes\n", + intel_crtc->pipe, intel_crtc->fdi_lanes); + /* Clamp lanes to avoid programming the hw with bogus values. */ + intel_crtc->fdi_lanes = 2; + + return false; + } + } else { + DRM_DEBUG_KMS("fdi link B uses too many lanes to enable link C\n"); + return false; + } + + cpt_enable_fdi_bc_bifurcation(dev); + + return true; + default: + BUG(); + } +} + static void ironlake_set_m_n(struct drm_crtc *crtc, struct drm_display_mode *mode, struct drm_display_mode *adjusted_mode) @@ -5076,7 +5184,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc, struct intel_encoder *encoder; u32 temp; int ret; - bool dither; + bool dither, fdi_config_ok; for_each_encoder_on_crtc(dev, crtc, encoder) { switch (encoder->type) { @@ -5213,8 +5321,12 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc, intel_set_pipe_timings(intel_crtc, mode, adjusted_mode); + /* Note, this also computes intel_crtc->fdi_lanes which is used below in + * ironlake_check_fdi_lanes. */ ironlake_set_m_n(crtc, mode, adjusted_mode); + fdi_config_ok = ironlake_check_fdi_lanes(intel_crtc); + if (is_cpu_edp) ironlake_set_pll_edp(crtc, adjusted_mode->clock); @@ -5232,7 +5344,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc, intel_update_linetime_watermarks(dev, pipe, adjusted_mode); - return ret; + return fdi_config_ok ? ret : -EINVAL; } static int haswell_crtc_mode_set(struct drm_crtc *crtc, @@ -8188,6 +8300,8 @@ static void intel_init_display(struct drm_device *dev) /* FIXME: detect B0+ stepping and use auto training */ dev_priv->display.fdi_link_train = ivb_manual_fdi_link_train; dev_priv->display.write_eld = ironlake_write_eld; + dev_priv->display.modeset_global_resources = + ivb_modeset_global_resources; } else if (IS_HASWELL(dev)) { dev_priv->display.fdi_link_train = hsw_fdi_link_train; dev_priv->display.write_eld = haswell_write_eld;