[Intel-gfx] [igt PATCH] gen9: fix gem_render_copy 3d state setup

[Daniel Vetter]

On Thu, Jan 29, 2015 at 04:15:55PM +0200, Mika Kuoppala wrote:
> Damien Lespiau <damien.lespiau at intel.com> writes:
> 
> > On Thu, Jan 29, 2015 at 11:32:46AM +0000, Chris Wilson wrote:
> >> On Thu, Jan 29, 2015 at 11:17:04AM +0000, Damien Lespiau wrote:
> >> > On Thu, Jan 29, 2015 at 11:12:46AM +0000, Chris Wilson wrote:
> >> > > On Thu, Jan 29, 2015 at 03:01:50AM -0800, Imre Deak wrote:
> >> > > > On Thu, 2015-01-29 at 10:51 +0000, Damien Lespiau wrote:
> >> > > > > On Thu, Jan 29, 2015 at 12:03:19AM -0800, Imre Deak wrote:
> >> > > > > > Without emitting the default 3DSTATE_WM_DEPTH_STENCIL state the test
> >> > > > > > will fail.
> >> > > > > > 
> >> > > > > > Signed-off-by: Imre Deak <imre.deak at intel.com>
> >> > > > > 
> >> > > > > Question: Wasn't the golden context supposed to paper over those?
> >> > > > 
> >> > > > Perhaps, currently the golden context doesn't include this.
> >> > > 
> >> > > Today, you cannot rely on the initial contents of the context even with
> >> > > the golden render state. There is no pristine context, every client is
> >> > > responsible for configuring the hardware exactly as they intend to use -
> >> > > at least as regards the untrusted commands (e.g. 3DSTATE).
> >> > 
> >> > Right. Now the question is, do we want to change that and have the
> >> > golden context with sane defaults?
> >> 
> >> You missed the point. The point is that we don't keep initialise every
> >> context from scratch. And there still doesn't seem to be any reason to
> >> be papering over userspace bugs.
> >
> > That's because I still think the end of the journey is a fully
> > initialized golden context image + copy of that context on context
> > creation.
> >
> 
> For me the end journey has looked like this:
> 
> For first (default) context:
> - run golden/null batch
> - emit workarounds
> - take a master copy from ctx_obj
> 
> Then:
>  - copy from master (with gpu or cpu) for every new fd/ctx

Can we just have it? I think we only need to do this here:


diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 8603bf48d3ee..e20cfa1ac88c 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -633,7 +633,7 @@ static int do_switch(struct intel_engine_cs *ring,
 			goto unpin_out;
 	}
 
-	if (!to->legacy_hw_ctx.initialized || i915_gem_context_is_default(to))
+	if (!to->legacy_hw_ctx.initialized)
 		hw_flags |= MI_RESTORE_INHIBIT;
 
 	ret = mi_set_context(ring, to, hw_flags);


Chris doesn't like it though for the perf implications. I've sent this out
a while ago but never got around to doing the microbenchmark Chris
requested. So testing (and if it works, volunteers for the microbenchmark)
highly welcome ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch