| 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);
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+ 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 --- drivers/gpu/drm/i915/intel_display.c | 57 ++++++++++++++++++---------------- 1 files changed, 30 insertions(+), 27 deletions(-)