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@xxxxxxxxx>
> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
> ---
> -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@xxxxxxxxxxxxxxxxxx>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
