From mboxrd@z Thu Jan 1 00:00:00 1970 From: Keith Packard Subject: Re: [PATCH 06/43] drm/i915: protect force_wake_(get|put) with the gt_lock Date: Thu, 05 Jan 2012 07:49:12 -0800 Message-ID: <86y5tm8807.fsf@sumi.keithp.com> References: <1323867460-5486-6-git-send-email-daniel.vetter@ffwll.ch> <86zke4mxg2.fsf@sumi.keithp.com> <86wr98mqw3.fsf@sumi.keithp.com> <86obukmkdq.fsf@sumi.keithp.com> <86zke3l5fj.fsf@sumi.keithp.com> <20120104181257.GI8004@phenom.ffwll.local> <86fwfulwge.fsf@sumi.keithp.com> <20120105112908.GB3831@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1462328240==" Return-path: Received: from keithp.com (home.keithp.com [63.227.221.253]) by gabe.freedesktop.org (Postfix) with ESMTP id 5D13B9E7A0 for ; Thu, 5 Jan 2012 07:49:20 -0800 (PST) In-Reply-To: <20120105112908.GB3831@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 List-Id: intel-gfx@lists.freedesktop.org --===============1462328240== Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" --=-=-= Content-Transfer-Encoding: quoted-printable On Thu, 5 Jan 2012 12:29:08 +0100, Daniel Vetter wrote: > - The reset code (running from a workqueue) does hold sturct mutex. It's > the hangcheck and error state capture code running from softirq/timer > context causing issues. Right, I mis-wrote; I meant the hangcheck timer (which I always think of as part of the reset code). > - Forcewake works like a reference counted resources. As long as all > _get/_put calls are properly balanced, I don't see how somebody could > sneak in in between (when we don't hold the spinlock) and cause havoc. > For paranaoia we might want to drop a WARN_ON in the _put call to check > whether it ever drops below 0. But all current users are clearly > balanced, so I didn't bother with it. Right, I was just confused somehow. Still seems weird to me to drop a spinlock, execute a single instruction, and then immediately re-acquire it, along with bumping forcewake_count twice. > - My missed IRQ w/a actually relies on this by grabbing a forcewake ref in > get_irq and dropping it again in put_irq. In between there's usually a > schedule(). This is essentially the same as the user-level forcewake and would be handled in the same way -- keep forcewake_count, but use it only for long-term values. > - I've pondered with Chris whether we should do your proposed optimization > but we then noticed that the gem code doesn't actually read from any > forcewake protected registers in normal execution (I don't consider > running the hangcheck timer normal ;-). With my missed irq w/a that now > changes, so we might need to reconsider this. But imo that's material > for a separate patch. Yeah, all sounds reasonable. That separate patch can actually use per-chip functions to read/write from the chip so we can also avoid checking the forcewake stuff on register reads for older generation hardware. Make it work, then make it work faster. =2D-=20 keith.packard@intel.com --=-=-= Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) iQIVAwUBTwXGeTYtFsjWk68qAQjsmA//SjslB/ibnzTmnWnkjd7KCWiFLDwCRGXa LCj6Bz1vQ+bC0wujipk/i+L8uxEadvH9g8YwjRdzCarTjgnQzo9RTAx9vUhC1BnC Moi4stdXkOtkpzY5VSKWjxaKd5JXUiMzYy1ulGT7+oz8lhkAmTfAoHufmnm4oVZX r6nmmx1lumQdC3DHV6tfUwsgMp1FeVtUcHSSta51BcRzMKBx7Lo9rYeGeimZrsss jQeSCFZlJ59YSvOVM8tf8mLaei03qwRrw3zavx/Zq3C0K2c/sX5y/aEwwEEDLd9K d+OlZ4+0dmIWBnYwDobp3/EsGxnMqBI7XbtbGJbw03Sx3F4T6S6Z0za1frU4XSX7 B6ZkBG+mM+c0VmdgSxerWSwcbEz+DgaPhQBO8EyVsDniH1BSQJL2fopj+7fVyEoO mf8f3dBwCMg9yGShKqrS/uTijoJQyS8662d42/1a2LxuSZ3GXud8Mn9fTuLmVilw S9ENVeR3VBKAsVxnSnWFuv8KOPAes8B+TvPc2gwAOm9Ebt5xte5yAbwQvFPukxac 6/QJe+bnNnIZSONtB4oCcCVArBDkJIO5Rz9nzE9bXyazC3Et0Y1SFT5ClzdW796X xgXiLR5dHFqG1iCmX2kSHZ7zzQIoV89TZi0+WKV4KihAHo5heWv9T0BSJ/EL4VXt +f5D2iMeDIE= =UiFw -----END PGP SIGNATURE----- --=-=-=-- --===============1462328240== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx --===============1462328240==--