From patchwork Wed Jul 11 14:27:50 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 1183441 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by patchwork1.kernel.org (Postfix) with ESMTP id 4A4A03FC8E for ; Wed, 11 Jul 2012 15:41:56 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 1B1FAA0E89 for ; Wed, 11 Jul 2012 08:41:56 -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 F07A39E8A8 for ; Wed, 11 Jul 2012 08:35:15 -0700 (PDT) Received: by mail-wi0-f177.google.com with SMTP id hm11so928073wib.12 for ; Wed, 11 Jul 2012 08:35:15 -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=GCRT9snYUPfydtmJ7WJlf5JnE1RPe4y6kYkMBCU0mBI=; b=JQFpZLlt5ylgDuiqfUkFr8aTc1ekMB9yzidlvZIHuqSSerqqwlG8WpwKNk3wZH4D4n 2fuVzWwxwx2NzszzvNJzEhVoUoFfASQdWz6/4WFMiIvf3ilZMNer3wygipKiaIX71kEy Qq1ch0vWPck1c/GDWob30keSWc4xUGLQh8ZN8= 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=GCRT9snYUPfydtmJ7WJlf5JnE1RPe4y6kYkMBCU0mBI=; b=S/p4YvhJ0ZpCvpYbPaj4J6Qb1o5jdklpMyzWReCz5KNNDMqeg+BQ6CKTScmunUau0z Iaai4GnuYjNwubozUnArgRibN2vPyZs4UaSjoCAvWY3giri9j8ms0syom9+PFHZmq1qx hOadqCC2FdZ/bXtfOMABVpz+btqYcguTjdwn1i8OzCdZNNjooxYBD3JAsTX61YGaQr/V ZLamhV9UZpkK++8vfK+Dl4/LTTCDl3lC7I4TJk/YVsatzaEGBvPEIANo59zA6QSewzFY jZTho2PwYBqTSiZQmbTOalFSYhd7ZgYT80cx8IYDXMOkNRaA0ptc9V+NnGDLZYlvxpNF TdEA== Received: by 10.216.220.89 with SMTP id n67mr2260612wep.73.1342020915690; Wed, 11 Jul 2012 08:35:15 -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 bc2sm5777080wib.0.2012.07.11.08.35.14 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 11 Jul 2012 08:35:15 -0700 (PDT) From: Daniel Vetter To: Intel Graphics Development Date: Wed, 11 Jul 2012 16:27:50 +0200 Message-Id: <1342016944-23395-8-git-send-email-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 1.7.7.6 In-Reply-To: <1342016944-23395-1-git-send-email-daniel.vetter@ffwll.ch> References: <1342016944-23395-1-git-send-email-daniel.vetter@ffwll.ch> X-Gm-Message-State: ALoCoQmX8GObpVF+59URzS++eM/H1HUlZzEss9L3NE/kQAU0kyHohm1R3EOLyIffOYR0RQFHZFGz Cc: Daniel Vetter Subject: [Intel-gfx] [PATCH 07/81] drm/i915/hdmi: convert to encoder->disable/enable 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 I've picked hdmi as the first encoder to convert because it's rather simple: - no cloning possible - no differences between prepare/commit and dpms off/on switching. A few changes are required to do so: - Split up the dpms code into an enable/disable function and wire it up with the intel encoder. - Noop out the existing encoder prepare/commit functions used by the crtc helper - our crtc enable/disable code now calls back into the encoder enable/disable code at the right spot. - Create new helper functions to handle dpms changes. - Add intel_encoder->connectors_active to better track dpms state. Atm this is unused, but it will be useful to correctly disable the entire display pipe for cloned configurations. Also note that for now this is only useful in the dpms code - thanks to the crtc helper's dpms confusion across a modeset operation we can't (yet) rely on this having a sensible value in all circumstances. - Rip out the encoder helper dpms callback, if this is still getting called somewhere we have a bug. The slight issue with that is that the crtc helper abuses dpms off to disable unused functions. Hence we also need to implement a default encoder disable function to do just that with the new encoder->disable callback. - Note that we drop the cpt modeset verification in the commit callback, too. The right place to do this would be in the crtc's enable function, _after_ all the encoders are set up. But because not all encoders are converted yet, we can't do that. Hence disable this check temporarily as a minor concession to bisectability. v2: Squash the dpms mode to only the supported values - connector->dpms is for internal tracking only, we can hence avoid needless state-changes a bit whithout causing harm. Signed-Off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_ddi.c | 28 +++++--- drivers/gpu/drm/i915/intel_display.c | 49 +++++++++++++ drivers/gpu/drm/i915/intel_drv.h | 8 ++- drivers/gpu/drm/i915/intel_hdmi.c | 126 +++++++++++++++++++++++----------- 4 files changed, 160 insertions(+), 51 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 933c748..fd60f48 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -738,21 +738,16 @@ void intel_ddi_mode_set(struct drm_encoder *encoder, intel_hdmi->set_infoframes(encoder, adjusted_mode); } -void intel_ddi_dpms(struct drm_encoder *encoder, int mode) +void intel_enable_ddi(struct intel_encoder *encoder) { - struct drm_device *dev = encoder->dev; + struct drm_device *dev = encoder->base.dev; struct drm_i915_private *dev_priv = dev->dev_private; - struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder); + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base); int port = intel_hdmi->ddi_port; u32 temp; temp = I915_READ(DDI_BUF_CTL(port)); - - if (mode != DRM_MODE_DPMS_ON) { - temp &= ~DDI_BUF_CTL_ENABLE; - } else { - temp |= DDI_BUF_CTL_ENABLE; - } + temp |= DDI_BUF_CTL_ENABLE; /* Enable DDI_BUF_CTL. In HDMI/DVI mode, the port width, * and swing/emphasis values are ignored so nothing special needs @@ -761,3 +756,18 @@ void intel_ddi_dpms(struct drm_encoder *encoder, int mode) I915_WRITE(DDI_BUF_CTL(port), temp); } + +void intel_disable_ddi(struct intel_encoder *encoder) +{ + struct drm_device *dev = encoder->base.dev; + struct drm_i915_private *dev_priv = dev->dev_private; + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base); + int port = intel_hdmi->ddi_port; + u32 temp; + + temp = I915_READ(DDI_BUF_CTL(port)); + temp &= ~DDI_BUF_CTL_ENABLE; + + I915_WRITE(DDI_BUF_CTL(port), + temp); +} diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 1ea3776..1a201b2 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3542,6 +3542,17 @@ void intel_encoder_commit(struct drm_encoder *encoder) intel_cpt_verify_modeset(dev, intel_crtc->pipe); } +void intel_encoder_noop(struct drm_encoder *encoder) +{ +} + +void intel_encoder_disable(struct drm_encoder *encoder) +{ + struct intel_encoder *intel_encoder = to_intel_encoder(encoder); + + intel_encoder->disable(intel_encoder); +} + void intel_encoder_destroy(struct drm_encoder *encoder) { struct intel_encoder *intel_encoder = to_intel_encoder(encoder); @@ -3550,6 +3561,44 @@ void intel_encoder_destroy(struct drm_encoder *encoder) kfree(intel_encoder); } +/* Simple dpms helper for encodres with just one connector, no cloning and only + * one kind of off state. It clamps all !ON modes to fully OFF and changes the + * state of the entire output pipe. */ +void intel_encoder_dpms(struct intel_encoder *encoder, int mode) +{ + if (mode == DRM_MODE_DPMS_ON) { + encoder->connectors_active = true; + + intel_crtc_dpms(encoder->base.crtc, DRM_MODE_DPMS_ON); + } else { + encoder->connectors_active = false; + + intel_crtc_dpms(encoder->base.crtc, DRM_MODE_DPMS_OFF); + } +} + +/* Even simpler default implementation, if there's really no special case to + * consider. */ +void intel_connector_dpms(struct drm_connector *connector, int mode) +{ + struct intel_encoder *encoder = intel_attached_encoder(connector); + + /* All the simple cases only support two dpms states. */ + if (mode != DRM_MODE_DPMS_ON) + mode = DRM_MODE_DPMS_OFF; + + if (mode == connector->dpms) + return; + + connector->dpms = mode; + + /* Only need to change hw state when actually enabled */ + if (encoder->base.crtc) + intel_encoder_dpms(encoder, mode); + else + encoder->connectors_active = false; +} + static bool intel_crtc_mode_fixup(struct drm_crtc *crtc, struct drm_display_mode *mode, struct drm_display_mode *adjusted_mode) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 4ac2373..16680e5 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -152,6 +152,7 @@ struct intel_encoder { struct drm_encoder base; int type; bool needs_tv_clock; + bool connectors_active; void (*hot_plug)(struct intel_encoder *); void (*enable)(struct intel_encoder *); void (*disable)(struct intel_encoder *); @@ -395,7 +396,11 @@ extern enum drm_connector_status intel_panel_detect(struct drm_device *dev); extern void intel_crtc_load_lut(struct drm_crtc *crtc); extern void intel_encoder_prepare(struct drm_encoder *encoder); extern void intel_encoder_commit(struct drm_encoder *encoder); +extern void intel_encoder_noop(struct drm_encoder *encoder); +extern void intel_encoder_disable(struct drm_encoder *encoder); extern void intel_encoder_destroy(struct drm_encoder *encoder); +extern void intel_encoder_dpms(struct intel_encoder *encoder, int mode); +extern void intel_connector_dpms(struct drm_connector *, int mode); static inline struct intel_encoder *intel_attached_encoder(struct drm_connector *connector) { @@ -504,7 +509,8 @@ extern void intel_disable_gt_powersave(struct drm_device *dev); extern void gen6_gt_check_fifodbg(struct drm_i915_private *dev_priv); extern void ironlake_teardown_rc6(struct drm_device *dev); -extern void intel_ddi_dpms(struct drm_encoder *encoder, int mode); +extern void intel_enable_ddi(struct intel_encoder *encoder); +extern void intel_disable_ddi(struct intel_encoder *encoder); extern void intel_ddi_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode, struct drm_display_mode *adjusted_mode); diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index 5b2c88c..188399f 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -601,11 +601,11 @@ static void intel_hdmi_mode_set(struct drm_encoder *encoder, intel_hdmi->set_infoframes(encoder, adjusted_mode); } -static void intel_hdmi_dpms(struct drm_encoder *encoder, int mode) +static void intel_enable_hdmi(struct intel_encoder *encoder) { - struct drm_device *dev = encoder->dev; + struct drm_device *dev = encoder->base.dev; struct drm_i915_private *dev_priv = dev->dev_private; - struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder); + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base); u32 temp; u32 enable_bits = SDVO_ENABLE; @@ -617,31 +617,12 @@ static void intel_hdmi_dpms(struct drm_encoder *encoder, int mode) /* 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->crtc; + struct drm_crtc *crtc = encoder->base.crtc; int pipe = crtc ? to_intel_crtc(crtc)->pipe : -1; - if (mode != DRM_MODE_DPMS_ON) { - if (temp & SDVO_PIPE_B_SELECT) { - temp &= ~SDVO_PIPE_B_SELECT; - I915_WRITE(intel_hdmi->sdvox_reg, temp); - POSTING_READ(intel_hdmi->sdvox_reg); - - /* Again we need to write this twice. */ - I915_WRITE(intel_hdmi->sdvox_reg, temp); - POSTING_READ(intel_hdmi->sdvox_reg); - - /* Transcoder selection bits only update - * effectively on vblank. */ - if (crtc) - intel_wait_for_vblank(dev, pipe); - else - msleep(50); - } - } else { - /* Restore the transcoder select bit. */ - if (pipe == PIPE_B) - enable_bits |= SDVO_PIPE_B_SELECT; - } + /* Restore the transcoder select bit. */ + if (pipe == PIPE_B) + enable_bits |= SDVO_PIPE_B_SELECT; } /* HW workaround, need to toggle enable bit off and on for 12bpc, but @@ -652,12 +633,67 @@ static void intel_hdmi_dpms(struct drm_encoder *encoder, int mode) POSTING_READ(intel_hdmi->sdvox_reg); } - if (mode != DRM_MODE_DPMS_ON) { - temp &= ~enable_bits; - } else { - temp |= enable_bits; + temp |= enable_bits; + + I915_WRITE(intel_hdmi->sdvox_reg, temp); + POSTING_READ(intel_hdmi->sdvox_reg); + + /* HW workaround, need to write this twice for issue that may result + * in first write getting masked. + */ + if (HAS_PCH_SPLIT(dev)) { + I915_WRITE(intel_hdmi->sdvox_reg, temp); + POSTING_READ(intel_hdmi->sdvox_reg); + } +} + +static void intel_disable_hdmi(struct intel_encoder *encoder) +{ + struct drm_device *dev = encoder->base.dev; + struct drm_i915_private *dev_priv = dev->dev_private; + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base); + u32 temp; + u32 enable_bits = SDVO_ENABLE; + + if (intel_hdmi->has_audio) + enable_bits |= SDVO_AUDIO_ENABLE; + + temp = I915_READ(intel_hdmi->sdvox_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->sdvox_reg, temp); + POSTING_READ(intel_hdmi->sdvox_reg); + + /* Again we need to write this twice. */ + I915_WRITE(intel_hdmi->sdvox_reg, temp); + POSTING_READ(intel_hdmi->sdvox_reg); + + /* Transcoder selection bits only update + * effectively on vblank. */ + if (crtc) + intel_wait_for_vblank(dev, pipe); + else + msleep(50); + } } + /* HW workaround, need to toggle enable bit off and on for 12bpc, but + * we do this anyway which shows more stable in testing. + */ + if (HAS_PCH_SPLIT(dev)) { + I915_WRITE(intel_hdmi->sdvox_reg, temp & ~SDVO_ENABLE); + POSTING_READ(intel_hdmi->sdvox_reg); + } + + temp &= ~enable_bits; + I915_WRITE(intel_hdmi->sdvox_reg, temp); POSTING_READ(intel_hdmi->sdvox_reg); @@ -849,23 +885,23 @@ static void intel_hdmi_destroy(struct drm_connector *connector) } static const struct drm_encoder_helper_funcs intel_hdmi_helper_funcs_hsw = { - .dpms = intel_ddi_dpms, .mode_fixup = intel_hdmi_mode_fixup, - .prepare = intel_encoder_prepare, + .prepare = intel_encoder_noop, .mode_set = intel_ddi_mode_set, - .commit = intel_encoder_commit, + .commit = intel_encoder_noop, + .disable = intel_encoder_disable, }; static const struct drm_encoder_helper_funcs intel_hdmi_helper_funcs = { - .dpms = intel_hdmi_dpms, .mode_fixup = intel_hdmi_mode_fixup, - .prepare = intel_encoder_prepare, + .prepare = intel_encoder_noop, .mode_set = intel_hdmi_mode_set, - .commit = intel_encoder_commit, + .commit = intel_encoder_noop, + .disable = intel_encoder_disable, }; static const struct drm_connector_funcs intel_hdmi_connector_funcs = { - .dpms = drm_helper_connector_dpms, + .dpms = intel_connector_dpms, .detect = intel_hdmi_detect, .fill_modes = drm_helper_probe_single_connector_modes, .set_property = intel_hdmi_set_property, @@ -987,10 +1023,18 @@ void intel_hdmi_init(struct drm_device *dev, int sdvox_reg) intel_hdmi->set_infoframes = cpt_set_infoframes; } - if (IS_HASWELL(dev)) - drm_encoder_helper_add(&intel_encoder->base, &intel_hdmi_helper_funcs_hsw); - else - drm_encoder_helper_add(&intel_encoder->base, &intel_hdmi_helper_funcs); + if (IS_HASWELL(dev)) { + intel_encoder->enable = intel_enable_ddi; + intel_encoder->disable = intel_disable_ddi; + drm_encoder_helper_add(&intel_encoder->base, + &intel_hdmi_helper_funcs_hsw); + } else { + intel_encoder->enable = intel_enable_hdmi; + intel_encoder->disable = intel_disable_hdmi; + drm_encoder_helper_add(&intel_encoder->base, + &intel_hdmi_helper_funcs); + } + intel_hdmi_add_properties(intel_hdmi, connector);