Re: Anonymoose ggtt_view_params

Tvrtko Ursulin
2017-01-13T11:23:49Z

On 13/01/2017 11:11, Chris Wilson wrote:
> On Fri, Jan 13, 2017 at 10:59:46AM +0000, Tvrtko Ursulin wrote:
>>
>> On 13/01/2017 10:33, Chris Wilson wrote:
>>> Ok, ok, this cover note only exists to continue the run on joke of my
>>> mispellings!
>>>
>>> Everything but
>>> [5/7] drm/i915: Convert i915_ggtt_view to use an anonymous union
>>> has a r-b, so this is a good time to complain if this is too much of a
>>> hack.
>>
>> If you could polish your clouded crystal ball to see if more view
>> types might be coming, which then might have a colliding parameters
>> size and foil the whole idea.
>>
>> I do think it is a little bit of hack with a questionable benefit.
>> And I think I asked a few times if you really see a performance
>> difference for a few bytes smaller memcmp? Presumably it would be
>> some test case with a huge number of partial views which could
>> theoretically maybe show something?
>
> It was the doubling code size of i915_vma_compare() that struck me as
> objectionable.

Why does this series shrink i915_vma_compare? Was it getting inlined in
your build? For me it doesn't.

>> Downside is if we need to revert it would be relatively "churny".
>
> We just split the type into two fields, the enum and size.

Yes realized that a bit later.

Regards,

Tvrtko