Re: [PATCH v4 1/6] drm/i915/gen8: Add infrastructure to initialize WA batch buffers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 16/06/2015 21:33, Chris Wilson wrote:
On Tue, Jun 16, 2015 at 08:25:20PM +0100, Arun Siluvery wrote:
+static int lrc_setup_wa_ctx_obj(struct intel_engine_cs *ring, u32 size)
+{
+	int ret;
+	struct drm_device *dev = ring->dev;

You only use it once, keeping it as a local seems counter-intuitive.

+	WARN_ON(ring->id != RCS);
+
+	size = roundup(size, PAGE_SIZE);

Out of curiousity is gcc smart enough to turn this into an ALIGN()?

replaced with PAGE_ALIGN(size)


+	ring->wa_ctx.obj = i915_gem_alloc_object(dev, size);
+	if (!ring->wa_ctx.obj) {
+		DRM_DEBUG_DRIVER("alloc LRC WA ctx backing obj failed.\n");
+		return -ENOMEM;
+	}
+
+	ret = i915_gem_obj_ggtt_pin(ring->wa_ctx.obj, GEN8_LR_CONTEXT_ALIGN, 0);

Strange choice of alignment since we pass around cacheline offsets.

this is from the initial version where it was part of context, sorry missed this, replaced with PAGE_SIZE.

+	if (ret) {
+		DRM_DEBUG_DRIVER("pin LRC WA ctx backing obj failed: %d\n",
+				 ret);
+		drm_gem_object_unreference(&ring->wa_ctx.obj->base);
+		return ret;
+	}
+
+	return 0;
+}
+
+static void lrc_destroy_wa_ctx_obj(struct intel_engine_cs *ring)
+{
+	WARN_ON(ring->id != RCS);
+
+	i915_gem_object_ggtt_unpin(ring->wa_ctx.obj);
+	drm_gem_object_unreference(&ring->wa_ctx.obj->base);
+	ring->wa_ctx.obj = NULL;
+}
+
  /**
   * intel_logical_ring_cleanup() - deallocate the Engine Command Streamer
   *
@@ -1474,7 +1612,29 @@ static int logical_render_ring_init(struct drm_device *dev)
  	if (ret)
  		return ret;

-	return intel_init_pipe_control(ring);
+	if (INTEL_INFO(ring->dev)->gen >= 8) {
+		ret = lrc_setup_wa_ctx_obj(ring, PAGE_SIZE);
+		if (ret) {
+			DRM_DEBUG_DRIVER("Failed to setup context WA page: %d\n",
+					 ret);
+			return ret;
+		}
+
+		ret = intel_init_workaround_bb(ring);
+		if (ret) {
+			lrc_destroy_wa_ctx_obj(ring);
+			DRM_ERROR("WA batch buffers are not initialized: %d\n",
+				  ret);
+		}
+	}
+
+	ret = intel_init_pipe_control(ring);

Did you consider stuffing it into the spare are of the pipe control
scatch bo? :)
Not exactly but I think it is better to keep them separate. It is not that a single page is not sufficient even if we add more WA in future but for logical reasons. In case if there is any error while initializing these WA we are destroying the page and continuing further which cannot be done with scratch page.

regards
Arun


-Chris


_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux