ASoC: SOF: Intel: hda: refine resume with init chip

[zhuyingjiang]

Unify resume code by using SOF common function hda_dsp_ctrl_init_chip() which can handle both HDA and
non-HDA cases.

[plbossart]

I don't have time to look into this so will have to rely on others to approve.

[ranj063]

@zhuyingjiang while the overall flow looks OK, can you please check why you need to clear WAKESTS twice in hda_dsp_ctrl_init_chip() for the HDA case?

Also, clearing WAKESTS should use snd_sof_dsp_write, update_bits() will not really clear it.

[kv2019i]

Code looks good, but again a few notes to the commit message. See also Ranjani's note on WAKESTS clearing.

Commit message for first -- minor change to make it easier to read.

Set the HDA stream position buffer during init chip. The position buffer
needs to be set in both HDA codec and nocodec cases. Use the SOF
defined function and move it to common code.

Commit message for the second -- here the explanation goes in a weird
order. I think it's better to start with the "unify resume code" to explain why
you are doing this and then add detail about the implementation. Otherwise
it's a bit hard to follow if you start with details and then reveal at the end
why you did it.

Unify resume code by using SOF common function hda_dsp_ctrl_init_chip()
which can handle both HDA and non-HDA cases. Move code to reset
stream-to-link mapping into hda_dsp_ctrl_init_chip.

Also in both, you can word-wrap to 74 columns.

[plbossart]

@zhuyingjiang also please resolve conflicts

[zhuyingjiang]

Also, clearing WAKESTS should use snd_sof_dsp_write, update_bits() will not really clear it.

@ranj063 As I checked, here should use snd_sof_dsp_write, I will update it.

[zhuyingjiang]

Code looks good, but again a few notes to the commit message. See also Ranjani's note on WAKESTS clearing.

Commit message for first -- minor change to make it easier to read.

Set the HDA stream position buffer during init chip. The position buffer
needs to be set in both HDA codec and nocodec cases. Use the SOF
defined function and move it to common code.

Commit message for the second -- here the explanation goes in a weird
order. I think it's better to start with the "unify resume code" to explain why
you are doing this and then add detail about the implementation. Otherwise
it's a bit hard to follow if you start with details and then reveal at the end
why you did it.

Unify resume code by using SOF common function hda_dsp_ctrl_init_chip()
which can handle both HDA and non-HDA cases. Move code to reset
stream-to-link mapping into hda_dsp_ctrl_init_chip.

Also in both, you can word-wrap to 74 columns.

Thanks @kv2019i Will update it after some other check.

[zhuyingjiang]

@plbossart @kv2019i @ranj063 Updated.

[ranj063]

Looks good, thanks!

[kv2019i]

Thanks, commits look good!

@RanderWang , @lyakh and @ranj063 : just to double check, is is safe to not do WAKESTS clearing before controller reset? The old code was wrong (it should not use update bits), but given we had many issues with the headset/wake detection, I want to be extra careful here.

[ranj063]

Thanks, commits look good!

@RanderWang , @lyakh and @ranj063 : just to double check, is is safe to not do WAKESTS clearing before controller reset? The old code was wrong (it should not use update bits), but given we had many issues with the headset/wake detection, I want to be extra careful here.

@kv2019i I have a feeling this should be fine. With WAKEEN , we could be resuming because of a codec status change which is recorded before the controller is reset. But let me test this PR today just to make sure

[zhuyingjiang]

Thanks, commits look good!

@RanderWang , @lyakh and @ranj063 : just to double check, is is safe to not do WAKESTS clearing before controller reset? The old code was wrong (it should not use update bits), but given we had many issues with the headset/wake detection, I want to be extra careful here.

@kv2019i I have already test this on WHL RVP. It is OK.
The register write steps are kept the same as other drivers, there clear is called multi-times in different functions.
Here we combine them in one function. and As WAKESTS resides in the Primary well, only one clear is OK for now.

[kv2019i]

@zhuyingjiang

@kv2019i I have already test this on WHL RVP. It is OK.

Ok great, thanks for confirming. Then this looks good to go.

[ranj063]

@zhuyingjiang can you please update your branch ?

[zhuyingjiang]

@zhuyingjiang can you please update your branch ?

@ranj063 Sorry I don't get what you mean by "update branch", the target branch is not sof-dev? or there still need some more modification?

[ranj063]

@zhuyingjiang can you please update your branch ?

@ranj063 Sorry I don't get what you mean by "update branch", the target branch is not sof-dev? or there still need some more modification?

@zhuyingjiang the status shows that the branch is out-of-date with the base branch. So I cant merge it as is

[zhuyingjiang]

@zhuyingjiang can you please rebase and resubmit. It still shows that the branch is out of date

@ranj063 @RanderWang @keyonjie Rebased.

[zhuyingjiang]

@lyakh

[zhuyingjiang]

@lyakh Your requested changes need to be addressed to merge the PR. Please review for me, thanks!

[zhuyingjiang]

@lyakh removed that initialization.

[ranj063]

@zhuyingjiang sorry I've just merged the upstream changes. So could you please rebase and resubmit?

[zhuyingjiang]

@zhuyingjiang sorry I've just merged the upstream changes. So could you please rebase and resubmit?

OK.

[zhuyingjiang]

@ranj063 @lyakh Rebased.

[lyakh]

formally looks fine to me.