drivers: dai: intel: ssp: RX FIFO drain rework and DMA request management changes (take 2) #64604
Conversation
…m blob When loading the SSP configuration from a blob ignore the bits which would enable the TX/RX or DMA requests at configuration phase. The TX/RX enable and DMA request is handled by the driver itself. If the blob wrongly enables any of the bits can have runtime (startup time) seemingly random issues. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
The actual FIFO depth is 32 and not 16 on CAV2.5 and AVS platforms. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
The receive FIFO needs to be drained in a different way depending when it is done. - before start If the RX FIFO is in overflow state then we must read all the entries out to empty it (it was after all full). - before stop The DMA might be already running to read out data. Check the FIFO level change in one sample time which gives us the needed information to decide to wait for another loop for the DMA burst to finish, wait for the DMA to start it's burst (DMA request was asserted) or drain the FIFO directly. No need to drain the RX fifo at probe time. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
ujfalusi
requested review from
kv2019i,
marcinszkudlinski and
abonislawski
as code owners
ujfalusi
force-pushed
the
peter/pr/ssp_updates_02
branch
from
e787569
to
39492f5
Compare
|
changes since v1:
|
ujfalusi
force-pushed
the
peter/pr/ssp_updates_02
branch
from
39492f5
to
c1e21f0
Compare
…ically Do not keep both DMA request enabled whenever the SSP is in use. Manage the SSCR1_TSRE and SSCR1_RSRE bits in sync with the enabled directions. When only playback is used there is no need to have the RX DMA request enabled for example. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Done and the 6th internal and SOF CI check is also passed. |
|
@kv2019i, the compliance issue is fixed now. |
|
@marcinszkudlinski, @abonislawski, can you take a look at this to move it forward? |
|
|
||
| if (regs->sstsa & SSTSA_TXEN || regs->ssrsa & SSRSA_RXEN || | ||
| regs->ssc1 & (SSCR1_RSRE | SSCR1_TSRE)) { | ||
| LOG_INF("%s: Ignoring %s%s%s%sfrom blob", __func__, |
There was a problem hiding this comment.
@ujfalusi we're seeing this error message pretty much everywhere:
Any idea why?
There was a problem hiding this comment.
Sorry the LOG_ERR was added later in commit 6423bc3 actually.
There was a problem hiding this comment.
@marc-hb, no, this print remained LOG_INF:
6423bc3#diff-69c3bde06e9c145ab8321fcce947ac6d765aa2ffe873641125e6d85d41f45943L1797-R1770
The blob is incorrect, it has the TX/RX enabled and/or the TX/RX DMA enabled. This is invalid and it is ignored, we don't want to enable the SSP when we load the blob, we want to enable it when it needs to be enabled.
We can lower the level, but the blob should be fixed.
There was a problem hiding this comment.
no, this print remained LOG_INF:
Yes that's what I meant, sorry for the confusion.
We can lower the level, but the blob should be fixed.
So should the "Ignoring ..." line be a WARN maybe?
There was a problem hiding this comment.
I would leave it to INF, the blob might be coming from BIOS/ROM and not from tplg, it is hard to fix those.
Hi,
#62596 caused regression in internal CI test and got reverted, however the RX FIFO rework is needed for error free operation.
The root cause of the failure has been identified and fixed by a new patch (the first one in the PR):
The generated blob that is used by the internal CI had the RX/TX enable bits set along with the RX/TX DMA request enable bits. This is not correct and it will be fixed in the blob but the driver should be able to handle such cases by ignoring these enable bits from the blob.
The SSP driver manages both the RX/TX enable and DMA request bits itself.
The other new patch in the series is the change on how the DMA request is managed. There is no reason to enable both at the same time since if we have only TX then why would we have the RX DMA request enabled?
The DMA request has nothing to do with clock generation either.
This also makes the code a bit easier to follow because we don't have the DMA request toggling scattered around.
The PR is passing now both SOF CI and Internal CI (5 test runs has been done to make sure that we will not end up reverting again).