| Message ID | 20210610043649.144416-7-matthew.brost@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Update firmware to v62.0.0 | expand |
On 6/9/2021 9:36 PM, Matthew Brost wrote: > From: Michal Wajdeczko <michal.wajdeczko@intel.com> > > Definition of the CTB descriptor has changed, leaving only > minimal shared fields like HEAD/TAIL/STATUS. > > Both HEAD and TAIL are now in dwords. > > Add some ABI documentation and implement required changes. > > v2: > (Daniele) > - Drop GUC_CTB_STATUS_NO_BACKCHANNEL, GUC_CTB_STATUS_NO_BACKCHANNEL > > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com> > Signed-off-by: Matthew Brost <matthew.brost@intel.com> > --- > .../gt/uc/abi/guc_communication_ctb_abi.h | 68 +++++++++++++----- > drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 70 +++++++++---------- > drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h | 2 +- > 3 files changed, 83 insertions(+), 57 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h b/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h > index d38935f47ecf..88f1fc2a19e0 100644 > --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h > +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h > @@ -7,6 +7,56 @@ > #define _ABI_GUC_COMMUNICATION_CTB_ABI_H > > #include <linux/types.h> > +#include <linux/build_bug.h> > + > +#include "guc_messages_abi.h" > + > +/** > + * DOC: CT Buffer > + * > + * TBD This still needs to either be filled in or removed. > + */ > + > +/** > + * DOC: CTB Descriptor > + * > + * +---+-------+--------------------------------------------------------------+ > + * | | Bits | Description | > + * +===+=======+==============================================================+ > + * | 0 | 31:0 | **HEAD** - offset (in dwords) to the last dword that was | > + * | | | read from the `CT Buffer`_. | > + * | | | It can only be updated by the receiver. | > + * +---+-------+--------------------------------------------------------------+ > + * | 1 | 31:0 | **TAIL** - offset (in dwords) to the last dword that was | > + * | | | written to the `CT Buffer`_. | > + * | | | It can only be updated by the sender. | > + * +---+-------+--------------------------------------------------------------+ > + * | 2 | 31:0 | **STATUS** - status of the CTB | > + * | | | | > + * | | | - _`GUC_CTB_STATUS_NO_ERROR` = 0 (normal operation) | > + * | | | - _`GUC_CTB_STATUS_OVERFLOW` = 1 (head/tail too large) | > + * | | | - _`GUC_CTB_STATUS_UNDERFLOW` = 2 (truncated message) | > + * | | | - _`GUC_CTB_STATUS_MISMATCH` = 4 (head/tail modified) | > + * +---+-------+--------------------------------------------------------------+ > + * |...| | RESERVED = MBZ | > + * +---+-------+--------------------------------------------------------------+ > + * | 15| 31:0 | RESERVED = MBZ | > + * +---+-------+--------------------------------------------------------------+ > + */ > + > +struct guc_ct_buffer_desc { > + u32 head; > + u32 tail; > + u32 status; > +#define GUC_CTB_STATUS_NO_ERROR 0 > +#define GUC_CTB_STATUS_OVERFLOW (1 << 0) > +#define GUC_CTB_STATUS_UNDERFLOW (1 << 1) > +#define GUC_CTB_STATUS_MISMATCH (1 << 2) > +#define GUC_CTB_STATUS_NO_BACKCHANNEL (1 << 3) > +#define GUC_CTB_STATUS_MALFORMED_MSG (1 << 4) You forgot to drop the defines here. Daniele > + u32 reserved[13]; > +} __packed; > +static_assert(sizeof(struct guc_ct_buffer_desc) == 64); > > /** > * DOC: CTB based communication > @@ -60,24 +110,6 @@ > * - **flags**, holds various bits to control message handling > */ > > -/* > - * Describes single command transport buffer. > - * Used by both guc-master and clients. > - */ > -struct guc_ct_buffer_desc { > - u32 addr; /* gfx address */ > - u64 host_private; /* host private data */ > - u32 size; /* size in bytes */ > - u32 head; /* offset updated by GuC*/ > - u32 tail; /* offset updated by owner */ > - u32 is_in_error; /* error indicator */ > - u32 reserved1; > - u32 reserved2; > - u32 owner; /* id of the channel owner */ > - u32 owner_sub_id; /* owner-defined field for extra tracking */ > - u32 reserved[5]; > -} __packed; > - > /* Type of command transport buffer */ > #define INTEL_GUC_CT_BUFFER_TYPE_SEND 0x0u > #define INTEL_GUC_CT_BUFFER_TYPE_RECV 0x1u > 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 63056ea0631e..3241a477196f 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c > @@ -112,32 +112,28 @@ 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) > +static void guc_ct_buffer_desc_init(struct guc_ct_buffer_desc *desc) > { > memset(desc, 0, sizeof(*desc)); > - desc->addr = cmds_addr; > - desc->size = size; > - desc->owner = CTB_OWNER_HOST; > } > > -static void guc_ct_buffer_reset(struct intel_guc_ct_buffer *ctb, u32 cmds_addr) > +static void guc_ct_buffer_reset(struct intel_guc_ct_buffer *ctb) > { > ctb->broken = false; > - guc_ct_buffer_desc_init(ctb->desc, cmds_addr, ctb->size); > + guc_ct_buffer_desc_init(ctb->desc); > } > > static void guc_ct_buffer_init(struct intel_guc_ct_buffer *ctb, > struct guc_ct_buffer_desc *desc, > - u32 *cmds, u32 size) > + u32 *cmds, u32 size_in_bytes) > { > - GEM_BUG_ON(size % 4); > + GEM_BUG_ON(size_in_bytes % 4); > > ctb->desc = desc; > ctb->cmds = cmds; > - ctb->size = size; > + ctb->size = size_in_bytes / 4; > > - guc_ct_buffer_reset(ctb, 0); > + guc_ct_buffer_reset(ctb); > } > > static int guc_action_register_ct_buffer(struct intel_guc *guc, > @@ -279,10 +275,10 @@ int intel_guc_ct_enable(struct intel_guc_ct *ct) > > /* (re)initialize descriptors */ > cmds = base + ptrdiff(ct->ctbs.send.cmds, blob); > - guc_ct_buffer_reset(&ct->ctbs.send, cmds); > + guc_ct_buffer_reset(&ct->ctbs.send); > > cmds = base + ptrdiff(ct->ctbs.recv.cmds, blob); > - guc_ct_buffer_reset(&ct->ctbs.recv, cmds); > + guc_ct_buffer_reset(&ct->ctbs.recv); > > /* > * Register both CT buffers starting with RECV buffer. > @@ -391,17 +387,15 @@ static int ct_write(struct intel_guc_ct *ct, > if (unlikely(ctb->broken)) > return -EPIPE; > > - if (unlikely(desc->is_in_error)) > + if (unlikely(desc->status)) > goto corrupted; > > - if (unlikely(!IS_ALIGNED(head | tail, 4) || > - (tail | head) >= size)) > + if (unlikely((tail | head) >= size)) { > + CT_ERROR(ct, "Invalid offsets head=%u tail=%u (size=%u)\n", > + head, tail, size); > + desc->status |= GUC_CTB_STATUS_OVERFLOW; > goto corrupted; > - > - /* later calculations will be done in dwords */ > - head /= 4; > - tail /= 4; > - size /= 4; > + } > > /* > * tail == head condition indicates empty. GuC FW does not support > @@ -447,14 +441,14 @@ static int ct_write(struct intel_guc_ct *ct, > */ > write_barrier(ct); > > - /* now update desc tail (back in bytes) */ > - desc->tail = tail * 4; > + /* now update descriptor */ > + WRITE_ONCE(desc->tail, tail); > + > return 0; > > corrupted: > - CT_ERROR(ct, "Corrupted descriptor addr=%#x head=%u tail=%u size=%u\n", > - desc->addr, desc->head, desc->tail, desc->size); > - desc->is_in_error = 1; > + CT_ERROR(ct, "Corrupted descriptor head=%u tail=%u status=%#x\n", > + desc->head, desc->tail, desc->status); > ctb->broken = true; > return -EPIPE; > } > @@ -640,17 +634,15 @@ static int ct_read(struct intel_guc_ct *ct, struct ct_incoming_msg **msg) > if (unlikely(ctb->broken)) > return -EPIPE; > > - if (unlikely(desc->is_in_error)) > + if (unlikely(desc->status)) > goto corrupted; > > - if (unlikely(!IS_ALIGNED(head | tail, 4) || > - (tail | head) >= size)) > + if (unlikely((tail | head) >= size)) { > + CT_ERROR(ct, "Invalid offsets head=%u tail=%u (size=%u)\n", > + head, tail, size); > + desc->status |= GUC_CTB_STATUS_OVERFLOW; > goto corrupted; > - > - /* later calculations will be done in dwords */ > - head /= 4; > - tail /= 4; > - size /= 4; > + } > > /* tail == head condition indicates empty */ > available = tail - head; > @@ -677,6 +669,7 @@ static int ct_read(struct intel_guc_ct *ct, struct ct_incoming_msg **msg) > size - head : available - 1), &cmds[head], > 4 * (head + available - 1 > size ? > available - 1 - size + head : 0), &cmds[0]); > + desc->status |= GUC_CTB_STATUS_UNDERFLOW; > goto corrupted; > } > > @@ -699,13 +692,14 @@ static int ct_read(struct intel_guc_ct *ct, struct ct_incoming_msg **msg) > } > CT_DEBUG(ct, "received %*ph\n", 4 * len, (*msg)->msg); > > - desc->head = head * 4; > + /* now update descriptor */ > + WRITE_ONCE(desc->head, head); > + > return available - len; > > corrupted: > - CT_ERROR(ct, "Corrupted descriptor addr=%#x head=%u tail=%u size=%u\n", > - desc->addr, desc->head, desc->tail, desc->size); > - desc->is_in_error = 1; > + CT_ERROR(ct, "Corrupted descriptor head=%u tail=%u status=%#x\n", > + desc->head, desc->tail, desc->status); > ctb->broken = true; > return -EPIPE; > } > 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 7d3cd375d6a7..905202caaad3 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h > @@ -31,7 +31,7 @@ struct intel_guc; > * @lock: protects access to the commands buffer and buffer descriptor > * @desc: pointer to the buffer descriptor > * @cmds: pointer to the commands buffer > - * @size: size of the commands buffer > + * @size: size of the commands buffer in dwords > * @broken: flag to indicate if descriptor data is broken > */ > struct intel_guc_ct_buffer {
diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h b/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h index d38935f47ecf..88f1fc2a19e0 100644 --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_communication_ctb_abi.h @@ -7,6 +7,56 @@ #define _ABI_GUC_COMMUNICATION_CTB_ABI_H #include <linux/types.h> +#include <linux/build_bug.h> + +#include "guc_messages_abi.h" + +/** + * DOC: CT Buffer + * + * TBD + */ + +/** + * DOC: CTB Descriptor + * + * +---+-------+--------------------------------------------------------------+ + * | | Bits | Description | + * +===+=======+==============================================================+ + * | 0 | 31:0 | **HEAD** - offset (in dwords) to the last dword that was | + * | | | read from the `CT Buffer`_. | + * | | | It can only be updated by the receiver. | + * +---+-------+--------------------------------------------------------------+ + * | 1 | 31:0 | **TAIL** - offset (in dwords) to the last dword that was | + * | | | written to the `CT Buffer`_. | + * | | | It can only be updated by the sender. | + * +---+-------+--------------------------------------------------------------+ + * | 2 | 31:0 | **STATUS** - status of the CTB | + * | | | | + * | | | - _`GUC_CTB_STATUS_NO_ERROR` = 0 (normal operation) | + * | | | - _`GUC_CTB_STATUS_OVERFLOW` = 1 (head/tail too large) | + * | | | - _`GUC_CTB_STATUS_UNDERFLOW` = 2 (truncated message) | + * | | | - _`GUC_CTB_STATUS_MISMATCH` = 4 (head/tail modified) | + * +---+-------+--------------------------------------------------------------+ + * |...| | RESERVED = MBZ | + * +---+-------+--------------------------------------------------------------+ + * | 15| 31:0 | RESERVED = MBZ | + * +---+-------+--------------------------------------------------------------+ + */ + +struct guc_ct_buffer_desc { + u32 head; + u32 tail; + u32 status; +#define GUC_CTB_STATUS_NO_ERROR 0 +#define GUC_CTB_STATUS_OVERFLOW (1 << 0) +#define GUC_CTB_STATUS_UNDERFLOW (1 << 1) +#define GUC_CTB_STATUS_MISMATCH (1 << 2) +#define GUC_CTB_STATUS_NO_BACKCHANNEL (1 << 3) +#define GUC_CTB_STATUS_MALFORMED_MSG (1 << 4) + u32 reserved[13]; +} __packed; +static_assert(sizeof(struct guc_ct_buffer_desc) == 64); /** * DOC: CTB based communication @@ -60,24 +110,6 @@ * - **flags**, holds various bits to control message handling */ -/* - * Describes single command transport buffer. - * Used by both guc-master and clients. - */ -struct guc_ct_buffer_desc { - u32 addr; /* gfx address */ - u64 host_private; /* host private data */ - u32 size; /* size in bytes */ - u32 head; /* offset updated by GuC*/ - u32 tail; /* offset updated by owner */ - u32 is_in_error; /* error indicator */ - u32 reserved1; - u32 reserved2; - u32 owner; /* id of the channel owner */ - u32 owner_sub_id; /* owner-defined field for extra tracking */ - u32 reserved[5]; -} __packed; - /* Type of command transport buffer */ #define INTEL_GUC_CT_BUFFER_TYPE_SEND 0x0u #define INTEL_GUC_CT_BUFFER_TYPE_RECV 0x1u 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 63056ea0631e..3241a477196f 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c @@ -112,32 +112,28 @@ 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) +static void guc_ct_buffer_desc_init(struct guc_ct_buffer_desc *desc) { memset(desc, 0, sizeof(*desc)); - desc->addr = cmds_addr; - desc->size = size; - desc->owner = CTB_OWNER_HOST; } -static void guc_ct_buffer_reset(struct intel_guc_ct_buffer *ctb, u32 cmds_addr) +static void guc_ct_buffer_reset(struct intel_guc_ct_buffer *ctb) { ctb->broken = false; - guc_ct_buffer_desc_init(ctb->desc, cmds_addr, ctb->size); + guc_ct_buffer_desc_init(ctb->desc); } static void guc_ct_buffer_init(struct intel_guc_ct_buffer *ctb, struct guc_ct_buffer_desc *desc, - u32 *cmds, u32 size) + u32 *cmds, u32 size_in_bytes) { - GEM_BUG_ON(size % 4); + GEM_BUG_ON(size_in_bytes % 4); ctb->desc = desc; ctb->cmds = cmds; - ctb->size = size; + ctb->size = size_in_bytes / 4; - guc_ct_buffer_reset(ctb, 0); + guc_ct_buffer_reset(ctb); } static int guc_action_register_ct_buffer(struct intel_guc *guc, @@ -279,10 +275,10 @@ int intel_guc_ct_enable(struct intel_guc_ct *ct) /* (re)initialize descriptors */ cmds = base + ptrdiff(ct->ctbs.send.cmds, blob); - guc_ct_buffer_reset(&ct->ctbs.send, cmds); + guc_ct_buffer_reset(&ct->ctbs.send); cmds = base + ptrdiff(ct->ctbs.recv.cmds, blob); - guc_ct_buffer_reset(&ct->ctbs.recv, cmds); + guc_ct_buffer_reset(&ct->ctbs.recv); /* * Register both CT buffers starting with RECV buffer. @@ -391,17 +387,15 @@ static int ct_write(struct intel_guc_ct *ct, if (unlikely(ctb->broken)) return -EPIPE; - if (unlikely(desc->is_in_error)) + if (unlikely(desc->status)) goto corrupted; - if (unlikely(!IS_ALIGNED(head | tail, 4) || - (tail | head) >= size)) + if (unlikely((tail | head) >= size)) { + CT_ERROR(ct, "Invalid offsets head=%u tail=%u (size=%u)\n", + head, tail, size); + desc->status |= GUC_CTB_STATUS_OVERFLOW; goto corrupted; - - /* later calculations will be done in dwords */ - head /= 4; - tail /= 4; - size /= 4; + } /* * tail == head condition indicates empty. GuC FW does not support @@ -447,14 +441,14 @@ static int ct_write(struct intel_guc_ct *ct, */ write_barrier(ct); - /* now update desc tail (back in bytes) */ - desc->tail = tail * 4; + /* now update descriptor */ + WRITE_ONCE(desc->tail, tail); + return 0; corrupted: - CT_ERROR(ct, "Corrupted descriptor addr=%#x head=%u tail=%u size=%u\n", - desc->addr, desc->head, desc->tail, desc->size); - desc->is_in_error = 1; + CT_ERROR(ct, "Corrupted descriptor head=%u tail=%u status=%#x\n", + desc->head, desc->tail, desc->status); ctb->broken = true; return -EPIPE; } @@ -640,17 +634,15 @@ static int ct_read(struct intel_guc_ct *ct, struct ct_incoming_msg **msg) if (unlikely(ctb->broken)) return -EPIPE; - if (unlikely(desc->is_in_error)) + if (unlikely(desc->status)) goto corrupted; - if (unlikely(!IS_ALIGNED(head | tail, 4) || - (tail | head) >= size)) + if (unlikely((tail | head) >= size)) { + CT_ERROR(ct, "Invalid offsets head=%u tail=%u (size=%u)\n", + head, tail, size); + desc->status |= GUC_CTB_STATUS_OVERFLOW; goto corrupted; - - /* later calculations will be done in dwords */ - head /= 4; - tail /= 4; - size /= 4; + } /* tail == head condition indicates empty */ available = tail - head; @@ -677,6 +669,7 @@ static int ct_read(struct intel_guc_ct *ct, struct ct_incoming_msg **msg) size - head : available - 1), &cmds[head], 4 * (head + available - 1 > size ? available - 1 - size + head : 0), &cmds[0]); + desc->status |= GUC_CTB_STATUS_UNDERFLOW; goto corrupted; } @@ -699,13 +692,14 @@ static int ct_read(struct intel_guc_ct *ct, struct ct_incoming_msg **msg) } CT_DEBUG(ct, "received %*ph\n", 4 * len, (*msg)->msg); - desc->head = head * 4; + /* now update descriptor */ + WRITE_ONCE(desc->head, head); + return available - len; corrupted: - CT_ERROR(ct, "Corrupted descriptor addr=%#x head=%u tail=%u size=%u\n", - desc->addr, desc->head, desc->tail, desc->size); - desc->is_in_error = 1; + CT_ERROR(ct, "Corrupted descriptor head=%u tail=%u status=%#x\n", + desc->head, desc->tail, desc->status); ctb->broken = true; return -EPIPE; } 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 7d3cd375d6a7..905202caaad3 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h @@ -31,7 +31,7 @@ struct intel_guc; * @lock: protects access to the commands buffer and buffer descriptor * @desc: pointer to the buffer descriptor * @cmds: pointer to the commands buffer - * @size: size of the commands buffer + * @size: size of the commands buffer in dwords * @broken: flag to indicate if descriptor data is broken */ struct intel_guc_ct_buffer {