Re: [PATCH 3/5] drm/i915: Track GEN6 page table usage

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Mar 16, 2015 at 04:00:56PM +0000, Michel Thierry wrote:
> +static inline uint32_t i915_pte_count(uint64_t addr, size_t length,
> +					uint32_t pde_shift)
> +{
> +	const uint64_t mask = ~((1 << pde_shift) - 1);
> +	uint64_t end;
> +
> +	BUG_ON(length == 0);
> +	BUG_ON(offset_in_page(addr|length));
> +
> +	end = addr + length;
> +
> +	if ((addr & mask) != (end & mask))
> +		return NUM_PTE(pde_shift) - i915_pte_index(addr, pde_shift);
> +
> +	return i915_pte_index(end, pde_shift) - i915_pte_index(addr, pde_shift);
> +}

Also this is an impressively big function, I'm pretty sure too big to get
inlined reasonable. Can you please follow-up with a patch to move this
into i915_gem_gtt.c?

General rule for static inline is that either
a) function body is obviously smaller when inlined in code-size
b) patch comes with solid justification in the form of hard performance
data that the inline is better

If neither applies drop the static inline. Generally these
microoptimizations are premature and only result in binary code size
increase. Which tends to thrash instruction caches and actually reduces
performance.

Iirc there's more of that going on in this series, so perhaps do a
follow-up for all of these.
Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx





[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux