Hi Peter,
On 2023-08-29 18:44, Peter Maydell wrote:
On Thu, 24 Aug 2023 at 19:35, Francisco Iglesias
<francisco.igles...@amd.com> wrote:
Introduce a model of Xilinx Versal's Configuration Frame controller
(CFRAME_REG).
Signed-off-by: Francisco Iglesias <francisco.igles...@amd.com>
+static void cfrm_fdri_post_write(RegisterInfo *reg, uint64_t val)
+{
+ XlnxVersalCFrameReg *s = XLNX_VERSAL_CFRAME_REG(reg->opaque);
+
+ if (s->row_configured && s->rowon && s->wcfg) {
+
+ if (fifo32_num_free(&s->new_f_data) >= N_WORDS_128BIT) {
+ fifo32_push(&s->new_f_data, s->regs[R_FDRI0]);
+ fifo32_push(&s->new_f_data, s->regs[R_FDRI1]);
+ fifo32_push(&s->new_f_data, s->regs[R_FDRI2]);
+ fifo32_push(&s->new_f_data, s->regs[R_FDRI3]);
+ }
+
+ if (fifo32_is_full(&s->new_f_data)) {
+ uint32_t addr = extract32(s->regs[R_FAR0], 0, 23);
+ XlnxCFrame *f = g_new(XlnxCFrame, 1);
+
+ memcpy(f->data, s->new_f_data.fifo.data, sizeof(f->data));
This works, but it's going under the hood of the Fifo32 abstraction
and makes the assumptions that (a) if you only push to the fifo
and never pop then the data is going to be contiguous from the
start of the fifo internal buffer and (b) that fifo32_push()
pushes the bytes of the 32 bit value in little endian order.
Those are both true at the moment, but fifo32 doesn't explicitly
guarantee either of them...
Undestood, to be safe I changed to pop the values from the fifo instead
of using memcpy.
+
+ g_tree_replace(s->cframes, GUINT_TO_POINTER(addr), f);
+
+ cframe_incr_far(s);
+
+ fifo32_reset(&s->new_f_data);
+ }
+ }
+}
+
+static void cfrm_readout_frames(XlnxVersalCFrameReg *s, uint32_t start_addr,
+ uint32_t end_addr)
+{
+ for (uint32_t addr = start_addr; addr < end_addr; addr++) {
+ XlnxCFrame *f = g_tree_lookup(s->cframes, GUINT_TO_POINTER(addr));
You don't need to g_tree_lookup() for every address. If
you use g_tree_lookup_node() you get a GTreeNode* back,
and you can then iterate through the tree from that point using
g_tree_node_next(), something like this:
for (node = g_tree_lookup_node(s->cframes, GUINT_TO_POINTER(addr));
node && GPOINTER_TO_UINT(g_tree_node_key(node)) < end_addr;
node = g_tree_node_next(node)) {
XlnxCFrame *f = g_tree_node_value(node); // definitely not NULL
/* Transmit the data */
for (int i = 0; i < FRAME_NUM_WORDS; i += 4) {
... etc ...
}
}
I tried above but it looks to require glib 2.68 so added a comment
instead (didn't compile on my machine with glib-2.64 :/, if I understand
meson.build correctly the min glib version supported is 2.56). Let me
know if some other solution would be preferred.
/*
* NB: when our minimum glib version is at least 2.68 we can improve the
* performance of the cframe traversal by using g_tree_lookup_node and
* g_tree_node_next (instead of calling g_tree_lookup for finding each
* cframe).
*/
I also added in the other improvements and correction below!
Thank you for the help again!
Best regards,
Francisco
+
+ /* Transmit the data if a frame was found */
+ if (f) {
+ for (int i = 0; i < FRAME_NUM_WORDS; i += 4) {
+ XlnxCfiPacket pkt = {};
+
+ pkt.data[0] = f->data[i];
+ pkt.data[1] = f->data[i + 1];
+ pkt.data[2] = f->data[i + 2];
+ pkt.data[3] = f->data[i + 3];
+
+ if (s->cfg.cfu_fdro) {
+ xlnx_cfi_transfer_packet(s->cfg.cfu_fdro, &pkt);
+ }
+ }
+ }
+ }
+}
+static void cframe_reg_reset_enter(Object *obj, ResetType type)
+{
+ XlnxVersalCFrameReg *s = XLNX_VERSAL_CFRAME_REG(obj);
+ unsigned int i;
+
j>> + for (i = 0; i < ARRAY_SIZE(s->regs_info); ++i) {
+ register_reset(&s->regs_info[i]);
+ }
+ memset(s->wfifo, 0, WFIFO_SZ * sizeof(uint32_t));
+ fifo32_reset(&s->new_f_data);
+
+ if (g_tree_height(s->cframes)) {
Calculating g_tree_height() requires walking the tree.
Using g_tree_nnodes() is faster because that's just a field
in the GTree struct.
+ g_tree_destroy(s->cframes);
+ s->cframes = g_tree_new_full((GCompareDataFunc)int_cmp, NULL,
+ NULL, (GDestroyNotify) g_free);
Faster to do
/*
* Take a reference so when g_tree_destroy() unrefs it we keep the
* GTree and only destroy its contents. NB: when our minimum
* glib version is at least 2.70 we could use g_tree_remove_all().
*/
g_tree_ref(s->cframes);
g_tree_destroy(s->cframes);
+ }
+}
+
+static void cframe_reg_init(Object *obj)
+{
+ XlnxVersalCFrameReg *s = XLNX_VERSAL_CFRAME_REG(obj);
+ SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+ RegisterInfoArray *reg_array;
+
+ memory_region_init(&s->iomem, obj, TYPE_XLNX_VERSAL_CFRAME_REG,
+ CFRAME_REG_R_MAX * 4);
+ reg_array =
+ register_init_block32(DEVICE(obj), cframe_reg_regs_info,
+ ARRAY_SIZE(cframe_reg_regs_info),
+ s->regs_info, s->regs,
+ &cframe_reg_ops,
+ XLNX_VERSAL_CFRAME_REG_ERR_DEBUG,
+ CFRAME_REG_R_MAX * 4);
+ memory_region_add_subregion(&s->iomem,
+ 0x0,
+ ®_array->mem);
+ sysbus_init_mmio(sbd, &s->iomem);
+ memory_region_init_io(&s->iomem_fdri, obj, &cframe_reg_fdri_ops, s,
+ TYPE_XLNX_VERSAL_CFRAME_REG "-fdri",
+ KEYHOLE_STREAM_4K);
+ sysbus_init_mmio(sbd, &s->iomem_fdri);
+ sysbus_init_irq(sbd, &s->irq_cfrm_imr);
+
+ s->cframes = g_tree_new_full((GCompareDataFunc)int_cmp, NULL,
+ NULL, (GDestroyNotify) g_free);
Stray space before 'g_free'.
+ fifo32_create(&s->new_f_data, FRAME_NUM_WORDS);
+}
thanks
-- PMM
