On Mon, 19 Sep 2011 21:25:03 -0700, Ben Widawsky <b...@bwidawsk.net> wrote:
> When doing a GTT mapping, the pages are not backed until taking a fault.
> If we know that the object is write only, when the fault happens we do not
> need
> to make the pages coherent with the CPU. This allows for semi fast prefaults
> to occur in libdrm at map time.
>
> For the non-GTT case, there isn't much to do except make sure we handle things
> properly we we move an object out of the GTT.
>
> Finally, modify the set domain IOCTL to actually mark objects as write only.
> Current limitation in that once an object is marked write only, it cannot be
> readable.
I couldn't see any reason for this restriction and it does impose a
significant burden on the userspace cache; any buffer used for
write-only mappings must be purged and can't be reused.
> Signed-off-by: Ben Widawsky <b...@bwidawsk.net>
> ---
> drivers/gpu/drm/i915/i915_gem.c | 32 ++++++++++++++++++++++++--------
> 1 files changed, 24 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index d4de7f7..48ea4bc 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1047,6 +1047,7 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void
> *data,
> struct drm_i915_gem_object *obj;
> uint32_t read_domains = args->read_domains;
> uint32_t write_domain = args->write_domain;
> + int write_only = 0;
> int ret;
>
> if (!(dev->driver->driver_features & DRIVER_GEM))
> @@ -1059,11 +1060,15 @@ i915_gem_set_domain_ioctl(struct drm_device *dev,
> void *data,
> if (read_domains & I915_GEM_GPU_DOMAINS)
> return -EINVAL;
>
> - /* Having something in the write domain implies it's in the read
> - * domain, and only that read domain. Enforce that in the request.
> - */
> - if (write_domain != 0 && read_domains != write_domain)
> + if (write_domain != 0 && read_domains == 0) {
Neat extension of the existing ioctl. Just a little wary of the
potential for mass foot shootings - but this whole extension is about
giving them a machinegun.
> + write_only = 1;
> + } else if (write_domain != 0 && read_domains != write_domain) {
> + /* Having something in the write domain implies it's in the read
> + * domain, and only that read domain. Enforce that in the
> + * request.
> + */
> return -EINVAL;
> + }
>
> ret = i915_mutex_lock_interruptible(dev);
> if (ret)
> @@ -1084,6 +1089,13 @@ i915_gem_set_domain_ioctl(struct drm_device *dev, void
> *data,
> */
> if (ret == -EINVAL)
> ret = 0;
> + } else if (obj->cpu_write_only && !write_only) {
> + DRM_ERROR("Not yet supported\n");
> + ret = -EINVAL;
> + goto unlock;
> + } else if (write_only) {
> + atomic_set(&obj->cpu_write_only, 1);
Atomic? Did you intend for obj->cpu_write_only be atomic_t?
> + obj->base.read_domains = 0;
> } else {
> ret = i915_gem_object_set_to_cpu_domain(obj, write_domain != 0);
> }
> @@ -1217,9 +1229,13 @@ int i915_gem_fault(struct vm_area_struct *vma, struct
> vm_fault *vmf)
> if (ret)
> goto unlock;
>
> - ret = i915_gem_object_set_to_gtt_domain(obj, write);
> - if (ret)
> - goto unlock;
> + if (obj->cpu_write_only) {
> + i915_gem_object_flush_cpu_write_domain(obj);
> + } else {
> + ret = i915_gem_object_set_to_gtt_domain(obj, write);
> + if (ret)
> + goto unlock;
> + }
> }
>
> if (obj->tiling_mode == I915_TILING_NONE)
> @@ -1586,7 +1602,7 @@ i915_gem_object_put_pages_gtt(struct
> drm_i915_gem_object *obj)
> obj->dirty = 0;
>
> for (i = 0; i < page_count; i++) {
> - if (obj->dirty)
> + if (obj->dirty || obj->cpu_write_only)
Could this be simplified by marking the bo as dirty upon setting the
object write-only?
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
