From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f65.google.com ([74.125.82.65]:36426 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752425AbcBJSgo (ORCPT ); Wed, 10 Feb 2016 13:36:44 -0500 Received: by mail-wm0-f65.google.com with SMTP id 128so5946724wmz.3 for ; Wed, 10 Feb 2016 10:36:43 -0800 (PST) Subject: Re: [PATCH 5/6] drm: Prevent vblank counter jumps with timestamp based update method. To: Daniel Vetter References: <1454894009-15466-1-git-send-email-mario.kleiner.de@gmail.com> <1454894009-15466-6-git-send-email-mario.kleiner.de@gmail.com> <20160209100917.GP11240@phenom.ffwll.local> <56B9EF5A.6050902@gmail.com> <20160209141149.GP23290@intel.com> <20160209150347.GZ11240@phenom.ffwll.local> <56BB652A.1010107@gmail.com> Cc: =?UTF-8?B?VmlsbGUgU3lyasOkbMOk?= , dri-devel , linux@bernd-steinhauser.de, stable , =?UTF-8?Q?Michel_D=c3=a4nzer?= , Vlastimil Babka , "alexander.deucher@amd.com" , =?UTF-8?Q?Christian_K=c3=b6nig?= From: Mario Kleiner Message-ID: <56BB8338.7000504@gmail.com> Date: Wed, 10 Feb 2016 19:36:40 +0100 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: stable-owner@vger.kernel.org List-ID: On 02/10/2016 06:17 PM, Daniel Vetter wrote: > On Wed, Feb 10, 2016 at 5:28 PM, Mario Kleiner > wrote: >> There's another scenario where this zero-ts case can be hit. If the driver >> drm_vblank_init()'s - setting all timestamps to zero - and then code starts >> using vblanks (drm_vblank_get()) without drm_vblank_on beforehand, which is >> afaics the case with nouveau. Unless that's considered an error as well, >> we'd need to init the timestamps to something non-zero but harmless like 1 >> usecs at drm_vblank_init() time? > > Both legacy modeset helpers and atomic ones assume by default that you > start out with everything disabled. Pre-atomic drivers make that > happen by calling disable_unused_functions() to shut down anything the > bios has enabled. I think this can't happen. > > For drivers that do take over bootloader display config they must call > vblank_on explicitly themselves, which i915 does. > >> What makes sense as output here? DRM_WARN_ONCE? > > I'd go with WARN_ON and tune it down if it's offensive. But WARN_ON > patch for 4.6 of course. > -Daniel > Ok, so does this one have your R-b for stable as is? What about a proper nouveau fix if i find one? Probably also for 4.6 then, given that this patch fixes it up good enough for stable? -mario