ASoC: SOF: Intel: pci-tgl: unblock S5 entry if DMA stop has failed #3388
Conversation
kv2019i
requested review from
ajitkupandey,
bardliao,
dbaluta,
keyonjie,
kuanhsuncheng,
lgirdwood,
libinyang,
lyakh,
plbossart,
ranj063,
ujfalusi and
yaochunhung
as code owners
| active_streams = hda_dsp_check_for_dma_streams(sdev); | ||
|
|
||
| sdev->system_suspend_target = SOF_SUSPEND_S3; | ||
| ret = snd_sof_suspend(sdev->dev); |
There was a problem hiding this comment.
@ranj063 had a comment ealrlier #3377 (comment)
what if this fails? static checkers will complain that 'ret' is overwritten before being used in some cases.
There was a problem hiding this comment.
I'd prefer to keep going. If the DMA state is still running after failed suspend attempt, that will block system entry to S5.
But hmm, static checker could complain about it, so let me add another variable.
| active_streams = hda_dsp_check_for_dma_streams(sdev); | ||
|
|
||
| sdev->system_suspend_target = SOF_SUSPEND_S3; | ||
| ret = snd_sof_suspend(sdev->dev); |
There was a problem hiding this comment.
I'd prefer to keep going. If the DMA state is still running after failed suspend attempt, that will block system entry to S5.
But hmm, static checker could complain about it, so let me add another variable.
If system shutdown has not been completed cleanly, it is possible the DMA stream shutdown has not been done, or was not clean. If this is the case, Intel TGL/ADL HDA platforms may fail to shutdown cleanly due to pending HDA DMA transactions. To avoid this, detect this scenario in the shutdown callback, and perform an additional controller reset. This has been tested to unblock S5 entry if this condition is hit. Co-developed-by: Archana Patni <archana.patni@intel.com> Signed-off-by: Archana Patni <archana.patni@intel.com> Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
kv2019i
force-pushed
the
topic/hda-shutdown-fix-tgladlonly
branch
from
252cff3
to
380b0c7
Compare
|
A new version addressing comments so far. |
| active_streams); | ||
| ret2 = hda_dsp_s5_quirk(sdev); | ||
| if (ret2 < 0) | ||
| dev_err(sdev->dev, "shutdown recovery failed (%d)\n", ret2); |
There was a problem hiding this comment.
nitpick: but if ret was OK, but ret2 was not, then should we
if (!ret)
ret = ret2;or does it matter at this point?
There was a problem hiding this comment.
@ujfalusi I'd say it doesn't matter at this point. The pci_driver driver ops has a void return type anyways.
More limited-scope-of-platforms version of #3377
If system shutdown has not been completed cleanly, it is possible the
DMA stream shutdown has not been done, or was not clean.
If this is the case, Intel TGL/ADL HDA platforms may fail to shutdown
cleanly due to pending HDA DMA transactions. To avoid this, detect this
scenario in the shutdown callback, and perform an additional controller
reset. This has been tested to unblock S5 entry if this condition is
hit.
Co-developed-by: Archana Patni archana.patni@intel.com
Signed-off-by: Archana Patni archana.patni@intel.com
Signed-off-by: Kai Vehmanen kai.vehmanen@linux.intel.com