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;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
