On Mon, 26 Mar 2018 17:29:32 +0200, Michał Winiarski
<michal.winiarski@xxxxxxxxx> wrote:
On Fri, Mar 23, 2018 at 02:47:23PM +0000, Michal Wajdeczko wrote:
Instead of returning small data in response status dword,
GuC may append longer data as response message payload.
If caller provides response buffer, we will copy received
data and use number of received data dwords as new success
return value. We will WARN if response from GuC does not
match caller expectation.
v2: fix timeout and checkpatch warnings (Michal)
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
Cc: Oscar Mateo <oscar.mateo@xxxxxxxxx>
Cc: Michel Thierry <michel.thierry@xxxxxxxxx>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
---
drivers/gpu/drm/i915/intel_guc_ct.c | 137
++++++++++++++++++++++++++++++++----
drivers/gpu/drm/i915/intel_guc_ct.h | 5 ++
2 files changed, 128 insertions(+), 14 deletions(-)
[SNIP]
@@ -418,13 +499,15 @@ static int ctch_send(struct intel_guc *guc,
static 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_channel *ctch = &guc->ct.host_channel;
+ 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(guc, ctch, action, len, &status);
+ ret = ctch_send(ct, ctch, 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);
@@ -503,8 +586,13 @@ static int ctb_read(struct intel_guc_ct_buffer
*ctb, u32 *data)
static int ct_handle_response(struct intel_guc_ct *ct, const u32 *msg)
{
u32 header = msg[0];
+ u32 fence = msg[1];
u32 status = msg[2];
u32 len = ct_header_get_len(header) + 1; /* total len with header */
+ u32 payload_len = len - 3; /* len<3 is treated as protocol error */
Magic numbers, please ether define 3 as min payload length or hide this
behind
macro.
Well, it looks that detailed CTB documentation can't wait any more...
(simplified doc was already there:
/* Response message shall at least include header, fence and status */
I'll try to include more docs in next series spin to make these numbers
more
clear for everyone
+ struct ct_request *req;
+ bool found = false;
+ unsigned long flags;
GEM_BUG_ON(!ct_header_is_response(header));
@@ -518,8 +606,29 @@ static int ct_handle_response(struct intel_guc_ct
*ct, const u32 *msg)
DRM_ERROR("CT: corrupted response %*phn\n", 4*len, msg);
return -EPROTO;
}
+ spin_lock_irqsave(&ct->lock, flags);
Isn't this called from the irq? We can use plain spin_lock here.
yes, will update, thanks
+ list_for_each_entry(req, &ct->pending_requests, link) {
+ if (req->fence != fence) {
+ DRM_DEBUG_DRIVER("CT: request %u awaits response\n",
+ req->fence);
+ continue;
Is this expected?
rather not, tempting to mark that with 'unlikely'
In other words - do we expect out of order responses?
not today, since we're using send_mutex to serialize all requests
(serialization was required for MMIO based comm, but not for CT)
Can we extract this into a helper (find request)?
will try, but can't promise
thanks,
/m
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
