[Intel-gfx] [RFC PATCH v3 2/7] drm/i915: Add support for audio driver notifications

[Pierre-Louis Bossart]

On 11/28/16 11:01 AM, Ville Syrjälä wrote:
>>>>>> +			if (pdata->notify_audio_lpe)
>>>>>> +				pdata->notify_audio_lpe(
>>>>>> +					(eld != NULL) ? &pdata->eld : NULL);
>>>>>> +			else
>>>>>> +				pdata->notify_pending = true;
>>>>> Still not sure why the "pending" thing is useful. Can't the audio
>>>>> driver just do its thing (whatever it is) unconditionally?
>>>>>
>>>> This is added to avoid race when audio driver loads late and the notification
>>> from display has already passed.
>>>
>>> You keep saying that but I can't see it.
>>>
>> I have seen this happen - before audio driver is loaded, codec enable completes and notification is sent to the audio driver.
>> Since the audio callbacks are not initialized, notification gets missed.
> Sure. But what does the extra notification_pending flag buy us? The
> audio driver could just check the eld/tmds_clock/port directly.
>
>>>>> When disabling just clear the port to INVALID, eld to zero, and tmds
>>>>> clock to 0, and it should all be fine no?
>>>>>
>>>> Yes, that's what is being done.
>>> Where?
>>>
>> Notify callback will have eld to NULL and tmds to zero sent in codec_disable
> But the driver can look those thigns up directly as well it seems. So
> this whole thing is a bit of a mess on account of sharing the platform
> as a communication channel and also trying to pass the things as
> paraameters to the notify hook. I think we need to pick one or the other
> approach, not some mismash of both.

Indeed it looks weird to have both a parameter for tmds_clock in the 
pdata AND the notify parameter, this can probably be cleaned-up.

That said, I am not sure I completely understand the feedback that the 
audio driver can get all the eld/tmds/port information directly. We are 
trying to avoid accessing the data structures of the i915 driver. Are 
you suggesting a scheme where the i915 driver would just provide a 
door-bell like notification and the audio driver would use a 
get_eld/tmds/port interface exposed by the i915 driver on startup and 
upon receiving this notification?