On Mon, Nov 28, 2016 at 01:13:31PM -0600, Pierre-Louis Bossart wrote:
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.
IIRC what I proposed originally didn't even expose the same structure to both sides, but that's not what we seem to have atm.
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?
Well, we could do it that way, or we'd do it the other way that the audio driver just calls i915 to triggers a single i915->audio notify call after the audio driver has finished its probe. Or we could do whatever we seem to have now is where the audio driver can just root around directly in the structure (at which point passing any parameters in the notify calls seems redundant as well).