Attachment 14639 Details for Bug 14696 – Fix-check patch

[patch] Fix-check patch

i915_fix.diff (text/plain), 10.24 KB, created by Thomas Hellström on 2008-02-28 03:25:37 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Thomas Hellström

Created: 2008-02-28 03:25:37 UTC

Size: 10.24 KB

patch

obsolete

>diff --git a/shared-core/i915_dma.c b/shared-core/i915_dma.c
>index f9e02c7..63003f1 100644
>--- a/shared-core/i915_dma.c
>+++ b/shared-core/i915_dma.c
>@@ -31,6 +31,11 @@ #include "drm.h"
> #include "i915_drm.h"
> #include "i915_drv.h"
> 
>+#undef FIX1
>+#undef FIX2
>+#undef FIX3
>+
>+
> /* Really want an OS-independent resettable timer.  Would like to have
>  * this loop run for (eg) 3 sec, but have the timer reset every time
>  * the head pointer changes, so that EBUSY only happens if the ring
>@@ -805,15 +810,16 @@ struct i915_relocatee_info {
> 	unsigned page_offset;
> 	struct drm_bo_kmap_obj kmap;
> 	int is_iomem;
>-	int idle;
> };
> 
> struct drm_i915_validate_buffer {
> 	struct drm_buffer_object *buffer;
>-	struct drm_bo_info_rep rep;
> 	int presumed_offset_correct;
>+#ifdef FIX2
>+	struct drm_bo_info_rep rep;
> 	void __user *data;
> 	int ret;
>+#endif
> };
> 
> static void i915_dereference_buffers_locked(struct drm_i915_validate_buffer *buffers,
>@@ -834,10 +840,6 @@ int i915_apply_reloc(struct drm_file *fi
> 	int ret, i;
> 	int buf_index = -1;
> 
>-	/*
>-	 * FIXME: O(relocs * buffers) complexity.
>-	 */
>-
> 	for (i = 0; i <= num_buffers; i++)
> 		if (buffers[i].buffer)
> 			if (reloc[2] == buffers[i].buffer->base.hash.key)
>@@ -859,22 +861,21 @@ int i915_apply_reloc(struct drm_file *fi
> 	if (!relocatee->data_page ||
> 	    !drm_bo_same_page(relocatee->offset, new_cmd_offset)) {
> 		drm_bo_kunmap(&relocatee->kmap);
>-		relocatee->data_page = NULL;
> 		relocatee->offset = new_cmd_offset;
>-		
>-		if (unlikely(!relocatee->idle)) {
>-			ret = drm_bo_wait(relocatee->buf, 0, 0, 0);
>-			if (ret)
>-				return ret;
>-			relocatee->idle = 1;
>+		mutex_lock (&relocatee->buf->mutex);
>+		ret = drm_bo_wait (relocatee->buf, 0, 0, FALSE);
>+		mutex_unlock (&relocatee->buf->mutex);
>+		if (ret) {
>+			DRM_ERROR("Could not wait for buffer to apply relocs\n %08lx", new_cmd_offset);
>+			return ret;
> 		}
>-
> 		ret = drm_bo_kmap(relocatee->buf, new_cmd_offset >> PAGE_SHIFT,
> 				  1, &relocatee->kmap);
> 		if (ret) {
> 			DRM_ERROR("Could not map command buffer to apply relocs\n %08lx", new_cmd_offset);
> 			return ret;
> 		}
>+
> 		relocatee->data_page = drm_bmo_virtual(&relocatee->kmap,
> 						       &relocatee->is_iomem);
> 		relocatee->page_offset = (relocatee->offset & PAGE_MASK);
>@@ -893,11 +894,7 @@ int i915_apply_reloc(struct drm_file *fi
> 				   reloc[0], reloc[1], buf_index, relocatee->data_page[index], val);
> 		}
> 	}
>-
>-	if (relocatee->is_iomem)
>-		iowrite32(val, relocatee->data_page + index);
>-	else
>-		relocatee->data_page[index] = val;
>+	relocatee->data_page[index] = val;
> 	return 0;
> }
> 
>@@ -965,13 +962,11 @@ int i915_process_relocs(struct drm_file 
> 	}
> 
> out:
>+
> 	if (reloc_buf)
> 		kfree(reloc_buf);
>-
>-	if (relocatee->data_page) {		
>-		drm_bo_kunmap(&relocatee->kmap);
>-		relocatee->data_page = NULL;
>-	}
>+	drm_bo_kunmap(&relocatee->kmap);
>+	relocatee->data_page = NULL;
> 
> 	return ret;
> }
>@@ -1011,22 +1006,23 @@ static int i915_exec_reloc(struct drm_fi
> 		goto out_err;
> 	}
> 
>-	mutex_lock (&relocatee.buf->mutex);
> 	while (reloc_user_ptr) {
> 		ret = i915_process_relocs(file_priv, buf_handle, &reloc_user_ptr, &relocatee, buffers, buf_count);
> 		if (ret) {
> 			DRM_ERROR("process relocs failed\n");
>-			goto out_err1;
>+			break;
> 		}
> 	}
> 
>-out_err1:
>-	mutex_unlock (&relocatee.buf->mutex);
>-	drm_bo_usage_deref_unlocked(&relocatee.buf);
>+	mutex_lock(&dev->struct_mutex);
>+	drm_bo_usage_deref_locked(&relocatee.buf);
>+	mutex_unlock(&dev->struct_mutex);
>+
> out_err:
> 	return ret;
> }
> 
>+#ifdef FIX3
> static int i915_check_presumed(struct drm_i915_op_arg *arg,
> 			       struct drm_buffer_object *bo,
> 			       uint32_t __user *data,
>@@ -1059,6 +1055,7 @@ static int i915_check_presumed(struct dr
> 	hint &= ~DRM_BO_HINT_PRESUMED_OFFSET;
> 	return __put_user(hint, data + hint_offset);
> }
>+#endif
> 
> 
> /*
>@@ -1071,11 +1068,16 @@ int i915_validate_buffer_list(struct drm
> {
> 	struct drm_i915_op_arg arg;
> 	struct drm_bo_op_req *req = &arg.d.req;
>+	struct drm_bo_arg_rep rep;
>+	unsigned long next = 0;
> 	int ret = 0;
> 	unsigned buf_count = 0;
>+	struct drm_device *dev = file_priv->head->dev;
> 	uint32_t buf_handle;
> 	uint32_t __user *reloc_user_ptr;
>+#ifdef FIX2
> 	struct drm_i915_validate_buffer *item = buffers;
>+#endif
> 
> 	do {
> 		if (buf_count >= *num_buffers) {
>@@ -1083,26 +1085,41 @@ int i915_validate_buffer_list(struct drm
> 			ret = -EINVAL;
> 			goto out_err;
> 		}
>-		item = buffers + buf_count;
>-		item->buffer = NULL;
>-		item->presumed_offset_correct = 0;
>-
>+#ifdef FIX2
>+		item = buffers + buf_count;	
>+#endif
> 		buffers[buf_count].buffer = NULL;
>+		buffers[buf_count].presumed_offset_correct = 0;
>+
> 
> 		if (copy_from_user(&arg, (void __user *)(unsigned long)data, sizeof(arg))) {
> 			ret = -EFAULT;
> 			goto out_err;
> 		}
>+#ifndef FIX2
>+		if (arg.handled) {
>+			data = arg.next;
>+			mutex_lock(&dev->struct_mutex);
>+			buffers[buf_count].buffer = drm_lookup_buffer_object(file_priv, req->arg_handle, 1);
>+			mutex_unlock(&dev->struct_mutex);
>+			buf_count++;
>+			continue;
>+		}
> 
>+		rep.ret = 0;
>+#else
> 		ret = 0;
>+#endif
> 		if (req->op != drm_bo_validate) {
> 			DRM_ERROR
> 			    ("Buffer object operation wasn't \"validate\".\n");
>+#ifdef FIX2
> 			ret = -EINVAL;
>+#else
>+			rep.ret = -EINVAL;
>+#endif
> 			goto out_err;
> 		}
>-		item->ret = 0;
>-		item->data = (void __user *) (unsigned long) data;
> 
> 		buf_handle = req->bo_req.handle;
> 		reloc_user_ptr = (uint32_t *)(unsigned long)arg.reloc_ptr;
>@@ -1114,45 +1131,89 @@ int i915_validate_buffer_list(struct drm
> 			DRM_MEMORYBARRIER();
> 		}
> 
>+#ifdef FIX2
>+	        item->ret = 0;
>+	        item->data = (void __user *) (unsigned long) data;
> 		ret = drm_bo_handle_validate(file_priv, req->bo_req.handle,
> 					     req->bo_req.flags, req->bo_req.mask,
> 					     req->bo_req.hint,
> 					     req->bo_req.fence_class, 0,
>-					     &item->rep,
>-					     &item->buffer);
>+					     &buffers[buf_count].rep,
>+					     &buffers[buf_count].buffer);
> 
> 		if (ret) {
> 			DRM_ERROR("error on handle validate %d\n", ret);
> 			goto out_err;
> 		}
>+#else
>+		rep.ret = drm_bo_handle_validate(file_priv, req->bo_req.handle,
>+						 req->bo_req.flags, req->bo_req.mask,
>+						 req->bo_req.hint,
>+						 req->bo_req.fence_class, 0,
>+						 &rep.bo_info,
>+						 &buffers[buf_count].buffer);
>+
>+		if (rep.ret) {
>+			DRM_ERROR("error on handle validate %d\n", rep.ret);
>+			goto out_err;
>+		}
>+#endif		
>+		
>+		/*
>+		 * If the user provided a presumed offset hint, check whether
>+		 * the buffer is in the same place, if so, relocations relative to
>+		 * this buffer need not be performed
>+		 */
> 
>-		buf_count++;
> 
>+
>+#ifdef FIX3		
> 		ret = i915_check_presumed(&arg, item->buffer,
> 					  (uint32_t __user *)
> 					  (unsigned long) data,
> 					  &item->presumed_offset_correct);
> 		if (ret)
> 			goto out_err;
>+#else
>+
>+		if ((req->bo_req.hint & DRM_BO_HINT_PRESUMED_OFFSET) &&
>+		    buffers[buf_count].buffer->offset == req->bo_req.presumed_offset) {
>+			buffers[buf_count].presumed_offset_correct = 1;
>+		}
>+#endif
>+
> 
>-		data = arg.next;
>+#ifndef FIX2
>+		next = arg.next;
>+		arg.handled = 1;
>+		arg.d.rep = rep;
>+		if (copy_to_user((void __user *)(unsigned long)data, &arg, sizeof(arg)))
>+			return -EFAULT;
>+
>+		data = next;
>+		buf_count++;
>+	} while (next != 0);
>+	*num_buffers = buf_count;
>+	return 0;
>+out_err:
>+	mutex_lock(&dev->struct_mutex);
>+	i915_dereference_buffers_locked(buffers, buf_count);
>+	mutex_unlock(&dev->struct_mutex);
>+	*num_buffers = 0;
>+	return (ret) ? ret : rep.ret;
>+}
>+#else
>+                data = arg.next;
>+		buf_count++;
> 	} while (data != 0);
> out_err:
> 	*num_buffers = buf_count;
> 	item->ret = (ret != -EAGAIN) ? ret : 0;
> 	return ret;
> }
>+#endif
> 
>-
>-/*
>- * Remove all buffers from the unfenced list.
>- * If the execbuffer operation was aborted, for example due to a signal,
>- * this also make sure that buffers retain their original state and
>- * fence pointers.
>- * Copy back buffer information to user-space unless we were interrupted
>- * by a signal. In which case the IOCTL must be rerun.
>- */
>-
>+#ifdef FIX2
> static int i915_handle_copyback(struct drm_device *dev,
> 				struct drm_i915_validate_buffer *buffers,
> 				unsigned int num_buffers, int ret)
>@@ -1174,9 +1235,10 @@ static int i915_handle_copyback(struct d
> 			buffers++;
> 		}
> 	}
>-
> 	return err;
> }
>+#endif
>+#ifdef FIX1
> 
> /*
>  * Create a fence object, and if that fails, pretend that everything is
>@@ -1252,7 +1314,7 @@ void i915_fence_or_sync(struct drm_file 
> 	else if (fence)
> 		drm_fence_usage_deref_unlocked(&fence);
> }
>-
>+#endif
> 
> static int i915_execbuffer(struct drm_device *dev, void *data,
> 			   struct drm_file *file_priv)
>@@ -1266,6 +1328,7 @@ static int i915_execbuffer(struct drm_de
> 	int num_buffers;
> 	int ret;
> 	struct drm_i915_validate_buffer *buffers;
>+	struct drm_fence_object *fence;
> 
> 	if (!dev_priv->allow_batchbuffer) {
> 		DRM_ERROR("Batchbuffer ioctl disabled\n");
>@@ -1309,8 +1372,13 @@ static int i915_execbuffer(struct drm_de
> 	/* validate buffer list + fixup relocations */
> 	ret = i915_validate_buffer_list(file_priv, 0, exec_buf->ops_list,
> 					buffers, &num_buffers);
>+#ifndef FIX2
>+	if (ret)
>+		goto out_free;
>+#else
> 	if (ret)
> 		goto out_err0;
>+#endif
> 
> 	/* make sure all previous memory operations have passed */
> 	DRM_MEMORYBARRIER();
>@@ -1329,16 +1397,36 @@ static int i915_execbuffer(struct drm_de
> 	if (sarea_priv)
> 		sarea_priv->last_dispatch = READ_BREADCRUMB(dev_priv);
> 
>-	i915_fence_or_sync(file_priv, fence_arg->flags, fence_arg, NULL);
>+#ifndef FIX1
>+	/* fence */
>+	ret = drm_fence_buffer_objects(dev, NULL, fence_arg->flags, 
>+				       NULL, &fence);
>+	if (ret)
>+		goto out_err0;
> 
>+	if (!(fence_arg->flags & DRM_FENCE_FLAG_NO_USER)) {
>+		ret = drm_fence_add_user_object(file_priv, fence, fence_arg->flags & DRM_FENCE_FLAG_SHAREABLE);
>+		if (!ret) {
>+			fence_arg->handle = fence->base.hash.key;
>+			fence_arg->fence_class = fence->fence_class;
>+			fence_arg->type = fence->type;
>+			fence_arg->signaled = fence->signaled_types;
>+		}
>+	}
>+	drm_fence_usage_deref_unlocked(&fence);
>+#else
>+	i915_fence_or_sync(file_priv, fence_arg->flags, fence_arg, NULL);
>+#endif
> out_err0:
>-
>-	/* handle errors */
>+#ifdef FIX2
> 	ret = i915_handle_copyback(dev, buffers, num_buffers, ret);
>+#endif
>+	/* handle errors */
> 	mutex_lock(&dev->struct_mutex);
> 	i915_dereference_buffers_locked(buffers, num_buffers);
> 	mutex_unlock(&dev->struct_mutex);
> 
>+out_free:
> 	drm_free(buffers, (exec_buf->num_buffers * sizeof(struct drm_buffer_object *)), DRM_MEM_DRIVER);
> 
> 	mutex_unlock(&dev_priv->cmdbuf_mutex);

Actions: View | Splinter Review

Attachments on bug 14696: 14609 | 14610 | 14634 | 14635 | 14639 | 14734

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.