From patchwork Thu Nov 19 17:46:28 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mario Kleiner X-Patchwork-Id: 7729351 Return-Path: X-Original-To: patchwork-dri-devel@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 7CC419F39B for ; Mon, 30 Nov 2015 20:13:01 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 5A519204D8 for ; Mon, 30 Nov 2015 20:13:00 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 233DA2049D for ; Mon, 30 Nov 2015 20:12:59 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 60E586E659; Mon, 30 Nov 2015 12:12:58 -0800 (PST) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by gabe.freedesktop.org (Postfix) with ESMTP id 2BE7C6E659 for ; Mon, 30 Nov 2015 12:12:57 -0800 (PST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga103.jf.intel.com with ESMTP; 30 Nov 2015 12:12:57 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.20,365,1444719600"; d="scan'208";a="850784469" Received: from stinkbox.fi.intel.com (HELO stinkbox) ([10.237.72.174]) by fmsmga001.fm.intel.com with SMTP; 30 Nov 2015 12:12:54 -0800 Received: by stinkbox (sSMTP sendmail emulation); Mon, 30 Nov 2015 22:12:54 +0200 Resent-From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Resent-Date: Mon, 30 Nov 2015 22:12:54 +0200 Resent-Message-ID: <20151130201254.GV4437@intel.com> Resent-To: dri-devel@lists.freedesktop.org X-Original-To: ville.syrjala@linux.intel.com Delivered-To: ville.syrjala@linux.intel.com Received: from linux.intel.com [10.23.219.25] by localhost with IMAP (fetchmail-6.3.26) for (single-drop); Thu, 19 Nov 2015 19:47:42 +0200 (EET) Received: from orsmga003.jf.intel.com (orsmga003.jf.intel.com [10.7.209.27]) by linux.intel.com (Postfix) with ESMTP id 532E82E055B for ; Thu, 19 Nov 2015 09:45:30 -0800 (PST) Received: from fmsmga103.fm.intel.com ([10.1.193.90]) by orsmga003-1.jf.intel.com with ESMTP; 19 Nov 2015 09:46:35 -0800 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: A0BMAAACCk5WlTVSfUpeGQEBAQEHCAEBAQGETa1RhwSKIQ6BZYYPgVA4FAEBAQEBAQERAQEBAQcLCwkfMIQ3ARURBBkBGxIKAQEDEhAWFgsCCwMCAQIBEREBBQEiAQwIAQEeh3YBAxIFohKBMT4xi0iBaoJ5hhoKGScNVoQSAQEBAQEBAQMBAQEBAQEUAwEFDo0EAYJ9gzeBRAWWTH+BWYFhiHWJDQwEkV02gRcfAYJHDQ0JB4FXcYUgAQEB X-IPAS-Result: A0BMAAACCk5WlTVSfUpeGQEBAQEHCAEBAQGETa1RhwSKIQ6BZYYPgVA4FAEBAQEBAQERAQEBAQcLCwkfMIQ3ARURBBkBGxIKAQEDEhAWFgsCCwMCAQIBEREBBQEiAQwIAQEeh3YBAxIFohKBMT4xi0iBaoJ5hhoKGScNVoQSAQEBAQEBAQMBAQEBAQEUAwEFDo0EAYJ9gzeBRAWWTH+BWYFhiHWJDQwEkV02gRcfAYJHDQ0JB4FXcYUgAQEB X-IronPort-AV: E=Sophos;i="5.20,318,1444719600"; d="scan'208";a="95361253" Received: from mail-wm0-f53.google.com ([74.125.82.53]) by mtab.intel.com with ESMTP; 19 Nov 2015 09:46:34 -0800 Received: by wmvv187 with SMTP id v187so37581408wmv.1 for ; Thu, 19 Nov 2015 09:46:34 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=to:from:subject:cc:message-id:date:user-agent:mime-version :content-type; bh=/HNBrSTRdK1fwz5kMyFe4n0iVqG2Bb9O7ustrim7/Og=; b=tKjE2Y1Cq0uqZE9VefM2a+X2nnx1SdS5UZsSW5JusakcSsEocpWNao6vVV2zWHMiCa kZVhHD8d1ofo5HNtg6OPNd4mS/ApvB3HG+qgCC8TpOFBQc+7mGL+kJOio5kYyXkk2A7R cmjjSC5mW98IjQvph/AG3tqcmVcIfy1g6Iz1nnA5REEENyRRLeraXGmi81kS6wxGlQky EOAIASx8/DLvlBrxS5D+im8jSD3LUAyKtzGQ3WMeTDORMsVSFE83Fny6dWthUaLNtopi nUs6+Bek9UuiH0fi19fpZXPqKwBU2j8zU3NZ3BTwtGhn9H7O0PH1qP4bYcS2pVvGhexp bk3A== X-Received: by 10.28.174.21 with SMTP id x21mr4943989wme.22.1447955193903; Thu, 19 Nov 2015 09:46:33 -0800 (PST) Received: from [172.25.194.222] (cin-11.medizin.uni-tuebingen.de. [134.2.118.242]) by smtp.gmail.com with ESMTPSA id q6sm8881380wjx.28.2015.11.19.09.46.32 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 19 Nov 2015 09:46:32 -0800 (PST) To: Alex Deucher , =?UTF-8?Q?Michel_D=c3=a4nzer?= From: Mario Kleiner Subject: Funky new vblank counter regressions in Linux 4.4-rc1 Message-ID: <564E0AF4.3050406@gmail.com> Date: Thu, 19 Nov 2015 18:46:28 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 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.1 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED,RCVD_IN_DNSWL_MED,T_DKIM_INVALID,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 Hi Alex and Michel and Ville, it's "fix vblank stuff" time again ;-) Ville's changes to the DRM's drm_handle_vblank() / drm_update_vblank_count() code in Linux 4.4 not only made that code more elegant, but also removed the robustness against the vblank irq quirks in AMD hw and similar hardware. So now i get tons of off-by-one errors and "[ 432.345] (WW) RADEON(1): radeon_dri2_flip_event_handler: Pageflip completion event has impossible msc 24803 < target_msc 24804" XOrg messages from that kernel. One of the reasons for trouble is that AMD hw quirk where the hw fires an extra vblank irq shortly after vblank irq's get enabled, not synchronized to vblank, but typically in the middle of active scanout, so we get a redundant call to drm_handle_vblank in the middle of scanout. To fix that i have a minor patch to make drm_update_vblank_count() again robust against such redundant calls, which i will send out later to the mailing list. Diff attached for reference. The second quirk of AMD hw is that the vblank interrupt fires a few scanlines before start of vblank, so drm_handle_vblank -> drm_update_vblank_count() -> dev->driver->get_vblank_counter() gets called before the start of the vblank for which the new vblank count should be queried. The third problem is that the DRM vblank handling always had the assumption that hardware vblank counters would essentially increment at leading edge of vblank - basically in sync with the firing of the vblank irq, so that a hw counter readout from within the vblank irq handler would always deliver the new incremented value. If this assumption is violated then the counting by use of the hw counter gets unreliable, because depending on random small delays in irq handling the code may end up sampling the hw counter pre- or post-increment, leading to inconsistent updating and funky bugs. It just so happens that AMD hardware doesn't increment the hw counter at leading edge of vblank, so stuff falls apart. So to fix those two problems i'm tinkering with cooking the hw vblank counter value returned by radeon_get_vblank_counter_kms() to make it appear as if the counter incremented at leading edge of vblank in sync with vblank irq. It almost sort of works on the rs600 code path, but i need a bit of info from you: 1. There's this register from the old specs for m76.pdf, which is not part of the current register defines for radeon-kms: "D1CRTC_STATUS_VF_COUNT - RW - 32 bits - [GpuF0MMReg:0x60A8]" It contains the lower 16 bits of framecounter and the 13 bits of vertical scanout position. It seems to give the same readings as the 24 bit R_0060A4_D1CRTC_STATUS_FRAME_COUNT we use for the hw counter. This would come handy. Does Evergreen and later have a same/similar register and where is it? 2. The hw framecounter seems to increment when the vertical scanout position wraps back from (VTOTAL - 1) to 0, at least on the one DCE-3 gpu i tested so far. Is this so on all asics? And is the hw counter increment happening exactly at the moment that vertical scanout position jumps back to zero, ie. both events are driven by the same signal? Or is the framecounter increment just happening somewhere inside either scanline VTOTAL-1 or scanline 0? If we can fix this and get it into rc2 or rc3 then we could avoid a bad regression and with a bit of luck at the same time improve by being able to set dev->vblank_disable_immediate = true then and allow vblank irqs to get turned off more aggressively for a bit of extra power saving. thanks, -mario -- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -172,9 +172,11 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, unsigned long flags) { struct drm_vblank_crtc *vblank = &dev->vblank[pipe]; - u32 cur_vblank, diff; + u32 cur_vblank, diff = 0; bool rc; struct timeval t_vblank; + const struct timeval *t_old; + u64 diff_ns; int count = DRM_TIMESTAMP_MAXRETRIES; int framedur_ns = vblank->framedur_ns; @@ -195,13 +197,15 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, rc = drm_get_last_vbltimestamp(dev, pipe, &t_vblank, flags); } while (cur_vblank != dev->driver->get_vblank_counter(dev, pipe) && --count > 0); - if (dev->max_vblank_count != 0) { - /* trust the hw counter when it's around */ - diff = (cur_vblank - vblank->last) & dev->max_vblank_count; - } else if (rc && framedur_ns) { - const struct timeval *t_old; - u64 diff_ns; - + /* + * Always use vblank timestamping based method if supported to reject + * redundant vblank irqs. E.g., AMD hardware needs this to not screw up + * due to some irq handling quirk. + * + * This also sets the diff value for use as fallback below in case the + * hw does not support a suitable hw vblank counter. + */ + if (rc && framedur_ns) { t_old = &vblanktimestamp(dev, pipe, vblank->count); diff_ns = timeval_to_ns(&t_vblank) - timeval_to_ns(t_old); @@ -212,11 +216,28 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, */ diff = DIV_ROUND_CLOSEST_ULL(diff_ns, framedur_ns); - if (diff == 0 && flags & DRM_CALLED_FROM_VBLIRQ) - DRM_DEBUG_VBL("crtc %u: Redundant vblirq ignored." - " diff_ns = %lld, framedur_ns = %d)\n", - pipe, (long long) diff_ns, framedur_ns); - } else { + if (diff == 0 && flags & DRM_CALLED_FROM_VBLIRQ) { + DRM_DEBUG_VBL("crtc %u: Redundant vblirq ignored." + " diff_ns = %lld, framedur_ns = %d)\n", + pipe, (long long) diff_ns, framedur_ns); + + /* Treat this redundant invocation as no-op. */ + WARN_ON_ONCE(cur_vblank != vblank->last); + return; + } + } + + /* + * hw counters, if marked as supported via max_vblank_count != 0, + * *must* increment at leading edge of vblank and in sync with + * the firing of the hw vblank irq, otherwise we have a race here + * between gpu and us, where small variations in our execution + * timing can cause off-by-one miscounting of vblanks. + */ + if (dev->max_vblank_count != 0) { + /* trust the hw counter when it's around */ + diff = (cur_vblank - vblank->last) & dev->max_vblank_count; + } else if (!(rc && framedur_ns)) { /* some kind of default for drivers w/o accurate vbl timestamping */ diff = (flags & DRM_CALLED_FROM_VBLIRQ) != 0; }