I had provided rb on vers 7 and i see the only difference here in v9 is the 
usage of I915_BO_ALLOC_CPU_CLEAR in
gsc_ext_om_alloc saving us a few lines for free. Thus:

Reviewed-by: Alan Previn <alan.previn.teres.ale...@intel.com>



On Thu, 2022-09-08 at 00:51 +0300, Winkler, Tomas wrote:
> GSC requires more operational memory than available on chip.
> Reserve 4M of LMEM for GSC operation. The memory is provided to the
> GSC as struct resource to the auxiliary data of the child device.
> 
> Cc: Alan Previn <alan.previn.teres.ale...@intel.com>
> Cc: Matthew Auld <matthew.a...@intel.com>
> Signed-off-by: Tomas Winkler <tomas.wink...@intel.com>
> Signed-off-by: Alexander Usyskin <alexander.usys...@intel.com>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospu...@intel.com>
> ---
> V9: Use I915_BO_ALLOC_CPU_CLEAR to clear the allocated memory
>     instead of doing a manual memset (Matt)
> 
>  drivers/gpu/drm/i915/gt/intel_gsc.c | 79 ++++++++++++++++++++++++++---
>  drivers/gpu/drm/i915/gt/intel_gsc.h |  3 ++
>  2 files changed, 75 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_gsc.c 
> b/drivers/gpu/drm/i915/gt/intel_gsc.c
> index e1040c8f2fd3..7af6db3194dd 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gsc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gsc.c
> @@ -7,6 +7,7 @@
>  #include <linux/mei_aux.h>
>  #include "i915_drv.h"
>  #include "i915_reg.h"
> +#include "gem/i915_gem_region.h"
>  #include "gt/intel_gsc.h"
>  #include "gt/intel_gt.h"
>  
> @@ -36,12 +37,56 @@ static int gsc_irq_init(int irq)
>       return irq_set_chip_data(irq, NULL);
>  }
>  
> +static int
> +gsc_ext_om_alloc(struct intel_gsc *gsc, struct intel_gsc_intf *intf, size_t 
> size)
> +{
> +     struct intel_gt *gt = gsc_to_gt(gsc);
> +     struct drm_i915_gem_object *obj;
> +     int err;
> +
> +     obj = i915_gem_object_create_lmem(gt->i915, size,
> +                                       I915_BO_ALLOC_CONTIGUOUS |
> +                                       I915_BO_ALLOC_CPU_CLEAR);
> +     if (IS_ERR(obj)) {
> +             drm_err(&gt->i915->drm, "Failed to allocate gsc memory\n");
> +             return PTR_ERR(obj);
> +     }
> +
> +     err = i915_gem_object_pin_pages_unlocked(obj);
> +     if (err) {
> +             drm_err(&gt->i915->drm, "Failed to pin pages for gsc memory\n");
> +             goto out_put;
> +     }
> +
> +     intf->gem_obj = obj;
> +
> +     return 0;
> +
> +out_put:
> +     i915_gem_object_put(obj);
> +     return err;
> +}
> +
> +static void gsc_ext_om_destroy(struct intel_gsc_intf *intf)
> +{
> +     struct drm_i915_gem_object *obj = fetch_and_zero(&intf->gem_obj);
> +
> +     if (!obj)
> +             return;
> +
> +     if (i915_gem_object_has_pinned_pages(obj))
> +             i915_gem_object_unpin_pages(obj);
> +
> +     i915_gem_object_put(obj);
> +}
> +
>  struct gsc_def {
>       const char *name;
>       unsigned long bar;
>       size_t bar_size;
>       bool use_polling;
>       bool slow_firmware;
> +     size_t lmem_size;
>  };
>  
>  /* gsc resources and definitions (HECI1 and HECI2) */
> @@ -74,6 +119,7 @@ static const struct gsc_def gsc_def_dg2[] = {
>               .name = "mei-gsc",
>               .bar = DG2_GSC_HECI1_BASE,
>               .bar_size = GSC_BAR_LENGTH,
> +             .lmem_size = SZ_4M,
>       },
>       {
>               .name = "mei-gscfi",
> @@ -90,26 +136,32 @@ static void gsc_release_dev(struct device *dev)
>       kfree(adev);
>  }
>  
> -static void gsc_destroy_one(struct intel_gsc_intf *intf)
> +static void gsc_destroy_one(struct drm_i915_private *i915,
> +                         struct intel_gsc *gsc, unsigned int intf_id)
>  {
> +     struct intel_gsc_intf *intf = &gsc->intf[intf_id];
> +
>       if (intf->adev) {
>               auxiliary_device_delete(&intf->adev->aux_dev);
>               auxiliary_device_uninit(&intf->adev->aux_dev);
>               intf->adev = NULL;
>       }
> +
>       if (intf->irq >= 0)
>               irq_free_desc(intf->irq);
>       intf->irq = -1;
> +
> +     gsc_ext_om_destroy(intf);
>  }
>  
> -static void gsc_init_one(struct drm_i915_private *i915,
> -                      struct intel_gsc_intf *intf,
> +static void gsc_init_one(struct drm_i915_private *i915, struct intel_gsc 
> *gsc,
>                        unsigned int intf_id)
>  {
>       struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
>       struct mei_aux_device *adev;
>       struct auxiliary_device *aux_dev;
>       const struct gsc_def *def;
> +     struct intel_gsc_intf *intf = &gsc->intf[intf_id];
>       int ret;
>  
>       intf->irq = -1;
> @@ -141,7 +193,7 @@ static void gsc_init_one(struct drm_i915_private *i915,
>       intf->irq = irq_alloc_desc(0);
>       if (intf->irq < 0) {
>               drm_err(&i915->drm, "gsc irq error %d\n", intf->irq);
> -             return;
> +             goto fail;
>       }
>  
>       ret = gsc_irq_init(intf->irq);
> @@ -155,6 +207,19 @@ static void gsc_init_one(struct drm_i915_private *i915,
>       if (!adev)
>               goto fail;
>  
> +     if (def->lmem_size) {
> +             drm_dbg(&i915->drm, "setting up GSC lmem\n");
> +
> +             if (gsc_ext_om_alloc(gsc, intf, def->lmem_size)) {
> +                     drm_err(&i915->drm, "setting up gsc extended 
> operational memory failed\n");
> +                     kfree(adev);
> +                     goto fail;
> +             }
> +
> +             adev->ext_op_mem.start = 
> i915_gem_object_get_dma_address(intf->gem_obj, 0);
> +             adev->ext_op_mem.end = adev->ext_op_mem.start + def->lmem_size;
> +     }
> +
>       adev->irq = intf->irq;
>       adev->bar.parent = &pdev->resource[0];
>       adev->bar.start = def->bar + pdev->resource[0].start;
> @@ -188,7 +253,7 @@ static void gsc_init_one(struct drm_i915_private *i915,
>  
>       return;
>  fail:
> -     gsc_destroy_one(intf);
> +     gsc_destroy_one(i915, gsc, intf->id);
>  }
>  
>  static void gsc_irq_handler(struct intel_gt *gt, unsigned int intf_id)
> @@ -229,7 +294,7 @@ void intel_gsc_init(struct intel_gsc *gsc, struct 
> drm_i915_private *i915)
>               return;
>  
>       for (i = 0; i < INTEL_GSC_NUM_INTERFACES; i++)
> -             gsc_init_one(i915, &gsc->intf[i], i);
> +             gsc_init_one(i915, gsc, i);
>  }
>  
>  void intel_gsc_fini(struct intel_gsc *gsc)
> @@ -241,5 +306,5 @@ void intel_gsc_fini(struct intel_gsc *gsc)
>               return;
>  
>       for (i = 0; i < INTEL_GSC_NUM_INTERFACES; i++)
> -             gsc_destroy_one(&gsc->intf[i]);
> +             gsc_destroy_one(gt->i915, gsc, i);
>  }
> diff --git a/drivers/gpu/drm/i915/gt/intel_gsc.h 
> b/drivers/gpu/drm/i915/gt/intel_gsc.h
> index 68582f912b21..fcac1775e9c3 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gsc.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gsc.h
> @@ -20,11 +20,14 @@ struct mei_aux_device;
>  
>  /**
>   * struct intel_gsc - graphics security controller
> + *
> + * @gem_obj: scratch memory GSC operations
>   * @intf : gsc interface
>   */
>  struct intel_gsc {
>       struct intel_gsc_intf {
>               struct mei_aux_device *adev;
> +             struct drm_i915_gem_object *gem_obj;
>               int irq;
>               unsigned int id;
>       } intf[INTEL_GSC_NUM_INTERFACES];
> -- 
> 2.37.2
> 

Reply via email to