[alsa-devel] [Intel-gfx] [PATCH 2/7] drm/i915: Add get_eld audio component

[Takashi Iwai]

On Mon, 30 Nov 2015 17:09:33 +0100,
Ville Syrjälä wrote:
> 
> On Mon, Nov 30, 2015 at 05:24:41PM +0200, Ville Syrjälä wrote:
> > On Mon, Nov 30, 2015 at 02:37:46PM +0100, Takashi Iwai wrote:
> > > Implement a new i915_audio_component_ops, get_eld().  It's called by
> > > the audio driver to fetch the current ELD of the given HDMI/DP port.
> > > It returns the size of ELD bytes if it's valid, or zero if the audio
> > > is disabled or unplugged, or a negative error code.
> > 
> > Why do we need this? Isn't it something the eld notify hook should
> > pass from i915 to the audio driver?
> > 
> > At least with the locking you have for this, the audio driver can not
> > call this from the eld notify hook since it would deadlock.
> 
> Hmm. Actually the locking isn't perhaps quite like that atm. But I guess
> the mode_config.mutex will make it so.
> 
> Apart from that it seesm to me that you should pull the av_mutex
> lock/unlock from the .audio_code_eanble/disable hooks into
> intel_audio_codec_enable/disable, so that it protects the audio_enabled
> flag as well. Not sure if the eld_notify should be called while holding
> that lock or not. If we need to avoid calling it from the eld_notify
> anywya due to other locks then maybe it can be under av_mutex as well.

Currently I'm thinking of:

- not allow to call get_eld directly from eld_notify;
  I found that the existing eld_repoll work in the HDA can be reused
  easily, so let's follow Daniel's advice.

- drm_select_eld() seems requring mode_config.mutex and
  connection_mutex modeset lock in anyway; so let get_eld taking
  both.

  Is it OK to call drm_modeset_lock_all() for simplicity?

- Get rid of av_mutex from get_eld instead;
  get_eld doesn't conflict with other ops

In that way, audio_enabled flag is protected in both places via
modeset lock, I suppose.


thanks,

Takashi