[Intel-gfx] [CI 1/6] drm/i915: Remove temporary allocation of dma addresses when rotating

[Tvrtko Ursulin]

On 17/02/2017 16:37, Chris Wilson wrote:
> On Fri, Feb 17, 2017 at 04:28:13PM +0000, Tvrtko Ursulin wrote:
>>
>> On 17/02/2017 15:12, Chris Wilson wrote:
>>> The object already stores (computed on the fly) the index to dma address
>>> so use it instead of reallocating a large temporary array every time we
>>> bind a rotated framebuffer.
>>
>> On the other hand how big is the radix tree for a large framebuffer?
>> I remember those nodes were quite chunky and will hang around for
>> the lifetime of the object. While the above mentioned large
>> temporary array needs to be allocated only if rotated VMAs have been
>> discarded due GGTT pressure, no?
>>
>> On the other other hand maybe the radix tree won't be so big in the
>> typical case, due sg entry coalescing, but it will hang around for
>> much longer.
>
> Also don't forget that we use the radixtree for mmaps, partials, single
> page lookups. In all likelihood it already exists, and it doesn't hang
> around forever.

Yeah, but for a typical framebuffer none of this are actually used to 
trigger creating the radix tree.

For a standard 1920x1080x32 fb, temporary allocation for rotation is 
only ~16KiB so don't see that it can cause any problems even with 4k 
screens.

Struct radix_tree_node is much chunkier so how many of those will a 
typical framebuffer need and how much memory in total would it tie up? 
It would only be dropped once we drop the backing store, so for 
framebuffers basically never.

>>> -	DRM_DEBUG_KMS("Created rotated page mapping for object size %zu (%ux%u tiles, %u pages)\n",
>>> -		      obj->base.size, rot_info->plane[0].width, rot_info->plane[0].height, size);
>>
>> Hm given how chatty KMS log level is this one wasn't that harmful
>> but OK. Use to save me looking in debugfs/i915_gem_framebuffer and
>> eyeball the VMA list. Granted that is much more manageable now after
>> you added the human readable output there.
>
> It's just the odd one out. If it is useful here, it presumably has some
> use on the other branches - and do we want it at page allocation time or
> vma creation. And I don't think we really want one at vma create, so I'd
> prefer to improve the debugfs (or other) probe.

I agree that debugfs output is sufficent nowadays.

Regards,

Tvrtko