Jesse Barnes wrote:
> On Friday, September 12, 2008 12:29 am Thomas Hellström wrote:
>
>> + if (drm_mm_init(&dev->offset_manager, DRM_FILE_PAGE_OFFSET_START,
>> + DRM_FILE_PAGE_OFFSET_SIZE)) {
>> + drm_ht_remove(&dev->map_hash);
>> + return -ENOMEM;
>> + }
>>
>> A driver not using GEM would probably not want the map_hash in the core
>> data structure. What about making a small struct that the driver can
>> include in its driver_private? Likewise, what about a gem-enabled VM system
>> that the driver can hook in instead of normal drm mmap, that detects GEM
>> objects and calls normal drm mmap otherwise?
>>
>
> So bypassing all the other unused drm_mmap stuff? Yeah that would make
> things
> cleaner. I'll do that and separate out the map_hash too.
>
>
Great. I think that looks a bit nicer, and you can avoid the core drm
map data structures that aren't really of much use for
gem objects anyway. One should perhaps also think of a separate map_hash
for this, which could allow us to drastically shrink the dev->map_hash
size, since the number of legacy drm maps is usually very small.
>
>
>> + pfn = ((dev->agp->base + obj_priv->gtt_offset) >> PAGE_SHIFT) +
>> + page_offset;
>> + /* Finally, remap it using the new GTT offset */
>> + ret = vm_insert_pfn(vma, (unsigned long)vmf->virtual_address, pfn);
>>
>> Perhaps speculatively fault a couple of more pages for performance?
>>
>
> Yeah I was thinking about that; I guess it depends on the typical use case.
> Right now, we're mainly using this for X fallbacks, and with tiling hopefully
> we won't be touching many pages each time, but some instrumentation would
> help here.
>
Yeah, OTOH inserting a pfn is very quick operation, but as you say, some
instrumentation is probably valuable.
>
>> +int
>> +i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void *data,
>> + struct drm_file *file_priv)
>> +{
>> + struct drm_i915_gem_mmap_gtt *args = data;
>> + struct drm_gem_object *obj;
>> + struct drm_i915_gem_object *obj_priv;
>> +
>> + if (!(dev->driver->driver_features & DRIVER_GEM))
>> + return -ENODEV;
>> +
>> + obj = drm_gem_object_lookup(dev, file_priv, args->handle);
>> + if (obj == NULL)
>> + return -EBADF;
>> +
>> + obj_priv = obj->driver_private;
>> +
>> + mutex_lock(&dev->struct_mutex);
>> + drm_gem_object_unreference(obj);
>> + mutex_unlock(&dev->struct_mutex);
>> +
>> + args->addr_ptr = obj_priv->mmap_offset;
>> +
>> + obj_priv->gtt_alignment = args->alignment;
>> +
>> + return 0;
>> +}
>>
>> I'm not sure I understand what this function is doing?
>>
>
> Basically just returning the fake mmap offset of a given object to userspace
> so it can use it for subsequent mmap calls. It also grabs the preferred
> alignment so we know how to GTT map it when the time comes. Is there some
> easier way of getting that info? I suppose we could just return it at create
> time instead since we initialize everything there anyway...
>
>
Ah.
Perhaps. I was also a bit worried, though, about touching the obj_priv
data after the object has been
unreferenced?
>> @@ -940,6 +1029,10 @@ i915_gem_object_unbind(struct drm_gem_object *obj)
>>
>> BUG_ON(obj_priv->active);
>>
>> + /* blow away mappings if mapped through GTT */
>> + if (vma)
>> + zap_vma_ptes(vma, vma->vm_start, vma->vm_end - vma->vm_start);
>> +
>> i915_gem_object_free_page_list(obj);
>>
>>
>> What stops this racing with fault(), and furthermore how are multiple vmas
>> to the same object handled? That happens if multiple processes map the same
>> object or if you hit a fork(). What are the intended fork() semantics for a
>> GTT mapped GEM object?
>>
>
> Oh, I hadn't thought about that case; I'll have to create a list of VMAs and
> maybe take an appropriate mutex.
>
>
Yes, if you use unmap_mapping_range() instead of zap_vma_ptes you won't
need the vma list,
but OTOH, zap_vma_ptes is probably more efficient since it would know
exactly which vmas to
touch.
/Thomas
-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
--
_______________________________________________
Dri-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/dri-devel
