Thomas Hellström wrote:
> Jerome Glisse wrote:
>
>> This change allow driver to pass memory space preference
>> on buffer object placement. Up to 15 differents places
>> which should be enought. The placement order is given
>> by the memory type encoded on 4bits in 64bits dword.
>> First 4bits is the number of memory placement. Next
>> 4bits is the prefered placement, then next 4bits is
>> the second prefered placement.
>>
>> In order to avoid long function prototype i used a structure
>> to path along driver wishes on the allocation. Beside
>> the 64bits dwords describing prefered placement, there
>> is flags indicating others preferences (caching attributes,
>> pinning, ...), there is also fpfn & lpfn which are the
>> first page and last page number btw which allocation can
>> happen. This allow to place a buffer in a certain range.
>> If those fields are set to 0 ttm will assume buffer can
>> be put anywhere in the address space (thus it avoids putting
>> a burden on the driver to always properly set those fields).
>>
>> This patch also factor few functions like evicting first
>> entry of lru list or getting a memory space. This avoid
>> code duplication.
>>
>> Signed-off-by: Jerome Glisse <jgli...@redhat.com>
>>
>>
> Jerome,
>
> I'm just reviewing the API change here. I'll do a more thorough code
> review when the API is agreed upon.
>
>
>> +
>> +/**
>> + * struct ttm_placement
>> + *
>> + * @fpfn: first valid page frame number to put the object
>> + * @lpfn: last valid page frame number to put the object
>> + * @placements: prefered placements, 4bits per placement,
>> + * first 4 bits are the the number of placement
>> + * @busy_placements: prefered placements when need to evict,
>> + * 4bits per placement, first 4 bits are the the number of placement
>> + * @flags: Additional placement flags
>> + *
>> + * Structure indicating the placement you request for an object.
>> + */
>> +struct ttm_placement {
>> + unsigned fpfn;
>> + unsigned lpfn;
>> + u64 placements;
>> + u32 flags;
>> +};
>> +
>>
>>
> The names fpfn and lpfn are a bit misleading, since the pfn can be
> confused with kernel pfns. These are really page offsets into the
> managed area? At least this confused me a bit.
>
> For @placements and @busy_placements, I take it you encode both the
> possible placements and the priority order (although you leave out the
> @busy_placements in the struct. I think @busy_placements are needed,
> though. Consider a situation where you want to place a texture in vram,
> but if that causes a pipeline stall due to VRAM eviction, rather place
> it in AGP.
>
> I dislike the coding of all this information in a single member, also I
> think one should distinguish between allowable placement and priority.
> The validation code may want to alter priority, which is not easily done
> in current TTM, and core TTM basically only wants to be able to move
> objects out to cached or uncached system memory.
>
> What about
>
> struct ttm_placement {
> unsigned long fpfn;
> unsigned long lpfn;
> const u64 *placements; //Encoded the traditional way
> unsigned num_placements;
> const u64 *busy_placements;
> unsigned num_busy_placements;
> }
>
>
> // For core ttm use.
> static inline struct ttm_placement *ttm_encode_simple_placement(u64
> desired, struct ttm_placement *placement)
> {
> placement->fpfn = placement->lpfn = 0;
> placement->placements = placement->busy_placements = &desired;
> placement->num_placements = placement->num_busy_placements = 1;
> }
>
>
>
Actually, on second thought this isn't particularly nice either.
What I'm after is
a) being able to specify both memory region and caching policy, for
example write-combined VRAM | cached AGP.
b) avoid encoding the stuff in a single member.
/Thomas
------------------------------------------------------------------------------
Join us December 9, 2009 for the Red Hat Virtual Experience,
a free event focused on virtualization and cloud computing.
Attend in-depth sessions from your desk. Your couch. Anywhere.
http://p.sf.net/sfu/redhat-sfdev2dev
--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel
