Re: [PATCH v2 06/11] drm/i915: Move pinning of dev_priv->kernel_context into its creator

Tvrtko Ursulin
2016-05-23T12:23:25Z

On 23/05/16 13:16, Chris Wilson wrote:
> On Mon, May 23, 2016 at 01:09:02PM +0100, Tvrtko Ursulin wrote:
>>> @@ -426,6 +401,26 @@ int i915_gem_context_init(struct drm_device *dev)
>>> return PTR_ERR(ctx);
>>> }
>>>
>>> + if (ctx->legacy_hw_ctx.rcs_state) {
>>> + int ret;
>>> +
>>> + /* We may need to do things with the shrinker which
>>> + * require us to immediately switch back to the default
>>> + * context. This can cause a problem as pinning the
>>> + * default context also requires GTT space which may not
>>> + * be available. To avoid this we always pin the default
>>> + * context.
>>> + */
>>> + ret = i915_gem_obj_ggtt_pin(ctx->legacy_hw_ctx.rcs_state,
>>> + get_context_alignment(dev_priv), 0);
>>> + if (ret) {
>>> + DRM_ERROR("Failed to pinned default global context (error %d)\n",
>>> + ret);
>>
>> idr_remove here as well? Maybe we should split out the idr context
>> API from the GEM context to make it explicit - create context,
>> export it to userspace and back?
>
> No, this one is not in an idr as file_priv is NULL for the kernel
> context.

Oh right, you can claim in the commit you fixed a bug as well then! :))

- const bool is_global_default_ctx = file_priv == NULL;
...
- if (is_global_default_ctx && ctx->legacy_hw_ctx.rcs_state) {
...
- goto err_destroy;
...
- idr_remove(&file_priv->context_idr, ctx->user_handle);

Regards,

Tvrtko