At Mon, 13 May 2013 10:55:46 +0200,
Jaroslav Kysela wrote:
>
> Date 13.5.2013 10:28, David Henningsson wrote:
> > On 05/13/2013 09:37 AM, Wang Xingchao wrote:
> >> I915 module maybe loaded after snd_hda_intel, the power-well
> >> API doesnot exist in such case. This patch intended to avoid
> >> loading dependency between snd-hda-intel and i915 module.
> >
> > Hi Xingchao and thanks for working on this.
> >
> > This patch seems to re-do some of the work done in other patches (a lot
> > of lines removed), so it's a little hard to follow. But I'll try to
> > write some overall comments on how I have envisioned things...
> >
> > 1. I don't think there's any need to create an additional kernel module,
> > we can just let hda_i915.c be in the snd-hda-intel.ko module, and only
> > compile it if DRM_I915 is defined.
> >
> > 2. We don't need an IS_HSW macro on the audio side. Instead declare a
> > new AZX_DCAPS_NEED_I915_POWER (or similar) driver quirk.
> >
> > 3. Somewhere in the beginning of the probing in hda_intel.c, we'll need
> > something like:
> >
> > if (driver_caps & AZX_DCAPS_NEED_I915_POWER)
> > hda_i915_init(chip);
> >
> > 4. The hda_i915_init does not need to be exported (they're now both in
> > the same module). hda_i915.h could have something like:
> >
> > #ifdef DRM_I915
> > void hda_i915_init(chip);
> > #else
> > #define hda_i915_init(chip) do {} while(0)
> > #endif
> >
> > 5. You're saying this patch is about avoid loading dependency between
> > snd-hda-intel and i915 module. That does not make sense to me, since the
> > powerwell API is in the i915 module, snd-hda-intel must load it when it
> > wants to enable power on haswell, right? Thus there is a loading
> > dependency. If the i915 module is not loaded at that point, we must wait
> > for it to load, so we can have proper power, instead of continuing
> > probing without the power well?
>
> Looking to the code, if the drm code requires a callback to the audio
> code, I would just register it in the audio driver init phase and
> unregister it when snd-hda-intel is unloaded. This cross module loading
> dependency looks really bad. Or it is not allowed to load the audio
> driver later than DRM one?
It's not allowed. The drm/i915 must be initialized before the audio.
And yet, we don't want this dependency in a hard way in the hda
driver, because the driver is not only for i915 but for other vendor's
controllers, too.
In the previous meeting, I suggested to split snd-hda-intel for
Haswell as an alternative solution. But this has obvious
disadvantages, and since the dynamic symbol lookup is already used in
a few other kernel codes, we decided to try this first.
Takashi
