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

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)/

• 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.