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

[pattgen] V2(S) Signoff #21039

Closed
msfschaffner opened this issue Jan 25, 2024 · 3 comments
Closed

[pattgen] V2(S) Signoff #21039

msfschaffner opened this issue Jan 25, 2024 · 3 comments
Assignees
Labels
Component:DV DV issue: testbench, test case, etc. IP:pattgen Type:Signoff

Comments

@msfschaffner
Copy link
Contributor

Description

Ensure V2(S) signoff criteria are still maintained (this is not a focus area block).

@msfschaffner msfschaffner added this to the Earlgrey-PROD.M2 milestone Jan 25, 2024
@msfschaffner msfschaffner added the Component:DV DV issue: testbench, test case, etc. label Jan 25, 2024
@rswarbrick
Copy link
Contributor

Commits since Earlgrey-ES tapeout

As of commit: 2714197

$ git log Earlgrey-M2.5.2-RC0..HEAD --oneline hw/ip/pattgen

gives:

  • 61a237e [util/reggen] reverse order of substruct generation
  • de31bdf [reggen] Remove the devmode input
  • 975a6eb [adc_ctrl,dv] Tidy up access to intr_state in env_cfg files
  • 4c38584 [pattgen,SiVal] Add features to pattgen.hjson
  • 1b16ca2 [reggen] Add mubi support SWAccess that sets/clears a reg
  • 59f8142 [doc] Moved badges over to using hosted images
  • 17dce53 [doc] pattgen registers and interfaces now use CMDGEN
  • 7688e71 [reggen] Add initial support for version and cip_id hjson fields
  • fbd888e Revert "[reggen] Add CIP_IDs and bump all major versions"
  • 0ba10b3 [reggen] Add CIP_IDs and bump all major versions

Skipping documentation tooling changes and the reverted change gives:

  • 61a237e [util/reggen] reverse order of substruct generation
    • This just rearranges some substructs. It will not be visible at block-level, so won't affect V2(S).
  • de31bdf [reggen] Remove the devmode input
    • No change to behaviour.
  • 975a6eb [adc_ctrl,dv] Tidy up access to intr_state in env_cfg files
    • Code tidyup (no functional change)
  • 4c38584 [pattgen,SiVal] Add features to pattgen.hjson
    • SiVal feature list. Downstream of V2(S)
  • 1b16ca2 [reggen] Add mubi support SWAccess that sets/clears a reg
    • Just a tooling change: this explicitly sets a zero mubi flag for
      fields, which was the default behaviour before.
  • 7688e71 [reggen] Add initial support for version and cip_id hjson fields
    • Only changes hjson file. No change to design or DV code.

Issues closed since the Earlgrey-ES tapeout

Currently open issues

Based on the filter is:issue is:open pattgen, but dropping D2(S)/V2(S) signoff and a sival tracking issue.

Coverage report from 21/02/2024

testplan-progress

Summary

There have essentially been no changes to the design or testbench. And
there are no new TODOs. For V2(S), the only requirements that might be
failing are pass rate and coverage.

  • Pass rate is 100%, except for a V3 test (not needed for V2(S)).
  • Code coverage is high (all scores above 96%)
  • BUT functional coverage is 89.95%, just below the 90% target.

Investigating the missing covergroup coverage, I found two items:

  • m_outstanding_item_w_same_addr_cov_obj
    • From tl_agent
    • This tracks a situation where two outstanding items have the same
      address in the TL interface.
    • Drilling into the coverage report, we're never seeing that happen
      (nor are we seeing transitions to/from that state)
  • tl_intg_err_cgs_wrap
    • From CIP base classes (sampled in cip_base_scoreboard.sv)
    • For cp_num_data_err_bits, we are seeing positive values rarely and
      the value 2 happens not to appear.
    • For cp_is_mem, the problem is that we're never seeing an access to
      a memory address. I don't believe that pattgen defines any
      memories, so this is unsurprising!

missing-coverage

In both cases, these covergroups are checking corner cases for generic
TL code. I think we might reasonably waive the requirement.

However, if we must improve the functional coverage above 90% for
signoff then the target is probably the "same_addr" coverage object.
To hit that, we probably just need to tweak an existing test to queue
up more TL transactions (reusing addresses).

@msfschaffner
Copy link
Contributor Author

Thanks for the analysis @rswarbrick.
Given the priorities, I think we can accept 89.95% coverage for covergroups in this case ;).

As for the signoff order, it probably makes sense to use most of this analysis to close out the D2S issue here first: #21004

Otherwise I think this is ok to sign-off at V2S, given the pass rates and coverage metrics reported.

@rswarbrick
Copy link
Contributor

D2S issue now closed (it was easier than this one!). Closing this as discussed above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component:DV DV issue: testbench, test case, etc. IP:pattgen Type:Signoff
Projects
None yet
Development

No branches or pull requests

3 participants