On Mon, May 01, 2017 at 06:17:09PM -0700, Lionel Landwerlin wrote:

Focusing on the bit I know best and leaving the hw mumbo jumbo to one
side...

> +static int gen8_configure_all_contexts(struct drm_i915_private *dev_priv)
> +{
> +     struct intel_engine_cs *engine = dev_priv->engine[RCS];
> +     struct i915_gem_context *ctx;
> +     int ret;
> +
> +     ret = i915_mutex_lock_interruptible(&dev_priv->drm);
> +     if (ret)
> +             return ret;
> +
> +     /* Switch away from any user context.  */
> +     ret = i915_gem_switch_to_kernel_context(dev_priv);
> +     if (ret) {
> +             mutex_unlock(&dev_priv->drm.struct_mutex);
> +             return ret;
> +     }
> +
> +     /* The OA register config is setup through the context image. This image
> +      * might be written to by the GPU on context switch (in particular on
> +      * lite-restore). This means we can't safely update a context's image,
> +      * if this context is scheduled/submitted to run on the GPU.
> +      *
> +      * We could emit the OA register config through the batch buffer but
> +      * this might leave small interval of time where the OA unit is
> +      * configured at an invalid sampling period.
> +      *
> +      * So far the best way to work around this issue seems to be draining
> +      * the GPU from any submitted work.
> +      */
> +     ret = i915_gem_wait_for_idle(dev_priv,
> +                                  I915_WAIT_INTERRUPTIBLE |
> +                                  I915_WAIT_LOCKED);
> +     if (ret) {
> +             mutex_unlock(&dev_priv->drm.struct_mutex);
> +             return ret;
> +     }
> +
> +     /* Update all contexts now that we've stalled the submission. */
> +     list_for_each_entry(ctx, &dev_priv->context_list, link) {
> +             ret = engine->context_pin(engine, ctx);
> +             if (ret) {
> +                     mutex_unlock(&dev_priv->drm.struct_mutex);
> +                     return ret;
> +             }
> +
> +             gen8_update_reg_state_unlocked(ctx,
> +                                            ctx->engine[RCS].lrc_reg_state);

engine->context_unpin is missing.

Since you do populate the OA settings for contexts initialised later,
you can skip the context_pin here (keeping the deferred initialisation
for currently unused contexts) and use

        list_for_each_entry(ctx, &dev_priv->context_list, link) {
                struct intel_context *ce = &ctx->engine[RCS];
                u32 *regs;

                if (!ce->state) /* OA settings will be set upon first use */
                        continue;

                /* Hmm, export this from intel_lrc.c? */
                regs = i915_gem_object_pin_map(ce->state->obj, I915_MAP_WB);
                if (IS_ERR(regs)) {
                        /* teardown? */
                        mutex_unlock(&ctx->i915->drm.struct_mutex);
                        return PTR_ERR(regs);
                }
                ce->state->obj->mm.dirty = true;
                regs += LRC_STATE_PN * PAGE_SIZE / sizeof(*regs);
                /* Probably best since we don't really want to expose
                 * the LRC_STATE_PN trick.
                 */

                gen8_update_reg_state_unlocked(ctx, regs);

                i915_gem_object_unpin_map(ce->state->obj);
        }

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to