Skip to content

[FEATURE] Allow components/buffers to XRUN #2478

Closed
Closed
[FEATURE] Allow components/buffers to XRUN#2478
Assignees
akloniex
Labels
enhancementNew feature or request
Milestone
v1.6

Description

@lgirdwood
lgirdwood
opened on Mar 3, 2020

Is your feature request related to a problem? Please describe.
Currently if a component source buffer underruns or a component sink buffier overruns the pipeline will enter an XRUN status and attempt recovery. This is desired in most pipelines but some topologies need to support use cases where we expect buffers to underrun or overrun.

Host ---> Proc----> Selector0 --> Buf0 ---- > DAI Playback
                      |
                      v
                     Buf1
                      |
                      v
Host <---------------Selector1 <----- Buf2 <----- Echo Ref DAI

In the example above we two host PCMs that can be independently started/stopped thereby causing buf1 to either underrun or overrun (and stop the pipelines). Buf1 should be permmitted to underrun or overrun without invoking pipeline XRUN logic and should over write oldest data (for overrun) and readback 0s (for underrun).

Describe the solution you'd like
Add two new topology token for buffers.

TOKEN_BUFFER_UNDERRUN_NO_XRUN
TOKEN_BUFFER_OVERRUN_NO_XRUN

Firmware should process these tokens and if set for a buffer will not invoke pipeline XRUN status.
If buffer overflows, it should overwrite oldest data.
If buffer underruns, it should read back 0s.

Activity

  1. lgirdwood
    added
    enhancementNew feature or request
    on Mar 3, 2020
  2. lgirdwood
    added this to the v1.6 milestone on Mar 3, 2020

  3. lgirdwood commented on Mar 3, 2020

    @lgirdwood
    lgirdwood
    on Mar 3, 2020
    MemberAuthor

    Needed for #2407 and #2462

  4. lgirdwood commented on Mar 3, 2020

    @lgirdwood
    lgirdwood
    on Mar 3, 2020
    MemberAuthor

    @akloniex @sebcarlucci fyi.

  5. bkokoszx commented on Mar 4, 2020

    @bkokoszx
    bkokoszx
    on Mar 4, 2020
    Collaborator

    @lgirdwood
    It will also be required in smart amp topology (#1896). I don't know whether somebody is working on it right now. If not, I can assign myself to it.

  6. lgirdwood commented on Mar 4, 2020

    @lgirdwood
    lgirdwood
    on Mar 4, 2020
    MemberAuthor

    @bkokoszx can you have a chat with @akloniex - also needed for echo ref

  7. akloniex
    self-assigned this
    on Mar 5, 2020
  8. lgirdwood
    changed the title [-][FEATURE] Allow componnets/buffers to XRUN[/-] [+][FEATURE] Allow components/buffers to XRUN[/+] on Mar 5, 2020

  9. akloniex commented on Mar 16, 2020

    @akloniex
    akloniex
    on Mar 16, 2020
    Member

    @lgirdwood Regarding implementation details for the underrun case, I can think of 2 ways of approaching the subject:

    1. Minimal changes, mostly focused around components' copy functions (check underrun-related flag in buffer, read as much as buffer has, fill in the zeros as needed)
    2. Far-fetched changes regarding buffer read management, so that underrun would be transparent to the components, by utilizing behaviour similar to std fread()

    Now, the first approach is much easier to develop and doesn't involve any big changes to the buffer API, but it comes with quite a heavy drawback - every component has to implement buffer underrun separately. The second approach is the exact opposite.

    If you have any suggestions on how to approach this, how to improve on any of the above ideas, or if you have a different one of your own, then please point me in the right direction.

  10. lbetlej commented on Mar 17, 2020

    @lbetlej
    lbetlej
    on Mar 17, 2020
    Collaborator

    @lgirdwood @akloniex @tlauda could you follow up on idea regarding buffer reader API ?

  11. tlauda commented on Mar 17, 2020

    @tlauda
    tlauda
    on Mar 17, 2020
    Contributor

    @lgirdwood Is this mechanism in some way related to the improved buffer locking API you've mentioned here #2545 (comment)?

  12. lgirdwood commented on Mar 18, 2020

    @lgirdwood
    lgirdwood
    on Mar 18, 2020
    MemberAuthor

    @tlauda yes, hooking into the buffer API is probably the best way forward. i.e.

    /* acquires lock for bytes to read - buffer will 0 bytes here if we underrun AND underrun flag is true */
buffer_rlock(buffer, bytes, flags);
/* acquires lock for bytes to write - buffer wont XRUN here if we overrun AND overrun flag is true */
buffer_wlock(buffer, bytes, flags);

  13. akloniex commented on Mar 26, 2020

    @akloniex
    akloniex
    on Mar 26, 2020
    Member

    @lgirdwood What should xrun-permitted buffers report when asked for avail/free data?
    (uint32_t)-1 seems 'most reasonable', but it doesn't feel comfortable, please clarify that for me.
    If such buffer reported actual avail/free, client component would prevent xrunning in the first place, so such buffers need to alter their reports somehow.

  14. lgirdwood commented on Mar 27, 2020

    @lgirdwood
    lgirdwood
    on Mar 27, 2020
    MemberAuthor

    @akloniex I think it's appropriate to say that these buffers will report they have buffer_size free and buffer_size avail.

  15. lgirdwood commented on Mar 27, 2020

    @lgirdwood
    lgirdwood
    on Mar 27, 2020
    MemberAuthor

    @akloniex in fact we probably want to be more logical here with rules.

    1. Buffer is full data - report buffer_size avail and period_size free
    2. Buffer is empty - report period_size avail and buffer_size free
    3. Buffer is not empty or full - report normally.

    This way we should not block existing users.

  16. paulstelian97 commented on Mar 27, 2020

    @paulstelian97
    paulstelian97
    on Mar 27, 2020
    Collaborator

    How would the data itself be extracted from the buffers now? Until now it was simply by requesting a pointer for each sample from the buffer but now I feel like this approach may not work properly. Did I miss a PR which changes this logic?

  17. lgirdwood commented on Mar 27, 2020

    @lgirdwood
    lgirdwood
    on Mar 27, 2020
    MemberAuthor

    It will us the same approach as the DMA trace buffer, i.e. oldest data is discarded

  18. akloniex commented on Mar 27, 2020

    @akloniex
    akloniex
    on Mar 27, 2020
    Member

    @paulstelian97 @lgirdwood wrote how this situation will be handled, few comments above:

    /* acquires lock for bytes to read - buffer will 0 bytes here if we underrun AND underrun flag is true */
buffer_rlock(buffer, bytes, flags);
/* acquires lock for bytes to write - buffer wont XRUN here if we overrun AND overrun flag is true */
buffer_wlock(buffer, bytes, flags);

    Meaning, that data (zeros) will be superimposed onto the buffer, and following logic - retrieving data pointer and reading is exactly the same.

  19. paulstelian97 commented on Mar 27, 2020

    @paulstelian97
    paulstelian97
    on Mar 27, 2020
    Collaborator

    So the buffer is modified accordingly. Okay.

  20. cujomalainey commented on Jul 31, 2020

    @cujomalainey
    cujomalainey
    on Jul 31, 2020
    Contributor

    This has been completed correct?

  21. lgirdwood commented on Aug 4, 2020

    @lgirdwood
    lgirdwood
    on Aug 4, 2020
    MemberAuthor

    @akloniex @juimonen is this now completed ?

  22. lgirdwood commented on Aug 18, 2020

    @lgirdwood
    lgirdwood
    on Aug 18, 2020
    MemberAuthor

    @bkokoszx @slawblauciak @ktrzcinx can we close this now ? I think all the parts are merged now ?

  23. ktrzcinx commented on Aug 18, 2020

    @ktrzcinx
    ktrzcinx
    on Aug 18, 2020
    Member

    I'have already use it, so I think it's ready

  24. bkokoszx commented on Aug 18, 2020

    @bkokoszx
    bkokoszx
    on Aug 18, 2020
    Collaborator

    @lgirdwood
    I think we can close this via #2745

  25. cujomalainey
    closed this as completedon Aug 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

enhancementNew feature or request

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions