Re: [Intel-gfx] [PATCH] Lower threshold for pixel doubling.

[Daniel Vetter]

On Tue, Jun 25, 2013 at 1:28 AM, Stuart Abercrombie
sabercrom...@google.com wrote:
 This is with the patch.

Hm, nothing really interesting bug I've finally gotten around to some
doc reading and they seem to insist pretty hard that double wide mode
on pipe A only works if we have pipe A linked up with cursor/plane A.
But on gen3 mobile we switch pipesplanes and Stéphane said on irc
that you've changed the logic in there a bit. Can you please check
whether you're still using plane B for pipe A or if there's something
broken?
-Daniel



 On Sun, Jun 23, 2013 at 11:59 PM, Daniel Vetter dan...@ffwll.ch wrote:

 On Sat, Jun 22, 2013 at 12:52 AM, Stuart Abercrombie
 sabercrom...@google.com wrote:
  Maybe I missed something, but I didn't see a response to this.  Can we
  get
  this fix in?

 Sorry for the delay, I've lost track of this. Can you please boot with
 drm.debug=0xe and attach the full dmesg (with or without your patch)?
 -Daniel

 
 
  On Wed, May 29, 2013 at 9:22 AM, Stuart Abercrombie
  sabercrom...@google.com wrote:
 
  Without this change I saw PIPE_FIFO_UNDERRUN_STATUS set in PIPEASTAT,
  which I took to indicate an underrun problem.
 
  Here's what I found with other modes on this monitor:
 
  1920x1200 works with pixel doubling enabled. Pixel clock 193.2 MHz.
  2048x1152 works with pixel doubling enabled. Pixel clock 198.0 MHz.
  1600x1200 works with pixel doubling disabled. Pixel clock 162.0 MHz.
  2048x1280 fails with pixel doubling disabled. PIxel clock 174.2 MHz..
 
  Based on this, it seems the threshold needs to be be between 162.0MHz
  and
  174.2MHz, whereas currently it's at 180MHz.  The change puts it at
  170MHz.
 
  Stuart
 
 
  On Wed, May 29, 2013 at 8:22 AM, Daniel Vetter dan...@ffwll.ch wrote:
 
  On Tue, May 28, 2013 at 10:39:07AM -0700, Stuart Abercrombie wrote:
   Any comments?
  
   Without this, plugging one of the older Chromebook models into a
   Dell
   U3011
   monitor produces a garbled display at the default 2048x1280
   resolution.
  
   The original threshold was apparently fairly arbitrary:
  
  
  
   http://cgit.freedesktop.org/~anholt/xf86-video-intel/commit/?id=8fcf9a81179ee8577ddab5e904c58fbfd14cf59c
 
  Do you see any pipe underruns without this patch? There are some
  not-yet
  implemented tricks we should be pulling around re-splitting DSP_ARB
  fifo
  entries, which tend to totally kill high-res modes.
  -Daniel
 
   .
  
   Stuart
  
  
   On Mon, May 20, 2013 at 11:15 AM, Stuart Abercrombie 
   sabercrom...@chromium.org wrote:
  
90% of core speed (=180MHz dot clock) is too high for 2048x1280 to
get
pixel doubling on Pineview, which it needs to avoid underruns, so
lower this to 85%.
   
Signed-off-by: Stuart Abercrombie sabercrom...@chromium.org
---
 drivers/gpu/drm/i915/intel_display.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
   
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index efe8299..9c924e9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4564,14 +4564,14 @@ static void i9xx_set_pipeconf(struct
intel_crtc
*intel_crtc)
pipeconf = I915_READ(PIPECONF(intel_crtc-pipe));
   
if (intel_crtc-pipe == 0  INTEL_INFO(dev)-gen  4) {
-   /* Enable pixel doubling when the dot clock is 
90%
of
the (display)
+   /* Enable pixel doubling when the dot clock is 
85%
of
the (display)
 * core speed.
 *
 * XXX: No double-wide on 915GM pipe B. Is that
the
only
reason for the
 * pipe == 0 check?
 */
if (intel_crtc-config.requested_mode.clock 
-   dev_priv-display.get_display_clock_speed(dev)
*
9 /
10)
+   dev_priv-display.get_display_clock_speed(dev)
*
17 /
20)
pipeconf |= PIPECONF_DOUBLE_WIDE;
else
pipeconf = ~PIPECONF_DOUBLE_WIDE;
--
1.8.2.1
   
   
 
   ___
   Intel-gfx mailing list
   Intel-gfx@lists.freedesktop.org
   http://lists.freedesktop.org/mailman/listinfo/intel-gfx
 
 
  --
  Daniel Vetter
  Software Engineer, Intel Corporation
  +41 (0) 79 365 57 48 - http://blog.ffwll.ch
 
 
 



 --
 Daniel Vetter
 Software Engineer, Intel Corporation
 +41 (0) 79 365 57 48 - http://blog.ffwll.ch





--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] Lower threshold for pixel doubling.

[Stéphane Marchesin]

On Thu, Jul 25, 2013 at 11:01 PM, Daniel Vetter dan...@ffwll.ch wrote:
 On Tue, Jun 25, 2013 at 1:28 AM, Stuart Abercrombie
 sabercrom...@google.com wrote:
 This is with the patch.

 Hm, nothing really interesting bug I've finally gotten around to some
 doc reading and they seem to insist pretty hard that double wide mode
 on pipe A only works if we have pipe A linked up with cursor/plane A.
 But on gen3 mobile we switch pipesplanes and Stéphane said on irc
 that you've changed the logic in there a bit. Can you please check
 whether you're still using plane B for pipe A or if there's something
 broken?

We had to change that logic because 3.8 doesn't display anything by
default on that machine (the autodetect logic is broken, as I reported
in
http://lists.freedesktop.org/archives/dri-devel/2013-March/036070.html
). Do you want to address that issue first?

Stéphane

 -Daniel



 On Sun, Jun 23, 2013 at 11:59 PM, Daniel Vetter dan...@ffwll.ch wrote:

 On Sat, Jun 22, 2013 at 12:52 AM, Stuart Abercrombie
 sabercrom...@google.com wrote:
  Maybe I missed something, but I didn't see a response to this.  Can we
  get
  this fix in?

 Sorry for the delay, I've lost track of this. Can you please boot with
 drm.debug=0xe and attach the full dmesg (with or without your patch)?
 -Daniel

 
 
  On Wed, May 29, 2013 at 9:22 AM, Stuart Abercrombie
  sabercrom...@google.com wrote:
 
  Without this change I saw PIPE_FIFO_UNDERRUN_STATUS set in PIPEASTAT,
  which I took to indicate an underrun problem.
 
  Here's what I found with other modes on this monitor:
 
  1920x1200 works with pixel doubling enabled. Pixel clock 193.2 MHz.
  2048x1152 works with pixel doubling enabled. Pixel clock 198.0 MHz.
  1600x1200 works with pixel doubling disabled. Pixel clock 162.0 MHz.
  2048x1280 fails with pixel doubling disabled. PIxel clock 174.2 MHz..
 
  Based on this, it seems the threshold needs to be be between 162.0MHz
  and
  174.2MHz, whereas currently it's at 180MHz.  The change puts it at
  170MHz.
 
  Stuart
 
 
  On Wed, May 29, 2013 at 8:22 AM, Daniel Vetter dan...@ffwll.ch wrote:
 
  On Tue, May 28, 2013 at 10:39:07AM -0700, Stuart Abercrombie wrote:
   Any comments?
  
   Without this, plugging one of the older Chromebook models into a
   Dell
   U3011
   monitor produces a garbled display at the default 2048x1280
   resolution.
  
   The original threshold was apparently fairly arbitrary:
  
  
  
   http://cgit.freedesktop.org/~anholt/xf86-video-intel/commit/?id=8fcf9a81179ee8577ddab5e904c58fbfd14cf59c
 
  Do you see any pipe underruns without this patch? There are some
  not-yet
  implemented tricks we should be pulling around re-splitting DSP_ARB
  fifo
  entries, which tend to totally kill high-res modes.
  -Daniel
 
   .
  
   Stuart
  
  
   On Mon, May 20, 2013 at 11:15 AM, Stuart Abercrombie 
   sabercrom...@chromium.org wrote:
  
90% of core speed (=180MHz dot clock) is too high for 2048x1280 to
get
pixel doubling on Pineview, which it needs to avoid underruns, so
lower this to 85%.
   
Signed-off-by: Stuart Abercrombie sabercrom...@chromium.org
---
 drivers/gpu/drm/i915/intel_display.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
   
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index efe8299..9c924e9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4564,14 +4564,14 @@ static void i9xx_set_pipeconf(struct
intel_crtc
*intel_crtc)
pipeconf = I915_READ(PIPECONF(intel_crtc-pipe));
   
if (intel_crtc-pipe == 0  INTEL_INFO(dev)-gen  4) {
-   /* Enable pixel doubling when the dot clock is 
90%
of
the (display)
+   /* Enable pixel doubling when the dot clock is 
85%
of
the (display)
 * core speed.
 *
 * XXX: No double-wide on 915GM pipe B. Is that
the
only
reason for the
 * pipe == 0 check?
 */
if (intel_crtc-config.requested_mode.clock 
-   dev_priv-display.get_display_clock_speed(dev)
*
9 /
10)
+   dev_priv-display.get_display_clock_speed(dev)
*
17 /
20)
pipeconf |= PIPECONF_DOUBLE_WIDE;
else
pipeconf = ~PIPECONF_DOUBLE_WIDE;
--
1.8.2.1
   
   
 
   ___
   Intel-gfx mailing list
   Intel-gfx@lists.freedesktop.org
   http://lists.freedesktop.org/mailman/listinfo/intel-gfx
 
 
  --
  Daniel Vetter
  Software Engineer, Intel Corporation
  +41 (0) 79 365 57 48 - http://blog.ffwll.ch
 
 
 



 --
 Daniel Vetter
 Software Engineer, Intel Corporation
 +41 (0) 79 365 57 48 - http://blog.ffwll.ch





 --
 Daniel Vetter
 Software Engineer, Intel Corporation
 +41 

[Intel-gfx] [PATCH 1/2] drm/i915: enable pnv gpu reset

[Daniel Vetter]

According to configdb the same register as for gen4 also exists on
pnv. Try to use it.

Cc: Chris Wilson ch...@chris-wilson.co.uk
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
 drivers/gpu/drm/i915/intel_uncore.c | 20 +++-
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c 
b/drivers/gpu/drm/i915/intel_uncore.c
index 8f5bc86..859c84d 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -566,11 +566,21 @@ int intel_gpu_reset(struct drm_device *dev)
 {
switch (INTEL_INFO(dev)-gen) {
case 7:
-   case 6: return gen6_do_reset(dev);
-   case 5: return ironlake_do_reset(dev);
-   case 4: return i965_do_reset(dev);
-   case 2: return i8xx_do_reset(dev);
-   default: return -ENODEV;
+   case 6:
+   return gen6_do_reset(dev);
+   case 5:
+   return ironlake_do_reset(dev);
+   case 4:
+   return i965_do_reset(dev);
+   case 3:
+   if (IS_PINEVIEW(dev))
+   return i965_do_reset(dev);
+   else
+   return -ENODEV;
+   case 2:
+   return i8xx_do_reset(dev);
+   default:
+   return -ENODEV;
}
 }
 
-- 
1.8.3.2

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

[Intel-gfx] [PATCH 2/2] drm/i915: fix pnv display core clock readout out

[Daniel Vetter]

We need the correct clock to accurately assess whether we need to
enable the double wide pipe mode or not.

Cc: Chris Wilson ch...@chris-wilson.co.uk
Cc: Stéphane Marchesin marc...@chromium.org
Cc: Stuart Abercrombie sabercrom...@google.com
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
 drivers/gpu/drm/i915/i915_reg.h  |  6 ++
 drivers/gpu/drm/i915/intel_display.c | 29 -
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 6caa748..3aebe5d 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -61,6 +61,12 @@
 #define   GC_LOW_FREQUENCY_ENABLE  (1  7)
 #define   GC_DISPLAY_CLOCK_190_200_MHZ (0  4)
 #define   GC_DISPLAY_CLOCK_333_MHZ (4  4)
+#define   GC_DISPLAY_CLOCK_267_MHZ_PNV (0  4)
+#define   GC_DISPLAY_CLOCK_333_MHZ_PNV (1  4)
+#define   GC_DISPLAY_CLOCK_444_MHZ_PNV (2  4)
+#define   GC_DISPLAY_CLOCK_200_MHZ_PNV (5  4)
+#define   GC_DISPLAY_CLOCK_133_MHZ_PNV (6  4)
+#define   GC_DISPLAY_CLOCK_167_MHZ_PNV (7  4)
 #define   GC_DISPLAY_CLOCK_MASK(7  4)
 #define   GM45_GC_RENDER_CLOCK_MASK(0xf  0)
 #define   GM45_GC_RENDER_CLOCK_266_MHZ (8  0)
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index b3389d7..3e66f05 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4163,6 +4163,30 @@ static int i9xx_misc_get_display_clock_speed(struct 
drm_device *dev)
return 20;
 }
 
+static int pnv_get_display_clock_speed(struct drm_device *dev)
+{
+   u16 gcfgc = 0;
+
+   pci_read_config_word(dev-pdev, GCFGC, gcfgc);
+
+   switch (gcfgc  GC_DISPLAY_CLOCK_MASK) {
+   case GC_DISPLAY_CLOCK_267_MHZ_PNV:
+   return 267000;
+   case GC_DISPLAY_CLOCK_333_MHZ_PNV:
+   return 333000;
+   case GC_DISPLAY_CLOCK_444_MHZ_PNV:
+   return 444000;
+   case GC_DISPLAY_CLOCK_200_MHZ_PNV:
+   return 20;
+   default:
+   DRM_ERROR(Unknown pnv display core clock 0x%04x\n, gcfgc);
+   case GC_DISPLAY_CLOCK_133_MHZ_PNV:
+   return 133000;
+   case GC_DISPLAY_CLOCK_167_MHZ_PNV:
+   return 167000;
+   }
+}
+
 static int i915gm_get_display_clock_speed(struct drm_device *dev)
 {
u16 gcfgc = 0;
@@ -9605,9 +9629,12 @@ static void intel_init_display(struct drm_device *dev)
else if (IS_I915G(dev))
dev_priv-display.get_display_clock_speed =
i915_get_display_clock_speed;
-   else if (IS_I945GM(dev) || IS_845G(dev) || IS_PINEVIEW_M(dev))
+   else if (IS_I945GM(dev) || IS_845G(dev))
dev_priv-display.get_display_clock_speed =
i9xx_misc_get_display_clock_speed;
+   else if (IS_PINEVIEW(dev))
+   dev_priv-display.get_display_clock_speed =
+   pnv_get_display_clock_speed;
else if (IS_I915GM(dev))
dev_priv-display.get_display_clock_speed =
i915gm_get_display_clock_speed;
-- 
1.8.3.2

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

Re: [Intel-gfx] [PATCH 2/2] drm/i915: fix pnv display core clock readout out

[Chris Wilson]

On Fri, Jul 26, 2013 at 08:35:42AM +0200, Daniel Vetter wrote:
 We need the correct clock to accurately assess whether we need to
 enable the double wide pipe mode or not.
 
 Cc: Chris Wilson ch...@chris-wilson.co.uk
 Cc: Stéphane Marchesin marc...@chromium.org
 Cc: Stuart Abercrombie sabercrom...@google.com
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch

I have not found a definition of GCFGC for NM10. Mind including a
reference?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] linux-next: Tree for Jul 25 [ call-trace: drm | drm-intel related? ]

[Sedat Dilek]

On Fri, Jul 26, 2013 at 1:25 AM, Chris Wilson ch...@chris-wilson.co.uk wrote:
 On Fri, Jul 26, 2013 at 01:21:07AM +0200, Sedat Dilek wrote:
 On Thu, Jul 25, 2013 at 11:52 PM, Chris Wilson ch...@chris-wilson.co.uk 
 wrote:
  On Thu, Jul 25, 2013 at 10:07:02PM +0200, Sedat Dilek wrote:
  What means the bang line?
 
  [54.564] (II) GLX: Initialized DRI2 GL provider for screen 0
  [54.565] bang: 1159
  [54.565]
  Fatal server error:
  [54.565] failed to create screen resources
 
  That means between the kernel reporting success for
  DRM_IOCTL_I915_GEM_MMAP_GTT and libdrm returning from
  drm_intel_gem_bo_map_gtt(), something went wrong. This implies that the
  call to mmap() failed.
 
  I don't see how changing versions of the ddx would unmask the bug, nor
  why the mmap() should suddenly start failing. Anybody have any
  suggestions other than
 
  diff --git a/src/intel_uxa.c b/src/intel_uxa.c
  index 2f14173..3872258 100644
  --- a/src/intel_uxa.c
  +++ b/src/intel_uxa.c
  @@ -1149,12 +1149,15 @@ Bool intel_uxa_create_screen_resources(ScreenPtr 
  screen)
  PixmapPtr pixmap;
  intel_screen_private *intel = intel_get_screen_private(scrn);
  dri_bo *bo = intel-front_buffer;
  +   int ret;
 
  if (!uxa_resources_init(screen))
  return FALSE;
 
  -   if (drm_intel_gem_bo_map_gtt(bo))
  +   if ((ret = drm_intel_gem_bo_map_gtt(bo))) {
  +   ErrorF(%s:%d bang, errno=%d\n, __func__, __LINE__, -ret);
  return FALSE;
  +   }
 
  pixmap = screen-GetScreenPixmap(screen);
  intel_set_pixmap_bo(pixmap, bo);
 
  which is most likely to report EINVAL (22)?

 Yupp, this shows me...

 [28.542] (II) GLX: Initialized DRI2 GL provider for screen 0
 [28.543] intel_uxa_create_screen_resources:1158 bang, errno=22
 [28.543]
 Fatal server error:
 [28.543] failed to create screen resources

 I'm out of ideas, could you bisect this? Either kernel, userspace or
 both. Thanks,

I will wait for today's official Linux-next (next-20130726) release
and check if the issue still exists (might be I mismerged).
Cannot promise for bisecting, but for me it seems to be a
kernel-related issue.
So, my 1st bisect-strategy would be to see before/after drm-intel
merge into -next.

For example: I could start my X with even doing ugly hacks like this...

[ intel-ddx (git) ]
...
Bool intel_uxa_create_screen_resources(ScreenPtr screen)
...
#if 0
if (drm_intel_gem_bo_map_gtt(bo))
return FALSE;
#endif
...

...with any other kernel.

- Sedat -

 -Chris

 --
 Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] linux-next: Tree for Jul 25 [ call-trace: drm | drm-intel related? ]

[Chris Wilson]

On Fri, Jul 26, 2013 at 09:15:14AM +0200, Sedat Dilek wrote:
 For example: I could start my X with even doing ugly hacks like this...
 
 [ intel-ddx (git) ]
 ...
 Bool intel_uxa_create_screen_resources(ScreenPtr screen)
 ...
 #if 0
   if (drm_intel_gem_bo_map_gtt(bo))
   return FALSE;
 #endif
 ...
 
 ...with any other kernel.

Yes. Acquiring the map there is just a bit of paranoia to ensure we
having the mapping into the scanout already in place in case of
emergencies (and so don't fail along failure paths due to resource
conflicts).

Hmm, though we only started checking for map failures in 2.20.10 - which
would explain why going back to the older ddx masks the issue. And yes,
this means we do require a kernel bisect - or some passing inspiron.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/i915: enable pnv gpu reset

[Chris Wilson]

On Fri, Jul 26, 2013 at 08:35:41AM +0200, Daniel Vetter wrote:
 According to configdb the same register as for gen4 also exists on
 pnv. Try to use it.
 
 Cc: Chris Wilson ch...@chris-wilson.co.uk
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch

I think I also remember it dying terminally afterwards (some missing
reinit), but it's been long enough that we may fare better today. I'll
poke around the next time I boot into a pnv.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] Linux 3.11-rc2 (acpi backlight, revert)

[Steven Newbury]

On Thu, 2013-07-25 at 15:00 +0200, Rafael J. Wysocki wrote:
 On Monday, July 22, 2013 09:54:21 PM Rafael J. Wysocki wrote:
  On Monday, July 22, 2013 11:11:54 AM Linus Torvalds wrote:
   On Mon, Jul 22, 2013 at 6:02 AM, Rafael J. Wysocki r...@sisk.pl wrote:
   
Linus, do you want me to send a pull request reverting 8c5bd7a and 
efaa14c?
   
   Yes, but let's wait a while. Not because I think we'll fix the problem
   (hey, miracles might happen), but because I think it would be useful
   to couple the reverts with information about the particular machines
   that broke (and the people who reported it). So that when we
   inevitably try again, we can perhaps get some testing effort with
   those machines/people.
   
   It doesn't seem to be a show-stopped for a large number of people, so
   there's no huge hurry. I'd suggest doing the revert just in time for
   rc3, but waiting until then to gather info about people who see
   breakage.
   
   Sound like a plan?
  
  Yes, it does.
 
 OK, time to revert I guess.
 
 James, Kamal, Steven, Jörg, Martin, Kalle, please check if the apppended patch
 fixes the backlight for you.
 
 Aaron, please double check if acpi_video_backlight_quirks() will still work as
 needed.
 
 Thanks,
 Rafael
 
 
 ---
 From: Rafael J. Wysocki rafael.j.wyso...@intel.com
 Subject: Revert ACPI / video / i915: No ACPI backlight if firmware expects 
 Windows 8
 
 We attempted to address a regression introduced by commit a57f7f9
 (ACPICA: Add Windows8/Server2012 string for _OSI method.) after which
 ACPI video backlight support doesn't work on a number of systems,
 because the relevant AML methods in the ACPI tables in their BIOSes
 become useless after the BIOS has been told that the OS is compatible
 with Windows 8.  That problem is tracked by the bug entry at:
 
 https://bugzilla.kernel.org/show_bug.cgi?id=51231
 
 Commit 8c5bd7a (ACPI / video / i915: No ACPI backlight if firmware
 expects Windows 8) introduced for this purpose essentially prevented
 the ACPI backlight support from being used if the BIOS had been told
 that the OS was compatible with Windows 8 and the i915 driver was
 loaded, in which case the backlight would always be handled by i915.
 Unfortunately, however, that turned out to cause problems with
 backlight to appear on multiple systems with symptoms indicating that
 i915 was unable to control the backlight on those systems as
 expected.
 
 For this reason, revert commit 8c5bd7a, but leave the function
 acpi_video_backlight_quirks() introduced by it, because another
 commit on top of it uses that function.
 
Works fine for me.

Tested-by: Steven Newbury st...@snewbury.org.uk

By the way, I'm willing to test any i915 backlight patches if it helps.

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

Re: [Intel-gfx] [PATCH 2/2] x86: add early quirk for reserving Intel graphics stolen memory v3

[Daniel Vetter]

On Thu, Jul 25, 2013 at 09:37:49AM -0700, Jesse Barnes wrote:
 Systems with Intel graphics controllers set aside memory exclusively for
 gfx driver use.  This memory is not marked in the E820 as reserved or as
 RAM, and so is subject to overlap from E820 manipulation later in the
 boot process.  On some systems, MMIO space is allocated on top, despite
 the efforts of the RAM buffer approach, which simply rounds memory
 boundaries up to 64M to try to catch space that may decode as RAM and so
 is not suitable for MMIO.
 
 v2: use read_pci_config for 32 bit reads instead of adding a new one
 (Chris)
 add gen6 stolen size function (Chris)
 use a function pointer (Chris)
 drop gen2 bits (Daniel)
 
 Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org

Our QA seems to be happy with this.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=66844
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=66726
Tested-by: lu hua huax...@intel.com

Cheers, Daniel
 ---
  arch/x86/kernel/early-quirks.c  |  158 
 +++
  drivers/gpu/drm/i915/i915_reg.h |   15 
  include/drm/i915_drm.h  |   32 
  3 files changed, 190 insertions(+), 15 deletions(-)
 
 diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c
 index 94ab6b9..bff8a6f 100644
 --- a/arch/x86/kernel/early-quirks.c
 +++ b/arch/x86/kernel/early-quirks.c
 @@ -12,6 +12,7 @@
  #include linux/pci.h
  #include linux/acpi.h
  #include linux/pci_ids.h
 +#include drm/i915_drm.h
  #include asm/pci-direct.h
  #include asm/dma.h
  #include asm/io_apic.h
 @@ -208,6 +209,161 @@ static void __init intel_remapping_check(int num, int 
 slot, int func)
  
  }
  
 +/*
 + * Systems with Intel graphics controllers set aside memory exclusively
 + * for gfx driver use.  This memory is not marked in the E820 as reserved
 + * or as RAM, and so is subject to overlap from E820 manipulation later
 + * in the boot process.  On some systems, MMIO space is allocated on top,
 + * despite the efforts of the RAM buffer approach, which simply rounds
 + * memory boundaries up to 64M to try to catch space that may decode
 + * as RAM and so is not suitable for MMIO.
 + *
 + * And yes, so far on current devices the base addr is always under 4G.
 + */
 +static u32 __init intel_stolen_base(int num, int slot, int func)
 +{
 + u32 base;
 +
 + /*
 +  * Almost universally we can find the Graphics Base of Stolen Memory
 +  * at offset 0x5c in the igfx configuration space. On a few (desktop)
 +  * machines this is also mirrored in the bridge device at different
 +  * locations, or in the MCHBAR.
 +  */
 + base = read_pci_config(num, slot, func, 0x5c);
 + base = ~((120) - 1);
 +
 + return base;
 +}
 +
 +#define KB(x)((x) * 1024)
 +#define MB(x)(KB (KB (x)))
 +#define GB(x)(MB (KB (x)))
 +
 +static size_t __init gen3_stolen_size(int num, int slot, int func)
 +{
 + size_t stolen_size;
 + u16 gmch_ctrl;
 +
 + gmch_ctrl = read_pci_config_16(0, 0, 0, I830_GMCH_CTRL);
 +
 + switch (gmch_ctrl  I855_GMCH_GMS_MASK) {
 + case I855_GMCH_GMS_STOLEN_1M:
 + stolen_size = MB(1);
 + break;
 + case I855_GMCH_GMS_STOLEN_4M:
 + stolen_size = MB(4);
 + break;
 + case I855_GMCH_GMS_STOLEN_8M:
 + stolen_size = MB(8);
 + break;
 + case I855_GMCH_GMS_STOLEN_16M:
 + stolen_size = MB(16);
 + break;
 + case I855_GMCH_GMS_STOLEN_32M:
 + stolen_size = MB(32);
 + break;
 + case I915_GMCH_GMS_STOLEN_48M:
 + stolen_size = MB(48);
 + break;
 + case I915_GMCH_GMS_STOLEN_64M:
 + stolen_size = MB(64);
 + break;
 + case G33_GMCH_GMS_STOLEN_128M:
 + stolen_size = MB(128);
 + break;
 + case G33_GMCH_GMS_STOLEN_256M:
 + stolen_size = MB(256);
 + break;
 + case INTEL_GMCH_GMS_STOLEN_96M:
 + stolen_size = MB(96);
 + break;
 + case INTEL_GMCH_GMS_STOLEN_160M:
 + stolen_size = MB(160);
 + break;
 + case INTEL_GMCH_GMS_STOLEN_224M:
 + stolen_size = MB(224);
 + break;
 + case INTEL_GMCH_GMS_STOLEN_352M:
 + stolen_size = MB(352);
 + break;
 + default:
 + stolen_size = 0;
 + break;
 + }
 +
 + return stolen_size;
 +}
 +
 +static size_t __init gen6_stolen_size(int num, int slot, int func)
 +{
 + u16 gmch_ctrl;
 +
 + gmch_ctrl = read_pci_config_16(num, slot, func, SNB_GMCH_CTRL);
 + gmch_ctrl = SNB_GMCH_GMS_SHIFT;
 + gmch_ctrl = SNB_GMCH_GMS_MASK;
 +
 + return gmch_ctrl  25; /* 32 MB units */
 +}
 +
 +typedef size_t (*stolen_size_fn)(int num, int slot, int func);
 +
 +static struct pci_device_id intel_stolen_ids[] __initdata = {
 + INTEL_I915G_IDS(gen3_stolen_size),
 + 

Re: [Intel-gfx] [PATCH 2/2] drm/i915: fix pnv display core clock readout out

[Jani Nikula]

On Fri, 26 Jul 2013, Daniel Vetter daniel.vet...@ffwll.ch wrote:
 We need the correct clock to accurately assess whether we need to
 enable the double wide pipe mode or not.

 Cc: Chris Wilson ch...@chris-wilson.co.uk
 Cc: Stéphane Marchesin marc...@chromium.org
 Cc: Stuart Abercrombie sabercrom...@google.com
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 ---
  drivers/gpu/drm/i915/i915_reg.h  |  6 ++
  drivers/gpu/drm/i915/intel_display.c | 29 -
  2 files changed, 34 insertions(+), 1 deletion(-)

 diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
 index 6caa748..3aebe5d 100644
 --- a/drivers/gpu/drm/i915/i915_reg.h
 +++ b/drivers/gpu/drm/i915/i915_reg.h
 @@ -61,6 +61,12 @@
  #define   GC_LOW_FREQUENCY_ENABLE(1  7)
  #define   GC_DISPLAY_CLOCK_190_200_MHZ   (0  4)
  #define   GC_DISPLAY_CLOCK_333_MHZ   (4  4)
 +#define   GC_DISPLAY_CLOCK_267_MHZ_PNV   (0  4)
 +#define   GC_DISPLAY_CLOCK_333_MHZ_PNV   (1  4)
 +#define   GC_DISPLAY_CLOCK_444_MHZ_PNV   (2  4)
 +#define   GC_DISPLAY_CLOCK_200_MHZ_PNV   (5  4)
 +#define   GC_DISPLAY_CLOCK_133_MHZ_PNV   (6  4)
 +#define   GC_DISPLAY_CLOCK_167_MHZ_PNV   (7  4)
  #define   GC_DISPLAY_CLOCK_MASK  (7  4)
  #define   GM45_GC_RENDER_CLOCK_MASK  (0xf  0)
  #define   GM45_GC_RENDER_CLOCK_266_MHZ   (8  0)
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index b3389d7..3e66f05 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -4163,6 +4163,30 @@ static int i9xx_misc_get_display_clock_speed(struct 
 drm_device *dev)
   return 20;
  }
  
 +static int pnv_get_display_clock_speed(struct drm_device *dev)
 +{
 + u16 gcfgc = 0;
 +
 + pci_read_config_word(dev-pdev, GCFGC, gcfgc);
 +
 + switch (gcfgc  GC_DISPLAY_CLOCK_MASK) {
 + case GC_DISPLAY_CLOCK_267_MHZ_PNV:
 + return 267000;
 + case GC_DISPLAY_CLOCK_333_MHZ_PNV:
 + return 333000;
 + case GC_DISPLAY_CLOCK_444_MHZ_PNV:
 + return 444000;
 + case GC_DISPLAY_CLOCK_200_MHZ_PNV:
 + return 20;
 + default:
 + DRM_ERROR(Unknown pnv display core clock 0x%04x\n, gcfgc);

Reading the spec, should the default/fallback be 333 MHz for desktop?
Otherwise,

Reviewed-by: Jani Nikula jani.nik...@intel.com

 + case GC_DISPLAY_CLOCK_133_MHZ_PNV:
 + return 133000;
 + case GC_DISPLAY_CLOCK_167_MHZ_PNV:
 + return 167000;
 + }
 +}
 +
  static int i915gm_get_display_clock_speed(struct drm_device *dev)
  {
   u16 gcfgc = 0;
 @@ -9605,9 +9629,12 @@ static void intel_init_display(struct drm_device *dev)
   else if (IS_I915G(dev))
   dev_priv-display.get_display_clock_speed =
   i915_get_display_clock_speed;
 - else if (IS_I945GM(dev) || IS_845G(dev) || IS_PINEVIEW_M(dev))
 + else if (IS_I945GM(dev) || IS_845G(dev))
   dev_priv-display.get_display_clock_speed =
   i9xx_misc_get_display_clock_speed;
 + else if (IS_PINEVIEW(dev))
 + dev_priv-display.get_display_clock_speed =
 + pnv_get_display_clock_speed;
   else if (IS_I915GM(dev))
   dev_priv-display.get_display_clock_speed =
   i915gm_get_display_clock_speed;
 -- 
 1.8.3.2

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

-- 
Jani Nikula, 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] linux-next: Tree for Jul 25 [ call-trace: drm | drm-intel related? ]

[Sedat Dilek]

On Fri, Jul 26, 2013 at 9:26 AM, Chris Wilson ch...@chris-wilson.co.uk wrote:
 On Fri, Jul 26, 2013 at 09:15:14AM +0200, Sedat Dilek wrote:
 For example: I could start my X with even doing ugly hacks like this...

 [ intel-ddx (git) ]
 ...
 Bool intel_uxa_create_screen_resources(ScreenPtr screen)
 ...
 #if 0
   if (drm_intel_gem_bo_map_gtt(bo))
   return FALSE;
 #endif
 ...

 ...with any other kernel.

 Yes. Acquiring the map there is just a bit of paranoia to ensure we
 having the mapping into the scanout already in place in case of
 emergencies (and so don't fail along failure paths due to resource
 conflicts).

 Hmm, though we only started checking for map failures in 2.20.10 - which
 would explain why going back to the older ddx masks the issue. And yes,
 this means we do require a kernel bisect - or some passing inspiron.

First confirmation...
OK, next-20130726 shows the same symptoms!

I tried diverse intel-ddx release and went back to v2.21.9... and
searched for a version like v2.20.0 which has no checking for map
failures...

[ intel-ddx (v2.20.0) ]
...
Bool intel_uxa_create_screen_resources(ScreenPtr screen)
{
ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
intel_screen_private *intel = intel_get_screen_private(scrn);
dri_bo *bo = intel-front_buffer;

if (!uxa_resources_init(screen))
return FALSE;

drm_intel_gem_bo_map_gtt(bo);

if (intel-use_shadow) {
intel_shadow_create(intel);
} else {
PixmapPtr pixmap = screen-GetScreenPixmap(screen);
intel_set_pixmap_bo(pixmap, bo);
intel_get_pixmap_private(pixmap)-pinned = 1;
screen-ModifyPixmapHeader(pixmap,
   scrn-virtualX,
   scrn-virtualY,
   -1, -1,
   intel-front_pitch,
   NULL);
scrn-displayWidth = intel-front_pitch / intel-cpp;
}
...
... but does not start as well, so it seems to be a kernel-issue as
assumed (2nd confirmation).

X.log attached.

- Sedat -

 -Chris

 --
 Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] linux-next: Tree for Jul 25 [ call-trace: drm | drm-intel related? ]

[Chris Wilson]

On Fri, Jul 26, 2013 at 10:27:03AM +0200, Sedat Dilek wrote:
 On Fri, Jul 26, 2013 at 10:25 AM, Sedat Dilek sedat.di...@gmail.com wrote:
  ...
  ... but does not start as well, so it seems to be a kernel-issue as
  assumed (2nd confirmation).
 
  X.log attached.
 
 
 Now, really w/ promised attachment.

Yes, same failure (GTT mmaps) but at a later point, and UXA has no
fallback plan.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] linux-next: Tree for Jul 25 [ call-trace: drm | drm-intel related? ]

[Daniel Vetter]

On Fri, Jul 26, 2013 at 10:50 AM, Chris Wilson ch...@chris-wilson.co.uk wrote:
 On Fri, Jul 26, 2013 at 10:27:03AM +0200, Sedat Dilek wrote:
 On Fri, Jul 26, 2013 at 10:25 AM, Sedat Dilek sedat.di...@gmail.com wrote:
  ...
  ... but does not start as well, so it seems to be a kernel-issue as
  assumed (2nd confirmation).
 
  X.log attached.
 

 Now, really w/ promised attachment.

 Yes, same failure (GTT mmaps) but at a later point, and UXA has no
 fallback plan.

I'm running igt on my machines here to prep a new -next test cycle,
and gtt mmaps seem to fail across the board :( Currently I'd wager
that the vma offset manager is the culprit, since I've only recently
pulled that stuff in from drm-next.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] linux-next: Tree for Jul 25 [ call-trace: drm | drm-intel related? ]

[Sedat Dilek]

On Fri, Jul 26, 2013 at 10:52 AM, Daniel Vetter daniel.vet...@ffwll.ch wrote:
 On Fri, Jul 26, 2013 at 10:50 AM, Chris Wilson ch...@chris-wilson.co.uk 
 wrote:
 On Fri, Jul 26, 2013 at 10:27:03AM +0200, Sedat Dilek wrote:
 On Fri, Jul 26, 2013 at 10:25 AM, Sedat Dilek sedat.di...@gmail.com wrote:
  ...
  ... but does not start as well, so it seems to be a kernel-issue as
  assumed (2nd confirmation).
 
  X.log attached.
 

 Now, really w/ promised attachment.

 Yes, same failure (GTT mmaps) but at a later point, and UXA has no
 fallback plan.

 I'm running igt on my machines here to prep a new -next test cycle,
 and gtt mmaps seem to fail across the board :( Currently I'd wager
 that the vma offset manager is the culprit, since I've only recently
 pulled that stuff in from drm-next.

So, I resetted next-20130726 to the merge-commit of
drm-intel/for-linux-next, which is as expected bad.

First, I looked w/ a Grrr at i915_gem_stolen.c with a focus on
drm/i915: Use Graphics Base of Stolen Memory on all gen3+, but this
commit is also in next-20130725.

Yeah, might be the stuff comes from the merge including vma stuff.

- Sedat -

[1] 
http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/drivers/gpu/drm/i915/i915_gem_stolen.c?id=4867509d9269228f69273312298fea11f414ae54

- Sedat -

 -Daniel
 --
 Daniel Vetter
 Software Engineer, Intel Corporation
 +41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] linux-next: Tree for Jul 25 [ call-trace: drm | drm-intel related? ]

[Sedat Dilek]

On Fri, Jul 26, 2013 at 11:02 AM, Sedat Dilek sedat.di...@gmail.com wrote:
 On Fri, Jul 26, 2013 at 10:52 AM, Daniel Vetter daniel.vet...@ffwll.ch 
 wrote:
 On Fri, Jul 26, 2013 at 10:50 AM, Chris Wilson ch...@chris-wilson.co.uk 
 wrote:
 On Fri, Jul 26, 2013 at 10:27:03AM +0200, Sedat Dilek wrote:
 On Fri, Jul 26, 2013 at 10:25 AM, Sedat Dilek sedat.di...@gmail.com 
 wrote:
  ...
  ... but does not start as well, so it seems to be a kernel-issue as
  assumed (2nd confirmation).
 
  X.log attached.
 

 Now, really w/ promised attachment.

 Yes, same failure (GTT mmaps) but at a later point, and UXA has no
 fallback plan.

 I'm running igt on my machines here to prep a new -next test cycle,
 and gtt mmaps seem to fail across the board :( Currently I'd wager
 that the vma offset manager is the culprit, since I've only recently
 pulled that stuff in from drm-next.

 So, I resetted next-20130726 to the merge-commit of
 drm-intel/for-linux-next, which is as expected bad.

 First, I looked w/ a Grrr at i915_gem_stolen.c with a focus on
 drm/i915: Use Graphics Base of Stolen Memory on all gen3+, but this
 commit is also in next-20130725.

 Yeah, might be the stuff comes from the merge including vma stuff.

 - Sedat -

 [1] 
 http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/drivers/gpu/drm/i915/i915_gem_stolen.c?id=4867509d9269228f69273312298fea11f414ae54


I have compared next-20130725 VS. next-20130726:

$ head -2313 next-20130725-VS-next-20130726.diff | grep ^+ | grep i915
+  drm/i915: Colocate all GT access routines in the same file
+  drm/i915: Use a private interface for register access within GT
+  drm/i915: Use the common register access functions for NOTRACE variants
+  drm/i915: Squash gen lookup through multiple indirections
inside GT access
+  drm/i915: Convert the register access tracepoint to be conditional
+  drm/i915: fix the racy object accounting
+  drm/i915: dvo_ch7xxx: fix vsync polarity setting
+  drm/i915: initialize gt_lock early with other spin locks
+  Revert ACPI / video / i915: No ACPI backlight if firmware
expects Windows 8
+ drivers/gpu/drm/i915/Makefile  |2 +
+ drivers/gpu/drm/i915/dvo_ch7xxx.c  |2 +-
+ drivers/gpu/drm/i915/i915_debugfs.c|  836 +--
+ drivers/gpu/drm/i915/i915_dma.c|   91 +-
+ drivers/gpu/drm/i915/i915_drv.c|  292 +-
+ drivers/gpu/drm/i915/i915_drv.h|  413 +-
+ drivers/gpu/drm/i915/i915_gem.c|  411 +-
+ drivers/gpu/drm/i915/i915_irq.c| 1347 ++---
+ drivers/gpu/drm/i915/i915_trace.h  |   16 +-
+ drivers/gpu/drm/i915/intel_display.c   | 1065 ++--
+ drivers/gpu/drm/i915/intel_drv.h   |   36 +-
+ drivers/gpu/drm/i915/intel_pm.c|  431 +-
+ drivers/gpu/drm/i915/intel_uncore.c|  595 +++

Any hints which of the commits could be culprit?

- Sedat -

 - Sedat -

 -Daniel
 --
 Daniel Vetter
 Software Engineer, Intel Corporation
 +41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] linux-next: Tree for Jul 25 [ call-trace: drm | drm-intel related? ]

[Daniel Vetter]

On Fri, Jul 26, 2013 at 11:11 AM, Sedat Dilek sedat.di...@gmail.com wrote:

 I have compared next-20130725 VS. next-20130726:

 $ head -2313 next-20130725-VS-next-20130726.diff | grep ^+ | grep i915
 +  drm/i915: Colocate all GT access routines in the same file
 +  drm/i915: Use a private interface for register access within GT
 +  drm/i915: Use the common register access functions for NOTRACE variants
 +  drm/i915: Squash gen lookup through multiple indirections
 inside GT access
 +  drm/i915: Convert the register access tracepoint to be conditional
 +  drm/i915: fix the racy object accounting
 +  drm/i915: dvo_ch7xxx: fix vsync polarity setting
 +  drm/i915: initialize gt_lock early with other spin locks
 +  Revert ACPI / video / i915: No ACPI backlight if firmware
 expects Windows 8
 + drivers/gpu/drm/i915/Makefile  |2 +
 + drivers/gpu/drm/i915/dvo_ch7xxx.c  |2 +-
 + drivers/gpu/drm/i915/i915_debugfs.c|  836 +--
 + drivers/gpu/drm/i915/i915_dma.c|   91 +-
 + drivers/gpu/drm/i915/i915_drv.c|  292 +-
 + drivers/gpu/drm/i915/i915_drv.h|  413 +-
 + drivers/gpu/drm/i915/i915_gem.c|  411 +-
 + drivers/gpu/drm/i915/i915_irq.c| 1347 ++---
 + drivers/gpu/drm/i915/i915_trace.h  |   16 +-
 + drivers/gpu/drm/i915/intel_display.c   | 1065 ++--
 + drivers/gpu/drm/i915/intel_drv.h   |   36 +-
 + drivers/gpu/drm/i915/intel_pm.c|  431 +-
 + drivers/gpu/drm/i915/intel_uncore.c|  595 +++

 Any hints which of the commits could be culprit?

None, it's the vma offset manager stuff in drm-next.

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 3613b50..1f76572 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -666,7 +666,7 @@ int drm_gem_mmap(struct file *filp, struct
vm_area_struct *vma)
}

obj = container_of(node, struct drm_gem_object, vma_node);
-   ret = drm_gem_mmap_obj(obj, drm_vma_node_size(node), vma);
+   ret = drm_gem_mmap_obj(obj, drm_vma_node_size(node)  PAGE_SHIFT, vma);

mutex_unlock(dev-struct_mutex);


That should fix stuff up again, David Herrmann is working on a real
patch already.

Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: fix gen4 digital port hotplug definitions

[Daniel Vetter]

Apparently Bspec is wrong in this case here even for gm45. Note that
Bspec is horribly misguided on i965g/gm, so we don't have any other
data points besides that it seems to make machines work better.

With this changes all the bits in PORT_HOTPLUG_STAT for the digital
ports are ordered the same way. This seems to agree with what register
dumps from the hpd storm handling code shows, where the LIVE bit and
the short/long pulse STATUS bits light up at the same time with this
enumeration (but no with the one from Bspec).

Also tested on my gm45 which has two DP+ ports, and everything seems
to still work as expected.

References: 
http://www.mail-archive.com/intel-gfx@lists.freedesktop.org/msg23054.html
Cc: Egbert Eich e...@suse.com
Cc: Jan Niggemann j...@hz6.de
Tested-by: Jan Niggemann j...@hz6.de
Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
---
 drivers/gpu/drm/i915/i915_reg.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 6caa748..2d4c884 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1925,9 +1925,9 @@
 
 #define PORT_HOTPLUG_STAT  (dev_priv-info-display_mmio_offset + 0x61114)
 /* HDMI/DP bits are gen4+ */
-#define   PORTB_HOTPLUG_LIVE_STATUS   (1  29)
+#define   PORTD_HOTPLUG_LIVE_STATUS   (1  29)
 #define   PORTC_HOTPLUG_LIVE_STATUS   (1  28)
-#define   PORTD_HOTPLUG_LIVE_STATUS   (1  27)
+#define   PORTB_HOTPLUG_LIVE_STATUS   (1  27)
 #define   PORTD_HOTPLUG_INT_STATUS (3  21)
 #define   PORTC_HOTPLUG_INT_STATUS (3  19)
 #define   PORTB_HOTPLUG_INT_STATUS (3  17)
-- 
1.8.3.2

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

Re: [Intel-gfx] linux-next: Tree for Jul 25 [ call-trace: drm | drm-intel related? ]

[Sedat Dilek]

On Fri, Jul 26, 2013 at 11:16 AM, Daniel Vetter daniel.vet...@ffwll.ch wrote:
 On Fri, Jul 26, 2013 at 11:11 AM, Sedat Dilek sedat.di...@gmail.com wrote:

 I have compared next-20130725 VS. next-20130726:

 $ head -2313 next-20130725-VS-next-20130726.diff | grep ^+ | grep i915
 +  drm/i915: Colocate all GT access routines in the same file
 +  drm/i915: Use a private interface for register access within GT
 +  drm/i915: Use the common register access functions for NOTRACE 
 variants
 +  drm/i915: Squash gen lookup through multiple indirections
 inside GT access
 +  drm/i915: Convert the register access tracepoint to be conditional
 +  drm/i915: fix the racy object accounting
 +  drm/i915: dvo_ch7xxx: fix vsync polarity setting
 +  drm/i915: initialize gt_lock early with other spin locks
 +  Revert ACPI / video / i915: No ACPI backlight if firmware
 expects Windows 8
 + drivers/gpu/drm/i915/Makefile  |2 +
 + drivers/gpu/drm/i915/dvo_ch7xxx.c  |2 +-
 + drivers/gpu/drm/i915/i915_debugfs.c|  836 +--
 + drivers/gpu/drm/i915/i915_dma.c|   91 +-
 + drivers/gpu/drm/i915/i915_drv.c|  292 +-
 + drivers/gpu/drm/i915/i915_drv.h|  413 +-
 + drivers/gpu/drm/i915/i915_gem.c|  411 +-
 + drivers/gpu/drm/i915/i915_irq.c| 1347 ++---
 + drivers/gpu/drm/i915/i915_trace.h  |   16 +-
 + drivers/gpu/drm/i915/intel_display.c   | 1065 ++--
 + drivers/gpu/drm/i915/intel_drv.h   |   36 +-
 + drivers/gpu/drm/i915/intel_pm.c|  431 +-
 + drivers/gpu/drm/i915/intel_uncore.c|  595 +++

 Any hints which of the commits could be culprit?

 None, it's the vma offset manager stuff in drm-next.

 diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
 index 3613b50..1f76572 100644
 --- a/drivers/gpu/drm/drm_gem.c
 +++ b/drivers/gpu/drm/drm_gem.c
 @@ -666,7 +666,7 @@ int drm_gem_mmap(struct file *filp, struct
 vm_area_struct *vma)
 }

 obj = container_of(node, struct drm_gem_object, vma_node);
 -   ret = drm_gem_mmap_obj(obj, drm_vma_node_size(node), vma);
 +   ret = drm_gem_mmap_obj(obj, drm_vma_node_size(node)  PAGE_SHIFT, 
 vma);

 mutex_unlock(dev-struct_mutex);


 That should fix stuff up again, David Herrmann is working on a real
 patch already.


The fix can be found at [1].

- Sedat -

[1] https://patchwork.kernel.org/patch/2833932/

 Cheers, Daniel
 --
 Daniel Vetter
 Software Engineer, Intel Corporation
 +41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: fix gen4 digital port hotplug definitions

[Jani Nikula]

On Fri, 26 Jul 2013, Daniel Vetter daniel.vet...@ffwll.ch wrote:
 Apparently Bspec is wrong in this case here even for gm45. Note that
 Bspec is horribly misguided on i965g/gm, so we don't have any other
 data points besides that it seems to make machines work better.

 With this changes all the bits in PORT_HOTPLUG_STAT for the digital
 ports are ordered the same way. This seems to agree with what register
 dumps from the hpd storm handling code shows, where the LIVE bit and
 the short/long pulse STATUS bits light up at the same time with this
 enumeration (but no with the one from Bspec).

Would a comment about this near the #defines be in order? To avoid the
these values are all wrong per bspec patches.

Cheers,
Jani.

 Also tested on my gm45 which has two DP+ ports, and everything seems
 to still work as expected.

 References: 
 http://www.mail-archive.com/intel-gfx@lists.freedesktop.org/msg23054.html
 Cc: Egbert Eich e...@suse.com
 Cc: Jan Niggemann j...@hz6.de
 Tested-by: Jan Niggemann j...@hz6.de
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 ---
  drivers/gpu/drm/i915/i915_reg.h | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

 diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
 index 6caa748..2d4c884 100644
 --- a/drivers/gpu/drm/i915/i915_reg.h
 +++ b/drivers/gpu/drm/i915/i915_reg.h
 @@ -1925,9 +1925,9 @@
  
  #define PORT_HOTPLUG_STAT(dev_priv-info-display_mmio_offset + 0x61114)
  /* HDMI/DP bits are gen4+ */
 -#define   PORTB_HOTPLUG_LIVE_STATUS   (1  29)
 +#define   PORTD_HOTPLUG_LIVE_STATUS   (1  29)
  #define   PORTC_HOTPLUG_LIVE_STATUS   (1  28)
 -#define   PORTD_HOTPLUG_LIVE_STATUS   (1  27)
 +#define   PORTB_HOTPLUG_LIVE_STATUS   (1  27)
  #define   PORTD_HOTPLUG_INT_STATUS   (3  21)
  #define   PORTC_HOTPLUG_INT_STATUS   (3  19)
  #define   PORTB_HOTPLUG_INT_STATUS   (3  17)
 -- 
 1.8.3.2

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

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

[Intel-gfx] [PATCH] drm/i915: Add messages usful for HPD strom detection debugging

[Egbert Eich]

For HPD storm detection we now mask out individual interrupt source
bits. We have already seen a case where HPD interrupt enable bits
were assigned to the wrong pins. To track these conditions more
easily add some debugging messages.

Signed-off-by: Egbert Eich e...@suse.de
---
 drivers/gpu/drm/i915/i915_irq.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index ee3e49c..1f3a971 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -919,6 +919,10 @@ static inline void intel_hpd_irq_handler(struct drm_device 
*dev,
spin_lock(dev_priv-irq_lock);
for (i = 1; i  HPD_NUM_PINS; i++) {
 
+   WARN(((hpd[i]  hotplug_trigger) 
+ dev_priv-hpd_stats[i].hpd_mark != HPD_ENABLED),
+Received HPD intterupt although disabled\n);
+
if (!(hpd[i]  hotplug_trigger) ||
dev_priv-hpd_stats[i].hpd_mark != HPD_ENABLED)
continue;
@@ -929,6 +933,7 @@ static inline void intel_hpd_irq_handler(struct drm_device 
*dev,
   + 
msecs_to_jiffies(HPD_STORM_DETECT_PERIOD))) {
dev_priv-hpd_stats[i].hpd_last_jiffies = jiffies;
dev_priv-hpd_stats[i].hpd_cnt = 0;
+   DRM_DEBUG_KMS(Received HPD interrupt on PIN %d - cnt: 
0\n, i);
} else if (dev_priv-hpd_stats[i].hpd_cnt  
HPD_STORM_THRESHOLD) {
dev_priv-hpd_stats[i].hpd_mark = HPD_MARK_DISABLED;
dev_priv-hpd_event_bits = ~(1  i);
@@ -936,6 +941,8 @@ static inline void intel_hpd_irq_handler(struct drm_device 
*dev,
storm_detected = true;
} else {
dev_priv-hpd_stats[i].hpd_cnt++;
+   DRM_DEBUG_KMS(Received HPD interrupt on PIN %d - cnt: 
%d\n, i,
+ dev_priv-hpd_stats[i].hpd_cnt);
}
}
 
-- 
1.8.1.4

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

[Intel-gfx] [PATCH] drm/i915: Make sure PORT_HOTPLUG_EN is written

[Egbert Eich]

Add posting read to make sure PORT_HOTPLUG_EN is written in
i915_hpd_irq_setup().

Signed-off-by: Egbert Eich e...@suse.de
---
 drivers/gpu/drm/i915/i915_irq.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 1f3a971..a38372b 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2805,6 +2805,7 @@ static void i915_hpd_irq_setup(struct drm_device *dev)
 
/* Ignore TV since it's buggy */
I915_WRITE(PORT_HOTPLUG_EN, hotplug_en);
+   POSTING_READ(PORT_HOTPLUG_EN);
}
 }
 
-- 
1.8.1.4

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

Re: [Intel-gfx] linux-next: Tree for Jul 25 [ call-trace: drm | drm-intel related? ]

[Sedat Dilek]

On Fri, Jul 26, 2013 at 11:32 AM, Sedat Dilek sedat.di...@gmail.com wrote:
 On Fri, Jul 26, 2013 at 11:16 AM, Daniel Vetter daniel.vet...@ffwll.ch 
 wrote:
 On Fri, Jul 26, 2013 at 11:11 AM, Sedat Dilek sedat.di...@gmail.com wrote:

 I have compared next-20130725 VS. next-20130726:

 $ head -2313 next-20130725-VS-next-20130726.diff | grep ^+ | grep i915
 +  drm/i915: Colocate all GT access routines in the same file
 +  drm/i915: Use a private interface for register access within GT
 +  drm/i915: Use the common register access functions for NOTRACE 
 variants
 +  drm/i915: Squash gen lookup through multiple indirections
 inside GT access
 +  drm/i915: Convert the register access tracepoint to be conditional
 +  drm/i915: fix the racy object accounting
 +  drm/i915: dvo_ch7xxx: fix vsync polarity setting
 +  drm/i915: initialize gt_lock early with other spin locks
 +  Revert ACPI / video / i915: No ACPI backlight if firmware
 expects Windows 8
 + drivers/gpu/drm/i915/Makefile  |2 +
 + drivers/gpu/drm/i915/dvo_ch7xxx.c  |2 +-
 + drivers/gpu/drm/i915/i915_debugfs.c|  836 +--
 + drivers/gpu/drm/i915/i915_dma.c|   91 +-
 + drivers/gpu/drm/i915/i915_drv.c|  292 +-
 + drivers/gpu/drm/i915/i915_drv.h|  413 +-
 + drivers/gpu/drm/i915/i915_gem.c|  411 +-
 + drivers/gpu/drm/i915/i915_irq.c| 1347 ++---
 + drivers/gpu/drm/i915/i915_trace.h  |   16 +-
 + drivers/gpu/drm/i915/intel_display.c   | 1065 ++--
 + drivers/gpu/drm/i915/intel_drv.h   |   36 +-
 + drivers/gpu/drm/i915/intel_pm.c|  431 +-
 + drivers/gpu/drm/i915/intel_uncore.c|  595 +++

 Any hints which of the commits could be culprit?

 None, it's the vma offset manager stuff in drm-next.

 diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
 index 3613b50..1f76572 100644
 --- a/drivers/gpu/drm/drm_gem.c
 +++ b/drivers/gpu/drm/drm_gem.c
 @@ -666,7 +666,7 @@ int drm_gem_mmap(struct file *filp, struct
 vm_area_struct *vma)
 }

 obj = container_of(node, struct drm_gem_object, vma_node);
 -   ret = drm_gem_mmap_obj(obj, drm_vma_node_size(node), vma);
 +   ret = drm_gem_mmap_obj(obj, drm_vma_node_size(node)  PAGE_SHIFT, 
 vma);

 mutex_unlock(dev-struct_mutex);


 That should fix stuff up again, David Herrmann is working on a real
 patch already.


 The fix can be found at [1].


v2: https://patchwork.kernel.org/patch/2833959/

- Sedat -

 - Sedat -

 [1] https://patchwork.kernel.org/patch/2833932/

 Cheers, Daniel
 --
 Daniel Vetter
 Software Engineer, Intel Corporation
 +41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] Linux 3.11-rc2 (acpi backlight, revert)

[Igor Gnatenko]

On Thu, 2013-07-25 at 21:47 +0200, Rafael J. Wysocki wrote:
 On Thursday, July 25, 2013 08:14:08 PM James Hogan wrote:
  On 25 July 2013 14:00, Rafael J. Wysocki r...@sisk.pl wrote:
   On Monday, July 22, 2013 09:54:21 PM Rafael J. Wysocki wrote:
   On Monday, July 22, 2013 11:11:54 AM Linus Torvalds wrote:
On Mon, Jul 22, 2013 at 6:02 AM, Rafael J. Wysocki r...@sisk.pl 
wrote:

 Linus, do you want me to send a pull request reverting 8c5bd7a and 
 efaa14c?
   
Yes, but let's wait a while. Not because I think we'll fix the problem
(hey, miracles might happen), but because I think it would be useful
to couple the reverts with information about the particular machines
that broke (and the people who reported it). So that when we
inevitably try again, we can perhaps get some testing effort with
those machines/people.
   
It doesn't seem to be a show-stopped for a large number of people, so
there's no huge hurry. I'd suggest doing the revert just in time for
rc3, but waiting until then to gather info about people who see
breakage.
   
Sound like a plan?
  
   Yes, it does.
  
   OK, time to revert I guess.
  
   James, Kamal, Steven, Jörg, Martin, Kalle, please check if the apppended 
   patch
   fixes the backlight for you.
  
  Works for me
 
 Great!
 
 James, Kamal, Jörg, thanks for confirmations.  I'll tentatively put the revert
 into linux-next in a while.
 
 Other people who experienced problems with backlight in 3.11-rc2, please let 
 me
 know whether or not the revert works for you too if you can.
 
 Rafael
 
 

Rafael, feel free to CC me in messages with backlight ;) I want to test
its)
-- 
Igor Gnatenko
Fedora release 19 (Schrödinger’s Cat)
Linux 3.9.9-302.fc19.x86_64

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

Re: [Intel-gfx] [PATCH] drm/i915: Add messages usful for HPD strom detection debugging

[Jani Nikula]

On Fri, 26 Jul 2013, Egbert Eich e...@suse.de wrote:
 For HPD storm detection we now mask out individual interrupt source
 bits. We have already seen a case where HPD interrupt enable bits
 were assigned to the wrong pins. To track these conditions more
 easily add some debugging messages.

The code seems okay, please spellcheck the subject and debug prints! :)

BR,
Jani.


 Signed-off-by: Egbert Eich e...@suse.de
 ---
  drivers/gpu/drm/i915/i915_irq.c | 7 +++
  1 file changed, 7 insertions(+)

 diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
 index ee3e49c..1f3a971 100644
 --- a/drivers/gpu/drm/i915/i915_irq.c
 +++ b/drivers/gpu/drm/i915/i915_irq.c
 @@ -919,6 +919,10 @@ static inline void intel_hpd_irq_handler(struct 
 drm_device *dev,
   spin_lock(dev_priv-irq_lock);
   for (i = 1; i  HPD_NUM_PINS; i++) {
  
 + WARN(((hpd[i]  hotplug_trigger) 
 +   dev_priv-hpd_stats[i].hpd_mark != HPD_ENABLED),
 +  Received HPD intterupt although disabled\n);
 +
   if (!(hpd[i]  hotplug_trigger) ||
   dev_priv-hpd_stats[i].hpd_mark != HPD_ENABLED)
   continue;
 @@ -929,6 +933,7 @@ static inline void intel_hpd_irq_handler(struct 
 drm_device *dev,
  + 
 msecs_to_jiffies(HPD_STORM_DETECT_PERIOD))) {
   dev_priv-hpd_stats[i].hpd_last_jiffies = jiffies;
   dev_priv-hpd_stats[i].hpd_cnt = 0;
 + DRM_DEBUG_KMS(Received HPD interrupt on PIN %d - cnt: 
 0\n, i);
   } else if (dev_priv-hpd_stats[i].hpd_cnt  
 HPD_STORM_THRESHOLD) {
   dev_priv-hpd_stats[i].hpd_mark = HPD_MARK_DISABLED;
   dev_priv-hpd_event_bits = ~(1  i);
 @@ -936,6 +941,8 @@ static inline void intel_hpd_irq_handler(struct 
 drm_device *dev,
   storm_detected = true;
   } else {
   dev_priv-hpd_stats[i].hpd_cnt++;
 + DRM_DEBUG_KMS(Received HPD interrupt on PIN %d - cnt: 
 %d\n, i,
 +   dev_priv-hpd_stats[i].hpd_cnt);
   }
   }
  
 -- 
 1.8.1.4

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

-- 
Jani Nikula, 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: Retry DP aux_ch communications with a different clock after failure

[Jani Nikula]

On Sun, 21 Jul 2013, Chris Wilson ch...@chris-wilson.co.uk wrote:
 The w/a db makes the recommendation to both use a non-default value for
 the initial clock and then to retry with an alternative clock for
 Haswell with the Lakeport PCH.

 On LPT:H, use a divider value of 63 decimal (03Fh). If there is a
 failure, retry at least three times with 63, then retry at least three
 times with 72 decimal (048h).

Overall I'd prefer adding the outer repeat loop first, keeping existing
behaviour, and only then adding the divider value changes in
get_aux_clock_divider(). But that's in the bikeshedding dept. Either
way,

Reviewed-by: Jani Nikula jani.nik...@intel.com

 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 ---
  drivers/gpu/drm/i915/intel_dp.c | 92 
 +++--
  1 file changed, 51 insertions(+), 41 deletions(-)

 diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
 index c6996ce..4a7ba5e 100644
 --- a/drivers/gpu/drm/i915/intel_dp.c
 +++ b/drivers/gpu/drm/i915/intel_dp.c
 @@ -276,7 +276,8 @@ intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool 
 has_aux_irq)
   return status;
  }
  
 -static uint32_t get_aux_clock_divider(struct intel_dp *intel_dp)
 +static uint32_t get_aux_clock_divider(struct intel_dp *intel_dp,
 +   int index)
  {
   struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
   struct drm_device *dev = intel_dig_port-base.base.dev;
 @@ -290,22 +291,27 @@ static uint32_t get_aux_clock_divider(struct intel_dp 
 *intel_dp)
* clock divider.
*/
   if (IS_VALLEYVIEW(dev)) {
 - return 100;
 + return index ? 0 : 100;
   } else if (intel_dig_port-port == PORT_A) {
 + if (index)
 + return 0;
   if (HAS_DDI(dev))
 - return DIV_ROUND_CLOSEST(
 - intel_ddi_get_cdclk_freq(dev_priv), 2000);
 + return 
 DIV_ROUND_CLOSEST(intel_ddi_get_cdclk_freq(dev_priv), 2000);
   else if (IS_GEN6(dev) || IS_GEN7(dev))
   return 200; /* SNB  IVB eDP input clock at 400Mhz */
   else
   return 225; /* eDP input clock at 450Mhz */
   } else if (dev_priv-pch_id == INTEL_PCH_LPT_DEVICE_ID_TYPE) {
   /* Workaround for non-ULT HSW */
 - return 74;
 + switch (index) {
 + case 0: return 63;
 + case 1: return 72;
 + default: return 0;
 + }
   } else if (HAS_PCH_SPLIT(dev)) {
 - return DIV_ROUND_UP(intel_pch_rawclk(dev), 2);
 + return index ? 0 : DIV_ROUND_UP(intel_pch_rawclk(dev), 2);
   } else {
 - return intel_hrawclk(dev) / 2;
 + return index ? 0 :intel_hrawclk(dev) / 2;
   }
  }
  
 @@ -319,10 +325,10 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
   struct drm_i915_private *dev_priv = dev-dev_private;
   uint32_t ch_ctl = intel_dp-aux_ch_ctl_reg;
   uint32_t ch_data = ch_ctl + 4;
 + uint32_t aux_clock_divider;
   int i, ret, recv_bytes;
   uint32_t status;
 - uint32_t aux_clock_divider = get_aux_clock_divider(intel_dp);
 - int try, precharge;
 + int try, precharge, clock = 0;
   bool has_aux_irq = INTEL_INFO(dev)-gen = 5  !IS_VALLEYVIEW(dev);
  
   /* dp aux is extremely sensitive to irq latency, hence request the
 @@ -353,37 +359,41 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
   goto out;
   }
  
 - /* Must try at least 3 times according to DP spec */
 - for (try = 0; try  5; try++) {
 - /* Load the send data into the aux channel data registers */
 - for (i = 0; i  send_bytes; i += 4)
 - I915_WRITE(ch_data + i,
 -pack_aux(send + i, send_bytes - i));
 -
 - /* Send the command and wait for it to complete */
 - I915_WRITE(ch_ctl,
 -DP_AUX_CH_CTL_SEND_BUSY |
 -(has_aux_irq ? DP_AUX_CH_CTL_INTERRUPT : 0) |
 -DP_AUX_CH_CTL_TIME_OUT_400us |
 -(send_bytes  DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
 -(precharge  DP_AUX_CH_CTL_PRECHARGE_2US_SHIFT) |
 -(aux_clock_divider  
 DP_AUX_CH_CTL_BIT_CLOCK_2X_SHIFT) |
 -DP_AUX_CH_CTL_DONE |
 -DP_AUX_CH_CTL_TIME_OUT_ERROR |
 -DP_AUX_CH_CTL_RECEIVE_ERROR);
 -
 - status = intel_dp_aux_wait_done(intel_dp, has_aux_irq);
 -
 - /* Clear done status and any errors */
 - I915_WRITE(ch_ctl,
 -status |
 -DP_AUX_CH_CTL_DONE |
 -DP_AUX_CH_CTL_TIME_OUT_ERROR |
 -DP_AUX_CH_CTL_RECEIVE_ERROR);
 -
 - if 

Re: [Intel-gfx] [PATCH] drm/i915: Make sure PORT_HOTPLUG_EN is written

[Chris Wilson]

On Fri, Jul 26, 2013 at 12:31:30PM +0200, Egbert Eich wrote:
 Add posting read to make sure PORT_HOTPLUG_EN is written in
 i915_hpd_irq_setup().

It's not vital that the write be flushed here. On the deferred reenable
path a further delay in rearming is not significant, and inside the irq
handler (for disabling) the write will be flushed.

In terms of the patch itself, you would also need to fixup
ibx_hpd_irq_setup as well.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Make sure PORT_HOTPLUG_EN is written

[Egbert Eich]

Chris Wilson writes:
  On Fri, Jul 26, 2013 at 12:31:30PM +0200, Egbert Eich wrote:
   Add posting read to make sure PORT_HOTPLUG_EN is written in
   i915_hpd_irq_setup().
  
  It's not vital that the write be flushed here. On the deferred reenable
  path a further delay in rearming is not significant, and inside the irq
  handler (for disabling) the write will be flushed.
  
  In terms of the patch itself, you would also need to fixup
  ibx_hpd_irq_setup as well.
  -Chris

We can probably drop this patch. It was one try figuring why HPD interrupts
are still seeen on pins which have been disabled. It turned out that the
root cause was a mixup in the mapping of pins to enable bits due to incorrect
documentation.

Cheers,
Egbert.

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

[Intel-gfx] [PATCH V2] drm/i915: Add messages useful for HPD storm detection debugging (v2)

[Egbert Eich]

For HPD storm detection we now mask out individual interrupt source
bits. We have already seen a case where HPD interrupt enable bits
were assigned to the wrong pins. To track these conditions more
easily add some debugging messages.

v2: Spelling fixes as suggested by Jani Nikula jani.nik...@linux.intel.com

Signed-off-by: Egbert Eich e...@suse.de
---
 drivers/gpu/drm/i915/i915_irq.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index ee3e49c..6a1c207 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -919,6 +919,10 @@ static inline void intel_hpd_irq_handler(struct drm_device 
*dev,
spin_lock(dev_priv-irq_lock);
for (i = 1; i  HPD_NUM_PINS; i++) {
 
+   WARN(((hpd[i]  hotplug_trigger) 
+ dev_priv-hpd_stats[i].hpd_mark != HPD_ENABLED),
+Received HPD interrupt although disabled\n);
+
if (!(hpd[i]  hotplug_trigger) ||
dev_priv-hpd_stats[i].hpd_mark != HPD_ENABLED)
continue;
@@ -929,6 +933,7 @@ static inline void intel_hpd_irq_handler(struct drm_device 
*dev,
   + 
msecs_to_jiffies(HPD_STORM_DETECT_PERIOD))) {
dev_priv-hpd_stats[i].hpd_last_jiffies = jiffies;
dev_priv-hpd_stats[i].hpd_cnt = 0;
+   DRM_DEBUG_KMS(Received HPD interrupt on PIN %d - cnt: 
0\n, i);
} else if (dev_priv-hpd_stats[i].hpd_cnt  
HPD_STORM_THRESHOLD) {
dev_priv-hpd_stats[i].hpd_mark = HPD_MARK_DISABLED;
dev_priv-hpd_event_bits = ~(1  i);
@@ -936,6 +941,8 @@ static inline void intel_hpd_irq_handler(struct drm_device 
*dev,
storm_detected = true;
} else {
dev_priv-hpd_stats[i].hpd_cnt++;
+   DRM_DEBUG_KMS(Received HPD interrupt on PIN %d - cnt: 
%d\n, i,
+ dev_priv-hpd_stats[i].hpd_cnt);
}
}
 
-- 
1.8.1.4

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

Re: [Intel-gfx] [PATCH V2] drm/i915: Add messages useful for HPD storm detection debugging (v2)

[Jani Nikula]

On Fri, 26 Jul 2013, Egbert Eich e...@suse.de wrote:
 For HPD storm detection we now mask out individual interrupt source
 bits. We have already seen a case where HPD interrupt enable bits
 were assigned to the wrong pins. To track these conditions more
 easily add some debugging messages.

 v2: Spelling fixes as suggested by Jani Nikula jani.nik...@linux.intel.com

Reviewed-by: Jani Nikula jani.nik...@intel.com

 Signed-off-by: Egbert Eich e...@suse.de
 ---
  drivers/gpu/drm/i915/i915_irq.c | 7 +++
  1 file changed, 7 insertions(+)

 diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
 index ee3e49c..6a1c207 100644
 --- a/drivers/gpu/drm/i915/i915_irq.c
 +++ b/drivers/gpu/drm/i915/i915_irq.c
 @@ -919,6 +919,10 @@ static inline void intel_hpd_irq_handler(struct 
 drm_device *dev,
   spin_lock(dev_priv-irq_lock);
   for (i = 1; i  HPD_NUM_PINS; i++) {
  
 + WARN(((hpd[i]  hotplug_trigger) 
 +   dev_priv-hpd_stats[i].hpd_mark != HPD_ENABLED),
 +  Received HPD interrupt although disabled\n);
 +
   if (!(hpd[i]  hotplug_trigger) ||
   dev_priv-hpd_stats[i].hpd_mark != HPD_ENABLED)
   continue;
 @@ -929,6 +933,7 @@ static inline void intel_hpd_irq_handler(struct 
 drm_device *dev,
  + 
 msecs_to_jiffies(HPD_STORM_DETECT_PERIOD))) {
   dev_priv-hpd_stats[i].hpd_last_jiffies = jiffies;
   dev_priv-hpd_stats[i].hpd_cnt = 0;
 + DRM_DEBUG_KMS(Received HPD interrupt on PIN %d - cnt: 
 0\n, i);
   } else if (dev_priv-hpd_stats[i].hpd_cnt  
 HPD_STORM_THRESHOLD) {
   dev_priv-hpd_stats[i].hpd_mark = HPD_MARK_DISABLED;
   dev_priv-hpd_event_bits = ~(1  i);
 @@ -936,6 +941,8 @@ static inline void intel_hpd_irq_handler(struct 
 drm_device *dev,
   storm_detected = true;
   } else {
   dev_priv-hpd_stats[i].hpd_cnt++;
 + DRM_DEBUG_KMS(Received HPD interrupt on PIN %d - cnt: 
 %d\n, i,
 +   dev_priv-hpd_stats[i].hpd_cnt);
   }
   }
  
 -- 
 1.8.1.4


-- 
Jani Nikula, 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] Linux 3.11-rc2 (acpi backlight, revert)

[Rafael J. Wysocki]

On Friday, July 26, 2013 02:09:08 PM Martin Steigerwald wrote:
 Am Donnerstag, 25. Juli 2013, 15:00:26 schrieb Rafael J. Wysocki:
  On Monday, July 22, 2013 09:54:21 PM Rafael J. Wysocki wrote:
   On Monday, July 22, 2013 11:11:54 AM Linus Torvalds wrote:
On Mon, Jul 22, 2013 at 6:02 AM, Rafael J. Wysocki r...@sisk.pl wrote:
 Linus, do you want me to send a pull request reverting 8c5bd7a and
 efaa14c?

Yes, but let's wait a while. Not because I think we'll fix the problem
(hey, miracles might happen), but because I think it would be useful
to couple the reverts with information about the particular machines
that broke (and the people who reported it). So that when we
inevitably try again, we can perhaps get some testing effort with
those machines/people.

It doesn't seem to be a show-stopped for a large number of people, so
there's no huge hurry. I'd suggest doing the revert just in time for
rc3, but waiting until then to gather info about people who see
breakage.

Sound like a plan?
   
   Yes, it does.
  
  OK, time to revert I guess.
  
  James, Kamal, Steven, Jörg, Martin, Kalle, please check if the apppended
  patch fixes the backlight for you.
 
 Rafael, do you still need more testing urgently? Otherwise I´d wait till its 
 in some next 3.11 rc and test then.

Well, it seems to work for everybody else (Steven, Joerg, thanks for your
reports!), so I don't think you need to test it urgently.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, 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] [Update][PATCH 0/3] Fix backlight issues on some Windows 8 systems

[Jani Nikula]

[distribution massively reduced]

On Sat, 20 Jul 2013, Felipe Contreras felipe.contre...@gmail.com wrote:
 I tried this patch series and it's as I expected, it's the same as
 acpi_backlight=vendor, and the intel backlight driver doesn't work
 correctly in this machine. If you are actually serious about the
 mantra of no user-space regressions, then for this machine at least,
 you need to use the ACPI backlight with Windows8 OSI disabled, until
 the intel backlight driver is fixed. My patch does that:

 http://article.gmane.org/gmane.linux.acpi.devel/60969

Hi Felipe, would you mind trying the following patch on top of -rc2,
without your quirk patch? Up front, I'm not sure what all your failure
modes are, I'm not claiming this fixes them, and frankly, it didn't do
anything on a machine I tried... I just have this hunch it might help
with the ACPI hotkeys on some BIOSes. Maybe you're the lucky one! :)

BR,
Jani.


From eb1493a7551dafb4bac3d65e4ed3ecd2e46f7f67 Mon Sep 17 00:00:00 2001
From: Jani Nikula jani.nik...@intel.com
Date: Fri, 7 Dec 2012 15:32:45 +0200
Subject: [PATCH] drm/i915: set the BIOS current backlight reference level
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
Cc: Jani Nikula jani.nik...@intel.com

This should keep BIOS brightness keys and i915 backlight device better in
sync.

Signed-off-by: Jani Nikula jani.nik...@intel.com
---
 drivers/gpu/drm/i915/i915_drv.h   |3 +++
 drivers/gpu/drm/i915/intel_opregion.c |   20 +++-
 drivers/gpu/drm/i915/intel_panel.c|3 +++
 3 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 82ea281..5a1cfa3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2059,10 +2059,13 @@ extern int intel_opregion_setup(struct drm_device *dev);
 extern void intel_opregion_init(struct drm_device *dev);
 extern void intel_opregion_fini(struct drm_device *dev);
 extern void intel_opregion_asle_intr(struct drm_device *dev);
+extern void intel_opregion_asle_set_cblv(struct drm_device *dev, u32 level, 
u32 max);
 #else
 static inline void intel_opregion_init(struct drm_device *dev) { return; }
 static inline void intel_opregion_fini(struct drm_device *dev) { return; }
 static inline void intel_opregion_asle_intr(struct drm_device *dev) { return; }
+}
+static inline void intel_opregion_asle_set_cblv(struct drm_device *dev, u32 
level, u32 max) { return; }
 #endif
 
 /* intel_acpi.c */
diff --git a/drivers/gpu/drm/i915/intel_opregion.c 
b/drivers/gpu/drm/i915/intel_opregion.c
index cfb8fb6..00fce49 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -158,6 +158,25 @@ struct opregion_asle {
 #define ACPI_LVDS_OUTPUT (48)
 
 #ifdef CONFIG_ACPI
+
+/* set backlight brightness reference to level in range [0..max] */
+void intel_opregion_asle_set_cblv(struct drm_device *dev, u32 level, u32 max)
+{
+   struct drm_i915_private *dev_priv = dev-dev_private;
+   struct opregion_asle __iomem *asle = dev_priv-opregion.asle;
+
+   if (!asle)
+   return;
+
+   if (level  max)
+   level = max;
+
+   /* scale to 0...100 per opregion spec */
+   level = level * 100 / max;
+
+   iowrite32(level | ASLE_CBLV_VALID, asle-cblv);
+}
+
 static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
 {
struct drm_i915_private *dev_priv = dev-dev_private;
@@ -173,7 +192,6 @@ static u32 asle_set_backlight(struct drm_device *dev, u32 
bclp)
return ASLE_BACKLIGHT_FAILED;
 
intel_panel_set_backlight(dev, bclp, 255);
-   iowrite32((bclp*0x64)/0xff | ASLE_CBLV_VALID, asle-cblv);
 
return 0;
 }
diff --git a/drivers/gpu/drm/i915/intel_panel.c 
b/drivers/gpu/drm/i915/intel_panel.c
index 67e2c1f..a497e20 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -497,6 +497,9 @@ void intel_panel_set_backlight(struct drm_device *dev, u32 
level, u32 max)
goto out;
}
 
+   /* set brightness reference value for bios hotkeys */
+   intel_opregion_asle_set_cblv(dev, level, max);
+
/* scale to hardware */
level = level * freq / max;
 
-- 
1.7.9.5

-- 
Jani Nikula, 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] Ugly patches for stolen reservation

[Jesse Barnes]

On Thu, 25 Jul 2013 20:31:27 -0700
H. Peter Anvin h...@zytor.com wrote:

 On 07/25/2013 07:14 PM, Jesse Barnes wrote:
  To clarify: it'll either be marked reserved or not listed at all in e820, 
  which is why I did this early, before any other e820 stuff like the RAM 
  buffer are allocated, and before we could use the iomem resource (or maybe 
  we could even early per Linus? I'll check). 
  
  Jesse
 
 If it is marked reserved or not listed at all it is much less of an
 issue.  Reserved is in fact the correct thing; not listed at all really
 isn't very problematic in most cases.

Yeah the problems seem to fall into two categories:
  1) mmio space is allocated in this range:
 https://bugs.freedesktop.org/show_bug.cgi?id=66726
  2) range gets partially allocated to the RAM buffer
 https://bugs.freedesktop.org/show_bug.cgi?id=66844

Case (1) is the one that worries me.  I'm guessing it'll mainly be a
problem on machines where MMIO space is limited or somehow structured
such that PCI resources end up there when we allocate them.  Depending
on what gets put there and the decode priority, behavior may be poor.

Case (2) isn't harmful, but ends up causing our driver to skip stolen
memory initialization, because of the conflict.

Anyway I'll look at Linus's suggestion of reserving in the iomem
resource really early and roll in Chris's stuff if that doesn't work
out.

Thanks,
-- 
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] Ugly patches for stolen reservation

[Jesse Barnes]

On Thu, 25 Jul 2013 17:31:29 -0700
Linus Torvalds torva...@linux-foundation.org wrote:

 On Thu, Jul 25, 2013 at 3:42 PM, H. Peter Anvin h...@zytor.com wrote:
  So the bootloader is just as likely to step on things... what happens 
  when/if it does?
 
 This isn't a new problem. We've had this firmware tables don't show
 all devices issue before.
 
 The only odd thing about this one is how the quirk in question uses
 e820_add_region() instead of just adding things to the MMIO list.
 And I think that's actually likely a mistake.
 
 So Jesse, why don't you do what the other quirks do, and claim an
 actual MMIO resource? If you make it a real resource, you'll get to
 use fancy things like REAL NAMES, and actually document it. With
 human-readable strings.
 
 See quirk_io_region() in drivers/pci/quirks.c for example. The same
 code except for IORESOURCE_MEM should do a lovely job..
 
 And even *if* it's already marked reserved in the e820 table, it just
 looks nice in /proc/iomem.
 
 Hmm?

I should have mentioned yesterday that we *do* try to reserve the
resource in our driver init, with pretty name and everything.

The issue here is making sure we are actually *able* to reserve it
without conflicts at driver init time.

If the PCI layer has put something there (misc MMIO or the RAM
buffer intended to prevent stuff like this) because it's not listed in
the E820 map, we'll fail to get at this memory in our driver init.

Thus the early reservation in early-quirks, followed by a real
request_mem_region later on.

Doing the request_mem_region before the PCI layer gets its hands on it
isn't really possible, because __request_region depends on the memory
allocator being initialized.  So rather than add a new hook elsewhere in
setup_arch or start_kernel I figured I'd use an early quirk.

Reasonable?  Note iomem in both cases.

Before (RAM buffer prevents our allocation):
cafff000-caff : System RAM
cb00-cbff : RAM buffer
cfa0-feaf : PCI Bus :00
  d000-dfff : :00:02.0

After (yay):
cb00-cb9f : RAM buffer
cba0-cf9f : reserved
  cba0-cf9f : Graphics Stolen Memory
cfa0-feaf : PCI Bus :00

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

[Intel-gfx] [PATCH] drm/i915: Acquire dpio_lock for VLV sideband programming in DP/HDMI

[Chris Wilson]

Otherwise we get flooded by the kernel warning us that we are doing
long sequences of IO without serialisation. For example,

 WARNING: CPU: 0 PID: 11136 at drivers/gpu/drm/i915/intel_sideband.c:40 
vlv_sideband_rw+0x48/0x1ef()
 Modules linked in:
 CPU: 0 PID: 11136 Comm: kworker/u2:0 Tainted: GW3.11.0-rc2+ #4
 Call Trace:
  [c2028564] ?  warn_slowpath_common+0x63/0x78
  [c227ad43] ?  vlv_sideband_rw+0x48/0x1ef
  [c20285dd] ?  warn_slowpath_null+0xf/0x13
  [c227ad43] ?  vlv_sideband_rw+0x48/0x1ef
  [c227b060] ?  vlv_dpio_write+0x1c/0x21
  [c2262b3b] ?  intel_dp_set_signal_levels+0x24a/0x385
  [c2264909] ?  intel_dp_complete_link_train+0x25/0x1d1
  [c2264c55] ?  intel_dp_check_link_status+0xf7/0x106
  [c2238ced] ?  i915_hotplug_work_func+0x17b/0x221
  [c203a204] ?  process_one_work+0x12e/0x210
  [c203a5e4] ?  worker_thread+0x116/0x1ad
  [c203a4ce] ?  rescuer_thread+0x1cb/0x1cb
  [c203d8f5] ?  kthread+0x67/0x6c
  [c2457ebb] ?  ret_from_kernel_thread+0x1b/0x30
  [c203d88e] ?  init_completion+0x18/0x18

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
 drivers/gpu/drm/i915/intel_dp.c   | 6 ++
 drivers/gpu/drm/i915/intel_hdmi.c | 4 
 2 files changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 4a7ba5e..2a455b8 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1719,6 +1719,7 @@ static void intel_pre_enable_dp(struct intel_encoder 
*encoder)
int pipe = intel_crtc-pipe;
u32 val;
 
+   mutex_lock(dev_priv-dpio_lock);
val = vlv_dpio_read(dev_priv, DPIO_DATA_LANE_A(port));
val = 0;
if (pipe)
@@ -1732,6 +1733,7 @@ static void intel_pre_enable_dp(struct intel_encoder 
*encoder)
 0x00760018);
vlv_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF8(port),
 0x00400888);
+   mutex_unlock(dev_priv-dpio_lock);
}
 }
 
@@ -1746,6 +1748,7 @@ static void intel_dp_pre_pll_enable(struct intel_encoder 
*encoder)
return;
 
/* Program Tx lane resets to default */
+   mutex_lock(dev_priv-dpio_lock);
vlv_dpio_write(dev_priv, DPIO_PCS_TX(port),
 DPIO_PCS_TX_LANE2_RESET |
 DPIO_PCS_TX_LANE1_RESET);
@@ -1759,6 +1762,7 @@ static void intel_dp_pre_pll_enable(struct intel_encoder 
*encoder)
vlv_dpio_write(dev_priv, DPIO_PCS_STAGGER1(port), 0x00750f00);
vlv_dpio_write(dev_priv, DPIO_TX_CTL(port), 0x1500);
vlv_dpio_write(dev_priv, DPIO_TX_LANE(port), 0x4040);
+   mutex_unlock(dev_priv-dpio_lock);
 }
 
 /*
@@ -1970,6 +1974,7 @@ static uint32_t intel_vlv_signal_levels(struct intel_dp 
*intel_dp)
return 0;
}
 
+   mutex_lock(dev_priv-dpio_lock);
vlv_dpio_write(dev_priv, DPIO_TX_OCALINIT(port), 0x);
vlv_dpio_write(dev_priv, DPIO_TX_SWING_CTL4(port), demph_reg_value);
vlv_dpio_write(dev_priv, DPIO_TX_SWING_CTL2(port),
@@ -1978,6 +1983,7 @@ static uint32_t intel_vlv_signal_levels(struct intel_dp 
*intel_dp)
vlv_dpio_write(dev_priv, DPIO_PCS_STAGGER0(port), 0x0003);
vlv_dpio_write(dev_priv, DPIO_PCS_CTL_OVER1(port), preemph_reg_value);
vlv_dpio_write(dev_priv, DPIO_TX_OCALINIT(port), 0x8000);
+   mutex_unlock(dev_priv-dpio_lock);
 
return 0;
 }
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
b/drivers/gpu/drm/i915/intel_hdmi.c
index 044d11d..5fa3035 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1033,6 +1033,7 @@ static void intel_hdmi_pre_enable(struct intel_encoder 
*encoder)
return;
 
/* Enable clock channels for this port */
+   mutex_lock(dev_priv-dpio_lock);
val = vlv_dpio_read(dev_priv, DPIO_DATA_LANE_A(port));
val = 0;
if (pipe)
@@ -1063,6 +1064,7 @@ static void intel_hdmi_pre_enable(struct intel_encoder 
*encoder)
 0x00760018);
vlv_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF8(port),
 0x00400888);
+   mutex_unlock(dev_priv-dpio_lock);
 }
 
 static void intel_hdmi_pre_pll_enable(struct intel_encoder *encoder)
@@ -1076,6 +1078,7 @@ static void intel_hdmi_pre_pll_enable(struct 
intel_encoder *encoder)
return;
 
/* Program Tx lane resets to default */
+   mutex_lock(dev_priv-dpio_lock);
vlv_dpio_write(dev_priv, DPIO_PCS_TX(port),
 DPIO_PCS_TX_LANE2_RESET |
 DPIO_PCS_TX_LANE1_RESET);
@@ -1094,6 +1097,7 @@ static void intel_hdmi_pre_pll_enable(struct 
intel_encoder *encoder)
 0x2000);
vlv_dpio_write(dev_priv, DPIO_TX_OCALINIT(port),
 DPIO_TX_OCALINIT_EN);
+   mutex_lock(dev_priv-dpio_lock);
 }
 
 static void 

Re: [Intel-gfx] [PATCH] drm/i915: Acquire dpio_lock for VLV sideband programming in DP/HDMI

[Daniel Vetter]

On Fri, Jul 26, 2013 at 6:17 PM, Chris Wilson ch...@chris-wilson.co.uk wrote:
 Otherwise we get flooded by the kernel warning us that we are doing
 long sequences of IO without serialisation. For example,

  WARNING: CPU: 0 PID: 11136 at drivers/gpu/drm/i915/intel_sideband.c:40 
 vlv_sideband_rw+0x48/0x1ef()
  Modules linked in:
  CPU: 0 PID: 11136 Comm: kworker/u2:0 Tainted: GW3.11.0-rc2+ #4
  Call Trace:
   [c2028564] ?  warn_slowpath_common+0x63/0x78
   [c227ad43] ?  vlv_sideband_rw+0x48/0x1ef
   [c20285dd] ?  warn_slowpath_null+0xf/0x13
   [c227ad43] ?  vlv_sideband_rw+0x48/0x1ef
   [c227b060] ?  vlv_dpio_write+0x1c/0x21
   [c2262b3b] ?  intel_dp_set_signal_levels+0x24a/0x385
   [c2264909] ?  intel_dp_complete_link_train+0x25/0x1d1
   [c2264c55] ?  intel_dp_check_link_status+0xf7/0x106
   [c2238ced] ?  i915_hotplug_work_func+0x17b/0x221
   [c203a204] ?  process_one_work+0x12e/0x210
   [c203a5e4] ?  worker_thread+0x116/0x1ad
   [c203a4ce] ?  rescuer_thread+0x1cb/0x1cb
   [c203d8f5] ?  kthread+0x67/0x6c
   [c2457ebb] ?  ret_from_kernel_thread+0x1b/0x30
   [c203d88e] ?  init_completion+0x18/0x18

 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk

The problem here is that Jesse was lazy and grabs the lock in
vlv_crtc_enable, so I think your code here will deadlock. But I agree
that grabbing the lock where we actually frob the sideband is what we
want ...
-Daniel

 ---
  drivers/gpu/drm/i915/intel_dp.c   | 6 ++
  drivers/gpu/drm/i915/intel_hdmi.c | 4 
  2 files changed, 10 insertions(+)

 diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
 index 4a7ba5e..2a455b8 100644
 --- a/drivers/gpu/drm/i915/intel_dp.c
 +++ b/drivers/gpu/drm/i915/intel_dp.c
 @@ -1719,6 +1719,7 @@ static void intel_pre_enable_dp(struct intel_encoder 
 *encoder)
 int pipe = intel_crtc-pipe;
 u32 val;

 +   mutex_lock(dev_priv-dpio_lock);
 val = vlv_dpio_read(dev_priv, DPIO_DATA_LANE_A(port));
 val = 0;
 if (pipe)
 @@ -1732,6 +1733,7 @@ static void intel_pre_enable_dp(struct intel_encoder 
 *encoder)
  0x00760018);
 vlv_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF8(port),
  0x00400888);
 +   mutex_unlock(dev_priv-dpio_lock);
 }
  }

 @@ -1746,6 +1748,7 @@ static void intel_dp_pre_pll_enable(struct 
 intel_encoder *encoder)
 return;

 /* Program Tx lane resets to default */
 +   mutex_lock(dev_priv-dpio_lock);
 vlv_dpio_write(dev_priv, DPIO_PCS_TX(port),
  DPIO_PCS_TX_LANE2_RESET |
  DPIO_PCS_TX_LANE1_RESET);
 @@ -1759,6 +1762,7 @@ static void intel_dp_pre_pll_enable(struct 
 intel_encoder *encoder)
 vlv_dpio_write(dev_priv, DPIO_PCS_STAGGER1(port), 0x00750f00);
 vlv_dpio_write(dev_priv, DPIO_TX_CTL(port), 0x1500);
 vlv_dpio_write(dev_priv, DPIO_TX_LANE(port), 0x4040);
 +   mutex_unlock(dev_priv-dpio_lock);
  }

  /*
 @@ -1970,6 +1974,7 @@ static uint32_t intel_vlv_signal_levels(struct intel_dp 
 *intel_dp)
 return 0;
 }

 +   mutex_lock(dev_priv-dpio_lock);
 vlv_dpio_write(dev_priv, DPIO_TX_OCALINIT(port), 0x);
 vlv_dpio_write(dev_priv, DPIO_TX_SWING_CTL4(port), demph_reg_value);
 vlv_dpio_write(dev_priv, DPIO_TX_SWING_CTL2(port),
 @@ -1978,6 +1983,7 @@ static uint32_t intel_vlv_signal_levels(struct intel_dp 
 *intel_dp)
 vlv_dpio_write(dev_priv, DPIO_PCS_STAGGER0(port), 0x0003);
 vlv_dpio_write(dev_priv, DPIO_PCS_CTL_OVER1(port), preemph_reg_value);
 vlv_dpio_write(dev_priv, DPIO_TX_OCALINIT(port), 0x8000);
 +   mutex_unlock(dev_priv-dpio_lock);

 return 0;
  }
 diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
 b/drivers/gpu/drm/i915/intel_hdmi.c
 index 044d11d..5fa3035 100644
 --- a/drivers/gpu/drm/i915/intel_hdmi.c
 +++ b/drivers/gpu/drm/i915/intel_hdmi.c
 @@ -1033,6 +1033,7 @@ static void intel_hdmi_pre_enable(struct intel_encoder 
 *encoder)
 return;

 /* Enable clock channels for this port */
 +   mutex_lock(dev_priv-dpio_lock);
 val = vlv_dpio_read(dev_priv, DPIO_DATA_LANE_A(port));
 val = 0;
 if (pipe)
 @@ -1063,6 +1064,7 @@ static void intel_hdmi_pre_enable(struct intel_encoder 
 *encoder)
  0x00760018);
 vlv_dpio_write(dev_priv, DPIO_PCS_CLOCKBUF8(port),
  0x00400888);
 +   mutex_unlock(dev_priv-dpio_lock);
  }

  static void intel_hdmi_pre_pll_enable(struct intel_encoder *encoder)
 @@ -1076,6 +1078,7 @@ static void intel_hdmi_pre_pll_enable(struct 
 intel_encoder *encoder)
 return;

 /* Program Tx lane resets to default */
 +   mutex_lock(dev_priv-dpio_lock);
 vlv_dpio_write(dev_priv, DPIO_PCS_TX(port),
  

Re: [Intel-gfx] Maintainer-review fluff (was: Re: [PATCH 01/12] drm/i915: plumb VM into object operations)

[Jesse Barnes]

On Fri, 26 Jul 2013 11:51:00 +0200
Daniel Vetter dan...@ffwll.ch wrote:

 HI all,
 
 So BenI had a bit a private discussion and one thing I've explained a bit
 more in detail is what kind of review I'm doing as maintainer. I've
 figured this is generally useful. We've also discussed a bit that for
 developers without their own lab it would be nice if QA could test random
 branches on their set of machines. But imo that'll take quite a while,
 there's lots of other stuff to improve in QA land first. Anyway, here's
 it:
 
 Now an explanation for why this freaked me out, which is essentially an
 explanation of what I do when I do maintainer reviews:
 
 Probably the most important question I ask myself when reading a patch is
 if a regression would bisect to this, and the bisect is the only useful
 piece of evidence, would I stand a chance to understand it?.  Your patch
 is big, has the appearance of doing a few unrelated things and could very
 well hide a bug which would take me an awful lot of time to spot. So imo
 the answer for your patch is a clear no.

This is definitely a good point.  Big patches are both hard to review
and hard to debug, so should be kept as simple as possible (but no
simpler!).

 I've merged a few such patches in the past  where I've had a similar hunch
 and regretted it almost always. I've also sometimes split-up the patch
 while applying, but that approach doesn't scale any more with our rather
 big team.
 
 The second thing I try to figure out is whether the patch author is indeed
 the local expert on the topic at hand now. With our team size and patch
 flow I don't stand a chance if I try to understand everything to the last
 detail. Instead I try to assess this through the proxy of convincing
 myself the the patch submitter understands stuff much better than I do. I
 tend to check that by asking random questions, proposing alternative
 approaches and also by rating code/patch clarity. The obj_set_color
 double-loop very much gave me the impression that you didn't have a clear
 idea about how exactly this should work, so that  hunk trigger this
 maintainer hunch.

This is the part I think is unfair (see below) when proposed
alternatives aren't clearly defined.

 I admit that this is all rather fluffy and very much an inexact science,
 but it's the only tools I have as a maintainer. The alternative of doing
 shit myself or checking everything myself in-depth just doesnt scale.

I'm glad you brought this up, but I see a contradiction here:  if
you're just asking random questions to convince yourself the author
knows what they're doing, but simultaneously you're not checking
everything yourself in-depth, you'll have no way to know whether your
questions are being dealt with properly.

I think the way out of that contradiction is to trust reviewers,
especially in specific areas.

There's a downside in that the design will be a little less coherent
(i.e. matching the vision of a single person), but as you said, that
doesn't scale.

So I'd suggest a couple of rules to help:
  1) every patch gets at least two reviewed-bys
  2) one of those reviewed-bys should be from a domain expert, e.g.:
 DP - Todd, Jani
 GEM - Chris, Daniel
 $PLATFORM - $PLATFORM owner
 HDMI - Paulo
 PSR/FBC - Rodrigo/Shobhit
 * - Daniel (you get to be a wildcard)
 etc.
  3) reviews aren't allowed to contain solely bikeshed/codingstyle
 change requests, if there's nothing substantial merge shouldn't be
 blocked (modulo egregious violations like Hungarian notation)
  4) review comments should be concrete and actionable, and ideally not
 leave the author hanging with hints about problems the reviewer
 has spotted, leaving the author looking for easter eggs

For the most part I think we adhere to this, though reviews from the
domain experts are done more on an ad-hoc basis these days...

Thoughts?

-- 
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] Maintainer-review fluff (was: Re: [PATCH 01/12] drm/i915: plumb VM into object operations)

[Chris Wilson]

On Fri, Jul 26, 2013 at 09:59:42AM -0700, Jesse Barnes wrote:
   4) review comments should be concrete and actionable, and ideally not
  leave the author hanging with hints about problems the reviewer
  has spotted, leaving the author looking for easter eggs

Where am I going to find my fun, if I am not allowed to tell you that
you missed a zero in a thousand line patch but not tell you where?
Spoilsport :-p
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Acquire dpio_lock for VLV sideband programming in DP/HDMI

[Chris Wilson]

On Fri, Jul 26, 2013 at 06:46:43PM +0200, Daniel Vetter wrote:
 On Fri, Jul 26, 2013 at 6:17 PM, Chris Wilson ch...@chris-wilson.co.uk 
 wrote:
  Otherwise we get flooded by the kernel warning us that we are doing
  long sequences of IO without serialisation. For example,
 
   WARNING: CPU: 0 PID: 11136 at drivers/gpu/drm/i915/intel_sideband.c:40 
  vlv_sideband_rw+0x48/0x1ef()
   Modules linked in:
   CPU: 0 PID: 11136 Comm: kworker/u2:0 Tainted: GW3.11.0-rc2+ #4
   Call Trace:
[c2028564] ?  warn_slowpath_common+0x63/0x78
[c227ad43] ?  vlv_sideband_rw+0x48/0x1ef
[c20285dd] ?  warn_slowpath_null+0xf/0x13
[c227ad43] ?  vlv_sideband_rw+0x48/0x1ef
[c227b060] ?  vlv_dpio_write+0x1c/0x21
[c2262b3b] ?  intel_dp_set_signal_levels+0x24a/0x385
[c2264909] ?  intel_dp_complete_link_train+0x25/0x1d1
[c2264c55] ?  intel_dp_check_link_status+0xf7/0x106
[c2238ced] ?  i915_hotplug_work_func+0x17b/0x221
[c203a204] ?  process_one_work+0x12e/0x210
[c203a5e4] ?  worker_thread+0x116/0x1ad
[c203a4ce] ?  rescuer_thread+0x1cb/0x1cb
[c203d8f5] ?  kthread+0x67/0x6c
[c2457ebb] ?  ret_from_kernel_thread+0x1b/0x30
[c203d88e] ?  init_completion+0x18/0x18
 
  Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 
 The problem here is that Jesse was lazy and grabs the lock in
 vlv_crtc_enable, so I think your code here will deadlock. But I agree
 that grabbing the lock where we actually frob the sideband is what we
 want ...

Looks like vlv_crtc_enable() is the only place that takes the lock around
callbacks, so I propose dropping the locking in that function (and
scoping it around the sideband sequences as above).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] x86: add early quirk for reserving Intel graphics stolen memory v3

[Jesse Barnes]

On Fri, 26 Jul 2013 09:58:45 -0700
H. Peter Anvin h...@zytor.com wrote:

 On 07/25/2013 09:37 AM, Jesse Barnes wrote:
  Systems with Intel graphics controllers set aside memory exclusively for
  +   /*
  +* Almost universally we can find the Graphics Base of Stolen Memory
  +* at offset 0x5c in the igfx configuration space. On a few (desktop)
  +* machines this is also mirrored in the bridge device at different
  +* locations, or in the MCHBAR.
  +*/
 
 This comment makes me nervous.  It isn't clear to me if it is saying:
 
 - All igfx devices has the graphics base at 0x5c, a few have it in
   other places, too (which doesn't matter, we can use 0x5c anyway), or
 - Most igfx devices have the graphics base at 0x5c, some don't, and we
   hope and pray we're not on one of those systems because we're not
   checking.
 
 I assume it is the former, but it really needs to be phrased better.

I mostly copied it from the driver where we had some other ways of
finding it too.  For the PCI IDs listed, we can always use 0x5c.  I'll
fix the comment.

-- 
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] Maintainer-review fluff (was: Re: [PATCH 01/12] drm/i915: plumb VM into object operations)

[Jesse Barnes]

On Fri, 26 Jul 2013 18:08:48 +0100
Chris Wilson ch...@chris-wilson.co.uk wrote:

 On Fri, Jul 26, 2013 at 09:59:42AM -0700, Jesse Barnes wrote:
4) review comments should be concrete and actionable, and ideally not
   leave the author hanging with hints about problems the reviewer
   has spotted, leaving the author looking for easter eggs
 
 Where am I going to find my fun, if I am not allowed to tell you that
 you missed a zero in a thousand line patch but not tell you where?
 Spoilsport :-p

You'll just need to take up golf or something. :)

-- 
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] Maintainer-review fluff (was: Re: [PATCH 01/12] drm/i915: plumb VM into object operations)

[Daniel Vetter]

On Fri, Jul 26, 2013 at 09:59:42AM -0700, Jesse Barnes wrote:
 On Fri, 26 Jul 2013 11:51:00 +0200
 Daniel Vetter dan...@ffwll.ch wrote:
 
  HI all,
  
  So BenI had a bit a private discussion and one thing I've explained a bit
  more in detail is what kind of review I'm doing as maintainer. I've
  figured this is generally useful. We've also discussed a bit that for
  developers without their own lab it would be nice if QA could test random
  branches on their set of machines. But imo that'll take quite a while,
  there's lots of other stuff to improve in QA land first. Anyway, here's
  it:
  
  Now an explanation for why this freaked me out, which is essentially an
  explanation of what I do when I do maintainer reviews:
  
  Probably the most important question I ask myself when reading a patch is
  if a regression would bisect to this, and the bisect is the only useful
  piece of evidence, would I stand a chance to understand it?.  Your patch
  is big, has the appearance of doing a few unrelated things and could very
  well hide a bug which would take me an awful lot of time to spot. So imo
  the answer for your patch is a clear no.
 
 This is definitely a good point.  Big patches are both hard to review
 and hard to debug, so should be kept as simple as possible (but no
 simpler!).
 
  I've merged a few such patches in the past  where I've had a similar hunch
  and regretted it almost always. I've also sometimes split-up the patch
  while applying, but that approach doesn't scale any more with our rather
  big team.
  
  The second thing I try to figure out is whether the patch author is indeed
  the local expert on the topic at hand now. With our team size and patch
  flow I don't stand a chance if I try to understand everything to the last
  detail. Instead I try to assess this through the proxy of convincing
  myself the the patch submitter understands stuff much better than I do. I
  tend to check that by asking random questions, proposing alternative
  approaches and also by rating code/patch clarity. The obj_set_color
  double-loop very much gave me the impression that you didn't have a clear
  idea about how exactly this should work, so that  hunk trigger this
  maintainer hunch.
 
 This is the part I think is unfair (see below) when proposed
 alternatives aren't clearly defined.

Ben split up the patches meanwhile and imo they now look great (so fully
address the first concern). I've read through them this morning and dumped
a few (imo actionable) quick comments on irc. For the example here my
request is to squash a double-loop over vma lists (which will also rip out
a function call indirection as a bonus).

  I admit that this is all rather fluffy and very much an inexact science,
  but it's the only tools I have as a maintainer. The alternative of doing
  shit myself or checking everything myself in-depth just doesnt scale.
 
 I'm glad you brought this up, but I see a contradiction here:  if
 you're just asking random questions to convince yourself the author
 knows what they're doing, but simultaneously you're not checking
 everything yourself in-depth, you'll have no way to know whether your
 questions are being dealt with properly.

Well if the reply is unsure or inconstistent then I tend to dig in. E.g.
with Paulo's pc8+ stuff I've asked a few questions about interactions with
gmbus/edid reading/gem execbuf and he replied that he doesn't know. His
2nd patch version was still a bit thin on details in that area, so I've
sat down read through stuff and made a concreteactionable list of
corner-cases I think we should exercise.

 I think the way out of that contradiction is to trust reviewers,
 especially in specific areas.
 
Imo I've already started with that, there's lots of patches where I only
do a very cursory read when merging since I trust $AUTHOR and $REVIEWER
to get it right.

 There's a downside in that the design will be a little less coherent
 (i.e. matching the vision of a single person), but as you said, that
 doesn't scale.

I think overall we can still achieve good consistency in the design, so
that's a part where I try to chip in. But with a larger team it's clear
that consistency in little details will fizzle out more, otoh doing such
cleanups after big reworks (heck I've been rather inconstinent in all the
refactoring in the modeset code myself) sounds like good material to drag
newbies into our codebase.
 
 So I'd suggest a couple of rules to help:
   1) every patch gets at least two reviewed-bys

We have a hard time doing our current review load in a timely manner
already, I don't expect this to scale if we do it formally. But ...

   2) one of those reviewed-bys should be from a domain expert, e.g.:
  DP - Todd, Jani
  GEM - Chris, Daniel
  $PLATFORM - $PLATFORM owner
  HDMI - Paulo
  PSR/FBC - Rodrigo/Shobhit
  * - Daniel (you get to be a wildcard)
  etc.

... this is something that I've started to take into account already. E.g.
when I ask 

Re: [Intel-gfx] i915 irq storm mitigation in 3.10

[Jan Niggemann]

Hi Egbert,

Am 23.07.2013 13:26, schrieb Egbert Eich:

I've looked at the logs a bit more. Here's a patch adding some more
debug information. Would you please apply this to your 3.10 kernel
and generate a log file the same way as you did before.
The driver will be even more chatty - but I don't expect any problems
from this.



diff --git a/drivers/gpu/drm/i915/i915_irq.c
b/drivers/gpu/drm/i915/i915_irq.c
index e43d809..46bb77c 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -919,6 +919,11 @@ static inline void intel_hpd_irq_handler(struct
drm_device *dev,
spin_lock(dev_priv-irq_lock);
for (i = 1; i  HPD_NUM_PINS; i++) {

+   if (IS_G4X(dev)  (hpd[i]  hotplug_trigger) 
+   dev_priv-hpd_stats[i].hpd_mark != HPD_ENABLED)
+   DRM_DEBUG_KMS(Received HPD intterupt although 
disabled\n,
+ I915_READ(PORT_HOTPLUG_EN));
+
if (!(hpd[i]  hotplug_trigger) ||
dev_priv-hpd_stats[i].hpd_mark != HPD_ENABLED)
continue;
@@ -929,6 +934,8 @@ static inline void intel_hpd_irq_handler(struct
drm_device *dev,
   + 
msecs_to_jiffies(HPD_STORM_DETECT_PERIOD))) {
dev_priv-hpd_stats[i].hpd_last_jiffies = jiffies;
dev_priv-hpd_stats[i].hpd_cnt = 0;
+			DRM_DEBUG_KMS(Received HPD interrupt on PIN %d - jiffies: %d\n, 
i,

+ dev_priv-hpd_stats[i].hpd_last_jiffies);
} else if (dev_priv-hpd_stats[i].hpd_cnt  
HPD_STORM_THRESHOLD) {
dev_priv-hpd_stats[i].hpd_mark = HPD_MARK_DISABLED;
dev_priv-hpd_event_bits = ~(1  i);
@@ -936,6 +943,8 @@ static inline void intel_hpd_irq_handler(struct
drm_device *dev,
storm_detected = true;
} else {
dev_priv-hpd_stats[i].hpd_cnt++;
+   DRM_DEBUG_KMS(Received HPD interrupt on PIN %d - cnt: 
%d\n, i,
+ dev_priv-hpd_stats[i].hpd_cnt);
}
}
I applied this patch (but not the one you sent on Monday), recompiled 
and logged:

uncompressed (8,2M) http://files.hz6.de/kern_20130724.log
bz2 (288 KB) http://files.hz6.de/kern_20130724.log.bz2

Hope this helps,
Regards
jan
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] Linux 3.11-rc2 (acpi backlight, revert)

[James Hogan]

On 25 July 2013 14:00, Rafael J. Wysocki r...@sisk.pl wrote:
 On Monday, July 22, 2013 09:54:21 PM Rafael J. Wysocki wrote:
 On Monday, July 22, 2013 11:11:54 AM Linus Torvalds wrote:
  On Mon, Jul 22, 2013 at 6:02 AM, Rafael J. Wysocki r...@sisk.pl wrote:
  
   Linus, do you want me to send a pull request reverting 8c5bd7a and 
   efaa14c?
 
  Yes, but let's wait a while. Not because I think we'll fix the problem
  (hey, miracles might happen), but because I think it would be useful
  to couple the reverts with information about the particular machines
  that broke (and the people who reported it). So that when we
  inevitably try again, we can perhaps get some testing effort with
  those machines/people.
 
  It doesn't seem to be a show-stopped for a large number of people, so
  there's no huge hurry. I'd suggest doing the revert just in time for
  rc3, but waiting until then to gather info about people who see
  breakage.
 
  Sound like a plan?

 Yes, it does.

 OK, time to revert I guess.

 James, Kamal, Steven, Jörg, Martin, Kalle, please check if the apppended patch
 fixes the backlight for you.

Works for me

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

Re: [Intel-gfx] Linux 3.11-rc2 (acpi backlight, revert)

[Jörg Otte]

2013/7/25 Rafael J. Wysocki r...@sisk.pl:
 On Monday, July 22, 2013 09:54:21 PM Rafael J. Wysocki wrote:
 On Monday, July 22, 2013 11:11:54 AM Linus Torvalds wrote:
  On Mon, Jul 22, 2013 at 6:02 AM, Rafael J. Wysocki r...@sisk.pl wrote:
  
   Linus, do you want me to send a pull request reverting 8c5bd7a and 
   efaa14c?
 
  Yes, but let's wait a while. Not because I think we'll fix the problem
  (hey, miracles might happen), but because I think it would be useful
  to couple the reverts with information about the particular machines
  that broke (and the people who reported it). So that when we
  inevitably try again, we can perhaps get some testing effort with
  those machines/people.
 
  It doesn't seem to be a show-stopped for a large number of people, so
  there's no huge hurry. I'd suggest doing the revert just in time for
  rc3, but waiting until then to gather info about people who see
  breakage.
 
  Sound like a plan?

 Yes, it does.

 OK, time to revert I guess.

 James, Kamal, Steven, Jörg, Martin, Kalle, please check if the apppended patch
 fixes the backlight for you.


Problems, problems :-) I tried to apply on top of 3.11-rc2:

jojo@ahorn:/data/kernel/linux$ git log --pretty=oneline | head -5
3b2f64d00c46e1e4e9bd0bb9bb12619adac27a4b Linux 3.11-rc2
ea45ea70b6131fa0b006f5b687b9b1398b24f681 Merge tag 'acpi-video-3.11'
of git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm
90db76e829479ef2ba1fed8f2552846015469831 Merge tag 'ext4_for_linus' of
git://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4
dda5690defe4af62ee120f055e98e40d97e4c760 ext3: fix a BUG when opening
a file with O_TMPFILE flag
e94bd3490f4ef342801cfc76b33d8baf9ccc9437 ext4: fix a BUG when opening
a file with O_TMPFILE flag

jojo@ahorn:/data/kernel/linux$ git apply --check
/data/kernel/acpi-backlight-revert.patch
error: patch failed: drivers/acpi/video.c:897
error: drivers/acpi/video.c: patch does not apply
error: patch failed: drivers/gpu/drm/i915/i915_dma.c:1648
error: drivers/gpu/drm/i915/i915_dma.c: patch does not apply
error: patch failed: include/acpi/video.h:17
error: include/acpi/video.h: patch does not apply
error: patch failed: drivers/acpi/video_detect.c:235
error: drivers/acpi/video_detect.c: patch does not apply
error: patch failed: include/linux/acpi.h:191
error: include/linux/acpi.h: patch does not apply
error: patch failed: drivers/acpi/internal.h:169
error: drivers/acpi/internal.h: patch does not apply
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] i915 irq storm mitigation in 3.10

[Jan Niggemann]

Hi Daniel,

Am 25.07.2013 11:58, schrieb Daniel Vetter:

Can you pls try the below
patch (on top of Egbert's debug stuff)?

diff --git a/drivers/gpu/drm/i915/i915_reg.h
b/drivers/gpu/drm/i915/i915_reg.h
index 6caa748..2d4c884 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1925,9 +1925,9 @@

 #define PORT_HOTPLUG_STAT  (dev_priv-info-display_mmio_offset
+ 0x61114)
 /* HDMI/DP bits are gen4+ */
-#define   PORTB_HOTPLUG_LIVE_STATUS   (1  29)
+#define   PORTD_HOTPLUG_LIVE_STATUS   (1  29)
 #define   PORTC_HOTPLUG_LIVE_STATUS   (1  28)
-#define   PORTD_HOTPLUG_LIVE_STATUS   (1  27)
+#define   PORTB_HOTPLUG_LIVE_STATUS   (1  27)
 #define   PORTD_HOTPLUG_INT_STATUS (3  21)
 #define   PORTC_HOTPLUG_INT_STATUS (3  19)
 #define   PORTB_HOTPLUG_INT_STATUS (3  17)

I did, here are the logs:
364K http://files.hz6.de/kern_20130724_2.log
I don't understand what exactly this patch does, but I noticed:
- much less drm debug info, resulting in a much smaller log
- no more noticeable lag on my system (even though a storm was 
detected).

I double checked the latter and the lag seems indeed to be gone...

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

Re: [Intel-gfx] Ugly patches for stolen reservation

[H. Peter Anvin]

So the bootloader is just as likely to step on things... what happens when/if 
it does?

Ingo Molnar mi...@kernel.org wrote:

* Jesse Barnes jbar...@virtuousgeek.org wrote:

 Patch 2/2 has the description, but suffice it to say I'm 
 not really pleased with this, though it does solve a 
 problem we have.  On some machines, we get MMIO space 
 allocated on top of this hidden memory, which can cause 
 problems.  I'm not sure if there are similar problems for 
 other hunks of the address space; if so it's possible 
 this could be made more general (though the bits for 
 looking up the address of this region are definitely 
 Intel graphics specific).

It looks pretty hardware specific. Discovering it the hard 
way and marking it e820 reserved in an early quirk is what 
the firmware should have done to begin with - and I doubt 
the kernel could do anything significantly cleaner.

How does Windows manage to not crash? By luckily never 
allocating PCI resources on top of the RAM? Or does it have 
a quirk?

 Chris has some patches on top to add a new E820 type so 
 we can look up the region later, which removes some 
 redundant code in the i915 driver at least.
 
 Any comments?  I assume no one likes this, but maybe it's 
 just another early quirk we'll have to live with...

No strong feelings against it - my only suggestion would be 
to make this more visible - right now it's added as e820 
reserved which hides amongst other areas already marked 
reserved - would a low-key printk() of the range added make 
it more apparent that a kernel quirk activated here?

Just so that people know that it came from the kernel, not 
the firmware.

But in any case:

Acked-by: Ingo Molnar mi...@kernel.org

Thanks,

   Ingo

-- 
Sent from my mobile phone. Please excuse brevity and lack of formatting.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] Ugly patches for stolen reservation

[H. Peter Anvin]

On 07/25/2013 05:31 PM, Linus Torvalds wrote:
 On Thu, Jul 25, 2013 at 3:42 PM, H. Peter Anvin h...@zytor.com wrote:
 So the bootloader is just as likely to step on things... what happens 
 when/if it does?
 
 This isn't a new problem. We've had this firmware tables don't show
 all devices issue before.
 

Yes, I just want to know what happens.

 The only odd thing about this one is how the quirk in question uses
 e820_add_region() instead of just adding things to the MMIO list.
 And I think that's actually likely a mistake.
 
 So Jesse, why don't you do what the other quirks do, and claim an
 actual MMIO resource? If you make it a real resource, you'll get to
 use fancy things like REAL NAMES, and actually document it. With
 human-readable strings.
 
 See quirk_io_region() in drivers/pci/quirks.c for example. The same
 code except for IORESOURCE_MEM should do a lovely job..
 
 And even *if* it's already marked reserved in the e820 table, it just
 looks nice in /proc/iomem.

We should do both -- mark it reserved in early boot, and add it as an
MMIO region later during boot.

The problem here, if I'm reading this right, is that this memory region
is marked as normal RAM in e820, which is much worse than just not
marking it as reserved; we need to intercept this memory before we
genuinely turn it into normal RAM.

At the same time, we have no protection against the bootloader using
this as memory or even placing the kernel there.  The BIOS needs to be
fixed regardless of what workarounds we do in the kernel.

-hpa


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

Re: [Intel-gfx] Ugly patches for stolen reservation

[H. Peter Anvin]

On 07/25/2013 04:17 PM, Jesse Barnes wrote:
 Well, it's ok if the boot loader writes to this memory, the worst
 that'll happen is you'll see garbage on the screen.  If the boot loader
 tries to do MMIO mapping on top it'll get into trouble... but why would
 it do that?
 
 Jesse

Much worse: it could be hunting for a place to put the kernel, and put
it there.

-hpa


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

Re: [Intel-gfx] Linux 3.11-rc2 (acpi backlight, revert)

[Joerg Platte]

On 25.07.2013 21:47, Rafael J. Wysocki wrote:


Other people who experienced problems with backlight in 3.11-rc2, please let me
know whether or not the revert works for you too if you can.


Before reverting the patch /sys/class/backlight was empty and backlight 
brightness was set to max, now it again contains a link to acpi_video0 
on my Thinkpad 420s with intel video and adjusting the backlight works 
again.


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

Re: [Intel-gfx] Ugly patches for stolen reservation

[H. Peter Anvin]

On 07/25/2013 07:14 PM, Jesse Barnes wrote:
 To clarify: it'll either be marked reserved or not listed at all in e820, 
 which is why I did this early, before any other e820 stuff like the RAM 
 buffer are allocated, and before we could use the iomem resource (or maybe 
 we could even early per Linus? I'll check). 
 
 Jesse

If it is marked reserved or not listed at all it is much less of an
issue.  Reserved is in fact the correct thing; not listed at all really
isn't very problematic in most cases.

-hpa


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

Re: [Intel-gfx] Linux 3.11-rc2 (acpi backlight, revert)

[Martin Steigerwald]

Am Donnerstag, 25. Juli 2013, 15:00:26 schrieb Rafael J. Wysocki:
 On Monday, July 22, 2013 09:54:21 PM Rafael J. Wysocki wrote:
  On Monday, July 22, 2013 11:11:54 AM Linus Torvalds wrote:
   On Mon, Jul 22, 2013 at 6:02 AM, Rafael J. Wysocki r...@sisk.pl wrote:
Linus, do you want me to send a pull request reverting 8c5bd7a and
efaa14c?
   
   Yes, but let's wait a while. Not because I think we'll fix the problem
   (hey, miracles might happen), but because I think it would be useful
   to couple the reverts with information about the particular machines
   that broke (and the people who reported it). So that when we
   inevitably try again, we can perhaps get some testing effort with
   those machines/people.
   
   It doesn't seem to be a show-stopped for a large number of people, so
   there's no huge hurry. I'd suggest doing the revert just in time for
   rc3, but waiting until then to gather info about people who see
   breakage.
   
   Sound like a plan?
  
  Yes, it does.
 
 OK, time to revert I guess.
 
 James, Kamal, Steven, Jörg, Martin, Kalle, please check if the apppended
 patch fixes the backlight for you.

Rafael, do you still need more testing urgently? Otherwise I´d wait till its 
in some next 3.11 rc and test then.

Thanks,
-- 
Martin 'Helios' Steigerwald - http://www.Lichtvoll.de
GPG: 03B0 0D6C 0040 0710 4AFA  B82F 991B EAAC A599 84C7
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] x86: add early quirk for reserving Intel graphics stolen memory v3

[H. Peter Anvin]

On 07/25/2013 09:37 AM, Jesse Barnes wrote:
 Systems with Intel graphics controllers set aside memory exclusively for
 + /*
 +  * Almost universally we can find the Graphics Base of Stolen Memory
 +  * at offset 0x5c in the igfx configuration space. On a few (desktop)
 +  * machines this is also mirrored in the bridge device at different
 +  * locations, or in the MCHBAR.
 +  */

This comment makes me nervous.  It isn't clear to me if it is saying:

- All igfx devices has the graphics base at 0x5c, a few have it in
  other places, too (which doesn't matter, we can use 0x5c anyway), or
- Most igfx devices have the graphics base at 0x5c, some don't, and we
  hope and pray we're not on one of those systems because we're not
  checking.

I assume it is the former, but it really needs to be phrased better.

-hpa


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

Re: [Intel-gfx] [PATCH] drm/i915: Acquire dpio_lock for VLV sideband programming in DP/HDMI

[Daniel Vetter]

On Fri, Jul 26, 2013 at 7:11 PM, Chris Wilson ch...@chris-wilson.co.uk wrote:
 The problem here is that Jesse was lazy and grabs the lock in
 vlv_crtc_enable, so I think your code here will deadlock. But I agree
 that grabbing the lock where we actually frob the sideband is what we
 want ...

 Looks like vlv_crtc_enable() is the only place that takes the lock around
 callbacks, so I propose dropping the locking in that function (and
 scoping it around the sideband sequences as above).

Yeah, I think moving the locking into callbacks/lower-level functions
is the right approach.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH V2] drm/i915: Add messages useful for HPD storm detection debugging (v2)

[Daniel Vetter]

On Fri, Jul 26, 2013 at 03:23:54PM +0300, Jani Nikula wrote:
 On Fri, 26 Jul 2013, Egbert Eich e...@suse.de wrote:
  For HPD storm detection we now mask out individual interrupt source
  bits. We have already seen a case where HPD interrupt enable bits
  were assigned to the wrong pins. To track these conditions more
  easily add some debugging messages.
 
  v2: Spelling fixes as suggested by Jani Nikula jani.nik...@linux.intel.com
 
 Reviewed-by: Jani Nikula jani.nik...@intel.com
Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Replace open-coded offset_in_page()

[Daniel Vetter]

On Sun, Jul 21, 2013 at 05:23:11PM +0100, Chris Wilson wrote:
 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Retry DP aux_ch communications with a different clock after failure

[Daniel Vetter]

On Fri, Jul 26, 2013 at 02:40:01PM +0300, Jani Nikula wrote:
 On Sun, 21 Jul 2013, Chris Wilson ch...@chris-wilson.co.uk wrote:
  The w/a db makes the recommendation to both use a non-default value for
  the initial clock and then to retry with an alternative clock for
  Haswell with the Lakeport PCH.
 
  On LPT:H, use a divider value of 63 decimal (03Fh). If there is a
  failure, retry at least three times with 63, then retry at least three
  times with 72 decimal (048h).
 
 Overall I'd prefer adding the outer repeat loop first, keeping existing
 behaviour, and only then adding the divider value changes in
 get_aux_clock_divider(). But that's in the bikeshedding dept. Either
 way,
 
 Reviewed-by: Jani Nikula jani.nik...@intel.com

Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] Ugly patches for stolen reservation

[Daniel Vetter]

On Thu, Jul 25, 2013 at 05:48:42PM -0700, H. Peter Anvin wrote:
 On 07/25/2013 05:31 PM, Linus Torvalds wrote:
  On Thu, Jul 25, 2013 at 3:42 PM, H. Peter Anvin h...@zytor.com wrote:
  So the bootloader is just as likely to step on things... what happens 
  when/if it does?
  
  This isn't a new problem. We've had this firmware tables don't show
  all devices issue before.
  
 
 Yes, I just want to know what happens.
 
  The only odd thing about this one is how the quirk in question uses
  e820_add_region() instead of just adding things to the MMIO list.
  And I think that's actually likely a mistake.
  
  So Jesse, why don't you do what the other quirks do, and claim an
  actual MMIO resource? If you make it a real resource, you'll get to
  use fancy things like REAL NAMES, and actually document it. With
  human-readable strings.
  
  See quirk_io_region() in drivers/pci/quirks.c for example. The same
  code except for IORESOURCE_MEM should do a lovely job..
  
  And even *if* it's already marked reserved in the e820 table, it just
  looks nice in /proc/iomem.
 
 We should do both -- mark it reserved in early boot, and add it as an
 MMIO region later during boot.

We started to reserve this range in drm/i915 (and still do), that's why
we've noticed that something is amiss.
 
 The problem here, if I'm reading this right, is that this memory region
 is marked as normal RAM in e820, which is much worse than just not
 marking it as reserved; we need to intercept this memory before we
 genuinely turn it into normal RAM.

