From patchwork Thu May 28 07:22:50 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Maarten Lankhorst X-Patchwork-Id: 6496431 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 33E8A9F38C for ; Thu, 28 May 2015 07:22:57 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 43FD52051C for ; Thu, 28 May 2015 07:22:56 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 38A29204A7 for ; Thu, 28 May 2015 07:22:55 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id C07C96E337; Thu, 28 May 2015 00:22:53 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id 44B416E1EC for ; Thu, 28 May 2015 00:22:52 -0700 (PDT) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga102.jf.intel.com with ESMTP; 28 May 2015 00:22:52 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.13,511,1427785200"; d="scan'208";a="716804855" Received: from lemogne-mobl.ger.corp.intel.com (HELO patser.lan) ([10.252.4.211]) by fmsmga001.fm.intel.com with ESMTP; 28 May 2015 00:22:52 -0700 Message-ID: <5566C24A.1020503@linux.intel.com> Date: Thu, 28 May 2015 09:22:50 +0200 From: Maarten Lankhorst User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: Matt Roper References: <1432129094-11765-1-git-send-email-maarten.lankhorst@linux.intel.com> <1432129094-11765-9-git-send-email-maarten.lankhorst@linux.intel.com> <20150528013726.GA15716@intel.com> In-Reply-To: <20150528013726.GA15716@intel.com> Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH v3 08/22] drm/i915: Zap call to drm_plane_helper_disable. 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: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable 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 Op 28-05-15 om 03:37 schreef Matt Roper: > On Wed, May 20, 2015 at 03:38:13PM +0200, Maarten Lankhorst wrote: >> The primary plane can still be configured when crtc is off, >> furthermore this is also a noop now that affected planes are >> added on modesets. >> >> Signed-off-by: Maarten Lankhorst > This was added in patch #2. Can we squash this change up, or was there > a reason we needed it for the intermediate patches? > > Actually, I'm not sure this is really quite a noop yet. > prepare_plane_fb is never called on the primary plane as far as I can > see, so I think our frontbuffer tracking and such might get confused. > In the regular plane update codepath, we have to handle this with a > special case by doing: > > intel_crtc->atomic.disabled_planes |= (1 << drm_plane_index(plane)); > > in the intel_plane_atomic_check function. But from what I can see, we > bypass that in this codepath. igt/kms_universal_plane shows that we do > run into frontbuffer tracking warnings if we don't put an equivalent > update to the 'disabled_planes' flag here. Well if this is the case we're really messing up on adding planes.. If we're fine with leaving the warning for now I'm fixing it up in series 3. Do the below patches fix it? commit 05fac5b2c29a85492c677e42ec68e94f9011daa2 Author: Maarten Lankhorst Date: Tue May 26 13:00:08 2015 +0200 drm/i915: update plane state during init Atomic planes updates rely on having a accurate plane_mask. Signed-off-by: Maarten Lankhorst diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 558fe51f151e..692cf9092f6a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2645,9 +2645,9 @@ valid_fb: dev_priv->preserve_bios_swizzle = true; primary->fb = fb; - primary->state->crtc = &intel_crtc->base; - primary->crtc = &intel_crtc->base; + primary->crtc = primary->state->crtc = &intel_crtc->base; update_state_fb(primary); + intel_crtc->base.state->plane_mask |= (1 << drm_plane_index(primary)); obj->frontbuffer_bits |= INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe); } @@ -14959,7 +14959,9 @@ void intel_modeset_gem_init(struct drm_device *dev) to_intel_crtc(c)->pipe); drm_framebuffer_unreference(c->primary->fb); c->primary->fb = NULL; + c->primary->crtc = c->primary->state->crtc = NULL; update_state_fb(c->primary); + c->state->plane_mask &= ~(1 << drm_plane_index(c->primary)); } } commit b2caffde98d9f7335e774d1859b881b0df8a109a Author: Maarten Lankhorst Date: Mon May 25 10:25:45 2015 +0200 drm/i915: Make sure all planes and connectors are added on modeset. Add missing calls to drm_atomic_add_affected_*. This is needed to convert to atomic planes. When converting to atomic all planes are needed on modeset. For good measure make sure all connectors are added too. Signed-off-by: Maarten Lankhorst diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 5c1e74d1fb1b..558fe51f151e 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5628,7 +5628,7 @@ static int valleyview_modeset_global_pipes(struct drm_atomic_state *state) struct drm_crtc *crtc; struct drm_crtc_state *crtc_state; int max_pixclk = intel_mode_max_pixclk(state->dev, state); - int cdclk, i; + int cdclk, ret = 0; if (max_pixclk < 0) return max_pixclk; @@ -5643,20 +5643,25 @@ static int valleyview_modeset_global_pipes(struct drm_atomic_state *state) /* add all active pipes to the state */ for_each_crtc(state->dev, crtc) { - if (!crtc->state->active) - continue; - crtc_state = drm_atomic_get_crtc_state(state, crtc); if (IS_ERR(crtc_state)) return PTR_ERR(crtc_state); - } - /* disable/enable all currently active pipes while we change cdclk */ - for_each_crtc_in_state(state, crtc, crtc_state, i) - if (crtc_state->active) - crtc_state->mode_changed = true; + if (!crtc_state->active || needs_modeset(crtc_state)) + continue; - return 0; + crtc_state->mode_changed = true; + + ret = drm_atomic_add_affected_connectors(state, crtc); + if (ret) + break; + + ret = drm_atomic_add_affected_planes(state, crtc); + if (ret) + break; + } + + return ret; } static void vlv_program_pfi_credits(struct drm_i915_private *dev_priv) @@ -11483,8 +11488,10 @@ encoder_retry: /* Check if we need to force a modeset */ if (pipe_config->has_audio != - to_intel_crtc_state(crtc->state)->has_audio) + to_intel_crtc_state(crtc->state)->has_audio) { pipe_config->base.mode_changed = true; + ret = drm_atomic_add_affected_planes(state, crtc); + } /* * Note we have an issue here with infoframes: current code @@ -11492,8 +11499,6 @@ encoder_retry: * requirements. So here we should be checking for any * required changes and forcing a mode set. */ - - return 0; fail: return ret; }