Re: [PATCH v2] drm/i915: move power domain init earlier during system resume

Re: [PATCH v2] drm/i915: move power domain init earlier during system resume

[Daniel Vetter]

On Tue, Apr 01, 2014 at 09:50:43PM +0300, Imre Deak wrote:
> On Tue, 2014-04-01 at 19:48 +0200, Daniel Vetter wrote:
> > On Tue, Apr 01, 2014 at 07:55:22PM +0300, Imre Deak wrote:
> > > During resume the intel hda audio driver depends on the i915 driver
> > > reinitializing the audio power domain. Since the order of calling the
> > > i915 resume handler wrt. that of the audio driver is not guaranteed,
> > > move the power domain reinitialization step to the resume_early
> > > handler. This is guaranteed to run before the resume handler of any
> > > other driver.
> > >
> > > The power domain initialization in turn requires us to enable the i915
> > > pci device first, so move that part earlier too.
> > >
> > > Accordingly disabling of the i915 pci device should happen after the
> > > audio suspend handler ran. So move the disabling later from the i915
> > > resume handler to the resume_late handler.
> > >
> > > v2:
> > > - move intel_uncore_sanitize/early_sanitize earlier too, so they don't
> > >   get reordered wrt. intel_power_domains_init_hw()
> > >
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76152
> > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> >
> > So this is kinda why we should have gone with something proper, like a new
> > hdmi sink platform device created by i915 and registered as a driver by
> > snd-hda. Then the power domains stuff in the device core should take care
> > of these kinds of ordering issues. Or at least snd-hda can tell it that it
> > needs to wait for the hdmi-sink power domain to go on first before it can
> > resume, I'm not really fluent on the details here.
> >
> > And having a hdmi sink bus would allow us to throw all kinds of crap into
> > a clearly-defined interface, e.g. eld handling, hdcp synchronization, hpd
> > forwarding and all the other fun stuff.
> >
> > So not sure what I should do with this here now.
>
> Right, I'm not too happy about this solution either, so if anything it
> could be considered only a stop-gap fix. What you suggest seems to be a
> cleaner way but it'd require more time to investigate/implement at least
> on my part (but I'm ok to put it on my TODO list).

We'd definitely need to discuss this with Takashi Iwai, since it would
only really be useful if it's good enough to solve the general pile of
sound/gfx coordination issues we have with hdmi. Adding him and alsa-dev.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [Intel-gfx] [PATCH v2] drm/i915: move power domain init earlier during system resume

[Takashi Iwai]

At Tue, 1 Apr 2014 22:26:20 +0200,
Daniel Vetter wrote:
> 
> On Tue, Apr 01, 2014 at 09:50:43PM +0300, Imre Deak wrote:
> > On Tue, 2014-04-01 at 19:48 +0200, Daniel Vetter wrote:
> > > On Tue, Apr 01, 2014 at 07:55:22PM +0300, Imre Deak wrote:
> > > > During resume the intel hda audio driver depends on the i915 driver
> > > > reinitializing the audio power domain. Since the order of calling the
> > > > i915 resume handler wrt. that of the audio driver is not guaranteed,
> > > > move the power domain reinitialization step to the resume_early
> > > > handler. This is guaranteed to run before the resume handler of any
> > > > other driver.
> > > >
> > > > The power domain initialization in turn requires us to enable the i915
> > > > pci device first, so move that part earlier too.
> > > >
> > > > Accordingly disabling of the i915 pci device should happen after the
> > > > audio suspend handler ran. So move the disabling later from the i915
> > > > resume handler to the resume_late handler.
> > > >
> > > > v2:
> > > > - move intel_uncore_sanitize/early_sanitize earlier too, so they don't
> > > >   get reordered wrt. intel_power_domains_init_hw()
> > > >
> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76152
> > > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > >
> > > So this is kinda why we should have gone with something proper, like a new
> > > hdmi sink platform device created by i915 and registered as a driver by
> > > snd-hda. Then the power domains stuff in the device core should take care
> > > of these kinds of ordering issues. Or at least snd-hda can tell it that it
> > > needs to wait for the hdmi-sink power domain to go on first before it can
> > > resume, I'm not really fluent on the details here.
> > >
> > > And having a hdmi sink bus would allow us to throw all kinds of crap into
> > > a clearly-defined interface, e.g. eld handling, hdcp synchronization, hpd
> > > forwarding and all the other fun stuff.
> > >
> > > So not sure what I should do with this here now.
> >
> > Right, I'm not too happy about this solution either, so if anything it
> > could be considered only a stop-gap fix. What you suggest seems to be a
> > cleaner way but it'd require more time to investigate/implement at least
> > on my part (but I'm ok to put it on my TODO list).
> 
> We'd definitely need to discuss this with Takashi Iwai, since it would
> only really be useful if it's good enough to solve the general pile of
> sound/gfx coordination issues we have with hdmi. Adding him and alsa-dev.

Yep, having a dedicated channel (hdmi sink bus or whatever) would be
definitely a better way to go.  OTOH, I agree that Imre's patch is
needed for the upcoming kernel.  The former implementation can't be
finished so quickly for 3.15.

I thought Mengdong started looking at (or working on) this, but
haven't heard the progress yet.  Mengdong, any news?


thanks,

Takashi

Re: [PATCH v2] drm/i915: move power domain init earlier during system resume

[Daniel Vetter]

On Wed, Apr 02, 2014 at 02:59:54PM +0200, Takashi Iwai wrote:
> At Tue, 1 Apr 2014 22:26:20 +0200,
> Daniel Vetter wrote:
> > 
> > On Tue, Apr 01, 2014 at 09:50:43PM +0300, Imre Deak wrote:
> > > On Tue, 2014-04-01 at 19:48 +0200, Daniel Vetter wrote:
> > > > On Tue, Apr 01, 2014 at 07:55:22PM +0300, Imre Deak wrote:
> > > > > During resume the intel hda audio driver depends on the i915 driver
> > > > > reinitializing the audio power domain. Since the order of calling the
> > > > > i915 resume handler wrt. that of the audio driver is not guaranteed,
> > > > > move the power domain reinitialization step to the resume_early
> > > > > handler. This is guaranteed to run before the resume handler of any
> > > > > other driver.
> > > > >
> > > > > The power domain initialization in turn requires us to enable the i915
> > > > > pci device first, so move that part earlier too.
> > > > >
> > > > > Accordingly disabling of the i915 pci device should happen after the
> > > > > audio suspend handler ran. So move the disabling later from the i915
> > > > > resume handler to the resume_late handler.
> > > > >
> > > > > v2:
> > > > > - move intel_uncore_sanitize/early_sanitize earlier too, so they don't
> > > > >   get reordered wrt. intel_power_domains_init_hw()
> > > > >
> > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76152
> > > > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > >
> > > > So this is kinda why we should have gone with something proper, like a new
> > > > hdmi sink platform device created by i915 and registered as a driver by
> > > > snd-hda. Then the power domains stuff in the device core should take care
> > > > of these kinds of ordering issues. Or at least snd-hda can tell it that it
> > > > needs to wait for the hdmi-sink power domain to go on first before it can
> > > > resume, I'm not really fluent on the details here.
> > > >
> > > > And having a hdmi sink bus would allow us to throw all kinds of crap into
> > > > a clearly-defined interface, e.g. eld handling, hdcp synchronization, hpd
> > > > forwarding and all the other fun stuff.
> > > >
> > > > So not sure what I should do with this here now.
> > >
> > > Right, I'm not too happy about this solution either, so if anything it
> > > could be considered only a stop-gap fix. What you suggest seems to be a
> > > cleaner way but it'd require more time to investigate/implement at least
> > > on my part (but I'm ok to put it on my TODO list).
> > 
> > We'd definitely need to discuss this with Takashi Iwai, since it would
> > only really be useful if it's good enough to solve the general pile of
> > sound/gfx coordination issues we have with hdmi. Adding him and alsa-dev.
> 
> Yep, having a dedicated channel (hdmi sink bus or whatever) would be
> definitely a better way to go.  OTOH, I agree that Imre's patch is
> needed for the upcoming kernel.  The former implementation can't be
> finished so quickly for 3.15.
> 
> I thought Mengdong started looking at (or working on) this, but
> haven't heard the progress yet.  Mengdong, any news?

Ok, I'll splash a big honky FIXME over this so we don't forget ;-) Is the
patch otherwise ok with you Takashi?

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH v2] drm/i915: move power domain init earlier during system resume

[Takashi Iwai]

At Thu, 3 Apr 2014 17:23:10 +0200,
Daniel Vetter wrote:
> 
> On Wed, Apr 02, 2014 at 02:59:54PM +0200, Takashi Iwai wrote:
> > At Tue, 1 Apr 2014 22:26:20 +0200,
> > Daniel Vetter wrote:
> > > 
> > > On Tue, Apr 01, 2014 at 09:50:43PM +0300, Imre Deak wrote:
> > > > On Tue, 2014-04-01 at 19:48 +0200, Daniel Vetter wrote:
> > > > > On Tue, Apr 01, 2014 at 07:55:22PM +0300, Imre Deak wrote:
> > > > > > During resume the intel hda audio driver depends on the i915 driver
> > > > > > reinitializing the audio power domain. Since the order of calling the
> > > > > > i915 resume handler wrt. that of the audio driver is not guaranteed,
> > > > > > move the power domain reinitialization step to the resume_early
> > > > > > handler. This is guaranteed to run before the resume handler of any
> > > > > > other driver.
> > > > > >
> > > > > > The power domain initialization in turn requires us to enable the i915
> > > > > > pci device first, so move that part earlier too.
> > > > > >
> > > > > > Accordingly disabling of the i915 pci device should happen after the
> > > > > > audio suspend handler ran. So move the disabling later from the i915
> > > > > > resume handler to the resume_late handler.
> > > > > >
> > > > > > v2:
> > > > > > - move intel_uncore_sanitize/early_sanitize earlier too, so they don't
> > > > > >   get reordered wrt. intel_power_domains_init_hw()
> > > > > >
> > > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76152
> > > > > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > > >
> > > > > So this is kinda why we should have gone with something proper, like a new
> > > > > hdmi sink platform device created by i915 and registered as a driver by
> > > > > snd-hda. Then the power domains stuff in the device core should take care
> > > > > of these kinds of ordering issues. Or at least snd-hda can tell it that it
> > > > > needs to wait for the hdmi-sink power domain to go on first before it can
> > > > > resume, I'm not really fluent on the details here.
> > > > >
> > > > > And having a hdmi sink bus would allow us to throw all kinds of crap into
> > > > > a clearly-defined interface, e.g. eld handling, hdcp synchronization, hpd
> > > > > forwarding and all the other fun stuff.
> > > > >
> > > > > So not sure what I should do with this here now.
> > > >
> > > > Right, I'm not too happy about this solution either, so if anything it
> > > > could be considered only a stop-gap fix. What you suggest seems to be a
> > > > cleaner way but it'd require more time to investigate/implement at least
> > > > on my part (but I'm ok to put it on my TODO list).
> > > 
> > > We'd definitely need to discuss this with Takashi Iwai, since it would
> > > only really be useful if it's good enough to solve the general pile of
> > > sound/gfx coordination issues we have with hdmi. Adding him and alsa-dev.
> > 
> > Yep, having a dedicated channel (hdmi sink bus or whatever) would be
> > definitely a better way to go.  OTOH, I agree that Imre's patch is
> > needed for the upcoming kernel.  The former implementation can't be
> > finished so quickly for 3.15.
> > 
> > I thought Mengdong started looking at (or working on) this, but
> > haven't heard the progress yet.  Mengdong, any news?
> 
> Ok, I'll splash a big honky FIXME over this so we don't forget ;-) Is the
> patch otherwise ok with you Takashi?

Yes, it's touching only i915 stuff in anyway ;)
Feel free to take my ack:
	Reviewed-by: Takashi Iwai <tiwai@suse.de>


thanks,

Takashi

Re: [PATCH v2] drm/i915: move power domain init earlier during system resume

[Daniel Vetter]

On Thu, Apr 03, 2014 at 05:31:17PM +0200, Takashi Iwai wrote:
> At Thu, 3 Apr 2014 17:23:10 +0200,
> Daniel Vetter wrote:
> > 
> > On Wed, Apr 02, 2014 at 02:59:54PM +0200, Takashi Iwai wrote:
> > > At Tue, 1 Apr 2014 22:26:20 +0200,
> > > Daniel Vetter wrote:
> > > > 
> > > > On Tue, Apr 01, 2014 at 09:50:43PM +0300, Imre Deak wrote:
> > > > > On Tue, 2014-04-01 at 19:48 +0200, Daniel Vetter wrote:
> > > > > > On Tue, Apr 01, 2014 at 07:55:22PM +0300, Imre Deak wrote:
> > > > > > > During resume the intel hda audio driver depends on the i915 driver
> > > > > > > reinitializing the audio power domain. Since the order of calling the
> > > > > > > i915 resume handler wrt. that of the audio driver is not guaranteed,
> > > > > > > move the power domain reinitialization step to the resume_early
> > > > > > > handler. This is guaranteed to run before the resume handler of any
> > > > > > > other driver.
> > > > > > >
> > > > > > > The power domain initialization in turn requires us to enable the i915
> > > > > > > pci device first, so move that part earlier too.
> > > > > > >
> > > > > > > Accordingly disabling of the i915 pci device should happen after the
> > > > > > > audio suspend handler ran. So move the disabling later from the i915
> > > > > > > resume handler to the resume_late handler.
> > > > > > >
> > > > > > > v2:
> > > > > > > - move intel_uncore_sanitize/early_sanitize earlier too, so they don't
> > > > > > >   get reordered wrt. intel_power_domains_init_hw()
> > > > > > >
> > > > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76152
> > > > > > > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > > > > >
> > > > > > So this is kinda why we should have gone with something proper, like a new
> > > > > > hdmi sink platform device created by i915 and registered as a driver by
> > > > > > snd-hda. Then the power domains stuff in the device core should take care
> > > > > > of these kinds of ordering issues. Or at least snd-hda can tell it that it
> > > > > > needs to wait for the hdmi-sink power domain to go on first before it can
> > > > > > resume, I'm not really fluent on the details here.
> > > > > >
> > > > > > And having a hdmi sink bus would allow us to throw all kinds of crap into
> > > > > > a clearly-defined interface, e.g. eld handling, hdcp synchronization, hpd
> > > > > > forwarding and all the other fun stuff.
> > > > > >
> > > > > > So not sure what I should do with this here now.
> > > > >
> > > > > Right, I'm not too happy about this solution either, so if anything it
> > > > > could be considered only a stop-gap fix. What you suggest seems to be a
> > > > > cleaner way but it'd require more time to investigate/implement at least
> > > > > on my part (but I'm ok to put it on my TODO list).
> > > > 
> > > > We'd definitely need to discuss this with Takashi Iwai, since it would
> > > > only really be useful if it's good enough to solve the general pile of
> > > > sound/gfx coordination issues we have with hdmi. Adding him and alsa-dev.
> > > 
> > > Yep, having a dedicated channel (hdmi sink bus or whatever) would be
> > > definitely a better way to go.  OTOH, I agree that Imre's patch is
> > > needed for the upcoming kernel.  The former implementation can't be
> > > finished so quickly for 3.15.
> > > 
> > > I thought Mengdong started looking at (or working on) this, but
> > > haven't heard the progress yet.  Mengdong, any news?
> > 
> > Ok, I'll splash a big honky FIXME over this so we don't forget ;-) Is the
> > patch otherwise ok with you Takashi?
> 
> Yes, it's touching only i915 stuff in anyway ;)
> Feel free to take my ack:
> 	Reviewed-by: Takashi Iwai <tiwai@suse.de>

Ok, I've pulled this into 3.15-fixes and added some big comments to the
newly-added late/early hooks. I'll try to poke Mengmeng about the status
of the hdmi infrastructure work.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch