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