| Message ID | 20180323083048.13327-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Chris Wilson (2018-03-23 08:30:48) > As we are careful not to register external interfaces before the > internals are brought up, we are not dependent upon a synchronous > probing and can allow ourselves to be probed from a secondary thread > during system bootup. We already do relegate some configuration to > asynchronous tasks (such as setting up the fbdev), now do the entire > probe. > > References: https://bugs.freedesktop.org/show_bug.cgi?id=105622 > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Imre Deak <imre.deak@intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: David Weinehall <david.weinehall@linux.intel.com> For doing what it says on the tin, Tested-by: Paul Menzel <pmenzel@molgen.mpg.de> -Chris
On Fri, Mar 23, 2018 at 08:30:48AM +0000, Chris Wilson wrote: > As we are careful not to register external interfaces before the > internals are brought up, we are not dependent upon a synchronous > probing and can allow ourselves to be probed from a secondary thread > during system bootup. We already do relegate some configuration to > asynchronous tasks (such as setting up the fbdev), now do the entire > probe. > > References: https://bugs.freedesktop.org/show_bug.cgi?id=105622 > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> LGTM, and still seems to apply cleanly. Reviewed-by: David Weinehall <david.weinehall@linux.intel.com> > Cc: Imre Deak <imre.deak@intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: David Weinehall <david.weinehall@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_pci.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c > index 4364922e935d..be7b03d48229 100644 > --- a/drivers/gpu/drm/i915/i915_pci.c > +++ b/drivers/gpu/drm/i915/i915_pci.c > @@ -726,6 +726,7 @@ static struct pci_driver i915_pci_driver = { > .probe = i915_pci_probe, > .remove = i915_pci_remove, > .driver.pm = &i915_pm_ops, > + .driver.probe_type = PROBE_PREFER_ASYNCHRONOUS, > }; > > static int __init i915_init(void) > -- > 2.16.2 >
Quoting David Weinehall (2018-04-26 13:41:57) > On Fri, Mar 23, 2018 at 08:30:48AM +0000, Chris Wilson wrote: > > As we are careful not to register external interfaces before the > > internals are brought up, we are not dependent upon a synchronous > > probing and can allow ourselves to be probed from a secondary thread > > during system bootup. We already do relegate some configuration to > > asynchronous tasks (such as setting up the fbdev), now do the entire > > probe. > > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=105622 > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > LGTM, and still seems to apply cleanly. > > Reviewed-by: David Weinehall <david.weinehall@linux.intel.com> The one downside to this was that it's only useful for builtins. For modules, the finit_module call serialises on the async task, so parallelisation has to occur in userspace. > > Cc: Imre Deak <imre.deak@intel.com> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Cc: David Weinehall <david.weinehall@linux.intel.com> > > --- > > drivers/gpu/drm/i915/i915_pci.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c > > index 4364922e935d..be7b03d48229 100644 > > --- a/drivers/gpu/drm/i915/i915_pci.c > > +++ b/drivers/gpu/drm/i915/i915_pci.c > > @@ -726,6 +726,7 @@ static struct pci_driver i915_pci_driver = { > > .probe = i915_pci_probe, > > .remove = i915_pci_remove, > > .driver.pm = &i915_pm_ops, > > + .driver.probe_type = PROBE_PREFER_ASYNCHRONOUS, I thought I sent a v2 with IS_BUILTIN() ? PREFER_ASYNC : 0, possibly not. -Chris
On Thu, Apr 26, 2018 at 03:41:57PM +0300, David Weinehall wrote: > On Fri, Mar 23, 2018 at 08:30:48AM +0000, Chris Wilson wrote: > > As we are careful not to register external interfaces before the > > internals are brought up, we are not dependent upon a synchronous > > probing and can allow ourselves to be probed from a secondary thread > > during system bootup. We already do relegate some configuration to > > asynchronous tasks (such as setting up the fbdev), now do the entire > > probe. > > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=105622 > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > LGTM, and still seems to apply cleanly. > > Reviewed-by: David Weinehall <david.weinehall@linux.intel.com> One problem with this is that atm in snd_hdac_i915_init() request_module() is expected to return only once the i915 probe function has run. With async probing this won't be any more the case. +Takashi > > > Cc: Imre Deak <imre.deak@intel.com> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Cc: David Weinehall <david.weinehall@linux.intel.com> > > --- > > drivers/gpu/drm/i915/i915_pci.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c > > index 4364922e935d..be7b03d48229 100644 > > --- a/drivers/gpu/drm/i915/i915_pci.c > > +++ b/drivers/gpu/drm/i915/i915_pci.c > > @@ -726,6 +726,7 @@ static struct pci_driver i915_pci_driver = { > > .probe = i915_pci_probe, > > .remove = i915_pci_remove, > > .driver.pm = &i915_pm_ops, > > + .driver.probe_type = PROBE_PREFER_ASYNCHRONOUS, > > }; > > > > static int __init i915_init(void) > > -- > > 2.16.2 > >
On Thu, 26 Apr 2018, Imre Deak <imre.deak@intel.com> wrote: > On Thu, Apr 26, 2018 at 03:41:57PM +0300, David Weinehall wrote: >> On Fri, Mar 23, 2018 at 08:30:48AM +0000, Chris Wilson wrote: >> > As we are careful not to register external interfaces before the >> > internals are brought up, we are not dependent upon a synchronous >> > probing and can allow ourselves to be probed from a secondary thread >> > during system bootup. We already do relegate some configuration to >> > asynchronous tasks (such as setting up the fbdev), now do the entire >> > probe. >> > >> > References: https://bugs.freedesktop.org/show_bug.cgi?id=105622 >> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> >> >> LGTM, and still seems to apply cleanly. >> >> Reviewed-by: David Weinehall <david.weinehall@linux.intel.com> > > One problem with this is that atm in snd_hdac_i915_init() > request_module() is expected to return only once the i915 probe function > has run. With async probing this won't be any more the case. > > +Takashi As I wrote to Yang (Cc'd) in the context of another patch, one approach that takes care of this would be adding a completion in hdac_i915.c, waiting for it with a timeout below request_module("i915") in snd_hdac_i915_init(), and completing it in hdac_component_master_bind(). How long the timeout should be is anyone's guess... BR, Jani. > >> >> > Cc: Imre Deak <imre.deak@intel.com> >> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> >> > Cc: David Weinehall <david.weinehall@linux.intel.com> >> > --- >> > drivers/gpu/drm/i915/i915_pci.c | 1 + >> > 1 file changed, 1 insertion(+) >> > >> > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c >> > index 4364922e935d..be7b03d48229 100644 >> > --- a/drivers/gpu/drm/i915/i915_pci.c >> > +++ b/drivers/gpu/drm/i915/i915_pci.c >> > @@ -726,6 +726,7 @@ static struct pci_driver i915_pci_driver = { >> > .probe = i915_pci_probe, >> > .remove = i915_pci_remove, >> > .driver.pm = &i915_pm_ops, >> > + .driver.probe_type = PROBE_PREFER_ASYNCHRONOUS, >> > }; >> > >> > static int __init i915_init(void) >> > -- >> > 2.16.2 >> > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, 27 Apr 2018 10:04:13 +0200, Jani Nikula wrote: > > On Thu, 26 Apr 2018, Imre Deak <imre.deak@intel.com> wrote: > > On Thu, Apr 26, 2018 at 03:41:57PM +0300, David Weinehall wrote: > >> On Fri, Mar 23, 2018 at 08:30:48AM +0000, Chris Wilson wrote: > >> > As we are careful not to register external interfaces before the > >> > internals are brought up, we are not dependent upon a synchronous > >> > probing and can allow ourselves to be probed from a secondary thread > >> > during system bootup. We already do relegate some configuration to > >> > asynchronous tasks (such as setting up the fbdev), now do the entire > >> > probe. > >> > > >> > References: https://bugs.freedesktop.org/show_bug.cgi?id=105622 > >> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > >> > >> LGTM, and still seems to apply cleanly. > >> > >> Reviewed-by: David Weinehall <david.weinehall@linux.intel.com> > > > > One problem with this is that atm in snd_hdac_i915_init() > > request_module() is expected to return only once the i915 probe function > > has run. With async probing this won't be any more the case. > > > > +Takashi > > As I wrote to Yang (Cc'd) in the context of another patch, one approach > that takes care of this would be adding a completion in hdac_i915.c, > waiting for it with a timeout below request_module("i915") in > snd_hdac_i915_init(), and completing it in > hdac_component_master_bind(). How long the timeout should be is anyone's > guess... Yes, that's also a workaround I'd take at next. Also the code can have a vgacon_text_force() check to skip the nomodeset case, at least, too. thanks, Takashi
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index 4364922e935d..be7b03d48229 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -726,6 +726,7 @@ static struct pci_driver i915_pci_driver = { .probe = i915_pci_probe, .remove = i915_pci_remove, .driver.pm = &i915_pm_ops, + .driver.probe_type = PROBE_PREFER_ASYNCHRONOUS, }; static int __init i915_init(void)
As we are careful not to register external interfaces before the internals are brought up, we are not dependent upon a synchronous probing and can allow ourselves to be probed from a secondary thread during system bootup. We already do relegate some configuration to asynchronous tasks (such as setting up the fbdev), now do the entire probe. References: https://bugs.freedesktop.org/show_bug.cgi?id=105622 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Imre Deak <imre.deak@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: David Weinehall <david.weinehall@linux.intel.com> --- drivers/gpu/drm/i915/i915_pci.c | 1 + 1 file changed, 1 insertion(+)