I haven't seen a case where it's marked as ram, but many systems don't
mark it as reserved. Then the pci code can go around and put a bar into
the middle of that range. The other issue (benign but ugly) is that the
RAM buffer (used to protect stolen memory without knowing where exactly it
is because it's not properly reserved in the e820 map) sometimes ends up
in the graphics stolen range. With the new iomeme reserve code in drm/i915
we've stopped using stolen in such cases to avoid surprises, but that
kills  a few neat features.

Thus far we haven't tracked down any bugs to such overlaps yet, but otoh
we don't use stolen all that much yet. But we have plans to put it to real
use (and use the full range) and I'd like to get the reservation conflicts
sorted out before we do so.

 At the same time, we have no protection against the bootloader using
 this as memory or even placing the kernel there.  The BIOS needs to be
 fixed regardless of what workarounds we do in the kernel.

Like I've said we haven't seen it earmarked as ram anywhere yet, so I
don't think this is something we need to concern ousrselves with.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: fix gen4 digital port hotplug definitions

[Daniel Vetter]

On Fri, Jul 26, 2013 at 01:21:48PM +0300, Jani Nikula wrote:
 On Fri, 26 Jul 2013, Daniel Vetter daniel.vet...@ffwll.ch wrote:
  Apparently Bspec is wrong in this case here even for gm45. Note that
  Bspec is horribly misguided on i965g/gm, so we don't have any other
  data points besides that it seems to make machines work better.
 
  With this changes all the bits in PORT_HOTPLUG_STAT for the digital
  ports are ordered the same way. This seems to agree with what register
  dumps from the hpd storm handling code shows, where the LIVE bit and
  the short/long pulse STATUS bits light up at the same time with this
  enumeration (but no with the one from Bspec).
 
 Would a comment about this near the #defines be in order? To avoid the
 these values are all wrong per bspec patches.

Yeah, good idea, I'll add a comment when merging this.
-Daniel

 
 Cheers,
 Jani.
 
  Also tested on my gm45 which has two DP+ ports, and everything seems
  to still work as expected.
 
  References: 
  http://www.mail-archive.com/intel-gfx@lists.freedesktop.org/msg23054.html
  Cc: Egbert Eich e...@suse.com
  Cc: Jan Niggemann j...@hz6.de
  Tested-by: Jan Niggemann j...@hz6.de
  Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
  ---
   drivers/gpu/drm/i915/i915_reg.h | 4 ++--
   1 file changed, 2 insertions(+), 2 deletions(-)
 
  diff --git a/drivers/gpu/drm/i915/i915_reg.h 
  b/drivers/gpu/drm/i915/i915_reg.h
  index 6caa748..2d4c884 100644
  --- a/drivers/gpu/drm/i915/i915_reg.h
  +++ b/drivers/gpu/drm/i915/i915_reg.h
  @@ -1925,9 +1925,9 @@
   
   #define PORT_HOTPLUG_STAT  (dev_priv-info-display_mmio_offset + 0x61114)
   /* HDMI/DP bits are gen4+ */
  -#define   PORTB_HOTPLUG_LIVE_STATUS   (1  29)
  +#define   PORTD_HOTPLUG_LIVE_STATUS   (1  29)
   #define   PORTC_HOTPLUG_LIVE_STATUS   (1  28)
  -#define   PORTD_HOTPLUG_LIVE_STATUS   (1  27)
  +#define   PORTB_HOTPLUG_LIVE_STATUS   (1  27)
   #define   PORTD_HOTPLUG_INT_STATUS (3  21)
   #define   PORTC_HOTPLUG_INT_STATUS (3  19)
   #define   PORTB_HOTPLUG_INT_STATUS (3  17)
  -- 
  1.8.3.2
 
  ___
  Intel-gfx mailing list
  Intel-gfx@lists.freedesktop.org
  http://lists.freedesktop.org/mailman/listinfo/intel-gfx
 
 -- 
 Jani Nikula, Intel Open Source Technology Center

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/i915: fix pnv display core clock readout out

[Daniel Vetter]

On Fri, Jul 26, 2013 at 11:18:21AM +0300, Jani Nikula wrote:
 On Fri, 26 Jul 2013, Daniel Vetter daniel.vet...@ffwll.ch wrote:
  We need the correct clock to accurately assess whether we need to
  enable the double wide pipe mode or not.
 
  Cc: Chris Wilson ch...@chris-wilson.co.uk
  Cc: Stéphane Marchesin marc...@chromium.org
  Cc: Stuart Abercrombie sabercrom...@google.com
  Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
  ---
   drivers/gpu/drm/i915/i915_reg.h  |  6 ++
   drivers/gpu/drm/i915/intel_display.c | 29 -
   2 files changed, 34 insertions(+), 1 deletion(-)
 
  diff --git a/drivers/gpu/drm/i915/i915_reg.h 
  b/drivers/gpu/drm/i915/i915_reg.h
  index 6caa748..3aebe5d 100644
  --- a/drivers/gpu/drm/i915/i915_reg.h
  +++ b/drivers/gpu/drm/i915/i915_reg.h
  @@ -61,6 +61,12 @@
   #define   GC_LOW_FREQUENCY_ENABLE  (1  7)
   #define   GC_DISPLAY_CLOCK_190_200_MHZ (0  4)
   #define   GC_DISPLAY_CLOCK_333_MHZ (4  4)
  +#define   GC_DISPLAY_CLOCK_267_MHZ_PNV (0  4)
  +#define   GC_DISPLAY_CLOCK_333_MHZ_PNV (1  4)
  +#define   GC_DISPLAY_CLOCK_444_MHZ_PNV (2  4)
  +#define   GC_DISPLAY_CLOCK_200_MHZ_PNV (5  4)
  +#define   GC_DISPLAY_CLOCK_133_MHZ_PNV (6  4)
  +#define   GC_DISPLAY_CLOCK_167_MHZ_PNV (7  4)
   #define   GC_DISPLAY_CLOCK_MASK(7  4)
   #define   GM45_GC_RENDER_CLOCK_MASK(0xf  0)
   #define   GM45_GC_RENDER_CLOCK_266_MHZ (8  0)
  diff --git a/drivers/gpu/drm/i915/intel_display.c 
  b/drivers/gpu/drm/i915/intel_display.c
  index b3389d7..3e66f05 100644
  --- a/drivers/gpu/drm/i915/intel_display.c
  +++ b/drivers/gpu/drm/i915/intel_display.c
  @@ -4163,6 +4163,30 @@ static int i9xx_misc_get_display_clock_speed(struct 
  drm_device *dev)
  return 20;
   }
   
  +static int pnv_get_display_clock_speed(struct drm_device *dev)
  +{
  +   u16 gcfgc = 0;
  +
  +   pci_read_config_word(dev-pdev, GCFGC, gcfgc);
  +
  +   switch (gcfgc  GC_DISPLAY_CLOCK_MASK) {
  +   case GC_DISPLAY_CLOCK_267_MHZ_PNV:
  +   return 267000;
  +   case GC_DISPLAY_CLOCK_333_MHZ_PNV:
  +   return 333000;
  +   case GC_DISPLAY_CLOCK_444_MHZ_PNV:
  +   return 444000;
  +   case GC_DISPLAY_CLOCK_200_MHZ_PNV:
  +   return 20;
  +   default:
  +   DRM_ERROR(Unknown pnv display core clock 0x%04x\n, gcfgc);
 
 Reading the spec, should the default/fallback be 333 MHz for desktop?
 Otherwise,

As discussed on irc the default case should never happen, but I've simply
picked the slowest frequency to be on the safe side and hopefully show
something on the screen.

 Reviewed-by: Jani Nikula jani.nik...@intel.com

Queued for -next, thanks for the review.
-Daniel

 
  +   case GC_DISPLAY_CLOCK_133_MHZ_PNV:
  +   return 133000;
  +   case GC_DISPLAY_CLOCK_167_MHZ_PNV:
  +   return 167000;
  +   }
  +}
  +
   static int i915gm_get_display_clock_speed(struct drm_device *dev)
   {
  u16 gcfgc = 0;
  @@ -9605,9 +9629,12 @@ static void intel_init_display(struct drm_device 
  *dev)
  else if (IS_I915G(dev))
  dev_priv-display.get_display_clock_speed =
  i915_get_display_clock_speed;
  -   else if (IS_I945GM(dev) || IS_845G(dev) || IS_PINEVIEW_M(dev))
  +   else if (IS_I945GM(dev) || IS_845G(dev))
  dev_priv-display.get_display_clock_speed =
  i9xx_misc_get_display_clock_speed;
  +   else if (IS_PINEVIEW(dev))
  +   dev_priv-display.get_display_clock_speed =
  +   pnv_get_display_clock_speed;
  else if (IS_I915GM(dev))
  dev_priv-display.get_display_clock_speed =
  i915gm_get_display_clock_speed;
  -- 
  1.8.3.2
 
  ___
  Intel-gfx mailing list
  Intel-gfx@lists.freedesktop.org
  http://lists.freedesktop.org/mailman/listinfo/intel-gfx
 
 -- 
 Jani Nikula, Intel Open Source Technology Center

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] 3.10.2 : new warning WARNING: at drivers/gpu/drm/i915/intel_display.c:1656 ironlake_crtc_disable+0x7ce/0x800 [i915]()

[Daniel Vetter]

On Wed, Jul 24, 2013 at 05:51:41PM +0200, Toralf Förster wrote:
 Realized this today while booting a ThinkPad T420 with integrated intel 
 graphic :

Can you please retest with latest upstream git from Linus' tree?

commit 35c95375f69ceec721fea67a0532bc17ceb5cf64
Author: Daniel Vetter daniel.vet...@ffwll.ch
Date:   Wed Jul 17 06:55:04 2013 +0200

drm/i915: Sanitize shared dpll state

Could be the fix for the issue you're hitting here.

If that doesn't help please boot with drm.debug=0xe added to your kernel
cmdline, reproduce the backtraces and then attch the complete dmesg.

Thanks, Daniel

 
 Jul 24 17:36:10 n22 kernel: [ cut here ]
 Jul 24 17:36:10 n22 kernel: WARNING: at 
 drivers/gpu/drm/i915/intel_display.c:1656 ironlake_crtc_disable+0x7ce/0x800 
 [i915]()
 Jul 24 17:36:10 n22 kernel: Modules linked in: cpufreq_stats bridge stp ipv6 
 llc tun i915 cfbfillrect cfbimgblt i2c_algo_bit cfbcopyarea intel_agp 
 intel_gtt sdhci_pci fbcon bitblit softcursor font drm_kms_helper drm uvcvideo 
 agpgart videobuf2_vmalloc videobuf2_memops videobuf2_core videodev usblp 
 coretemp kvm_intel arc4 iwldvm sdhci fb kvm mmc_core e1000e mac80211 psmouse 
 fbdev evdev iwlwifi acpi_cpufreq mperf video ptp pps_core thermal processor 
 thinkpad_acpi cfg80211 thermal_sys nvram rfkill wmi ac tpm_tis tpm tpm_bios 
 button battery snd_hda_codec_conexant snd_hda_intel snd_hda_codec snd_pcm 
 i2c_i801 snd_page_alloc i2c_core snd_timer 8250_pci snd 8250 soundcore 
 serial_core hwmon aesni_intel ablk_helper cryptd lrw aes_i586 xts gf128mul 
 cbc fuse nfs lockd sunrpc dm_crypt dm_mod hid_monterey hid_microsoft 
 hid_logitech hid_ezkey hid_cypress hid_chicony hid_cherry hid_belkin 
 hid_apple hid_a4tech hid_generic usbhid hid sr_mod cdrom sg [last unloaded: 
 microcode]
 Jul 24 17:36:10 n22 kernel: CPU: 1 PID: 4759 Comm: kworker/u16:38 Not tainted 
 3.10.2 #6
 Jul 24 17:36:10 n22 kernel: Hardware name: LENOVO 4180F65/4180F65, BIOS 
 83ET75WW (1.45 ) 05/10/2013
 Jul 24 17:36:10 n22 kernel: Workqueue: events_unbound async_run_entry_fn
 Jul 24 17:36:10 n22 kernel: f8be6eec f8be6eec ecd13cc0 c13d42ed ecd13ce8 
 c1033e84 c1490e7d f8be6eec
 Jul 24 17:36:10 n22 kernel: 0678 f8ba69fe f8ba69fe f17d3210 ed81 
 ed81161c ecd13cf8 c1033ec2
 Jul 24 17:36:10 n22 kernel: 0009  ecd13d4c f8ba69fe 0004 
 f8bee15e f8bdd140 f8be765c
 Jul 24 17:36:10 n22 kernel: Call Trace:
 Jul 24 17:36:10 n22 kernel: [c13d42ed] dump_stack+0x16/0x18
 Jul 24 17:36:10 n22 kernel: [c1033e84] warn_slowpath_common+0x64/0x80
 Jul 24 17:36:10 n22 kernel: [f8ba69fe] ? ironlake_crtc_disable+0x7ce/0x800 
 [i915]
 Jul 24 17:36:10 n22 kernel: [f8ba69fe] ? ironlake_crtc_disable+0x7ce/0x800 
 [i915]
 Jul 24 17:36:10 n22 kernel: [c1033ec2] warn_slowpath_null+0x22/0x30
 Jul 24 17:36:10 n22 kernel: [f8ba69fe] ironlake_crtc_disable+0x7ce/0x800 
 [i915]
 Jul 24 17:36:10 n22 kernel: [f8ba7d64] __intel_set_mode+0x414/0xc10 [i915]
 Jul 24 17:36:10 n22 kernel: [f8bb1645] 
 intel_modeset_setup_hw_state+0x665/0x990 [i915]
 Jul 24 17:36:10 n22 kernel: [c13d5c48] ? mutex_lock+0x18/0x40
 Jul 24 17:36:10 n22 kernel: [f8b8021d] __i915_drm_thaw+0xed/0x180 [i915]
 Jul 24 17:36:10 n22 kernel: [f8b9adf2] ? 
 i915_gem_restore_gtt_mappings+0x92/0xd0 [i915]
 Jul 24 17:36:10 n22 kernel: [f8b806cd] i915_resume+0x6d/0xc0 [i915]
 Jul 24 17:36:10 n22 kernel: [f8b80732] i915_pm_resume+0x12/0x20 [i915]
 Jul 24 17:36:10 n22 kernel: [c124fa06] pci_pm_restore+0x66/0xd0
 Jul 24 17:36:10 n22 kernel: [c12bd6d1] dpm_run_callback.isra.4+0x31/0x60
 Jul 24 17:36:10 n22 kernel: [c105d39e] ? complete_all+0x4e/0x60
 Jul 24 17:36:10 n22 kernel: [c124f9a0] ? pci_pm_suspend_noirq+0x160/0x160
 Jul 24 17:36:10 n22 kernel: [c12bd77f] device_resume+0x7f/0xf0
 Jul 24 17:36:10 n22 kernel: [c12bd80e] async_resume+0x1e/0x50
 Jul 24 17:36:10 n22 kernel: [c10621a6] ? try_to_wake_up+0xa6/0x220
 Jul 24 17:36:10 n22 kernel: [c105ae68] async_run_entry_fn+0x38/0x140
 Jul 24 17:36:10 n22 kernel: [c104e5fe] process_one_work+0x10e/0x390
 Jul 24 17:36:10 n22 kernel: [c104c7f5] ? start_worker+0x25/0x30
 Jul 24 17:36:10 n22 kernel: [c104f3c8] ? manage_workers.isra.24+0x188/0x290
 Jul 24 17:36:10 n22 kernel: [c104f5d2] worker_thread+0x102/0x320
 Jul 24 17:36:10 n22 kernel: [c104f4d0] ? manage_workers.isra.24+0x290/0x290
 Jul 24 17:36:10 n22 kernel: [c1054b44] kthread+0x94/0xa0
 Jul 24 17:36:10 n22 kernel: [c13d8bb7] ret_from_kernel_thread+0x1b/0x28
 Jul 24 17:36:10 n22 kernel: [c1054ab0] ? flush_kthread_worker+0x90/0x90
 Jul 24 17:36:10 n22 kernel: ---[ end trace 7f63a6cbe7b8fcf3 ]---
 Jul 24 17:36:10 n22 kernel: [ cut here ]
 Jul 24 17:36:10 n22 kernel: WARNING: at 
 drivers/gpu/drm/i915/intel_display.c:1091 assert_pch_pll+0x193/0x200 [i915]()
 Jul 24 17:36:10 n22 kernel: PCH PLL state for reg c6014 assertion failure 
 (expected off, current on), val=c4800080
 Jul 24 17:36:10 n22 kernel: Modules linked in: cpufreq_stats bridge stp ipv6 
 llc tun i915 cfbfillrect cfbimgblt 

Re: [Intel-gfx] [PATCH 02/13] drm/i915/dvo: switch -mode_fixup to -compute_config

[Rodrigo Vivi]

On Sun, Jul 21, 2013 at 4:36 PM, Daniel Vetter daniel.vet...@ffwll.ch wrote:
 This is the last encoder -mode_fixup callback we have left, so
 convert it.

 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 ---
  drivers/gpu/drm/i915/intel_dvo.c | 14 --
  1 file changed, 8 insertions(+), 6 deletions(-)

 diff --git a/drivers/gpu/drm/i915/intel_dvo.c 
 b/drivers/gpu/drm/i915/intel_dvo.c
 index d820884..1297ea3 100644
 --- a/drivers/gpu/drm/i915/intel_dvo.c
 +++ b/drivers/gpu/drm/i915/intel_dvo.c
 @@ -241,11 +241,11 @@ static int intel_dvo_mode_valid(struct drm_connector 
 *connector,
 return intel_dvo-dev.dev_ops-mode_valid(intel_dvo-dev, mode);
  }

 -static bool intel_dvo_mode_fixup(struct drm_encoder *encoder,
 -const struct drm_display_mode *mode,
 -struct drm_display_mode *adjusted_mode)
 +static bool intel_dvo_compute_config(struct intel_encoder *encoder,
 +struct intel_crtc_config *pipe_config)
  {
 -   struct intel_dvo *intel_dvo = enc_to_dvo(to_intel_encoder(encoder));
 +   struct intel_dvo *intel_dvo = enc_to_dvo(encoder);
 +   struct drm_display_mode *adjusted_mode = pipe_config-adjusted_mode;

 /* If we have timings from the BIOS for the panel, put them in
  * to the adjusted mode.  The CRTC will be set up for this mode,
 @@ -267,7 +267,9 @@ static bool intel_dvo_mode_fixup(struct drm_encoder 
 *encoder,
 }

 if (intel_dvo-dev.dev_ops-mode_fixup)
 -   return intel_dvo-dev.dev_ops-mode_fixup(intel_dvo-dev, 
 mode, adjusted_mode);
 +   return intel_dvo-dev.dev_ops-mode_fixup(intel_dvo-dev,
 + 
 pipe_config-requested_mode,
 + adjusted_mode);

Since we are completely removing mode_fixup shouldn't we rip this out?


 return true;
  }
 @@ -370,7 +372,6 @@ static void intel_dvo_destroy(struct drm_connector 
 *connector)
  }

  static const struct drm_encoder_helper_funcs intel_dvo_helper_funcs = {
 -   .mode_fixup = intel_dvo_mode_fixup,
 .mode_set = intel_dvo_mode_set,
  };

 @@ -468,6 +469,7 @@ void intel_dvo_init(struct drm_device *dev)
 intel_encoder-enable = intel_enable_dvo;
 intel_encoder-get_hw_state = intel_dvo_get_hw_state;
 intel_encoder-get_config = intel_dvo_get_config;
 +   intel_encoder-compute_config = intel_dvo_compute_config;
 intel_connector-get_hw_state = intel_dvo_connector_get_hw_state;

 /* Now, try to find a controller */
 --
 1.8.1.4

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



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 13/13] drm/i915: clean up crtc timings computation

[Rodrigo Vivi]

On Sun, Jul 21, 2013 at 4:37 PM, Daniel Vetter daniel.vet...@ffwll.ch wrote:
 In the old days of the crtc helpers we've only had the encoder and
 crtc -mode_fixup callbacks. So when the lvds connector wanted to
 adjust the crtc timings it had to set a driver-private mode flag to
 tell the crtc mode fixup code to not overwrite them with the generic
 ones.

 When converting things to the new infrastructure I've kept the entire
 logic and only moved the flag to pipe_config-timings_set. But this
 logic is pretty tricky and already caused regressions:

 commit 21d8a4756af5fdf4a42e79a77cf3b6f52678d443
 Author: Daniel Vetter daniel.vet...@ffwll.ch
 Date:   Fri Jul 12 08:07:30 2013 +0200

 drm/i915: fix pfit regression for non-autoscaled resolutions

 So take advantage of the flexibility our own modeset infrastructure
 affords us and prefill default crtc timings. This allows us to rip out
 -timings_set. Note that we overwrite things again when retrying the
 pipe config computation due to bandwidth constraints to avoid bogus
 crtc timings if the encoder only does relative adjustments (which is
 how the pfit code works). Only a theoretical concern though since
 platforms where we retry (pch-split platforms) do not need
 adjustements (since only the old gmch pfit needs that). But let's
 better be safe than sorry.

 Cc: Jesse Barnes jbar...@virtuousgeek.org
 Cc: Mika Kuoppala mika.kuopp...@intel.com
 Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
 ---
  drivers/gpu/drm/i915/intel_display.c | 9 +++--
  drivers/gpu/drm/i915/intel_drv.h | 4 
  drivers/gpu/drm/i915/intel_panel.c   | 3 ---
  3 files changed, 3 insertions(+), 13 deletions(-)

 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index b07f891e..aebdadc 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -4108,12 +4108,6 @@ static int intel_crtc_compute_config(struct intel_crtc 
 *crtc,
 return -EINVAL;
 }

 -   /* All interlaced capable intel hw wants timings in frames. Note 
 though
 -* that intel_lvds_compute_config does some funny tricks with the crtc
 -* timings, so we need to be careful not to clobber these.*/
 -   if (!pipe_config-timings_set)
 -   drm_mode_set_crtcinfo(adjusted_mode, 0);
 -
 /* Cantiga+ cannot handle modes with a hsync front porch of 0.
  * WaPruneModeWithIncorrectHsyncOffset:ctg,elk,ilk,snb,ivb,vlv,hsw.
  */
 @@ -7882,6 +7876,9 @@ encoder_retry:
 pipe_config-port_clock = 0;
 pipe_config-pixel_multiplier = 1;

 +   /* Fill in default crtc timings, allow encoders to overwrite them. */
 +   drm_mode_set_crtcinfo(pipe_config-adjusted_mode, 0);
 +
 /* Pass our mode to the connectors and the CRTC to give them a chance 
 to
  * adjust it according to limitations or connector properties, and 
 also
  * a chance to reject the mode entirely.
 diff --git a/drivers/gpu/drm/i915/intel_drv.h 
 b/drivers/gpu/drm/i915/intel_drv.h
 index 31087ff..a9eca0e 100644
 --- a/drivers/gpu/drm/i915/intel_drv.h
 +++ b/drivers/gpu/drm/i915/intel_drv.h
 @@ -208,10 +208,6 @@ struct intel_crtc_config {

 struct drm_display_mode requested_mode;
 struct drm_display_mode adjusted_mode;
 -   /* This flag must be set by the encoder's compute_config callback if 
 it
 -* changes the crtc timings in the mode to prevent the crtc fixup from
 -* overwriting them.  Currently only lvds needs that. */
 -   bool timings_set;
 /* Whether to set up the PCH/FDI. Note that we never allow sharing
  * between pch encoders and cpu encoders. */
 bool has_pch_encoder;
 diff --git a/drivers/gpu/drm/i915/intel_panel.c 
 b/drivers/gpu/drm/i915/intel_panel.c
 index 67e2c1f..01b5a51 100644
 --- a/drivers/gpu/drm/i915/intel_panel.c
 +++ b/drivers/gpu/drm/i915/intel_panel.c
 @@ -194,9 +194,6 @@ void intel_gmch_panel_fitting(struct intel_crtc 
 *intel_crtc,
 adjusted_mode-vdisplay == mode-vdisplay)
 goto out;

 -   drm_mode_set_crtcinfo(adjusted_mode, 0);

I didn't get why this set here isn't needed anymore..

 -   pipe_config-timings_set = true;
 -
 switch (fitting_mode) {
 case DRM_MODE_SCALE_CENTER:
 /*
 --
 1.8.1.4

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



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 00/13] modeset interface cleanups

[Rodrigo Vivi]

Although I had those 2 doubts and aren't 100% sure this wont cause any
regression I'm confortable in give my rv-b to full series.

So,

Reviewed-by: Rodrigo Vivi rodrigo.v...@gmail.com



On Sun, Jul 21, 2013 at 4:36 PM, Daniel Vetter daniel.vet...@ffwll.ch wrote:
 Hi all,

 I've figured it's time that we rip out our legacy modeset encoder callbacks 
 from
 the crtc helpers and only use the new ones. This patch series converts over 
 the
 last few stragglers of encoder_helper_funcs-mode_set and -mode_fixup.

 As the icing on the cake I've thrown in a conversion of the old timings_set
 logic to something more natural in our new world where we precompute the 
 entire
 pipe config and can freely control the code flow in that stage.

 Comments, flames and testing highly welcome.

 Cheers, Daniel

 Daniel Vetter (13):
   drm/i915/dvo: use intel_encoder to the upcast macro
   drm/i915/dvo: switch -mode_fixup to -compute_config
   drm/i915: rip out legacy encoder-mode_fixup logic
   drm/i915/dvo: use native encoder -mode_set callback
   drm/i915/sdvo: use intel_encoder for upcast helper
   drm/i915/tv: Use native encoder-mode_set callback
   drm/i915/crt: use native encoder-mode_set callback
   drm/i915/hdmi: use native encoder mode_set callback
   drm/i915/dp: use native encoder -mode_set callback
   drm/i915/lvds: use the native encoder -mode_set callback
   drm/i915/ddi: use the native encoder -mode_set callback
   drm/i915: rip out legacy encoder-mode_set callback
   drm/i915: clean up crtc timings computation

  drivers/gpu/drm/i915/intel_crt.c | 34 +-
  drivers/gpu/drm/i915/intel_ddi.c | 39 +++--
  drivers/gpu/drm/i915/intel_display.c | 36 ---
  drivers/gpu/drm/i915/intel_dp.c  | 19 +---
  drivers/gpu/drm/i915/intel_drv.h |  4 ---
  drivers/gpu/drm/i915/intel_dvo.c | 56 
 +---
  drivers/gpu/drm/i915/intel_hdmi.c| 29 ---
  drivers/gpu/drm/i915/intel_lvds.c| 16 ---
  drivers/gpu/drm/i915/intel_panel.c   |  3 --
  drivers/gpu/drm/i915/intel_sdvo.c| 23 +++
  drivers/gpu/drm/i915/intel_tv.c  | 27 ++---
  11 files changed, 105 insertions(+), 181 deletions(-)

 --
 1.8.1.4

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



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 02/13] drm/i915/dvo: switch -mode_fixup to -compute_config

[Daniel Vetter]

On Fri, Jul 26, 2013 at 03:39:51PM -0300, Rodrigo Vivi wrote:
 On Sun, Jul 21, 2013 at 4:36 PM, Daniel Vetter daniel.vet...@ffwll.ch wrote:
  This is the last encoder -mode_fixup callback we have left, so
  convert it.
 
  Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch
  ---
   drivers/gpu/drm/i915/intel_dvo.c | 14 --
   1 file changed, 8 insertions(+), 6 deletions(-)
 
  diff --git a/drivers/gpu/drm/i915/intel_dvo.c 
  b/drivers/gpu/drm/i915/intel_dvo.c
  index d820884..1297ea3 100644
  --- a/drivers/gpu/drm/i915/intel_dvo.c
  +++ b/drivers/gpu/drm/i915/intel_dvo.c
  @@ -241,11 +241,11 @@ static int intel_dvo_mode_valid(struct drm_connector 
  *connector,
  return intel_dvo-dev.dev_ops-mode_valid(intel_dvo-dev, mode);
   }
 
  -static bool intel_dvo_mode_fixup(struct drm_encoder *encoder,
  -const struct drm_display_mode *mode,
  -struct drm_display_mode *adjusted_mode)
  +static bool intel_dvo_compute_config(struct intel_encoder *encoder,
  +struct intel_crtc_config *pipe_config)
   {
  -   struct intel_dvo *intel_dvo = enc_to_dvo(to_intel_encoder(encoder));
  +   struct intel_dvo *intel_dvo = enc_to_dvo(encoder);
  +   struct drm_display_mode *adjusted_mode = 
  pipe_config-adjusted_mode;
 
  /* If we have timings from the BIOS for the panel, put them in
   * to the adjusted mode.  The CRTC will be set up for this mode,
  @@ -267,7 +267,9 @@ static bool intel_dvo_mode_fixup(struct drm_encoder 
  *encoder,
  }
 
  if (intel_dvo-dev.dev_ops-mode_fixup)
  -   return intel_dvo-dev.dev_ops-mode_fixup(intel_dvo-dev, 
  mode, adjusted_mode);
  +   return intel_dvo-dev.dev_ops-mode_fixup(intel_dvo-dev,
  + 
  pipe_config-requested_mode,
  + adjusted_mode);
 
 Since we are completely removing mode_fixup shouldn't we rip this out?

We are ripping out the encoder-mode_fixup callback. This here is the
dvo_slave-mode_fixup callback. dvo is gen2 only, so we won't ever touch
this again. Hence why I didn't go through all 6-7 dvo slave drivers and
give them the same treatment. I'll add a note to the commit message about
this when merging, presuming there's nothing else in the patch that needs
to be fixed up.
-Daniel

 
 
  return true;
   }
  @@ -370,7 +372,6 @@ static void intel_dvo_destroy(struct drm_connector 
  *connector)
   }
 
   static const struct drm_encoder_helper_funcs intel_dvo_helper_funcs = {
  -   .mode_fixup = intel_dvo_mode_fixup,
  .mode_set = intel_dvo_mode_set,
   };
 
  @@ -468,6 +469,7 @@ void intel_dvo_init(struct drm_device *dev)
  intel_encoder-enable = intel_enable_dvo;
  intel_encoder-get_hw_state = intel_dvo_get_hw_state;
  intel_encoder-get_config = intel_dvo_get_config;
  +   intel_encoder-compute_config = intel_dvo_compute_config;
  intel_connector-get_hw_state = intel_dvo_connector_get_hw_state;
 
  /* Now, try to find a controller */
  --
  1.8.1.4
 
  ___
  Intel-gfx mailing list
  Intel-gfx@lists.freedesktop.org
  http://lists.freedesktop.org/mailman/listinfo/intel-gfx
 
 
 
 -- 
 Rodrigo Vivi
 Blog: http://blog.vivi.eng.br

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: fix gen4 digital port hotplug definitions

[Daniel Vetter]

On Fri, Jul 26, 2013 at 07:54:22PM +0200, Daniel Vetter wrote:
 On Fri, Jul 26, 2013 at 01:21:48PM +0300, Jani Nikula wrote:
  On Fri, 26 Jul 2013, Daniel Vetter daniel.vet...@ffwll.ch wrote:
   Apparently Bspec is wrong in this case here even for gm45. Note that
   Bspec is horribly misguided on i965g/gm, so we don't have any other
   data points besides that it seems to make machines work better.
  
   With this changes all the bits in PORT_HOTPLUG_STAT for the digital
   ports are ordered the same way. This seems to agree with what register
   dumps from the hpd storm handling code shows, where the LIVE bit and
   the short/long pulse STATUS bits light up at the same time with this
   enumeration (but no with the one from Bspec).
  
  Would a comment about this near the #defines be in order? To avoid the
  these values are all wrong per bspec patches.
 
 Yeah, good idea, I'll add a comment when merging this.

Done and merged with your irc-ack.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] Updated drm-intel-testing tree

