[Intel-gfx] [PATCH v2 07/37] drm/i915: support creating LMEM objects

[Chris Wilson]

Quoting Matthew Auld (2019-06-27 21:56:03)
> We currently define LMEM, or local memory, as just another memory
> region, like system memory or stolen, which we can expose to userspace
> and can be mapped to the CPU via some BAR.
> 
> Signed-off-by: Matthew Auld <matthew.auld at intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: Abdiel Janulgue <abdiel.janulgue at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/Makefile                 |  1 +
>  drivers/gpu/drm/i915/i915_drv.h               |  5 ++
>  drivers/gpu/drm/i915/intel_region_lmem.c      | 66 +++++++++++++++++++
>  drivers/gpu/drm/i915/intel_region_lmem.h      | 16 +++++
>  .../drm/i915/selftests/i915_live_selftests.h  |  1 +
>  .../drm/i915/selftests/intel_memory_region.c  | 43 ++++++++++++
>  6 files changed, 132 insertions(+)
>  create mode 100644 drivers/gpu/drm/i915/intel_region_lmem.c
>  create mode 100644 drivers/gpu/drm/i915/intel_region_lmem.h
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 28fac19f7b04..e782f7d10524 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -132,6 +132,7 @@ i915-y += \
>           i915_scheduler.o \
>           i915_trace_points.o \
>           i915_vma.o \
> +         intel_region_lmem.o \
>           intel_wopcm.o
>  
>  # general-purpose microcontroller (GuC) support
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 838a796d9c55..7cbdffe3f129 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -93,6 +93,8 @@
>  #include "gt/intel_timeline.h"
>  #include "i915_vma.h"
>  
> +#include "intel_region_lmem.h"
> +
>  #include "intel_gvt.h"
>  
>  /* General customization:
> @@ -1341,6 +1343,8 @@ struct drm_i915_private {
>          */
>         resource_size_t stolen_usable_size;     /* Total size minus reserved ranges */
>  
> +       struct intel_memory_region *regions[ARRAY_SIZE(intel_region_map)];
> +
>         struct intel_uncore uncore;
>  
>         struct i915_virtual_gpu vgpu;
> @@ -2289,6 +2293,7 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
>  #define HAS_IPC(dev_priv)               (INTEL_INFO(dev_priv)->display.has_ipc)
>  
>  #define HAS_REGION(i915, i) (INTEL_INFO(i915)->memory_regions & (i))
> +#define HAS_LMEM(i915) HAS_REGION(i915, REGION_LMEM)
>  
>  /*
>   * For now, anything with a GuC requires uCode loading, and then supports
> diff --git a/drivers/gpu/drm/i915/intel_region_lmem.c b/drivers/gpu/drm/i915/intel_region_lmem.c
> new file mode 100644
> index 000000000000..c4b5a88627a3
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_region_lmem.c
> @@ -0,0 +1,66 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2019 Intel Corporation
> + */
> +
> +#include "i915_drv.h"
> +#include "intel_memory_region.h"
> +#include "intel_region_lmem.h"
> +
> +static const struct drm_i915_gem_object_ops region_lmem_obj_ops = {
> +       .get_pages = i915_memory_region_get_pages_buddy,
> +       .put_pages = i915_memory_region_put_pages_buddy,
> +       .release = i915_gem_object_release_memory_region,
> +};
> +
> +static struct drm_i915_gem_object *
> +lmem_create_object(struct intel_memory_region *mem,
> +                  resource_size_t size,
> +                  unsigned int flags)
> +{
> +       struct drm_i915_private *i915 = mem->i915;
> +       struct drm_i915_gem_object *obj;
> +       unsigned int cache_level;
> +
> +       if (flags & I915_BO_ALLOC_CONTIGUOUS)
> +               size = roundup_pow_of_two(size);

That should not be required. Seems like a missed opportunity to
pass the flag down to the allocator.

> +       if (size > BIT(mem->mm.max_order) * mem->mm.min_size)
> +               return ERR_PTR(-E2BIG);
> +
> +       obj = i915_gem_object_alloc();
> +       if (!obj)
> +               return ERR_PTR(-ENOMEM);
> +
> +       drm_gem_private_object_init(&i915->drm, &obj->base, size);
> +       i915_gem_object_init(obj, &region_lmem_obj_ops);


> +       obj->read_domains = I915_GEM_DOMAIN_CPU | I915_GEM_DOMAIN_GTT;
> +       cache_level = HAS_LLC(i915) ? I915_CACHE_LLC : I915_CACHE_NONE;
> +       i915_gem_object_set_cache_coherency(obj, cache_level);

That seems a little optimistic. I would strongly suggest pulling that
information from the intel_memory_region.
-Chris