diff mbox series

ALSA: hda: hdac-i915: make i915 timeout configurable

Message ID 20230915134527.16679-1-peter.ujfalusi@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series ALSA: hda: hdac-i915: make i915 timeout configurable | expand

Commit Message

Peter Ujfalusi Sept. 15, 2023, 1:45 p.m. UTC
From: Fred Oh <fred.oh@linux.intel.com>

When enabling new platforms, missing dependencies on i915 driver
updates cause i915 timeouts. Rather than wait 60s for no good reason,
this patch adds a kernel parameter so that developers can cut to the
chase and skip a known problem.

Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Chao Song <chao.song@linux.intel.com>
Signed-off-by: Fred Oh <fred.oh@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
---
 sound/hda/hdac_i915.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

Takashi Iwai Sept. 15, 2023, 3:43 p.m. UTC | #1
On Fri, 15 Sep 2023 15:45:27 +0200,
Peter Ujfalusi wrote:
> 
> From: Fred Oh <fred.oh@linux.intel.com>
> 
> When enabling new platforms, missing dependencies on i915 driver
> updates cause i915 timeouts. Rather than wait 60s for no good reason,
> this patch adds a kernel parameter so that developers can cut to the
> chase and skip a known problem.
> 
> Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
> Reviewed-by: Chao Song <chao.song@linux.intel.com>
> Signed-off-by: Fred Oh <fred.oh@linux.intel.com>
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

The change looks trivial, but this would be little use if we support
EPROBE_DEFER, right?  Or does it need to be merged into 6.6?


thanks,

Takashi
Pierre-Louis Bossart Sept. 15, 2023, 3:46 p.m. UTC | #2
On 9/15/23 11:43, Takashi Iwai wrote:
> On Fri, 15 Sep 2023 15:45:27 +0200,
> Peter Ujfalusi wrote:
>>
>> From: Fred Oh <fred.oh@linux.intel.com>
>>
>> When enabling new platforms, missing dependencies on i915 driver
>> updates cause i915 timeouts. Rather than wait 60s for no good reason,
>> this patch adds a kernel parameter so that developers can cut to the
>> chase and skip a known problem.
>>
>> Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
>> Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
>> Reviewed-by: Chao Song <chao.song@linux.intel.com>
>> Signed-off-by: Fred Oh <fred.oh@linux.intel.com>
>> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
> 
> The change looks trivial, but this would be little use if we support
> EPROBE_DEFER, right?  Or does it need to be merged into 6.6?

It'd be fine to keep this in the SOF tree for now, and remove it from
our tree once the deferred probe is implemented.
Takashi Iwai Sept. 15, 2023, 3:49 p.m. UTC | #3
On Fri, 15 Sep 2023 17:46:32 +0200,
Pierre-Louis Bossart wrote:
> 
> On 9/15/23 11:43, Takashi Iwai wrote:
> > On Fri, 15 Sep 2023 15:45:27 +0200,
> > Peter Ujfalusi wrote:
> >>
> >> From: Fred Oh <fred.oh@linux.intel.com>
> >>
> >> When enabling new platforms, missing dependencies on i915 driver
> >> updates cause i915 timeouts. Rather than wait 60s for no good reason,
> >> this patch adds a kernel parameter so that developers can cut to the
> >> chase and skip a known problem.
> >>
> >> Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> >> Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
> >> Reviewed-by: Chao Song <chao.song@linux.intel.com>
> >> Signed-off-by: Fred Oh <fred.oh@linux.intel.com>
> >> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> >> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
> > 
> > The change looks trivial, but this would be little use if we support
> > EPROBE_DEFER, right?  Or does it need to be merged into 6.6?
> 
> It'd be fine to keep this in the SOF tree for now, and remove it from
> our tree once the deferred probe is implemented.

OK, then let's postpone (or drop) this.  If EPROBE_DEFER stuff can't
be achieved in time, we can reconsider to take this workaround for
6.7.


thanks,

Takashi
diff mbox series

Patch

diff --git a/sound/hda/hdac_i915.c b/sound/hda/hdac_i915.c
index b428537f284c..5927d5200785 100644
--- a/sound/hda/hdac_i915.c
+++ b/sound/hda/hdac_i915.c
@@ -11,6 +11,10 @@ 
 #include <sound/hda_i915.h>
 #include <sound/hda_register.h>
 
+static int hdac_i915_timeout_ms = 60000;
+module_param(hdac_i915_timeout_ms, int, 0444);
+MODULE_PARM_DESC(hdac_i915_timeout_ms, "i915 initialization timeout in msec");
+
 /**
  * snd_hdac_i915_set_bclk - Reprogram BCLK for HSW/BDW
  * @bus: HDA core bus
@@ -165,9 +169,8 @@  int snd_hdac_i915_init(struct hdac_bus *bus)
 	if (!acomp->ops) {
 		if (!IS_ENABLED(CONFIG_MODULES) ||
 		    !request_module("i915")) {
-			/* 60s timeout */
 			wait_for_completion_killable_timeout(&acomp->master_bind_complete,
-							     msecs_to_jiffies(60 * 1000));
+						     msecs_to_jiffies(hdac_i915_timeout_ms));
 		}
 	}
 	if (!acomp->ops) {