[Daniel Vetter]

Hi all,

New feature tree pushed, hightlights:
- proper eLLC support for HSW from Ben
- more interrupt refactoring
- add w/a tags where we implement them already (Damien)
- hangcheck fixes (Chris) + hangcheck stats (Mika)
- flesh out the new vm structs for ppgtt and ggtt (Ben)
- PSR for Haswell, still disabled by default (Rodrigo et al.)
- pc8+ refclock sequence code from Paulo
- more interrupt refactoring from Paulo, unifying ilk/snb with the ivb/hsw
  interrupt code
- full solution for the Haswell concurrent reg access issues (Chris)
- fix racy object accounting, used by some new leak tests
- fix sync polarity settings on ch7xxx dvo encoder
- random bitspieces, little fixes and better debug output all over

Happy testing!

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] Maintainer-review fluff (was: Re: [PATCH 01/12] drm/i915: plumb VM into object operations)

[Ben Widawsky]

On Fri, Jul 26, 2013 at 11:51:00AM +0200, Daniel Vetter wrote:
 HI all,
 
 So BenI had a bit a private discussion and one thing I've explained a bit
 more in detail is what kind of review I'm doing as maintainer. I've
 figured this is generally useful. We've also discussed a bit that for
 developers without their own lab it would be nice if QA could test random
 branches on their set of machines. But imo that'll take quite a while,
 there's lots of other stuff to improve in QA land first. Anyway, here's
 it:
 
 Now an explanation for why this freaked me out, which is essentially an
 explanation of what I do when I do maintainer reviews:
 
 Probably the most important question I ask myself when reading a patch is
 if a regression would bisect to this, and the bisect is the only useful
 piece of evidence, would I stand a chance to understand it?.  Your patch
 is big, has the appearance of doing a few unrelated things and could very
 well hide a bug which would take me an awful lot of time to spot. So imo
 the answer for your patch is a clear no.
 
 I've merged a few such patches in the past  where I've had a similar hunch
 and regretted it almost always. I've also sometimes split-up the patch
 while applying, but that approach doesn't scale any more with our rather
 big team.

You should never do this, IMO. If you require the patches to be split in
your tree, the developer should do it. See below for reasons I think
this sucks.

 
 The second thing I try to figure out is whether the patch author is indeed
 the local expert on the topic at hand now. With our team size and patch
 flow I don't stand a chance if I try to understand everything to the last
 detail. Instead I try to assess this through the proxy of convincing
 myself the the patch submitter understands stuff much better than I do. I
 tend to check that by asking random questions, proposing alternative
 approaches and also by rating code/patch clarity. The obj_set_color
 double-loop very much gave me the impression that you didn't have a clear
 idea about how exactly this should work, so that  hunk trigger this
 maintainer hunch.
 
 I admit that this is all rather fluffy and very much an inexact science,
 but it's the only tools I have as a maintainer. The alternative of doing
 shit myself or checking everything myself in-depth just doesnt scale.
 
 Cheers, Daniel
 
 
 On Mon, Jul 22, 2013 at 4:08 AM, Ben Widawsky b...@bwidawsk.net wrote:

I think the subthread Jesse started had a bunch of good points, but
concisely I see 3 problems with our current process (and these were
addressed in my original mail, but I guess you didn't want to air my
dirty laundry :p):

1. Delay hurts QA. Balking on patches because they're hard to review
limits QA on that patch, and reduces QA time on the fixed up patches. I
agree this is something which is fixable within QA, but it doesn't exist
at present.

2. We don't have a way to bound review/merge. I tried to do this on this
series. After your initial review, I gave a list of things I was going
to fix, and asked you for an ack that if I fixed those, you would merge.
IMO, you didn't stick to this agreement, and came back with rework
requests on a patch I had already submitted. I don't know how to fix
this one because I think you should be entitled to change your mind.

A caveat to this: I did make some mistakes on rebase that needed
addressing. ie. the ends justified the means.

3a. Reworking code introduces bugs. I feel I am more guilty here than
most, but, consider even in the best case of those new bugs being
caught in review. In such a case, you've now introduced at least 2 extra
revs, and 2 extra lag cycles waiting for review. That assumes further
work doesn't spiral into more requested fixups, or more bugs. In the
less ideal case, you've simply introduced a new bug in addition to the
delay.

3b. Patch splitting is art not science.

There is a really delicate balance between splitting patches because
it's logically a functional split vs. splitting things up to make things
easier to chew on. Now in my case specifically, I think overall the
series has improved, and I found some crud that got squashed in which
shouldn't have been there. I also believe a lot of the splitting really
doesn't make much sense other than for review purposes and sometimes
that is okay.

In my case, I had a huge patch, but a lot of that patch was actually a
sed job of s/obj/obj,vm/.  You came back with, you're doing a bunch
of extra lookups. That was exactly the point of the patch; the extra
lookups should have made the review simpler, and could be cleaned up
later.

My point is: A larger quantity of small patches is not always easier to
review than a small quantity of large patches. Large patch series review
often requires the reviewer to keep a lot of context as they review.

*4. The result of all this is I think a lot of the time we (the
developers) end up writing your patch for you. While I respect your
opinion very highly, and I think 

Re: [Intel-gfx] Ugly patches for stolen reservation

[H. Peter Anvin]

On 07/26/2013 01:24 PM, Ingo Molnar wrote:
 
 Am I being too pedantic in expecting that we still mark it 
 e820-reserved?
 
 This area really isnt an ordinary PCI resource such as a 
 BAR or an MMIO region. It's truly system RAM (which cannot 
 be moved/reallocated), used by graphics hardware, and the 
 firmware should have marked it reserved.
 
 (The end result should be the same in any case, so this is 
 a detail.)
 

I, too, would prefer to see it marked as reserved.

-hpa


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

[Intel-gfx] Updated stolen mem patches

[Jesse Barnes]

These address the comments I've received so far, but omit the new E820
type for this mem.

Chris's patches could go on top if desired; they add a new type and
resource reservation function for looking up regions by name.  That
allows us to remove some duplicate code in the driver for finding stolen
space.

But I think these two are ready as-is.  How should we merge them?  Just
through the i915 tree since the first one touches our headers?  that and
there probably won't be conflicts on the early-quirks file; that's not
touch too often...

Thanks,
Jesse

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

[Intel-gfx] [PATCH 1/2] drm/i915: split PCI IDs out into i915_drm.h v4

[Jesse Barnes]

For use by userspace (at some point in the future) and other kernel code.

v2: move PCI IDs to uabi (Chris)
move PCI IDs to drm/ (Dave)
v3: fixup Quanta detection - needs to come first (Daniel)
v4: fix up PCI match structure init for easier use by userspace (Chris)

Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org
---
 drivers/gpu/drm/i915/i915_drv.c |  164 +++---
 include/drm/i915_drm.h  |2 +
 include/drm/i915_pciids.h   |  211 +++
 3 files changed, 247 insertions(+), 130 deletions(-)
 create mode 100644 include/drm/i915_pciids.h

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index b07362f..e87bccf 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -140,25 +140,6 @@ MODULE_PARM_DESC(fastboot, Try to skip unnecessary mode 
sets at boot time 
 static struct drm_driver driver;
 extern int intel_agp_enabled;
 
-#define INTEL_VGA_DEVICE(id, info) {   \
-   .class = PCI_BASE_CLASS_DISPLAY  16,  \
-   .class_mask = 0xff, \
-   .vendor = 0x8086,   \
-   .device = id,   \
-   .subvendor = PCI_ANY_ID,\
-   .subdevice = PCI_ANY_ID,\
-   .driver_data = (unsigned long) info }
-
-#define INTEL_QUANTA_VGA_DEVICE(info) {\
-   .class = PCI_BASE_CLASS_DISPLAY  16,  \
-   .class_mask = 0xff, \
-   .vendor = 0x8086,   \
-   .device = 0x16a,\
-   .subvendor = 0x152d,\
-   .subdevice = 0x8990,\
-   .driver_data = (unsigned long) info }
-
-
 static const struct intel_device_info intel_i830_info = {
.gen = 2, .is_mobile = 1, .cursor_needs_physical = 1, .num_pipes = 2,
.has_overlay = 1, .overlay_needs_physical = 1,
@@ -333,118 +314,41 @@ static const struct intel_device_info 
intel_haswell_m_info = {
.has_vebox_ring = 1,
 };
 
+/*
+ * Make sure any device matches here are from most specific to most
+ * general.  For example, since the Quanta match is based on the subsystem
+ * and subvendor IDs, we need it to come before the more general IVB
+ * PCI ID matches, otherwise we'll use the wrong info struct above.
+ */
+#define INTEL_PCI_IDS \
+   INTEL_I830_IDS(intel_i830_info),   \
+   INTEL_I845G_IDS(intel_845g_info),  \
+   INTEL_I85X_IDS(intel_i85x_info),   \
+   INTEL_I865G_IDS(intel_i865g_info), \
+   INTEL_I915G_IDS(intel_i915g_info), \
+   INTEL_I915GM_IDS(intel_i915gm_info),   \
+   INTEL_I945G_IDS(intel_i945g_info), \
+   INTEL_I945GM_IDS(intel_i945gm_info),   \
+   INTEL_I965G_IDS(intel_i965g_info), \
+   INTEL_G33_IDS(intel_g33_info), \
+   INTEL_I965GM_IDS(intel_i965gm_info),   \
+   INTEL_GM45_IDS(intel_gm45_info),   \
+   INTEL_G45_IDS(intel_g45_info), \
+   INTEL_PINEVIEW_IDS(intel_pineview_info),   \
+   INTEL_IRONLAKE_D_IDS(intel_ironlake_d_info),   \
+   INTEL_IRONLAKE_M_IDS(intel_ironlake_m_info),   \
+   INTEL_SNB_D_IDS(intel_sandybridge_d_info), \
+   INTEL_SNB_M_IDS(intel_sandybridge_m_info), \
+   INTEL_IVB_Q_IDS(intel_ivybridge_q_info), /* must be first IVB */ \
+   INTEL_IVB_M_IDS(intel_ivybridge_m_info),   \
+   INTEL_IVB_D_IDS(intel_ivybridge_d_info),   \
+   INTEL_HSW_D_IDS(intel_haswell_d_info), \
+   INTEL_HSW_M_IDS(intel_haswell_m_info), \
+   INTEL_VLV_M_IDS(intel_valleyview_m_info),  \
+   INTEL_VLV_D_IDS(intel_valleyview_d_info)
+
 static const struct pci_device_id pciidlist[] = {  /* aka */
-   INTEL_VGA_DEVICE(0x3577, intel_i830_info), /* I830_M */
-   INTEL_VGA_DEVICE(0x2562, intel_845g_info), /* 845_G */
-   INTEL_VGA_DEVICE(0x3582, intel_i85x_info), /* I855_GM */
-   INTEL_VGA_DEVICE(0x358e, intel_i85x_info),
-   INTEL_VGA_DEVICE(0x2572, intel_i865g_info),/* I865_G */
-   INTEL_VGA_DEVICE(0x2582, intel_i915g_info),/* I915_G */
-   INTEL_VGA_DEVICE(0x258a, intel_i915g_info),/* E7221_G */
-   INTEL_VGA_DEVICE(0x2592, intel_i915gm_info),   /* I915_GM */
-   INTEL_VGA_DEVICE(0x2772, intel_i945g_info),/* I945_G */
-   INTEL_VGA_DEVICE(0x27a2, intel_i945gm_info),   /* I945_GM */
-   INTEL_VGA_DEVICE(0x27ae, intel_i945gm_info),   /* I945_GME */
-   INTEL_VGA_DEVICE(0x2972, intel_i965g_info),/* I946_GZ */
-   INTEL_VGA_DEVICE(0x2982, intel_i965g_info),/* G35_G */
-   INTEL_VGA_DEVICE(0x2992, intel_i965g_info),/* I965_Q */
-   INTEL_VGA_DEVICE(0x29a2, intel_i965g_info),/* I965_G */
-   INTEL_VGA_DEVICE(0x29b2, intel_g33_info),  /* Q35_G */
-   

Re: [Intel-gfx] Maintainer-review fluff (was: Re: [PATCH 01/12] drm/i915: plumb VM into object operations)

[Daniel Vetter]

On Fri, Jul 26, 2013 at 10:15 PM, Ben Widawsky b...@bwidawsk.net wrote:
 I think the subthread Jesse started had a bunch of good points, but
 concisely I see 3 problems with our current process (and these were
 addressed in my original mail, but I guess you didn't want to air my
 dirty laundry :p):

I've cut out some of the later discussion in my mail (and that thread)
since I've figured it's not the main point I wanted to make. No fear
of dirty laundry ;-)


 1. Delay hurts QA. Balking on patches because they're hard to review
 limits QA on that patch, and reduces QA time on the fixed up patches. I
 agree this is something which is fixable within QA, but it doesn't exist
 at present.

Yeah, I agree that this is an issue for developers without their
private lab ;-) And it's also an issue for those with one, since
running tests without a good fully automated system is a pian.

With discussed this a bit with Jesse yesterday on irc, but my point is
that currentl QA doesn't have a quick enough turn-around even for
testing -nightly that this would be feasible. And I also think that
something like this should be started with userspace (i.e. mesa)
testing first, which is already in progress.

Once QA has infrastructure to test arbitrary branches and once they
have enough horsepower and automation (and people to do all this) we
can take a look again. But imo trying to do this early is just wishful
thinking, we have to deal with what we have, not what we'd like to get
for Xmas.

 2. We don't have a way to bound review/merge. I tried to do this on this
 series. After your initial review, I gave a list of things I was going
 to fix, and asked you for an ack that if I fixed those, you would merge.
 IMO, you didn't stick to this agreement, and came back with rework
 requests on a patch I had already submitted. I don't know how to fix
 this one because I think you should be entitled to change your mind.

 A caveat to this: I did make some mistakes on rebase that needed
 addressing. ie. the ends justified the means.

Yeah, the problem is that for really big stuff like your ppgtt series
the merge process is incremental: We'll do a rough plan and then pull
in parts one-by-one. And then when the sub-series get reviewed new
things pop up. And sometimes the reviewer is simply confused and asks
for stupid things ...

I don't think we can fix this since that's just how it works. But we
can certainly keep this in mind when estimating the effort to get
features in - big stuff will have some uncertainty (and hence need for
time buffers) even after the first review. For the ppgtt work I need
to blame myself too since the original plan was way too optimistic,
but I really wanted to get this in before you get sucked away into the
next big thing lined up (which in this case unfortunately came
attached with a deadline).

 3a. Reworking code introduces bugs. I feel I am more guilty here than
 most, but, consider even in the best case of those new bugs being
 caught in review. In such a case, you've now introduced at least 2 extra
 revs, and 2 extra lag cycles waiting for review. That assumes further
 work doesn't spiral into more requested fixups, or more bugs. In the
 less ideal case, you've simply introduced a new bug in addition to the
 delay.

I'm trying to address this by sharing rebase BKMs as much as possible.
Since I'm the one on the team doing the most rebasing (with -internal)
that hopefully helps.

 3b. Patch splitting is art not science.

 There is a really delicate balance between splitting patches because
 it's logically a functional split vs. splitting things up to make things
 easier to chew on. Now in my case specifically, I think overall the
 series has improved, and I found some crud that got squashed in which
 shouldn't have been there. I also believe a lot of the splitting really
 doesn't make much sense other than for review purposes and sometimes
 that is okay.

Imo splitting patches has two functions: Make the reviewer's life
easier (not really the developers) and have simple patches in case of
a regression which bisects to it. Ime you get about a 1-in-5
regression rate in dinq, so that chance is very much neglectable. And
for the ugly regressions where we have no clue we can easily blow
through a few man-months of engineer time to track them time.

 In my case, I had a huge patch, but a lot of that patch was actually a
 sed job of s/obj/obj,vm/.  You came back with, you're doing a bunch
 of extra lookups. That was exactly the point of the patch; the extra
 lookups should have made the review simpler, and could be cleaned up
 later.

 My point is: A larger quantity of small patches is not always easier to
 review than a small quantity of large patches. Large patch series review
 often requires the reviewer to keep a lot of context as they review.

I don't mind big sed jobs or moving functions to new files (well those
quite a bit since they're a pain for rebasing -internal). But such a
big patch needs to be 

[Intel-gfx] [GIT PULL] ACPI and power management fixes for v3.11-rc3

[Rafael J. Wysocki]

Hi Linus,

Please pull from the git repository at

  git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git 
pm+acpi-3.11-rc3

to receive ACPI and power management fixes for v3.11-rc3 with
top-most commit 8e5c2b776ae4c35f54547c017e0a943429f5748a

  Revert ACPI / video / i915: No ACPI backlight if firmware expects Windows 8

on top of commit 3b2f64d00c46e1e4e9bd0bb9bb12619adac27a4b

  Linux 3.11-rc2

These are just two fixes, a revert of the would-be backlight fix that didn't
work and an intel_pstate fix for two problems related to maximum P-state
selection.

Specifics:

- Revert of the ACPI video commit that I hoped would help fix
  backlight problems related to Windows 8 compatibility on some
  systems.  Unfortunately, it turned out to cause problems to happen
  too.

- Fix for two problems in intel_pstate, a possible failure to respond
  to a load change on a quiet system and a possible failure to select
  the highest available P-state on some systems.  From Dirk Brandewie.

Thanks!


---

Dirk Brandewie (1):
  cpufreq / intel_pstate: Change to scale off of max P-state

Rafael J. Wysocki (1):
  Revert ACPI / video / i915: No ACPI backlight if firmware expects 
Windows 8

---

 drivers/acpi/internal.h |2 --
 drivers/acpi/video.c|   67 
++-
 drivers/acpi/video_detect.c |   15 +--
 drivers/cpufreq/intel_pstate.c  |   12 -
 drivers/gpu/drm/i915/i915_dma.c |2 +-
 include/acpi/video.h|   11 +---
 include/linux/acpi.h|1 -
 7 files changed, 17 insertions(+), 93 deletions(-)

-- 
I speak only for myself.
Rafael J. Wysocki, 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 06/12] drm/i915: Use the new vm [un]bind functions

[Ben Widawsky]

On Tue, Jul 23, 2013 at 06:54:43PM +0200, Daniel Vetter wrote:
 On Sun, Jul 21, 2013 at 07:08:13PM -0700, Ben Widawsky wrote:
  Building on the last patch which created the new function pointers in
  the VM for bind/unbind, here we actually put those new function pointers
  to use.
  
  Split out as a separate patch to aid in review. I'm fine with squashing
  into the previous patch if people request it.
  
  v2: Updated to address the smart ggtt which can do aliasing as needed
  Make sure we bind to global gtt when mappable and fenceable. I thought
  we could get away without this initialy, but we cannot.
  
  Signed-off-by: Ben Widawsky b...@bwidawsk.net
 
 Meta review on the patch split: If you create new functions in a prep
 patch, then switch and then kill the old functions it's much harder to
 review whether any unwanted functional changes have been introduced.
 Reviewers have to essentially keep both the old and new code open and
 compare by hand.  And generally the really hard regression in gem have
 been due to such deeply-hidden accidental changes, and we frankly don't
 yet have the test coverage to just gloss over this.
 
 If you instead first prepare the existing functions by changing the
 arguments and logic, and then once everything is in place switch over to
 vfuncs in the 2nd patch changes will be in-place. In-place changes are
 much easier to review since diff compresses away unchanged parts.
 
 Second reason for this approach is that the functions stay at the same
 place in the source code file, which reduces the amount of spurious
 conflicts when rebasing a large set of patches around such changes ...
 
 I need to ponder this more.
 -Daniel

ping

 
  ---
   drivers/gpu/drm/i915/i915_drv.h| 10 --
   drivers/gpu/drm/i915/i915_gem.c| 37 +
   drivers/gpu/drm/i915/i915_gem_context.c|  7 ++--
   drivers/gpu/drm/i915/i915_gem_execbuffer.c | 29 
   drivers/gpu/drm/i915/i915_gem_gtt.c| 53 
  ++
   5 files changed, 37 insertions(+), 99 deletions(-)
  
  diff --git a/drivers/gpu/drm/i915/i915_drv.h 
  b/drivers/gpu/drm/i915/i915_drv.h
  index f3f2825..8d6aa34 100644
  --- a/drivers/gpu/drm/i915/i915_drv.h
  +++ b/drivers/gpu/drm/i915/i915_drv.h
  @@ -1933,18 +1933,8 @@ int i915_gem_context_destroy_ioctl(struct drm_device 
  *dev, void *data,
   
   /* i915_gem_gtt.c */
   void i915_gem_cleanup_aliasing_ppgtt(struct drm_device *dev);
  -void i915_ppgtt_bind_object(struct i915_hw_ppgtt *ppgtt,
  -   struct drm_i915_gem_object *obj,
  -   enum i915_cache_level cache_level);
  -void i915_ppgtt_unbind_object(struct i915_hw_ppgtt *ppgtt,
  - struct drm_i915_gem_object *obj);
  -
   void i915_gem_restore_gtt_mappings(struct drm_device *dev);
   int __must_check i915_gem_gtt_prepare_object(struct drm_i915_gem_object 
  *obj);
  -/* FIXME: this is never okay with full PPGTT */
  -void i915_gem_gtt_bind_object(struct drm_i915_gem_object *obj,
  -   enum i915_cache_level cache_level);
  -void i915_gem_gtt_unbind_object(struct drm_i915_gem_object *obj);
   void i915_gem_gtt_finish_object(struct drm_i915_gem_object *obj);
   void i915_gem_init_global_gtt(struct drm_device *dev);
   void i915_gem_setup_global_gtt(struct drm_device *dev, unsigned long start,
  diff --git a/drivers/gpu/drm/i915/i915_gem.c 
  b/drivers/gpu/drm/i915/i915_gem.c
  index 9ea6424..63297d7 100644
  --- a/drivers/gpu/drm/i915/i915_gem.c
  +++ b/drivers/gpu/drm/i915/i915_gem.c
  @@ -2653,12 +2653,9 @@ i915_gem_object_unbind(struct drm_i915_gem_object 
  *obj,
   
  trace_i915_gem_object_unbind(obj, vm);
   
  -   if (obj-has_global_gtt_mapping  i915_is_ggtt(vm))
  -   i915_gem_gtt_unbind_object(obj);
  -   if (obj-has_aliasing_ppgtt_mapping) {
  -   i915_ppgtt_unbind_object(dev_priv-mm.aliasing_ppgtt, obj);
  -   obj-has_aliasing_ppgtt_mapping = 0;
  -   }
  +   vma = i915_gem_obj_to_vma(obj, vm);
  +   vm-unmap_vma(vma);
  +
  i915_gem_gtt_finish_object(obj);
  i915_gem_object_unpin_pages(obj);
   
  @@ -2666,7 +2663,6 @@ i915_gem_object_unbind(struct drm_i915_gem_object 
  *obj,
  if (i915_is_ggtt(vm))
  obj-map_and_fenceable = true;
   
  -   vma = i915_gem_obj_to_vma(obj, vm);
  list_del(vma-mm_list);
  list_del(vma-vma_link);
  drm_mm_remove_node(vma-node);
  @@ -3372,7 +3368,6 @@ int i915_gem_object_set_cache_level(struct 
  drm_i915_gem_object *obj,
  enum i915_cache_level cache_level)
   {
  struct drm_device *dev = obj-base.dev;
  -   drm_i915_private_t *dev_priv = dev-dev_private;
  struct i915_vma *vma = i915_gem_obj_to_vma(obj, vm);
  int ret;
   
  @@ -3407,13 +3402,8 @@ int i915_gem_object_set_cache_level(struct 
  drm_i915_gem_object *obj,
  return ret;
  }
   
  -   if 

Re: [Intel-gfx] [PATCH 04/12] drm/i915: Track active by VMA instead of object

[Ben Widawsky]

On Tue, Jul 23, 2013 at 06:48:09PM +0200, Daniel Vetter wrote:
 On Sun, Jul 21, 2013 at 07:08:11PM -0700, Ben Widawsky wrote:
  Even though we want to be able to track active by VMA, the rest of the
  code is still using objects for most internal APIs. To solve this,
  create an object_is_active() function to help us in converting over to
  VMA usage.
  
  Because we intend to keep around some functions that care about objects,
  and not VMAs, having this function around will be useful even as we
  begin to use VMAs more in function arguments.
  
  Signed-off-by: Ben Widawsky b...@bwidawsk.net
 
 Still not really convinced. For access synchronization we don't care
 through which vm a bo is still access, only how (read/write) and when was
 the last access (ring + seqno).
 
 Note that this means that the per-vm lru doesn't really need an
 active/inactive split anymore, for evict_something we only care about the
 ordering and not whether a bo is active or not. unbind() will care but I'm
 not sure that the same bo in multiple address spaces needs to be evicted
 use-case is something we even should care about.
 
 So imo this commit needs a good justificatio for _why_ we want to track
 active per-vma. Atm I don't see a use-case, but I see complexity.
 -Daniel

I'm fine with deferring this until needed.

 
  ---
   drivers/gpu/drm/i915/i915_drv.h| 15 +++
   drivers/gpu/drm/i915/i915_gem.c| 64 
  ++
   drivers/gpu/drm/i915/i915_gem_execbuffer.c |  2 +-
   3 files changed, 48 insertions(+), 33 deletions(-)
  
  diff --git a/drivers/gpu/drm/i915/i915_drv.h 
  b/drivers/gpu/drm/i915/i915_drv.h
  index f809204..bdce9c1 100644
  --- a/drivers/gpu/drm/i915/i915_drv.h
  +++ b/drivers/gpu/drm/i915/i915_drv.h
  @@ -541,6 +541,13 @@ struct i915_vma {
  struct drm_i915_gem_object *obj;
  struct i915_address_space *vm;
   
  +   /**
  +* This is set if the object is on the active lists (has pending
  +* rendering and so a non-zero seqno), and is not set if it i s on
  +* inactive (ready to be unbound) list.
  +*/
  +   unsigned int active:1;
  +
  /** This object's place on the active/inactive lists */
  struct list_head mm_list;
   
  @@ -1266,13 +1273,6 @@ struct drm_i915_gem_object {
  struct list_head exec_list;
   
  /**
  -* This is set if the object is on the active lists (has pending
  -* rendering and so a non-zero seqno), and is not set if it i s on
  -* inactive (ready to be unbound) list.
  -*/
  -   unsigned int active:1;
  -
  -   /**
   * This is set if the object has been written to since last bound
   * to the GTT
   */
  @@ -1726,6 +1726,7 @@ static inline void i915_gem_object_unpin_pages(struct 
  drm_i915_gem_object *obj)
   int __must_check i915_mutex_lock_interruptible(struct drm_device *dev);
   int i915_gem_object_sync(struct drm_i915_gem_object *obj,
   struct intel_ring_buffer *to);
  +bool i915_gem_object_is_active(struct drm_i915_gem_object *obj);
   void i915_gem_object_move_to_active(struct drm_i915_gem_object *obj,
  struct i915_address_space *vm,
  struct intel_ring_buffer *ring);
  diff --git a/drivers/gpu/drm/i915/i915_gem.c 
  b/drivers/gpu/drm/i915/i915_gem.c
  index 6bdf89d..9ea6424 100644
  --- a/drivers/gpu/drm/i915/i915_gem.c
  +++ b/drivers/gpu/drm/i915/i915_gem.c
  @@ -119,10 +119,22 @@ int i915_mutex_lock_interruptible(struct drm_device 
  *dev)
  return 0;
   }
   
  +/* NB: Not the same as !i915_gem_object_is_inactive */
  +bool i915_gem_object_is_active(struct drm_i915_gem_object *obj)
  +{
  +   struct i915_vma *vma;
  +
  +   list_for_each_entry(vma, obj-vma_list, vma_link)
  +   if (vma-active)
  +   return true;
  +
  +   return false;
  +}
  +
   static inline bool
   i915_gem_object_is_inactive(struct drm_i915_gem_object *obj)
   {
  -   return i915_gem_obj_bound_any(obj)  !obj-active;
  +   return i915_gem_obj_bound_any(obj)  !i915_gem_object_is_active(obj);
   }
   
   int
  @@ -1883,14 +1895,14 @@ i915_gem_object_move_to_active(struct 
  drm_i915_gem_object *obj,
  }
  obj-ring = ring;
   
  +   /* Move from whatever list we were on to the tail of execution. */
  +   vma = i915_gem_obj_to_vma(obj, vm);
  /* Add a reference if we're newly entering the active list. */
  -   if (!obj-active) {
  +   if (!vma-active) {
  drm_gem_object_reference(obj-base);
  -   obj-active = 1;
  +   vma-active = 1;
  }
   
  -   /* Move from whatever list we were on to the tail of execution. */
  -   vma = i915_gem_obj_to_vma(obj, vm);
  list_move_tail(vma-mm_list, vm-active_list);
  list_move_tail(obj-ring_list, ring-active_list);
   
  @@ -1911,16 +1923,23 @@ i915_gem_object_move_to_active(struct 
  drm_i915_gem_object *obj,
   }
   
   static void
  -i915_gem_object_move_to_inactive(struct 

Re: [Intel-gfx] [PATCH 06/12] drm/i915: Use the new vm [un]bind functions

[Daniel Vetter]

On Fri, Jul 26, 2013 at 02:48:32PM -0700, Ben Widawsky wrote:
 On Tue, Jul 23, 2013 at 06:54:43PM +0200, Daniel Vetter wrote:
  On Sun, Jul 21, 2013 at 07:08:13PM -0700, Ben Widawsky wrote:
   Building on the last patch which created the new function pointers in
   the VM for bind/unbind, here we actually put those new function pointers
   to use.
   
   Split out as a separate patch to aid in review. I'm fine with squashing
   into the previous patch if people request it.
   
   v2: Updated to address the smart ggtt which can do aliasing as needed
   Make sure we bind to global gtt when mappable and fenceable. I thought
   we could get away without this initialy, but we cannot.
   
   Signed-off-by: Ben Widawsky b...@bwidawsk.net
  
  Meta review on the patch split: If you create new functions in a prep
  patch, then switch and then kill the old functions it's much harder to
  review whether any unwanted functional changes have been introduced.
  Reviewers have to essentially keep both the old and new code open and
  compare by hand.  And generally the really hard regression in gem have
  been due to such deeply-hidden accidental changes, and we frankly don't
  yet have the test coverage to just gloss over this.
  
  If you instead first prepare the existing functions by changing the
  arguments and logic, and then once everything is in place switch over to
  vfuncs in the 2nd patch changes will be in-place. In-place changes are
  much easier to review since diff compresses away unchanged parts.
  
  Second reason for this approach is that the functions stay at the same
  place in the source code file, which reduces the amount of spurious
  conflicts when rebasing a large set of patches around such changes ...
  
  I need to ponder this more.
  -Daniel
 
 ping

Keep it in mind for next time around. I think my general approach is
easier on reviewers ... but hey, vacation!
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 00/13] modeset interface cleanups

[Daniel Vetter]

On Fri, Jul 26, 2013 at 03:42:57PM -0300, Rodrigo Vivi wrote:
 Although I had those 2 doubts and aren't 100% sure this wont cause any
 regression I'm confortable in give my rv-b to full series.

I've added my answers to both questions to the commit messages.
 
 So,
 
 Reviewed-by: Rodrigo Vivi rodrigo.v...@gmail.com

Entire series merged to dinq, thanks for the review.
-Daniel

 
 
 
 On Sun, Jul 21, 2013 at 4:36 PM, Daniel Vetter daniel.vet...@ffwll.ch wrote:
  Hi all,
 
  I've figured it's time that we rip out our legacy modeset encoder callbacks 
  from
  the crtc helpers and only use the new ones. This patch series converts over 
  the
  last few stragglers of encoder_helper_funcs-mode_set and -mode_fixup.
 
  As the icing on the cake I've thrown in a conversion of the old timings_set
  logic to something more natural in our new world where we precompute the 
  entire
  pipe config and can freely control the code flow in that stage.
 
  Comments, flames and testing highly welcome.
 
  Cheers, Daniel
 
  Daniel Vetter (13):
drm/i915/dvo: use intel_encoder to the upcast macro
drm/i915/dvo: switch -mode_fixup to -compute_config
drm/i915: rip out legacy encoder-mode_fixup logic
drm/i915/dvo: use native encoder -mode_set callback
drm/i915/sdvo: use intel_encoder for upcast helper
drm/i915/tv: Use native encoder-mode_set callback
drm/i915/crt: use native encoder-mode_set callback
drm/i915/hdmi: use native encoder mode_set callback
drm/i915/dp: use native encoder -mode_set callback
drm/i915/lvds: use the native encoder -mode_set callback
drm/i915/ddi: use the native encoder -mode_set callback
drm/i915: rip out legacy encoder-mode_set callback
drm/i915: clean up crtc timings computation
 
   drivers/gpu/drm/i915/intel_crt.c | 34 +-
   drivers/gpu/drm/i915/intel_ddi.c | 39 +++--
   drivers/gpu/drm/i915/intel_display.c | 36 ---
   drivers/gpu/drm/i915/intel_dp.c  | 19 +---
   drivers/gpu/drm/i915/intel_drv.h |  4 ---
   drivers/gpu/drm/i915/intel_dvo.c | 56 
  +---
   drivers/gpu/drm/i915/intel_hdmi.c| 29 ---
   drivers/gpu/drm/i915/intel_lvds.c| 16 ---
   drivers/gpu/drm/i915/intel_panel.c   |  3 --
   drivers/gpu/drm/i915/intel_sdvo.c| 23 +++
   drivers/gpu/drm/i915/intel_tv.c  | 27 ++---
   11 files changed, 105 insertions(+), 181 deletions(-)
 
  --
  1.8.1.4
 
  ___
  Intel-gfx mailing list
  Intel-gfx@lists.freedesktop.org
  http://lists.freedesktop.org/mailman/listinfo/intel-gfx
 
 
 
 -- 
 Rodrigo Vivi
 Blog: http://blog.vivi.eng.br

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: Adjustable Render Power State policies

[Chris Wilson]

As a complement to the suggestions put forward by Jesse in

drm/i915: Scotty, I need more power!
(1372436290-3297-1-git-send-email-jbar...@virtuousgeek.org)

and

drm/i915: boost GPU and CPU freq when leaving idle
(1372438472-3233-1-git-send-email-jbar...@virtuousgeek.org)

we also have the ability to fine tune how we respond to the GPU's
requests for more power.

This patch introduces the i915.rps_policy module parameter that adjusts
how we respond to the upclock request:

  1 - Powersaving (current implementation)
  Increase the frequency by one bin on every upclock request.

  2 - Conservative (new default)
  When waking from idle, boost the render frequency to the
  default render frequency (RP1)

  3 - Ondemand
  At every upclock request, increase to halfway between the current
  frequency and maximum. This should give rapid upclocking similar
  to tcp window scaling.

  4 - Performance
  Every time the GPU wants more power, give it everything we have.

Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
---
 drivers/gpu/drm/i915/i915_drv.c |  8 +++
 drivers/gpu/drm/i915/i915_drv.h |  2 ++
 drivers/gpu/drm/i915/i915_irq.c | 46 +
 drivers/gpu/drm/i915/intel_pm.c |  1 +
 4 files changed, 44 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 1d8cedc..7ad80f4 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -72,6 +72,14 @@ MODULE_PARM_DESC(i915_enable_rc6,
For example, 3 would enable rc6 and deep rc6, and 7 would 
enable everything. 
default: -1 (use per-chip default));
 
+int i915_rps_policy __read_mostly = 1;
+module_param_named(rps_policy, i915_rps_policy, int, 0600);
+MODULE_PARM_DESC(rps_policy,
+   Specify a policy to use to respond to requests to change the 
render clock. 
+   Different levels of uplocking agressiveness can be selected
+   (0 = Powersaving, slowly uplock; 1 = Conservative, when the 
gpu wakes from idle go straight to RP1/RPe (recommended minimum active GPU 
frequency); 2 = Ondemand, rapidly increase frequency whilst upclocking; 4 = 
Performance, always upclock to maximum frequency). 
+   default: 1 (Conservative));
+
 int i915_enable_fbc __read_mostly = -1;
 module_param_named(i915_enable_fbc, i915_enable_fbc, int, 0600);
 MODULE_PARM_DESC(i915_enable_fbc,
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ec14124..71232bc 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -811,6 +811,7 @@ struct intel_gen6_power_mgmt {
u8 min_delay;
u8 max_delay;
u8 rpe_delay;
+   u8 rp1_delay;
u8 hw_max;
 
struct delayed_work delayed_resume_work;
@@ -1635,6 +1636,7 @@ extern int i915_lvds_channel_mode __read_mostly;
 extern int i915_panel_use_ssc __read_mostly;
 extern int i915_vbt_sdvo_panel_type __read_mostly;
 extern int i915_enable_rc6 __read_mostly;
+extern int i915_rps_policy __read_mostly;
 extern int i915_enable_fbc __read_mostly;
 extern bool i915_enable_hangcheck __read_mostly;
 extern int i915_enable_ppgtt __read_mostly;
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index ee3e49c..edd73c0 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -728,14 +728,32 @@ static void gen6_pm_rps_work(struct work_struct *work)
mutex_lock(dev_priv-rps.hw_lock);
 
if (pm_iir  GEN6_PM_RP_UP_THRESHOLD) {
-   new_delay = dev_priv-rps.cur_delay + 1;
+   switch (i915_rps_policy) {
+   case 0: /* Powersave, slow increase in frequency */
+   new_delay = dev_priv-rps.cur_delay + 1;
+   break;
+
+   case 1: /* Conservative, jump directly to RPe */
+   new_delay = dev_priv-rps.rp1_delay;
+   if (dev_priv-rps.cur_delay = new_delay)
+   new_delay = dev_priv-rps.cur_delay + 1;
+   break;
+
+   case 2: /* Ondemand, halfway to maximum */
+   new_delay = dev_priv-rps.cur_delay + 1;
+   if (new_delay  dev_priv-rps.rp1_delay)
+   new_delay = dev_priv-rps.rp1_delay;
+   else
+   new_delay = (dev_priv-rps.max_delay + 
new_delay)  1;
+   break;
+
+   default: /* Performance, straight to maximum */
+   new_delay = dev_priv-rps.max_delay;
+   break;
+   }
 
-   /*
-* For better performance, jump directly
-* to RPe if we're below it.
-*/
if (IS_VALLEYVIEW(dev_priv-dev) 
-   dev_priv-rps.cur_delay  

Re: [Intel-gfx] [PATCH] drm/i915: Adjustable Render Power State policies

[Jesse Barnes]

Cool... I'd like to see if Ouping and Mengmeng can work together to get
power  power numbers across a bunch of workloads for these policies.

Ouping  Mengmeng, can you run your usual battery of stuff against each
of these settings and report back the results?

Thanks,
Jesse

On Fri, 26 Jul 2013 23:24:11 +0100
Chris Wilson ch...@chris-wilson.co.uk wrote:

 As a complement to the suggestions put forward by Jesse in
 
 drm/i915: Scotty, I need more power!
 (1372436290-3297-1-git-send-email-jbar...@virtuousgeek.org)
 
 and
 
 drm/i915: boost GPU and CPU freq when leaving idle
 (1372438472-3233-1-git-send-email-jbar...@virtuousgeek.org)
 
 we also have the ability to fine tune how we respond to the GPU's
 requests for more power.
 
 This patch introduces the i915.rps_policy module parameter that adjusts
 how we respond to the upclock request:
 
   1 - Powersaving (current implementation)
   Increase the frequency by one bin on every upclock request.
 
   2 - Conservative (new default)
   When waking from idle, boost the render frequency to the
   default render frequency (RP1)
 
   3 - Ondemand
   At every upclock request, increase to halfway between the current
   frequency and maximum. This should give rapid upclocking similar
   to tcp window scaling.
 
   4 - Performance
   Every time the GPU wants more power, give it everything we have.
 
 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 ---
  drivers/gpu/drm/i915/i915_drv.c |  8 +++
  drivers/gpu/drm/i915/i915_drv.h |  2 ++
  drivers/gpu/drm/i915/i915_irq.c | 46 
 +
  drivers/gpu/drm/i915/intel_pm.c |  1 +
  4 files changed, 44 insertions(+), 13 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
 index 1d8cedc..7ad80f4 100644
 --- a/drivers/gpu/drm/i915/i915_drv.c
 +++ b/drivers/gpu/drm/i915/i915_drv.c
 @@ -72,6 +72,14 @@ MODULE_PARM_DESC(i915_enable_rc6,
   For example, 3 would enable rc6 and deep rc6, and 7 would 
 enable everything. 
   default: -1 (use per-chip default));
  
 +int i915_rps_policy __read_mostly = 1;
 +module_param_named(rps_policy, i915_rps_policy, int, 0600);
 +MODULE_PARM_DESC(rps_policy,
 + Specify a policy to use to respond to requests to change the 
 render clock. 
 + Different levels of uplocking agressiveness can be selected
 + (0 = Powersaving, slowly uplock; 1 = Conservative, when the 
 gpu wakes from idle go straight to RP1/RPe (recommended minimum active GPU 
 frequency); 2 = Ondemand, rapidly increase frequency whilst upclocking; 4 = 
 Performance, always upclock to maximum frequency). 
 + default: 1 (Conservative));
 +
  int i915_enable_fbc __read_mostly = -1;
  module_param_named(i915_enable_fbc, i915_enable_fbc, int, 0600);
  MODULE_PARM_DESC(i915_enable_fbc,
 diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
 index ec14124..71232bc 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -811,6 +811,7 @@ struct intel_gen6_power_mgmt {
   u8 min_delay;
   u8 max_delay;
   u8 rpe_delay;
 + u8 rp1_delay;
   u8 hw_max;
  
   struct delayed_work delayed_resume_work;
 @@ -1635,6 +1636,7 @@ extern int i915_lvds_channel_mode __read_mostly;
  extern int i915_panel_use_ssc __read_mostly;
  extern int i915_vbt_sdvo_panel_type __read_mostly;
  extern int i915_enable_rc6 __read_mostly;
 +extern int i915_rps_policy __read_mostly;
  extern int i915_enable_fbc __read_mostly;
  extern bool i915_enable_hangcheck __read_mostly;
  extern int i915_enable_ppgtt __read_mostly;
 diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
 index ee3e49c..edd73c0 100644
 --- a/drivers/gpu/drm/i915/i915_irq.c
 +++ b/drivers/gpu/drm/i915/i915_irq.c
 @@ -728,14 +728,32 @@ static void gen6_pm_rps_work(struct work_struct *work)
   mutex_lock(dev_priv-rps.hw_lock);
  
   if (pm_iir  GEN6_PM_RP_UP_THRESHOLD) {
 - new_delay = dev_priv-rps.cur_delay + 1;
 + switch (i915_rps_policy) {
 + case 0: /* Powersave, slow increase in frequency */
 + new_delay = dev_priv-rps.cur_delay + 1;
 + break;
 +
 + case 1: /* Conservative, jump directly to RPe */
 + new_delay = dev_priv-rps.rp1_delay;
 + if (dev_priv-rps.cur_delay = new_delay)
 + new_delay = dev_priv-rps.cur_delay + 1;
 + break;
 +
 + case 2: /* Ondemand, halfway to maximum */
 + new_delay = dev_priv-rps.cur_delay + 1;
 + if (new_delay  dev_priv-rps.rp1_delay)
 + new_delay = dev_priv-rps.rp1_delay;
 + else
 + new_delay = (dev_priv-rps.max_delay + 
 new_delay)  1;
 + break;
 +
 +

Re: [Intel-gfx] Maintainer-review fluff (was: Re: [PATCH 01/12] drm/i915: plumb VM into object operations)

[Dave Airlie]

 Hey, overall it's actually quite a bit of fun.

 I do agree that QA is really important for a fastpaced process, but
 it's also not the only peace to get something in. Review (both of the
 patch itself but also of  the test coverage) catches a lot of issues,
 and in many cases not the same ones as QA would. Especially if the
 testcoverage of a new feature is less than stellar, which imo is still
 the case for gem due to the tons of finickle cornercases.

Just my 2c worth on this topic, since I like the current process, and
I believe making it too formal is probably going to make things suck
too much.

I'd rather Daniel was slowing you guys down up front more, I don't
give a crap about Intel project management or personal manager relying
on getting features merged when, I do care that you engineers when you
merge something generally get transferred 100% onto something else and
don't react strongly enough to issues on older code you have created
that either have lain dormant since patches merged or are regressions
since patches merged. So I believe the slowing down of merging
features gives a better chance of QA or other random devs of finding
the misc regressions while you are still focused on the code and
hitting the long term bugs that you guys rarely get resourced to fix
unless I threaten to stop pulling stuff.

So whatever Daniel says goes as far as I'm concerned, if I even
suspect he's taken some internal Intel pressure to merge some feature,
I'm going to stop pulling from him faster than I stopped pulling from
the previous maintainers :-), so yeah engineers should be prepared to
backup what they post even if Daniel is wrong, but on the other hand
they need to demonstrate they understand the code they are pushing and
sometimes with ppgtt and contexts I'm not sure anyone really
understands how the hw works let alone the sw :-P

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

Re: [Intel-gfx] Maintainer-review fluff (was: Re: [PATCH 01/12] drm/i915: plumb VM into object operations)

[Ben Widawsky]

On Sat, Jul 27, 2013 at 09:13:38AM +1000, Dave Airlie wrote:
 
  Hey, overall it's actually quite a bit of fun.
 
  I do agree that QA is really important for a fastpaced process, but
  it's also not the only peace to get something in. Review (both of the
  patch itself but also of  the test coverage) catches a lot of issues,
  and in many cases not the same ones as QA would. Especially if the
  testcoverage of a new feature is less than stellar, which imo is still
  the case for gem due to the tons of finickle cornercases.
 
 Just my 2c worth on this topic, since I like the current process, and
 I believe making it too formal is probably going to make things suck
 too much.
 
 I'd rather Daniel was slowing you guys down up front more, I don't
 give a crap about Intel project management or personal manager relying
 on getting features merged when, I do care that you engineers when you
 merge something generally get transferred 100% onto something else and
 don't react strongly enough to issues on older code you have created
 that either have lain dormant since patches merged or are regressions
 since patches merged. So I believe the slowing down of merging
 features gives a better chance of QA or other random devs of finding
 the misc regressions while you are still focused on the code and
 hitting the long term bugs that you guys rarely get resourced to fix
 unless I threaten to stop pulling stuff.
 
 So whatever Daniel says goes as far as I'm concerned, if I even
 suspect he's taken some internal Intel pressure to merge some feature,
 I'm going to stop pulling from him faster than I stopped pulling from
 the previous maintainers :-), so yeah engineers should be prepared to
 backup what they post even if Daniel is wrong, but on the other hand
 they need to demonstrate they understand the code they are pushing and
 sometimes with ppgtt and contexts I'm not sure anyone really
 understands how the hw works let alone the sw :-P
 
 Dave.

Honestly, I wouldn't have responded if you didn't mention the Intel
program management thing...

The problem I am trying to emphasize, and let's use contexts/ppgtt as an
example, is we have three options:
1. It's complicated, and a big change, so let's not do it.
2. I continue to rebase the massive change on top of the extremely fast
paced i915 tree, with no QA coverage.
3. We get decent bits merged ASAP by putting it in a repo that both gets
much wider usage than my personal branch, and gets nightly QA coverage.

PPGTT + Contexts have existed for a while, and so we went with #1 for
quite a while.

Now we're at #2. There's two sides to your 'developer needs to
defend...' I need Daniel to give succinct feedback, and agree upon steps
required to get code merged. My original gripe was that it's hard to
deal with the, that patch is too big comments almost 2 months after
the first version was sent. Equally, that looks funny without a real
explanation of what looks funny, or sufficient thought up front about
what might look better is just as hard to deal with. Inevitably, yes -
it's a big scary series of patches - but if we're honest with ourselves,
it's almost guaranteed to blow up somewhere regardless of how much we
rework it, and who reviews it. Blowing up long before you merge would
always be better than the after you merge.

My desire is to get to something like #3. I had a really long paragraph
on why and how we could do that, but I've redacted it. Let's just leave
it as, I think that should be the goal.

Finally, let me clear that none of the discussion I'm having with Daniel
that spawned this thread are inspired by Intel program management. My
personal opinion is that your firm stance has really helped us
internally to fight back stupid decisions. Honestly, I wish you had a
more direct input into our management, and product planners.

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