| Message ID | 1379661263-5831-1-git-send-email-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2013/9/20 <ville.syrjala@linux.intel.com>: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > VGA registers live inside the power well on HSW, so in order to write > the VGA MSR register we need the power well to be on. > > We really must write to the register to properly clear the > VGA_MSR_MEM_EN enable bit, even if all VGA registers get zeroed when > the power well is down. It seems that the implicit zeroing done by > the power well is not enough to propagate the VGA_MSR_MEM_EN bit to > whomever is actually responsible for the memory decode ranges. > > If we leave VGA memory decode enabled, and then turn off the power well, > all VGA memory reads will return zeroes. But if we first disable VGA > memory deocde and then turn off the power well, VGA memory reads > return all ones, indicating that the access wasn't claimed by anyone. > For the vga arbiter to function correctly the IGD must not claim the > VGA memory accesses. > > Previously we were doing the VGA_MSR register access while the power well > was excplicitly powered up during driver init. But ever since > > commit 6e1b4fdad5157bb9e88777d525704aba24389bee > Author: Ville Syrjälä <ville.syrjala@linux.intel.com> > Date: Thu Sep 5 20:40:52 2013 +0300 > > drm/i915: Delay disabling of VGA memory until vgacon->fbcon handoff is done > > we delay the VGA memory disable until fbcon has initialized, and so > there's a possibility that the power well got turned off during the > fbcon modeset. Also vgacon_save_screen() will need the power well to be > on to be able to read the VGA memory. > > So immediately after enabling the power well during init grab a refence > for VGA purposes, and after all the VGA handling is done, release it. > > v2: Add intel_display_power_put() for the num_pipes==0 case Even though the patch was applied, the issue didn't go away. It looks like the conflict-merging went wrong. Look: http://cgit.freedesktop.org/~danvet/drm-intel/commit/?h=drm-intel-next-queued&id=971b0597b3a13576e1af1e0a90379aa308f498e5 The new _put function is not after i915_disable_vga_mem(). > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_dma.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > index e5c7b10..956d143 100644 > --- a/drivers/gpu/drm/i915/i915_dma.c > +++ b/drivers/gpu/drm/i915/i915_dma.c > @@ -1326,13 +1326,18 @@ static int i915_load_modeset_init(struct drm_device *dev) > > intel_init_power_well(dev); > > + /* Keep VGA alive until i915_disable_vga_mem() */ > + intel_display_power_get(dev, POWER_DOMAIN_VGA); > + > intel_modeset_gem_init(dev); > > /* Always safe in the mode setting case. */ > /* FIXME: do pre/post-mode set stuff in core KMS code */ > dev->vblank_disable_allowed = 1; > - if (INTEL_INFO(dev)->num_pipes == 0) > + if (INTEL_INFO(dev)->num_pipes == 0) { > + intel_display_power_put(dev, POWER_DOMAIN_VGA); > return 0; > + } > > ret = intel_fbdev_init(dev); > if (ret) > @@ -1358,6 +1363,7 @@ static int i915_load_modeset_init(struct drm_device *dev) > * vgacon_save_screen() works during the handover. > */ > i915_disable_vga_mem(dev); > + intel_display_power_put(dev, POWER_DOMAIN_VGA); > > /* Only enable hotplug handling once the fbdev is fully set up. */ > dev_priv->enable_hotplug_processing = true; > -- > 1.8.1.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, Sep 23, 2013 at 01:48:42PM -0300, Paulo Zanoni wrote: > 2013/9/20 <ville.syrjala@linux.intel.com>: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > VGA registers live inside the power well on HSW, so in order to write > > the VGA MSR register we need the power well to be on. > > > > We really must write to the register to properly clear the > > VGA_MSR_MEM_EN enable bit, even if all VGA registers get zeroed when > > the power well is down. It seems that the implicit zeroing done by > > the power well is not enough to propagate the VGA_MSR_MEM_EN bit to > > whomever is actually responsible for the memory decode ranges. > > > > If we leave VGA memory decode enabled, and then turn off the power well, > > all VGA memory reads will return zeroes. But if we first disable VGA > > memory deocde and then turn off the power well, VGA memory reads > > return all ones, indicating that the access wasn't claimed by anyone. > > For the vga arbiter to function correctly the IGD must not claim the > > VGA memory accesses. > > > > Previously we were doing the VGA_MSR register access while the power well > > was excplicitly powered up during driver init. But ever since > > > > commit 6e1b4fdad5157bb9e88777d525704aba24389bee > > Author: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Date: Thu Sep 5 20:40:52 2013 +0300 > > > > drm/i915: Delay disabling of VGA memory until vgacon->fbcon handoff is done > > > > we delay the VGA memory disable until fbcon has initialized, and so > > there's a possibility that the power well got turned off during the > > fbcon modeset. Also vgacon_save_screen() will need the power well to be > > on to be able to read the VGA memory. > > > > So immediately after enabling the power well during init grab a refence > > for VGA purposes, and after all the VGA handling is done, release it. > > > > v2: Add intel_display_power_put() for the num_pipes==0 case > > Even though the patch was applied, the issue didn't go away. It looks > like the conflict-merging went wrong. Look: > http://cgit.freedesktop.org/~danvet/drm-intel/commit/?h=drm-intel-next-queued&id=971b0597b3a13576e1af1e0a90379aa308f498e5 > > The new _put function is not after i915_disable_vga_mem(). Oops, so much for trusting the tools too much ;-) Should be fixed now, thanks for catching this. -Daniel > > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > --- > > drivers/gpu/drm/i915/i915_dma.c | 8 +++++++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > > index e5c7b10..956d143 100644 > > --- a/drivers/gpu/drm/i915/i915_dma.c > > +++ b/drivers/gpu/drm/i915/i915_dma.c > > @@ -1326,13 +1326,18 @@ static int i915_load_modeset_init(struct drm_device *dev) > > > > intel_init_power_well(dev); > > > > + /* Keep VGA alive until i915_disable_vga_mem() */ > > + intel_display_power_get(dev, POWER_DOMAIN_VGA); > > + > > intel_modeset_gem_init(dev); > > > > /* Always safe in the mode setting case. */ > > /* FIXME: do pre/post-mode set stuff in core KMS code */ > > dev->vblank_disable_allowed = 1; > > - if (INTEL_INFO(dev)->num_pipes == 0) > > + if (INTEL_INFO(dev)->num_pipes == 0) { > > + intel_display_power_put(dev, POWER_DOMAIN_VGA); > > return 0; > > + } > > > > ret = intel_fbdev_init(dev); > > if (ret) > > @@ -1358,6 +1363,7 @@ static int i915_load_modeset_init(struct drm_device *dev) > > * vgacon_save_screen() works during the handover. > > */ > > i915_disable_vga_mem(dev); > > + intel_display_power_put(dev, POWER_DOMAIN_VGA); > > > > /* Only enable hotplug handling once the fbdev is fully set up. */ > > dev_priv->enable_hotplug_processing = true; > > -- > > 1.8.1.5 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > Paulo Zanoni > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index e5c7b10..956d143 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1326,13 +1326,18 @@ static int i915_load_modeset_init(struct drm_device *dev) intel_init_power_well(dev); + /* Keep VGA alive until i915_disable_vga_mem() */ + intel_display_power_get(dev, POWER_DOMAIN_VGA); + intel_modeset_gem_init(dev); /* Always safe in the mode setting case. */ /* FIXME: do pre/post-mode set stuff in core KMS code */ dev->vblank_disable_allowed = 1; - if (INTEL_INFO(dev)->num_pipes == 0) + if (INTEL_INFO(dev)->num_pipes == 0) { + intel_display_power_put(dev, POWER_DOMAIN_VGA); return 0; + } ret = intel_fbdev_init(dev); if (ret) @@ -1358,6 +1363,7 @@ static int i915_load_modeset_init(struct drm_device *dev) * vgacon_save_screen() works during the handover. */ i915_disable_vga_mem(dev); + intel_display_power_put(dev, POWER_DOMAIN_VGA); /* Only enable hotplug handling once the fbdev is fully set up. */ dev_priv->enable_hotplug_processing = true;