On Wed, Apr 13, 2016 at 05:05:05PM +0200, Daniel Vetter wrote:
> > static bool
> > -needs_pd_load_post(struct intel_engine_cs *engine, struct intel_context *to,
> > - u32 hw_flags)
> > +needs_pd_load_post(struct intel_context *to, u32 hw_flags)
> > {
> > - struct drm_i915_private *dev_priv = engine->dev->dev_private;
> > -
> > if (!to->ppgtt)
> > return false;
> >
> > - if (!IS_GEN8(engine->dev))
> > - return false;
> > -
> > - if (engine != &dev_priv->engine[RCS])
> > + if (!IS_GEN8(to->i915))
> > return false;
> >
> > if (hw_flags & MI_RESTORE_INHIBIT)
>
> Above hunk might be better in the previous patch, but meh.
Previous patch changed needs_pd_load_pre() :-p
> > @@ -667,12 +662,11 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
> > {
> > struct intel_context *to = req->ctx;
> > struct intel_engine_cs *engine = req->engine;
> > - struct intel_context *from = engine->last_context;
> > - u32 hw_flags = 0;
> > - bool uninitialized = false;
> > + struct intel_context *from;
> > + u32 hw_flags;
> > int ret, i;
> >
> > - if (skip_rcs_switch(engine, from, to))
> > + if (skip_rcs_switch(engine->last_context, to))
> > return 0;
> >
> > /* Trying to pin first makes error handling easier. */
> > @@ -686,9 +680,23 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
> > * Pin can switch back to the default context if we end up calling into
> > * evict_everything - as a last ditch gtt defrag effort that also
> > * switches to the default context. Hence we need to reload from here.
> > + *
> > + * XXX: Doing so is painfully broken!
>
> Why the XXX addition here? I thought the point of this patch is to fix
> this ...
No, that requires my seqno->requests patches....
To fix this we have to move the context pinning into request allocation,
so that if we do have to emit a switch to the default context we do so
before we take ownership of the ring for ourselves.
/* Trying to pin first makes error handling easier. */
is not kidding.
> > */
> > from = engine->last_context;
> >
> > + /*
> > + * Clear this page out of any CPU caches for coherent swap-in/out. Note
> > + * that thanks to write = false in this call and us not setting any gpu
> > + * write domains when putting a context object onto the active list
> > + * (when switching away from it), this won't block.
> > + *
> > + * XXX: We need a real interface to do this instead of trickery.
> > + */
> > + ret = i915_gem_object_set_to_gtt_domain(to->legacy_hw_ctx.rcs_state, false);
> > + if (ret)
> > + goto unpin_out;
> > +
> > if (needs_pd_load_pre(engine, to)) {
> > /* Older GENs and non render rings still want the load first,
> > * "PP_DCLV followed by PP_DIR_BASE register through Load
> > @@ -700,70 +708,36 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
> > goto unpin_out;
> >
> > /* Doing a PD load always reloads the page dirs */
> > - to->ppgtt->pd_dirty_rings &= ~intel_engine_flag(engine);
> > + to->ppgtt->pd_dirty_rings &= ~(1 << RCS);
>
> Superflous change for the imo worse.
* shrug. I was just trying to reinforce this was RCS, not any random
engine.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
