Re: Allow relocation to be skipped when buffers don't move.

[<th...@tu...>]

Keith Packard wrote:

>commit 4d0b98d14dd6638ba000150686328d93cda6f410
>Author: Keith Packard <ke...@ke...>
>Date:   Tue Dec 4 12:22:30 2007 -0800
>
>    Allow relocation to be skipped when buffers don't move.
>    
>    One of the costs of superioctl has been the need to perform relocations
>    inside the kernel. The cost of mapping the buffers to the CPU and writing
>    data is fairly high, especially if those buffers have been mapped and read
>    by the GPU.
>    
>    If we assume that buffers don't move around very often, we can have the
>    client compute the relocations itself using the previous GPU address. When
>    that object doesn't move, the kernel can skip computing and writing the
>    updated data.
>    
>    Here's a patch which adds a new field to struct drm_bo_info_req called
>    'presumed_offset', and a new DRM_BO_HINT_PRESUMED_OFFSET that is set when
>    this field has been filled in by the client.
>    
>    There are two separate optimizations performed when the presumed_offset is
>    correct:
>    
>  
>
A valuable optimization!

Some comments below:

>     1. i915_exec_reloc checks to see if all previous buffer offsets were guessed
>        correctly. If so, there's no need for it to look at *any* of the
>        relocations for a buffer. When this happens, it skips the whole
>        relocation process, simply returning success.
>  
>  
>
>     2. i915_apply_reloc checks to see if the target buffer offset was guessed
>        correctly. If so, it skips mapping the relocatee, computing the
>        relocation and writing the value. If no relocations are needed, the
>        relocatee should never be mapped to the CPU, and so the kernel shouldn't
>        need to wait for any fences to pass.
>  
>
Kernel mapping of buffer object doesn't wait for fences to pass, however 
the kernel code needs to make sure before mapping, that the buffer is 
either on the unfenced list or validated with NO_EVICT, since a buffer 
must not move while the kernel has it mapped.

>diff --git a/shared-core/drm.h b/shared-core/drm.h
>index ec07b89..70a25b2 100644
>--- a/shared-core/drm.h
>+++ b/shared-core/drm.h
>@@ -753,6 +753,7 @@ struct drm_fence_arg {
> /* Don't place this buffer on the unfenced list.*/
> #define DRM_BO_HINT_DONT_FENCE  0x00000004
> #define DRM_BO_HINT_WAIT_LAZY   0x00000008
>+#define DRM_BO_HINT_PRESUMED_OFFSET 0x00000010
> 
> #define DRM_BO_INIT_MAGIC 0xfe769812
> #define DRM_BO_INIT_MAJOR 1
>
>
>@@ -769,6 +770,7 @@ struct drm_bo_info_req {
> 	unsigned int desired_tile_stride;
> 	unsigned int tile_info;
> 	unsigned int pad64;
>+	uint64_t presumed_offset;
> };
>  
>
Is this meant to be backwards compatible with an older kernel? In that case
we should bump DRM_BO_INIT_MINOR. If not, we should put the new member 
with the other 64-bit members at the top of the struct and bump 
DRM_BO_INIT_MAJOR.

/Thomas