Re: [PATCH] ASoC: Intel: skl_hda_dsp_generic: Fix NULLptr dereference in autosuspend delay

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




W dniu 8/12/2020 o 3:48 PM, Kai Vehmanen pisze:
Hi,

On Wed, 22 Jul 2020, Pierre-Louis Bossart wrote:

Change the string in conditional statement to "ehdaudio0D0" to ensure
that only the HDAudio codec is handled by this function.
I am not sure this is correct.

I may be wrong, but my understanding is the following:

Before 5bf73b1b1dec, the driver would use the first dailink of the card,
and in the case of devices without an HDaudio codec (e.g. Up2 board) it
would set the auto suspend delay using that first dailink. See the code in
skl_hda_fill_card_info(), it reorders the dailinks when HDaudio codecs are
not present so if you test for 'edhaudio00' you no longer allow for this
HDMI-only case to be handled with autosuspend.

Kai would need to review this, so this will have to wait I am afraid.

So, for_each_card_rtds needs to be context aware (hdmi type). Right now,
introduced _autosuspend_delay is causing kernel panics.
The code before 5bf73b1b1dec would use an HDMI dailink when HDaudio codecs are
not present, so I don't really see the point on being context aware. Either
this never worked or there's a side effect. In both cases, I would kindly ask
that this does not get merged before Kai is back.
the patch from Mateusz might be most pragmatic way to solve this.

The original problem was not setting autosuspend for external HDA codecs
which cause jack detection issues with some codecs. So we added the call
to set autosuspend timeout for all HDA codecs (patch "ASoC: intel/skl/hda
- set autosuspend timeout for hda codecs"). This is not strictly needed
for HDMI, but as it (seemed) cleaner to just call autosuspend for all HDA
codecs, the patch did that. Later we have hit issues with special cases
for HDMI, first with the case of disabled HDMI codec (my patch "ASoC:
intel/skl/hda - fix oops on systems without i915 audio codec") and now
issues with systems using hdac_hdmi.

So what we really want to do is to confirm the codec driver is hdac_hda
(and not hdac_hdmi or any other drivers), and if yes, then call the
autosuspend function. I did spend some time trying to find a clean(er) way
to do this, but codec name seemed the best option. I'll test the hdmi-only
case, but I believe Mateusz patch will work in that case as well.

Br, Kai


Thanks for the review,


is there anything else I can do with this patch?


Also, Mark, please let me know if I should resend this change.


Thanks,
Mateusz




[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux