Re: [Intel-gfx] [PATCH] drm/i915/audio: error log non-zero audio power refcount after unbind

[Jani Nikula <jani.nikula@intel.com>]

On Fri, 17 Apr 2020, Kai Vehmanen <kai.vehmanen@linux.intel.com> wrote:
> Hi Jani,
>
> On Fri, 17 Apr 2020, Jani Nikula wrote:
>
>> We have some module unload/reload tests hitting an issue with i915
>> unbinding the component interface before the audio driver has properly
>> put the power. Log an error about it for ease of debugging. (Normally
>
> thanks, this is a good addition:
> Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>

Thanks for the review, pushed to drm-intel-next-queued.

> Maybe one point to consider is whether to take the next step and just 
> block the unload. On audio side, once acomp binding is done to i915 
> driver, it is only released at hda driver unload. So any test case where 
> audio driver is bound to i915, and test unloads i915 without unloading 
> the audio driver first, will not work. Even if no immediate failure is 
> seen at unload, functionality will be impacted after i915 is loaded 
> again.
>
> Not sure how to do this though. Normally module refcounts would take care
> of this (and block i915 unload), but now that we have the component 
> framework in between, something else is needed.

Heh, had I known what to do, I'd have posted that. So I just opted for
logging to make the failure mode obvious.

I admit I didn't check what having an unbalanced get will actually
do. If we go ahead and power down the power well, I assume it'll cripple
the audio driver. I think our refcounting and power well handling should
read the hardware state and set up everything properly on load, but the
audio driver will have lost its marbles in the mean time. Even if by
some chance it doesn't lose its power during i915 unload/load cycle, it
won't have the component interface and can't ensure i915 doesn't go
ahead and cut power later.

BR,
Jani.


>
> PS Audio driver also doesn't implement component unbind(), but I don't
>    immediately see what it could do there. It can't return an error
>    and the audio framework is not really prepared for invidual codec
>    drivers to disappear at runtime. We can handle hotplug of complete 
>    cards (like USB), but individual codec drivers are expected to stay loaded.

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