On 2017.01.06 19:58:16 +0000, Chris Wilson wrote:
> set_gma_to_bb_cmd() is completely bogus - it is (incorrectly) applying
> the rules to read a GTT offset from a command as opposed to writing the
> GTT offset. And to cap it all set_gma_to_bb_cmd() is called within a list
> iterator of the most strange construction.
>

Looks fine to me. I'll pick up this after validating with batch buffer scan on.

Thanks.

> Fixes: be1da7070aea ("drm/i915/gvt: vGPU command scanner")
> Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> Cc: Zhenyu Wang <zhen...@linux.intel.com>
> Cc: Zhi Wang <zhi.a.w...@intel.com>
> Cc: Yulei Zhang <yulei.zh...@intel.com>
> Cc: <drm-intel-fi...@lists.freedesktop.org> # v4.10-rc1+
> ---
>  drivers/gpu/drm/i915/gvt/execlist.c  | 64 
> ++++++++++--------------------------
>  drivers/gpu/drm/i915/gvt/scheduler.h |  2 +-
>  2 files changed, 19 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/execlist.c 
> b/drivers/gpu/drm/i915/gvt/execlist.c
> index 9a98553ad742..4b8454b445cd 100644
> --- a/drivers/gpu/drm/i915/gvt/execlist.c
> +++ b/drivers/gpu/drm/i915/gvt/execlist.c
> @@ -364,58 +364,30 @@ static void free_workload(struct intel_vgpu_workload 
> *workload)
>  #define get_desc_from_elsp_dwords(ed, i) \
>       ((struct execlist_ctx_descriptor_format *)&((ed)->data[i * 2]))
>  
> -
> -#define BATCH_BUFFER_ADDR_MASK ((1UL << 32) - (1U << 2))
> -#define BATCH_BUFFER_ADDR_HIGH_MASK ((1UL << 16) - (1U))
> -static int set_gma_to_bb_cmd(struct intel_shadow_bb_entry *entry_obj,
> -                          unsigned long add, int gmadr_bytes)
> -{
> -     if (WARN_ON(gmadr_bytes != 4 && gmadr_bytes != 8))
> -             return -1;
> -
> -     *((u32 *)(entry_obj->bb_start_cmd_va + (1 << 2))) = add &
> -             BATCH_BUFFER_ADDR_MASK;
> -     if (gmadr_bytes == 8) {
> -             *((u32 *)(entry_obj->bb_start_cmd_va + (2 << 2))) =
> -                     add & BATCH_BUFFER_ADDR_HIGH_MASK;
> -     }
> -
> -     return 0;
> -}
> -
>  static void prepare_shadow_batch_buffer(struct intel_vgpu_workload *workload)
>  {
> -     int gmadr_bytes = workload->vgpu->gvt->device_info.gmadr_bytes_in_cmd;
> +     const int gmadr_bytes = 
> workload->vgpu->gvt->device_info.gmadr_bytes_in_cmd;
> +     struct intel_shadow_bb_entry *entry_obj;
>  
>       /* pin the gem object to ggtt */
> -     if (!list_empty(&workload->shadow_bb)) {
> -             struct intel_shadow_bb_entry *entry_obj =
> -                     list_first_entry(&workload->shadow_bb,
> -                                      struct intel_shadow_bb_entry,
> -                                      list);
> -             struct intel_shadow_bb_entry *temp;
> +     list_for_each_entry(entry_obj, &workload->shadow_bb, list) {
> +             struct i915_vma *vma;
>  
> -             list_for_each_entry_safe(entry_obj, temp, &workload->shadow_bb,
> -                             list) {
> -                     struct i915_vma *vma;
> -
> -                     vma = i915_gem_object_ggtt_pin(entry_obj->obj, NULL, 0,
> -                                                    4, 0);
> -                     if (IS_ERR(vma)) {
> -                             gvt_err("Cannot pin\n");
> -                             return;
> -                     }
> -
> -                     /* FIXME: we are not tracking our pinned VMA leaving it
> -                      * up to the core to fix up the stray pin_count upon
> -                      * free.
> -                      */
> -
> -                     /* update the relocate gma with shadow batch buffer*/
> -                     set_gma_to_bb_cmd(entry_obj,
> -                                       i915_ggtt_offset(vma),
> -                                       gmadr_bytes);
> +             vma = i915_gem_object_ggtt_pin(entry_obj->obj, NULL, 0, 4, 0);
> +             if (IS_ERR(vma)) {
> +                     gvt_err("Cannot pin\n");
> +                     return;
>               }
> +
> +             /* FIXME: we are not tracking our pinned VMA leaving it
> +              * up to the core to fix up the stray pin_count upon
> +              * free.
> +              */
> +
> +             /* update the relocate gma with shadow batch buffer*/
> +             entry_obj->bb_start_cmd_va[1] = i915_ggtt_offset(vma);
> +             if (gmadr_bytes == 8)
> +                     entry_obj->bb_start_cmd_va[2] = 0;
>       }
>  }
>  
> diff --git a/drivers/gpu/drm/i915/gvt/scheduler.h 
> b/drivers/gpu/drm/i915/gvt/scheduler.h
> index 3b30c28bff51..2833dfa8c9ae 100644
> --- a/drivers/gpu/drm/i915/gvt/scheduler.h
> +++ b/drivers/gpu/drm/i915/gvt/scheduler.h
> @@ -113,7 +113,7 @@ struct intel_shadow_bb_entry {
>       struct drm_i915_gem_object *obj;
>       void *va;
>       unsigned long len;
> -     void *bb_start_cmd_va;
> +     u32 *bb_start_cmd_va;
>  };
>  
>  #define workload_q_head(vgpu, ring_id) \
> -- 
> 2.11.0
> 

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to