From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 2/2] tests: add kms_edp_vdd_race Date: Mon, 11 Nov 2013 22:10:38 +0100 Message-ID: <20131111211038.GI14978@phenom.ffwll.local> References: <1384189570-2955-1-git-send-email-przanoni@gmail.com> <1384189570-2955-2-git-send-email-przanoni@gmail.com> <20131111181546.GD14978@phenom.ffwll.local> <20131111184134.GG14978@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ee0-f48.google.com (mail-ee0-f48.google.com [74.125.83.48]) by gabe.freedesktop.org (Postfix) with ESMTP id 0CBA0FA563 for ; Mon, 11 Nov 2013 13:10:06 -0800 (PST) Received: by mail-ee0-f48.google.com with SMTP id e49so303090eek.21 for ; Mon, 11 Nov 2013 13:10:06 -0800 (PST) Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: Paulo Zanoni Cc: Intel Graphics Development , Paulo Zanoni List-Id: intel-gfx@lists.freedesktop.org On Mon, Nov 11, 2013 at 04:54:32PM -0200, Paulo Zanoni wrote: > 2013/11/11 Daniel Vetter : > > On Mon, Nov 11, 2013 at 04:25:36PM -0200, Paulo Zanoni wrote: > >> 2013/11/11 Daniel Vetter : > >> > On Mon, Nov 11, 2013 at 03:06:10PM -0200, Paulo Zanoni wrote: > >> >> From: Paulo Zanoni > >> >> > >> >> We recently fixed a bug where it was impossible to do I2C transactions > >> >> on eDP panels when they were disabled. Now it should be possible to do > >> >> these transactions when the panel is disabled, but there's a race > >> >> condition that triggers dmesg errors if we try do do the I2C > >> >> transactions and set a mode on the panel at the same time. This > >> >> program should reproduce this bug and check dmesg for errors. > >> >> > >> >> Signed-off-by: Paulo Zanoni > >> > > >> > Like I've said in the previous mail I think the generic dmesg error > >> > checking should be somewhere generic (and probably in piglit). Otherwise > >> > the test looks good. And the naming also matches the new convention ;-) > >> > >> Then this test will always give a SUCCESS. Not really what I wanted :( > > > > It's not the only one. We have tests that only annoy the in-kernel debug > > features like lockdep, object use-after-free and other stuff. Or all the > > WARN backtraces from testdisplay. And very often they all "succeed". > > And that's the problem I'm trying to solve. We have a solution, it's > useful not just for me - you just gave examples of where it would be > useful too -, yet, IMHO, you still didn't give a good technical reason > on why you're rejecting it. > > > > > Checking dmesg in individual tests really doesn't make much sense imo > > Well, IMHO it makes a lot of sense. It's even helping me write code, > as I already explained. > > > > and > > needs to be somewhere where it's done for _all_ testcases. > > My code is not preventing that. In fact, I think it's helping us get > to that point. > > > > QA already has > > that in their own testrunner infrastructure, unfortunately that's not > > shared with developers so we get to invent a new wheel. > > I just proposed these new wheels... Ok, lazy me finally got around to just doing it. I've sent 2 patches to the piglit mailing list which enable dmesg checking for igt runs by default in less than 5 lines of code. For all tests, at the subtest granularity. Imo this simplicity a technical reason to do it in piglit ;-) That leaves us with your use-case of very fine-grained checking of dmesg errors within a testcase. tbh I'm not really sold on this being that useful, but I'd be ok with merging the helper code if you convinced it's a great idea. One thing though which could be improved is the cleanup - imo it's much simpler to just have an atexit handler for such helpers. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch