From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
To: "Ville Syrjälä" <ville.syrjala@linux.intel.com>,
"Anand, Jerome" <jerome.anand@intel.com>
Cc: "tiwai@suse.de" <tiwai@suse.de>,
"intel-gfx@lists.freedesktop.org"
<intel-gfx@lists.freedesktop.org>,
"broonie@kernel.org" <broonie@kernel.org>,
"Ughreja, Rakesh A" <rakesh.a.ughreja@intel.com>,
"alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>
Subject: Re: [RFC PATCH v3 2/7] drm/i915: Add support for audio driver notifications
Date: Mon, 28 Nov 2016 13:13:31 -0600 [thread overview]
Message-ID: <bd87ae9c-cbde-87f3-8b0d-700df08b2585@linux.intel.com> (raw)
In-Reply-To: <20161128170108.GK31595@intel.com>
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?
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2016-11-28 19:13 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20161124235548.16440-1-jerome.anand@intel.com>
[not found] ` <20161124235548.16440-2-jerome.anand@intel.com>
2016-11-24 13:31 ` [RFC PATCH v3 1/7] drm/i915: setup bridge for HDMI LPE audio driver Ville Syrjälä
2016-11-25 5:42 ` Anand, Jerome
2016-11-25 10:18 ` Ville Syrjälä
2016-11-25 10:52 ` Jani Nikula
2016-11-28 1:50 ` Anand, Jerome
2016-11-28 8:06 ` Jani Nikula
2016-11-27 18:20 ` [alsa-devel] " Pierre-Louis Bossart
2016-11-28 13:43 ` Ville Syrjälä
[not found] ` <20161124235548.16440-3-jerome.anand@intel.com>
2016-11-24 13:32 ` [RFC PATCH v3 2/7] drm/i915: Add support for audio driver notifications Ville Syrjälä
2016-11-25 5:51 ` Anand, Jerome
2016-11-28 14:00 ` Ville Syrjälä
2016-11-28 16:50 ` Anand, Jerome
2016-11-28 17:01 ` Ville Syrjälä
2016-11-28 19:13 ` Pierre-Louis Bossart [this message]
2016-11-28 19:30 ` Ville Syrjälä
2016-11-28 21:56 ` Pierre-Louis Bossart
2016-11-29 8:27 ` Takashi Iwai
2016-11-29 16:22 ` Anand, Jerome
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=bd87ae9c-cbde-87f3-8b0d-700df08b2585@linux.intel.com \
--to=pierre-louis.bossart@linux.intel.com \
--cc=alsa-devel@alsa-project.org \
--cc=broonie@kernel.org \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jerome.anand@intel.com \
--cc=rakesh.a.ughreja@intel.com \
--cc=tiwai@suse.de \
--cc=ville.syrjala@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox