From: "Ville Syrjälä" <ville.syrjala@linux.intel.com> To: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: DRI Development <dri-devel@lists.freedesktop.org>, Intel Graphics Development <intel-gfx@lists.freedesktop.org>, Patrik Jakobsson <patrik.jakobsson@linux.intel.com>, Imre Deak <imre.deak@intel.com>, Jani Nikula <jani.nikula@intel.com>, Meelis Roos <mroos@linux.ee>, Chris Wilson <chris@chris-wilson.co.uk>, stable <stable@vger.kernel.org>, Daniel Vetter <daniel.vetter@intel.com> Subject: Re: [PATCH] drm/i915: Init power domains early in driver load Date: Thu, 7 Jan 2016 15:26:14 +0200 [thread overview] Message-ID: <20160107132614.GC4437@intel.com> (raw) In-Reply-To: <CAKMK7uE0a-Fh95Z_HODwd3sG-7MnWa4Ejfhis64x0tGyFRXb9g@mail.gmail.com> On Thu, Jan 07, 2016 at 02:15:50PM +0100, Daniel Vetter wrote: > On Thu, Jan 7, 2016 at 1:52 PM, Ville Syrj�l� > <ville.syrjala@linux.intel.com> wrote: > > On Thu, Jan 07, 2016 at 12:44:21PM +0100, Daniel Vetter wrote: > >> Since > >> > >> commit ac9b8236551d1177fd07b56aef9b565d1864420d > >> Author: Ville Syrj�l� <ville.syrjala@linux.intel.com> > >> Date: Fri Nov 27 18:55:26 2015 +0200 > >> > >> drm/i915: Introduce a gmbus power domain > >> > >> gmbus also needs the power domain infrastructure right from the start, > >> since as soon as we register the i2c controllers someone can use them. > >> > >> v2: Adjust cleanup paths too (Chris). > >> > >> Cc: Ville Syrj�l� <ville.syrjala@linux.intel.com> > >> Cc: Patrik Jakobsson <patrik.jakobsson@linux.intel.com> > >> Cc: Imre Deak <imre.deak@intel.com> > >> Cc: Jani Nikula <jani.nikula@intel.com> > >> Cc: Meelis Roos <mroos@linux.ee> > >> Cc: Chris Wilson <chris@chris-wilson.co.uk> > >> Fixes: ac9b8236551d ("drm/i915: Introduce a gmbus power domain") > >> Cc: stable@vger.kernel.org > >> References: http://www.spinics.net/lists/intel-gfx/msg83075.html > >> Tested-by: Meelis Roos <mroos@linux.ee> > >> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > >> --- > >> drivers/gpu/drm/i915/i915_dma.c | 11 +++++------ > >> 1 file changed, 5 insertions(+), 6 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > >> index 988a3806512a..490d8b0d931e 100644 > >> --- a/drivers/gpu/drm/i915/i915_dma.c > >> +++ b/drivers/gpu/drm/i915/i915_dma.c > >> @@ -398,7 +398,6 @@ static int i915_load_modeset_init(struct drm_device *dev) > >> if (ret) > >> goto cleanup_vga_switcheroo; > >> > >> - intel_power_domains_init_hw(dev_priv, false); > >> > >> intel_csr_ucode_init(dev_priv); > >> > >> @@ -1025,6 +1024,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) > >> > >> intel_irq_init(dev_priv); > >> intel_uncore_sanitize(dev); > >> + intel_power_domains_init(dev_priv); > >> + intel_power_domains_init_hw(dev_priv); > > > > I think intel_init_dpio() needs to be moved too. We need to know the > > DPIO IOSF ports before attempting to talk to the PHY (which can happen > > from intel_power_domains_init_hw()). > > Ugh, will change. I think I placed the dpio init in the current place so that it sits next to intel_device_info_runtime_init(). Doing a lot of hw bashing before all this runtime device info stuff has been set up seems rather wrong to me. > > > I'm also wondering why we're doing gmbus init this early. We shouldn't > > need it until modeset init. > > Anyone can access the gmbus controller once we register it. Userspace > can (like what seems to happen on Meelis' box), but also the i2c core > has some auto-probed stuff in some configs afaik. Sure, but I don't see any reason why we'd need to init it that early. The only requirement is that we need to init before we ourselves use it, which I think means we don't actually need it until output setup. And gmbus being a component of the display engine means the init should really be part of the modeset init. So I tend to think the better fix would be to move gmbus init to happen later. -- Ville Syrj�l� Intel OTC
WARNING: multiple messages have this Message-ID (diff)
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com> To: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: DRI Development <dri-devel@lists.freedesktop.org>, Intel Graphics Development <intel-gfx@lists.freedesktop.org>, Patrik Jakobsson <patrik.jakobsson@linux.intel.com>, Imre Deak <imre.deak@intel.com>, Jani Nikula <jani.nikula@intel.com>, Meelis Roos <mroos@linux.ee>, Chris Wilson <chris@chris-wilson.co.uk>, stable <stable@vger.kernel.org>, Daniel Vetter <daniel.vetter@intel.com> Subject: Re: [PATCH] drm/i915: Init power domains early in driver load Date: Thu, 7 Jan 2016 15:26:14 +0200 [thread overview] Message-ID: <20160107132614.GC4437@intel.com> (raw) In-Reply-To: <CAKMK7uE0a-Fh95Z_HODwd3sG-7MnWa4Ejfhis64x0tGyFRXb9g@mail.gmail.com> On Thu, Jan 07, 2016 at 02:15:50PM +0100, Daniel Vetter wrote: > On Thu, Jan 7, 2016 at 1:52 PM, Ville Syrjälä > <ville.syrjala@linux.intel.com> wrote: > > On Thu, Jan 07, 2016 at 12:44:21PM +0100, Daniel Vetter wrote: > >> Since > >> > >> commit ac9b8236551d1177fd07b56aef9b565d1864420d > >> Author: Ville Syrjälä <ville.syrjala@linux.intel.com> > >> Date: Fri Nov 27 18:55:26 2015 +0200 > >> > >> drm/i915: Introduce a gmbus power domain > >> > >> gmbus also needs the power domain infrastructure right from the start, > >> since as soon as we register the i2c controllers someone can use them. > >> > >> v2: Adjust cleanup paths too (Chris). > >> > >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > >> Cc: Patrik Jakobsson <patrik.jakobsson@linux.intel.com> > >> Cc: Imre Deak <imre.deak@intel.com> > >> Cc: Jani Nikula <jani.nikula@intel.com> > >> Cc: Meelis Roos <mroos@linux.ee> > >> Cc: Chris Wilson <chris@chris-wilson.co.uk> > >> Fixes: ac9b8236551d ("drm/i915: Introduce a gmbus power domain") > >> Cc: stable@vger.kernel.org > >> References: http://www.spinics.net/lists/intel-gfx/msg83075.html > >> Tested-by: Meelis Roos <mroos@linux.ee> > >> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > >> --- > >> drivers/gpu/drm/i915/i915_dma.c | 11 +++++------ > >> 1 file changed, 5 insertions(+), 6 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > >> index 988a3806512a..490d8b0d931e 100644 > >> --- a/drivers/gpu/drm/i915/i915_dma.c > >> +++ b/drivers/gpu/drm/i915/i915_dma.c > >> @@ -398,7 +398,6 @@ static int i915_load_modeset_init(struct drm_device *dev) > >> if (ret) > >> goto cleanup_vga_switcheroo; > >> > >> - intel_power_domains_init_hw(dev_priv, false); > >> > >> intel_csr_ucode_init(dev_priv); > >> > >> @@ -1025,6 +1024,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) > >> > >> intel_irq_init(dev_priv); > >> intel_uncore_sanitize(dev); > >> + intel_power_domains_init(dev_priv); > >> + intel_power_domains_init_hw(dev_priv); > > > > I think intel_init_dpio() needs to be moved too. We need to know the > > DPIO IOSF ports before attempting to talk to the PHY (which can happen > > from intel_power_domains_init_hw()). > > Ugh, will change. I think I placed the dpio init in the current place so that it sits next to intel_device_info_runtime_init(). Doing a lot of hw bashing before all this runtime device info stuff has been set up seems rather wrong to me. > > > I'm also wondering why we're doing gmbus init this early. We shouldn't > > need it until modeset init. > > Anyone can access the gmbus controller once we register it. Userspace > can (like what seems to happen on Meelis' box), but also the i2c core > has some auto-probed stuff in some configs afaik. Sure, but I don't see any reason why we'd need to init it that early. The only requirement is that we need to init before we ourselves use it, which I think means we don't actually need it until output setup. And gmbus being a component of the display engine means the init should really be part of the modeset init. So I tend to think the better fix would be to move gmbus init to happen later. -- Ville Syrjälä Intel OTC
next prev parent reply other threads:[~2016-01-07 13:26 UTC|newest] Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-01-07 9:10 [PATCH] drm/i915: Init power domains early in driver load Daniel Vetter 2016-01-07 9:10 ` Daniel Vetter 2016-01-07 9:22 ` [Intel-gfx] " Chris Wilson 2016-01-07 9:22 ` Chris Wilson 2016-01-07 10:23 ` Meelis Roos 2016-01-07 10:23 ` Meelis Roos 2016-01-07 11:44 ` Daniel Vetter 2016-01-07 11:52 ` Chris Wilson 2016-01-07 11:52 ` Chris Wilson 2016-01-07 11:52 ` [Intel-gfx] " kbuild test robot 2016-01-07 11:52 ` kbuild test robot 2016-01-07 12:14 ` Jani Nikula 2016-01-07 12:14 ` Jani Nikula 2016-01-07 12:52 ` Ville Syrjälä 2016-01-07 12:52 ` Ville Syrjälä 2016-01-07 13:15 ` Daniel Vetter 2016-01-07 13:25 ` Jani Nikula 2016-01-07 13:25 ` Jani Nikula 2016-01-07 13:26 ` Ville Syrjälä [this message] 2016-01-07 13:26 ` Ville Syrjälä 2016-01-07 13:16 ` Daniel Vetter 2016-01-07 13:51 ` Daniel Vetter 2016-01-07 14:32 ` Ville Syrjälä 2016-01-07 14:32 ` Ville Syrjälä 2016-01-07 15:49 ` Meelis Roos 2016-01-07 19:41 ` Meelis Roos 2016-01-07 19:41 ` Meelis Roos 2016-01-11 9:12 ` ✗ failure: Fi.CI.BAT Patchwork 2016-01-13 10:55 [PATCH] drm/i915: Init power domains early in driver load Daniel Vetter
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=20160107132614.GC4437@intel.com \ --to=ville.syrjala@linux.intel.com \ --cc=chris@chris-wilson.co.uk \ --cc=daniel.vetter@ffwll.ch \ --cc=daniel.vetter@intel.com \ --cc=dri-devel@lists.freedesktop.org \ --cc=imre.deak@intel.com \ --cc=intel-gfx@lists.freedesktop.org \ --cc=jani.nikula@intel.com \ --cc=mroos@linux.ee \ --cc=patrik.jakobsson@linux.intel.com \ --cc=stable@vger.kernel.org \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.