[Intel-gfx] [RFC] i915: make the probe asynchronous

[Feng Tang]

Hi,

On Wed, Jun 20, 2018 at 11:45:25AM +0200, Takashi Iwai wrote:
> > > >
> > > > Thanks for the info, I see your intension now.
> > > >
> > > > In previous discussion, Jani suggested to use a completion to indicate
> > > > the probe done, I can't figure out another way :)
> > > 
> > > I suggested you do that within hdac_i915.c. Wait in snd_hdac_i915_init()
> > > below request_module(), complete in hdac_component_master_bind().
> > 
> > I thgouth this kind of cross-driver dependency is supposed to be handled
> > using EPROBE_DEFER? Why do we need to hand-roll that here?
> > 
> > Or is this some other kind of synchronization need that needs a bespoke
> > solution?
> 
> The binding with i915 from HD-audio controller is done in an async
> work invoked from the probe callback.  It makes harder to deal with
> EPROBEDEFER.
> 
> IMO, a saner way would be to rather wait for the component binding.
> The component mechanism is there just for that purpose, after all.
> 
> Does a patch like below work (totally untested)?

When I was testing this patch, I further checked the kernel module code,
and found that: we may need NOT to add any new codes to prepare for
i915's async probe feature!

Say when i915 module is being loader due to HDA's request_module() call,
in the callchain, do_init_module() has such code: 

    if (!mod->async_probe_requested && (current->flags & PF_USED_ASYNC))
            async_synchronize_full();

This will garantee the asynced probe is done before it returns.

I have just tested and this seems to be enough. If I am not wrong, then
we can take the i915 async patch directly. What do you think?

Thanks,
Feng


> 
> 
> thanks,
> 
> Takashi
> 
> -- 8< --
> --- a/sound/hda/hdac_i915.c
> +++ b/sound/hda/hdac_i915.c
> @@ -23,6 +23,7 @@
>  #include <sound/hda_register.h>
>  
>  static struct i915_audio_component *hdac_acomp;
> +static DECLARE_COMPLETION(acomp_bound);
>  
>  /**
>   * snd_hdac_set_codec_wakeup - Enable / disable HDMI/DP codec wakeup
> @@ -284,6 +285,7 @@ static int hdac_component_master_bind(struct device *dev)
>  		goto out_unbind;
>  	}
>  
> +	complete_all(&acomp_bound);
>  	return 0;
>  
>  out_unbind:
> @@ -382,11 +384,8 @@ int snd_hdac_i915_init(struct hdac_bus *bus)
>  	if (ret < 0)
>  		goto out_err;
>  
> -	/*
> -	 * Atm, we don't support deferring the component binding, so make sure
> -	 * i915 is loaded and that the binding successfully completes.
> -	 */
>  	request_module("i915");
> +	wait_for_completion_timeout(&acomp_bound, 10000); /* 10s timeout */
>  
>  	if (!acomp->ops) {
>  		ret = -ENODEV;