[Intel-gfx] [PATCH 05/10] drm/i915: make dsm struct resource centric

[Chris Wilson]

Quoting Matthew Auld (2017-12-06 18:17:25)
> Now that we are using struct resource to track the stolen region, it is
> more convenient if we track dsm in a resource as well.
> 
> v2: check range_overflow when writing to 32b registers (Chris)
>     pepper in some comments (Chris)
> v3: refit i915_stolen_to_dma()
> v4: kill ggtt->stolen_size
> 
> Signed-off-by: Matthew Auld <matthew.auld at intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
> ---
> -static dma_addr_t i915_stolen_to_dma(struct drm_i915_private *dev_priv)
> +static int i915_adjust_stolen(struct drm_i915_private *dev_priv,
> +                             struct resource *dsm)
>  {
>         struct i915_ggtt *ggtt = &dev_priv->ggtt;
> -       dma_addr_t base = intel_graphics_stolen_res.start;
>         struct resource *r;
>  
> -       GEM_BUG_ON(overflows_type(intel_graphics_stolen_res.start, base));
> +       if (dsm->start == 0 || add_overflows(dsm->start, resource_size(dsm)))

Now s/add_overflows/dsm->end <= dsm->start/

> +               return -EINVAL;
>  
> -       if (base == 0 || add_overflows(base, ggtt->stolen_size))
> -               return 0;
> -
> -       /* make sure we don't clobber the GTT if it's within stolen memory */
> +       /* Make sure we don't clobber the GTT if it's within stolen memory */
>         if (INTEL_GEN(dev_priv) <= 4 &&
>             !IS_G33(dev_priv) && !IS_PINEVIEW(dev_priv) && !IS_G4X(dev_priv)) {
> -               struct {
> -                       dma_addr_t start, end;
> -               } stolen[2] = {
> -                       { .start = base, .end = base + ggtt->stolen_size, },
> -                       { .start = base, .end = base + ggtt->stolen_size, },
> +               struct resource stolen[2] = {
> +                       DEFINE_RES_MEM(dsm->start, resource_size(dsm)),
> +                       DEFINE_RES_MEM(dsm->start, resource_size(dsm)),

struct resource stolen[2] = { *dsm, *dsm } ?

>                 };
> -               u64 ggtt_start, ggtt_end;
> +               struct resource ggtt_res;
> +               u64 ggtt_start;

>         /* Verify that nothing else uses this physical address. Stolen
>          * memory should be reserved by the BIOS and hidden from the
>          * kernel. So if the region is already marked as busy, something
>          * is seriously wrong.
>          */
> -       r = devm_request_mem_region(dev_priv->drm.dev, base, ggtt->stolen_size,
> +       r = devm_request_mem_region(dev_priv->drm.dev, dsm->start,
> +                                   resource_size(dsm),
>                                     "Graphics Stolen Memory");

A future task is that we really don't need the allocation here anymore,
and should be able to reserve the resource directly.

> @@ -351,14 +339,18 @@ int i915_gem_init_stolen(struct drm_i915_private *dev_priv)
>                 return 0;
>         }
>  
> -       if (ggtt->stolen_size == 0)
> +       if (resource_size(&intel_graphics_stolen_res) == 0)
>                 return 0;
>  
> -       dev_priv->mm.stolen_base = i915_stolen_to_dma(dev_priv);
> -       if (dev_priv->mm.stolen_base == 0)
> +       dev_priv->dsm = intel_graphics_stolen_res;
> +
> +       if (i915_adjust_stolen(dev_priv, &dev_priv->dsm))
>                 return 0;
>  
> -       stolen_top = dev_priv->mm.stolen_base + ggtt->stolen_size;
> +       GEM_BUG_ON(dev_priv->dsm.start == 0);
> +       GEM_BUG_ON(resource_size(&dev_priv->dsm) == 0);

GEM_BUG_ON(dsm.end <= dsm.start); instead of size == 0

> +
> +       stolen_top = dev_priv->dsm.end + 1;
>         reserved_base = 0;
>         reserved_size = 0;
>  
> @@ -399,12 +391,12 @@ int i915_gem_init_stolen(struct drm_i915_private *dev_priv)
>                 reserved_base = stolen_top;
>         }
>  
> -       if (reserved_base < dev_priv->mm.stolen_base ||
> +       if (reserved_base < dev_priv->dsm.start ||
>             reserved_base + reserved_size > stolen_top) {
>                 dma_addr_t reserved_top = reserved_base + reserved_size;
>                 DRM_ERROR("Stolen reserved area [%pad - %pad] outside stolen memory [%pad - %pad]\n",
>                           &reserved_base, &reserved_top,
> -                         &dev_priv->mm.stolen_base, &stolen_top);
> +                         &dev_priv->dsm.start, &stolen_top);

%pR here?

Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
-Chris