Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220806122636.43068-1-tomas.winkler@intel.com> <20220806122636.43068-15-tomas.winkler@intel.com>
In-Reply-To: <20220806122636.43068-15-tomas.winkler@intel.com>
From:   Matthew Auld <matthew.william.auld@gmail.com>
Date:   Fri, 2 Sep 2022 09:25:33 +0100
Message-ID: <CAM0jSHO7sDegQysvZgfOXf5B-h6oe7nmfHeuH+KMAmFtnM3d2w@mail.gmail.com>
Subject: Re: [Intel-gfx] [PATCH v7 14/15] drm/i915/gsc: allocate extended
 operational memory in LMEM
To:     Tomas Winkler <tomas.winkler@intel.com>
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        David Airlie <airlied@linux.ie>,
        Daniel Vetter <daniel@ffwll.ch>,
        Alan Previn <alan.previn.teres.alexis@intel.com>,
        Intel Graphics Development <intel-gfx@lists.freedesktop.org>,
        Alexander Usyskin <alexander.usyskin@intel.com>,
        kernel list <linux-kernel@vger.kernel.org>,
        Rodrigo Vivi <rodrigo.vivi@intel.com>,
        Vitaly Lubart <vitaly.lubart@intel.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Sat, 6 Aug 2022 at 13:34, Tomas Winkler <tomas.winkler@intel.com> 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.alexis@intel.com>
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_gsc.c | 91 ++++++++++++++++++++++++++---
>  drivers/gpu/drm/i915/gt/intel_gsc.h |  3 +
>  2 files changed, 87 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..162bea57fbb5 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,68 @@ 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;
> +       void *vaddr;
> +       int err;
> +
> +       obj = i915_gem_object_create_lmem(gt->i915, size, I915_BO_ALLOC_CONTIGUOUS);
> +       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;
> +       }
> +
> +       vaddr = i915_gem_object_pin_map_unlocked(obj, i915_coherent_map_type(gt->i915, obj, true));
> +       if (IS_ERR(vaddr)) {
> +               err = PTR_ERR(vaddr);
> +               drm_err(&gt->i915->drm, "Failed to map gsc memory\n");
> +               goto out_unpin;
> +       }
> +
> +       memset(vaddr, 0, obj->base.size);
> +
> +       i915_gem_object_unpin_map(obj);

I think this was mentioned before, here we should rather use:

create_lmem(gt->i915, size,
                      I915_BO_ALLOC_CONTIGUOUS |
                      I915_BO_ALLOC_CPU_CLEAR);

That way we don't need to manually map and clear it here. Instead when
first allocating the pages (like with pin_pages), the clear will be
done for you.