Hi Daniel, > -----Original Message----- > From: Daniel Vetter [mailto:daniel.vetter@xxxxxxxx] On Behalf Of > Daniel Vetter > Sent: Wednesday, August 12, 2015 9:43 PM > To: Jani Nikula > Cc: Daniel Vetter; Yang, Libin; alsa-devel@xxxxxxxxxxxxxxxx; > tiwai@xxxxxxx; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; daniel.vetter@xxxxxxxx > Subject: Re: [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@xxxxxxxx> wrote: > > > On Wed, Aug 12, 2015 at 09:20:17AM +0300, Jani Nikula wrote: > > >> On Wed, 12 Aug 2015, "Yang, Libin" <libin.yang@xxxxxxxxx> wrote: > > >> > Hi Jani, > > >> > > > >> >> -----Original Message----- > > >> >> From: Jani Nikula [mailto:jani.nikula@xxxxxxxxxxxxxxx] > > >> >> Sent: Tuesday, August 11, 2015 4:14 PM > > >> >> To: Yang, Libin; alsa-devel@xxxxxxxxxxxxxxxx; tiwai@xxxxxxx; > intel- > > >> >> gfx@xxxxxxxxxxxxxxxxxxxxx; daniel.vetter@xxxxxxxx > > >> >> Subject: RE: [PATCH v4 4/4] drm/i915: set proper > N/CTS in > > >> >> modeset > > >> >> > > >> >> On Tue, 11 Aug 2015, "Yang, Libin" <libin.yang@xxxxxxxxx> > wrote: > > >> >> > Hi Jani, > > >> >> > > > >> >> >> -----Original Message----- > > >> >> >> From: Jani Nikula [mailto:jani.nikula@xxxxxxxxxxxxxxx] > > >> >> >> Sent: Tuesday, August 11, 2015 3:14 PM > > >> >> >> To: Yang, Libin; alsa-devel@xxxxxxxxxxxxxxxx; tiwai@xxxxxxx; > intel- > > >> >> >> gfx@xxxxxxxxxxxxxxxxxxxxx; daniel.vetter@xxxxxxxx > > >> >> >> Subject: RE: [PATCH v4 4/4] drm/i915: set proper > N/CTS in > > >> >> >> modeset > > >> >> >> > > >> >> >> On Tue, 11 Aug 2015, "Yang, Libin" <libin.yang@xxxxxxxxx> > wrote: > > >> >> >> > Hi Jani, > > >> >> >> > > > >> >> >> > Thanks for careful reviewing for all the patches, please > > >> >> >> > see my comments. > > >> >> >> > > > >> >> >> >> -----Original Message----- > > >> >> >> >> From: Jani Nikula [mailto:jani.nikula@xxxxxxxxxxxxxxx] > > >> >> >> >> Sent: Monday, August 10, 2015 8:10 PM > > >> >> >> >> To: Yang, Libin; alsa-devel@xxxxxxxxxxxxxxxx; > tiwai@xxxxxxx; intel- > > >> >> >> >> gfx@xxxxxxxxxxxxxxxxxxxxx; daniel.vetter@xxxxxxxx > > >> >> >> >> Subject: Re: [PATCH v4 4/4] drm/i915: set > proper N/CTS > > >> >> in > > >> >> >> >> modeset > > >> >> >> >> > > >> >> >> >> On Mon, 10 Aug 2015, libin.yang@xxxxxxxxx wrote: > > >> >> >> >> > From: Libin Yang <libin.yang@xxxxxxxxx> > > >> >> >> >> > > > >> >> >> >> > 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
