Re: [PATCH 1/1] drm/i915: move audio component intialization before audio driver use it

From: Jani Nikula <jani.nikula@linux.intel.com>
To: "Shi, Yang A" <yang.a.shi@intel.com>,
	Chris Wilson <chris@chris-wilson.co.uk>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>
Cc: "De Marchi, Lucas" <lucas.demarchi@intel.com>,
	"He, Bo" <bo.he@intel.com>
Subject: Re: [PATCH 1/1] drm/i915: move audio component intialization before audio driver use it
Date: Wed, 11 Apr 2018 12:48:47 +0300	[thread overview]
Message-ID: <87woxegjk0.fsf@intel.com> (raw)
In-Reply-To: <E3ED5BF57CCD1C40876FA2D3385ED5D37B618350@SHSMSX101.ccr.corp.intel.com>

On Wed, 11 Apr 2018, "Shi, Yang A" <yang.a.shi@intel.com> wrote:
>>No, that's not true. Just as an example, dev_priv->cdclk.hw.cdclk
>>hasn't been initialized.
> i915_load_modeset_init->intel_power_domains_init_hw->bxt_display_core_init->bxt_init_cdclk
> dev_priv->cdclk.hw.cdclk will be initialized here.
> It happened before driver enter into intel_modeset_init.
> So it should be safe to be used from audio driver.

Was it easy to figure that out? Imagine double checking everything
whenever a) intel_audio.c gets changed, or b) anything in our probe that
might impact intel_audio.c gets changed. It's untenable. Our probe
ordering is what it is to manage the complexity of the driver. When
everything is done, we expose the driver to the rest of the system. Same
with sysfs, debugfs, etc.

>>> Would you like to suggest a better place to do intel_audio_init?
>>
>>I think the call is already where it is supposed to be. We expose
>>ourselves to the rest of the system when we are ready. If it takes
>>long, it takes long. I think you have a race in your driver, and you
>>need to deal with it properly in your driver.
>>
> No, display driver consume too much time to do initialization when it
> don't connect any display Output device.It influence whole audio
> driver and cause whole audio driver can't works well.  This don't make
> sense.

Our probe takes as long as it takes. It's a complicated driver with
complicated probing to do. Why does your design require i915 to be
probed as part of your probe? The component interface supports the
master and client binding in any order, asynchronously.

If you have issues because of the timing, it's probably a race condition
in your driver. Doing the audio init earlier on our side just papers
over your race condition without actually fixing it.

If we were to optimize our probe, it would probably mean more
parallelism and asynchronous probing, and that in turn means even
stricter requirements for finding the root cause instead of just
fiddling with the call site.

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx