Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support IOB tiles with only one site (IOB_SING) #488

Closed
wants to merge 5 commits into from

Conversation

mcmasterg
Copy link
Contributor

Fixes #341

These were held off because they generate some weird special cases I didn't want to think about at the time. So here's a proposed solution.

First, the problem: in a7, IOBs are typically in LIOB33 tiles. These have two IOB sites, just like a SLICE has several CLBs. However, unlike CLBs, these IOBs are sometimes split into LIOB33_SING tiles, which only have a single IOB33. The problem is that in some cases the single instance is the Y=1 case, which basically starts at word 2 in our LIOB33_SING tile (if you follow the pattern in LIOB33 or want a Y=0 and a Y=1 LIOB33_SING to have a consistent address space). The issue is that this actually occurs at FDRI word 0, meaning we essentially have a negative tile address (-2) if we want the reference bits, starting at address 2, to be in frame 0.

Sample tilegrid output from running this fuzzer:

//Special bottom entry
//its missing the Y=0 site
"LIOB33_SING_X0Y100": {
    "bits": {
        "CLB_IO_CLK": {
            "baseaddr": "0x00020000",
            "frames": 42,
            "height": 4,
            "offset": -2,
            "words": 4
        }
    },
//A typical entry
"LIOB33_X0Y101": {
    "bits": {
        "CLB_IO_CLK": {
            "baseaddr": "0x00020000",
            "frames": 42,
            "height": 4,
            "offset": 2,
            "words": 4
        }
    },
...
//Special bottom entry
//its missing the Y=1 site
"LIOB33_SING_X0Y149": {
    "bits": {
        "CLB_IO_CLK": {
            "baseaddr": "0x00020000",
            "frames": 42,
            "height": 2,
            "offset": 99,
            "words": 2
        }
    },

I ran segprint and the 030-iob fuzzer. Both seemed happy with this, so I'm inclined to commit this as is and move forward.

Signed-off-by: John McMaster <johndmcmaster@gmail.com>
Signed-off-by: John McMaster <johndmcmaster@gmail.com>
Signed-off-by: John McMaster <johndmcmaster@gmail.com>
Signed-off-by: John McMaster <johndmcmaster@gmail.com>
Copy link
Contributor

@litghost litghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does the segbits file look like for a negative base word offset?
Do the negative base word offset round trip (bits -> fasm -> bit -> fasm)?

fuzzers/005-tilegrid/add_tdb.py Show resolved Hide resolved
@mcmasterg
Copy link
Contributor Author

No, there is not a negative segbits file. Only the upper half of the file is used

@litghost
Copy link
Contributor

No, there is not a negative segbits file. Only the upper half of the file is used

Not sure I understand what is being suggested. Are we saying that LIOB33_SING_X0Y100 and LIOB33_SING_X0Y149 will use different database entries? LIOB33_SING_X0Y100 will use Y1 entries and LIOB33_SING_X0Y149 will use Y0 entries?

Signed-off-by: John McMaster <johndmcmaster@gmail.com>
@mcmasterg
Copy link
Contributor Author

the latter

@mithro mithro changed the title Iob sign Support IOB tiles with only one site (IOB_SING) Jan 11, 2019
@@ -11,24 +11,19 @@


def gen_iobs():
'''
IOB33S: main IOB of a diff pair
IOB33M: secondary IOB of a diff pair
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IOB33S was actually the main? I would have guess the S and M stood for Master, Slave or Main, Secondary.

@mithro
Copy link
Contributor

mithro commented Jan 11, 2019

If I understand correctly, what you are saying is that;

  • Like CLBs, an IOBs has two sites, Y0 and Y1.
  • However, LIOB33_SING is a special case of an LIOB33 were either the Y0 or the Y1 site is missing.

How do we deal with this in CLBL verse CLBM. In one case we have two SLICEL and the other case we have SLICEL and a SLICEM? IE The table below;

Tile Site Y0 Site Y1
CLBL SLICEL SLICEL
CLBM SLICEL SLICEM

Isn't the IOB just three cases like below?

Tile Site Y0 Site Y1
IOB33 IOB IOB
IOB33S_TOP <empty> IOB
IOB33S_BOT IOB <empty>

Is it really hard to make the tools able to handle the "<empty>" in the first site of a tile?

@JohnDMcMaster
Copy link
Contributor

JohnDMcMaster commented Jan 11, 2019

Tim: is that question for me or @litghost ?

Maybe to answer: the problem is that the tile_type for "IOB33S_TOP" and "IOB33S_BOT" is the same

@litghost
Copy link
Contributor

litghost commented Jan 11, 2019

Tim: is that question for me or @litghost ?

Maybe to answer: the problem is that the tile_type for "IOB33S_TOP" and "IOB33S_BOT" is the same

Exactly, IOB33S_TOP and IOB33S_BOT are both just IOB33_SING. To make matters worse, the existing convention for tile type definition of IOB33_SING doesn't even defined a site Y0 and site Y1, only a site Y0. The current way tile types are generated has no ability to detect whether the _SING tile is a top or bottom and generate a site Y0 only for the top and site Y1 only for the bottom (or vise-versa).

The ideal case would've been for these tiles to shares the same segbits. I assume we've confirmed this is not the case?

@mithro
Copy link
Contributor

mithro commented Jan 11, 2019

Can we just special case IOBS and generate the IOBS_TOP / IOBS_BOT ourselves?

@litghost
Copy link
Contributor

Can we just special case IOBS and generate the IOBS_TOP / IOBS_BOT ourselves?

Sure, we just need to agree on an approach. If we go with iob_top and iob_bot, it changes how the fuzzers will output database data. The original approach was to have both _top and _bot in the same segbit. It's not obvious which requires less rework overall, but I'd say that iob_top and iob_bot is probably most consistent with how the other tiles work.

@litghost litghost closed this Mar 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants