* [Intel-gfx] [PATCH 0/5] Simplify GuC communication handling
@ 2019-12-10 21:09 Daniele Ceraolo Spurio
2019-12-10 21:09 ` [Intel-gfx] [PATCH 1/5] drm/i915/guc: Merge communication_stop and communication_disable Daniele Ceraolo Spurio
` (7 more replies)
0 siblings, 8 replies; 16+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-12-10 21:09 UTC (permalink / raw)
To: intel-gfx
Since H2G communication will be in the hot path of GuC submission,
the main aim of this series is to get rid of the function pointers to
speed things up and avoid retpolines (in case the compiler decides
they're required). While at it, simplify the general communication
enabling/disabling by removing support for multiple channels since
it is extremely unlikely that we'll ever use more than one.
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: John Harrison <John.C.Harrison@Intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Daniele Ceraolo Spurio (5):
drm/i915/guc: Merge communication_stop and communication_disable
drm/i915/guc/ct: stop expecting multiple CT channels
drm/i915/guc: remove function pointers for send/receive calls
drm/i915/guc: unify notify() functions
HAX: force enable_guc=2 and WA i915#571
drivers/gpu/drm/i915/gt/intel_gt_irq.c | 2 +-
drivers/gpu/drm/i915/gt/selftest_gt_pm.c | 9 +
drivers/gpu/drm/i915/gt/uc/intel_guc.c | 43 +--
drivers/gpu/drm/i915/gt/uc/intel_guc.h | 41 +--
drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 298 +++++++-----------
drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h | 55 ++--
drivers/gpu/drm/i915/gt/uc/intel_guc_log.c | 6 +-
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 1 -
drivers/gpu/drm/i915/gt/uc/intel_uc.c | 36 +--
drivers/gpu/drm/i915/i915_params.h | 2 +-
10 files changed, 186 insertions(+), 307 deletions(-)
--
2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Intel-gfx] [PATCH 1/5] drm/i915/guc: Merge communication_stop and communication_disable
2019-12-10 21:09 [Intel-gfx] [PATCH 0/5] Simplify GuC communication handling Daniele Ceraolo Spurio
@ 2019-12-10 21:09 ` Daniele Ceraolo Spurio
2019-12-11 13:12 ` Michal Wajdeczko
2019-12-10 21:09 ` [Intel-gfx] [PATCH 2/5] drm/i915/guc/ct: stop expecting multiple CT channels Daniele Ceraolo Spurio
` (6 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-12-10 21:09 UTC (permalink / raw)
To: intel-gfx
The only difference from the GuC POV between guc_communication_stop and
guc_communication_disable is that the former can be called after GuC
has been reset. Instead of having two separate paths, we can just skip
the call into GuC in the disabling path and re-use that.
Note that by using the disable() path instead of the stop() one there
are two additional changes in SW side for the stop path:
- interrupts are now disabled before disabling the CT, which is ok
because we do not want interrupts with CT disabled;
- guc_get_mmio_msg() is called in the stop case as well, which is ok
because if there are errors before the reset we do want to record
them.
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: John Harrison <John.C.Harrison@Intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
---
drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 14 ++++++++------
drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h | 5 -----
drivers/gpu/drm/i915/gt/uc/intel_uc.c | 18 ++----------------
3 files changed, 10 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
index 5fb7f957edf9..f74ba4750a94 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
@@ -241,12 +241,14 @@ static void ctch_disable(struct intel_guc *guc,
ctch->enabled = false;
- guc_action_deregister_ct_buffer(guc,
- ctch->owner,
- INTEL_GUC_CT_BUFFER_TYPE_SEND);
- guc_action_deregister_ct_buffer(guc,
- ctch->owner,
- INTEL_GUC_CT_BUFFER_TYPE_RECV);
+ if (intel_guc_is_running(guc)) {
+ guc_action_deregister_ct_buffer(guc,
+ ctch->owner,
+ INTEL_GUC_CT_BUFFER_TYPE_SEND);
+ guc_action_deregister_ct_buffer(guc,
+ ctch->owner,
+ INTEL_GUC_CT_BUFFER_TYPE_RECV);
+ }
}
static u32 ctch_get_next_fence(struct intel_guc_ct_channel *ctch)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
index 7c24d83f5c24..77c80d6cc25d 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
@@ -81,11 +81,6 @@ void intel_guc_ct_fini(struct intel_guc_ct *ct);
int intel_guc_ct_enable(struct intel_guc_ct *ct);
void intel_guc_ct_disable(struct intel_guc_ct *ct);
-static inline void intel_guc_ct_stop(struct intel_guc_ct *ct)
-{
- ct->host_channel.enabled = false;
-}
-
int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len,
u32 *response_buf, u32 response_buf_size);
void intel_guc_to_host_event_handler_ct(struct intel_guc *guc);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index c6519066a0f6..7566af8ab46e 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -224,7 +224,7 @@ static int guc_enable_communication(struct intel_guc *guc)
return 0;
}
-static void __guc_stop_communication(struct intel_guc *guc)
+static void guc_disable_communication(struct intel_guc *guc)
{
/*
* Events generated during or after CT disable are logged by guc in
@@ -237,20 +237,6 @@ static void __guc_stop_communication(struct intel_guc *guc)
guc->send = intel_guc_send_nop;
guc->handler = intel_guc_to_host_event_handler_nop;
-}
-
-static void guc_stop_communication(struct intel_guc *guc)
-{
- intel_guc_ct_stop(&guc->ct);
-
- __guc_stop_communication(guc);
-
- DRM_INFO("GuC communication stopped\n");
-}
-
-static void guc_disable_communication(struct intel_guc *guc)
-{
- __guc_stop_communication(guc);
intel_guc_ct_disable(&guc->ct);
@@ -557,7 +543,7 @@ void intel_uc_reset_prepare(struct intel_uc *uc)
if (!intel_guc_is_running(guc))
return;
- guc_stop_communication(guc);
+ guc_disable_communication(guc);
__uc_sanitize(uc);
}
--
2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Intel-gfx] [PATCH 2/5] drm/i915/guc/ct: stop expecting multiple CT channels
2019-12-10 21:09 [Intel-gfx] [PATCH 0/5] Simplify GuC communication handling Daniele Ceraolo Spurio
2019-12-10 21:09 ` [Intel-gfx] [PATCH 1/5] drm/i915/guc: Merge communication_stop and communication_disable Daniele Ceraolo Spurio
@ 2019-12-10 21:09 ` Daniele Ceraolo Spurio
2019-12-11 13:43 ` Michal Wajdeczko
2019-12-10 21:09 ` [Intel-gfx] [PATCH 3/5] drm/i915/guc: remove function pointers for send/receive calls Daniele Ceraolo Spurio
` (5 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-12-10 21:09 UTC (permalink / raw)
To: intel-gfx
The GuC supports having multiple CT buffer pairs and we designed our
implementation with that in mind. However, the different channels are not
processed in parallel within the GuC, so there is very little advantage
in having multiple channels (independent locks?), compared to the
drawbacks (one channel can starve the other if messages keep being
submitted to it). Given this, it is unlikely we'll ever add a second
channel and therefore we can simplify our code by removing the
flexibility.
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: John Harrison <John.C.Harrison@Intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
---
drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 276 +++++++++-------------
drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h | 39 +--
2 files changed, 118 insertions(+), 197 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
index f74ba4750a94..96ce6d74f0b2 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
@@ -37,13 +37,10 @@ static void ct_incoming_request_worker_func(struct work_struct *w);
*/
void intel_guc_ct_init_early(struct intel_guc_ct *ct)
{
- /* we're using static channel owners */
- ct->host_channel.owner = CTB_OWNER_HOST;
-
- spin_lock_init(&ct->lock);
- INIT_LIST_HEAD(&ct->pending_requests);
- INIT_LIST_HEAD(&ct->incoming_requests);
- INIT_WORK(&ct->worker, ct_incoming_request_worker_func);
+ spin_lock_init(&ct->requests.lock);
+ INIT_LIST_HEAD(&ct->requests.pending);
+ INIT_LIST_HEAD(&ct->requests.incoming);
+ INIT_WORK(&ct->requests.worker, ct_incoming_request_worker_func);
}
static inline struct intel_guc *ct_to_guc(struct intel_guc_ct *ct)
@@ -64,14 +61,14 @@ static inline const char *guc_ct_buffer_type_to_str(u32 type)
}
static void guc_ct_buffer_desc_init(struct guc_ct_buffer_desc *desc,
- u32 cmds_addr, u32 size, u32 owner)
+ u32 cmds_addr, u32 size)
{
- CT_DEBUG_DRIVER("CT: desc %p init addr=%#x size=%u owner=%u\n",
- desc, cmds_addr, size, owner);
+ CT_DEBUG_DRIVER("CT: desc %p init addr=%#x size=%u\n",
+ desc, cmds_addr, size);
memset(desc, 0, sizeof(*desc));
desc->addr = cmds_addr;
desc->size = size;
- desc->owner = owner;
+ desc->owner = CTB_OWNER_HOST;
}
static void guc_ct_buffer_desc_reset(struct guc_ct_buffer_desc *desc)
@@ -104,12 +101,11 @@ static int guc_action_register_ct_buffer(struct intel_guc *guc,
}
static int guc_action_deregister_ct_buffer(struct intel_guc *guc,
- u32 owner,
u32 type)
{
u32 action[] = {
INTEL_GUC_ACTION_DEREGISTER_COMMAND_TRANSPORT_BUFFER,
- owner,
+ CTB_OWNER_HOST,
type
};
int err;
@@ -117,19 +113,28 @@ static int guc_action_deregister_ct_buffer(struct intel_guc *guc,
/* Can't use generic send(), CT deregistration must go over MMIO */
err = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action), NULL, 0);
if (err)
- DRM_ERROR("CT: deregister %s buffer failed; owner=%d err=%d\n",
- guc_ct_buffer_type_to_str(type), owner, err);
+ DRM_ERROR("CT: deregister %s buffer failed; err=%d\n",
+ guc_ct_buffer_type_to_str(type), err);
return err;
}
-static int ctch_init(struct intel_guc *guc,
- struct intel_guc_ct_channel *ctch)
+/**
+ * intel_guc_ct_init - Init CT communication
+ * @ct: pointer to CT struct
+ *
+ * Allocate memory required for communication via
+ * the CT channel.
+ *
+ * Return: 0 on success, a negative errno code on failure.
+ */
+int intel_guc_ct_init(struct intel_guc_ct *ct)
{
+ struct intel_guc *guc = ct_to_guc(ct);
void *blob;
int err;
int i;
- GEM_BUG_ON(ctch->vma);
+ GEM_BUG_ON(ct->vma);
/* We allocate 1 page to hold both descriptors and both buffers.
* ___________.....................
@@ -153,57 +158,67 @@ static int ctch_init(struct intel_guc *guc,
* other code will need updating as well.
*/
- err = intel_guc_allocate_and_map_vma(guc, PAGE_SIZE, &ctch->vma, &blob);
+ err = intel_guc_allocate_and_map_vma(guc, PAGE_SIZE, &ct->vma, &blob);
if (err) {
- CT_DEBUG_DRIVER("CT: channel %d initialization failed; err=%d\n",
- ctch->owner, err);
+ DRM_ERROR("CT: channel allocation failed; err=%d\n", err);
return err;
}
CT_DEBUG_DRIVER("CT: vma base=%#x\n",
- intel_guc_ggtt_offset(guc, ctch->vma));
+ intel_guc_ggtt_offset(guc, ct->vma));
/* store pointers to desc and cmds */
- for (i = 0; i < ARRAY_SIZE(ctch->ctbs); i++) {
- GEM_BUG_ON((i != CTB_SEND) && (i != CTB_RECV));
- ctch->ctbs[i].desc = blob + PAGE_SIZE/4 * i;
- ctch->ctbs[i].cmds = blob + PAGE_SIZE/4 * i + PAGE_SIZE/2;
+ for (i = 0; i < ARRAY_SIZE(ct->ctbs); i++) {
+ GEM_BUG_ON((i != CTB_SEND) && (i != CTB_RECV));
+ ct->ctbs[i].desc = blob + PAGE_SIZE/4 * i;
+ ct->ctbs[i].cmds = blob + PAGE_SIZE/4 * i + PAGE_SIZE/2;
}
return 0;
}
-static void ctch_fini(struct intel_guc *guc,
- struct intel_guc_ct_channel *ctch)
+/**
+ * intel_guc_ct_fini - Fini CT communication
+ * @ct: pointer to CT struct
+ *
+ * Deallocate memory required for communication via
+ * the CT channel.
+ */
+void intel_guc_ct_fini(struct intel_guc_ct *ct)
{
- GEM_BUG_ON(ctch->enabled);
+ GEM_BUG_ON(ct->enabled);
- i915_vma_unpin_and_release(&ctch->vma, I915_VMA_RELEASE_MAP);
+ i915_vma_unpin_and_release(&ct->vma, I915_VMA_RELEASE_MAP);
}
-static int ctch_enable(struct intel_guc *guc,
- struct intel_guc_ct_channel *ctch)
+/**
+ * intel_guc_ct_enable - Enable buffer based command transport.
+ * @ct: pointer to CT struct
+ *
+ * Return: 0 on success, a negative errno code on failure.
+ */
+int intel_guc_ct_enable(struct intel_guc_ct *ct)
{
+ struct intel_guc *guc = ct_to_guc(ct);
u32 base;
int err;
int i;
- GEM_BUG_ON(!ctch->vma);
-
- GEM_BUG_ON(ctch->enabled);
+ if (ct->enabled)
+ return 0;
/* vma should be already allocated and map'ed */
- base = intel_guc_ggtt_offset(guc, ctch->vma);
+ GEM_BUG_ON(!ct->vma);
+ base = intel_guc_ggtt_offset(guc, ct->vma);
/* (re)initialize descriptors
* cmds buffers are in the second half of the blob page
*/
- for (i = 0; i < ARRAY_SIZE(ctch->ctbs); i++) {
+ for (i = 0; i < ARRAY_SIZE(ct->ctbs); i++) {
GEM_BUG_ON((i != CTB_SEND) && (i != CTB_RECV));
- guc_ct_buffer_desc_init(ctch->ctbs[i].desc,
+ guc_ct_buffer_desc_init(ct->ctbs[i].desc,
base + PAGE_SIZE/4 * i + PAGE_SIZE/2,
- PAGE_SIZE/4,
- ctch->owner);
+ PAGE_SIZE/4);
}
/* register buffers, starting wirh RECV buffer
@@ -221,40 +236,43 @@ static int ctch_enable(struct intel_guc *guc,
if (unlikely(err))
goto err_deregister;
- ctch->enabled = true;
+ ct->enabled = true;
return 0;
err_deregister:
guc_action_deregister_ct_buffer(guc,
- ctch->owner,
INTEL_GUC_CT_BUFFER_TYPE_RECV);
err_out:
- DRM_ERROR("CT: can't open channel %d; err=%d\n", ctch->owner, err);
+ DRM_ERROR("CT: can't open channel; err=%d\n", err);
return err;
}
-static void ctch_disable(struct intel_guc *guc,
- struct intel_guc_ct_channel *ctch)
+/**
+ * intel_guc_ct_disable - Disable buffer based command transport.
+ * @ct: pointer to CT struct
+ */
+void intel_guc_ct_disable(struct intel_guc_ct *ct)
{
- GEM_BUG_ON(!ctch->enabled);
+ struct intel_guc *guc = ct_to_guc(ct);
- ctch->enabled = false;
+ if (!ct->enabled)
+ return;
+
+ ct->enabled = false;
if (intel_guc_is_running(guc)) {
guc_action_deregister_ct_buffer(guc,
- ctch->owner,
INTEL_GUC_CT_BUFFER_TYPE_SEND);
guc_action_deregister_ct_buffer(guc,
- ctch->owner,
INTEL_GUC_CT_BUFFER_TYPE_RECV);
}
}
-static u32 ctch_get_next_fence(struct intel_guc_ct_channel *ctch)
+static u32 ct_get_next_fence(struct intel_guc_ct *ct)
{
/* For now it's trivial */
- return ++ctch->next_fence;
+ return ++ct->next_fence;
}
/**
@@ -427,35 +445,34 @@ static int wait_for_ct_request_update(struct ct_request *req, u32 *status)
return err;
}
-static int ctch_send(struct intel_guc_ct *ct,
- struct intel_guc_ct_channel *ctch,
- const u32 *action,
- u32 len,
- u32 *response_buf,
- u32 response_buf_size,
- u32 *status)
+static int ct_send(struct intel_guc_ct *ct,
+ const u32 *action,
+ u32 len,
+ u32 *response_buf,
+ u32 response_buf_size,
+ u32 *status)
{
- struct intel_guc_ct_buffer *ctb = &ctch->ctbs[CTB_SEND];
+ struct intel_guc_ct_buffer *ctb = &ct->ctbs[CTB_SEND];
struct guc_ct_buffer_desc *desc = ctb->desc;
struct ct_request request;
unsigned long flags;
u32 fence;
int err;
- GEM_BUG_ON(!ctch->enabled);
+ GEM_BUG_ON(!ct->enabled);
GEM_BUG_ON(!len);
GEM_BUG_ON(len & ~GUC_CT_MSG_LEN_MASK);
GEM_BUG_ON(!response_buf && response_buf_size);
- fence = ctch_get_next_fence(ctch);
+ fence = ct_get_next_fence(ct);
request.fence = fence;
request.status = 0;
request.response_len = response_buf_size;
request.response_buf = response_buf;
- spin_lock_irqsave(&ct->lock, flags);
- list_add_tail(&request.link, &ct->pending_requests);
- spin_unlock_irqrestore(&ct->lock, flags);
+ spin_lock_irqsave(&ct->requests.lock, flags);
+ list_add_tail(&request.link, &ct->requests.pending);
+ spin_unlock_irqrestore(&ct->requests.lock, flags);
err = ctb_write(ctb, action, len, fence, !!response_buf);
if (unlikely(err))
@@ -488,9 +505,9 @@ static int ctch_send(struct intel_guc_ct *ct,
}
unlink:
- spin_lock_irqsave(&ct->lock, flags);
+ spin_lock_irqsave(&ct->requests.lock, flags);
list_del(&request.link);
- spin_unlock_irqrestore(&ct->lock, flags);
+ spin_unlock_irqrestore(&ct->requests.lock, flags);
return err;
}
@@ -502,14 +519,12 @@ int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len,
u32 *response_buf, u32 response_buf_size)
{
struct intel_guc_ct *ct = &guc->ct;
- struct intel_guc_ct_channel *ctch = &ct->host_channel;
u32 status = ~0; /* undefined */
int ret;
mutex_lock(&guc->send_mutex);
- ret = ctch_send(ct, ctch, action, len, response_buf, response_buf_size,
- &status);
+ ret = ct_send(ct, action, len, response_buf, response_buf_size, &status);
if (unlikely(ret < 0)) {
DRM_ERROR("CT: send action %#X failed; err=%d status=%#X\n",
action[0], ret, status);
@@ -640,8 +655,8 @@ static int ct_handle_response(struct intel_guc_ct *ct, const u32 *msg)
CT_DEBUG_DRIVER("CT: response fence %u status %#x\n", fence, status);
- spin_lock(&ct->lock);
- list_for_each_entry(req, &ct->pending_requests, link) {
+ spin_lock(&ct->requests.lock);
+ list_for_each_entry(req, &ct->requests.pending, link) {
if (unlikely(fence != req->fence)) {
CT_DEBUG_DRIVER("CT: request %u awaits response\n",
req->fence);
@@ -659,7 +674,7 @@ static int ct_handle_response(struct intel_guc_ct *ct, const u32 *msg)
found = true;
break;
}
- spin_unlock(&ct->lock);
+ spin_unlock(&ct->requests.lock);
if (!found)
DRM_ERROR("CT: unsolicited response %*ph\n", 4 * msglen, msg);
@@ -697,13 +712,13 @@ static bool ct_process_incoming_requests(struct intel_guc_ct *ct)
u32 *payload;
bool done;
- spin_lock_irqsave(&ct->lock, flags);
- request = list_first_entry_or_null(&ct->incoming_requests,
+ spin_lock_irqsave(&ct->requests.lock, flags);
+ request = list_first_entry_or_null(&ct->requests.incoming,
struct ct_incoming_request, link);
if (request)
list_del(&request->link);
- done = !!list_empty(&ct->incoming_requests);
- spin_unlock_irqrestore(&ct->lock, flags);
+ done = !!list_empty(&ct->requests.incoming);
+ spin_unlock_irqrestore(&ct->requests.lock, flags);
if (!request)
return true;
@@ -721,12 +736,13 @@ static bool ct_process_incoming_requests(struct intel_guc_ct *ct)
static void ct_incoming_request_worker_func(struct work_struct *w)
{
- struct intel_guc_ct *ct = container_of(w, struct intel_guc_ct, worker);
+ struct intel_guc_ct *ct =
+ container_of(w, struct intel_guc_ct, requests.worker);
bool done;
done = ct_process_incoming_requests(ct);
if (!done)
- queue_work(system_unbound_wq, &ct->worker);
+ queue_work(system_unbound_wq, &ct->requests.worker);
}
/**
@@ -764,22 +780,26 @@ static int ct_handle_request(struct intel_guc_ct *ct, const u32 *msg)
}
memcpy(request->msg, msg, 4 * msglen);
- spin_lock_irqsave(&ct->lock, flags);
- list_add_tail(&request->link, &ct->incoming_requests);
- spin_unlock_irqrestore(&ct->lock, flags);
+ spin_lock_irqsave(&ct->requests.lock, flags);
+ list_add_tail(&request->link, &ct->requests.incoming);
+ spin_unlock_irqrestore(&ct->requests.lock, flags);
- queue_work(system_unbound_wq, &ct->worker);
+ queue_work(system_unbound_wq, &ct->requests.worker);
return 0;
}
-static void ct_process_host_channel(struct intel_guc_ct *ct)
+/*
+ * When we're communicating with the GuC over CT, GuC uses events
+ * to notify us about new messages being posted on the RECV buffer.
+ */
+void intel_guc_to_host_event_handler_ct(struct intel_guc *guc)
{
- struct intel_guc_ct_channel *ctch = &ct->host_channel;
- struct intel_guc_ct_buffer *ctb = &ctch->ctbs[CTB_RECV];
+ struct intel_guc_ct *ct = &guc->ct;
+ struct intel_guc_ct_buffer *ctb = &ct->ctbs[CTB_RECV];
u32 msg[GUC_CT_MSG_LEN_MASK + 1]; /* one extra dw for the header */
int err = 0;
- if (!ctch->enabled)
+ if (!ct->enabled)
return;
do {
@@ -798,87 +818,3 @@ static void ct_process_host_channel(struct intel_guc_ct *ct)
ctb->desc->is_in_error = 1;
}
}
-
-/*
- * When we're communicating with the GuC over CT, GuC uses events
- * to notify us about new messages being posted on the RECV buffer.
- */
-void intel_guc_to_host_event_handler_ct(struct intel_guc *guc)
-{
- struct intel_guc_ct *ct = &guc->ct;
-
- ct_process_host_channel(ct);
-}
-
-/**
- * intel_guc_ct_init - Init CT communication
- * @ct: pointer to CT struct
- *
- * Allocate memory required for communication via
- * the CT channel.
- *
- * Return: 0 on success, a negative errno code on failure.
- */
-int intel_guc_ct_init(struct intel_guc_ct *ct)
-{
- struct intel_guc *guc = ct_to_guc(ct);
- struct intel_guc_ct_channel *ctch = &ct->host_channel;
- int err;
-
- err = ctch_init(guc, ctch);
- if (unlikely(err)) {
- DRM_ERROR("CT: can't open channel %d; err=%d\n",
- ctch->owner, err);
- return err;
- }
-
- GEM_BUG_ON(!ctch->vma);
- return 0;
-}
-
-/**
- * intel_guc_ct_fini - Fini CT communication
- * @ct: pointer to CT struct
- *
- * Deallocate memory required for communication via
- * the CT channel.
- */
-void intel_guc_ct_fini(struct intel_guc_ct *ct)
-{
- struct intel_guc *guc = ct_to_guc(ct);
- struct intel_guc_ct_channel *ctch = &ct->host_channel;
-
- ctch_fini(guc, ctch);
-}
-
-/**
- * intel_guc_ct_enable - Enable buffer based command transport.
- * @ct: pointer to CT struct
- *
- * Return: 0 on success, a negative errno code on failure.
- */
-int intel_guc_ct_enable(struct intel_guc_ct *ct)
-{
- struct intel_guc *guc = ct_to_guc(ct);
- struct intel_guc_ct_channel *ctch = &ct->host_channel;
-
- if (ctch->enabled)
- return 0;
-
- return ctch_enable(guc, ctch);
-}
-
-/**
- * intel_guc_ct_disable - Disable buffer based command transport.
- * @ct: pointer to CT struct
- */
-void intel_guc_ct_disable(struct intel_guc_ct *ct)
-{
- struct intel_guc *guc = ct_to_guc(ct);
- struct intel_guc_ct_channel *ctch = &ct->host_channel;
-
- if (!ctch->enabled)
- return;
-
- ctch_disable(guc, ctch);
-}
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
index 77c80d6cc25d..4bb1d1fcc860 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
@@ -35,44 +35,29 @@ struct intel_guc_ct_buffer {
u32 *cmds;
};
+
/** Represents pair of command transport buffers.
*
* Buffers go in pairs to allow bi-directional communication.
* To simplify the code we place both of them in the same vma.
* Buffers from the same pair must share unique owner id.
- *
- * @vma: pointer to the vma with pair of CT buffers
- * @ctbs: buffers for sending(0) and receiving(1) commands
- * @owner: unique identifier
- * @next_fence: fence to be used with next send command
*/
-struct intel_guc_ct_channel {
+struct intel_guc_ct {
struct i915_vma *vma;
- struct intel_guc_ct_buffer ctbs[2];
- u32 owner;
- u32 next_fence;
bool enabled;
-};
-/** Holds all command transport channels.
- *
- * @host_channel: main channel used by the host
- */
-struct intel_guc_ct {
- struct intel_guc_ct_channel host_channel;
- /* other channels are tbd */
-
- /** @lock: protects pending requests list */
- spinlock_t lock;
-
- /** @pending_requests: list of requests waiting for response */
- struct list_head pending_requests;
+ /* buffers for sending(0) and receiving(1) commands */
+ struct intel_guc_ct_buffer ctbs[2];
- /** @incoming_requests: list of incoming requests */
- struct list_head incoming_requests;
+ /* fence to be used with next send command */
+ u32 next_fence;
- /** @worker: worker for handling incoming requests */
- struct work_struct worker;
+ struct {
+ spinlock_t lock; /* protects pending requests list */
+ struct list_head pending; /* requests waiting for response */
+ struct list_head incoming; /* incoming requests */
+ struct work_struct worker; /* handler for incoming requests */
+ } requests;
};
void intel_guc_ct_init_early(struct intel_guc_ct *ct);
--
2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Intel-gfx] [PATCH 3/5] drm/i915/guc: remove function pointers for send/receive calls
2019-12-10 21:09 [Intel-gfx] [PATCH 0/5] Simplify GuC communication handling Daniele Ceraolo Spurio
2019-12-10 21:09 ` [Intel-gfx] [PATCH 1/5] drm/i915/guc: Merge communication_stop and communication_disable Daniele Ceraolo Spurio
2019-12-10 21:09 ` [Intel-gfx] [PATCH 2/5] drm/i915/guc/ct: stop expecting multiple CT channels Daniele Ceraolo Spurio
@ 2019-12-10 21:09 ` Daniele Ceraolo Spurio
2019-12-11 14:04 ` Michal Wajdeczko
2019-12-10 21:09 ` [Intel-gfx] [PATCH 4/5] drm/i915/guc: unify notify() functions Daniele Ceraolo Spurio
` (4 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-12-10 21:09 UTC (permalink / raw)
To: intel-gfx
Since we started using CT buffers on all gens, the function pointers can
only be set to either the _nop() or the _ct() functions. Since the
_nop() case applies to when the CT are disabled, we can just handle that
case in the _ct() functions and call them directly.
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: John Harrison <John.C.Harrison@Intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
---
drivers/gpu/drm/i915/gt/intel_gt_irq.c | 2 +-
drivers/gpu/drm/i915/gt/uc/intel_guc.c | 22 +++-----------
drivers/gpu/drm/i915/gt/uc/intel_guc.h | 29 -------------------
drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 14 +++++++--
drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h | 17 +++++++++--
drivers/gpu/drm/i915/gt/uc/intel_guc_log.c | 6 ++--
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 1 -
drivers/gpu/drm/i915/gt/uc/intel_uc.c | 18 ++++--------
8 files changed, 40 insertions(+), 69 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_irq.c b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
index 332b12a574fb..3183b4426c7b 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_irq.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
@@ -16,7 +16,7 @@
static void guc_irq_handler(struct intel_guc *guc, u16 iir)
{
if (iir & GUC_INTR_GUC2HOST)
- intel_guc_to_host_event_handler(guc);
+ intel_guc_to_host_event_handler_ct(guc);
}
static void
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
index 922a19635d20..eb94635eeecd 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
@@ -177,8 +177,6 @@ void intel_guc_init_early(struct intel_guc *guc)
mutex_init(&guc->send_mutex);
spin_lock_init(&guc->irq_lock);
- guc->send = intel_guc_send_nop;
- guc->handler = intel_guc_to_host_event_handler_nop;
if (INTEL_GEN(i915) >= 11) {
guc->notify = gen11_guc_raise_irq;
guc->interrupts.reset = gen11_reset_guc_interrupts;
@@ -403,18 +401,6 @@ void intel_guc_fini(struct intel_guc *guc)
intel_uc_fw_cleanup_fetch(&guc->fw);
}
-int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len,
- u32 *response_buf, u32 response_buf_size)
-{
- WARN(1, "Unexpected send: action=%#x\n", *action);
- return -ENODEV;
-}
-
-void intel_guc_to_host_event_handler_nop(struct intel_guc *guc)
-{
- WARN(1, "Unexpected event: no suitable handler\n");
-}
-
/*
* This function implements the MMIO based host to GuC interface.
*/
@@ -515,7 +501,7 @@ int intel_guc_sample_forcewake(struct intel_guc *guc)
/* bit 0 and 1 are for Render and Media domain separately */
action[1] = GUC_FORCEWAKE_RENDER | GUC_FORCEWAKE_MEDIA;
- return intel_guc_send(guc, action, ARRAY_SIZE(action));
+ return intel_guc_send_ct(guc, action, ARRAY_SIZE(action));
}
/**
@@ -536,7 +522,7 @@ int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset)
rsa_offset
};
- return intel_guc_send(guc, action, ARRAY_SIZE(action));
+ return intel_guc_send_ct(guc, action, ARRAY_SIZE(action));
}
/**
@@ -573,7 +559,7 @@ int intel_guc_suspend(struct intel_guc *guc)
intel_uncore_write(uncore, SOFT_SCRATCH(14),
INTEL_GUC_SLEEP_STATE_INVALID_MASK);
- ret = intel_guc_send(guc, action, ARRAY_SIZE(action));
+ ret = intel_guc_send_ct(guc, action, ARRAY_SIZE(action));
if (ret)
return ret;
@@ -625,7 +611,7 @@ int intel_guc_resume(struct intel_guc *guc)
if (!intel_guc_submission_is_enabled(guc))
return 0;
- return intel_guc_send(guc, action, ARRAY_SIZE(action));
+ return intel_guc_send_ct(guc, action, ARRAY_SIZE(action));
}
/**
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
index cd09c912e361..c0b32db1c6ad 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -70,40 +70,15 @@ struct intel_guc {
/* To serialize the intel_guc_send actions */
struct mutex send_mutex;
- /* GuC's FW specific send function */
- int (*send)(struct intel_guc *guc, const u32 *data, u32 len,
- u32 *response_buf, u32 response_buf_size);
-
- /* GuC's FW specific event handler function */
- void (*handler)(struct intel_guc *guc);
-
/* GuC's FW specific notify function */
void (*notify)(struct intel_guc *guc);
};
-static
-inline int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len)
-{
- return guc->send(guc, action, len, NULL, 0);
-}
-
-static inline int
-intel_guc_send_and_receive(struct intel_guc *guc, const u32 *action, u32 len,
- u32 *response_buf, u32 response_buf_size)
-{
- return guc->send(guc, action, len, response_buf, response_buf_size);
-}
-
static inline void intel_guc_notify(struct intel_guc *guc)
{
guc->notify(guc);
}
-static inline void intel_guc_to_host_event_handler(struct intel_guc *guc)
-{
- guc->handler(guc);
-}
-
/* GuC addresses above GUC_GGTT_TOP also don't map through the GTT */
#define GUC_GGTT_TOP 0xFEE00000
@@ -136,12 +111,8 @@ void intel_guc_init_send_regs(struct intel_guc *guc);
void intel_guc_write_params(struct intel_guc *guc);
int intel_guc_init(struct intel_guc *guc);
void intel_guc_fini(struct intel_guc *guc);
-int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len,
- u32 *response_buf, u32 response_buf_size);
int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len,
u32 *response_buf, u32 response_buf_size);
-void intel_guc_to_host_event_handler(struct intel_guc *guc);
-void intel_guc_to_host_event_handler_nop(struct intel_guc *guc);
int intel_guc_to_host_process_recv_msg(struct intel_guc *guc,
const u32 *payload, u32 len);
int intel_guc_sample_forcewake(struct intel_guc *guc);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
index 96ce6d74f0b2..60b19f83e153 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
@@ -515,13 +515,19 @@ static int ct_send(struct intel_guc_ct *ct,
/*
* Command Transport (CT) buffer based GuC send function.
*/
-int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len,
- u32 *response_buf, u32 response_buf_size)
+int intel_guc_send_and_receive_ct(struct intel_guc *guc, const u32 *action,
+ u32 len, u32 *response_buf,
+ u32 response_buf_size)
{
struct intel_guc_ct *ct = &guc->ct;
u32 status = ~0; /* undefined */
int ret;
+ if (unlikely(!ct->enabled)) {
+ WARN(1, "Unexpected send: action=%#x\n", *action);
+ return -ENODEV;
+ }
+
mutex_lock(&guc->send_mutex);
ret = ct_send(ct, action, len, response_buf, response_buf_size, &status);
@@ -799,8 +805,10 @@ void intel_guc_to_host_event_handler_ct(struct intel_guc *guc)
u32 msg[GUC_CT_MSG_LEN_MASK + 1]; /* one extra dw for the header */
int err = 0;
- if (!ct->enabled)
+ if (!ct->enabled) {
+ WARN(1, "Unexpected event: no suitable handler\n");
return;
+ }
do {
err = ctb_read(ctb, msg);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
index 4bb1d1fcc860..929483b1f013 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
@@ -66,8 +66,21 @@ void intel_guc_ct_fini(struct intel_guc_ct *ct);
int intel_guc_ct_enable(struct intel_guc_ct *ct);
void intel_guc_ct_disable(struct intel_guc_ct *ct);
-int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len,
- u32 *response_buf, u32 response_buf_size);
+static inline bool intel_guc_ct_enabled(struct intel_guc_ct *ct)
+{
+ return ct->enabled;
+}
+
+int
+intel_guc_send_and_receive_ct(struct intel_guc *guc, const u32 *action, u32 len,
+ u32 *response_buf, u32 response_buf_size);
+
+static inline int
+intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len)
+{
+ return intel_guc_send_and_receive_ct(guc, action, len, NULL, 0);
+}
+
void intel_guc_to_host_event_handler_ct(struct intel_guc *guc);
#endif /* _INTEL_GUC_CT_H_ */
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
index caed0d57e704..5938127fb129 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
@@ -27,7 +27,7 @@ static int guc_action_flush_log_complete(struct intel_guc *guc)
INTEL_GUC_ACTION_LOG_BUFFER_FILE_FLUSH_COMPLETE
};
- return intel_guc_send(guc, action, ARRAY_SIZE(action));
+ return intel_guc_send_ct(guc, action, ARRAY_SIZE(action));
}
static int guc_action_flush_log(struct intel_guc *guc)
@@ -37,7 +37,7 @@ static int guc_action_flush_log(struct intel_guc *guc)
0
};
- return intel_guc_send(guc, action, ARRAY_SIZE(action));
+ return intel_guc_send_ct(guc, action, ARRAY_SIZE(action));
}
static int guc_action_control_log(struct intel_guc *guc, bool enable,
@@ -52,7 +52,7 @@ static int guc_action_control_log(struct intel_guc *guc, bool enable,
GEM_BUG_ON(verbosity > GUC_LOG_VERBOSITY_MAX);
- return intel_guc_send(guc, action, ARRAY_SIZE(action));
+ return intel_guc_send_ct(guc, action, ARRAY_SIZE(action));
}
static inline struct intel_guc *log_to_guc(struct intel_guc_log *log)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index 172220e83079..fd7008bb128c 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -43,7 +43,6 @@
* Firmware writes a success/fail code back to the action register after
* processes the request. The kernel driver polls waiting for this update and
* then proceeds.
- * See intel_guc_send()
*
* Work Items:
* There are several types of work items that the host may place into a
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 7566af8ab46e..18a5eaf3052c 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -123,6 +123,11 @@ static void __uc_free_load_err_log(struct intel_uc *uc)
i915_gem_object_put(log);
}
+static inline bool guc_communication_enabled(struct intel_guc *guc)
+{
+ return intel_guc_ct_enabled(&guc->ct);
+}
+
/*
* Events triggered while CT buffers are disabled are logged in the SCRATCH_15
* register using the same bits used in the CT message payload. Since our
@@ -158,7 +163,7 @@ static void guc_handle_mmio_msg(struct intel_guc *guc)
struct drm_i915_private *i915 = guc_to_gt(guc)->i915;
/* we need communication to be enabled to reply to GuC */
- GEM_BUG_ON(guc->handler == intel_guc_to_host_event_handler_nop);
+ GEM_BUG_ON(!guc_communication_enabled(guc));
if (!guc->mmio_msg)
return;
@@ -185,11 +190,6 @@ static void guc_disable_interrupts(struct intel_guc *guc)
guc->interrupts.disable(guc);
}
-static inline bool guc_communication_enabled(struct intel_guc *guc)
-{
- return guc->send != intel_guc_send_nop;
-}
-
static int guc_enable_communication(struct intel_guc *guc)
{
struct drm_i915_private *i915 = guc_to_gt(guc)->i915;
@@ -205,9 +205,6 @@ static int guc_enable_communication(struct intel_guc *guc)
if (ret)
return ret;
- guc->send = intel_guc_send_ct;
- guc->handler = intel_guc_to_host_event_handler_ct;
-
/* check for mmio messages received before/during the CT enable */
guc_get_mmio_msg(guc);
guc_handle_mmio_msg(guc);
@@ -235,9 +232,6 @@ static void guc_disable_communication(struct intel_guc *guc)
guc_disable_interrupts(guc);
- guc->send = intel_guc_send_nop;
- guc->handler = intel_guc_to_host_event_handler_nop;
-
intel_guc_ct_disable(&guc->ct);
/*
--
2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Intel-gfx] [PATCH 4/5] drm/i915/guc: unify notify() functions
2019-12-10 21:09 [Intel-gfx] [PATCH 0/5] Simplify GuC communication handling Daniele Ceraolo Spurio
` (2 preceding siblings ...)
2019-12-10 21:09 ` [Intel-gfx] [PATCH 3/5] drm/i915/guc: remove function pointers for send/receive calls Daniele Ceraolo Spurio
@ 2019-12-10 21:09 ` Daniele Ceraolo Spurio
2019-12-11 14:26 ` Michal Wajdeczko
2019-12-10 21:09 ` [Intel-gfx] [PATCH 5/5] HAX: force enable_guc=2 and WA i915#571 Daniele Ceraolo Spurio
` (3 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-12-10 21:09 UTC (permalink / raw)
To: intel-gfx
The Gen11+ and the legacy function differ in the register and value
written to interrupt the GuC. However, while on older gen the value
matches a bit on the register, on Gen11+ the value is a SW defined
payload that is sent to the FW. Since the FW behaves the same no matter
what value we pass to it, we can just write the same thing on all gens
and get rid of the function pointer by saving the register offset.
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: John Harrison <John.C.Harrison@Intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
---
drivers/gpu/drm/i915/gt/uc/intel_guc.c | 21 ++++++++++-----------
drivers/gpu/drm/i915/gt/uc/intel_guc.h | 12 ++++--------
2 files changed, 14 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
index eb94635eeecd..d84cd2e93534 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
@@ -32,18 +32,17 @@
* just the HuC, but more are expected to land in the future).
*/
-static void gen8_guc_raise_irq(struct intel_guc *guc)
+void intel_guc_notify(struct intel_guc *guc)
{
struct intel_gt *gt = guc_to_gt(guc);
- intel_uncore_write(gt->uncore, GUC_SEND_INTERRUPT, GUC_SEND_TRIGGER);
-}
-
-static void gen11_guc_raise_irq(struct intel_guc *guc)
-{
- struct intel_gt *gt = guc_to_gt(guc);
-
- intel_uncore_write(gt->uncore, GEN11_GUC_HOST_INTERRUPT, 0);
+ /*
+ * On Gen11+, the value written to the register is passes as a payload
+ * to the FW. However, the FW currently treats all values the same way
+ * (H2G interrupt), so we can just write the value that the HW expects
+ * on older gens.
+ */
+ intel_uncore_write(gt->uncore, guc->notify_reg, GUC_SEND_TRIGGER);
}
static inline i915_reg_t guc_send_reg(struct intel_guc *guc, u32 i)
@@ -178,12 +177,12 @@ void intel_guc_init_early(struct intel_guc *guc)
mutex_init(&guc->send_mutex);
spin_lock_init(&guc->irq_lock);
if (INTEL_GEN(i915) >= 11) {
- guc->notify = gen11_guc_raise_irq;
+ guc->notify_reg = GEN11_GUC_HOST_INTERRUPT;
guc->interrupts.reset = gen11_reset_guc_interrupts;
guc->interrupts.enable = gen11_enable_guc_interrupts;
guc->interrupts.disable = gen11_disable_guc_interrupts;
} else {
- guc->notify = gen8_guc_raise_irq;
+ guc->notify_reg = GUC_SEND_INTERRUPT;
guc->interrupts.reset = gen9_reset_guc_interrupts;
guc->interrupts.enable = gen9_enable_guc_interrupts;
guc->interrupts.disable = gen9_disable_guc_interrupts;
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
index c0b32db1c6ad..b95d2b3528a2 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -64,21 +64,16 @@ struct intel_guc {
enum forcewake_domains fw_domains;
} send_regs;
+ /* register used to send interrupts to the GuC FW */
+ i915_reg_t notify_reg;
+
/* Store msg (e.g. log flush) that we see while CTBs are disabled */
u32 mmio_msg;
/* To serialize the intel_guc_send actions */
struct mutex send_mutex;
-
- /* GuC's FW specific notify function */
- void (*notify)(struct intel_guc *guc);
};
-static inline void intel_guc_notify(struct intel_guc *guc)
-{
- guc->notify(guc);
-}
-
/* GuC addresses above GUC_GGTT_TOP also don't map through the GTT */
#define GUC_GGTT_TOP 0xFEE00000
@@ -111,6 +106,7 @@ void intel_guc_init_send_regs(struct intel_guc *guc);
void intel_guc_write_params(struct intel_guc *guc);
int intel_guc_init(struct intel_guc *guc);
void intel_guc_fini(struct intel_guc *guc);
+void intel_guc_notify(struct intel_guc *guc);
int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len,
u32 *response_buf, u32 response_buf_size);
int intel_guc_to_host_process_recv_msg(struct intel_guc *guc,
--
2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Intel-gfx] [PATCH 5/5] HAX: force enable_guc=2 and WA i915#571
2019-12-10 21:09 [Intel-gfx] [PATCH 0/5] Simplify GuC communication handling Daniele Ceraolo Spurio
` (3 preceding siblings ...)
2019-12-10 21:09 ` [Intel-gfx] [PATCH 4/5] drm/i915/guc: unify notify() functions Daniele Ceraolo Spurio
@ 2019-12-10 21:09 ` Daniele Ceraolo Spurio
2019-12-11 5:25 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Simplify GuC communication handling Patchwork
` (2 subsequent siblings)
7 siblings, 0 replies; 16+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-12-10 21:09 UTC (permalink / raw)
To: intel-gfx
To get a full run with GuC loading and HuC auth enabled.
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
drivers/gpu/drm/i915/gt/selftest_gt_pm.c | 9 +++++++++
drivers/gpu/drm/i915/i915_params.h | 2 +-
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/selftest_gt_pm.c b/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
index 09ff8e4f88af..86b176c887b4 100644
--- a/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
+++ b/drivers/gpu/drm/i915/gt/selftest_gt_pm.c
@@ -12,8 +12,11 @@ static int live_gt_resume(void *arg)
{
struct intel_gt *gt = arg;
IGT_TIMEOUT(end_time);
+ intel_wakeref_t wakeref;
int err;
+ wakeref = intel_runtime_pm_get(gt->uncore->rpm);
+
/* Do several suspend/resume cycles to check we don't explode! */
do {
intel_gt_suspend_prepare(gt);
@@ -26,6 +29,10 @@ static int live_gt_resume(void *arg)
break;
}
+ err = intel_gt_init_hw(gt);
+ if (err)
+ break;
+
err = intel_gt_resume(gt);
if (err)
break;
@@ -45,6 +52,8 @@ static int live_gt_resume(void *arg)
}
} while (!__igt_timeout(end_time, NULL));
+ intel_runtime_pm_put(gt->uncore->rpm, wakeref);
+
return err;
}
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 31b88f297fbc..acda9f2a1207 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -54,7 +54,7 @@ struct drm_printer;
param(int, disable_power_well, -1) \
param(int, enable_ips, 1) \
param(int, invert_brightness, 0) \
- param(int, enable_guc, 0) \
+ param(int, enable_guc, 2) \
param(int, guc_log_level, -1) \
param(char *, guc_firmware_path, NULL) \
param(char *, huc_firmware_path, NULL) \
--
2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Simplify GuC communication handling
2019-12-10 21:09 [Intel-gfx] [PATCH 0/5] Simplify GuC communication handling Daniele Ceraolo Spurio
` (4 preceding siblings ...)
2019-12-10 21:09 ` [Intel-gfx] [PATCH 5/5] HAX: force enable_guc=2 and WA i915#571 Daniele Ceraolo Spurio
@ 2019-12-11 5:25 ` Patchwork
2019-12-11 5:54 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2019-12-11 11:33 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
7 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2019-12-11 5:25 UTC (permalink / raw)
To: Daniele Ceraolo Spurio; +Cc: intel-gfx
== Series Details ==
Series: Simplify GuC communication handling
URL : https://patchwork.freedesktop.org/series/70717/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
3f4d1cd323a6 drm/i915/guc: Merge communication_stop and communication_disable
6a2cdf54548f drm/i915/guc/ct: stop expecting multiple CT channels
-:133: CHECK:SPACING: spaces preferred around that '/' (ctx:VxV)
#133: FILE: drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c:173:
+ ct->ctbs[i].desc = blob + PAGE_SIZE/4 * i;
^
-:134: CHECK:SPACING: spaces preferred around that '/' (ctx:VxV)
#134: FILE: drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c:174:
+ ct->ctbs[i].cmds = blob + PAGE_SIZE/4 * i + PAGE_SIZE/2;
^
-:134: CHECK:SPACING: spaces preferred around that '/' (ctx:VxV)
#134: FILE: drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c:174:
+ ct->ctbs[i].cmds = blob + PAGE_SIZE/4 * i + PAGE_SIZE/2;
^
-:195: CHECK:SPACING: spaces preferred around that '/' (ctx:VxV)
#195: FILE: drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c:221:
+ PAGE_SIZE/4);
^
-:515: CHECK:LINE_SPACING: Please don't use multiple blank lines
#515: FILE: drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h:38:
+
total: 0 errors, 0 warnings, 5 checks, 440 lines checked
396ef783e1de drm/i915/guc: remove function pointers for send/receive calls
5ad83c593530 drm/i915/guc: unify notify() functions
b60deedca8ec HAX: force enable_guc=2 and WA i915#571
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Intel-gfx] ✓ Fi.CI.BAT: success for Simplify GuC communication handling
2019-12-10 21:09 [Intel-gfx] [PATCH 0/5] Simplify GuC communication handling Daniele Ceraolo Spurio
` (5 preceding siblings ...)
2019-12-11 5:25 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Simplify GuC communication handling Patchwork
@ 2019-12-11 5:54 ` Patchwork
2019-12-11 11:33 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
7 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2019-12-11 5:54 UTC (permalink / raw)
To: Daniele Ceraolo Spurio; +Cc: intel-gfx
== Series Details ==
Series: Simplify GuC communication handling
URL : https://patchwork.freedesktop.org/series/70717/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_7534 -> Patchwork_15681
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/index.html
Known issues
------------
Here are the changes found in Patchwork_15681 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@i915_pm_rpm@module-reload:
- fi-skl-6770hq: [PASS][1] -> [FAIL][2] ([i915#178])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
* igt@i915_selftest@live_blt:
- fi-hsw-4770: [PASS][3] -> [DMESG-FAIL][4] ([i915#553] / [i915#725])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/fi-hsw-4770/igt@i915_selftest@live_blt.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/fi-hsw-4770/igt@i915_selftest@live_blt.html
* igt@i915_selftest@live_gem_contexts:
- fi-byt-n2820: [PASS][5] -> [INCOMPLETE][6] ([i915#45])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/fi-byt-n2820/igt@i915_selftest@live_gem_contexts.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/fi-byt-n2820/igt@i915_selftest@live_gem_contexts.html
#### Possible fixes ####
* igt@gem_ctx_create@basic-files:
- {fi-tgl-u}: [INCOMPLETE][7] ([fdo#111735]) -> [PASS][8]
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/fi-tgl-u/igt@gem_ctx_create@basic-files.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/fi-tgl-u/igt@gem_ctx_create@basic-files.html
* igt@i915_selftest@live_gem_contexts:
- fi-hsw-peppy: [INCOMPLETE][9] ([i915#694]) -> [PASS][10]
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/fi-hsw-peppy/igt@i915_selftest@live_gem_contexts.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/fi-hsw-peppy/igt@i915_selftest@live_gem_contexts.html
* igt@i915_selftest@live_gt_pm:
- fi-icl-guc: [DMESG-FAIL][11] ([i915#571]) -> [PASS][12]
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/fi-icl-guc/igt@i915_selftest@live_gt_pm.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/fi-icl-guc/igt@i915_selftest@live_gt_pm.html
#### Warnings ####
* igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
- fi-kbl-x1275: [DMESG-WARN][13] ([i915#62] / [i915#92] / [i915#95]) -> [DMESG-WARN][14] ([i915#62] / [i915#92]) +5 similar issues
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/fi-kbl-x1275/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/fi-kbl-x1275/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html
* igt@kms_flip@basic-flip-vs-modeset:
- fi-kbl-x1275: [DMESG-WARN][15] ([i915#62] / [i915#92]) -> [DMESG-WARN][16] ([i915#62] / [i915#92] / [i915#95]) +4 similar issues
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/fi-kbl-x1275/igt@kms_flip@basic-flip-vs-modeset.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/fi-kbl-x1275/igt@kms_flip@basic-flip-vs-modeset.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#111735]: https://bugs.freedesktop.org/show_bug.cgi?id=111735
[i915#178]: https://gitlab.freedesktop.org/drm/intel/issues/178
[i915#45]: https://gitlab.freedesktop.org/drm/intel/issues/45
[i915#553]: https://gitlab.freedesktop.org/drm/intel/issues/553
[i915#571]: https://gitlab.freedesktop.org/drm/intel/issues/571
[i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62
[i915#694]: https://gitlab.freedesktop.org/drm/intel/issues/694
[i915#707]: https://gitlab.freedesktop.org/drm/intel/issues/707
[i915#725]: https://gitlab.freedesktop.org/drm/intel/issues/725
[i915#92]: https://gitlab.freedesktop.org/drm/intel/issues/92
[i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95
Participating hosts (53 -> 46)
------------------------------
Additional (1): fi-hsw-4770r
Missing (8): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-tgl-y fi-byt-clapper fi-bdw-samus
Build changes
-------------
* CI: CI-20190529 -> None
* Linux: CI_DRM_7534 -> Patchwork_15681
CI-20190529: 20190529
CI_DRM_7534: 66a6222c16abb82d6a4480b0a7ff8e375cc6a3a6 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5342: 3e43fb3fa97ce25819444d63848439acf6e397b5 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_15681: b60deedca8ec5379c3c39da494f30c0a8684d9b2 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
b60deedca8ec HAX: force enable_guc=2 and WA i915#571
5ad83c593530 drm/i915/guc: unify notify() functions
396ef783e1de drm/i915/guc: remove function pointers for send/receive calls
6a2cdf54548f drm/i915/guc/ct: stop expecting multiple CT channels
3f4d1cd323a6 drm/i915/guc: Merge communication_stop and communication_disable
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Intel-gfx] ✗ Fi.CI.IGT: failure for Simplify GuC communication handling
2019-12-10 21:09 [Intel-gfx] [PATCH 0/5] Simplify GuC communication handling Daniele Ceraolo Spurio
` (6 preceding siblings ...)
2019-12-11 5:54 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
@ 2019-12-11 11:33 ` Patchwork
2019-12-11 11:52 ` Chris Wilson
7 siblings, 1 reply; 16+ messages in thread
From: Patchwork @ 2019-12-11 11:33 UTC (permalink / raw)
To: Daniele Ceraolo Spurio; +Cc: intel-gfx
== Series Details ==
Series: Simplify GuC communication handling
URL : https://patchwork.freedesktop.org/series/70717/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_7534_full -> Patchwork_15681_full
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_15681_full absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_15681_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_15681_full:
### IGT changes ###
#### Possible regressions ####
* igt@gem_exec_parallel@rcs0-contexts:
- shard-iclb: [PASS][1] -> [CRASH][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-iclb4/igt@gem_exec_parallel@rcs0-contexts.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-iclb7/igt@gem_exec_parallel@rcs0-contexts.html
Known issues
------------
Here are the changes found in Patchwork_15681_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_ctx_persistence@bcs0-mixed-process:
- shard-skl: [PASS][3] -> [FAIL][4] ([i915#679])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-skl1/igt@gem_ctx_persistence@bcs0-mixed-process.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-skl5/igt@gem_ctx_persistence@bcs0-mixed-process.html
* igt@gem_ctx_persistence@vcs1-queued:
- shard-iclb: [PASS][5] -> [SKIP][6] ([fdo#109276] / [fdo#112080]) +3 similar issues
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-iclb1/igt@gem_ctx_persistence@vcs1-queued.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-iclb3/igt@gem_ctx_persistence@vcs1-queued.html
* igt@gem_ctx_shared@q-smoketest-bsd1:
- shard-tglb: [PASS][7] -> [INCOMPLETE][8] ([fdo#111735])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-tglb5/igt@gem_ctx_shared@q-smoketest-bsd1.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-tglb9/igt@gem_ctx_shared@q-smoketest-bsd1.html
* igt@gem_eio@in-flight-10ms:
- shard-tglb: [PASS][9] -> [INCOMPLETE][10] ([i915#435] / [i915#534])
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-tglb3/igt@gem_eio@in-flight-10ms.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-tglb6/igt@gem_eio@in-flight-10ms.html
* igt@gem_exec_capture@capture-bsd:
- shard-iclb: [PASS][11] -> [SKIP][12] ([fdo#112146])
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-iclb8/igt@gem_exec_capture@capture-bsd.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-iclb1/igt@gem_exec_capture@capture-bsd.html
* igt@gem_exec_parallel@fds:
- shard-tglb: [PASS][13] -> [INCOMPLETE][14] ([i915#470])
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-tglb8/igt@gem_exec_parallel@fds.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-tglb6/igt@gem_exec_parallel@fds.html
* igt@gem_exec_parse_blt@allowed-all:
- shard-glk: [PASS][15] -> [DMESG-WARN][16] ([i915#716])
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-glk9/igt@gem_exec_parse_blt@allowed-all.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-glk2/igt@gem_exec_parse_blt@allowed-all.html
* igt@gem_exec_schedule@preempt-queue-bsd1:
- shard-iclb: [PASS][17] -> [SKIP][18] ([fdo#109276]) +15 similar issues
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-iclb4/igt@gem_exec_schedule@preempt-queue-bsd1.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-iclb7/igt@gem_exec_schedule@preempt-queue-bsd1.html
* igt@gem_exec_schedule@smoketest-bsd1:
- shard-tglb: [PASS][19] -> [INCOMPLETE][20] ([i915#463])
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-tglb2/igt@gem_exec_schedule@smoketest-bsd1.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-tglb3/igt@gem_exec_schedule@smoketest-bsd1.html
* igt@gem_ppgtt@flink-and-close-vma-leak:
- shard-kbl: [PASS][21] -> [FAIL][22] ([i915#644])
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-kbl3/igt@gem_ppgtt@flink-and-close-vma-leak.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-kbl6/igt@gem_ppgtt@flink-and-close-vma-leak.html
- shard-iclb: [PASS][23] -> [FAIL][24] ([i915#644])
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-iclb1/igt@gem_ppgtt@flink-and-close-vma-leak.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-iclb3/igt@gem_ppgtt@flink-and-close-vma-leak.html
* igt@gem_userptr_blits@sync-unmap-cycles:
- shard-snb: [PASS][25] -> [DMESG-WARN][26] ([fdo#111870])
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-snb1/igt@gem_userptr_blits@sync-unmap-cycles.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-snb1/igt@gem_userptr_blits@sync-unmap-cycles.html
* igt@i915_pm_dc@dc6-dpms:
- shard-iclb: [PASS][27] -> [FAIL][28] ([i915#454])
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-iclb6/igt@i915_pm_dc@dc6-dpms.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-iclb3/igt@i915_pm_dc@dc6-dpms.html
* igt@i915_selftest@live_requests:
- shard-tglb: [PASS][29] -> [INCOMPLETE][30] ([fdo#112057])
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-tglb5/igt@i915_selftest@live_requests.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-tglb1/igt@i915_selftest@live_requests.html
* igt@i915_selftest@mock_sanitycheck:
- shard-snb: [PASS][31] -> [DMESG-WARN][32] ([i915#747])
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-snb4/igt@i915_selftest@mock_sanitycheck.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-snb2/igt@i915_selftest@mock_sanitycheck.html
* igt@kms_cursor_crc@pipe-a-cursor-64x64-sliding:
- shard-skl: [PASS][33] -> [FAIL][34] ([i915#54]) +1 similar issue
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-skl1/igt@kms_cursor_crc@pipe-a-cursor-64x64-sliding.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-skl5/igt@kms_cursor_crc@pipe-a-cursor-64x64-sliding.html
* igt@kms_cursor_crc@pipe-a-cursor-suspend:
- shard-skl: [PASS][35] -> [INCOMPLETE][36] ([i915#300])
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-skl8/igt@kms_cursor_crc@pipe-a-cursor-suspend.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-skl4/igt@kms_cursor_crc@pipe-a-cursor-suspend.html
* igt@kms_draw_crc@draw-method-xrgb2101010-render-xtiled:
- shard-skl: [PASS][37] -> [INCOMPLETE][38] ([i915#646])
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-skl4/igt@kms_draw_crc@draw-method-xrgb2101010-render-xtiled.html
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-skl6/igt@kms_draw_crc@draw-method-xrgb2101010-render-xtiled.html
* igt@kms_flip@flip-vs-expired-vblank-interruptible:
- shard-glk: [PASS][39] -> [FAIL][40] ([i915#79])
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-glk1/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-glk4/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
* igt@kms_flip@plain-flip-fb-recreate-interruptible:
- shard-skl: [PASS][41] -> [FAIL][42] ([i915#34])
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-skl9/igt@kms_flip@plain-flip-fb-recreate-interruptible.html
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-skl7/igt@kms_flip@plain-flip-fb-recreate-interruptible.html
* igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-shrfb-draw-render:
- shard-tglb: [PASS][43] -> [FAIL][44] ([i915#49]) +1 similar issue
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-tglb9/igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-shrfb-draw-render.html
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-tglb8/igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-shrfb-draw-render.html
* igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-fullscreen:
- shard-tglb: [PASS][45] -> [INCOMPLETE][46] ([i915#435] / [i915#667])
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-tglb9/igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-fullscreen.html
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-tglb8/igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-fullscreen.html
* igt@kms_plane@pixel-format-pipe-a-planes:
- shard-kbl: [PASS][47] -> [INCOMPLETE][48] ([fdo#103665])
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-kbl3/igt@kms_plane@pixel-format-pipe-a-planes.html
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-kbl2/igt@kms_plane@pixel-format-pipe-a-planes.html
* igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
- shard-skl: [PASS][49] -> [FAIL][50] ([fdo#108145])
[49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-skl1/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
[50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-skl9/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
* igt@perf_pmu@busy-check-all-vcs1:
- shard-iclb: [PASS][51] -> [SKIP][52] ([fdo#112080]) +6 similar issues
[51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-iclb4/igt@perf_pmu@busy-check-all-vcs1.html
[52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-iclb3/igt@perf_pmu@busy-check-all-vcs1.html
#### Possible fixes ####
* igt@gem_ctx_isolation@rcs0-s3:
- shard-kbl: [DMESG-WARN][53] ([i915#180]) -> [PASS][54] +11 similar issues
[53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-kbl4/igt@gem_ctx_isolation@rcs0-s3.html
[54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-kbl6/igt@gem_ctx_isolation@rcs0-s3.html
* igt@gem_ctx_shared@exec-single-timeline-bsd:
- shard-iclb: [SKIP][55] ([fdo#110841]) -> [PASS][56]
[55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-iclb4/igt@gem_ctx_shared@exec-single-timeline-bsd.html
[56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-iclb3/igt@gem_ctx_shared@exec-single-timeline-bsd.html
* igt@gem_eio@in-flight-suspend:
- shard-skl: [INCOMPLETE][57] ([i915#69]) -> [PASS][58] +1 similar issue
[57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-skl10/igt@gem_eio@in-flight-suspend.html
[58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-skl3/igt@gem_eio@in-flight-suspend.html
* {igt@gem_exec_balancer@bonded-chain}:
- shard-iclb: [FAIL][59] ([i915#669]) -> [PASS][60]
[59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-iclb1/igt@gem_exec_balancer@bonded-chain.html
[60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-iclb3/igt@gem_exec_balancer@bonded-chain.html
* {igt@gem_exec_schedule@pi-distinct-iova-bsd2}:
- shard-iclb: [SKIP][61] ([fdo#109276]) -> [PASS][62] +1 similar issue
[61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-iclb8/igt@gem_exec_schedule@pi-distinct-iova-bsd2.html
[62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-iclb1/igt@gem_exec_schedule@pi-distinct-iova-bsd2.html
* igt@gem_exec_schedule@preempt-queue-contexts-chain-bsd:
- shard-iclb: [SKIP][63] ([fdo#112146]) -> [PASS][64] +1 similar issue
[63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-iclb4/igt@gem_exec_schedule@preempt-queue-contexts-chain-bsd.html
[64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-iclb3/igt@gem_exec_schedule@preempt-queue-contexts-chain-bsd.html
* igt@gem_ppgtt@flink-and-close-vma-leak:
- shard-skl: [FAIL][65] ([i915#644]) -> [PASS][66]
[65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-skl7/igt@gem_ppgtt@flink-and-close-vma-leak.html
[66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-skl5/igt@gem_ppgtt@flink-and-close-vma-leak.html
* igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy:
- shard-snb: [DMESG-WARN][67] ([fdo#111870]) -> [PASS][68] +1 similar issue
[67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-snb2/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy.html
[68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-snb7/igt@gem_userptr_blits@map-fixed-invalidate-overlap-busy.html
* igt@i915_selftest@mock_sanitycheck:
- shard-skl: [DMESG-WARN][69] ([i915#747]) -> [PASS][70]
[69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-skl2/igt@i915_selftest@mock_sanitycheck.html
[70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-skl7/igt@i915_selftest@mock_sanitycheck.html
* igt@kms_atomic_transition@plane-all-modeset-transition:
- shard-hsw: [INCOMPLETE][71] ([i915#61]) -> [PASS][72]
[71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-hsw7/igt@kms_atomic_transition@plane-all-modeset-transition.html
[72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-hsw2/igt@kms_atomic_transition@plane-all-modeset-transition.html
* igt@kms_color@pipe-b-ctm-blue-to-red:
- shard-skl: [DMESG-WARN][73] ([i915#109]) -> [PASS][74]
[73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-skl7/igt@kms_color@pipe-b-ctm-blue-to-red.html
[74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-skl3/igt@kms_color@pipe-b-ctm-blue-to-red.html
* igt@kms_cursor_crc@pipe-b-cursor-256x85-sliding:
- shard-skl: [FAIL][75] ([i915#54]) -> [PASS][76] +2 similar issues
[75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-skl9/igt@kms_cursor_crc@pipe-b-cursor-256x85-sliding.html
[76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-skl4/igt@kms_cursor_crc@pipe-b-cursor-256x85-sliding.html
* igt@kms_cursor_crc@pipe-c-cursor-256x85-sliding:
- shard-apl: [DMESG-WARN][77] ([IGT#6]) -> [PASS][78]
[77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-apl2/igt@kms_cursor_crc@pipe-c-cursor-256x85-sliding.html
[78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-apl2/igt@kms_cursor_crc@pipe-c-cursor-256x85-sliding.html
* igt@kms_flip@flip-vs-expired-vblank:
- shard-glk: [FAIL][79] ([i915#79]) -> [PASS][80]
[79]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-glk9/igt@kms_flip@flip-vs-expired-vblank.html
[80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-glk2/igt@kms_flip@flip-vs-expired-vblank.html
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-mmap-gtt:
- shard-tglb: [INCOMPLETE][81] ([i915#474] / [i915#667]) -> [PASS][82]
[81]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-tglb5/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-mmap-gtt.html
[82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-tglb3/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-mmap-gtt.html
* igt@kms_frontbuffer_tracking@fbcpsr-suspend:
- shard-tglb: [INCOMPLETE][83] ([i915#456] / [i915#460] / [i915#474]) -> [PASS][84]
[83]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-tglb1/igt@kms_frontbuffer_tracking@fbcpsr-suspend.html
[84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-tglb1/igt@kms_frontbuffer_tracking@fbcpsr-suspend.html
* igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
- shard-tglb: [INCOMPLETE][85] ([i915#456] / [i915#460]) -> [PASS][86] +1 similar issue
[85]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-tglb8/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html
[86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-tglb2/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html
- shard-apl: [DMESG-WARN][87] ([i915#180]) -> [PASS][88] +1 similar issue
[87]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-apl6/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html
[88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-apl2/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes.html
* igt@kms_plane@plane-panning-bottom-right-suspend-pipe-d-planes:
- shard-tglb: [INCOMPLETE][89] ([i915#460]) -> [PASS][90]
[89]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-tglb8/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-d-planes.html
[90]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-tglb6/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-d-planes.html
* igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
- shard-skl: [FAIL][91] ([fdo#108145]) -> [PASS][92]
[91]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-skl10/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
[92]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-skl3/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
* igt@perf_pmu@busy-accuracy-98-vcs1:
- shard-iclb: [SKIP][93] ([fdo#112080]) -> [PASS][94]
[93]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-iclb8/igt@perf_pmu@busy-accuracy-98-vcs1.html
[94]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-iclb1/igt@perf_pmu@busy-accuracy-98-vcs1.html
#### Warnings ####
* igt@gem_ctx_isolation@vcs2-clean:
- shard-tglb: [SKIP][95] ([fdo#112080]) -> [SKIP][96] ([fdo#111912] / [fdo#112080])
[95]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-tglb9/igt@gem_ctx_isolation@vcs2-clean.html
[96]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-tglb8/igt@gem_ctx_isolation@vcs2-clean.html
* igt@gem_eio@kms:
- shard-snb: [INCOMPLETE][97] ([i915#82]) -> [DMESG-WARN][98] ([i915#444])
[97]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-snb7/igt@gem_eio@kms.html
[98]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-snb6/igt@gem_eio@kms.html
* igt@kms_plane@pixel-format-pipe-b-planes-source-clamping:
- shard-skl: [INCOMPLETE][99] ([i915#648]) -> [INCOMPLETE][100] ([fdo#112391] / [i915#648] / [i915#667])
[99]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-skl6/igt@kms_plane@pixel-format-pipe-b-planes-source-clamping.html
[100]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-skl7/igt@kms_plane@pixel-format-pipe-b-planes-source-clamping.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[IGT#6]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/6
[fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
[fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
[fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
[fdo#110841]: https://bugs.freedesktop.org/show_bug.cgi?id=110841
[fdo#111735]: https://bugs.freedesktop.org/show_bug.cgi?id=111735
[fdo#111870]: https://bugs.freedesktop.org/show_bug.cgi?id=111870
[fdo#111912]: https://bugs.freedesktop.org/show_bug.cgi?id=111912
[fdo#112057]: https://bugs.freedesktop.org/show_bug.cgi?id=112057
[fdo#112080]: https://bugs.freedesktop.org/show_bug.cgi?id=112080
[fdo#112146]: https://bugs.freedesktop.org/show_bug.cgi?id=112146
[fdo#112391]: https://bugs.freedesktop.org/show_bug.cgi?id=112391
[i915#109]: https://gitlab.freedesktop.org/drm/intel/issues/109
[i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
[i915#300]: https://gitlab.freedesktop.org/drm/intel/issues/300
[i915#34]: https://gitlab.freedesktop.org/drm/intel/issues/34
[i915#435]: https://gitlab.freedesktop.org/drm/intel/issues/435
[i915#444]: https://gitlab.freedesktop.org/drm/intel/issues/444
[i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
[i915#456]: https://gitlab.freedesktop.org/drm/intel/issues/456
[i915#460]: https://gitlab.freedesktop.org/drm/intel/issues/460
[i915#463]: https://gitlab.freedesktop.org/drm/intel/issues/463
[i915#470]: https://gitlab.freedesktop.org/drm/intel/issues/470
[i915#474]: https://gitlab.freedesktop.org/drm/intel/issues/474
[i915#49]: https://gitlab.freedesktop.org/drm/intel/issues/49
[i915#534]: https://gitlab.freedesktop.org/drm/intel/issues/534
[i915#54]: https://gitlab.freedesktop.org/drm/intel/issues/54
[i915#61]: https://gitlab.freedesktop.org/drm/intel/issues/61
[i915#644]: https://gitlab.freedesktop.org/drm/intel/issues/644
[i915#646]: https://gitlab.freedesktop.org/drm/intel/issues/646
[i915#648]: https://gitlab.freedesktop.org/drm/intel/issues/648
[i915#667]: https://gitlab.freedesktop.org/drm/intel/issues/667
[i915#669]: https://gitlab.freedesktop.org/drm/intel/issues/669
[i915#677]: https://gitlab.freedesktop.org/drm/intel/issues/677
[i915#679]: https://gitlab.freedesktop.org/drm/intel/issues/679
[i915#69]: https://gitlab.freedesktop.org/drm/intel/issues/69
[i915#716]: https://gitlab.freedesktop.org/drm/intel/issues/716
[i915#747]: https://gitlab.freedesktop.org/drm/intel/issues/747
[i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
[i915#82]: https://gitlab.freedesktop.org/drm/intel/issues/82
Participating hosts (11 -> 11)
------------------------------
No changes in participating hosts
Build changes
-------------
* CI: CI-20190529 -> None
* Linux: CI_DRM_7534 -> Patchwork_15681
CI-20190529: 20190529
CI_DRM_7534: 66a6222c16abb82d6a4480b0a7ff8e375cc6a3a6 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5342: 3e43fb3fa97ce25819444d63848439acf6e397b5 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_15681: b60deedca8ec5379c3c39da494f30c0a8684d9b2 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] ✗ Fi.CI.IGT: failure for Simplify GuC communication handling
2019-12-11 11:33 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
@ 2019-12-11 11:52 ` Chris Wilson
0 siblings, 0 replies; 16+ messages in thread
From: Chris Wilson @ 2019-12-11 11:52 UTC (permalink / raw)
To: Daniele Ceraolo Spurio, Patchwork, intel-gfx; +Cc: intel-gfx
Quoting Patchwork (2019-12-11 11:33:35)
> #### Possible regressions ####
>
> * igt@gem_exec_parallel@rcs0-contexts:
> - shard-iclb: [PASS][1] -> [CRASH][2]
> [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7534/shard-iclb4/igt@gem_exec_parallel@rcs0-contexts.html
> [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15681/shard-iclb7/igt@gem_exec_parallel@rcs0-contexts.html
Where did __gem_execbuf() == -25 [ENOTTY] come from?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH 1/5] drm/i915/guc: Merge communication_stop and communication_disable
2019-12-10 21:09 ` [Intel-gfx] [PATCH 1/5] drm/i915/guc: Merge communication_stop and communication_disable Daniele Ceraolo Spurio
@ 2019-12-11 13:12 ` Michal Wajdeczko
0 siblings, 0 replies; 16+ messages in thread
From: Michal Wajdeczko @ 2019-12-11 13:12 UTC (permalink / raw)
To: intel-gfx, Daniele Ceraolo Spurio
On Tue, 10 Dec 2019 22:09:15 +0100, Daniele Ceraolo Spurio
<daniele.ceraolospurio@intel.com> wrote:
> The only difference from the GuC POV between guc_communication_stop and
> guc_communication_disable is that the former can be called after GuC
> has been reset. Instead of having two separate paths, we can just skip
> the call into GuC in the disabling path and re-use that.
>
> Note that by using the disable() path instead of the stop() one there
> are two additional changes in SW side for the stop path:
>
> - interrupts are now disabled before disabling the CT, which is ok
> because we do not want interrupts with CT disabled;
> - guc_get_mmio_msg() is called in the stop case as well, which is ok
> because if there are errors before the reset we do want to record
> them.
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: John Harrison <John.C.Harrison@Intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> ---
Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH 2/5] drm/i915/guc/ct: stop expecting multiple CT channels
2019-12-10 21:09 ` [Intel-gfx] [PATCH 2/5] drm/i915/guc/ct: stop expecting multiple CT channels Daniele Ceraolo Spurio
@ 2019-12-11 13:43 ` Michal Wajdeczko
2019-12-11 17:47 ` Daniele Ceraolo Spurio
0 siblings, 1 reply; 16+ messages in thread
From: Michal Wajdeczko @ 2019-12-11 13:43 UTC (permalink / raw)
To: intel-gfx, Daniele Ceraolo Spurio
On Tue, 10 Dec 2019 22:09:16 +0100, Daniele Ceraolo Spurio
<daniele.ceraolospurio@intel.com> wrote:
> The GuC supports having multiple CT buffer pairs and we designed our
> implementation with that in mind. However, the different channels are not
> processed in parallel within the GuC, so there is very little advantage
> in having multiple channels (independent locks?), compared to the
> drawbacks (one channel can starve the other if messages keep being
> submitted to it). Given this, it is unlikely we'll ever add a second
> channel and therefore we can simplify our code by removing the
> flexibility.
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: John Harrison <John.C.Harrison@Intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> ---
> drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 276 +++++++++-------------
> drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h | 39 +--
> 2 files changed, 118 insertions(+), 197 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
> index f74ba4750a94..96ce6d74f0b2 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
> @@ -37,13 +37,10 @@ static void ct_incoming_request_worker_func(struct
> work_struct *w);
> */
> void intel_guc_ct_init_early(struct intel_guc_ct *ct)
> {
> - /* we're using static channel owners */
> - ct->host_channel.owner = CTB_OWNER_HOST;
> -
> - spin_lock_init(&ct->lock);
> - INIT_LIST_HEAD(&ct->pending_requests);
> - INIT_LIST_HEAD(&ct->incoming_requests);
> - INIT_WORK(&ct->worker, ct_incoming_request_worker_func);
> + spin_lock_init(&ct->requests.lock);
> + INIT_LIST_HEAD(&ct->requests.pending);
> + INIT_LIST_HEAD(&ct->requests.incoming);
> + INIT_WORK(&ct->requests.worker, ct_incoming_request_worker_func);
> }
> static inline struct intel_guc *ct_to_guc(struct intel_guc_ct *ct)
> @@ -64,14 +61,14 @@ static inline const char
> *guc_ct_buffer_type_to_str(u32 type)
> }
> static void guc_ct_buffer_desc_init(struct guc_ct_buffer_desc *desc,
> - u32 cmds_addr, u32 size, u32 owner)
> + u32 cmds_addr, u32 size)
> {
> - CT_DEBUG_DRIVER("CT: desc %p init addr=%#x size=%u owner=%u\n",
> - desc, cmds_addr, size, owner);
> + CT_DEBUG_DRIVER("CT: desc %p init addr=%#x size=%u\n",
> + desc, cmds_addr, size);
please drop %p
> memset(desc, 0, sizeof(*desc));
> desc->addr = cmds_addr;
> desc->size = size;
> - desc->owner = owner;
> + desc->owner = CTB_OWNER_HOST;
> }
> static void guc_ct_buffer_desc_reset(struct guc_ct_buffer_desc *desc)
> @@ -104,12 +101,11 @@ static int guc_action_register_ct_buffer(struct
> intel_guc *guc,
> }
> static int guc_action_deregister_ct_buffer(struct intel_guc *guc,
> - u32 owner,
> u32 type)
> {
> u32 action[] = {
> INTEL_GUC_ACTION_DEREGISTER_COMMAND_TRANSPORT_BUFFER,
> - owner,
> + CTB_OWNER_HOST,
> type
> };
> int err;
> @@ -117,19 +113,28 @@ static int guc_action_deregister_ct_buffer(struct
> intel_guc *guc,
> /* Can't use generic send(), CT deregistration must go over MMIO */
> err = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action), NULL, 0);
> if (err)
> - DRM_ERROR("CT: deregister %s buffer failed; owner=%d err=%d\n",
> - guc_ct_buffer_type_to_str(type), owner, err);
> + DRM_ERROR("CT: deregister %s buffer failed; err=%d\n",
> + guc_ct_buffer_type_to_str(type), err);
> return err;
> }
> -static int ctch_init(struct intel_guc *guc,
> - struct intel_guc_ct_channel *ctch)
> +/**
> + * intel_guc_ct_init - Init CT communication
maybe to match other descriptions:
"intel_guc_ct_init - Init buffer based communication"
> + * @ct: pointer to CT struct
> + *
> + * Allocate memory required for communication via
> + * the CT channel.
CT channel ? maybe
"Allocate memory required for buffer based communication"
> + *
> + * Return: 0 on success, a negative errno code on failure.
> + */
> +int intel_guc_ct_init(struct intel_guc_ct *ct)
> {
> + struct intel_guc *guc = ct_to_guc(ct);
> void *blob;
> int err;
> int i;
> - GEM_BUG_ON(ctch->vma);
> + GEM_BUG_ON(ct->vma);
> /* We allocate 1 page to hold both descriptors and both buffers.
> * ___________.....................
> @@ -153,57 +158,67 @@ static int ctch_init(struct intel_guc *guc,
> * other code will need updating as well.
> */
> - err = intel_guc_allocate_and_map_vma(guc, PAGE_SIZE, &ctch->vma,
> &blob);
> + err = intel_guc_allocate_and_map_vma(guc, PAGE_SIZE, &ct->vma, &blob);
> if (err) {
> - CT_DEBUG_DRIVER("CT: channel %d initialization failed; err=%d\n",
> - ctch->owner, err);
> + DRM_ERROR("CT: channel allocation failed; err=%d\n", err);
> return err;
> }
> CT_DEBUG_DRIVER("CT: vma base=%#x\n",
> - intel_guc_ggtt_offset(guc, ctch->vma));
> + intel_guc_ggtt_offset(guc, ct->vma));
> /* store pointers to desc and cmds */
> - for (i = 0; i < ARRAY_SIZE(ctch->ctbs); i++) {
> - GEM_BUG_ON((i != CTB_SEND) && (i != CTB_RECV));
> - ctch->ctbs[i].desc = blob + PAGE_SIZE/4 * i;
> - ctch->ctbs[i].cmds = blob + PAGE_SIZE/4 * i + PAGE_SIZE/2;
> + for (i = 0; i < ARRAY_SIZE(ct->ctbs); i++) {
> + GEM_BUG_ON((i != CTB_SEND) && (i != CTB_RECV));
> + ct->ctbs[i].desc = blob + PAGE_SIZE/4 * i;
> + ct->ctbs[i].cmds = blob + PAGE_SIZE/4 * i + PAGE_SIZE/2;
> }
> return 0;
> }
> -static void ctch_fini(struct intel_guc *guc,
> - struct intel_guc_ct_channel *ctch)
> +/**
> + * intel_guc_ct_fini - Fini CT communication
> + * @ct: pointer to CT struct
> + *
> + * Deallocate memory required for communication via
> + * the CT channel.
> + */
> +void intel_guc_ct_fini(struct intel_guc_ct *ct)
> {
> - GEM_BUG_ON(ctch->enabled);
> + GEM_BUG_ON(ct->enabled);
> - i915_vma_unpin_and_release(&ctch->vma, I915_VMA_RELEASE_MAP);
> + i915_vma_unpin_and_release(&ct->vma, I915_VMA_RELEASE_MAP);
> }
> -static int ctch_enable(struct intel_guc *guc,
> - struct intel_guc_ct_channel *ctch)
> +/**
> + * intel_guc_ct_enable - Enable buffer based command transport.
> + * @ct: pointer to CT struct
> + *
> + * Return: 0 on success, a negative errno code on failure.
> + */
> +int intel_guc_ct_enable(struct intel_guc_ct *ct)
> {
> + struct intel_guc *guc = ct_to_guc(ct);
> u32 base;
> int err;
> int i;
> - GEM_BUG_ON(!ctch->vma);
> -
> - GEM_BUG_ON(ctch->enabled);
> + if (ct->enabled)
> + return 0;
btw, do we still need this check?
> /* vma should be already allocated and map'ed */
> - base = intel_guc_ggtt_offset(guc, ctch->vma);
> + GEM_BUG_ON(!ct->vma);
> + base = intel_guc_ggtt_offset(guc, ct->vma);
> /* (re)initialize descriptors
> * cmds buffers are in the second half of the blob page
> */
> - for (i = 0; i < ARRAY_SIZE(ctch->ctbs); i++) {
> + for (i = 0; i < ARRAY_SIZE(ct->ctbs); i++) {
> GEM_BUG_ON((i != CTB_SEND) && (i != CTB_RECV));
> - guc_ct_buffer_desc_init(ctch->ctbs[i].desc,
> + guc_ct_buffer_desc_init(ct->ctbs[i].desc,
> base + PAGE_SIZE/4 * i + PAGE_SIZE/2,
> - PAGE_SIZE/4,
> - ctch->owner);
> + PAGE_SIZE/4);
> }
> /* register buffers, starting wirh RECV buffer
> @@ -221,40 +236,43 @@ static int ctch_enable(struct intel_guc *guc,
> if (unlikely(err))
> goto err_deregister;
> - ctch->enabled = true;
> + ct->enabled = true;
> return 0;
> err_deregister:
> guc_action_deregister_ct_buffer(guc,
> - ctch->owner,
> INTEL_GUC_CT_BUFFER_TYPE_RECV);
> err_out:
> - DRM_ERROR("CT: can't open channel %d; err=%d\n", ctch->owner, err);
> + DRM_ERROR("CT: can't open channel; err=%d\n", err);
> return err;
> }
> -static void ctch_disable(struct intel_guc *guc,
> - struct intel_guc_ct_channel *ctch)
> +/**
> + * intel_guc_ct_disable - Disable buffer based command transport.
> + * @ct: pointer to CT struct
> + */
> +void intel_guc_ct_disable(struct intel_guc_ct *ct)
> {
> - GEM_BUG_ON(!ctch->enabled);
> + struct intel_guc *guc = ct_to_guc(ct);
> - ctch->enabled = false;
> + if (!ct->enabled)
> + return;
> +
> + ct->enabled = false;
> if (intel_guc_is_running(guc)) {
> guc_action_deregister_ct_buffer(guc,
> - ctch->owner,
> INTEL_GUC_CT_BUFFER_TYPE_SEND);
> guc_action_deregister_ct_buffer(guc,
> - ctch->owner,
> INTEL_GUC_CT_BUFFER_TYPE_RECV);
> }
> }
> -static u32 ctch_get_next_fence(struct intel_guc_ct_channel *ctch)
> +static u32 ct_get_next_fence(struct intel_guc_ct *ct)
> {
> /* For now it's trivial */
> - return ++ctch->next_fence;
> + return ++ct->next_fence;
> }
> /**
> @@ -427,35 +445,34 @@ static int wait_for_ct_request_update(struct
> ct_request *req, u32 *status)
> return err;
> }
> -static int ctch_send(struct intel_guc_ct *ct,
> - struct intel_guc_ct_channel *ctch,
> - const u32 *action,
> - u32 len,
> - u32 *response_buf,
> - u32 response_buf_size,
> - u32 *status)
> +static int ct_send(struct intel_guc_ct *ct,
> + const u32 *action,
> + u32 len,
> + u32 *response_buf,
> + u32 response_buf_size,
> + u32 *status)
> {
> - struct intel_guc_ct_buffer *ctb = &ctch->ctbs[CTB_SEND];
> + struct intel_guc_ct_buffer *ctb = &ct->ctbs[CTB_SEND];
> struct guc_ct_buffer_desc *desc = ctb->desc;
> struct ct_request request;
> unsigned long flags;
> u32 fence;
> int err;
> - GEM_BUG_ON(!ctch->enabled);
> + GEM_BUG_ON(!ct->enabled);
> GEM_BUG_ON(!len);
> GEM_BUG_ON(len & ~GUC_CT_MSG_LEN_MASK);
> GEM_BUG_ON(!response_buf && response_buf_size);
> - fence = ctch_get_next_fence(ctch);
> + fence = ct_get_next_fence(ct);
> request.fence = fence;
> request.status = 0;
> request.response_len = response_buf_size;
> request.response_buf = response_buf;
> - spin_lock_irqsave(&ct->lock, flags);
> - list_add_tail(&request.link, &ct->pending_requests);
> - spin_unlock_irqrestore(&ct->lock, flags);
> + spin_lock_irqsave(&ct->requests.lock, flags);
> + list_add_tail(&request.link, &ct->requests.pending);
> + spin_unlock_irqrestore(&ct->requests.lock, flags);
> err = ctb_write(ctb, action, len, fence, !!response_buf);
> if (unlikely(err))
> @@ -488,9 +505,9 @@ static int ctch_send(struct intel_guc_ct *ct,
> }
> unlink:
> - spin_lock_irqsave(&ct->lock, flags);
> + spin_lock_irqsave(&ct->requests.lock, flags);
> list_del(&request.link);
> - spin_unlock_irqrestore(&ct->lock, flags);
> + spin_unlock_irqrestore(&ct->requests.lock, flags);
> return err;
> }
> @@ -502,14 +519,12 @@ int intel_guc_send_ct(struct intel_guc *guc, const
> u32 *action, u32 len,
> u32 *response_buf, u32 response_buf_size)
> {
> struct intel_guc_ct *ct = &guc->ct;
> - struct intel_guc_ct_channel *ctch = &ct->host_channel;
> u32 status = ~0; /* undefined */
> int ret;
> mutex_lock(&guc->send_mutex);
> - ret = ctch_send(ct, ctch, action, len, response_buf, response_buf_size,
> - &status);
> + ret = ct_send(ct, action, len, response_buf, response_buf_size,
> &status);
> if (unlikely(ret < 0)) {
> DRM_ERROR("CT: send action %#X failed; err=%d status=%#X\n",
> action[0], ret, status);
> @@ -640,8 +655,8 @@ static int ct_handle_response(struct intel_guc_ct
> *ct, const u32 *msg)
> CT_DEBUG_DRIVER("CT: response fence %u status %#x\n", fence, status);
> - spin_lock(&ct->lock);
> - list_for_each_entry(req, &ct->pending_requests, link) {
> + spin_lock(&ct->requests.lock);
> + list_for_each_entry(req, &ct->requests.pending, link) {
> if (unlikely(fence != req->fence)) {
> CT_DEBUG_DRIVER("CT: request %u awaits response\n",
> req->fence);
> @@ -659,7 +674,7 @@ static int ct_handle_response(struct intel_guc_ct
> *ct, const u32 *msg)
> found = true;
> break;
> }
> - spin_unlock(&ct->lock);
> + spin_unlock(&ct->requests.lock);
> if (!found)
> DRM_ERROR("CT: unsolicited response %*ph\n", 4 * msglen, msg);
> @@ -697,13 +712,13 @@ static bool ct_process_incoming_requests(struct
> intel_guc_ct *ct)
> u32 *payload;
> bool done;
> - spin_lock_irqsave(&ct->lock, flags);
> - request = list_first_entry_or_null(&ct->incoming_requests,
> + spin_lock_irqsave(&ct->requests.lock, flags);
> + request = list_first_entry_or_null(&ct->requests.incoming,
> struct ct_incoming_request, link);
> if (request)
> list_del(&request->link);
> - done = !!list_empty(&ct->incoming_requests);
> - spin_unlock_irqrestore(&ct->lock, flags);
> + done = !!list_empty(&ct->requests.incoming);
> + spin_unlock_irqrestore(&ct->requests.lock, flags);
> if (!request)
> return true;
> @@ -721,12 +736,13 @@ static bool ct_process_incoming_requests(struct
> intel_guc_ct *ct)
> static void ct_incoming_request_worker_func(struct work_struct *w)
> {
> - struct intel_guc_ct *ct = container_of(w, struct intel_guc_ct, worker);
> + struct intel_guc_ct *ct =
> + container_of(w, struct intel_guc_ct, requests.worker);
> bool done;
> done = ct_process_incoming_requests(ct);
> if (!done)
> - queue_work(system_unbound_wq, &ct->worker);
> + queue_work(system_unbound_wq, &ct->requests.worker);
> }
> /**
> @@ -764,22 +780,26 @@ static int ct_handle_request(struct intel_guc_ct
> *ct, const u32 *msg)
> }
> memcpy(request->msg, msg, 4 * msglen);
> - spin_lock_irqsave(&ct->lock, flags);
> - list_add_tail(&request->link, &ct->incoming_requests);
> - spin_unlock_irqrestore(&ct->lock, flags);
> + spin_lock_irqsave(&ct->requests.lock, flags);
> + list_add_tail(&request->link, &ct->requests.incoming);
> + spin_unlock_irqrestore(&ct->requests.lock, flags);
> - queue_work(system_unbound_wq, &ct->worker);
> + queue_work(system_unbound_wq, &ct->requests.worker);
> return 0;
> }
> -static void ct_process_host_channel(struct intel_guc_ct *ct)
> +/*
> + * When we're communicating with the GuC over CT, GuC uses events
> + * to notify us about new messages being posted on the RECV buffer.
> + */
> +void intel_guc_to_host_event_handler_ct(struct intel_guc *guc)
> {
> - struct intel_guc_ct_channel *ctch = &ct->host_channel;
> - struct intel_guc_ct_buffer *ctb = &ctch->ctbs[CTB_RECV];
> + struct intel_guc_ct *ct = &guc->ct;
> + struct intel_guc_ct_buffer *ctb = &ct->ctbs[CTB_RECV];
> u32 msg[GUC_CT_MSG_LEN_MASK + 1]; /* one extra dw for the header */
> int err = 0;
> - if (!ctch->enabled)
> + if (!ct->enabled)
> return;
> do {
> @@ -798,87 +818,3 @@ static void ct_process_host_channel(struct
> intel_guc_ct *ct)
> ctb->desc->is_in_error = 1;
> }
> }
> -
> -/*
> - * When we're communicating with the GuC over CT, GuC uses events
> - * to notify us about new messages being posted on the RECV buffer.
> - */
> -void intel_guc_to_host_event_handler_ct(struct intel_guc *guc)
> -{
> - struct intel_guc_ct *ct = &guc->ct;
> -
> - ct_process_host_channel(ct);
> -}
> -
> -/**
> - * intel_guc_ct_init - Init CT communication
> - * @ct: pointer to CT struct
> - *
> - * Allocate memory required for communication via
> - * the CT channel.
> - *
> - * Return: 0 on success, a negative errno code on failure.
> - */
> -int intel_guc_ct_init(struct intel_guc_ct *ct)
> -{
> - struct intel_guc *guc = ct_to_guc(ct);
> - struct intel_guc_ct_channel *ctch = &ct->host_channel;
> - int err;
> -
> - err = ctch_init(guc, ctch);
> - if (unlikely(err)) {
> - DRM_ERROR("CT: can't open channel %d; err=%d\n",
> - ctch->owner, err);
> - return err;
> - }
> -
> - GEM_BUG_ON(!ctch->vma);
> - return 0;
> -}
> -
> -/**
> - * intel_guc_ct_fini - Fini CT communication
> - * @ct: pointer to CT struct
> - *
> - * Deallocate memory required for communication via
> - * the CT channel.
> - */
> -void intel_guc_ct_fini(struct intel_guc_ct *ct)
> -{
> - struct intel_guc *guc = ct_to_guc(ct);
> - struct intel_guc_ct_channel *ctch = &ct->host_channel;
> -
> - ctch_fini(guc, ctch);
> -}
> -
> -/**
> - * intel_guc_ct_enable - Enable buffer based command transport.
> - * @ct: pointer to CT struct
> - *
> - * Return: 0 on success, a negative errno code on failure.
> - */
> -int intel_guc_ct_enable(struct intel_guc_ct *ct)
> -{
> - struct intel_guc *guc = ct_to_guc(ct);
> - struct intel_guc_ct_channel *ctch = &ct->host_channel;
> -
> - if (ctch->enabled)
> - return 0;
> -
> - return ctch_enable(guc, ctch);
> -}
> -
> -/**
> - * intel_guc_ct_disable - Disable buffer based command transport.
> - * @ct: pointer to CT struct
> - */
> -void intel_guc_ct_disable(struct intel_guc_ct *ct)
> -{
> - struct intel_guc *guc = ct_to_guc(ct);
> - struct intel_guc_ct_channel *ctch = &ct->host_channel;
> -
> - if (!ctch->enabled)
> - return;
> -
> - ctch_disable(guc, ctch);
> -}
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
> index 77c80d6cc25d..4bb1d1fcc860 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
> @@ -35,44 +35,29 @@ struct intel_guc_ct_buffer {
> u32 *cmds;
> };
> +
> /** Represents pair of command transport buffers.
intel_guc_ct is now little more that old ctch
> *
> * Buffers go in pairs to allow bi-directional communication.
> * To simplify the code we place both of them in the same vma.
> * Buffers from the same pair must share unique owner id.
we'll have only one pair and one fixed owner,
maybe worth to rephrase whole description ?
> - *
> - * @vma: pointer to the vma with pair of CT buffers
> - * @ctbs: buffers for sending(0) and receiving(1) commands
> - * @owner: unique identifier
> - * @next_fence: fence to be used with next send command
> */
> -struct intel_guc_ct_channel {
> +struct intel_guc_ct {
> struct i915_vma *vma;
> - struct intel_guc_ct_buffer ctbs[2];
> - u32 owner;
> - u32 next_fence;
> bool enabled;
> -};
> -/** Holds all command transport channels.
> - *
> - * @host_channel: main channel used by the host
> - */
> -struct intel_guc_ct {
> - struct intel_guc_ct_channel host_channel;
> - /* other channels are tbd */
> -
> - /** @lock: protects pending requests list */
> - spinlock_t lock;
> -
> - /** @pending_requests: list of requests waiting for response */
> - struct list_head pending_requests;
> + /* buffers for sending(0) and receiving(1) commands */
> + struct intel_guc_ct_buffer ctbs[2];
> - /** @incoming_requests: list of incoming requests */
> - struct list_head incoming_requests;
> + /* fence to be used with next send command */
> + u32 next_fence;
fence is only used while sending requests,
so maybe move it under below requests struct ?
> - /** @worker: worker for handling incoming requests */
> - struct work_struct worker;
> + struct {
> + spinlock_t lock; /* protects pending requests list */
> + struct list_head pending; /* requests waiting for response */
> + struct list_head incoming; /* incoming requests */
> + struct work_struct worker; /* handler for incoming requests */
> + } requests;
maybe above change should be in separate patch to make diff smaller?
> };
> void intel_guc_ct_init_early(struct intel_guc_ct *ct);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH 3/5] drm/i915/guc: remove function pointers for send/receive calls
2019-12-10 21:09 ` [Intel-gfx] [PATCH 3/5] drm/i915/guc: remove function pointers for send/receive calls Daniele Ceraolo Spurio
@ 2019-12-11 14:04 ` Michal Wajdeczko
2019-12-11 18:01 ` Daniele Ceraolo Spurio
0 siblings, 1 reply; 16+ messages in thread
From: Michal Wajdeczko @ 2019-12-11 14:04 UTC (permalink / raw)
To: intel-gfx, Daniele Ceraolo Spurio
On Tue, 10 Dec 2019 22:09:17 +0100, Daniele Ceraolo Spurio
<daniele.ceraolospurio@intel.com> wrote:
> Since we started using CT buffers on all gens, the function pointers can
> only be set to either the _nop() or the _ct() functions. Since the
> _nop() case applies to when the CT are disabled, we can just handle that
> case in the _ct() functions and call them directly.
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: John Harrison <John.C.Harrison@Intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> ---
> drivers/gpu/drm/i915/gt/intel_gt_irq.c | 2 +-
> drivers/gpu/drm/i915/gt/uc/intel_guc.c | 22 +++-----------
> drivers/gpu/drm/i915/gt/uc/intel_guc.h | 29 -------------------
> drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 14 +++++++--
> drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h | 17 +++++++++--
> drivers/gpu/drm/i915/gt/uc/intel_guc_log.c | 6 ++--
> .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 1 -
> drivers/gpu/drm/i915/gt/uc/intel_uc.c | 18 ++++--------
> 8 files changed, 40 insertions(+), 69 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_irq.c
> b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
> index 332b12a574fb..3183b4426c7b 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_irq.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
> @@ -16,7 +16,7 @@
> static void guc_irq_handler(struct intel_guc *guc, u16 iir)
> {
> if (iir & GUC_INTR_GUC2HOST)
> - intel_guc_to_host_event_handler(guc);
> + intel_guc_to_host_event_handler_ct(guc);
> }
> static void
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> index 922a19635d20..eb94635eeecd 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> @@ -177,8 +177,6 @@ void intel_guc_init_early(struct intel_guc *guc)
> mutex_init(&guc->send_mutex);
> spin_lock_init(&guc->irq_lock);
> - guc->send = intel_guc_send_nop;
> - guc->handler = intel_guc_to_host_event_handler_nop;
> if (INTEL_GEN(i915) >= 11) {
> guc->notify = gen11_guc_raise_irq;
> guc->interrupts.reset = gen11_reset_guc_interrupts;
> @@ -403,18 +401,6 @@ void intel_guc_fini(struct intel_guc *guc)
> intel_uc_fw_cleanup_fetch(&guc->fw);
> }
> -int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32
> len,
> - u32 *response_buf, u32 response_buf_size)
> -{
> - WARN(1, "Unexpected send: action=%#x\n", *action);
> - return -ENODEV;
> -}
> -
> -void intel_guc_to_host_event_handler_nop(struct intel_guc *guc)
> -{
> - WARN(1, "Unexpected event: no suitable handler\n");
> -}
> -
> /*
> * This function implements the MMIO based host to GuC interface.
> */
> @@ -515,7 +501,7 @@ int intel_guc_sample_forcewake(struct intel_guc *guc)
> /* bit 0 and 1 are for Render and Media domain separately */
> action[1] = GUC_FORCEWAKE_RENDER | GUC_FORCEWAKE_MEDIA;
> - return intel_guc_send(guc, action, ARRAY_SIZE(action));
> + return intel_guc_send_ct(guc, action, ARRAY_SIZE(action));
> }
> /**
> @@ -536,7 +522,7 @@ int intel_guc_auth_huc(struct intel_guc *guc, u32
> rsa_offset)
> rsa_offset
> };
> - return intel_guc_send(guc, action, ARRAY_SIZE(action));
> + return intel_guc_send_ct(guc, action, ARRAY_SIZE(action));
> }
> /**
> @@ -573,7 +559,7 @@ int intel_guc_suspend(struct intel_guc *guc)
> intel_uncore_write(uncore, SOFT_SCRATCH(14),
> INTEL_GUC_SLEEP_STATE_INVALID_MASK);
> - ret = intel_guc_send(guc, action, ARRAY_SIZE(action));
> + ret = intel_guc_send_ct(guc, action, ARRAY_SIZE(action));
> if (ret)
> return ret;
> @@ -625,7 +611,7 @@ int intel_guc_resume(struct intel_guc *guc)
> if (!intel_guc_submission_is_enabled(guc))
> return 0;
> - return intel_guc_send(guc, action, ARRAY_SIZE(action));
> + return intel_guc_send_ct(guc, action, ARRAY_SIZE(action));
> }
> /**
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> index cd09c912e361..c0b32db1c6ad 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> @@ -70,40 +70,15 @@ struct intel_guc {
> /* To serialize the intel_guc_send actions */
> struct mutex send_mutex;
> - /* GuC's FW specific send function */
> - int (*send)(struct intel_guc *guc, const u32 *data, u32 len,
> - u32 *response_buf, u32 response_buf_size);
> -
> - /* GuC's FW specific event handler function */
> - void (*handler)(struct intel_guc *guc);
> -
> /* GuC's FW specific notify function */
> void (*notify)(struct intel_guc *guc);
> };
> -static
> -inline int intel_guc_send(struct intel_guc *guc, const u32 *action, u32
> len)
> -{
> - return guc->send(guc, action, len, NULL, 0);
> -}
> -
> -static inline int
> -intel_guc_send_and_receive(struct intel_guc *guc, const u32 *action,
> u32 len,
> - u32 *response_buf, u32 response_buf_size)
> -{
> - return guc->send(guc, action, len, response_buf, response_buf_size);
> -}
instead of dropping above inlines, I would rather just change them to:
return intel_guc_ct_send(&guc->ct, ...);
a) we will not have to change existing callers
b) we will maintain modularity (separation of ct code)
> -
> static inline void intel_guc_notify(struct intel_guc *guc)
> {
> guc->notify(guc);
> }
> -static inline void intel_guc_to_host_event_handler(struct intel_guc
> *guc)
> -{
> - guc->handler(guc);
intel_guc_ct_event_handler(&guc->ct); ?
> -}
> -
> /* GuC addresses above GUC_GGTT_TOP also don't map through the GTT */
> #define GUC_GGTT_TOP 0xFEE00000
> @@ -136,12 +111,8 @@ void intel_guc_init_send_regs(struct intel_guc
> *guc);
> void intel_guc_write_params(struct intel_guc *guc);
> int intel_guc_init(struct intel_guc *guc);
> void intel_guc_fini(struct intel_guc *guc);
> -int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32
> len,
> - u32 *response_buf, u32 response_buf_size);
> int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32
> len,
> u32 *response_buf, u32 response_buf_size);
> -void intel_guc_to_host_event_handler(struct intel_guc *guc);
> -void intel_guc_to_host_event_handler_nop(struct intel_guc *guc);
> int intel_guc_to_host_process_recv_msg(struct intel_guc *guc,
> const u32 *payload, u32 len);
> int intel_guc_sample_forcewake(struct intel_guc *guc);
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
> index 96ce6d74f0b2..60b19f83e153 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
> @@ -515,13 +515,19 @@ static int ct_send(struct intel_guc_ct *ct,
> /*
> * Command Transport (CT) buffer based GuC send function.
> */
> -int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len,
> - u32 *response_buf, u32 response_buf_size)
> +int intel_guc_send_and_receive_ct(struct intel_guc *guc, const u32
> *action,
to have proper modularization, this should be:
intel_guc_ct_send_and_receive(struct intel_guc_ct *ct, ...
or
intel_guc_ct_send(struct intel_guc_ct *ct, ...
> + u32 len, u32 *response_buf,
> + u32 response_buf_size)
> {
> struct intel_guc_ct *ct = &guc->ct;
> u32 status = ~0; /* undefined */
> int ret;
> + if (unlikely(!ct->enabled)) {
> + WARN(1, "Unexpected send: action=%#x\n", *action);
> + return -ENODEV;
> + }
> +
> mutex_lock(&guc->send_mutex);
> ret = ct_send(ct, action, len, response_buf, response_buf_size,
> &status);
> @@ -799,8 +805,10 @@ void intel_guc_to_host_event_handler_ct(struct
> intel_guc *guc)
> u32 msg[GUC_CT_MSG_LEN_MASK + 1]; /* one extra dw for the header */
> int err = 0;
> - if (!ct->enabled)
> + if (!ct->enabled) {
> + WARN(1, "Unexpected event: no suitable handler\n");
hmm, there is a handler, but CTB is not working ;)
> return;
> + }
> do {
> err = ctb_read(ctb, msg);
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
> index 4bb1d1fcc860..929483b1f013 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
> @@ -66,8 +66,21 @@ void intel_guc_ct_fini(struct intel_guc_ct *ct);
> int intel_guc_ct_enable(struct intel_guc_ct *ct);
> void intel_guc_ct_disable(struct intel_guc_ct *ct);
> -int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len,
> - u32 *response_buf, u32 response_buf_size);
> +static inline bool intel_guc_ct_enabled(struct intel_guc_ct *ct)
> +{
> + return ct->enabled;
> +}
> +
> +int
> +intel_guc_send_and_receive_ct(struct intel_guc *guc, const u32 *action,
> u32 len,
> + u32 *response_buf, u32 response_buf_size);
> +
> +static inline int
> +intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len)
> +{
> + return intel_guc_send_and_receive_ct(guc, action, len, NULL, 0);
> +}
> +
> void intel_guc_to_host_event_handler_ct(struct intel_guc *guc);
> #endif /* _INTEL_GUC_CT_H_ */
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
> index caed0d57e704..5938127fb129 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
> @@ -27,7 +27,7 @@ static int guc_action_flush_log_complete(struct
> intel_guc *guc)
> INTEL_GUC_ACTION_LOG_BUFFER_FILE_FLUSH_COMPLETE
> };
> - return intel_guc_send(guc, action, ARRAY_SIZE(action));
> + return intel_guc_send_ct(guc, action, ARRAY_SIZE(action));
> }
> static int guc_action_flush_log(struct intel_guc *guc)
> @@ -37,7 +37,7 @@ static int guc_action_flush_log(struct intel_guc *guc)
> 0
> };
> - return intel_guc_send(guc, action, ARRAY_SIZE(action));
> + return intel_guc_send_ct(guc, action, ARRAY_SIZE(action));
> }
> static int guc_action_control_log(struct intel_guc *guc, bool enable,
> @@ -52,7 +52,7 @@ static int guc_action_control_log(struct intel_guc
> *guc, bool enable,
> GEM_BUG_ON(verbosity > GUC_LOG_VERBOSITY_MAX);
> - return intel_guc_send(guc, action, ARRAY_SIZE(action));
> + return intel_guc_send_ct(guc, action, ARRAY_SIZE(action));
> }
> static inline struct intel_guc *log_to_guc(struct intel_guc_log *log)
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> index 172220e83079..fd7008bb128c 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> @@ -43,7 +43,6 @@
> * Firmware writes a success/fail code back to the action register after
> * processes the request. The kernel driver polls waiting for this
> update and
> * then proceeds.
> - * See intel_guc_send()
> *
> * Work Items:
> * There are several types of work items that the host may place into a
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> index 7566af8ab46e..18a5eaf3052c 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> @@ -123,6 +123,11 @@ static void __uc_free_load_err_log(struct intel_uc
> *uc)
> i915_gem_object_put(log);
> }
> +static inline bool guc_communication_enabled(struct intel_guc *guc)
> +{
> + return intel_guc_ct_enabled(&guc->ct);
> +}
if this is really needed, please move to intel_guc.h
> +
> /*
> * Events triggered while CT buffers are disabled are logged in the
> SCRATCH_15
> * register using the same bits used in the CT message payload. Since
> our
> @@ -158,7 +163,7 @@ static void guc_handle_mmio_msg(struct intel_guc
> *guc)
> struct drm_i915_private *i915 = guc_to_gt(guc)->i915;
> /* we need communication to be enabled to reply to GuC */
> - GEM_BUG_ON(guc->handler == intel_guc_to_host_event_handler_nop);
> + GEM_BUG_ON(!guc_communication_enabled(guc));
> if (!guc->mmio_msg)
> return;
> @@ -185,11 +190,6 @@ static void guc_disable_interrupts(struct intel_guc
> *guc)
> guc->interrupts.disable(guc);
> }
> -static inline bool guc_communication_enabled(struct intel_guc *guc)
> -{
> - return guc->send != intel_guc_send_nop;
> -}
> -
> static int guc_enable_communication(struct intel_guc *guc)
> {
> struct drm_i915_private *i915 = guc_to_gt(guc)->i915;
> @@ -205,9 +205,6 @@ static int guc_enable_communication(struct intel_guc
> *guc)
> if (ret)
> return ret;
> - guc->send = intel_guc_send_ct;
> - guc->handler = intel_guc_to_host_event_handler_ct;
> -
> /* check for mmio messages received before/during the CT enable */
> guc_get_mmio_msg(guc);
> guc_handle_mmio_msg(guc);
> @@ -235,9 +232,6 @@ static void guc_disable_communication(struct
> intel_guc *guc)
> guc_disable_interrupts(guc);
> - guc->send = intel_guc_send_nop;
> - guc->handler = intel_guc_to_host_event_handler_nop;
> -
> intel_guc_ct_disable(&guc->ct);
> /*
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH 4/5] drm/i915/guc: unify notify() functions
2019-12-10 21:09 ` [Intel-gfx] [PATCH 4/5] drm/i915/guc: unify notify() functions Daniele Ceraolo Spurio
@ 2019-12-11 14:26 ` Michal Wajdeczko
0 siblings, 0 replies; 16+ messages in thread
From: Michal Wajdeczko @ 2019-12-11 14:26 UTC (permalink / raw)
To: intel-gfx, Daniele Ceraolo Spurio
On Tue, 10 Dec 2019 22:09:18 +0100, Daniele Ceraolo Spurio
<daniele.ceraolospurio@intel.com> wrote:
> The Gen11+ and the legacy function differ in the register and value
> written to interrupt the GuC. However, while on older gen the value
> matches a bit on the register, on Gen11+ the value is a SW defined
> payload that is sent to the FW. Since the FW behaves the same no matter
> what value we pass to it, we can just write the same thing on all gens
> and get rid of the function pointer by saving the register offset.
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: John Harrison <John.C.Harrison@Intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> ---
Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH 2/5] drm/i915/guc/ct: stop expecting multiple CT channels
2019-12-11 13:43 ` Michal Wajdeczko
@ 2019-12-11 17:47 ` Daniele Ceraolo Spurio
0 siblings, 0 replies; 16+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-12-11 17:47 UTC (permalink / raw)
To: Michal Wajdeczko, intel-gfx
On 12/11/19 5:43 AM, Michal Wajdeczko wrote:
> On Tue, 10 Dec 2019 22:09:16 +0100, Daniele Ceraolo Spurio
> <daniele.ceraolospurio@intel.com> wrote:
>
>> The GuC supports having multiple CT buffer pairs and we designed our
>> implementation with that in mind. However, the different channels are not
>> processed in parallel within the GuC, so there is very little advantage
>> in having multiple channels (independent locks?), compared to the
>> drawbacks (one channel can starve the other if messages keep being
>> submitted to it). Given this, it is unlikely we'll ever add a second
>> channel and therefore we can simplify our code by removing the
>> flexibility.
>>
>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: John Harrison <John.C.Harrison@Intel.com>
>> Cc: Matthew Brost <matthew.brost@intel.com>
>> ---
>> drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 276 +++++++++-------------
>> drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h | 39 +--
>> 2 files changed, 118 insertions(+), 197 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
>> b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
>> index f74ba4750a94..96ce6d74f0b2 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
>> @@ -37,13 +37,10 @@ static void ct_incoming_request_worker_func(struct
>> work_struct *w);
>> */
>> void intel_guc_ct_init_early(struct intel_guc_ct *ct)
>> {
>> - /* we're using static channel owners */
>> - ct->host_channel.owner = CTB_OWNER_HOST;
>> -
>> - spin_lock_init(&ct->lock);
>> - INIT_LIST_HEAD(&ct->pending_requests);
>> - INIT_LIST_HEAD(&ct->incoming_requests);
>> - INIT_WORK(&ct->worker, ct_incoming_request_worker_func);
>> + spin_lock_init(&ct->requests.lock);
>> + INIT_LIST_HEAD(&ct->requests.pending);
>> + INIT_LIST_HEAD(&ct->requests.incoming);
>> + INIT_WORK(&ct->requests.worker, ct_incoming_request_worker_func);
>> }
>> static inline struct intel_guc *ct_to_guc(struct intel_guc_ct *ct)
>> @@ -64,14 +61,14 @@ static inline const char
>> *guc_ct_buffer_type_to_str(u32 type)
>> }
>> static void guc_ct_buffer_desc_init(struct guc_ct_buffer_desc *desc,
>> - u32 cmds_addr, u32 size, u32 owner)
>> + u32 cmds_addr, u32 size)
>> {
>> - CT_DEBUG_DRIVER("CT: desc %p init addr=%#x size=%u owner=%u\n",
>> - desc, cmds_addr, size, owner);
>> + CT_DEBUG_DRIVER("CT: desc %p init addr=%#x size=%u\n",
>> + desc, cmds_addr, size);
>
> please drop %p
>
You mean just leave the cmds_addr and size?
>> memset(desc, 0, sizeof(*desc));
>> desc->addr = cmds_addr;
>> desc->size = size;
>> - desc->owner = owner;
>> + desc->owner = CTB_OWNER_HOST;
>> }
>> static void guc_ct_buffer_desc_reset(struct guc_ct_buffer_desc *desc)
>> @@ -104,12 +101,11 @@ static int guc_action_register_ct_buffer(struct
>> intel_guc *guc,
>> }
>> static int guc_action_deregister_ct_buffer(struct intel_guc *guc,
>> - u32 owner,
>> u32 type)
>> {
>> u32 action[] = {
>> INTEL_GUC_ACTION_DEREGISTER_COMMAND_TRANSPORT_BUFFER,
>> - owner,
>> + CTB_OWNER_HOST,
>> type
>> };
>> int err;
>> @@ -117,19 +113,28 @@ static int
>> guc_action_deregister_ct_buffer(struct intel_guc *guc,
>> /* Can't use generic send(), CT deregistration must go over MMIO */
>> err = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action), NULL, 0);
>> if (err)
>> - DRM_ERROR("CT: deregister %s buffer failed; owner=%d err=%d\n",
>> - guc_ct_buffer_type_to_str(type), owner, err);
>> + DRM_ERROR("CT: deregister %s buffer failed; err=%d\n",
>> + guc_ct_buffer_type_to_str(type), err);
>> return err;
>> }
>> -static int ctch_init(struct intel_guc *guc,
>> - struct intel_guc_ct_channel *ctch)
>> +/**
>> + * intel_guc_ct_init - Init CT communication
>
> maybe to match other descriptions:
>
> "intel_guc_ct_init - Init buffer based communication"
>
>> + * @ct: pointer to CT struct
>> + *
>> + * Allocate memory required for communication via
>> + * the CT channel.
>
> CT channel ? maybe
>
> "Allocate memory required for buffer based communication"
>
ok for both
>
>> + *
>> + * Return: 0 on success, a negative errno code on failure.
>> + */
>> +int intel_guc_ct_init(struct intel_guc_ct *ct)
>> {
>> + struct intel_guc *guc = ct_to_guc(ct);
>> void *blob;
>> int err;
>> int i;
>> - GEM_BUG_ON(ctch->vma);
>> + GEM_BUG_ON(ct->vma);
>> /* We allocate 1 page to hold both descriptors and both buffers.
>> * ___________.....................
>> @@ -153,57 +158,67 @@ static int ctch_init(struct intel_guc *guc,
>> * other code will need updating as well.
>> */
>> - err = intel_guc_allocate_and_map_vma(guc, PAGE_SIZE, &ctch->vma,
>> &blob);
>> + err = intel_guc_allocate_and_map_vma(guc, PAGE_SIZE, &ct->vma,
>> &blob);
>> if (err) {
>> - CT_DEBUG_DRIVER("CT: channel %d initialization failed;
>> err=%d\n",
>> - ctch->owner, err);
>> + DRM_ERROR("CT: channel allocation failed; err=%d\n", err);
>> return err;
>> }
>> CT_DEBUG_DRIVER("CT: vma base=%#x\n",
>> - intel_guc_ggtt_offset(guc, ctch->vma));
>> + intel_guc_ggtt_offset(guc, ct->vma));
>> /* store pointers to desc and cmds */
>> - for (i = 0; i < ARRAY_SIZE(ctch->ctbs); i++) {
>> - GEM_BUG_ON((i != CTB_SEND) && (i != CTB_RECV));
>> - ctch->ctbs[i].desc = blob + PAGE_SIZE/4 * i;
>> - ctch->ctbs[i].cmds = blob + PAGE_SIZE/4 * i + PAGE_SIZE/2;
>> + for (i = 0; i < ARRAY_SIZE(ct->ctbs); i++) {
>> + GEM_BUG_ON((i != CTB_SEND) && (i != CTB_RECV));
>> + ct->ctbs[i].desc = blob + PAGE_SIZE/4 * i;
>> + ct->ctbs[i].cmds = blob + PAGE_SIZE/4 * i + PAGE_SIZE/2;
>> }
>> return 0;
>> }
>> -static void ctch_fini(struct intel_guc *guc,
>> - struct intel_guc_ct_channel *ctch)
>> +/**
>> + * intel_guc_ct_fini - Fini CT communication
>> + * @ct: pointer to CT struct
>> + *
>> + * Deallocate memory required for communication via
>> + * the CT channel.
>> + */
>> +void intel_guc_ct_fini(struct intel_guc_ct *ct)
>> {
>> - GEM_BUG_ON(ctch->enabled);
>> + GEM_BUG_ON(ct->enabled);
>> - i915_vma_unpin_and_release(&ctch->vma, I915_VMA_RELEASE_MAP);
>> + i915_vma_unpin_and_release(&ct->vma, I915_VMA_RELEASE_MAP);
>> }
>> -static int ctch_enable(struct intel_guc *guc,
>> - struct intel_guc_ct_channel *ctch)
>> +/**
>> + * intel_guc_ct_enable - Enable buffer based command transport.
>> + * @ct: pointer to CT struct
>> + *
>> + * Return: 0 on success, a negative errno code on failure.
>> + */
>> +int intel_guc_ct_enable(struct intel_guc_ct *ct)
>> {
>> + struct intel_guc *guc = ct_to_guc(ct);
>> u32 base;
>> int err;
>> int i;
>> - GEM_BUG_ON(!ctch->vma);
>> -
>> - GEM_BUG_ON(ctch->enabled);
>> + if (ct->enabled)
>> + return 0;
>
> btw, do we still need this check?
>
I don't think so, AFAICS we only call intel_guc_ct_enable from
guc_enable_communication(), which already has a BUG_ON if communication
is already enabled. I'll get rid of it, but I'll split that change in a
separate patch.
>> /* vma should be already allocated and map'ed */
>> - base = intel_guc_ggtt_offset(guc, ctch->vma);
>> + GEM_BUG_ON(!ct->vma);
>> + base = intel_guc_ggtt_offset(guc, ct->vma);
>> /* (re)initialize descriptors
>> * cmds buffers are in the second half of the blob page
>> */
>> - for (i = 0; i < ARRAY_SIZE(ctch->ctbs); i++) {
>> + for (i = 0; i < ARRAY_SIZE(ct->ctbs); i++) {
>> GEM_BUG_ON((i != CTB_SEND) && (i != CTB_RECV));
>> - guc_ct_buffer_desc_init(ctch->ctbs[i].desc,
>> + guc_ct_buffer_desc_init(ct->ctbs[i].desc,
>> base + PAGE_SIZE/4 * i + PAGE_SIZE/2,
>> - PAGE_SIZE/4,
>> - ctch->owner);
>> + PAGE_SIZE/4);
>> }
>> /* register buffers, starting wirh RECV buffer
>> @@ -221,40 +236,43 @@ static int ctch_enable(struct intel_guc *guc,
>> if (unlikely(err))
>> goto err_deregister;
>> - ctch->enabled = true;
>> + ct->enabled = true;
>> return 0;
>> err_deregister:
>> guc_action_deregister_ct_buffer(guc,
>> - ctch->owner,
>> INTEL_GUC_CT_BUFFER_TYPE_RECV);
>> err_out:
>> - DRM_ERROR("CT: can't open channel %d; err=%d\n", ctch->owner, err);
>> + DRM_ERROR("CT: can't open channel; err=%d\n", err);
>> return err;
>> }
>> -static void ctch_disable(struct intel_guc *guc,
>> - struct intel_guc_ct_channel *ctch)
>> +/**
>> + * intel_guc_ct_disable - Disable buffer based command transport.
>> + * @ct: pointer to CT struct
>> + */
>> +void intel_guc_ct_disable(struct intel_guc_ct *ct)
>> {
>> - GEM_BUG_ON(!ctch->enabled);
>> + struct intel_guc *guc = ct_to_guc(ct);
>> - ctch->enabled = false;
>> + if (!ct->enabled)
>> + return;
>> +
>> + ct->enabled = false;
>> if (intel_guc_is_running(guc)) {
>> guc_action_deregister_ct_buffer(guc,
>> - ctch->owner,
>> INTEL_GUC_CT_BUFFER_TYPE_SEND);
>> guc_action_deregister_ct_buffer(guc,
>> - ctch->owner,
>> INTEL_GUC_CT_BUFFER_TYPE_RECV);
>> }
>> }
>> -static u32 ctch_get_next_fence(struct intel_guc_ct_channel *ctch)
>> +static u32 ct_get_next_fence(struct intel_guc_ct *ct)
>> {
>> /* For now it's trivial */
>> - return ++ctch->next_fence;
>> + return ++ct->next_fence;
>> }
>> /**
>> @@ -427,35 +445,34 @@ static int wait_for_ct_request_update(struct
>> ct_request *req, u32 *status)
>> return err;
>> }
>> -static int ctch_send(struct intel_guc_ct *ct,
>> - struct intel_guc_ct_channel *ctch,
>> - const u32 *action,
>> - u32 len,
>> - u32 *response_buf,
>> - u32 response_buf_size,
>> - u32 *status)
>> +static int ct_send(struct intel_guc_ct *ct,
>> + const u32 *action,
>> + u32 len,
>> + u32 *response_buf,
>> + u32 response_buf_size,
>> + u32 *status)
>> {
>> - struct intel_guc_ct_buffer *ctb = &ctch->ctbs[CTB_SEND];
>> + struct intel_guc_ct_buffer *ctb = &ct->ctbs[CTB_SEND];
>> struct guc_ct_buffer_desc *desc = ctb->desc;
>> struct ct_request request;
>> unsigned long flags;
>> u32 fence;
>> int err;
>> - GEM_BUG_ON(!ctch->enabled);
>> + GEM_BUG_ON(!ct->enabled);
>> GEM_BUG_ON(!len);
>> GEM_BUG_ON(len & ~GUC_CT_MSG_LEN_MASK);
>> GEM_BUG_ON(!response_buf && response_buf_size);
>> - fence = ctch_get_next_fence(ctch);
>> + fence = ct_get_next_fence(ct);
>> request.fence = fence;
>> request.status = 0;
>> request.response_len = response_buf_size;
>> request.response_buf = response_buf;
>> - spin_lock_irqsave(&ct->lock, flags);
>> - list_add_tail(&request.link, &ct->pending_requests);
>> - spin_unlock_irqrestore(&ct->lock, flags);
>> + spin_lock_irqsave(&ct->requests.lock, flags);
>> + list_add_tail(&request.link, &ct->requests.pending);
>> + spin_unlock_irqrestore(&ct->requests.lock, flags);
>> err = ctb_write(ctb, action, len, fence, !!response_buf);
>> if (unlikely(err))
>> @@ -488,9 +505,9 @@ static int ctch_send(struct intel_guc_ct *ct,
>> }
>> unlink:
>> - spin_lock_irqsave(&ct->lock, flags);
>> + spin_lock_irqsave(&ct->requests.lock, flags);
>> list_del(&request.link);
>> - spin_unlock_irqrestore(&ct->lock, flags);
>> + spin_unlock_irqrestore(&ct->requests.lock, flags);
>> return err;
>> }
>> @@ -502,14 +519,12 @@ int intel_guc_send_ct(struct intel_guc *guc,
>> const u32 *action, u32 len,
>> u32 *response_buf, u32 response_buf_size)
>> {
>> struct intel_guc_ct *ct = &guc->ct;
>> - struct intel_guc_ct_channel *ctch = &ct->host_channel;
>> u32 status = ~0; /* undefined */
>> int ret;
>> mutex_lock(&guc->send_mutex);
>> - ret = ctch_send(ct, ctch, action, len, response_buf,
>> response_buf_size,
>> - &status);
>> + ret = ct_send(ct, action, len, response_buf, response_buf_size,
>> &status);
>> if (unlikely(ret < 0)) {
>> DRM_ERROR("CT: send action %#X failed; err=%d status=%#X\n",
>> action[0], ret, status);
>> @@ -640,8 +655,8 @@ static int ct_handle_response(struct intel_guc_ct
>> *ct, const u32 *msg)
>> CT_DEBUG_DRIVER("CT: response fence %u status %#x\n", fence, status);
>> - spin_lock(&ct->lock);
>> - list_for_each_entry(req, &ct->pending_requests, link) {
>> + spin_lock(&ct->requests.lock);
>> + list_for_each_entry(req, &ct->requests.pending, link) {
>> if (unlikely(fence != req->fence)) {
>> CT_DEBUG_DRIVER("CT: request %u awaits response\n",
>> req->fence);
>> @@ -659,7 +674,7 @@ static int ct_handle_response(struct intel_guc_ct
>> *ct, const u32 *msg)
>> found = true;
>> break;
>> }
>> - spin_unlock(&ct->lock);
>> + spin_unlock(&ct->requests.lock);
>> if (!found)
>> DRM_ERROR("CT: unsolicited response %*ph\n", 4 * msglen, msg);
>> @@ -697,13 +712,13 @@ static bool ct_process_incoming_requests(struct
>> intel_guc_ct *ct)
>> u32 *payload;
>> bool done;
>> - spin_lock_irqsave(&ct->lock, flags);
>> - request = list_first_entry_or_null(&ct->incoming_requests,
>> + spin_lock_irqsave(&ct->requests.lock, flags);
>> + request = list_first_entry_or_null(&ct->requests.incoming,
>> struct ct_incoming_request, link);
>> if (request)
>> list_del(&request->link);
>> - done = !!list_empty(&ct->incoming_requests);
>> - spin_unlock_irqrestore(&ct->lock, flags);
>> + done = !!list_empty(&ct->requests.incoming);
>> + spin_unlock_irqrestore(&ct->requests.lock, flags);
>> if (!request)
>> return true;
>> @@ -721,12 +736,13 @@ static bool ct_process_incoming_requests(struct
>> intel_guc_ct *ct)
>> static void ct_incoming_request_worker_func(struct work_struct *w)
>> {
>> - struct intel_guc_ct *ct = container_of(w, struct intel_guc_ct,
>> worker);
>> + struct intel_guc_ct *ct =
>> + container_of(w, struct intel_guc_ct, requests.worker);
>> bool done;
>> done = ct_process_incoming_requests(ct);
>> if (!done)
>> - queue_work(system_unbound_wq, &ct->worker);
>> + queue_work(system_unbound_wq, &ct->requests.worker);
>> }
>> /**
>> @@ -764,22 +780,26 @@ static int ct_handle_request(struct intel_guc_ct
>> *ct, const u32 *msg)
>> }
>> memcpy(request->msg, msg, 4 * msglen);
>> - spin_lock_irqsave(&ct->lock, flags);
>> - list_add_tail(&request->link, &ct->incoming_requests);
>> - spin_unlock_irqrestore(&ct->lock, flags);
>> + spin_lock_irqsave(&ct->requests.lock, flags);
>> + list_add_tail(&request->link, &ct->requests.incoming);
>> + spin_unlock_irqrestore(&ct->requests.lock, flags);
>> - queue_work(system_unbound_wq, &ct->worker);
>> + queue_work(system_unbound_wq, &ct->requests.worker);
>> return 0;
>> }
>> -static void ct_process_host_channel(struct intel_guc_ct *ct)
>> +/*
>> + * When we're communicating with the GuC over CT, GuC uses events
>> + * to notify us about new messages being posted on the RECV buffer.
>> + */
>> +void intel_guc_to_host_event_handler_ct(struct intel_guc *guc)
>> {
>> - struct intel_guc_ct_channel *ctch = &ct->host_channel;
>> - struct intel_guc_ct_buffer *ctb = &ctch->ctbs[CTB_RECV];
>> + struct intel_guc_ct *ct = &guc->ct;
>> + struct intel_guc_ct_buffer *ctb = &ct->ctbs[CTB_RECV];
>> u32 msg[GUC_CT_MSG_LEN_MASK + 1]; /* one extra dw for the header */
>> int err = 0;
>> - if (!ctch->enabled)
>> + if (!ct->enabled)
>> return;
>> do {
>> @@ -798,87 +818,3 @@ static void ct_process_host_channel(struct
>> intel_guc_ct *ct)
>> ctb->desc->is_in_error = 1;
>> }
>> }
>> -
>> -/*
>> - * When we're communicating with the GuC over CT, GuC uses events
>> - * to notify us about new messages being posted on the RECV buffer.
>> - */
>> -void intel_guc_to_host_event_handler_ct(struct intel_guc *guc)
>> -{
>> - struct intel_guc_ct *ct = &guc->ct;
>> -
>> - ct_process_host_channel(ct);
>> -}
>> -
>> -/**
>> - * intel_guc_ct_init - Init CT communication
>> - * @ct: pointer to CT struct
>> - *
>> - * Allocate memory required for communication via
>> - * the CT channel.
>> - *
>> - * Return: 0 on success, a negative errno code on failure.
>> - */
>> -int intel_guc_ct_init(struct intel_guc_ct *ct)
>> -{
>> - struct intel_guc *guc = ct_to_guc(ct);
>> - struct intel_guc_ct_channel *ctch = &ct->host_channel;
>> - int err;
>> -
>> - err = ctch_init(guc, ctch);
>> - if (unlikely(err)) {
>> - DRM_ERROR("CT: can't open channel %d; err=%d\n",
>> - ctch->owner, err);
>> - return err;
>> - }
>> -
>> - GEM_BUG_ON(!ctch->vma);
>> - return 0;
>> -}
>> -
>> -/**
>> - * intel_guc_ct_fini - Fini CT communication
>> - * @ct: pointer to CT struct
>> - *
>> - * Deallocate memory required for communication via
>> - * the CT channel.
>> - */
>> -void intel_guc_ct_fini(struct intel_guc_ct *ct)
>> -{
>> - struct intel_guc *guc = ct_to_guc(ct);
>> - struct intel_guc_ct_channel *ctch = &ct->host_channel;
>> -
>> - ctch_fini(guc, ctch);
>> -}
>> -
>> -/**
>> - * intel_guc_ct_enable - Enable buffer based command transport.
>> - * @ct: pointer to CT struct
>> - *
>> - * Return: 0 on success, a negative errno code on failure.
>> - */
>> -int intel_guc_ct_enable(struct intel_guc_ct *ct)
>> -{
>> - struct intel_guc *guc = ct_to_guc(ct);
>> - struct intel_guc_ct_channel *ctch = &ct->host_channel;
>> -
>> - if (ctch->enabled)
>> - return 0;
>> -
>> - return ctch_enable(guc, ctch);
>> -}
>> -
>> -/**
>> - * intel_guc_ct_disable - Disable buffer based command transport.
>> - * @ct: pointer to CT struct
>> - */
>> -void intel_guc_ct_disable(struct intel_guc_ct *ct)
>> -{
>> - struct intel_guc *guc = ct_to_guc(ct);
>> - struct intel_guc_ct_channel *ctch = &ct->host_channel;
>> -
>> - if (!ctch->enabled)
>> - return;
>> -
>> - ctch_disable(guc, ctch);
>> -}
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
>> b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
>> index 77c80d6cc25d..4bb1d1fcc860 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
>> @@ -35,44 +35,29 @@ struct intel_guc_ct_buffer {
>> u32 *cmds;
>> };
>> +
>> /** Represents pair of command transport buffers.
>
> intel_guc_ct is now little more that old ctch
>
>> *
>> * Buffers go in pairs to allow bi-directional communication.
>> * To simplify the code we place both of them in the same vma.
>> * Buffers from the same pair must share unique owner id.
>
> we'll have only one pair and one fixed owner,
> maybe worth to rephrase whole description ?
>
ok
>> - *
>> - * @vma: pointer to the vma with pair of CT buffers
>> - * @ctbs: buffers for sending(0) and receiving(1) commands
>> - * @owner: unique identifier
>> - * @next_fence: fence to be used with next send command
>> */
>> -struct intel_guc_ct_channel {
>> +struct intel_guc_ct {
>> struct i915_vma *vma;
>> - struct intel_guc_ct_buffer ctbs[2];
>> - u32 owner;
>> - u32 next_fence;
>> bool enabled;
>> -};
>> -/** Holds all command transport channels.
>> - *
>> - * @host_channel: main channel used by the host
>> - */
>> -struct intel_guc_ct {
>> - struct intel_guc_ct_channel host_channel;
>> - /* other channels are tbd */
>> -
>> - /** @lock: protects pending requests list */
>> - spinlock_t lock;
>> -
>> - /** @pending_requests: list of requests waiting for response */
>> - struct list_head pending_requests;
>> + /* buffers for sending(0) and receiving(1) commands */
>> + struct intel_guc_ct_buffer ctbs[2];
>> - /** @incoming_requests: list of incoming requests */
>> - struct list_head incoming_requests;
>> + /* fence to be used with next send command */
>> + u32 next_fence;
>
> fence is only used while sending requests,
> so maybe move it under below requests struct ?
>
>> - /** @worker: worker for handling incoming requests */
>> - struct work_struct worker;
>> + struct {
>> + spinlock_t lock; /* protects pending requests list */
>> + struct list_head pending; /* requests waiting for response */
>> + struct list_head incoming; /* incoming requests */
>> + struct work_struct worker; /* handler for incoming requests */
>> + } requests;
>
> maybe above change should be in separate patch to make diff smaller?
>
You mean split this in one patch to merge the CT and CTCH structures and
one to move some fields inside the requests sub-structure?
Daniele
>> };
>> void intel_guc_ct_init_early(struct intel_guc_ct *ct);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Intel-gfx] [PATCH 3/5] drm/i915/guc: remove function pointers for send/receive calls
2019-12-11 14:04 ` Michal Wajdeczko
@ 2019-12-11 18:01 ` Daniele Ceraolo Spurio
0 siblings, 0 replies; 16+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-12-11 18:01 UTC (permalink / raw)
To: Michal Wajdeczko, intel-gfx
On 12/11/19 6:04 AM, Michal Wajdeczko wrote:
> On Tue, 10 Dec 2019 22:09:17 +0100, Daniele Ceraolo Spurio
> <daniele.ceraolospurio@intel.com> wrote:
>
>> Since we started using CT buffers on all gens, the function pointers can
>> only be set to either the _nop() or the _ct() functions. Since the
>> _nop() case applies to when the CT are disabled, we can just handle that
>> case in the _ct() functions and call them directly.
>>
>> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: John Harrison <John.C.Harrison@Intel.com>
>> Cc: Matthew Brost <matthew.brost@intel.com>
>> ---
>> drivers/gpu/drm/i915/gt/intel_gt_irq.c | 2 +-
>> drivers/gpu/drm/i915/gt/uc/intel_guc.c | 22 +++-----------
>> drivers/gpu/drm/i915/gt/uc/intel_guc.h | 29 -------------------
>> drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 14 +++++++--
>> drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h | 17 +++++++++--
>> drivers/gpu/drm/i915/gt/uc/intel_guc_log.c | 6 ++--
>> .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 1 -
>> drivers/gpu/drm/i915/gt/uc/intel_uc.c | 18 ++++--------
>> 8 files changed, 40 insertions(+), 69 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_irq.c
>> b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
>> index 332b12a574fb..3183b4426c7b 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_gt_irq.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_gt_irq.c
>> @@ -16,7 +16,7 @@
>> static void guc_irq_handler(struct intel_guc *guc, u16 iir)
>> {
>> if (iir & GUC_INTR_GUC2HOST)
>> - intel_guc_to_host_event_handler(guc);
>> + intel_guc_to_host_event_handler_ct(guc);
>> }
>> static void
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
>> b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
>> index 922a19635d20..eb94635eeecd 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
>> @@ -177,8 +177,6 @@ void intel_guc_init_early(struct intel_guc *guc)
>> mutex_init(&guc->send_mutex);
>> spin_lock_init(&guc->irq_lock);
>> - guc->send = intel_guc_send_nop;
>> - guc->handler = intel_guc_to_host_event_handler_nop;
>> if (INTEL_GEN(i915) >= 11) {
>> guc->notify = gen11_guc_raise_irq;
>> guc->interrupts.reset = gen11_reset_guc_interrupts;
>> @@ -403,18 +401,6 @@ void intel_guc_fini(struct intel_guc *guc)
>> intel_uc_fw_cleanup_fetch(&guc->fw);
>> }
>> -int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32
>> len,
>> - u32 *response_buf, u32 response_buf_size)
>> -{
>> - WARN(1, "Unexpected send: action=%#x\n", *action);
>> - return -ENODEV;
>> -}
>> -
>> -void intel_guc_to_host_event_handler_nop(struct intel_guc *guc)
>> -{
>> - WARN(1, "Unexpected event: no suitable handler\n");
>> -}
>> -
>> /*
>> * This function implements the MMIO based host to GuC interface.
>> */
>> @@ -515,7 +501,7 @@ int intel_guc_sample_forcewake(struct intel_guc *guc)
>> /* bit 0 and 1 are for Render and Media domain separately */
>> action[1] = GUC_FORCEWAKE_RENDER | GUC_FORCEWAKE_MEDIA;
>> - return intel_guc_send(guc, action, ARRAY_SIZE(action));
>> + return intel_guc_send_ct(guc, action, ARRAY_SIZE(action));
>> }
>> /**
>> @@ -536,7 +522,7 @@ int intel_guc_auth_huc(struct intel_guc *guc, u32
>> rsa_offset)
>> rsa_offset
>> };
>> - return intel_guc_send(guc, action, ARRAY_SIZE(action));
>> + return intel_guc_send_ct(guc, action, ARRAY_SIZE(action));
>> }
>> /**
>> @@ -573,7 +559,7 @@ int intel_guc_suspend(struct intel_guc *guc)
>> intel_uncore_write(uncore, SOFT_SCRATCH(14),
>> INTEL_GUC_SLEEP_STATE_INVALID_MASK);
>> - ret = intel_guc_send(guc, action, ARRAY_SIZE(action));
>> + ret = intel_guc_send_ct(guc, action, ARRAY_SIZE(action));
>> if (ret)
>> return ret;
>> @@ -625,7 +611,7 @@ int intel_guc_resume(struct intel_guc *guc)
>> if (!intel_guc_submission_is_enabled(guc))
>> return 0;
>> - return intel_guc_send(guc, action, ARRAY_SIZE(action));
>> + return intel_guc_send_ct(guc, action, ARRAY_SIZE(action));
>> }
>> /**
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
>> b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
>> index cd09c912e361..c0b32db1c6ad 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
>> @@ -70,40 +70,15 @@ struct intel_guc {
>> /* To serialize the intel_guc_send actions */
>> struct mutex send_mutex;
>> - /* GuC's FW specific send function */
>> - int (*send)(struct intel_guc *guc, const u32 *data, u32 len,
>> - u32 *response_buf, u32 response_buf_size);
>> -
>> - /* GuC's FW specific event handler function */
>> - void (*handler)(struct intel_guc *guc);
>> -
>> /* GuC's FW specific notify function */
>> void (*notify)(struct intel_guc *guc);
>> };
>> -static
>> -inline int intel_guc_send(struct intel_guc *guc, const u32 *action,
>> u32 len)
>> -{
>> - return guc->send(guc, action, len, NULL, 0);
>> -}
>> -
>> -static inline int
>> -intel_guc_send_and_receive(struct intel_guc *guc, const u32 *action,
>> u32 len,
>> - u32 *response_buf, u32 response_buf_size)
>> -{
>> - return guc->send(guc, action, len, response_buf, response_buf_size);
>> -}
>
> instead of dropping above inlines, I would rather just change them to:
>
> return intel_guc_ct_send(&guc->ct, ...);
>
> a) we will not have to change existing callers
> b) we will maintain modularity (separation of ct code)
>
My POV here was that the caller needs to know if a message needs to go
via mmio or via CT so it isn't really abstracted away and
intel_guc_send() ends up being used as if it was intel_guc_send_ct().
Why not call the latter directly if that's the case?
Anyway, I don't have any strong feeling here, so if you thing only the
mmio case is the only one that deserves being called directly I don't
mind sticking with intel_guc_send().
>> -
>> static inline void intel_guc_notify(struct intel_guc *guc)
>> {
>> guc->notify(guc);
>> }
>> -static inline void intel_guc_to_host_event_handler(struct intel_guc
>> *guc)
>> -{
>> - guc->handler(guc);
>
> intel_guc_ct_event_handler(&guc->ct); ?
>
>> -}
>> -
>> /* GuC addresses above GUC_GGTT_TOP also don't map through the GTT */
>> #define GUC_GGTT_TOP 0xFEE00000
>> @@ -136,12 +111,8 @@ void intel_guc_init_send_regs(struct intel_guc
>> *guc);
>> void intel_guc_write_params(struct intel_guc *guc);
>> int intel_guc_init(struct intel_guc *guc);
>> void intel_guc_fini(struct intel_guc *guc);
>> -int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32
>> len,
>> - u32 *response_buf, u32 response_buf_size);
>> int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32
>> len,
>> u32 *response_buf, u32 response_buf_size);
>> -void intel_guc_to_host_event_handler(struct intel_guc *guc);
>> -void intel_guc_to_host_event_handler_nop(struct intel_guc *guc);
>> int intel_guc_to_host_process_recv_msg(struct intel_guc *guc,
>> const u32 *payload, u32 len);
>> int intel_guc_sample_forcewake(struct intel_guc *guc);
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
>> b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
>> index 96ce6d74f0b2..60b19f83e153 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
>> @@ -515,13 +515,19 @@ static int ct_send(struct intel_guc_ct *ct,
>> /*
>> * Command Transport (CT) buffer based GuC send function.
>> */
>> -int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len,
>> - u32 *response_buf, u32 response_buf_size)
>> +int intel_guc_send_and_receive_ct(struct intel_guc *guc, const u32
>> *action,
>
> to have proper modularization, this should be:
>
> intel_guc_ct_send_and_receive(struct intel_guc_ct *ct, ...
> or
> intel_guc_ct_send(struct intel_guc_ct *ct, ...
>
>> + u32 len, u32 *response_buf,
>> + u32 response_buf_size)
>> {
>> struct intel_guc_ct *ct = &guc->ct;
>> u32 status = ~0; /* undefined */
>> int ret;
>> + if (unlikely(!ct->enabled)) {
>> + WARN(1, "Unexpected send: action=%#x\n", *action);
>> + return -ENODEV;
>> + }
>> +
>> mutex_lock(&guc->send_mutex);
>> ret = ct_send(ct, action, len, response_buf, response_buf_size,
>> &status);
>> @@ -799,8 +805,10 @@ void intel_guc_to_host_event_handler_ct(struct
>> intel_guc *guc)
>> u32 msg[GUC_CT_MSG_LEN_MASK + 1]; /* one extra dw for the header */
>> int err = 0;
>> - if (!ct->enabled)
>> + if (!ct->enabled) {
>> + WARN(1, "Unexpected event: no suitable handler\n");
>
> hmm, there is a handler, but CTB is not working ;)
>
I've been lazy here and just moved the error msg as it was... :P
>> return;
>> + }
>> do {
>> err = ctb_read(ctb, msg);
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
>> b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
>> index 4bb1d1fcc860..929483b1f013 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
>> @@ -66,8 +66,21 @@ void intel_guc_ct_fini(struct intel_guc_ct *ct);
>> int intel_guc_ct_enable(struct intel_guc_ct *ct);
>> void intel_guc_ct_disable(struct intel_guc_ct *ct);
>> -int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len,
>> - u32 *response_buf, u32 response_buf_size);
>> +static inline bool intel_guc_ct_enabled(struct intel_guc_ct *ct)
>> +{
>> + return ct->enabled;
>> +}
>> +
>> +int
>> +intel_guc_send_and_receive_ct(struct intel_guc *guc, const u32
>> *action, u32 len,
>> + u32 *response_buf, u32 response_buf_size);
>> +
>> +static inline int
>> +intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len)
>> +{
>> + return intel_guc_send_and_receive_ct(guc, action, len, NULL, 0);
>> +}
>> +
>> void intel_guc_to_host_event_handler_ct(struct intel_guc *guc);
>> #endif /* _INTEL_GUC_CT_H_ */
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
>> b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
>> index caed0d57e704..5938127fb129 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
>> @@ -27,7 +27,7 @@ static int guc_action_flush_log_complete(struct
>> intel_guc *guc)
>> INTEL_GUC_ACTION_LOG_BUFFER_FILE_FLUSH_COMPLETE
>> };
>> - return intel_guc_send(guc, action, ARRAY_SIZE(action));
>> + return intel_guc_send_ct(guc, action, ARRAY_SIZE(action));
>> }
>> static int guc_action_flush_log(struct intel_guc *guc)
>> @@ -37,7 +37,7 @@ static int guc_action_flush_log(struct intel_guc *guc)
>> 0
>> };
>> - return intel_guc_send(guc, action, ARRAY_SIZE(action));
>> + return intel_guc_send_ct(guc, action, ARRAY_SIZE(action));
>> }
>> static int guc_action_control_log(struct intel_guc *guc, bool enable,
>> @@ -52,7 +52,7 @@ static int guc_action_control_log(struct intel_guc
>> *guc, bool enable,
>> GEM_BUG_ON(verbosity > GUC_LOG_VERBOSITY_MAX);
>> - return intel_guc_send(guc, action, ARRAY_SIZE(action));
>> + return intel_guc_send_ct(guc, action, ARRAY_SIZE(action));
>> }
>> static inline struct intel_guc *log_to_guc(struct intel_guc_log *log)
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
>> b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
>> index 172220e83079..fd7008bb128c 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
>> @@ -43,7 +43,6 @@
>> * Firmware writes a success/fail code back to the action register after
>> * processes the request. The kernel driver polls waiting for this
>> update and
>> * then proceeds.
>> - * See intel_guc_send()
>> *
>> * Work Items:
>> * There are several types of work items that the host may place into a
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
>> b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
>> index 7566af8ab46e..18a5eaf3052c 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
>> @@ -123,6 +123,11 @@ static void __uc_free_load_err_log(struct
>> intel_uc *uc)
>> i915_gem_object_put(log);
>> }
>> +static inline bool guc_communication_enabled(struct intel_guc *guc)
>> +{
>> + return intel_guc_ct_enabled(&guc->ct);
>> +}
>
> if this is really needed, please move to intel_guc.h
>
Why? it is only needed in this .c file, no need to have it in a header, no?
Daniele
>> +
>> /*
>> * Events triggered while CT buffers are disabled are logged in the
>> SCRATCH_15
>> * register using the same bits used in the CT message payload. Since
>> our
>> @@ -158,7 +163,7 @@ static void guc_handle_mmio_msg(struct intel_guc
>> *guc)
>> struct drm_i915_private *i915 = guc_to_gt(guc)->i915;
>> /* we need communication to be enabled to reply to GuC */
>> - GEM_BUG_ON(guc->handler == intel_guc_to_host_event_handler_nop);
>> + GEM_BUG_ON(!guc_communication_enabled(guc));
>> if (!guc->mmio_msg)
>> return;
>> @@ -185,11 +190,6 @@ static void guc_disable_interrupts(struct
>> intel_guc *guc)
>> guc->interrupts.disable(guc);
>> }
>> -static inline bool guc_communication_enabled(struct intel_guc *guc)
>> -{
>> - return guc->send != intel_guc_send_nop;
>> -}
>> -
>> static int guc_enable_communication(struct intel_guc *guc)
>> {
>> struct drm_i915_private *i915 = guc_to_gt(guc)->i915;
>> @@ -205,9 +205,6 @@ static int guc_enable_communication(struct
>> intel_guc *guc)
>> if (ret)
>> return ret;
>> - guc->send = intel_guc_send_ct;
>> - guc->handler = intel_guc_to_host_event_handler_ct;
>> -
>> /* check for mmio messages received before/during the CT enable */
>> guc_get_mmio_msg(guc);
>> guc_handle_mmio_msg(guc);
>> @@ -235,9 +232,6 @@ static void guc_disable_communication(struct
>> intel_guc *guc)
>> guc_disable_interrupts(guc);
>> - guc->send = intel_guc_send_nop;
>> - guc->handler = intel_guc_to_host_event_handler_nop;
>> -
>> intel_guc_ct_disable(&guc->ct);
>> /*
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2019-12-11 18:01 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-10 21:09 [Intel-gfx] [PATCH 0/5] Simplify GuC communication handling Daniele Ceraolo Spurio
2019-12-10 21:09 ` [Intel-gfx] [PATCH 1/5] drm/i915/guc: Merge communication_stop and communication_disable Daniele Ceraolo Spurio
2019-12-11 13:12 ` Michal Wajdeczko
2019-12-10 21:09 ` [Intel-gfx] [PATCH 2/5] drm/i915/guc/ct: stop expecting multiple CT channels Daniele Ceraolo Spurio
2019-12-11 13:43 ` Michal Wajdeczko
2019-12-11 17:47 ` Daniele Ceraolo Spurio
2019-12-10 21:09 ` [Intel-gfx] [PATCH 3/5] drm/i915/guc: remove function pointers for send/receive calls Daniele Ceraolo Spurio
2019-12-11 14:04 ` Michal Wajdeczko
2019-12-11 18:01 ` Daniele Ceraolo Spurio
2019-12-10 21:09 ` [Intel-gfx] [PATCH 4/5] drm/i915/guc: unify notify() functions Daniele Ceraolo Spurio
2019-12-11 14:26 ` Michal Wajdeczko
2019-12-10 21:09 ` [Intel-gfx] [PATCH 5/5] HAX: force enable_guc=2 and WA i915#571 Daniele Ceraolo Spurio
2019-12-11 5:25 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for Simplify GuC communication handling Patchwork
2019-12-11 5:54 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2019-12-11 11:33 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2019-12-11 11:52 ` Chris Wilson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.