From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chris Wilson Subject: Re: [PATCH 4/4] drm/i915: optional fewer warning patch Date: Sat, 09 Apr 2011 23:31:14 +0100 Message-ID: <849307$cd80vp@azsmga001.ch.intel.com> References: <1302284850-8274-1-git-send-email-ben@bwidawsk.net> <1302381082-3167-1-git-send-email-ben@bwidawsk.net> <1302381082-3167-2-git-send-email-ben@bwidawsk.net> <1302381082-3167-3-git-send-email-ben@bwidawsk.net> <1302381082-3167-4-git-send-email-ben@bwidawsk.net> <1302381082-3167-5-git-send-email-ben@bwidawsk.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga03.intel.com (mga03.intel.com [143.182.124.21]) by gabe.freedesktop.org (Postfix) with ESMTP id 48BA19E710 for ; Sat, 9 Apr 2011 15:31:18 -0700 (PDT) In-Reply-To: <1302381082-3167-5-git-send-email-ben@bwidawsk.net> 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: Ben Widawsky , intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Sat, 9 Apr 2011 13:31:22 -0700, Ben Widawsky wrote: > + * > + * Intelligent users of the interface may do a force_wake_get() followed > + * by many register reads and writes, knowing that the reference count > + * is already incremented. So we do not want to warn on those. > */ Hmm, any place where we touch registers without holding a lock, unless under exceptional conditions such as !SYSTEM_RUNNING, is a bug waiting to happen. And probably already happened. Nasty, hard to reproduce race conditions. The complication is that some registers will be protected by dev->config.mode_lock rather than dev->struct_mutex (and a very rare set are protected by their own irq spinlocks). This sounds like a job for lockdep... And a lot of documentation. Fortunately, we only have those two conditions (KMS (detection and mode setting) vs GEM (execution and memory managment)) and their intersection to worry about. One of the first steps would be to review all the comments Jesse added to document the KMS locking and back those up with assertions and updates. Plenty of work to do before we can feel sure that the locking is sane. -Chris -- Chris Wilson, Intel Open Source Technology Centre