From patchwork Fri Aug 31 18:12:10 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 1394291 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 6F91EDFFCF for ; Fri, 31 Aug 2012 19:20:24 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id DF2B19E94D for ; Fri, 31 Aug 2012 12:20:23 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mail-wi0-f169.google.com (mail-wi0-f169.google.com [209.85.212.169]) by gabe.freedesktop.org (Postfix) with ESMTP id 32F159E9FF for ; Fri, 31 Aug 2012 12:19:23 -0700 (PDT) Received: by wibhm2 with SMTP id hm2so1524499wib.0 for ; Fri, 31 Aug 2012 12:19:22 -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=j1E51UYBAbdTE0Zimpwf8OnkgVRd5uBZeSKCOlLC9dI=; b=k8wA9SR2NvVmq4wgbiviVYdVmRa6qgN1sHAOOv57H5Y1/tBvt8MtHlJjbDoAwnNuJG 5xNdATgoJqZQX25dwrh6zvlu2gH5O7z0wUHuIYlh/hBqjYP81S+PiLQzscQk9PFRcVTt 5BZTArxH1H6aypptGNsQXsYuEAMCJ/1PV3TDE= 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=j1E51UYBAbdTE0Zimpwf8OnkgVRd5uBZeSKCOlLC9dI=; b=WP6fMV6HcvnPH8KcoH6flTA18we9RJ0Wrbh/VEYNfnSwA5uQRlT2Nhc95/Z4VaK/1r JVUomB6yyrZlgxdEse7WvAYLgpAXOcg1D0Eskuk9v9M2hgPJ6k1gGI31eKDdM8PGWjJg hJENnjd4rwVSQrSmKONruCFx5AJBeitTlYLKUUFLCUmlgqQWsAwvcckuSHdEQM7Te0Ze 50vdGpU7moDEhuIVDffxcwj/mbppN3uCCzJExIg/JXl4HxfuyHqEAupv56j6NZOpLkjM HfWkNg0USBont3i+O2dv4CUpTgRJmTQAgReL3g9/LCnL7upSgYpo4Tg5g+tU+gjYifId YZpA== Received: by 10.180.82.230 with SMTP id l6mr6524259wiy.21.1346440762049; Fri, 31 Aug 2012 12:19:22 -0700 (PDT) Received: from wespe.ffwll.local (178-83-130-250.dynamic.hispeed.ch. [178.83.130.250]) by mx.google.com with ESMTPS id ef5sm3606842wib.3.2012.08.31.12.19.20 (version=TLSv1/SSLv3 cipher=OTHER); Fri, 31 Aug 2012 12:19:21 -0700 (PDT) From: Daniel Vetter To: Intel Graphics Development Date: Fri, 31 Aug 2012 20:12:10 +0200 Message-Id: <1346436730-22889-1-git-send-email-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 1.7.11.2 In-Reply-To: <1345403595-9678-57-git-send-email-daniel.vetter@ffwll.ch> References: <1345403595-9678-57-git-send-email-daniel.vetter@ffwll.ch> X-Gm-Message-State: ALoCoQmwfplrPLHUymQzt1iQq/ltm2aaGhvyyvouHOQGXlR86VZ0LZh9leCJNCYndiMuWv2w7nhl Cc: Daniel Vetter Subject: [Intel-gfx] [PATCH] drm/i915: push commit_output_state past the crtc/encoder preparing 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 With this change we can (finally!) rip out a few of the temporary hacks and clean up a few other things: - Kill intel_crtc_prepare_encoders, now unused. - Kill the hacks in the crtc_disable/enable functions to always call the encoder callbacks, we now always call the crtc functions with the right encoder -> crtc links. - Also push down the crtc->enable, encoder and connector dpms state updates. Unfortunately we can't add a WARN in the crtc_disable callbacks to ensure that the crtc is always still enabled when disabling an output pipe - the crtc sanitizer of the hw readout path can hit this when it needs to disable an active pipe without any enabled outputs. - Only call crtc->disable if the pipe is already enabled - again avoids running afoul of the new WARN. v2: Copy&paste our own version of crtc_in_use, too. v3: We need to update the dpms an encoder->connectors_active states, too. v4: I've forgotten to kill the unconditional encoder->disable calls in the crtc_disable functions. v5: Rip out leftover debug printk. v6: Properly clear intel_encoder->connectors_active. This wasn't properly cleared when disabling an encoder because it was no longer on the new connector list, but the crtc was still enabled (i.e. switching the encoder of an active crtc). Reported by Jani Nikula. v7: Don't clobber the encoder->connectors_active state of untouched encoders. Since X likes to first disable all outputs with dpms off before setting a new framebuffer, this hit a few warnings. Reported by Paulo Zanoni. Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_display.c | 108 +++++++++++++++++++++++------------ 1 file changed, 70 insertions(+), 38 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index dd2098a..991f8e8 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3221,10 +3221,8 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc) WARN_ON(!crtc->enabled); - /* XXX: For compatability with the crtc helper code, call the encoder's - * enable function unconditionally for now. */ if (intel_crtc->active) - goto encoders; + return; intel_crtc->active = true; intel_update_watermarks(dev); @@ -3272,7 +3270,6 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc) intel_crtc_update_cursor(crtc, true); -encoders: for_each_encoder_on_crtc(dev, crtc, encoder) encoder->enable(encoder); @@ -3290,14 +3287,13 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc) int plane = intel_crtc->plane; u32 reg, temp; - /* XXX: For compatability with the crtc helper code, call the encoder's - * disable function unconditionally for now. */ - for_each_encoder_on_crtc(dev, crtc, encoder) - encoder->disable(encoder); if (!intel_crtc->active) return; + for_each_encoder_on_crtc(dev, crtc, encoder) + encoder->disable(encoder); + intel_crtc_wait_for_pending_flips(crtc); drm_vblank_off(dev, pipe); intel_crtc_update_cursor(crtc, false); @@ -3399,10 +3395,8 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc) WARN_ON(!crtc->enabled); - /* XXX: For compatability with the crtc helper code, call the encoder's - * enable function unconditionally for now. */ if (intel_crtc->active) - goto encoders; + return; intel_crtc->active = true; intel_update_watermarks(dev); @@ -3418,7 +3412,6 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc) intel_crtc_dpms_overlay(intel_crtc, true); intel_crtc_update_cursor(crtc, true); -encoders: for_each_encoder_on_crtc(dev, crtc, encoder) encoder->enable(encoder); } @@ -3432,14 +3425,13 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc) int pipe = intel_crtc->pipe; int plane = intel_crtc->plane; - /* XXX: For compatability with the crtc helper code, call the encoder's - * disable function unconditionally for now. */ - for_each_encoder_on_crtc(dev, crtc, encoder) - encoder->disable(encoder); if (!intel_crtc->active) return; + for_each_encoder_on_crtc(dev, crtc, encoder) + encoder->disable(encoder); + /* Give the overlay scaler a chance to disable if it's on this pipe */ intel_crtc_wait_for_pending_flips(crtc); drm_vblank_off(dev, pipe); @@ -6631,18 +6623,6 @@ static bool intel_encoder_crtc_ok(struct drm_encoder *encoder, return false; } -static void -intel_crtc_prepare_encoders(struct drm_device *dev) -{ - struct intel_encoder *encoder; - - list_for_each_entry(encoder, &dev->mode_config.encoder_list, base.head) { - /* Disable unused encoders */ - if (encoder->base.crtc == NULL) - encoder->disable(encoder); - } -} - /** * intel_modeset_update_staged_output_state * @@ -6822,6 +6802,60 @@ intel_modeset_affected_pipes(struct drm_crtc *crtc, unsigned *modeset_pipes, *prepare_pipes &= ~(*disable_pipes); } +static bool intel_crtc_in_use(struct drm_crtc *crtc) +{ + struct drm_encoder *encoder; + struct drm_device *dev = crtc->dev; + + list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) + if (encoder->crtc == crtc) + return true; + + return false; +} + +static void +intel_modeset_update_state(struct drm_device *dev, unsigned prepare_pipes) +{ + struct intel_encoder *intel_encoder; + struct intel_crtc *intel_crtc; + struct drm_connector *connector; + + list_for_each_entry(intel_encoder, &dev->mode_config.encoder_list, + base.head) { + if (!intel_encoder->base.crtc) + continue; + + intel_crtc = to_intel_crtc(intel_encoder->base.crtc); + + if (prepare_pipes & (1 << intel_crtc->pipe)) + intel_encoder->connectors_active = false; + } + + intel_modeset_commit_output_state(dev); + + /* Update computed state. */ + list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list, + base.head) { + intel_crtc->base.enabled = intel_crtc_in_use(&intel_crtc->base); + } + + list_for_each_entry(connector, &dev->mode_config.connector_list, head) { + if (!connector->encoder || !connector->encoder->crtc) + continue; + + intel_crtc = to_intel_crtc(connector->encoder->crtc); + + if (prepare_pipes & (1 << intel_crtc->pipe)) { + connector->dpms = DRM_MODE_DPMS_ON; + + intel_encoder = to_intel_encoder(connector->encoder); + intel_encoder->connectors_active = true; + } + } + +} + #define for_each_intel_crtc_masked(dev, mask, intel_crtc) \ list_for_each_entry((intel_crtc), \ &(dev)->mode_config.crtc_list, \ @@ -6850,12 +6884,6 @@ bool intel_set_mode(struct drm_crtc *crtc, for_each_intel_crtc_masked(dev, disable_pipes, intel_crtc) intel_crtc_disable(&intel_crtc->base); - intel_modeset_commit_output_state(dev); - - list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list, - base.head) - intel_crtc->base.enabled = drm_helper_crtc_in_use(crtc); - saved_hwmode = crtc->hwmode; saved_mode = crtc->mode; @@ -6870,12 +6898,12 @@ bool intel_set_mode(struct drm_crtc *crtc, if (IS_ERR(adjusted_mode)) { return false; } - - intel_crtc_prepare_encoders(dev); } - for_each_intel_crtc_masked(dev, prepare_pipes, intel_crtc) - dev_priv->display.crtc_disable(&intel_crtc->base); + for_each_intel_crtc_masked(dev, prepare_pipes, intel_crtc) { + if (intel_crtc->base.enabled) + dev_priv->display.crtc_disable(&intel_crtc->base); + } if (modeset_pipes) { crtc->mode = *mode; @@ -6883,6 +6911,10 @@ bool intel_set_mode(struct drm_crtc *crtc, crtc->y = y; } + /* Only after disabling all output pipelines that will be changed can we + * update the the output configuration. */ + intel_modeset_update_state(dev, prepare_pipes); + /* Set up the DPLL and any encoders state that needs to adjust or depend * on the DPLL. */