Hi Daniel,
-----Original Message----- From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter Sent: Wednesday, August 12, 2015 9:43 PM To: Jani Nikula Cc: Daniel Vetter; Yang, Libin; alsa-devel@alsa-project.org; tiwai@suse.de; intel-gfx@lists.freedesktop.org; daniel.vetter@ffwll.ch Subject: Re: [Intel-gfx] [PATCH v4 4/4] drm/i915: set proper N/CTS in modeset
On Wed, Aug 12, 2015 at 04:23:12PM +0300, Jani Nikula wrote:
On Wed, 12 Aug 2015, Daniel Vetter daniel@ffwll.ch wrote:
On Wed, Aug 12, 2015 at 09:20:17AM +0300, Jani Nikula wrote:
On Wed, 12 Aug 2015, "Yang, Libin" libin.yang@intel.com wrote:
Hi Jani,
-----Original Message----- From: Jani Nikula [mailto:jani.nikula@linux.intel.com] Sent: Tuesday, August 11, 2015 4:14 PM To: Yang, Libin; alsa-devel@alsa-project.org; tiwai@suse.de;
intel-
gfx@lists.freedesktop.org; daniel.vetter@ffwll.ch Subject: RE: [Intel-gfx] [PATCH v4 4/4] drm/i915: set proper
N/CTS in
modeset
On Tue, 11 Aug 2015, "Yang, Libin" libin.yang@intel.com
wrote:
> Hi Jani, > >> -----Original Message----- >> From: Jani Nikula [mailto:jani.nikula@linux.intel.com] >> Sent: Tuesday, August 11, 2015 3:14 PM >> To: Yang, Libin; alsa-devel@alsa-project.org; tiwai@suse.de;
intel-
>> gfx@lists.freedesktop.org; daniel.vetter@ffwll.ch >> Subject: RE: [Intel-gfx] [PATCH v4 4/4] drm/i915: set proper
N/CTS in
>> modeset >> >> On Tue, 11 Aug 2015, "Yang, Libin" libin.yang@intel.com
wrote:
>> > Hi Jani, >> > >> > Thanks for careful reviewing for all the patches, please >> > see my comments. >> > >> >> -----Original Message----- >> >> From: Jani Nikula [mailto:jani.nikula@linux.intel.com] >> >> Sent: Monday, August 10, 2015 8:10 PM >> >> To: Yang, Libin; alsa-devel@alsa-project.org;
tiwai@suse.de; intel-
>> >> gfx@lists.freedesktop.org; daniel.vetter@ffwll.ch >> >> Subject: Re: [Intel-gfx] [PATCH v4 4/4] drm/i915: set
proper N/CTS
in >> >> modeset >> >> >> >> On Mon, 10 Aug 2015, libin.yang@intel.com wrote: >> >> > From: Libin Yang libin.yang@intel.com >> >> > >> >> > When modeset occurs and the TMDS frequency is set
to some
>> >> > speical value, the N/CTS need to be set manually if
audio
>> >> > is playing. >> >> >> >> When modeset occurs, we disable everything, and then
re-enable
>> >> everything, and notify the audio driver every step of the
way. IIUC
>> >> there should be no audio playing across the modeset,
and when
the >> >> modeset is complete and we've set the valid ELD, the
audio driver
>> >> should >> >> call the callback from the earlier patches to set the
correct audio
>> >> rate. >> >> >> >> Why is this patch needed? Please enlighten me. >> > >> > Please image this scenario: when audio is playing, the gfx
driver
>> > does the modeset. In this situation, after modeset, audio
driver
>> > will do nothing and continue playing. And audio driver will
not call
>> > set_ncts. >> >> Why does the audio driver not do anything here? Whenever
there's
a >> modeset, the graphics driver will disable audio and
invalidate the
ELD, >> which should cause an interrupt to notify the audio driver
about
>> this. This is no different from a hot-unplug, in fact I can't see
how
>> the audio driver could even detect whether there's been a
hotplug
or >> not >> when there's a codec disable/enable. > > Yes, it will trigger an interrupt to audio driver when unplug > and another interrupt for hotplug. But from audio driver, > we will not stop the playing and resume the playing based > on the hotplug or modeset. The audio is always playing
during
> the hotplug/modeset.
But the whole display, and consequently ELD, might have
changed
during the hotplug, why do you assume you can just keep playing?!
The
display might even have changed from HDMI to DP or vice versa.
The eld info changing will not impact the audio playing. Actually, you can image the monitor as a digital speaker, just like the headphone to the analog audio. Unplug the speaker will not impact on the audio playing. Of course , there is a little difference, when monitor hotplug, in the interrupt, we may change the codec configuration dynamically.
And audio driver supply the mechanism to stop the audio playing when hotplug if the HW requires.
We've also been putting a lot of effort into not depending on
previous
state when enabling the outputs, for more reliability. We
generally
don't do partial changes, we disable everything and then
enable
again. And now you're suggesting we look at some register
which for all
we know may have been valid for some other display?
In the patch, it will not depend on previous state, I think. We read the register value which is based on the state after
modeset.
Okay, let's have the patches cleaned up and see. It'll be easier and quicker to solicit more review on that than this expanding thread.
It sounds like we actually need to ping the audio side _before_ we
do the
Do you mean "read _HSW_AUD_STR_DESC_*" by "ping the audio
side" above?
No, I mean call into snd-hda-intel before we do the modeset to ask it for what it actually wants to have set up. At least it feels a bit like only reacting to a modeset after it's done leads to confusion. Or maybe we just need to update the register values we want at compute_time instead of doing rwm fun ...
Actually we read the _HSW_AUD_STR_DESC_* register to get what value we are going to set. This is similar with calling audio driver to ask for the parameters. The _HSW_AUD_STR_DESC_* will tell us what audio sample rates it's using. :-)
Regards, Libin
Pure gut feeling comment, I didn't look into details ;-)
-Daniel
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch