Re: [PATCH 12/18] drm/i915: Unify request submission

Daniel Vetter
2016-07-28T11:49:39Z

On Thu, Jul 28, 2016 at 11:25:39AM +0100, Dave Gordon wrote:
> On 27/07/16 19:09, Chris Wilson wrote:
> > On Wed, Jul 27, 2016 at 06:51:35PM +0100, Dave Gordon wrote:
> > > > > @@ -1006,6 +1005,10 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
> > > > > host2guc_sample_forcewake(guc, client);
> > > > > guc_init_doorbell_hw(guc);
> > > > >
> > > > > + /* Take over from manual control of ELSP (execlists) */
> > > > > + for_each_engine(engine, dev_priv)
> > > > > + engine->submit_request = i915_guc_submit;
> > >
> > > This doesn't get undone in i915_guc_submission_disable().
> > > That will prevent the runtime fallback from working.
> >
> > I honestly wasn't sure if that was supported. (runtime enabling would be
> > nice...)
>
> Any time the GuC (re)loading process fails, it will revert (forever) to
> execlists mode. At present there's no way to switch back to GuC mode
> thereafter. Of course, we don't actually *expect* it to fail on a reload,
> but we did observe this in action a while back, before we discovered why
> reload on resume didn't always work. That's fixed now, but the fallback is
> still there just to make sure that any undiscovered issues around GuC reload
> don't leave you with a blank screen and an unusable machine.

Do we really get a black screen? I kinda hoped dying with -EIO is still
semi-acceptable. Still not sure maintaining all these fallback paths is
worth it. And -EIO should be good enough to grab logs of why the gpu died
and then reboot.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation