Re: [PATCH v3 23/37] drm/i915: do not map aperture if it is not available.

Chris Wilson
2019-08-10T11:02:41Z

Quoting Matthew Auld (2019-08-09 23:26:29)
> From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>
> Skip both setup and cleanup of the aperture mapping if the HW doesn't
> have an aperture bar.
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Matthew Auld <matthew.auld@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 36 ++++++++++++++++++-----------
> 1 file changed, 22 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 5bcf71b18e5f..dd28c54527e3 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2795,8 +2795,10 @@ static void ggtt_cleanup_hw(struct i915_ggtt *ggtt)
>
> mutex_unlock(&i915->drm.struct_mutex);
>
> - arch_phys_wc_del(ggtt->mtrr);
> - io_mapping_fini(&ggtt->iomap);
> + if (HAS_MAPPABLE_APERTURE(i915)) {

I feel here it should be less shouting.

> + arch_phys_wc_del(ggtt->mtrr);

wc_del() doesn't care if we call it without a mtrr (nothing has been
using an mtrr since PAT).

if (ggtt->iomap.size)
> + io_mapping_fini(&ggtt->iomap);

> + }
> }
>
> /**
> @@ -2992,10 +2994,13 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
> int err;
>
> /* TODO: We're not aware of mappable constraints on gen8 yet */
> - ggtt->gmadr =
> - (struct resource) DEFINE_RES_MEM(pci_resource_start(pdev, 2),
> - pci_resource_len(pdev, 2));
> - ggtt->mappable_end = resource_size(&ggtt->gmadr);
> + /* FIXME: We probably need to add do device_info or runtime_info */
> + if (!HAS_LMEM(dev_priv)) {
> + ggtt->gmadr =
> + (struct resource) DEFINE_RES_MEM(pci_resource_start(pdev, 2),
> + pci_resource_len(pdev, 2));
> + ggtt->mappable_end = resource_size(&ggtt->gmadr);
> + }
>
> err = pci_set_dma_mask(pdev, DMA_BIT_MASK(39));
> if (!err)
> @@ -3220,15 +3225,18 @@ static int ggtt_init_hw(struct i915_ggtt *ggtt)
> if (!HAS_LLC(i915) && !HAS_PPGTT(i915))
> ggtt->vm.mm.color_adjust = i915_gtt_color_adjust;
>
> - if (!io_mapping_init_wc(&ggtt->iomap,
> - ggtt->gmadr.start,
> - ggtt->mappable_end)) {
> - ggtt->vm.cleanup(&ggtt->vm);
> - ret = -EIO;
> - goto out;
> - }
> + if (HAS_MAPPABLE_APERTURE(i915)) {
> + if (!io_mapping_init_wc(&ggtt->iomap,

if (ggtt->mappable_end) {

In this file, we are dealing with the low level details so I would
prefer less obfuscation.

> + ggtt->gmadr.start,
> + ggtt->mappable_end)) {
> + ggtt->vm.cleanup(&ggtt->vm);
> + ret = -EIO;
> + goto out;
> + }
>
> - ggtt->mtrr = arch_phys_wc_add(ggtt->gmadr.start, ggtt->mappable_end);
> + ggtt->mtrr = arch_phys_wc_add(ggtt->gmadr.start,
> + ggtt->mappable_end);
> + }
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org