linux-kernel - Re: [PATCH] ASoC: SOF: Intel: Remove deferred probe for SOF

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87edl4f4pb.wl-tiwai@suse.de>
Date:   Wed, 19 Jul 2023 14:47:44 +0200
From:   Takashi Iwai <tiwai@...e.de>
To:     Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>
Cc:     Kai Vehmanen <kai.vehmanen@...ux.intel.com>,
        Alsa-devel <alsa-devel@...a-project.org>,
        sound-open-firmware@...a-project.org, linux-kernel@...r.kernel.org,
        Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>,
        Cezary Rojewski <cezary.rojewski@...el.com>,
        Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>,
        Liam Girdwood <liam.r.girdwood@...ux.intel.com>,
        Peter Ujfalusi <peter.ujfalusi@...ux.intel.com>,
        Bard Liao <yung-chuan.liao@...ux.intel.com>,
        Ranjani Sridharan <ranjani.sridharan@...ux.intel.com>,
        Mark Brown <broonie@...nel.org>,
        Daniel Baluta <daniel.baluta@....com>,
        Matthew Auld <matthew.auld@...el.com>
Subject: Re: [PATCH] ASoC: SOF: Intel: Remove deferred probe for SOF

On Wed, 19 Jul 2023 14:13:59 +0200,
Maarten Lankhorst wrote:
> 
> 
> On 2023-07-19 13:06, Takashi Iwai wrote:
> > On Wed, 19 Jul 2023 11:48:06 +0200,
> > Maarten Lankhorst wrote:
> >> 
> >>      The 60 seconds timeout is a thing "better than complete disablement",
> >>      so it's not ideal, either.  Maybe we can add something like the
> >>      following:
> >>           - Check when the deferred probe takes too long, and warn
> >> it
> >>      - Provide some runtime option to disable the component binding, so
> >>        that user can work around it if needed
> >>      A module option to snd_hdac_i915_init would probably be the
> >> least of all evils
> >> here.
> > 
> > Yes, probably it's the easiest option and sufficient.
> > 
> > 
> > thanks,
> > 
> > Takashi
> Hey,
> 
> Patch below, can be applied immediately iresspective of the other patches.
> 
> ---->8----------
> 
> Selecting CONFIG_DRM selects CONFIG_VIDEO_NOMODESET, which exports
> video_firmware_drivers_only(). This can be used as a first
> approximation
> on whether i915 will be available. It's safe to use as this is only
> built when CONFIG_SND_HDA_I915 is selected by CONFIG_I915.
> 
> It's not completely fool proof, as you can boot with "nomodeset
> i915.modeset=1" to make i915 load regardless, or use
> "i915.force_probe=!*" to never load i915, but the common case of booting
> with nomodeset to disable all GPU drivers this will work as intended.

The check of video_firmware_drivers_only() may help a bit, but I
believe we still need an option to override the behavior, from the
same reason as why i915.modeset option behaves so.  In general,
nomodeset is for a debugging purpose, and without an option, you'll
have no way to re-enable the HD-audio even if you could reload the
graphics driver.


thanks,

Takashi

> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>
> ---
> diff --git a/sound/hda/hdac_i915.c b/sound/hda/hdac_i915.c
> index 1637dc6e630a6..90bcf84f7b2ce 100644
> --- a/sound/hda/hdac_i915.c
> +++ b/sound/hda/hdac_i915.c
> @@ -11,6 +11,8 @@
>  #include <sound/hda_i915.h>
>  #include <sound/hda_register.h>
> 
> +#include <video/nomodeset.h>
> +
>  #define IS_HSW_CONTROLLER(pci) (((pci)->device == 0x0a0c) || \
>  				((pci)->device == 0x0c0c) || \
>  				((pci)->device == 0x0d0c) || \
> @@ -122,6 +124,9 @@ static int i915_gfx_present(struct pci_dev *hdac_pci)
>  {
>  	struct pci_dev *display_dev = NULL;
> 
> +	if (video_firmware_drivers_only())
> +		return false;
> +
>  	for_each_pci_dev(display_dev) {
>  		if (display_dev->vendor == PCI_VENDOR_ID_INTEL &&
>  		    (display_dev->class >> 16) == PCI_BASE_CLASS_DISPLAY &&
>