[Intel-gfx] [PATCH] Correct alignment for framebuffer

Jesse Barnes jbarnes at virtuousgeek.org
Tue Feb 3 19:08:48 CET 2009


On Tuesday, February 3, 2009 9:50 am Jesse Barnes wrote:
> On Tuesday, February 3, 2009 8:29 am Chris Wilson wrote:
> > Hello all,
> >
> > intel_pipe_set_base() sets an invalid alignment for I915_TILING_X on an
> > i915, triggering the warning "i915_write_fence_reg: object 0x00a00000
> > not 1M or size (0x400000) aligned".
> >
> > The attached patch uses i915_gem_get_gtt_alignment() to compute the
> > correct alignment for a fenced object.
> >
> > Thanks.
> > -ickle
>
> Looks good, thanks Chris.  Eric can you pick this up?
>
> Acked-by: Jesse Barnes <jbarnes at virtuousgeek.org>

Spoke too soon.  How about we just remove alignment and let the core handle
it.  No more confusing "alignment" parameters to calculate & pass everywhere
(or unused 0s everywhere).

Remove the unused "alignment" parameter to the various pinning & binding
functions, since we should always calculate it at bind time anyway.

Signed-off-by: Jesse Barnes <jbarnes at virtuousgeek.org>


diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a70bf77..8a6eaa1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -600,7 +600,7 @@ int i915_gem_proc_init(struct drm_minor *minor);
 void i915_gem_proc_cleanup(struct drm_minor *minor);
 int i915_gem_init_object(struct drm_gem_object *obj);
 void i915_gem_free_object(struct drm_gem_object *obj);
