From patchwork Wed Jul 15 11:57:35 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lukas Wunner X-Patchwork-Id: 7000701 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 684EA9F373 for ; Wed, 12 Aug 2015 11:40:00 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 654D720678 for ; Wed, 12 Aug 2015 11:39:59 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 4B7FD206D4 for ; Wed, 12 Aug 2015 11:39:58 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 9A7596EB2E; Wed, 12 Aug 2015 04:39:53 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mailout2.hostsharing.net (mailout2.hostsharing.net [83.223.90.233]) by gabe.freedesktop.org (Postfix) with ESMTPS id EA3156EB22; Wed, 12 Aug 2015 04:39:51 -0700 (PDT) Received: from h08.hostsharing.net (h08.hostsharing.net [83.223.95.28]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mailout2.hostsharing.net (Postfix) with ESMTPS id B7F82100C6F29; Wed, 12 Aug 2015 13:31:15 +0200 (CEST) Received: from localhost (6-38-90-81.adsl.cmo.de [81.90.38.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by h08.hostsharing.net (Postfix) with ESMTPSA id 3AEB460423EA; Wed, 12 Aug 2015 13:31:13 +0200 (CEST) X-Mailbox-Line: From 88b49891e95ced43bfb57cc8472bb69985827153 Mon Sep 17 00:00:00 2001 Message-Id: <88b49891e95ced43bfb57cc8472bb69985827153.1439288957.git.lukas@wunner.de> In-Reply-To: References: <29bed586baf62f6be77b7ab0ba1b8f5cb3be3aad.1439288957.git.lukas@wunner.de> <164b43588e80baaddb7a4d1081785c4d03a89c4b.1439288957.git.lukas@wunner.de> <27944adb13aa1ab246ee4a1ebb833e397324d073.1439288957.git.lukas@wunner.de> <2ac3eca0759cedd1009221cbef908605f8d29e1e.1439288957.git.lukas@wunner.de> <832f1cfceab9d9403b541b51733b87110fd8e019.1439288957.git.lukas@wunner.de> From: Lukas Wunner Date: Wed, 15 Jul 2015 13:57:35 +0200 To: dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org Cc: Andreas Heider , Paul Hordiienko , William Brown , Bruno Bierbaumer , Matthew Garrett , Dave Airlie Subject: [Intel-gfx] [PATCH v2 12/22] drm/i915: Preserve SSC earlier X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Spam-Status: No, score=-0.8 required=5.0 tests=BAYES_00, DATE_IN_PAST_96_XX, RCVD_IN_DNSWL_MED,RP_MATCHES_RCVD,UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Commit 92122789b2d6 ("drm/i915: preserve SSC if previously set v3") added code to intel_modeset_gem_init to override the SSC status read from VBT with the SSC status set by BIOS. However, intel_modeset_gem_init is invoked *after* intel_modeset_init, which calls intel_setup_outputs, which *modifies* SSC status by way of intel_init_pch_refclk. So unlike advertised, intel_modeset_gem_init doesn't preserve the SSC status set by BIOS but whatever intel_init_pch_refclk decided on. This is a problem on dual gpu laptops such as the MacBook Pro which require either a handler to switch DDC lines, or the discrete gpu to proxy DDC/AUX communication: Both the handler and the discrete gpu may initialize after the i915 driver, and consequently, an LVDS connector may initially seem disconnected and the SSC therefore is disabled by intel_init_pch_refclk, but on reprobe the connector may turn out to be connected and the SSC must then be enabled. Due to 92122789b2d6 however, the SSC is not enabled on reprobe since it is assumed BIOS disabled it while in fact it was disabled by intel_init_pch_refclk. Also, because the SSC status is preserved so late, the preserved value only ever gets used on resume but not on panel initialization: intel_modeset_init calls intel_init_display which indirectly calls intel_panel_use_ssc via multiple subroutines, *before* the BIOS value overrides the VBT value in intel_modeset_gem_init (intel_panel_use_ssc is the sole user of dev_priv->vbt.lvds_use_ssc). Fix this by moving the code introduced by 92122789b2d6 from intel_modeset_gem_init to intel_modeset_init before the invocation of intel_setup_outputs and intel_init_display. Add a DRM_DEBUG_KMS as suggested way back by Jani: http://lists.freedesktop.org/archives/intel-gfx/2014-June/046666.html Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=88861 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=61115 Tested-by: Paul Hordiienko [MBP 6,2 2010 intel ILK + nvidia GT216 pre-retina] Tested-by: William Brown [MBP 8,2 2011 intel SNB + amd turks pre-retina] Tested-by: Lukas Wunner [MBP 9,1 2012 intel IVB + nvidia GK107 pre-retina] Tested-by: Bruno Bierbaumer [MBP 11,3 2013 intel HSW + nvidia GK107 retina -- work in progress] Fixes: 92122789b2d6 ("drm/i915: preserve SSC if previously set v3") Signed-off-by: Lukas Wunner Reviewed-by: Jesse Barnes --- drivers/gpu/drm/i915/intel_display.c | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index af0bcfe..6335883 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -14893,6 +14893,24 @@ void intel_modeset_init(struct drm_device *dev) if (INTEL_INFO(dev)->num_pipes == 0) return; + /* + * There may be no VBT; and if the BIOS enabled SSC we can + * just keep using it to avoid unnecessary flicker. Whereas if the + * BIOS isn't using it, don't assume it will work even if the VBT + * indicates as much. + */ + if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) { + bool bios_lvds_use_ssc = !!(I915_READ(PCH_DREF_CONTROL) & + DREF_SSC1_ENABLE); + + if (dev_priv->vbt.lvds_use_ssc != bios_lvds_use_ssc) { + DRM_DEBUG_KMS("SSC %sabled by BIOS, overriding VBT which says %sabled\n", + bios_lvds_use_ssc ? "en" : "dis", + dev_priv->vbt.lvds_use_ssc ? "en" : "dis"); + dev_priv->vbt.lvds_use_ssc = bios_lvds_use_ssc; + } + } + intel_init_display(dev); intel_init_audio(dev); @@ -15446,7 +15464,6 @@ err: void intel_modeset_gem_init(struct drm_device *dev) { - struct drm_i915_private *dev_priv = dev->dev_private; struct drm_crtc *c; struct drm_i915_gem_object *obj; int ret; @@ -15455,16 +15472,6 @@ void intel_modeset_gem_init(struct drm_device *dev) intel_init_gt_powersave(dev); mutex_unlock(&dev->struct_mutex); - /* - * There may be no VBT; and if the BIOS enabled SSC we can - * just keep using it to avoid unnecessary flicker. Whereas if the - * BIOS isn't using it, don't assume it will work even if the VBT - * indicates as much. - */ - if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) - dev_priv->vbt.lvds_use_ssc = !!(I915_READ(PCH_DREF_CONTROL) & - DREF_SSC1_ENABLE); - intel_modeset_init_hw(dev); intel_setup_overlay(dev);