On 8/4/2017 9:26 AM, Michal Wajdeczko wrote:
GuC may return additional data in the command status response.
Format and meaning of this data is action specific.
We will use this non-negative data as a new success return value.
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 | 14 +++++++-------
drivers/gpu/drm/i915/intel_guc_fwif.h | 6 ++++++
drivers/gpu/drm/i915/intel_uc.c | 5 ++++-
3 files changed, 17 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
index c4cbec1..1249868 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -387,9 +387,9 @@ static int ctch_send(struct intel_guc *guc,
err = wait_for_response(desc, fence, status);
if (unlikely(err))
return err;
- if (*status != INTEL_GUC_STATUS_SUCCESS)
+ if (INTEL_GUC_RECV_TO_STATUS(*status) != INTEL_GUC_STATUS_SUCCESS)
return -EIO;
- return 0;
+ return INTEL_GUC_RECV_TO_DATA(*status);
}
/*
@@ -399,18 +399,18 @@ static int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len)
{
struct intel_guc_ct_channel *ctch = &guc->ct.host_channel;
u32 status = ~0; /* undefined */
- int err;
+ int ret;
mutex_lock(&guc->send_mutex);
- err = ctch_send(guc, ctch, action, len, &status);
- if (unlikely(err)) {
+ ret = ctch_send(guc, ctch, action, len, &status);
+ if (unlikely(ret < 0)) {
DRM_ERROR("CT: send action %#X failed; err=%d status=%#X\n",
- action[0], err, status);
+ action[0], ret, status);
}
mutex_unlock(&guc->send_mutex);
- return err;
+ return ret;
}
/**
diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 5fa2860..98c0560 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -567,10 +567,16 @@ enum intel_guc_action {
* command in SS0. The response is distinguishable from a command
* by the fact that all the MASK bits are set. The remaining bits
* give more detail.
+ * Bits [16:27] are reserved for optional data reporting.
*/
#define INTEL_GUC_RECV_MASK ((u32)0xF0000000)
#define INTEL_GUC_RECV_IS_RESPONSE(x) ((u32)(x) >= INTEL_GUC_RECV_MASK)
#define INTEL_GUC_RECV_STATUS(x) (INTEL_GUC_RECV_MASK | (x))
+#define INTEL_GUC_RECV_DATA_SHIFT 16
+#define INTEL_GUC_RECV_DATA_MASK (0xFFF << INTEL_GUC_RECV_DATA_SHIFT)
+#define INTEL_GUC_RECV_TO_STATUS(x) ((x) & ~ INTEL_GUC_RECV_DATA_MASK)
checkpatch should have complained about the blank space after '~'.
+#define INTEL_GUC_RECV_TO_DATA(x) (((x) & INTEL_GUC_RECV_DATA_MASK) >> \
+ INTEL_GUC_RECV_DATA_SHIFT)
/* GUC will return status back to SOFT_SCRATCH_O_REG */
enum intel_guc_status {
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 27e072c..ff25477 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -502,7 +502,7 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
INTEL_GUC_RECV_MASK,
INTEL_GUC_RECV_MASK,
10, 10, &status);
- if (status != INTEL_GUC_STATUS_SUCCESS) {
+ if (INTEL_GUC_RECV_TO_STATUS(status) != INTEL_GUC_STATUS_SUCCESS) {
/*
* Either the GuC explicitly returned an error (which
* we convert to -EIO here) or no response at all was
@@ -514,6 +514,9 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
DRM_WARN("INTEL_GUC_SEND: Action 0x%X failed;"
" ret=%d status=0x%08X response=0x%08X\n",
action[0], ret, status, I915_READ(SOFT_SCRATCH(15)));
+ } else {
+ /* Use data encoded in status dword as return value */
+ ret = INTEL_GUC_RECV_TO_DATA(status);
}
intel_uncore_forcewake_put(dev_priv, guc->send_regs.fw_domains);
Other than the blank space after that '~', it looks good to me.
Just a note, for other people reading this; there are 3 cases in which
intel_guc_send return value is only checked for truthiness (i.e.
__guc_allocate_doorbell, __guc_deallocate_doorbell and
intel_guc_sample_forcewake callers are checking for 'if (err)').
I know none of the existing H2G commands will return any extra data, so
intel_guc_send should be returning only negative numbers or zero (for now).
-Michel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