-int i915_gem_object_pin(struct drm_gem_object *obj, uint32_t alignment);
+int i915_gem_object_pin(struct drm_gem_object *obj);
 void i915_gem_object_unpin(struct drm_gem_object *obj);
 int i915_gem_object_unbind(struct drm_gem_object *obj);
 void i915_gem_lastclose(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 6a9e3a8..dd2f014 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -50,8 +50,7 @@ static void 
i915_gem_object_set_to_full_cpu_read_domain(struct drm_gem_object *o
 static int i915_gem_object_get_page_list(struct drm_gem_object *obj);
 static void i915_gem_object_free_page_list(struct drm_gem_object *obj);
 static int i915_gem_object_wait_rendering(struct drm_gem_object *obj);
-static int i915_gem_object_bind_to_gtt(struct drm_gem_object *obj,
-					   unsigned alignment);
+static int i915_gem_object_bind_to_gtt(struct drm_gem_object *obj);
 static int i915_gem_object_get_fence_reg(struct drm_gem_object *obj, bool 
write);
 static void i915_gem_clear_fence_reg(struct drm_gem_object *obj);
 static int i915_gem_evict_something(struct drm_device *dev);
@@ -266,7 +265,7 @@ i915_gem_gtt_pwrite(struct drm_device *dev, struct 
drm_gem_object *obj,
 
 
 	mutex_lock(&dev->struct_mutex);
-	ret = i915_gem_object_pin(obj, 0);
+	ret = i915_gem_object_pin(obj);
 	if (ret) {
 		mutex_unlock(&dev->struct_mutex);
 		return ret;
@@ -576,7 +575,7 @@ int i915_gem_fault(struct vm_area_struct *vma, struct 
vm_fault *vmf)
 	/* Now bind it into the GTT if needed */
 	mutex_lock(&dev->struct_mutex);
 	if (!obj_priv->gtt_space) {
-		ret = i915_gem_object_bind_to_gtt(obj, obj_priv->gtt_alignment);
+		ret = i915_gem_object_bind_to_gtt(obj);
 		if (ret) {
 			mutex_unlock(&dev->struct_mutex);
 			return VM_FAULT_SIGBUS;
@@ -777,7 +776,7 @@ i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void 
*data,
 	 * initial fault faster and any subsequent flushing possible).
 	 */
 	if (!obj_priv->agp_mem) {
-		ret = i915_gem_object_bind_to_gtt(obj, obj_priv->gtt_alignment);
+		ret = i915_gem_object_bind_to_gtt(obj);
 		if (ret) {
 			drm_gem_object_unreference(obj);
 			mutex_unlock(&dev->struct_mutex);
@@ -906,7 +905,9 @@ i915_add_request(struct drm_device *dev, uint32_t 
flush_domains)
 	OUT_RING(MI_USER_INTERRUPT);
 	ADVANCE_LP_RING();
 
+#if WATCH_BUF
 	DRM_DEBUG("%d\n", seqno);
+#endif
 
 	request->seqno = seqno;
 	request->emitted_jiffies = jiffies;
@@ -1640,22 +1641,19 @@ i915_gem_clear_fence_reg(struct drm_gem_object *obj)
  * Finds free space in the GTT aperture and binds the object there.
  */
 static int
-i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment)
+i915_gem_object_bind_to_gtt(struct drm_gem_object *obj)
 {
 	struct drm_device *dev = obj->dev;
 	drm_i915_private_t *dev_priv = dev->dev_private;
 	struct drm_i915_gem_object *obj_priv = obj->driver_private;
 	struct drm_mm_node *free_space;
 	int page_count, ret;
+	unsigned int alignment;
 
 	if (dev_priv->mm.suspended)
 		return -EBUSY;
-	if (alignment == 0)
-		alignment = i915_gem_get_gtt_alignment(obj);
-	if (alignment & (PAGE_SIZE - 1)) {
-		DRM_ERROR("Invalid object alignment requested %u\n", alignment);
-		return -EINVAL;
-	}
+
+	alignment = i915_gem_get_gtt_alignment(obj);
 
  search_free:
 	free_space = drm_mm_search_free(&dev_priv->mm.gtt_space,
@@ -2179,7 +2177,7 @@ i915_gem_object_pin_and_relocate(struct drm_gem_object 
*obj,
 	void __iomem *reloc_page;
 
 	/* Choose the GTT offset for our buffer and put it there. */
-	ret = i915_gem_object_pin(obj, (uint32_t) entry->alignment);
+	ret = i915_gem_object_pin(obj);
 	if (ret)
 		return ret;
 
@@ -2658,7 +2656,7 @@ pre_mutex_err:
 }
 
 int
-i915_gem_object_pin(struct drm_gem_object *obj, uint32_t alignment)
+i915_gem_object_pin(struct drm_gem_object *obj)
 {
 	struct drm_device *dev = obj->dev;
 	struct drm_i915_gem_object *obj_priv = obj->driver_private;
@@ -2666,7 +2664,7 @@ i915_gem_object_pin(struct drm_gem_object *obj, uint32_t 
alignment)
 
 	i915_verify_inactive(dev, __FILE__, __LINE__);
 	if (obj_priv->gtt_space == NULL) {
-		ret = i915_gem_object_bind_to_gtt(obj, alignment);
+		ret = i915_gem_object_bind_to_gtt(obj);
 		if (ret != 0) {
 			if (ret != -EBUSY && ret != -ERESTARTSYS)
 				DRM_ERROR("Failure to bind: %d", ret);
@@ -2758,7 +2756,7 @@ i915_gem_pin_ioctl(struct drm_device *dev, void *data,
 	obj_priv->user_pin_count++;
 	obj_priv->pin_filp = file_priv;
 	if (obj_priv->user_pin_count == 1) {
-		ret = i915_gem_object_pin(obj, args->alignment);
+		ret = i915_gem_object_pin(obj);
 		if (ret != 0) {
 			drm_gem_object_unreference(obj);
 			mutex_unlock(&dev->struct_mutex);
@@ -3081,7 +3079,7 @@ i915_gem_init_hws(struct drm_device *dev)
 	obj_priv = obj->driver_private;
 	obj_priv->agp_type = AGP_USER_CACHED_MEMORY;
 
-	ret = i915_gem_object_pin(obj, 4096);
+	ret = i915_gem_object_pin(obj);
 	if (ret != 0) {
 		drm_gem_object_unreference(obj);
 		return ret;
@@ -3126,7 +3124,7 @@ i915_gem_init_ringbuffer(struct drm_device *dev)
 	}
 	obj_priv = obj->driver_private;
 
-	ret = i915_gem_object_pin(obj, 4096);
+	ret = i915_gem_object_pin(obj);
 	if (ret != 0) {
 		drm_gem_object_unreference(obj);
 		return ret;
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index bbdd729..d01c489 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -360,7 +360,7 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
 	int dspsurf = (pipe == 0 ? DSPASURF : DSPBSURF);
 	int dspstride = (pipe == 0) ? DSPASTRIDE : DSPBSTRIDE;
 	int dspcntr_reg = (pipe == 0) ? DSPACNTR : DSPBCNTR;
-	u32 dspcntr, alignment;
+	u32 dspcntr;
 
 	/* no fb bound */
 	if (!crtc->fb) {
@@ -372,25 +372,7 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
 	obj = intel_fb->obj;
 	obj_priv = obj->driver_private;
 
-	switch (obj_priv->tiling_mode) {
-	case I915_TILING_NONE:
-		alignment = 64 * 1024;
-		break;
-	case I915_TILING_X:
-		if (IS_I9XX(dev))
-			alignment = 1024 * 1024;
-		else
-			alignment = 512 * 1024;
-		break;
-	case I915_TILING_Y:
-		/* FIXME: Is this true? */
-		DRM_ERROR("Y tiled not allowed for scan out buffers\n");
-		return;
-	default:
-		BUG();
-	}
-
-	if (i915_gem_object_pin(intel_fb->obj, alignment))
+	if (i915_gem_object_pin(intel_fb->obj))
 		return;
 
 	i915_gem_object_set_to_gtt_domain(intel_fb->obj, 1);
@@ -1024,7 +1006,7 @@ static int intel_crtc_cursor_set(struct drm_crtc *crtc,
 
 	/* we only need to pin inside GTT if cursor is non-phy */
 	if (!dev_priv->cursor_needs_physical) {
-		ret = i915_gem_object_pin(bo, PAGE_SIZE);
+		ret = i915_gem_object_pin(bo);
 		if (ret) {
 			DRM_ERROR("failed to pin cursor bo\n");
 			goto fail;
diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index afd1217..526b0bb 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -461,7 +461,7 @@ static int intelfb_create(struct drm_device *dev, uint32_t 
fb_width,
 
 	mutex_lock(&dev->struct_mutex);
 
-	ret = i915_gem_object_pin(fbo, PAGE_SIZE);
+	ret = i915_gem_object_pin(fbo);
 	if (ret) {
 		DRM_ERROR("failed to pin fb: %d\n", ret);
 		goto out_unref;
diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
index 912cd52..560fd4f 100644
--- a/include/drm/i915_drm.h
+++ b/include/drm/i915_drm.h
@@ -562,7 +562,7 @@ struct drm_i915_gem_pin {
 	uint32_t handle;
 	uint32_t pad;
 
-	/** alignment required within the aperture */
+	/** Ignored, the kernel now calculates this */
 	uint64_t alignment;
 
 	/** Returned GTT offset of the buffer. */



More information about the Intel-gfx mailing list