From: Daniel J Blueman <daniel.blueman@gmail.com> To: Eric Anholt <eric@anholt.net> Cc: Ben Widawsky <ben@bwidawsk.net>, intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org, Dave Airlie <airlied@redhat.com> Subject: Re: [Intel-gfx] [PATCH 3.0-rc3] i915: Fix gen6 (SNB) GPU stalling Date: Thu, 16 Jun 2011 11:45:26 +0800 [thread overview] Message-ID: <BANLkTin_DqRJNXdVO3FTs6c59oKaAyV1zw@mail.gmail.com> (raw) In-Reply-To: <87sjrbkq2h.fsf@eliezer.anholt.net> On 16 June 2011 00:38, Eric Anholt <eric@anholt.net> wrote: > On Wed, 15 Jun 2011 13:04:51 +0800, Daniel J Blueman <daniel.blueman@gmail.com> wrote: >> The render HWSTAM is tweaked in preinstall, but we need to tweak the >> blitter HWSTAM (new to gen6). > > This still doesn't *really* make sense -- HWSTAM is supposed to be > masking updates to the status page's copy of the IIR, which we never > read, and not be involved in masking updates to the MMIO I[IS]R. So it > seems to me that this is just happening to get lucky and serialize in > the hardware for the way that we do actually read IIR (through MMIO). > And hey, we should be using the status page copy instead of MMIO some > day anyway, so that's more reason to do this patch even if we don't like > workarounds. I see we're checking only the MMIO IIR in the interrupt handler. Perhaps we should come up with a way to prove that we're not immediately seeing the correct state in the MMIO IIR when the interrupt handler is fired without the unmasking. We could also check if we get only one interrupt bit set (ie the interrupt occurred for what we wanted and not something else) when we issue a MI_USER_INTERRUPT to the blitter ring, to see if there is some unexpected behaviour on gen6. What do you think? Also, perhaps I add a comment into the patch to show it's a workaround, right? >> To me, it makes sense to reset the blitter HWSTAM register to what the >> driver expects, in case anything before the i915 module loads and >> adjusts it for a particular purpose (including debug); the render >> HWSTAM is set this way too. I could add a comment to both perhaps? >> >> Updating the blitter HWSTAM in the postinstall was a marginally safer >> choice, as there'll not be any potential race with a blitter user >> interrupt getting emitted before we're ready (which wouldn't have been >> tested), but the risk is probably so low that it could just go into >> the preinstall. > > The GPU is idle before our driver shows up, so there's no risk (there's > a bunch of leftover paranoia in the driver from the DRI1 days, none of > which ever made much sense). -- Daniel J Blueman
WARNING: multiple messages have this Message-ID (diff)
From: Daniel J Blueman <daniel.blueman@gmail.com> To: Eric Anholt <eric@anholt.net> Cc: Dave Airlie <airlied@redhat.com>, intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 3.0-rc3] i915: Fix gen6 (SNB) GPU stalling Date: Thu, 16 Jun 2011 11:45:26 +0800 [thread overview] Message-ID: <BANLkTin_DqRJNXdVO3FTs6c59oKaAyV1zw@mail.gmail.com> (raw) In-Reply-To: <87sjrbkq2h.fsf@eliezer.anholt.net> On 16 June 2011 00:38, Eric Anholt <eric@anholt.net> wrote: > On Wed, 15 Jun 2011 13:04:51 +0800, Daniel J Blueman <daniel.blueman@gmail.com> wrote: >> The render HWSTAM is tweaked in preinstall, but we need to tweak the >> blitter HWSTAM (new to gen6). > > This still doesn't *really* make sense -- HWSTAM is supposed to be > masking updates to the status page's copy of the IIR, which we never > read, and not be involved in masking updates to the MMIO I[IS]R. So it > seems to me that this is just happening to get lucky and serialize in > the hardware for the way that we do actually read IIR (through MMIO). > And hey, we should be using the status page copy instead of MMIO some > day anyway, so that's more reason to do this patch even if we don't like > workarounds. I see we're checking only the MMIO IIR in the interrupt handler. Perhaps we should come up with a way to prove that we're not immediately seeing the correct state in the MMIO IIR when the interrupt handler is fired without the unmasking. We could also check if we get only one interrupt bit set (ie the interrupt occurred for what we wanted and not something else) when we issue a MI_USER_INTERRUPT to the blitter ring, to see if there is some unexpected behaviour on gen6. What do you think? Also, perhaps I add a comment into the patch to show it's a workaround, right? >> To me, it makes sense to reset the blitter HWSTAM register to what the >> driver expects, in case anything before the i915 module loads and >> adjusts it for a particular purpose (including debug); the render >> HWSTAM is set this way too. I could add a comment to both perhaps? >> >> Updating the blitter HWSTAM in the postinstall was a marginally safer >> choice, as there'll not be any potential race with a blitter user >> interrupt getting emitted before we're ready (which wouldn't have been >> tested), but the risk is probably so low that it could just go into >> the preinstall. > > The GPU is idle before our driver shows up, so there's no risk (there's > a bunch of leftover paranoia in the driver from the DRI1 days, none of > which ever made much sense). -- Daniel J Blueman
next prev parent reply other threads:[~2011-06-16 3:45 UTC|newest] Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top 2011-06-14 16:51 [PATCH 3.0-rc3] i915: Fix gen6 (SNB) GPU stalling Daniel J Blueman 2011-06-14 17:23 ` Chris Wilson 2011-06-14 17:23 ` Chris Wilson 2011-06-15 2:06 ` Eric Anholt 2011-06-15 2:06 ` Eric Anholt 2011-06-15 3:24 ` Daniel J Blueman 2011-06-15 4:43 ` [Intel-gfx] " Ben Widawsky 2011-06-15 4:43 ` Ben Widawsky 2011-06-15 5:04 ` [Intel-gfx] " Daniel J Blueman 2011-06-15 5:04 ` Daniel J Blueman 2011-06-15 15:16 ` [Intel-gfx] " Ben Widawsky 2011-06-16 2:45 ` Daniel J Blueman 2011-06-16 2:45 ` Daniel J Blueman 2011-06-16 18:36 ` [Intel-gfx] " Eric Anholt 2011-06-16 18:36 ` Eric Anholt 2011-06-15 16:38 ` [Intel-gfx] " Eric Anholt 2011-06-16 3:45 ` Daniel J Blueman [this message] 2011-06-16 3:45 ` Daniel J Blueman 2011-06-15 17:11 ` [Intel-gfx] " Kenneth Graunke 2011-06-15 17:11 ` Kenneth Graunke 2011-06-17 7:52 ` Sun, Yi 2011-06-17 14:12 ` [Intel-gfx] " Jesse Barnes 2011-06-17 14:12 ` Jesse Barnes 2011-06-17 15:29 ` [Intel-gfx] " Robert Hooker 2011-06-17 15:29 ` Robert Hooker
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=BANLkTin_DqRJNXdVO3FTs6c59oKaAyV1zw@mail.gmail.com \ --to=daniel.blueman@gmail.com \ --cc=airlied@redhat.com \ --cc=ben@bwidawsk.net \ --cc=eric@anholt.net \ --cc=intel-gfx@lists.freedesktop.org \ --cc=linux-kernel@vger.kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.