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

["Gorski, Mateusz" <mateusz.gorski@linux.intel.com>]

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