Re: [Intel-gfx] [PATCH] drm/i915: Hold struct_mutex during hotplug processing

2011-07-29 Thread Daniel Vetter
On Thu, Jul 28, 2011 at 03:50:00PM -0700, Keith Packard wrote:
 On Wed, 27 Jul 2011 09:03:31 -0700, Jesse Barnes jbar...@virtuousgeek.org 
 wrote:
  On Wed, 27 Jul 2011 02:21:24 -0700
  Keith Packard kei...@keithp.com wrote:
   So the work may get executed immediately rather than being run later at
   some point?
  
  It sure looks that way... but I don't remember any rule about work
  queue items having inter dependencies like this.

I've checked the workqueue code and haven't found it to run a work
immediately, it's always queued. Further this problem is very easy to
diagnose: Even without lockdep the scheduler will notice the stuck task
after about 120s and the backtrace should make matters extremely clear. On
the other hand if somebody adds some nice state clobbering in the drm
helper, we have a very hard bug to track down.

Generally modesetting isn't perf critical, so I vote for more locking,
just in case.
-Daniel
-- 
Daniel Vetter
Mail: dan...@ffwll.ch
Mobile: +41 (0)79 365 57 48
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] i915 SSC Patch

2011-07-29 Thread Ben Brewer
I've added a global SSC (Spread Spectrum Clock) parameter to the i915 
driver, since having SSC enabled breaks (distorts) VGA output on some 
Core i5/i7 chips (see https://bugs.freedesktop.org/show_bug.cgi?id=38750).
SSC is still enabled by default so the behaviour won't change but 
setting the global_use_ssc parameter will turn this feature off and 
allow affected devices to function correctly (notably the Dell Vostro 3300).


Numerous people have tested this and reported it working (as seen in the 
bug report thread) which isn't surprising considering it's a basic 
modification of Chris Wilsons work.


Any comments, or anybody willing to include this patch?

Thanks,
Ben Brewer (CodeThink)
diff -uNr linux-2.6.38/drivers/gpu/drm/i915/i915_drv.c 
linux-2.6.38-nossc/drivers/gpu/drm/i915/i915_drv.c
--- linux-2.6.38/drivers/gpu/drm/i915/i915_drv.c2011-03-15 
01:20:32.0 +
+++ linux-2.6.38-nossc/drivers/gpu/drm/i915/i915_drv.c  2011-07-26 
15:06:34.762058717 +0100
@@ -55,7 +55,10 @@
 unsigned int i915_lvds_downclock = 0;
 module_param_named(lvds_downclock, i915_lvds_downclock, int, 0400);
 
-unsigned int i915_panel_use_ssc = 1;
+unsigned int i915_use_ssc = 1;
+module_param_named(global_use_ssc, i915_use_ssc, int, 0600);
+
+unsigned int i915_panel_use_ssc = 1;
 module_param_named(lvds_use_ssc, i915_panel_use_ssc, int, 0600);
 
 bool i915_try_reset = true;
diff -uNr linux-2.6.38/drivers/gpu/drm/i915/i915_drv.h 
linux-2.6.38-nossc/drivers/gpu/drm/i915/i915_drv.h
--- linux-2.6.38/drivers/gpu/drm/i915/i915_drv.h2011-03-15 
01:20:32.0 +
+++ linux-2.6.38-nossc/drivers/gpu/drm/i915/i915_drv.h  2011-07-26 
13:50:31.198058201 +0100
@@ -344,6 +344,7 @@
unsigned int lvds_vbt:1;
unsigned int int_crt_support:1;
unsigned int lvds_use_ssc:1;
+   unsigned int display_clock_mode:1;
int lvds_ssc_freq;
struct {
int rate;
@@ -958,6 +959,7 @@
 extern unsigned int i915_powersave;
 extern unsigned int i915_semaphores;
 extern unsigned int i915_lvds_downclock;
+extern unsigned int i915_use_ssc;
 extern unsigned int i915_panel_use_ssc;
 extern unsigned int i915_enable_rc6;
 
diff -uNr linux-2.6.38/drivers/gpu/drm/i915/intel_bios.c 
linux-2.6.38-nossc/drivers/gpu/drm/i915/intel_bios.c
--- linux-2.6.38/drivers/gpu/drm/i915/intel_bios.c  2011-03-15 
01:20:32.0 +
+++ linux-2.6.38-nossc/drivers/gpu/drm/i915/intel_bios.c2011-07-20 
12:53:52.281036594 +0100
@@ -270,6 +270,8 @@
dev_priv-lvds_ssc_freq = general-ssc_freq ? 100 : 120;
else
dev_priv-lvds_ssc_freq = general-ssc_freq ? 100 : 96;
+
+   dev_priv-display_clock_mode = general-display_clock_mode;
}
 }
 
