From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jesse Barnes Subject: Re: [PATCH] drm/i915: reset GPU after clock gating init Date: Sat, 17 Nov 2012 07:34:43 -0800 Message-ID: <20121117073443.4137a357@jbarnes-desktop> References: <1353003843-22947-1-git-send-email-jbarnes@virtuousgeek.org> <20121115121028.6e0c13ad@jbarnes-desktop> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from oproxy9.bluehost.com (oproxy9.bluehost.com [69.89.24.6]) by gabe.freedesktop.org (Postfix) with SMTP id 81B22E5EBF for ; Sat, 17 Nov 2012 07:34:06 -0800 (PST) In-Reply-To: <20121115121028.6e0c13ad@jbarnes-desktop> 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: Jesse Barnes Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Thu, 15 Nov 2012 12:10:28 -0800 Jesse Barnes wrote: > On Thu, 15 Nov 2012 21:05:04 +0100 > Daniel Vetter wrote: > > > On Thu, Nov 15, 2012 at 7:24 PM, Jesse Barnes wrote: > > > This is needed for SNB at least after disabling CSunit clock gating, and > > > shouldn't hurt on other platforms either. > > > > > > This fixes an issue on James's machine where RC6 wouldn't always get > > > enabled. > > > > > > Tested-by: James Kukunas > > > Signed-off-by: Jesse Barnes > > > --- > > > drivers/gpu/drm/i915/intel_display.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > > index 6d8a5ed..2fccd8f 100644 > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > @@ -8718,6 +8718,7 @@ void intel_modeset_init_hw(struct drm_device *dev) > > > intel_prepare_ddi(dev); > > > > > > intel_init_clock_gating(dev); > > > + intel_gpu_reset(dev); > > > > Unconditionally resetting machines tends to upset users, especially > > since it isn't implemented or doesn't work on gen2/3, leaving a wedged > > machine behind. And it spews ERRORs into dmesg. Also, a tiny comment > > explaining what's going on would be preferable. > > > > I think the better approach is to either call the low-level reset > > function from the relevant clock-gating callbacks (maybe shovel the > > reset functions into dev_priv->gt.reset while at it). Or alternatively > > figure out what's wrong with our init sequence and reorder things > > (maybe that specific w/a needs to happen before we enable rings, it > > would not be the first one). > > No this workaround is specifically: disable CSunit, reset render. > > Yeah it can be stuffed in the gen6 clock gating init, or even pushed > off into the GT rps init work handler, since it may take awhile. I'll > clean it up. > Apparently this causes performance regressions on the Dell machines it "fixes". Can anyone else confirm that? Maybe we're not restoring some state after reset which causes trouble... -- Jesse Barnes, Intel Open Source Technology Center