Message ID | alpine.DEB.2.00.1009011649150.339@hungry |
---|---|
State | Rejected |
Delegated to: | Leann Ogasawara |
Headers | show |
Hi Manoj, On Wed, 2010-09-01 at 16:54 -0500, Manoj Iyer wrote: > This patch fixes an issue where dell latitude E4310 (and I suspect E6410) > freezes when external monitor is attached and monitor is switched though > app or hot-key. Please pull and apply to maverick. > > The following changes since commit > 9f02aef9921f0f213a6680768641eb2af6113c3b: > Leann Ogasawara (1): > UBUNTU: Start new release > > are available in the git repository at: > > > git://kernel.ubuntu.com/git/manjo/ubuntu-maverick.git lp602281 > > Jesse Barnes (1): > drm/i915: fix VGA plane disable for Ironlake+ Examining this patch, it appears to actually be upstream: commit 9cce37f4855a30cc7c364edf18522282782f7ddc Author: Jesse Barnes <jbarnes@virtuousgeek.org> Date: Fri Aug 13 15:11:26 2010 -0700 drm/i915: fix VGA plane disable for Ironlake+ However, I noticed a discrepancy between the you've provided and the upstream version of the patch (see below). > drivers/gpu/drm/i915/intel_display.c | 57 > ++++++++++++++++++---------------- > 1 files changed, 30 insertions(+), 27 deletions(-) > > > From 072f81737105c2b7b15f37c7dd2566a84297c693 Mon Sep 17 00:00:00 2001 > From: Jesse Barnes <jbarnes@virtuousgeek.org> > Date: Fri, 13 Aug 2010 15:11:26 -0700 > Subject: [PATCH] drm/i915: fix VGA plane disable for Ironlake+ > > We need to use I/O port instructions to access VGA registers on > Ironlake+, and it doesn't hurt on other platforms, so switch the VGA > plane disable function over to using them. Move it to init time as well > while we're at it, no need to repeatedly disable the VGA plane with > every mode set and DPMS event. > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > Signed-off-by: Manoj Iyer <manoj.iyer@canonical.com> > > Buglink: http://launchpad.net/bugs/602281 Minor nit pick, this should be "BugLink:" and if it could be moved to above the SOB's that'd be great. More below ... > --- > drivers/gpu/drm/i915/intel_display.c | 57 ++++++++++++++++++---------------- > 1 files changed, 30 insertions(+), 27 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 8a84306..9922539 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -29,6 +29,7 @@ > #include <linux/i2c.h> > #include <linux/kernel.h> > #include <linux/slab.h> > +#include <linux/vgaarb.h> > #include "drmP.h" > #include "intel_drv.h" > #include "i915_drm.h" > @@ -1458,29 +1459,6 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y, > return 0; > } > > -/* Disable the VGA plane that we never use */ > -static void i915_disable_vga (struct drm_device *dev) > -{ > - struct drm_i915_private *dev_priv = dev->dev_private; > - u8 sr1; > - u32 vga_reg; > - > - if (HAS_PCH_SPLIT(dev)) > - vga_reg = CPU_VGACNTRL; > - else > - vga_reg = VGACNTRL; > - > - if (I915_READ(vga_reg) & VGA_DISP_DISABLE) > - return; > - > - I915_WRITE8(VGA_SR_INDEX, 1); > - sr1 = I915_READ8(VGA_SR_DATA); > - I915_WRITE8(VGA_SR_DATA, sr1 | (1 << 5)); > - udelay(100); > - > - I915_WRITE(vga_reg, VGA_DISP_DISABLE); > -} > - > static void ironlake_disable_pll_edp (struct drm_crtc *crtc) > { > struct drm_device *dev = crtc->dev; > @@ -1995,7 +1973,9 @@ static void ironlake_crtc_dpms(struct drm_crtc *crtc, int mode) > I915_READ(dspbase_reg); > } > > - i915_disable_vga(dev); > + if (dev_priv->cfb_plane == plane && > + dev_priv->display.disable_fbc) > + dev_priv->display.disable_fbc(dev); The upstream version of patch 9cce37f4 does not add the above lines. It seems the above lines are added as part of upstream commit: commit b52eb4dcab23fe0c52a437276258e0afcf913ef5 Author: Zhao Yakui <yakui.zhao@intel.com> Date: Sat Jun 12 14:32:27 2010 +0800 drm/i915: Add frame buffer compression support on Ironlake mobile Did you unintentionally add these lines when backporting upstream patch 9cce37f4? If so, care to fix this up and re-send? Also, if you do so, can you add the following line in the commit message: (backported from upstream commit 9cce37f4855a30cc7c364edf18522282782f7ddc) Thanks, Leann > /* disable cpu pipe, disable after all planes disabled */ > temp = I915_READ(pipeconf_reg); > @@ -2254,9 +2234,6 @@ static void i9xx_crtc_dpms(struct drm_crtc *crtc, int mode) > dev_priv->display.disable_fbc) > dev_priv->display.disable_fbc(dev); > > - /* Disable the VGA plane that we never use */ > - i915_disable_vga(dev); > - > /* Disable display plane */ > temp = I915_READ(dspcntr_reg); > if ((temp & DISPLAY_PLANE_ENABLE) != 0) { > @@ -5633,6 +5610,29 @@ static void intel_init_quirks(struct drm_device *dev) > } > } > > +/* Disable the VGA plane that we never use */ > +static void i915_disable_vga(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + u8 sr1; > + u32 vga_reg; > + > + if (HAS_PCH_SPLIT(dev)) > + vga_reg = CPU_VGACNTRL; > + else > + vga_reg = VGACNTRL; > + > + vga_get_uninterruptible(dev->pdev, VGA_RSRC_LEGACY_IO); > + outb(1, VGA_SR_INDEX); > + sr1 = inb(VGA_SR_DATA); > + outb(sr1 | 1<<5, VGA_SR_DATA); > + vga_put(dev->pdev, VGA_RSRC_LEGACY_IO); > + udelay(300); > + > + I915_WRITE(vga_reg, VGA_DISP_DISABLE); > + POSTING_READ(vga_reg); > +} > + > void intel_modeset_init(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > @@ -5681,6 +5681,9 @@ void intel_modeset_init(struct drm_device *dev) > > intel_init_clock_gating(dev); > > + /* Just disable it once at startup */ > + i915_disable_vga(dev); > + > if (IS_IRONLAKE_M(dev)) { > ironlake_enable_drps(dev); > intel_init_emon(dev); > -- > 1.7.0.4 > > > > Cheers > --- manjo >
> > Examining this patch, it appears to actually be upstream: > > commit 9cce37f4855a30cc7c364edf18522282782f7ddc > Author: Jesse Barnes <jbarnes@virtuousgeek.org> > Date: Fri Aug 13 15:11:26 2010 -0700 > > drm/i915: fix VGA plane disable for Ironlake+ > Yes this was cherry-pick from upstream with minor backporting. > However, I noticed a discrepancy between the you've provided and the > upstream version of the patch (see below). > >> >> Buglink: http://launchpad.net/bugs/602281 > > Minor nit pick, this should be "BugLink:" and if it could be moved to > above the SOB's that'd be great. More below ... > Sorry, that was a typo. >> >> - i915_disable_vga(dev); >> + if (dev_priv->cfb_plane == plane && >> + dev_priv->display.disable_fbc) >> + dev_priv->display.disable_fbc(dev); > > The upstream version of patch 9cce37f4 does not add the above lines. It > seems the above lines are added as part of upstream commit: > > commit b52eb4dcab23fe0c52a437276258e0afcf913ef5 > Author: Zhao Yakui <yakui.zhao@intel.com> > Date: Sat Jun 12 14:32:27 2010 +0800 > > drm/i915: Add frame buffer compression support on Ironlake mobile > > Did you unintentionally add these lines when backporting upstream patch > 9cce37f4? If so, care to fix this up and re-send? Also, if you do so, > can you add the following line in the commit message: > > (backported from upstream commit 9cce37f4855a30cc7c364edf18522282782f7ddc) Yep that is definitely my bad, I had that patch in that tree by mistake. Thanks for catching that error. I will send you a fresh patch with the correct bits from the patch. I will add the backported from string to the commit log.
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 8a84306..9922539 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -29,6 +29,7 @@ #include <linux/i2c.h> #include <linux/kernel.h> #include <linux/slab.h> +#include <linux/vgaarb.h> #include "drmP.h" #include "intel_drv.h" #include "i915_drm.h" @@ -1458,29 +1459,6 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y, return 0; } -/* Disable the VGA plane that we never use */ -static void i915_disable_vga (struct drm_device *dev) -{ - struct drm_i915_private *dev_priv = dev->dev_private; - u8 sr1; - u32 vga_reg; - - if (HAS_PCH_SPLIT(dev)) - vga_reg = CPU_VGACNTRL; - else - vga_reg = VGACNTRL; - - if (I915_READ(vga_reg) & VGA_DISP_DISABLE) - return; - - I915_WRITE8(VGA_SR_INDEX, 1); - sr1 = I915_READ8(VGA_SR_DATA); - I915_WRITE8(VGA_SR_DATA, sr1 | (1 << 5)); - udelay(100); - - I915_WRITE(vga_reg, VGA_DISP_DISABLE); -} - static void ironlake_disable_pll_edp (struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; @@ -1995,7 +1973,9 @@ static void ironlake_crtc_dpms(struct drm_crtc *crtc, int mode) I915_READ(dspbase_reg); } - i915_disable_vga(dev); + if (dev_priv->cfb_plane == plane && + dev_priv->display.disable_fbc) + dev_priv->display.disable_fbc(dev); /* disable cpu pipe, disable after all planes disabled */ temp = I915_READ(pipeconf_reg); @@ -2254,9 +2234,6 @@ static void i9xx_crtc_dpms(struct drm_crtc *crtc, int mode) dev_priv->display.disable_fbc) dev_priv->display.disable_fbc(dev); - /* Disable the VGA plane that we never use */ - i915_disable_vga(dev); - /* Disable display plane */ temp = I915_READ(dspcntr_reg); if ((temp & DISPLAY_PLANE_ENABLE) != 0) { @@ -5633,6 +5610,29 @@ static void intel_init_quirks(struct drm_device *dev) } } +/* Disable the VGA plane that we never use */ +static void i915_disable_vga(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + u8 sr1; + u32 vga_reg; + + if (HAS_PCH_SPLIT(dev)) + vga_reg = CPU_VGACNTRL; + else + vga_reg = VGACNTRL; + + vga_get_uninterruptible(dev->pdev, VGA_RSRC_LEGACY_IO); + outb(1, VGA_SR_INDEX); + sr1 = inb(VGA_SR_DATA); + outb(sr1 | 1<<5, VGA_SR_DATA); + vga_put(dev->pdev, VGA_RSRC_LEGACY_IO); + udelay(300); + + I915_WRITE(vga_reg, VGA_DISP_DISABLE); + POSTING_READ(vga_reg); +} + void intel_modeset_init(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; @@ -5681,6 +5681,9 @@ void intel_modeset_init(struct drm_device *dev) intel_init_clock_gating(dev); + /* Just disable it once at startup */ + i915_disable_vga(dev); + if (IS_IRONLAKE_M(dev)) { ironlake_enable_drps(dev); intel_init_emon(dev);