On Mon, Mar 14, 2022 at 11:23:07AM +0530, Siva Mullati wrote:
On 09/03/22 07:08, Lucas De Marchi wrote:
On Tue, Mar 08, 2022 at 03:08:05PM +0530, Mullati Siva wrote:
From: Siva Mullati <siva.mull...@intel.com>
Convert CT commands and descriptors to use iosys_map rather
than plain pointer and save it in the intel_guc_ct_buffer struct.
This will help with ct_write and ct_read for cmd send and receive
after the initialization by abstracting the IO vs system memory.
Signed-off-by: Siva Mullati <siva.mull...@intel.com>
---
drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 170 +++++++++++++---------
drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h | 9 +-
2 files changed, 110 insertions(+), 69 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 f01325cd1b62..457deca1c25a 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
@@ -44,6 +44,11 @@ static inline struct drm_device *ct_to_drm(struct
intel_guc_ct *ct)
#define CT_PROBE_ERROR(_ct, _fmt, ...) \
i915_probe_error(ct_to_i915(ct), "CT: " _fmt, ##__VA_ARGS__)
+#define ct_desc_read(desc_map_, field_) \
+ iosys_map_rd_field(desc_map_, 0, struct guc_ct_buffer_desc, field_)
one thing I found useful in intel_guc_ads, was to use the first argument
as the struct type instead of map. That's because then I enforce the
right type to be passed rather than a random iosys_map. See :
#define ads_blob_read(guc_, field_) \
iosys_map_rd_field(&(guc_)->ads_map, 0, struct __guc_ads_blob, field_)
First arg is expected to be `struct intel_guc *`. Yes, I didn't do this
for info_map_{read,write}, because there were situation in which I had
to pass a info from outside (forcefully from system memory). If you
don't have such case, I think it would be better to pass the typed
pointer.
understood, will change it as a "typed pointer".
+#define ct_desc_write(desc_map_, field_, val_) \
+ iosys_map_wr_field(desc_map_, 0, struct guc_ct_buffer_desc, field_, val_)
+
/**
* DOC: CTB Blob
*
@@ -113,9 +118,9 @@ void intel_guc_ct_init_early(struct intel_guc_ct *ct)
init_waitqueue_head(&ct->wq);
}
-static void guc_ct_buffer_desc_init(struct guc_ct_buffer_desc *desc)
+static void guc_ct_buffer_desc_init(struct iosys_map *desc)
if you can apply the comment above, then leave it as
struct intel_guc_ct_buffer.
yes
{
- memset(desc, 0, sizeof(*desc));
+ iosys_map_memset(desc, 0, 0, sizeof(struct guc_ct_buffer_desc));
}
static void guc_ct_buffer_reset(struct intel_guc_ct_buffer *ctb)
@@ -128,17 +133,24 @@ static void guc_ct_buffer_reset(struct
intel_guc_ct_buffer *ctb)
space = CIRC_SPACE(ctb->tail, ctb->head, ctb->size) - ctb->resv_space;
atomic_set(&ctb->space, space);
- guc_ct_buffer_desc_init(ctb->desc);
+ guc_ct_buffer_desc_init(&ctb->desc_map);
}
static void guc_ct_buffer_init(struct intel_guc_ct_buffer *ctb,
- struct guc_ct_buffer_desc *desc,
- u32 *cmds, u32 size_in_bytes, u32 resv_space)
+ void *desc, void *cmds, u32 size_in_bytes,
+ u32 resv_space, bool lmem)
bool arguments are really hard to read, because you always have to
lookup the function prototype to understand what that is about.
Tried to avoid bool but could not find a better alternative code path.
Please suggest, if you have something.
humn... suggestion as as below:
Here we could turn struct guc_ct_buffer_desc *desc into the map, and let
the caller prepare it for iomem or system memory.
In other words, maybe instead of receiving guc_ct_buffer_desc as
parameter, receiving a struct iosys_map *desc_map. That map can be
created by the caller and then you do:
ctb->desc_map = *desc_map;
Lucas De Marchi
