[PATCH] drm/radeon: Add support for userspace fence waits

[Alex Deucher]

On Tue, Jan 31, 2012 at 2:07 PM, Jerome Glisse <j.glisse at gmail.com> wrote:
> On Tue, Jan 31, 2012 at 01:55:43PM -0500, David Airlie wrote:
>>
>>
>> Some comments below.
>>
>> > +   struct radeon_device *rdev = dev->dev_private;
>> > +   struct drm_gem_object *gobj;
>> > +   struct radeon_bo *robj;
>> > +   void *buffer_data;
>> > +   uint32_t *fence_data;
>> > +   int r = 0;
>> > +   long timeout;
>> > +   int ring = RADEON_RING_TYPE_GFX_INDEX;
>> > +
>> > +   /* If you're implementing this for other rings, you'll want to
>> > share
>> > +      code with radeon_cs_get_ring in radeon_cs.c */
>> > +   if (args->ring != RADEON_CS_RING_GFX) {
>> > +           return -EINVAL;
>> > +   }
>> > +
>> > +   gobj = drm_gem_object_lookup(dev, filp, args->handle);
>> > +   if (gobj == NULL) {
>> > +           return -ENOENT;
>> > +   }
>> > +   robj = gem_to_radeon_bo(gobj);
>> > +
>> > +   if (gobj->size < args->offset) {
>> > +           r = -EINVAL;
>> > +           goto unreference;
>> > +   }
>> > +
>> > +   r = radeon_bo_reserve(robj, true);
>> > +   if (r) {
>> > +           goto unreference;
>> > +   }
>> > +
>> > +   r = radeon_bo_pin(robj, RADEON_GEM_DOMAIN_GTT, NULL);
>> > +   if (r) {
>> > +           goto unreserve;
>> > +   }
>> > +
>> > +   r = radeon_bo_kmap(robj, &buffer_data);
>> > +   if (r) {
>> > +           goto unpin;
>> > +   }
>> > +
>>
>>
>> Do you need to pin it? I think if you have it reserved and you are in here you shouldn't need to. (unless kmapping requires a pin)/
>
> No you don't need to pin, actually it's bad to pin the buffer you might
> want to wait on might be in vram.
>
> Also you never assume that things could go wrong and that your value
> might never be written to the buffer. So it will wait forever in the
> kernel.
>
> As i said in the other mail i would rather as a wait on submited cs
> ioctl and add some return value from cs ioctl. I can't remember the
> outcome of this discusion we had when we were doing virtual memory
> support. Alex ? Michel ? better memory than i do ? :)
>

I vaguely recall the discussion, but I don't remember the details,
I'll have to look through my old mail.  Maybe a new CS ioctl flag
requesting EOP irqs for the CS would be a better approach than a
separate ioctl.

Alex

> Cheers,
> Jerome
>
>>
>>
>> > +   radeon_irq_kms_sw_irq_get(rdev, ring);
>> > +
>> > +   fence_data = (uint32_t*)buffer_data;
>> > +   timeout =
>> > an
>> > + * interrupt and the value in the buffer might have changed.
>> > + */
>> > +struct drm_radeon_gem_wait_user_fence {
>> > +   uint32_t                handle;
>> > +   uint32_t                ring;
>> > +   uint64_t                offset;
>> > +   uint32_t                value;
>> > +   uint64_t                timeout_usec;
>>
>> Align things here, 64 then 64 then 32 32 32 and a 32 pad.
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel