From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Widawsky Subject: Re: [PATCH 1/2] drm/i915: Dumb down the semaphore logic Date: Sat, 10 Sep 2011 20:44:47 -0700 Message-ID: <20110910204447.56d949d1@bwidawsk.net> References: <1315437162-14312-1-git-send-email-ben@bwidawsk.net> <20110908075236.GA2884@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from cloud01.chad-versace.us (184-106-247-128.static.cloud-ips.com [184.106.247.128]) by gabe.freedesktop.org (Postfix) with ESMTP id 97AD09E75E for ; Sat, 10 Sep 2011 20:44:36 -0700 (PDT) In-Reply-To: <20110908075236.GA2884@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Daniel Vetter Cc: Daniel Vetter , intel-gfx@lists.freedesktop.org, Eric@freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Thu, 8 Sep 2011 09:52:36 +0200 Daniel Vetter wrote: > On Wed, Sep 07, 2011 at 04:12:41PM -0700, Ben Widawsky wrote: > > While I think the previous code is correct, it was hard to follow > > and hard to debug. Since we already have a ring abstraction, might > > as well use it to handle the semaphore updates and compares. > > > > I don't expect this code to make semaphores better or worse, but you > > never know... > > I kinda start to like this ;-) As I said, I am biased. To me it is much simpler to understand, but it's definitely more code, and more abstraction. > > While you stare at this, two things I'm pondering: > - Would it make sense to also move the !semaphores ring_sync_to case > into the ringbuffer abstraction? That is an excellent idea. I will work on it with the other recommendations from Keith. > - Can we have a basic testcase for the magic values (and semaphores in > general), please? I really like what you've done with the test case after the fixes discussed on IRC. I will update my commit message to mention it if you manage to push that before I fix up the other things. Ben