[Intel-gfx] [PATCH 2/4] drm/i915/guc: put all guc objects in lmem when available
John Harrison
john.c.harrison at intel.com
Fri Aug 6 18:43:39 UTC 2021
On 8/2/2021 22:11, Matthew Brost wrote:
> From: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
>
> The firmware binary has to be loaded from lmem and the recommendation is
> to put all other objects in there as well. Note that we don't fall back
> to system memory if the allocation in lmem fails because all objects are
> allocated during driver load and if we have issues with lmem at that point
> something is seriously wrong with the system, so no point in trying to
> handle it.
>
> Cc: Matthew Auld <matthew.auld at intel.com>
> Cc: Abdiel Janulgue <abdiel.janulgue at linux.intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Vinay Belgaumkar <vinay.belgaumkar at intel.com>
> Cc: Radoslaw Szwichtenberg <radoslaw.szwichtenberg at intel.com>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Signed-off-by: Matthew Brost <matthew.brost at intel.com>
> ---
> drivers/gpu/drm/i915/gem/i915_gem_lmem.c | 26 ++++++++
> drivers/gpu/drm/i915/gem/i915_gem_lmem.h | 4 ++
> drivers/gpu/drm/i915/gt/uc/intel_guc.c | 9 ++-
> drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 11 +++-
> drivers/gpu/drm/i915/gt/uc/intel_huc.c | 14 ++++-
> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 75 +++++++++++++++++++++--
> 6 files changed, 127 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
> index eb345305dc52..034226c5d4d0 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_lmem.c
> @@ -103,6 +103,32 @@ __i915_gem_object_create_lmem_with_ps(struct drm_i915_private *i915,
> size, page_size, flags);
> }
>
> +struct drm_i915_gem_object *
> +i915_gem_object_create_lmem_from_data(struct drm_i915_private *i915,
> + const void *data, size_t size)
> +{
> + struct drm_i915_gem_object *obj;
> + void *map;
> +
> + obj = i915_gem_object_create_lmem(i915,
> + round_up(size, PAGE_SIZE),
> + I915_BO_ALLOC_CONTIGUOUS);
> + if (IS_ERR(obj))
> + return obj;
> +
> + map = i915_gem_object_pin_map_unlocked(obj, I915_MAP_WC);
> + if (IS_ERR(map)) {
> + i915_gem_object_put(obj);
> + return map;
> + }
> +
> + memcpy(map, data, size);
> +
> + i915_gem_object_unpin_map(obj);
> +
> + return obj;
> +}
> +
> struct drm_i915_gem_object *
> i915_gem_object_create_lmem(struct drm_i915_private *i915,
> resource_size_t size,
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_lmem.h b/drivers/gpu/drm/i915/gem/i915_gem_lmem.h
> index 4ee81fc66302..1b88ea13435c 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_lmem.h
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_lmem.h
> @@ -23,6 +23,10 @@ bool i915_gem_object_is_lmem(struct drm_i915_gem_object *obj);
>
> bool __i915_gem_object_is_lmem(struct drm_i915_gem_object *obj);
>
> +struct drm_i915_gem_object *
> +i915_gem_object_create_lmem_from_data(struct drm_i915_private *i915,
> + const void *data, size_t size);
> +
> struct drm_i915_gem_object *
> __i915_gem_object_create_lmem_with_ps(struct drm_i915_private *i915,
> resource_size_t size,
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> index 979128e28372..55160d3e401a 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> @@ -3,6 +3,7 @@
> * Copyright © 2014-2019 Intel Corporation
> */
>
> +#include "gem/i915_gem_lmem.h"
> #include "gt/intel_gt.h"
> #include "gt/intel_gt_irq.h"
> #include "gt/intel_gt_pm_irq.h"
> @@ -630,7 +631,13 @@ struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size)
> u64 flags;
> int ret;
>
> - obj = i915_gem_object_create_shmem(gt->i915, size);
> + if (HAS_LMEM(gt->i915))
> + obj = i915_gem_object_create_lmem(gt->i915, size,
> + I915_BO_ALLOC_CPU_CLEAR |
> + I915_BO_ALLOC_CONTIGUOUS);
> + else
> + obj = i915_gem_object_create_shmem(gt->i915, size);
> +
> if (IS_ERR(obj))
> return ERR_CAST(obj);
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
> index 76fe766ad1bc..962be0c12208 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
> @@ -41,7 +41,7 @@ static void guc_prepare_xfer(struct intel_uncore *uncore)
> }
>
> /* Copy RSA signature from the fw image to HW for verification */
> -static void guc_xfer_rsa(struct intel_uc_fw *guc_fw,
> +static int guc_xfer_rsa(struct intel_uc_fw *guc_fw,
> struct intel_uncore *uncore)
> {
> u32 rsa[UOS_RSA_SCRATCH_COUNT];
> @@ -49,10 +49,13 @@ static void guc_xfer_rsa(struct intel_uc_fw *guc_fw,
> int i;
>
> copied = intel_uc_fw_copy_rsa(guc_fw, rsa, sizeof(rsa));
> - GEM_BUG_ON(copied < sizeof(rsa));
> + if (copied < sizeof(rsa))
> + return -ENOMEM;
>
> for (i = 0; i < UOS_RSA_SCRATCH_COUNT; i++)
> intel_uncore_write(uncore, UOS_RSA_SCRATCH(i), rsa[i]);
> +
> + return 0;
> }
>
> /*
> @@ -141,7 +144,9 @@ int intel_guc_fw_upload(struct intel_guc *guc)
> * by the DMA engine in one operation, whereas the RSA signature is
> * loaded via MMIO.
> */
> - guc_xfer_rsa(&guc->fw, uncore);
> + ret = guc_xfer_rsa(&guc->fw, uncore);
> + if (ret)
> + goto out;
>
> /*
> * Current uCode expects the code to be loaded at 8k; locations below
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> index fc5387b410a2..ff4b6869b80b 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> @@ -87,17 +87,25 @@ static int intel_huc_rsa_data_create(struct intel_huc *huc)
> vma->obj, true));
> if (IS_ERR(vaddr)) {
> i915_vma_unpin_and_release(&vma, 0);
> - return PTR_ERR(vaddr);
> + err = PTR_ERR(vaddr);
> + goto unpin_out;
> }
>
> copied = intel_uc_fw_copy_rsa(&huc->fw, vaddr, vma->size);
> - GEM_BUG_ON(copied < huc->fw.rsa_size);
> -
> i915_gem_object_unpin_map(vma->obj);
>
> + if (copied < huc->fw.rsa_size) {
> + err = -ENOMEM;
> + goto unpin_out;
> + }
> +
> huc->rsa_data = vma;
>
> return 0;
> +
> +unpin_out:
> + i915_vma_unpin_and_release(&vma, 0);
> + return err;
> }
>
> static void intel_huc_rsa_data_destroy(struct intel_huc *huc)
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> index f632dbd32b42..f8cb00ffb506 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> @@ -7,6 +7,7 @@
> #include <linux/firmware.h>
> #include <drm/drm_print.h>
>
> +#include "gem/i915_gem_lmem.h"
> #include "intel_uc_fw.h"
> #include "intel_uc_fw_abi.h"
> #include "i915_drv.h"
> @@ -370,7 +371,11 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
> if (uc_fw->type == INTEL_UC_FW_TYPE_GUC)
> uc_fw->private_data_size = css->private_data_size;
>
> - obj = i915_gem_object_create_shmem_from_data(i915, fw->data, fw->size);
> + if (HAS_LMEM(i915))
> + obj = i915_gem_object_create_lmem_from_data(i915, fw->data, fw->size);
> + else
> + obj = i915_gem_object_create_shmem_from_data(i915, fw->data, fw->size);
> +
> if (IS_ERR(obj)) {
> err = PTR_ERR(obj);
> goto fail;
> @@ -414,6 +419,7 @@ static void uc_fw_bind_ggtt(struct intel_uc_fw *uc_fw)
> struct drm_i915_gem_object *obj = uc_fw->obj;
> struct i915_ggtt *ggtt = __uc_fw_to_gt(uc_fw)->ggtt;
> struct i915_vma *dummy = &uc_fw->dummy;
> + u32 pte_flags = 0;
>
> dummy->node.start = uc_fw_ggtt_offset(uc_fw);
> dummy->node.size = obj->base.size;
> @@ -424,9 +430,13 @@ static void uc_fw_bind_ggtt(struct intel_uc_fw *uc_fw)
> GEM_BUG_ON(dummy->node.size > ggtt->uc_fw.size);
>
> /* uc_fw->obj cache domains were not controlled across suspend */
> - drm_clflush_sg(dummy->pages);
> + if (i915_gem_object_has_struct_page(obj))
> + drm_clflush_sg(dummy->pages);
> +
> + if (i915_gem_object_is_lmem(obj))
> + pte_flags |= PTE_LM;
>
> - ggtt->vm.insert_entries(&ggtt->vm, dummy, I915_CACHE_NONE, 0);
> + ggtt->vm.insert_entries(&ggtt->vm, dummy, I915_CACHE_NONE, pte_flags);
> }
>
> static void uc_fw_unbind_ggtt(struct intel_uc_fw *uc_fw)
> @@ -585,13 +595,68 @@ void intel_uc_fw_cleanup_fetch(struct intel_uc_fw *uc_fw)
> */
> size_t intel_uc_fw_copy_rsa(struct intel_uc_fw *uc_fw, void *dst, u32 max_len)
> {
> - struct sg_table *pages = uc_fw->obj->mm.pages;
> + struct intel_memory_region *mr = uc_fw->obj->mm.region;
> u32 size = min_t(u32, uc_fw->rsa_size, max_len);
> u32 offset = sizeof(struct uc_css_header) + uc_fw->ucode_size;
> + struct sgt_iter iter;
> + size_t count = 0;
> + int idx;
>
> + /* Called during reset handling, must be atomic [no fs_reclaim] */
> GEM_BUG_ON(!intel_uc_fw_is_available(uc_fw));
>
> - return sg_pcopy_to_buffer(pages->sgl, pages->nents, dst, size, offset);
> + idx = offset >> PAGE_SHIFT;
> + offset = offset_in_page(offset);
> + if (i915_gem_object_has_struct_page(uc_fw->obj)) {
> + struct page *page;
> +
> + for_each_sgt_page(page, iter, uc_fw->obj->mm.pages) {
Why can't this just use 'sg_pcopy_to_buffer' as before?
John.
> + u32 len = min_t(u32, size, PAGE_SIZE - offset);
> + void *vaddr;
> +
> + if (idx > 0) {
> + idx--;
> + continue;
> + }
> +
> + vaddr = kmap_atomic(page);
> + memcpy(dst, vaddr + offset, len);
> + kunmap_atomic(vaddr);
> +
> + offset = 0;
> + dst += len;
> + size -= len;
> + count += len;
> + if (!size)
> + break;
> + }
> + } else {
> + dma_addr_t addr;
> +
> + for_each_sgt_daddr(addr, iter, uc_fw->obj->mm.pages) {
> + u32 len = min_t(u32, size, PAGE_SIZE - offset);
> + void __iomem *vaddr;
> +
> + if (idx > 0) {
> + idx--;
> + continue;
> + }
> +
> + vaddr = io_mapping_map_atomic_wc(&mr->iomap,
> + addr - mr->region.start);
> + memcpy_fromio(dst, vaddr + offset, len);
> + io_mapping_unmap_atomic(vaddr);
> +
> + offset = 0;
> + dst += len;
> + size -= len;
> + count += len;
> + if (!size)
> + break;
> + }
> + }
> +
> + return count;
> }
>
> /**
More information about the dri-devel
mailing list