| Message ID | 1353003843-22947-1-git-send-email-jbarnes@virtuousgeek.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Nov 15, 2012 at 7:24 PM, Jesse Barnes <jbarnes@virtuousgeek.org> 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 <james.t.kukunas@intel.com> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > --- > 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). -Daniel > > mutex_lock(&dev->struct_mutex); > intel_enable_gt_powersave(dev); > -- > 1.7.9.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, 15 Nov 2012 21:05:04 +0100 Daniel Vetter <daniel@ffwll.ch> wrote: > On Thu, Nov 15, 2012 at 7:24 PM, Jesse Barnes <jbarnes@virtuousgeek.org> 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 <james.t.kukunas@intel.com> > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > > --- > > 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.
On Thu, 15 Nov 2012 12:10:28 -0800 Jesse Barnes <jbarnes@virtuousgeek.org> wrote: > On Thu, 15 Nov 2012 21:05:04 +0100 > Daniel Vetter <daniel@ffwll.ch> wrote: > > > On Thu, Nov 15, 2012 at 7:24 PM, Jesse Barnes <jbarnes@virtuousgeek.org> 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 <james.t.kukunas@intel.com> > > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > > > --- > > > 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...
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); mutex_lock(&dev->struct_mutex); intel_enable_gt_powersave(dev);