On 11/28/16 1:30 PM, Ville Syrjälä wrote:
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).
Looking at some older emails, i think you recommended a 'register'
callback to let the audio driver signal to the i915 side it completed
its initialization, with a single notify generated if needed (what you
described just above as 'the other way')
If you look at the path 4 of the series, Jerome was trying to implement
this with a 'notify_pending' field in the platform data set by the i915
side and used during the audio driver probe
+ if (pdata->notify_pending) {
+
+ pr_debug("%s: handle pending notification\n", __func__);
+ notify_audio_lpe(&pdata->eld);
+ pdata->notify_pending = false;
+ }
Maybe an explicit handshake is more self-explanatory and safer?
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
