On to, 2017-02-09 at 02:23 -0800, Oscar Mateo wrote:
> From: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
>
> The GuC descriptor is big in size. If we use a local definition of
> guc_desc we have a chance to overflow stack, so avoid it.
>
> Also, Chris abhors scatterlists :)
>
> Signed-off-by: Oscar Mateo <oscar.mateo@xxxxxxxxx>
<SNIP>
> @@ -116,15 +114,8 @@ static int guc_update_doorbell_id(struct intel_guc *guc,
> }
>
> /* Update the GuC's idea of the doorbell ID */
> - len = sg_pcopy_to_buffer(sg->sgl, sg->nents, &desc, sizeof(desc),
> - sizeof(desc) * client->ctx_index);
> - if (len != sizeof(desc))
> - return -EFAULT;
> - desc.db_id = new_id;
> - len = sg_pcopy_from_buffer(sg->sgl, sg->nents, &desc, sizeof(desc),
> - sizeof(desc) * client->ctx_index);
> - if (len != sizeof(desc))
> - return -EFAULT;
> + desc = guc->ctx_pool_vaddr + sizeof(*desc) * client->ctx_index;
As Chris mentioned, could use a __get_doorbell_desc helper for this.
> @@ -895,6 +876,13 @@ int i915_guc_submission_init(struct drm_i915_private *dev_priv)
> return PTR_ERR(vma);
>
> guc->ctx_pool_vma = vma;
> +
> + vaddr = i915_gem_object_pin_map(vma->obj, I915_MAP_WB);
> + if (IS_ERR(vaddr))
After onion teardown added to this function, propagate errors;
err = ERR_PTR(vaddr);
> + goto err;
> +
> + guc->ctx_pool_vaddr = vaddr;
> +
> @@ -983,8 +971,11 @@ void i915_guc_submission_fini(struct drm_i915_private *dev_priv)
> i915_vma_unpin_and_release(&guc->ads_vma);
> i915_vma_unpin_and_release(&guc->log.vma);
>
> - if (guc->ctx_pool_vma)
> + if (guc->ctx_pool_vaddr) {
> ida_destroy(&guc->ctx_ids);
> + i915_gem_object_unpin_map(guc->ctx_pool_vma->obj);
> + }
This converted to unconditional chain of teardown.
> +
> i915_vma_unpin_and_release(&guc->ctx_pool_vma);
> }
>
> @@ -1040,5 +1031,3 @@ int intel_guc_resume(struct drm_i915_private *dev_priv)
>
> > return intel_guc_send(guc, data, ARRAY_SIZE(data));
> }
> -
> -
> diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
> index 8a33a46..29ee373 100644
> --- a/drivers/gpu/drm/i915/intel_uc.h
> +++ b/drivers/gpu/drm/i915/intel_uc.h
> @@ -155,6 +155,7 @@ struct intel_guc {
>
> struct i915_vma *ads_vma;
> struct i915_vma *ctx_pool_vma;
s/ctx_pool_vma/ctx_pool/ would bring out the dependency explicitly.
> + void *ctx_pool_vaddr;
> struct ida ctx_ids;
>
> struct i915_guc_client *execbuf_client;
This needs a rebase on top of my cleanup, but is very welcome
improvement.
The types of variables are still wild, but not introduced by this
change.
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
