[Intel-gfx] [PATCH 05/15] drm/i915/guc: Log runtime should consist of both mapping and relay

Chris Wilson chris at chris-wilson.co.uk
Tue Mar 6 13:50:25 UTC 2018


Quoting MichaƂ Winiarski (2018-02-27 12:52:20)
> -static int guc_log_runtime_create(struct intel_guc *guc)
> +static int guc_log_map(struct intel_guc *guc)
>  {
>         struct drm_i915_private *dev_priv = guc_to_i915(guc);
>         void *vaddr;
>         int ret;
>  
> -       lockdep_assert_held(&dev_priv->drm.struct_mutex);
> -
>         if (!guc->log.vma)
>                 return -ENODEV;
>  
> -       GEM_BUG_ON(guc_log_has_runtime(guc));
> -
> +       mutex_lock(&dev_priv->drm.struct_mutex);
>         ret = i915_gem_object_set_to_wc_domain(guc->log.vma->obj, true);
>         if (ret)
>                 return ret;
> @@ -416,20 +390,16 @@ static int guc_log_runtime_create(struct intel_guc *guc)
>                 DRM_ERROR("Couldn't map log buffer pages %d\n", ret);
>                 return PTR_ERR(vaddr);
>         }
> +       mutex_unlock(&dev_priv->drm.struct_mutex);

Whilst you are here, we don't need struct_mutex for
i915_gem_object_pin_map(), and we don't strictly need it for
set_to_wc_domain, but I haven't replaced that with
i915_gem_object_lock() yet.

If you could restrict the locking to set_to_wc_domain (and not introduce
an unbalanced lock on error return) that would be grand.

	mutex_lock(&dev_priv->drm.struct_mutex);
	ret = i915_gem_object_set_to_wc_domain(guc->log.vma->obj, true);
	mutex_unlock(&dev_priv->drm.struct_mutex);
	if (ret)
		return ret;
-Chris


More information about the Intel-gfx mailing list