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 ? :)
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@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
