From patchwork Fri Oct 26 08:58:19 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 1650691 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 82981DF2F6 for ; Fri, 26 Oct 2012 09:51:11 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 5359C9E997 for ; Fri, 26 Oct 2012 02:51:11 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-wi0-f177.google.com (mail-wi0-f177.google.com [209.85.212.177]) by gabe.freedesktop.org (Postfix) with ESMTP id EB8E99EBEF for ; Fri, 26 Oct 2012 01:58:33 -0700 (PDT) Received: by mail-wi0-f177.google.com with SMTP id hj13so153744wib.12 for ; Fri, 26 Oct 2012 01:58:32 -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=HihGVpVebqagh53hk/SohuUjfv7kyg5VldNZdbqIQgo=; b=DPj/ZIK5B7ytHjm9bfD5+elziot2UKjo9+0pjIBp+3T9QXikR2Bbcjs4sXOZ1VvNig ba4moLot4FFzoo9FYbJrUqIUnImHsdkt2kDfkRCv42Im/nuO/VwjrsQu0/8BOlFTC10p HroHYszPEwtdONtIbz59u5txS/Qe1jiF2AQX0= 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=HihGVpVebqagh53hk/SohuUjfv7kyg5VldNZdbqIQgo=; b=pvfATfg4ZSrsVruyjvziwhqhZMWXZOG7b8aa/fBzCZgGks5IuFOWx5ibOs4xrrO8MS vFgCox56jaRNN+xKbqbqc548GJw+E39Q8wIEg8RnO53QGlBi2nZAuL9WbWYvlMbWayOc 913FQCvM/pd9Ju3ruDBwK7K5xtPvMVFXIvySlFWSGsHg1VhPaDbeXerAbYzrybSQlstl wrzfi8iiBLfZw3QmH7xFtLu3JCDpUCtlgl8s9S7b+kw2drHYwKJNHUdAYh/whOxB7AAB T7MSbJMubifJQa8P4wB5njEyR2iGgYXU4e0AK3SxGkso6s9Rf+4QAnj/dKtRZrTrI4GB 1dbA== Received: by 10.216.202.164 with SMTP id d36mr12555315weo.55.1351241912313; Fri, 26 Oct 2012 01:58:32 -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 k20sm14217364wiv.11.2012.10.26.01.58.31 (version=TLSv1/SSLv3 cipher=OTHER); Fri, 26 Oct 2012 01:58:31 -0700 (PDT) From: Daniel Vetter To: Intel Graphics Development Date: Fri, 26 Oct 2012 10:58:19 +0200 Message-Id: <1351241899-7870-10-git-send-email-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 1.7.11.4 In-Reply-To: <1351241899-7870-1-git-send-email-daniel.vetter@ffwll.ch> References: <1351241899-7870-1-git-send-email-daniel.vetter@ffwll.ch> X-Gm-Message-State: ALoCoQkDbKHKVqf8w/56/gFeddtknL/HwWHFQGH2l+Gs2aSNvspvGs/S+v/CnFNtT/w6uKha7VqD Cc: Daniel Vetter Subject: [Intel-gfx] [PATCH 9/9] 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. Signed-off-by: Daniel Vetter v3: Use the new ->modeset_global_resources callback. v4: Move the WARNs to the right place. Oh how I hate hacks. 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 7009c0f..90617cf 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_gloabl_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 enabled 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; } } @@ -4875,6 +4901,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) @@ -5073,7 +5181,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) { @@ -5210,8 +5318,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); @@ -5229,7 +5341,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, @@ -8185,6 +8297,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_gloabl_resources; } else if (IS_HASWELL(dev)) { dev_priv->display.fdi_link_train = hsw_fdi_link_train; dev_priv->display.write_eld = haswell_write_eld;