Re: [CI 4/5] drm/i915/guc: kill the GuC client

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

 



<snip>

@@ -220,7 +200,7 @@ static void guc_wq_item_append(struct intel_guc_client *client,
  	GEM_BUG_ON(wq_off & (wqi_size - 1));
/* WQ starts from the page after process_desc */

Just realized that I stupidly forgot to actually commit the fix...

I'll send the fixed (for real) version after the bugfix for bug 112205 is merged, so I can get a clean CI run.

Daniele

-	wqi = client->vaddr + wq_off + GUC_PD_SIZE;
+	wqi = guc->workqueue_vaddr + wq_off;
/* Now fill in the 4-word work queue item */
  	wqi->header = WQ_TYPE_INORDER |
@@ -238,12 +218,11 @@ static void guc_wq_item_append(struct intel_guc_client *client,
static void guc_add_request(struct intel_guc *guc, struct i915_request *rq)
  {
-	struct intel_guc_client *client = guc->execbuf_client;
  	struct intel_engine_cs *engine = rq->engine;
  	u32 ctx_desc = lower_32_bits(rq->hw_context->lrc_desc);
  	u32 ring_tail = intel_ring_set_tail(rq->ring, rq->tail) / sizeof(u64);
- guc_wq_item_append(client, engine->guc_id, ctx_desc,
+	guc_wq_item_append(guc, engine->guc_id, ctx_desc,
  			   ring_tail, rq->fence.seqno);
  }
@@ -267,9 +246,8 @@ static void guc_submit(struct intel_engine_cs *engine,
  		       struct i915_request **end)
  {
  	struct intel_guc *guc = &engine->gt->uc.guc;
-	struct intel_guc_client *client = guc->execbuf_client;
- spin_lock(&client->wq_lock);
+	spin_lock(&guc->wq_lock);
do {
  		struct i915_request *rq = *out++;
@@ -278,7 +256,7 @@ static void guc_submit(struct intel_engine_cs *engine,
  		guc_add_request(guc, rq);
  	} while (out != end);
- spin_unlock(&client->wq_lock);
+	spin_unlock(&guc->wq_lock);
  }
static inline int rq_prio(const struct i915_request *rq)
@@ -529,126 +507,6 @@ static void guc_reset_finish(struct intel_engine_cs *engine)
   * path of guc_submit() above.
   */
-/**
- * guc_client_alloc() - Allocate an intel_guc_client
- * @guc:	the intel_guc structure
- * @priority:	four levels priority _CRITICAL, _HIGH, _NORMAL and _LOW
- *		The kernel client to replace ExecList submission is created with
- *		NORMAL priority. Priority of a client for scheduler can be HIGH,
- *		while a preemption context can use CRITICAL.
- *
- * Return:	An intel_guc_client object if success, else NULL.
- */
-static struct intel_guc_client *
-guc_client_alloc(struct intel_guc *guc, u32 priority)
-{
-	struct intel_guc_client *client;
-	struct i915_vma *vma;
-	void *vaddr;
-	int ret;
-
-	client = kzalloc(sizeof(*client), GFP_KERNEL);
-	if (!client)
-		return ERR_PTR(-ENOMEM);
-
-	client->guc = guc;
-	client->priority = priority;
-	spin_lock_init(&client->wq_lock);
-
-	ret = ida_simple_get(&guc->stage_ids, 0, GUC_MAX_STAGE_DESCRIPTORS,
-			     GFP_KERNEL);
-	if (ret < 0)
-		goto err_client;
-
-	client->stage_id = ret;
-
-	/* The first page is proc_desc. Two following pages are wq. */
-	vma = intel_guc_allocate_vma(guc, GUC_PD_SIZE + GUC_WQ_SIZE);
-	if (IS_ERR(vma)) {
-		ret = PTR_ERR(vma);
-		goto err_id;
-	}
-
-	/* We'll keep just the first (doorbell/proc) page permanently kmap'd. */
-	client->vma = vma;
-
-	vaddr = i915_gem_object_pin_map(vma->obj, I915_MAP_WB);
-	if (IS_ERR(vaddr)) {
-		ret = PTR_ERR(vaddr);
-		goto err_vma;
-	}
-	client->vaddr = vaddr;
-
-	DRM_DEBUG_DRIVER("new priority %u client %p: stage_id %u\n",
-			 priority, client, client->stage_id);
-
-	return client;
-
-err_vma:
-	i915_vma_unpin_and_release(&client->vma, 0);
-err_id:
-	ida_simple_remove(&guc->stage_ids, client->stage_id);
-err_client:
-	kfree(client);
-	return ERR_PTR(ret);
-}
-
-static void guc_client_free(struct intel_guc_client *client)
-{
-	i915_vma_unpin_and_release(&client->vma, I915_VMA_RELEASE_MAP);
-	ida_simple_remove(&client->guc->stage_ids, client->stage_id);
-	kfree(client);
-}
-
-static int guc_clients_create(struct intel_guc *guc)
-{
-	struct intel_guc_client *client;
-
-	GEM_BUG_ON(guc->execbuf_client);
-
-	client = guc_client_alloc(guc, GUC_CLIENT_PRIORITY_KMD_NORMAL);
-	if (IS_ERR(client)) {
-		DRM_ERROR("Failed to create GuC client for submission!\n");
-		return PTR_ERR(client);
-	}
-	guc->execbuf_client = client;
-
-	return 0;
-}
-
-static void guc_clients_destroy(struct intel_guc *guc)
-{
-	struct intel_guc_client *client;
-
-	client = fetch_and_zero(&guc->execbuf_client);
-	if (client)
-		guc_client_free(client);
-}
-
-static void __guc_client_enable(struct intel_guc_client *client)
-{
-	guc_proc_desc_init(client);
-	guc_stage_desc_init(client);
-}
-
-static void __guc_client_disable(struct intel_guc_client *client)
-{
-	/* Note: By the time we're here, GuC may have already been reset */
-	guc_stage_desc_fini(client);
-	guc_proc_desc_fini(client);
-}
-
-static void guc_clients_enable(struct intel_guc *guc)
-{
-	__guc_client_enable(guc->execbuf_client);
-}
-
-static void guc_clients_disable(struct intel_guc *guc)
-{
-	if (guc->execbuf_client)
-		__guc_client_disable(guc->execbuf_client);
-}
-
  /*
   * Set up the memory resources to be shared with the GuC (via the GGTT)
   * at firmware loading time.
@@ -669,12 +527,20 @@ int intel_guc_submission_init(struct intel_guc *guc)
  	 */
  	GEM_BUG_ON(!guc->stage_desc_pool);
- ret = guc_clients_create(guc);
+	ret = guc_workqueue_create(guc);
  	if (ret)
  		goto err_pool;
+ ret = guc_proc_desc_create(guc);
+	if (ret)
+		goto err_workqueue;
+
+	spin_lock_init(&guc->wq_lock);
+
  	return 0;
+err_workqueue:
+	guc_workqueue_destroy(guc);
  err_pool:
  	guc_stage_desc_pool_destroy(guc);
  	return ret;
@@ -682,10 +548,11 @@ int intel_guc_submission_init(struct intel_guc *guc)
void intel_guc_submission_fini(struct intel_guc *guc)
  {
-	guc_clients_destroy(guc);
-
-	if (guc->stage_desc_pool)
+	if (guc->stage_desc_pool) {
+		guc_proc_desc_destroy(guc);
+		guc_workqueue_destroy(guc);
  		guc_stage_desc_pool_destroy(guc);
+	}
  }
static void guc_interrupts_capture(struct intel_gt *gt)
@@ -771,9 +638,8 @@ void intel_guc_submission_enable(struct intel_guc *guc)
  		     sizeof(struct guc_wq_item) *
  		     I915_NUM_ENGINES > GUC_WQ_SIZE);
- GEM_BUG_ON(!guc->execbuf_client);
-
-	guc_clients_enable(guc);
+	guc_proc_desc_init(guc);
+	guc_stage_desc_init(guc);
/* Take over from manual control of ELSP (execlists) */
  	guc_interrupts_capture(gt);
@@ -790,8 +656,12 @@ void intel_guc_submission_disable(struct intel_guc *guc)
GEM_BUG_ON(gt->awake); /* GT should be parked first */ + /* Note: By the time we're here, GuC may have already been reset */
+
  	guc_interrupts_release(gt);
-	guc_clients_disable(guc);
+
+	guc_stage_desc_fini(guc);
+	guc_proc_desc_fini(guc);
  }
static bool __guc_submission_support(struct intel_guc *guc)
@@ -809,3 +679,8 @@ void intel_guc_submission_init_early(struct intel_guc *guc)
  {
  	guc->submission_supported = __guc_submission_support(guc);
  }
+
+bool intel_engine_in_guc_submission_mode(const struct intel_engine_cs *engine)
+{
+	return engine->set_default_submission == guc_set_default_submission;
+}
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
index e2deb4e6f42a..e402a2932592 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.h
@@ -6,48 +6,10 @@
  #ifndef _INTEL_GUC_SUBMISSION_H_
  #define _INTEL_GUC_SUBMISSION_H_
-#include <linux/spinlock.h>
+#include <linux/types.h>
-#include "gt/intel_engine_types.h"
-
-#include "i915_gem.h"
-#include "i915_selftest.h"
-
-struct drm_i915_private;
-
-/*
- * This structure primarily describes the GEM object shared with the GuC.
- * The specs sometimes refer to this object as a "GuC context", but we use
- * the term "client" to avoid confusion with hardware contexts. This
- * GEM object is held for the entire lifetime of our interaction with
- * the GuC, being allocated before the GuC is loaded with its firmware.
- * Because there's no way to update the address used by the GuC after
- * initialisation, the shared object must stay pinned into the GGTT as
- * long as the GuC is in use. We also keep the first page (only) mapped
- * into kernel address space, as it includes shared data that must be
- * updated on every request submission.
- *
- * The single GEM object described here is actually made up of several
- * separate areas, as far as the GuC is concerned. The first page (kept
- * kmap'd) includes the "process descriptor" which holds sequence data for
- * the doorbell, and one cacheline which actually *is* the doorbell; a
- * write to this will "ring the doorbell" (i.e. send an interrupt to the
- * GuC). The subsequent  pages of the client object constitute the work
- * queue (a circular array of work items), again described in the process
- * descriptor. Work queue pages are mapped momentarily as required.
- */
-struct intel_guc_client {
-	struct i915_vma *vma;
-	void *vaddr;
-	struct intel_guc *guc;
-
-	/* bitmap of (host) engine ids */
-	u32 priority;
-	u32 stage_id;
-
-	/* Protects GuC client's WQ access */
-	spinlock_t wq_lock;
-};
+struct intel_guc;
+struct intel_engine_cs;
void intel_guc_submission_init_early(struct intel_guc *guc);
  int intel_guc_submission_init(struct intel_guc *guc);
@@ -56,5 +18,6 @@ void intel_guc_submission_disable(struct intel_guc *guc);
  void intel_guc_submission_fini(struct intel_guc *guc);
  int intel_guc_preempt_work_create(struct intel_guc *guc);
  void intel_guc_preempt_work_destroy(struct intel_guc *guc);
+bool intel_engine_in_guc_submission_mode(const struct intel_engine_cs *engine);
#endif
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 5d5974e7f3ed..f32e7b016197 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1802,23 +1802,12 @@ static void i915_guc_log_info(struct seq_file *m,
  static int i915_guc_info(struct seq_file *m, void *data)
  {
  	struct drm_i915_private *dev_priv = node_to_i915(m->private);
-	const struct intel_guc *guc = &dev_priv->gt.uc.guc;
-	struct intel_guc_client *client = guc->execbuf_client;
if (!USES_GUC(dev_priv))
  		return -ENODEV;
i915_guc_log_info(m, dev_priv); - if (!USES_GUC_SUBMISSION(dev_priv))
-		return 0;
-
-	GEM_BUG_ON(!guc->execbuf_client);
-
-	seq_printf(m, "\nGuC execbuf client @ %p:\n", client);
-	seq_printf(m, "\tPriority %d, GuC stage index: %u\n",
-		   client->priority,
-		   client->stage_id);
  	/* Add more as required ... */
return 0;

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




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux