From patchwork Wed Jul 1 14:54:06 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ander Conselvan de Oliveira X-Patchwork-Id: 6703891 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 3893BC05AC for ; Wed, 1 Jul 2015 14:54:10 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 524892064C for ; Wed, 1 Jul 2015 14:54:09 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 58A2320648 for ; Wed, 1 Jul 2015 14:54:08 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 8A56B72128; Wed, 1 Jul 2015 07:54:07 -0700 (PDT) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-pd0-f176.google.com (mail-pd0-f176.google.com [209.85.192.176]) by gabe.freedesktop.org (Postfix) with ESMTPS id 0B6227201F; Wed, 1 Jul 2015 07:54:06 -0700 (PDT) Received: by pdbci14 with SMTP id ci14so27042390pdb.2; Wed, 01 Jul 2015 07:54:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=message-id:subject:from:to:cc:date:in-reply-to:references :content-type:mime-version:content-transfer-encoding; bh=36hvE99S35noE5/4Z0N23YQBUcCDsM8ivuN73D+Jl4o=; b=eGX3RE3md3l/HosZ5QsKsgnFAqoIQ6zODrRc4kYTdbkua1OoDjApehOxTulPaTqBDc OOJAy4td1eAR5koTGj8sqJ9qgQIM7N2myjTerXY8GxU0yZcSpp61BQ1ArF3zVV5cfdRQ DKHYRU0kvJZJheVO6vlCHQiGFx2qdUmZOxgFv71HvR74DEjRfQBzJVfPoNPWc/CZ8ACc qlijQgaXfhK7N4bX0I4HcSs4imBLE8VolRqIIqOPiN7HGas2BGscU3MeVpET6y5Zci6Q 1y09zr+A/+vmnGroOyzollZSYnAoTVN2DaW0fGfkfRCokD4xZjpxVdhWm45kWOn2WQ/L 0PIg== X-Received: by 10.66.62.229 with SMTP id b5mr18581446pas.81.1435762445324; Wed, 01 Jul 2015 07:54:05 -0700 (PDT) Received: from aconselv-mobl3.fi.intel.com (jfdmzpr01-ext.jf.intel.com. [134.134.139.70]) by mx.google.com with ESMTPSA id gi6sm2562198pbd.69.2015.07.01.07.54.01 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 01 Jul 2015 07:54:04 -0700 (PDT) Message-ID: <1435762446.2645.11.camel@gmail.com> Subject: Re: [Intel-gfx] [PATCH] drm/i915: Clear pipe's pll hw state in hsw_dp_set_ddi_pll_sel() From: Ander Conselvan De Oliveira To: Jani Nikula Date: Wed, 01 Jul 2015 17:54:06 +0300 In-Reply-To: <87pp4d2ogi.fsf@intel.com> References: <878ub1wdur.fsf@intel.com> <1435669838-24747-1-git-send-email-ander.conselvan.de.oliveira@intel.com> <876165wbol.fsf@intel.com> <20150630142256.GZ30960@phenom.ffwll.local> <87pp4d2ogi.fsf@intel.com> X-Mailer: Evolution 3.12.10 (3.12.10-1.fc21) Mime-Version: 1.0 Cc: intel-gfx , Linux Kernel Mailing List , DRI mailing list , Daniel Vetter , Linus Torvalds X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Spam-Status: No, score=-4.7 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, T_DKIM_INVALID, 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 On Tue, 2015-06-30 at 18:41 +0300, Jani Nikula wrote: > On Tue, 30 Jun 2015, Daniel Vetter wrote: > > On Tue, Jun 30, 2015 at 04:47:06PM +0300, Jani Nikula wrote: > >> On Tue, 30 Jun 2015, Ander Conselvan de Oliveira wrote: > >> > Similarly to what is done for SKL, clear the dpll_hw_state of the pipe > >> > config in hsw_dp_set_ddi_pll_sel(), since it main contain stale values. > >> > That can happen if a crtc that was previously driving an HDMI connector > >> > switches to a DP connector. In that case, the wrpll field was left with > >> > its old value, leading to warnings like the one below: > >> > > >> > [drm:check_crtc_state [i915]] *ERROR* mismatch in dpll_hw_state.wrpll (expected 0xb035061f, found 0x00000000) > >> > ------------[ cut here ]------------ > >> > WARNING: CPU: 1 PID: 767 at drivers/gpu/drm/i915/intel_display.c:12324 check_crtc_state+0x975/0x10b0 [i915]() > >> > pipe state doesn't match! > >> > > >> > This regression was indroduced in > >> > > >> > commit dd3cd74acf12723045a64f1f2c6298ac7b34a5d5 > >> > Author: Ander Conselvan de Oliveira > >> > Date: Fri May 15 13:34:29 2015 +0300 > >> > > >> > drm/i915: Don't overwrite (e)DP PLL selection on SKL > >> > > >> > Signed-off-by: Ander Conselvan de Oliveira > >> > >> Reported-by: Linus Torvalds > >> Tested-by: Jani Nikula > > > > Yeah makes sense as a fix for 4.2. But for 4.3 I wonder whether the > > original commit that started this chain needs to be changed a bit: > > > > commit 4978cc93d9ac240b435ce60431aef24239b4c270 > > Author: Ander Conselvan de Oliveira > > Date: Tue Apr 21 17:13:21 2015 +0300 > > > > drm/i915: Preserve shared DPLL information in new pipe_config > > > > All the trouble this caused is because it not only preserves the sharing > > config (in crtc_state->shared_dpll) but also the ->dpll_hw_state. And I > > think with Maarten's latest code (for 4.3) we'd just do an unconditional > > compute_config (need it for fast pfit updates and fastboot), which means > > the bogus values in ->dpll_hw_state aren't a problem any more since we'll > > overwrite them again. And then we could remove that sprinkle of memsets we > > have all over, which would be good (since the current approach is > > obviously a bit fragile). Anyway: > > > > Reviewed-by: Daniel Vetter > > Pushed to drm-intel-next-fixes, thanks for the patch and review. One > down, another one left to fix. I made some progress on the second issue, but I'm afraid Jani might have a found a third bug. The warning he gets happens because we try to wait for vblanks while updating the primary plane during the modeset. At that point, the crtc is off. The problem is in intel_check_primary_plane(), which is called from drm_atomic_helper_check_planes(). That function makes decisions about waiting for a vblank based on intel_crtc->active. Since the check is called before we disable the crtcs, active might be true, even though the plane update is done with crtcs disable. The patch below makes the warning go away, but I still need to figure out how to set crtc_state->planes_changed properly if we are going down that route. The backtrace on Linus' machine is different, though. It comes from the call to intel_crtc_disable_planes() in __intel_set_mode(). That would indicate we have a crtc with crtc->state->enable == true but that is actually inactive. I'm still not sure how we can get in that state. Ander diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index dcb1d25..f14727c 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12480,10 +12480,6 @@ intel_modeset_compute_config(struct drm_crtc *crtc, intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,"[modeset]"); - ret = drm_atomic_helper_check_planes(state->dev, state); - if (ret) - return ERR_PTR(ret); - return pipe_config; }