| Message ID | 1348581217-396-1-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com> On Tue, Sep 25, 2012 at 10:53 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > As using the contexts (with mesa) causes an instant hard hang on my > i5-2500 SandyBridge GT1 desktop, they are not ready for universal > enabling. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/i915_drv.c | 5 +++++ > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/i915_gem_context.c | 10 +++++++++- > 3 files changed, 15 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 705b2e1..6a87b21 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -64,6 +64,11 @@ module_param_named(semaphores, i915_semaphores, int, 0600); > MODULE_PARM_DESC(semaphores, > "Use semaphores for inter-ring sync (default: -1 (use per-chip defaults))"); > > +int i915_hw_contexts __read_mostly = 0; > +module_param_named(hw_contexts, i915_hw_contexts, int, 0400); > +MODULE_PARM_DESC(hw_contexts, > + "Enable hardware context support for userspace (default: disabled))"); > + > int i915_enable_rc6 __read_mostly = -1; > module_param_named(i915_enable_rc6, i915_enable_rc6, int, 0400); > MODULE_PARM_DESC(i915_enable_rc6, > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index dddc3dc..04b2134 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1245,6 +1245,7 @@ extern unsigned int i915_fbpercrtc __always_unused; > extern int i915_panel_ignore_lid __read_mostly; > extern unsigned int i915_powersave __read_mostly; > extern int i915_semaphores __read_mostly; > +extern int i915_hw_contexts __read_mostly; > extern unsigned int i915_lvds_downclock __read_mostly; > extern int i915_lvds_channel_mode __read_mostly; > extern int i915_panel_use_ssc __read_mostly; > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c > index 3d3fc10..b26b592 100644 > --- a/drivers/gpu/drm/i915/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > @@ -247,12 +247,20 @@ err_destroy: > return ret; > } > > +static bool intel_has_hw_contexts(struct drm_device *dev) > +{ > + if (!i915_hw_contexts) > + return false; > + > + return HAS_HW_CONTEXTS(dev); > +} > + > void i915_gem_context_init(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > uint32_t ctx_size; > > - if (!HAS_HW_CONTEXTS(dev)) > + if (!intel_has_hw_contexts(dev)) > dev_priv->hw_contexts_disabled = true; > > if (dev_priv->hw_contexts_disabled) > -- > 1.7.10.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, 25 Sep 2012 14:53:37 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:
> "Enable hardware context support for userspace (default: disabled))");
You've found one platform this doesn't work on, and a bunch of features
rely on this, and yet we default to disabled? That seems a bit harsh to
me.
On Tue, 25 Sep 2012 12:04:14 -0700, Ben Widawsky <ben@bwidawsk.net> wrote: > On Tue, 25 Sep 2012 14:53:37 +0100 > Chris Wilson <chris@chris-wilson.co.uk> wrote: > > > "Enable hardware context support for userspace (default: disabled))"); > > You've found one platform this doesn't work on, and a bunch of features > rely on this, and yet we default to disabled? That seems a bit harsh to > me. Exactly, it's meant to be harsh. In the limited testing that hw contexts have been exposed to, we have a number of hangs for which they are implicated. Therefore they are not safe to enable yet... Unless you can prove otherwise. :-p -Chris
On Tue, 25 Sep 2012 20:41:45 +0100 Chris Wilson <chris@chris-wilson.co.uk> wrote: > On Tue, 25 Sep 2012 12:04:14 -0700, Ben Widawsky <ben@bwidawsk.net> > wrote: > > On Tue, 25 Sep 2012 14:53:37 +0100 > > Chris Wilson <chris@chris-wilson.co.uk> wrote: > > > > > "Enable hardware context support for userspace (default: > > > disabled))"); > > > > You've found one platform this doesn't work on, and a bunch of > > features rely on this, and yet we default to disabled? That seems a > > bit harsh to me. > > Exactly, it's meant to be harsh. In the limited testing that hw > contexts have been exposed to, we have a number of hangs for which > they are implicated. Therefore they are not safe to enable yet... > Unless you can prove otherwise. :-p > -Chris > The reason I've always resisted a module parameter is that rc6 and contexts are tied so very closely together. We've had a number of issues around rc6 already, I do not want contexts to be conflated with those issues. It's interesting if rc6=0 and failures still occur. I've yet to hear of such an issue, but I'd like to know if that's the case here. As a side note, I'll mention yet again that we're missing a workaround which Daniel (and I believe you as well) have previously preemptively shot down involving sending a 3d primitive down the pipe at certain times. IIRC those are only required with rc6 enabled. Also, AFAIK many HW problems are fixed in IVB, and even more fixed in HSW, so disabling for all platforms seems like not the right decision to me. At least, as long as we only have evidence of one failing platform. As mesa will depend on this feature more and more, and with no error state or other info to go on, again I think we should defer globally disabling until we get more info, or else we risk not getting more info.
On Tue, 25 Sep 2012 13:35:31 -0700, Ben Widawsky <ben@bwidawsk.net> wrote: > On Tue, 25 Sep 2012 20:41:45 +0100 > Chris Wilson <chris@chris-wilson.co.uk> wrote: > > > On Tue, 25 Sep 2012 12:04:14 -0700, Ben Widawsky <ben@bwidawsk.net> > > wrote: > > > On Tue, 25 Sep 2012 14:53:37 +0100 > > > Chris Wilson <chris@chris-wilson.co.uk> wrote: > > > > > > > "Enable hardware context support for userspace (default: > > > > disabled))"); > > > > > > You've found one platform this doesn't work on, and a bunch of > > > features rely on this, and yet we default to disabled? That seems a > > > bit harsh to me. > > > > Exactly, it's meant to be harsh. In the limited testing that hw > > contexts have been exposed to, we have a number of hangs for which > > they are implicated. Therefore they are not safe to enable yet... > > Unless you can prove otherwise. :-p > > -Chris > > > > The reason I've always resisted a module parameter is that rc6 and > contexts are tied so very closely together. We've had a number of > issues around rc6 already, I do not want contexts to be conflated with > those issues. It's interesting if rc6=0 and failures still occur. I've > yet to hear of such an issue, but I'd like to know if that's the case > here. Disabling rc6 but keeping hw contexts works but regresses performance in old school games. rc6=0 hw_contexts=0 -> works rc6=0 hw_contexts=1 -> works rc6=1 hw_contexts=0 -> works (current default up to 3.6) rc6=1 hw_contexts=1 -> hard hang with gl (new default in 3.6) -Chris
On Tue, Sep 25, 2012 at 10:46 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > rc6=0 hw_contexts=0 -> works > rc6=0 hw_contexts=1 -> works > rc6=1 hw_contexts=0 -> works (current default up to 3.6) > rc6=1 hw_contexts=1 -> hard hang with gl (new default in 3.6) I think we need that workaround ... Ben, can you hack up what should be required? Since as soon as we get the first external regression report I'll pretty much have to disable it by default, which isn't really great. Dumping an entire 3d setup batch into the kernel looks like the lesser evil. -Daniel
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 705b2e1..6a87b21 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -64,6 +64,11 @@ module_param_named(semaphores, i915_semaphores, int, 0600); MODULE_PARM_DESC(semaphores, "Use semaphores for inter-ring sync (default: -1 (use per-chip defaults))"); +int i915_hw_contexts __read_mostly = 0; +module_param_named(hw_contexts, i915_hw_contexts, int, 0400); +MODULE_PARM_DESC(hw_contexts, + "Enable hardware context support for userspace (default: disabled))"); + int i915_enable_rc6 __read_mostly = -1; module_param_named(i915_enable_rc6, i915_enable_rc6, int, 0400); MODULE_PARM_DESC(i915_enable_rc6, diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index dddc3dc..04b2134 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1245,6 +1245,7 @@ extern unsigned int i915_fbpercrtc __always_unused; extern int i915_panel_ignore_lid __read_mostly; extern unsigned int i915_powersave __read_mostly; extern int i915_semaphores __read_mostly; +extern int i915_hw_contexts __read_mostly; extern unsigned int i915_lvds_downclock __read_mostly; extern int i915_lvds_channel_mode __read_mostly; extern int i915_panel_use_ssc __read_mostly; diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 3d3fc10..b26b592 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -247,12 +247,20 @@ err_destroy: return ret; } +static bool intel_has_hw_contexts(struct drm_device *dev) +{ + if (!i915_hw_contexts) + return false; + + return HAS_HW_CONTEXTS(dev); +} + void i915_gem_context_init(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; uint32_t ctx_size; - if (!HAS_HW_CONTEXTS(dev)) + if (!intel_has_hw_contexts(dev)) dev_priv->hw_contexts_disabled = true; if (dev_priv->hw_contexts_disabled)
As using the contexts (with mesa) causes an instant hard hang on my i5-2500 SandyBridge GT1 desktop, they are not ready for universal enabling. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_drv.c | 5 +++++ drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_gem_context.c | 10 +++++++++- 3 files changed, 15 insertions(+), 1 deletion(-)