From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jesse Barnes Subject: Re: [PATCH] Fixed the review issues for pm_rc6_residency IGT case Date: Mon, 16 Jun 2014 15:03:02 -0700 Message-ID: <20140616150302.01a7d581@jbarnes-desktop> References: <1402303007-25566-1-git-send-email-wendy.wang@intel.com> <20140616184330.GB1718@intel.com> <20140616133822.713a7765@jbarnes-desktop> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-pb0-f45.google.com (mail-pb0-f45.google.com [209.85.160.45]) by gabe.freedesktop.org (Postfix) with ESMTP id 72A5589664 for ; Mon, 16 Jun 2014 15:02:43 -0700 (PDT) Received: by mail-pb0-f45.google.com with SMTP id rr13so2369592pbb.32 for ; Mon, 16 Jun 2014 15:02:43 -0700 (PDT) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Daniel Vetter Cc: intel-gfx , Paul Parenteau , Lei Liu , "Jin, Gordon" , Ben Widawsky List-Id: intel-gfx@lists.freedesktop.org On Mon, 16 Jun 2014 23:55:24 +0200 Daniel Vetter wrote: > On Mon, Jun 16, 2014 at 10:38 PM, Jesse Barnes wrote: > > On Mon, 16 Jun 2014 11:43:30 -0700 > > Ben Widawsky wrote: > > > >> Hi Wendy. Daniel has reverted your original commit here: > >> commit 35554a1bcaaea55c1cfa88c0176c58d2fb3b8013 > >> Author: Daniel Vetter > >> Date: Tue Jun 10 11:05:16 2014 +0200 > >> > >> Revert "Add rc6_residency_counter subtest" > >> > >> Note that I absolutely do not agree with the decision to revert your > >> patch as was stated in the commit message. I am not sure how Daniel got > >> the impression that I thought this was "in order." > >> > >> Can you please resubmit the patch based on the latest intel-gpu-tools? > > > > I also made that clear when Daniel and I discussed it. I simply don't > > understand why a revert was necessary, especially given that we had an > > incremental patch to address many of the comments. Was the test > > breaking i-g-t runs (i.e. preventing tests from running)? Was it > > somehow crashing and causing false reports? > > Ok, I've reverted the revert since people are too unhappy with it. Fine, but that doesn't address why we needed the revert in the first place. Until we have some clear explanation of that and some criteria, this will just happen all over again down the road. So, what is revert-worthy in i-g-t? Open review items? Requests for change? False test failures? False test passes? Crashing tests? I'd vote for the latter 3 myself; did this fall into any of those categories? Thanks, -- Jesse Barnes, Intel Open Source Technology Center