diff -uNr linux-2.6.38/drivers/gpu/drm/i915/intel_bios.h 
linux-2.6.38-nossc/drivers/gpu/drm/i915/intel_bios.h
--- linux-2.6.38/drivers/gpu/drm/i915/intel_bios.h  2011-03-15 
01:20:32.0 +
+++ linux-2.6.38-nossc/drivers/gpu/drm/i915/intel_bios.h2011-07-20 
12:52:50.309036565 +0100
@@ -120,7 +120,9 @@
u8 ssc_freq:1;
u8 enable_lfp_on_override:1;
u8 disable_ssc_ddt:1;
-   u8 rsvd8:3; /* finish byte */
+   u8 rsvd7:1;
+   u8 display_clock_mode:1;
+   u8 rsvd8:1; /* finish byte */
 
 /* bits 3 */
u8 disable_smooth_vision:1;
diff -uNr linux-2.6.38/drivers/gpu/drm/i915/intel_display.c 
linux-2.6.38-nossc/drivers/gpu/drm/i915/intel_display.c
--- linux-2.6.38/drivers/gpu/drm/i915/intel_display.c   2011-07-25 
10:12:09.904820505 +0100
+++ linux-2.6.38-nossc/drivers/gpu/drm/i915/intel_display.c 2011-07-26 
14:05:07.298058301 +0100
@@ -3924,7 +3924,7 @@
 
 static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv)
 {
-   return dev_priv-lvds_use_ssc  i915_panel_use_ssc;
+   return dev_priv-lvds_use_ssc  i915_panel_use_ssc  i915_use_ssc;
 }
 
 static int intel_crtc_mode_set(struct drm_crtc *crtc,
@@ -4153,39 +4153,59 @@
 */
if (HAS_PCH_SPLIT(dev)) {
temp = I915_READ(PCH_DREF_CONTROL);
-   /* Always enable nonspread source */
temp = ~DREF_NONSPREAD_SOURCE_MASK;
-   temp |= DREF_NONSPREAD_SOURCE_ENABLE;
temp = ~DREF_SSC_SOURCE_MASK;
-   temp |= DREF_SSC_SOURCE_ENABLE;
+   if (i915_use_ssc) {
+   /* Always enable nonspread source */
+   temp |= DREF_NONSPREAD_SOURCE_ENABLE;
+   temp |= DREF_SSC_SOURCE_ENABLE;
+   } else {
+   temp = ~DREF_SSC1_ENABLE;
+   temp = ~DREF_SSC4_ENABLE;
+   temp = ~DREF_SUPERSPREAD_SOURCE_ENABLE;
+   temp = ~DREF_CPU_SOURCE_OUTPUT_MASK;
+   }
I915_WRITE(PCH_DREF_CONTROL, temp);
 
POSTING_READ(PCH_DREF_CONTROL);
udelay(200);
 
-   if 

Re: [Intel-gfx] i915 SSC Patch

2011-07-29 Thread Paul Menzel
Dear Ben,


Am Freitag, den 29.07.2011, 13:55 +0100 schrieb Ben Brewer:
 I've added a global SSC (Spread Spectrum Clock) parameter to the i915 
 driver, since having SSC enabled breaks (distorts) VGA output on some 
 Core i5/i7 chips (see https://bugs.freedesktop.org/show_bug.cgi?id=38750).
 SSC is still enabled by default so the behaviour won't change but 
 setting the global_use_ssc parameter will turn this feature off and 
 allow affected devices to function correctly (notably the Dell Vostro 3300).
 
 Numerous people have tested this and reported it working (as seen in the 
 bug report thread) which isn't surprising considering it's a basic 
 modification of Chris Wilsons work.
 
 Any comments, or anybody willing to include this patch?

You at least need to include your Signed-off-by line [1].

Additionally I do not know if your patch applies against latest master.
So using Git with `git format-patch -s` is recommended.


Thanks,

Paul


[1] 
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=tree;f=Documentation


signature.asc
Description: This is a digitally signed message part
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: apply phase pointer override on SNB+ too

2011-07-29 Thread Keith Packard
On Thu, 28 Jul 2011 17:07:12 -0700, Jesse Barnes jbar...@virtuousgeek.org 
wrote:

 +#define _TRANSA_CHICKEN2  0xf0064
 +#define _TRANSB_CHICKEN2  0xf1064
 +#define TRANS_CHICKEN2(pipe) _PIPE(pipe, _TRANSA_CHICKEN2, _TRANSB_CHICKEN2)
 +#define   TRANS_AUTOTRAIN_GEN_STALL_DIS  (131)
 +
 +#define SOUTH_CHICKEN1   0xc2000
 +#define  FDIA_PHASE_SYNC_SHIFT   18
 +#define  FDI_PHASE_SYNC_OVR_EN   (3)

How about

#define FDIA_PHASE_SYNC_OVERRIDE_SHIFT  19
#define FDIA_PHASE_SYNC_ENABLE_SHIFT18
#define FDI_PHASE_SYNC_OVERRIDE(pipe) (1  (FDIA_PHASE_SYNC_OVERRIDE_SHIFT - 
(pipe) * 2))
#define FDI_PHASE_SYNC_ENABLE(pipe)   (1  (FDIA_PHASE_SYNC_ENABLE_SHIFT - 
(pipe) * 2))

defines instead?

Then use FDI_PHASE_SYNC_OVERRIDE(pipe) | FDI_PHASE_SYNC_ENABLE(pipe)

in the code.

-- 
keith.pack...@intel.com


pgp6nFQPoNZ42.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] drm/i915: apply phase pointer override on SNB+ too

2011-07-29 Thread Jesse Barnes
These bits moved around on SNB and above.

v2: again with the git send-email fail
v3: add macros for getting per-pipe override  enable bits

Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
---
 drivers/gpu/drm/i915/i915_reg.h  |5 +
 drivers/gpu/drm/i915/intel_display.c |   28 +++-
 2 files changed, 32 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 00bd510..abab1f5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3077,6 +3077,11 @@
 #define TRANS_CHICKEN2(pipe) _PIPE(pipe, _TRANSA_CHICKEN2, _TRANSB_CHICKEN2)
 #define   TRANS_AUTOTRAIN_GEN_STALL_DIS(131)
 
+#define SOUTH_CHICKEN1 0xc2000
+#define  FDIA_PHASE_SYNC_SHIFT_OVR 19
+#define  FDIA_PHASE_SYNC_SHIFT_EN  18
+#define FDI_PHASE_SYNC_OVR(pipe) (1(FDIA_PHASE_SYNC_SHIFT_OVR - ((pipe) * 
2)))
+#define FDI_PHASE_SYNC_EN(pipe) (1(FDIA_PHASE_SYNC_SHIFT_EN - ((pipe) * 2)))
 #define SOUTH_CHICKEN2 0xc2004
 #define  DPLS_EDP_PPS_FIX_DIS  (10)
 
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 8f7ed73..6f1dfc3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2110,6 +2110,18 @@ static void intel_fdi_normal_train(struct drm_crtc *crtc)
   FDI_FE_ERRC_ENABLE);
 }
 
+static void cpt_phase_pointer_enable(struct drm_device *dev, int pipe)
+{
+   struct drm_i915_private *dev_priv = dev-dev_private;
+   u32 flags = I915_READ(SOUTH_CHICKEN1);
+
+   flags |= FDI_PHASE_SYNC_OVR(pipe);
+   I915_WRITE(SOUTH_CHICKEN1, flags); /* once to unlock... */
+   flags |= FDI_PHASE_SYNC_EN(pipe);
+   I915_WRITE(SOUTH_CHICKEN1, flags); /* then again to enable */
+   POSTING_READ(SOUTH_CHICKEN1);
+}
+
 /* The FDI link training functions for ILK/Ibexpeak. */
 static void ironlake_fdi_link_train(struct drm_crtc *crtc)
 {
@@ -2157,7 +2169,8 @@ static void ironlake_fdi_link_train(struct drm_crtc *crtc)
I915_WRITE(FDI_RX_CHICKEN(pipe), FDI_RX_PHASE_SYNC_POINTER_OVR);
I915_WRITE(FDI_RX_CHICKEN(pipe), FDI_RX_PHASE_SYNC_POINTER_OVR |
   FDI_RX_PHASE_SYNC_POINTER_EN);
-   }
+   } else if (HAS_PCH_CPT(dev))
+   cpt_phase_pointer_enable(dev, pipe);
 
reg = FDI_RX_IIR(pipe);
for (tries = 0; tries  5; tries++) {
@@ -2485,6 +2498,17 @@ static void ironlake_fdi_pll_enable(struct drm_crtc 
*crtc)
}
 }
 
+static void cpt_phase_pointer_disable(struct drm_device *dev, int pipe)
+{
+   struct drm_i915_private *dev_priv = dev-dev_private;
+   u32 flags = I915_READ(SOUTH_CHICKEN1);
+
+   flags = ~(FDI_PHASE_SYNC_EN(pipe));
+   I915_WRITE(SOUTH_CHICKEN1, flags); /* once to disable... */
+   flags = ~(FDI_PHASE_SYNC_OVR(pipe));
+   I915_WRITE(SOUTH_CHICKEN1, flags); /* then again to lock */
+   POSTING_READ(SOUTH_CHICKEN1);
+}
 static void ironlake_fdi_disable(struct drm_crtc *crtc)
 {
struct drm_device *dev = crtc-dev;
@@ -2514,6 +2538,8 @@ static void ironlake_fdi_disable(struct drm_crtc *crtc)
I915_WRITE(FDI_RX_CHICKEN(pipe),
   I915_READ(FDI_RX_CHICKEN(pipe) 
 ~FDI_RX_PHASE_SYNC_POINTER_EN));
+   } else if (HAS_PCH_CPT(dev)) {
+   cpt_phase_pointer_disable(dev, pipe);
}
 
/* still set train pattern 1 */
-- 
1.7.4.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] i915 SSC Patch

2011-07-29 Thread Keith Packard
On Fri, 29 Jul 2011 20:02:27 +0100, Chris Wilson ch...@chris-wilson.co.uk 
wrote:

 It's not meant to be and it causes havoc, from wavy/blurry output to no
 sync. The other part of the patch on that bug was to walk the crtcs and
 turn off SSC on the shared refclk if any output could not handle SSC. At
 that point, an objection was raised that we shouldn't even be touching
 the refclk if any output was currently being driven from it.

So the correct fix is to turn off everything, disable SSC and turn
everything back on? That seems tedious, but not impossible (just DPMS
off the world, then DPMS everything back on...).

-- 
keith.pack...@intel.com


pgpicQHlpBKKS.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: apply phase pointer override on SNB+ too

2011-07-29 Thread Keith Packard
On Fri, 29 Jul 2011 12:42:37 -0700, Jesse Barnes jbar...@virtuousgeek.org 
wrote:

 v2: again with the git send-email fail
 v3: add macros for getting per-pipe override  enable bits
 v4: enable phase sync pointer on SNB and IVB configs as well

Yeah, this looks good -- a bit tricky in ironlake_fdi_disable -- it
works only because ILK was never supported with CPT, right?

-- 
keith.pack...@intel.com


pgpAXocr39xB5.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] drm/i915: Ignored GPU wedged errors while pinning scanout buffers

2011-07-29 Thread Keith Packard
Failing to pin a scanout buffer will most likely lead to a black
screen, so if the GPU is wedged, then just let the pin happen and hope
that things work out OK.

Signed-off-by: Keith Packard kei...@keithp.com
---
 drivers/gpu/drm/i915/i915_gem.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index a087e1b..d5c7c7b 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3019,7 +3019,7 @@ i915_gem_object_set_to_display_plane(struct 
drm_i915_gem_object *obj,
/* Currently, we are always called from an non-interruptible context. */
if (pipelined != obj-ring) {
ret = i915_gem_object_wait_rendering(obj);
-   if (ret)
+   if (ret == -ERESTARTSYS)
return ret;
}
 
-- 
1.7.5.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: apply phase pointer override on SNB+ too

2011-07-29 Thread Jesse Barnes
On Fri, 29 Jul 2011 14:22:24 -0700
Keith Packard kei...@keithp.com wrote:

 On Fri, 29 Jul 2011 12:42:37 -0700, Jesse Barnes jbar...@virtuousgeek.org 
 wrote:
 
  v2: again with the git send-email fail
  v3: add macros for getting per-pipe override  enable bits
  v4: enable phase sync pointer on SNB and IVB configs as well
 
 Yeah, this looks good -- a bit tricky in ironlake_fdi_disable -- it
 works only because ILK was never supported with CPT, right?

Right, I don't think we have any configs like that.

-- 
Jesse Barnes, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915/pch: Save/restore PCH_PORT_HOTPLUG across suspend

2011-07-29 Thread Keith Packard
On Tue, 26 Jul 2011 14:00:32 -0700, Jesse Barnes jbar...@virtuousgeek.org 
wrote:

 We should be writing this reg.  The only question is whether we should
 be trusting the BIOS values (which may have custom pulse duration
 settings) or just unconditionally enabling hot plug for ports we care
 about with the default 2ms pulse width (per the DP spec).

I'll merge the suspend/resume save/restore stuff, if we decide to
customize the value, we can do that later.

-- 
keith.pack...@intel.com


pgpXISY4z7fGm.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx