| Message ID | 1372838878-11153-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jul 03, 2013 at 10:07:56AM +0200, Daniel Vetter wrote: > Originally I've thought that this fixes up the reset issues on my > gm45, but that was just a red herring due to b0rked testing. > > Still I much prefer writing the right values (all other fields are > reserved) instead of potentially dragging gunk around. Hence also > clear the register to 0 after a reset. "reserved; mbz" or "reserved; do not modify" ? -Chris
On Wed, Jul 03, 2013 at 09:22:49AM +0100, Chris Wilson wrote: > On Wed, Jul 03, 2013 at 10:07:56AM +0200, Daniel Vetter wrote: > > Originally I've thought that this fixes up the reset issues on my > > gm45, but that was just a red herring due to b0rked testing. > > > > Still I much prefer writing the right values (all other fields are > > reserved) instead of potentially dragging gunk around. Hence also > > clear the register to 0 after a reset. > > "reserved; mbz" or "reserved; do not modify" ? "Reserved, Acccess: RO, Default value: 0" I guess that means we can't possible get something non-zero in there ... -Daniel
On Wed, Jul 03, 2013 at 10:36:21AM +0200, Daniel Vetter wrote: > On Wed, Jul 03, 2013 at 09:22:49AM +0100, Chris Wilson wrote: > > On Wed, Jul 03, 2013 at 10:07:56AM +0200, Daniel Vetter wrote: > > > Originally I've thought that this fixes up the reset issues on my > > > gm45, but that was just a red herring due to b0rked testing. > > > > > > Still I much prefer writing the right values (all other fields are > > > reserved) instead of potentially dragging gunk around. Hence also > > > clear the register to 0 after a reset. > > > > "reserved; mbz" or "reserved; do not modify" ? > > "Reserved, Acccess: RO, Default value: 0" I guess that means we can't > possible get something non-zero in there ... Well they did leave that weasel word "default" in there... ;-) But that is a little more convincing that we aren't going to clear hidden state. -Chris
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 33cb973..ed9262c 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -798,28 +798,29 @@ static int i965_reset_complete(struct drm_device *dev) static int i965_do_reset(struct drm_device *dev) { int ret; - u8 gdrst; /* * Set the domains we want to reset (GRDOM/bits 2 and 3) as * well as the reset bit (GR/bit 0). Setting the GR bit * triggers the reset; when done, the hardware will clear it. */ - pci_read_config_byte(dev->pdev, I965_GDRST, &gdrst); pci_write_config_byte(dev->pdev, I965_GDRST, - gdrst | GRDOM_RENDER | - GRDOM_RESET_ENABLE); + GRDOM_RENDER | GRDOM_RESET_ENABLE); ret = wait_for(i965_reset_complete(dev), 500); if (ret) return ret; /* We can't reset render&media without also resetting display ... */ - pci_read_config_byte(dev->pdev, I965_GDRST, &gdrst); pci_write_config_byte(dev->pdev, I965_GDRST, - gdrst | GRDOM_MEDIA | - GRDOM_RESET_ENABLE); + GRDOM_MEDIA | GRDOM_RESET_ENABLE); - return wait_for(i965_reset_complete(dev), 500); + ret = wait_for(i965_reset_complete(dev), 500); + if (ret) + return ret; + + pci_write_config_byte(dev->pdev, I965_GDRST, 0); + + return 0; } static int ironlake_do_reset(struct drm_device *dev)
Originally I've thought that this fixes up the reset issues on my gm45, but that was just a red herring due to b0rked testing. Still I much prefer writing the right values (all other fields are reserved) instead of potentially dragging gunk around. Hence also clear the register to 0 after a reset. v2: Stop claiming this fixes anything and return 0 if successful instead of stack garbage. Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/i915/i915_drv.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)