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

[Michel Thierry <michel.thierry@intel.com>]

On 3/19/2015 4:13 PM, Daniel Vetter wrote:
> 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
That's the biggest one, the rest will be 2-3 lines long. I'll send a 
follow-up patch for this one.

Thanks
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx