Re: [PATCH 1/3] drm/i915/hda: Add audio component stub

[Takashi Iwai <tiwai@suse.de>]

On Thu, 07 Apr 2016 14:30:45 +0200,
Imre Deak wrote:
> 
> On to, 2016-04-07 at 14:55 +0300, Ville Syrjälä wrote:
> > On Thu, Apr 07, 2016 at 12:57:22PM +0200, Takashi Iwai wrote:
> > > From: Imre Deak <imre.deak@intel.com>
> > > 
> > > 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.
> > > 
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > > ---
> > >  drivers/gpu/drm/i915/i915_drv.c    | 34 +++++++++++++--------
> > >  drivers/gpu/drm/i915/intel_audio.c | 61
> > > ++++++++++++++++++++++++++++++++++++++
> > >  drivers/gpu/drm/i915/intel_drv.h   |  2 ++
> > >  include/drm/i915_component.h       |  5 ++++
> > >  4 files changed, 90 insertions(+), 12 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.c
> > > b/drivers/gpu/drm/i915/i915_drv.c
> > > index 20e82008b8b6..21c6bac5468e 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > @@ -950,6 +950,19 @@ static int i915_pci_probe(struct pci_dev
> > > *pdev, const struct pci_device_id *ent)
> > >  	struct intel_device_info *intel_info =
> > >  		(struct intel_device_info *) ent->driver_data;
> > >  
> > > +	if (!(driver.driver_features & DRIVER_MODESET)) {
> > > +		/*
> > > +		 * Notify the HDA driver so that it doesn't block
> > > waiting for
> > > +		 * i915 to become ready.
> > > +		 */
> > > +		i915_audio_component_stub_init(&pdev->dev);
> > > +
> > > +		/* Silently fail loading to not upset userspace.
> > > */
> > > +		DRM_DEBUG_DRIVER("KMS and UMS disabled.\n");
> > > +
> > > +		return 0;
> > > +	}
> > > +
> > 
> > Moving this here means we're actually going to bind to the device
> > even
> > w/o DRIVER_MODESET. Hopefully the fact that we don't register with
> > any
> > subsystems would make that OK. But since we need the actual device
> > for
> > the component stuff, I guess this is the only way.
> 
> Yes, the need for the device was the reason I moved this here. An
> alternative would be to add a miscdevice. I wanted to keep things
> simple and didn't see any problem of keeping the device bound, so chose
> this way.
> 
> One other thing I was thinking about is to also bind the stub component
> if the real probing failed to make things more consistent and avoid the
> longish timeout on the HDA side. This would also get messier with the
> miscdevice approach, since the probe error is not propagated to the
> caller of pci_register_driver(). If people don't see a problem with
> this I could add this in the next version.

Is a newer version already available?


thanks,

Takashi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx