[Intel-gfx] [PATCH] drm/i915: Avoid div-by-zero on gen2

Chris Wilson chris at chris-wilson.co.uk
Sun Mar 21 16:28:07 UTC 2021


Quoting Ville Syrjala (2021-03-21 16:10:38)
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> Gen2 tiles are 2KiB in size so i915_gem_object_get_tile_row_size()
> can in fact return <4KiB, which leads to div-by-zero here.
> Avoid that.

So long as we overestimate it is fine, since we only care to find a
suitably small chunk that is large enough. I thought it was
overestimating, oh well.
 
> Not sure i915_gem_object_get_tile_row_size() is entirely
> sane anyway since it doesn't account for the different tile
> layouts on i8xx/i915...

It should not matter so long as we pick a common divisor, suitable for
the fence register.
 
> I'm not able to hit this before commit 6846895fde05 ("drm/i915:
> Replace PIN_NONFAULT with calls to PIN_NOEVICT") and it looks
> like I also need to run recent version of Mesa. With those in
> place xonotic trips on this quite easily on my 85x.

NOEVICT will make it much less eager to remove older bindings, with the
preference then to use smaller views of objects. The theory being that
the workingset is less than the whole object, so we can fit more active
pages in and cause less thrashing when moving the unused pages around
in the GTT.

> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/gem/i915_gem_mman.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> index ec28a6cde49b..0b2434e29d00 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> @@ -189,7 +189,7 @@ compute_partial_view(const struct drm_i915_gem_object *obj,
>         struct i915_ggtt_view view;
>  
>         if (i915_gem_object_is_tiled(obj))
> -               chunk = roundup(chunk, tile_row_pages(obj));
> +               chunk = roundup(chunk, tile_row_pages(obj) ?: 1);

I was thinking the answer would be to align to the next page, and hey
presto!

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


More information about the Intel-gfx mailing list