On Thu, Mar 16, 2017 at 11:44:10AM +0530, sourab.gu...@intel.com wrote:
> @@ -3593,6 +3670,7 @@ void i915_oa_init_reg_state(struct intel_engine_cs 
> *engine,
>  void i915_oa_update_reg_state(struct intel_engine_cs *engine,
>                             struct i915_gem_context *ctx,
>                             uint32_t *reg_state);
> +void i915_perf_command_stream_hook(struct drm_i915_gem_request *req);
>  
>  /* i915_gem_evict.c */
>  int __must_check i915_gem_evict_something(struct i915_address_space *vm,
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
> b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index aa75ea2..7af32c97 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -1441,12 +1441,16 @@ static void eb_export_fence(struct 
> drm_i915_gem_object *obj,
>       if (exec_len == 0)
>               exec_len = params->batch->size - 
> params->args_batch_start_offset;
>  
> +     i915_perf_command_stream_hook(params->request);

Could you have named it anything more cyptic and inconsistent?

Quite clearly this can fail, so justify the non handling of errors.

DRM_ERROR is not error handling, it is an indication that this patch
isn't ready.

> +void i915_perf_command_stream_hook(struct drm_i915_gem_request *request)
> +{
> +     struct intel_engine_cs *engine = request->engine;
> +     struct drm_i915_private *dev_priv = engine->i915;
> +     struct i915_perf_stream *stream;
> +
> +     if (!dev_priv->perf.initialized)
> +             return;
> +
> +     mutex_lock(&dev_priv->perf.streams_lock);

Justify a new global lock very, very carefully on execbuf.

> +     list_for_each_entry(stream, &dev_priv->perf.streams, link) {
> +             if ((stream->state == I915_PERF_STREAM_ENABLED) &&
> +                                     stream->cs_mode)
> +                     stream->ops->command_stream_hook(stream, request);
> +     }
> +     mutex_unlock(&dev_priv->perf.streams_lock);
> +}

> +static void i915_perf_command_stream_hook_oa(struct i915_perf_stream *stream,
> +                                     struct drm_i915_gem_request *request)
> +{
> +     struct drm_i915_private *dev_priv = request->i915;
> +     struct i915_gem_context *ctx = request->ctx;
> +     struct i915_perf_cs_sample *sample;
> +     u32 addr = 0;
> +     u32 cmd, *cs;
> +
> +     sample = kzalloc(sizeof(*sample), GFP_KERNEL);
> +     if (sample == NULL) {
> +             DRM_ERROR("Perf sample alloc failed\n");
> +             return;
> +     }
> +
> +     cs = intel_ring_begin(request, 4);
> +     if (IS_ERR(cs)) {
> +             kfree(sample);
> +             return;
> +     }
> +
> +     sample->ctx_id = ctx->hw_id;
> +     i915_gem_request_assign(&sample->request, request);

> +
> +     i915_gem_active_set(&stream->last_request, request);

Hint, you don't need you own request trap.
-Chris

-- 
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