[Intel-gfx] [PATCH v2 1/3] drm/i915/hda: Add audio component stub

[Takashi Iwai]

On Tue, 17 May 2016 11:42:17 +0200,
Daniel Vetter wrote:
> 
> On Tue, May 17, 2016 at 09:37:12AM +0200, Takashi Iwai wrote:
> > On Tue, 17 May 2016 09:20:48 +0200,
> > Daniel Vetter wrote:
> > > 
> > > On Thu, May 12, 2016 at 09:06:53PM +0300, Imre Deak wrote:
> > > > User may pass nomodeset or i915.modeset=0 option to disable i915 KMS
> > > > explicitly.  Although this itself works fine, it breaks the weak
> > > > dependency the HD-audio driver requires, and it's the reason the
> > > > delayed component binding isn't implemented in HD-audio.  Since i915
> > > > doesn't notify its disablement, HD-audio would be blocked
> > > > unnecessarily endlessly, waiting for the bind with i915.
> > > > 
> > > > This patch introduces a stub audio component binding when i915 driver
> > > > is loaded with KMS off like the boot options above.  Then i915 driver
> > > > still registers the slave component but with the new "disabled" ops
> > > > flag, so that the master component (HD-audio) can know clearly the
> > > > slave state.
> > > > 
> > > > v2:
> > > > - Fail the probe in case component registration fails, instead of
> > > >   suppressing the error. (Ville)
> > > > - Register the component only for the real PCI device function.
> > > > 
> > > > CC: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > > Signed-off-by: Imre Deak <imre.deak at intel.com>
> > > > Signed-off-by: Takashi Iwai <tiwai at suse.de>
> > > 
> > > We don't support not running with modesetting. Why do we suddenly care?
> > 
> > This is needed for the patch 2 and 3.  Right now we have no blocking
> > or deferred component binding, so far, in HD-audio side.  This caused
> > problems when async module probe was done.
> > 
> > So, the patch 3 implements the blocking behavior of HD-audio side.  It
> > would lead to another regression when i915 doesn't notify its disabled
> > state by this patch.  Otherwise the HD-audio driver will be blocked
> > endlessly of unnecessarily long.
> > 
> > > Same for users creating a .config that fails to boot or work ...
> > 
> > The config isn't cared much, but the problem is about the runtime boot
> > option.
> > 
> > > If HDA needs to coporate with gfx to get things done, then imo we should
> > > just require that i915.ko is there.
> > 
> > Other way round: we do already require i915 in HD-audio side.  But in
> > this case, we do *not* want to require i915 when it's disabled in
> > runtime. 
> 
> That's what I mean: If you boot with i915.nomodeset you're explicitly fine
> with a somewhat non-useable system - that option is for debugging only
> really. If that means audio also doesn't work, then I think that's ok.

It's not only "it doesn't work".  The module load gets stuck.  So we
need some notification for the blocked component binding.

> Adding complexity for this case (which means more error paths we don't
> ever test and hence _will_ break) seems over the top.
> 
> I'm quite opposed to adding error handling for every condition in general
> because the combinatorial testing madness just can't be handled. The one
> exception in the i915.ko driver is that when the render side died
> (terminal gpu hang) we'll try our best to keep the display alive. But
> that's it, and the justification for that is "we want users to be able to
> get the bug report out". I don't see a justification of that magnitude for
> this feature here at all.

Well, actually the patchset was proposed just because Intel CI tests
failed due to async module probes.  If Intel is happy with continued
CI test failures, I'm also happy with the current situation ;)


Takashi