date:20140530

On Thu, May 29, 2014 at 03:10:13PM +0530, sourab.gu...@intel.com wrote:
> + if (intel_use_mmio_flip(dev))
> + dev_priv->display.queue_flip = intel_queue_mmio_flip;
> +

Note that this patch creates the i915.use_mmio_flip as 0600 so this
cannot be a static assignment anyway.
-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 3/3] drm/i915: Make module param for MMIO flip selection as tristate

I was thinking this patch should be more like

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3201495..ab9b5f7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2060,7 +2060,7 @@ struct i915_params {
bool reset;
bool disable_display;
bool disable_vtd_wa;
-   bool use_mmio_flip;
+   int use_mmio_flip;
 };
 extern struct i915_params i915 __read_mostly;
 
diff --git a/drivers/gpu/drm/i915/i915_params.c 
b/drivers/gpu/drm/i915/i915_params.c
index e0d44df..6d7c580 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -158,5 +158,5 @@ module_param_named(enable_cmd_parser, 
i915.enable_cmd_parser, int, 0600);
 MODULE_PARM_DESC(enable_cmd_parser,
 "Enable command parsing (1=enabled [default], 0=disabled)");
 
-module_param_named(use_mmio_flip, i915.use_mmio_flip, bool, 0600);
-MODULE_PARM_DESC(use_mmio_flip, "use MMIO flips (default: false)");
+module_param_named(use_mmio_flip, i915.use_mmio_flip, int, 0600);
+MODULE_PARM_DESC(use_mmio_flip, "use MMIO flips (-1=never, 0=driver discretion 
[default], 1=always)");
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index ac93ae4..b6c8fce 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9207,24 +9207,24 @@ static int intel_gen7_queue_flip(struct drm_device *dev,
return 0;
 }
 
-static bool intel_use_mmio_flip(struct drm_device *dev)
+static bool use_mmio_flip(struct intel_engine_cs *ring,
+ struct drm_i915_gem_object *obj)
 {
-   /* If module parameter is disabled, use CS flips.
-* Otherwise, use MMIO flips starting from Gen5.
-* This is not being used for older platforms, because
+   /* This is not being used for older platforms, because
 * non-availability of flip done interrupt forces us to use
 * CS flips. Older platforms derive flip done using some clever
 * tricks involving the flip_pending status bits and vblank irqs.
 * So using MMIO flips there would disrupt this mechanism.
 */
-
-   if (i915.use_mmio_flip == 0)
+   if (INTEL_INFO(dev)->gen < 5)
return false;
 
-   if (INTEL_INFO(dev)->gen >= 5)
+   if (i915.use_mmio_flip < 0)
+   return false;
+   else if (i915.use_mmio_flip > 0)
return true;
else
-   return false;
+   return ring != obj->ring;
 }
 
 static void intel_do_mmio_flip(struct intel_crtc *intel_crtc)
@@ -9290,9 +9290,9 @@ void intel_notify_mmio_flip(struct intel_engine_cs *ring)
struct intel_mmio_flip *mmio_flip;
 
mmio_flip = &intel_crtc->mmio_flip;
-
if (mmio_flip->seqno == 0)
continue;
+
if (ring->id != mmio_flip->ring_id)
continue;
 
@@ -9306,26 +9306,25 @@ void intel_notify_mmio_flip(struct intel_engine_cs 
*ring)
 }
 
 static int intel_queue_mmio_flip(struct drm_device *dev,
-   struct drm_crtc *crtc,
-   struct drm_framebuffer *fb,
-   struct drm_i915_gem_object *obj,
-   struct intel_engine_cs *ring,
-   uint32_t flags)
+struct drm_crtc *crtc,
+struct drm_framebuffer *fb,
+struct drm_i915_gem_object *obj,
+struct intel_engine_cs *ring,
+uint32_t flags)
 {
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
unsigned long irq_flags;
int ret;
 
-   if (WARN_ON(intel_crtc->mmio_flip.seqno)) {
-   ret = -EBUSY;
-   goto err;
-   }
+   if (WARN_ON(intel_crtc->mmio_flip.seqno))
+   return -EBUSY;
 
ret = intel_postpone_flip(obj);
-   if (ret < 0) {
-   goto err;
-   } else if (ret == 0) {
+   if (ret < 0)
+   return ret;
+
+   if (ret == 0) {
intel_do_mmio_flip(intel_crtc);
return 0;
}
@@ -9340,8 +9339,6 @@ static int intel_queue_mmio_flip(struct drm_device *dev,
 */
intel_notify_mmio_flip(obj->ring);
return 0;
-err:
-   return ret;
 }
 
 static int intel_default_queue_flip(struct drm_device *dev,
@@ -9529,7 +9526,10 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
i915_gem_obj_ggtt_offset(obj) + intel_crtc->dspaddr_offset;
work->enable_stall_check = true;
 
-   ret = dev_priv->display.queue_flip(dev, crtc, fb, obj, ring, 
page_flip_flags);
+   if (use_mmio_flip(ring, obj))
+   ret = intel_queue_mmio_flip(ring, obj)(dev, crtc, fb, obj, 
ring, page_flip

[Intel-gfx] [Intel 945] BSM: How to determine size of DRAM used for internal graphics?

2014-05-30 Thread Paul Menzel

Dear Intel graphics folks,

since commit 17fec8a0 [1]

drm/i915: Use Graphics Base of Stolen Memory on all gen3+

Linux reads the register BSM (Base of Stolen Memory) directly to get the
base address of graphics stolen memory. With coreboot [2] and native
graphics init – note that everything works with the proprietary VGA
BIOS/Option ROM – this causes a regression [3] as this register is not
programmed at all.

From the datasheet *Mobile Intel® 945 Express Chipset Family* [4] the
register BSM is described on page 290.

Graphics Stolen Memory and TSEG are within DRAM space defined
under TOLUD. From the top of low used DRAM, (G)MCH claims 1 to
64 MBs of DRAM for internal graphics if enabled.

This register contains bits 31 to 20 of the base address of
stolen
DRAM memory. The host interface determines the base of
graphics stolen memory by subtracting the graphics stolen
memory size from TOLUD. See Device 0 TOLUD for more
explanations.

Also see Figure 12 *Main Memory Address Range* in section 9.2 on page
325.

Unfortunately I am unable to find out how the graphics stolen memory
size is determined. I’d have thought it is used for the framebuffer, but
according to page 93 (Graphics Mode select (GMS)) that the framebuffer
size can only be 1 MB or 8 MB, which contradicts that it can be up to 64
MB.

If it is determined implicitly by the value I set the BSM to, where can
I find the recommendations what size to use? I’d guess it is dependent
on the RAM size, that means dependent if the system has 512 MB or 4 GB
for example.

Thanks,

Paul

[1]
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=17fec8a08698bcab98788e1e89f5b8e7502ababd
[2] http://www.coreboot.org/
[3] https://bugs.freedesktop.org/show_bug.cgi?id=79038
[4]
http://www.intel.com/content/dam/www/public/us/en/documents/datasheets/mobile-945-express-chipset-datasheet.pdf
Document Number: 309219-006

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

Re: [Intel-gfx] [Intel 945] BSM: How to determine size of DRAM used for internal graphics?

2014-05-30 Thread Paul Menzel

Dear Intel graphics folks,



Am Freitag, den 30.05.2014, 13:45 +0200 schrieb Paul Menzel:

> since commit 17fec8a0 [1]
> 
> drm/i915: Use Graphics Base of Stolen Memory on all gen3+
> 
> Linux reads the register BSM (Base of Stolen Memory) directly to get the
> base address of graphics stolen memory. With coreboot [2] and native
> graphics init – note that everything works with the proprietary VGA
> BIOS/Option ROM – this causes a regression [3] as this register is not
> programmed at all.
> 
> From the datasheet *Mobile Intel® 945 Express Chipset Family* [4] the
> register BSM is described on page 290.
> 
> Graphics Stolen Memory and TSEG are within DRAM space defined
> under TOLUD. From the top of low used DRAM, (G)MCH claims 1 to
> 64 MBs of DRAM for internal graphics if enabled.
> 
> This register contains bits 31 to 20 of the base address of
> stolen
> DRAM memory. The host interface determines the base of
> graphics stolen memory by subtracting the graphics stolen
> memory size from TOLUD. See Device 0 TOLUD for more
> explanations.
> 
> Also see Figure 12 *Main Memory Address Range* in section 9.2 on page
> 325.
> 
> Unfortunately I am unable to find out how the graphics stolen memory
> size is determined. I’d have thought it is used for the framebuffer, but
> according to page 93 (Graphics Mode select (GMS)) that the framebuffer
> size can only be 1 MB or 8 MB, which contradicts that it can be up to 64
> MB.
> 
> If it is determined implicitly by the value I set the BSM to, where can
> I find the recommendations what size to use? I’d guess it is dependent
> on the RAM size, that means dependent if the system has 512 MB or 4 GB
> for example.

The datasheet documents the bits of the register BSM as Read Only (RO).
So I am even more confused now.


Thanks,

Paul


> [1] 
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=17fec8a08698bcab98788e1e89f5b8e7502ababd
> [2] http://www.coreboot.org/
> [3] https://bugs.freedesktop.org/show_bug.cgi?id=79038
> [4] 
> http://www.intel.com/content/dam/www/public/us/en/documents/datasheets/mobile-945-express-chipset-datasheet.pdf
> Document Number: 309219-006


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

Re: [Intel-gfx] [PATCH 1/4] drm/i915: disable power wells on suspend

On Thu, 2014-05-29 at 14:11 -0700, Jesse Barnes wrote:
> From: Kristen Carlson Accardi 
> 
> We want to make sure everything is disabled and at its lowest power when
> freezing.
> 
> Signed-off-by: Kristen Carlson Accardi 
> Signed-off-by: Jesse Barnes 

Looks ok to me:
Reviewed-by: Imre Deak 

> ---
>  drivers/gpu/drm/i915/i915_drv.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index e2bfdda..66c6ffb 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -551,6 +551,8 @@ static int i915_drm_freeze(struct drm_device *dev)
>  
>   dev_priv->suspend_count++;
>  
> + intel_display_set_init_power(dev_priv, false);
> +
>   return 0;
>  }
>  



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

Re: [Intel-gfx] [PATCH 2/4] drm/i915: leave rc6 enabled at suspend time

On Thu, 2014-05-29 at 14:11 -0700, Jesse Barnes wrote:
> From: Kristen Carlson Accardi 
> 
> This allows the system to enter the lowest power mode during system freeze.
> 
> Signed-off-by: Jesse Barnes 
> ---
>  drivers/gpu/drm/i915/i915_drv.c  |  3 ---
>  drivers/gpu/drm/i915/intel_drv.h |  1 +
>  drivers/gpu/drm/i915/intel_pm.c  | 16 +++-
>  3 files changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 66c6ffb..433bdfa 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -521,8 +521,6 @@ static int i915_drm_freeze(struct drm_device *dev)
>   drm_irq_uninstall(dev);
>   dev_priv->enable_hotplug_processing = false;
>  
> - intel_disable_gt_powersave(dev);
> -

I wonder what was the reason for this call. One possibility is that
i915_save_state() depends on it to save the correct registers, but it
would be good to clarify this.

It also cancels some deferred works which we do need here. But we could
also add that to intel_enable_gt_powersave_sync() in this patch.

>   /*
>* Disable CRTCs directly since we want to preserve sw state
>* for _thaw.
> @@ -543,7 +541,6 @@ static int i915_drm_freeze(struct drm_device *dev)
>   i915_save_state(dev);
>  
>   intel_opregion_fini(dev);
> - intel_uncore_fini(dev);

Some stuff in the above call is unrelated to this patch, like
intel_uncore_forcewake_reset(). At least canceling force_wake_timer
there seems to be needed here. In any case it would be better to have
the above two changes in a separate patch.

With that fixed this patch looks ok to me. The original patch was from
me, so fwiw:
Reviewed-by: Imre Deak 
 
>   console_lock();
>   intel_fbdev_set_suspend(dev, FBINFO_STATE_SUSPENDED);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> b/drivers/gpu/drm/i915/intel_drv.h
> index c597b0d..bf90e7d 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -956,6 +956,7 @@ void intel_power_domains_init_hw(struct drm_i915_private 
> *dev_priv);
>  void intel_init_gt_powersave(struct drm_device *dev);
>  void intel_cleanup_gt_powersave(struct drm_device *dev);
>  void intel_enable_gt_powersave(struct drm_device *dev);
> +void intel_enable_gt_powersave_sync(struct drm_device *dev);
>  void intel_disable_gt_powersave(struct drm_device *dev);
>  void intel_reset_gt_powersave(struct drm_device *dev);
>  void ironlake_teardown_rc6(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 1840d15..8d9e036 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4891,12 +4891,9 @@ void intel_disable_gt_powersave(struct drm_device *dev)
>   }
>  }
>  
> -static void intel_gen6_powersave_work(struct work_struct *work)
> +void intel_enable_gt_powersave_sync(struct drm_device *dev)
>  {
> - struct drm_i915_private *dev_priv =
> - container_of(work, struct drm_i915_private,
> -  rps.delayed_resume_work.work);
> - struct drm_device *dev = dev_priv->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
>  
>   mutex_lock(&dev_priv->rps.hw_lock);
>  
> @@ -4917,6 +4914,15 @@ static void intel_gen6_powersave_work(struct 
> work_struct *work)
>   intel_runtime_pm_put(dev_priv);
>  }
>  
> +static void intel_gen6_powersave_work(struct work_struct *work)
> +{
> + struct drm_i915_private *dev_priv =
> + container_of(work, struct drm_i915_private,
> +  rps.delayed_resume_work.work);
> +
> + intel_enable_gt_powersave_sync(dev_priv->dev);
> +}
> +
>  void intel_enable_gt_powersave(struct drm_device *dev)
>  {
>   struct drm_i915_private *dev_priv = dev->dev_private;



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

[Intel-gfx] [PATCH] drm/i915: Only mark the ctx as initialised after a SET_CONTEXT operation

Fallout from

commit 46470fc932ac8a0e8317a220b3f4ea4ed903338e
Author: Mika Kuoppala 
Date:   Wed May 21 19:01:06 2014 +0300

drm/i915: Add null state batch to active list

undid the earlier fix of only marking the ctx as initialised after it is
saved by the hardware during a SET_CONTEXT operation.

Signed-off-by: Chris Wilson 
Cc: Ville Syrjälä 
Cc: Damien Lespiau 
Cc: Mika Kuoppala 
Cc: Ben Widawsky 
---
 drivers/gpu/drm/i915/i915_gem_context.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index 5a71ef1975b3..34a0b49e6add 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -641,6 +641,7 @@ static int do_switch(struct intel_engine_cs *ring,
struct intel_context *from = ring->last_context;
struct i915_hw_ppgtt *ppgtt = ctx_to_ppgtt(to);
u32 hw_flags = 0;
+   bool uninitialized = false;
int ret, i;
 
if (from != NULL && ring == &dev_priv->ring[RCS]) {
@@ -739,18 +740,19 @@ static int do_switch(struct intel_engine_cs *ring,
i915_gem_context_unreference(from);
}
 
+   uninitialized = !to->is_initialized && from == NULL;
+   to->is_initialized = true;
+
 done:
i915_gem_context_reference(to);
ring->last_context = to;
 
-   if (ring->id == RCS && !to->is_initialized && from == NULL) {
+   if (uninitialized) {
ret = i915_gem_render_state_init(ring);
if (ret)
DRM_ERROR("init render state: %d\n", ret);
}
 
-   to->is_initialized = true;
-
return 0;
 
 unpin_out:
-- 
2.0.0.rc4

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

[Intel-gfx] [PATCH v2] drm/i915: Always apply cursor width changes

It is possible for userspace to create a big object large enough for a
256x256, and then switch over to using it as a 64x64 cursor. This
requires the cursor update routines to check for a change in width on
every update, rather than just when the cursor is originally enabled.

This also fixes an issue with 845g/865g which cannot change the base
address of the cursor whilst it is active.

Signed-off-by: Chris Wilson 
[Antti:rebased, adjusted macro names and moved some lines, no functional
changes]
Reviewed-by: Antti Koskipaa 
Tested-by: Antti Koskipaa 
---
 drivers/gpu/drm/i915/i915_debugfs.c  |   2 +-
 drivers/gpu/drm/i915/intel_display.c | 106 +--
 drivers/gpu/drm/i915/intel_drv.h |   3 +-
 3 files changed, 53 insertions(+), 58 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 2e5f76a..04d0b7c 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2353,7 +2353,7 @@ static int i915_display_info(struct seq_file *m, void 
*unused)
 
active = cursor_position(dev, crtc->pipe, &x, &y);
seq_printf(m, "\tcursor visible? %s, position (%d, %d), 
addr 0x%08x, active? %s\n",
-  yesno(crtc->cursor_visible),
+  yesno(crtc->cursor_base),
   x, y, crtc->cursor_addr,
   yesno(active));
}
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 731cd01..955f92d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7872,29 +7872,33 @@ static void i845_update_cursor(struct drm_crtc *crtc, 
u32 base)
struct drm_device *dev = crtc->dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-   bool visible = base != 0;
-   u32 cntl;
+   uint32_t cntl;
 
-   if (intel_crtc->cursor_visible == visible)
-   return;
-
-   cntl = I915_READ(_CURACNTR);
-   if (visible) {
+   if (base != intel_crtc->cursor_base) {
/* On these chipsets we can only modify the base whilst
 * the cursor is disabled.
 */
+   if (intel_crtc->cursor_cntl) {
+   I915_WRITE(_CURACNTR, 0);
+   POSTING_READ(_CURACNTR);
+   intel_crtc->cursor_cntl = 0;
+   }
+
I915_WRITE(_CURABASE, base);
+   POSTING_READ(_CURABASE);
+   }
 
-   cntl &= ~(CURSOR_FORMAT_MASK);
-   /* XXX width must be 64, stride 256 => 0x00 << 28 */
-   cntl |= CURSOR_ENABLE |
+   /* XXX width must be 64, stride 256 => 0x00 << 28 */
+   cntl = 0;
+   if (base)
+   cntl = (CURSOR_ENABLE |
CURSOR_GAMMA_ENABLE |
-   CURSOR_FORMAT_ARGB;
-   } else
-   cntl &= ~(CURSOR_ENABLE | CURSOR_GAMMA_ENABLE);
-   I915_WRITE(_CURACNTR, cntl);
-
-   intel_crtc->cursor_visible = visible;
+   CURSOR_FORMAT_ARGB);
+   if (intel_crtc->cursor_cntl != cntl) {
+   I915_WRITE(_CURACNTR, cntl);
+   POSTING_READ(_CURACNTR);
+   intel_crtc->cursor_cntl = cntl;
+   }
 }
 
 static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base)
@@ -7903,16 +7907,12 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, 
u32 base)
struct drm_i915_private *dev_priv = dev->dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
int pipe = intel_crtc->pipe;
-   bool visible = base != 0;
-
-   if (intel_crtc->cursor_visible != visible) {
-   int16_t width = intel_crtc->cursor_width;
-   uint32_t cntl = I915_READ(CURCNTR(pipe));
-   if (base) {
-   cntl &= ~(CURSOR_MODE | MCURSOR_PIPE_SELECT);
-   cntl |= MCURSOR_GAMMA_ENABLE;
+   uint32_t cntl;
 
-   switch (width) {
+   cntl = 0;
+   if (base) {
+   cntl = MCURSOR_GAMMA_ENABLE;
+   switch (intel_crtc->cursor_width) {
case 64:
cntl |= CURSOR_MODE_64_ARGB_AX;
break;
@@ -7925,18 +7925,16 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, 
u32 base)
default:
WARN_ON(1);
return;
-   }
-   cntl |= pipe << 28; /* Connect to correct pipe */
-   } else {
-   cntl &= ~(CURSOR_MODE | MCURSOR_GAMMA_ENABLE);
-   cntl |= CURSOR_MODE_DISABLE;
}
+   cntl |= pipe << 28; /*

Re: [Intel-gfx] [PATCH 4/4] drm/i915: make sure PC8 is enabled on suspend and disabled on resume

On Thu, 2014-05-29 at 14:11 -0700, Jesse Barnes wrote:
> From: Kristen Carlson Accardi 
> 
> This matches the runtime suspend paths and allows the system to enter
> the lowest power mode at freeze time.
> 
> Signed-off-by: Kristen Carlson Accardi 
> Signed-off-by: Jesse Barnes 
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index b6211d7..24dc856 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -558,6 +558,9 @@ static int i915_drm_freeze(struct drm_device *dev)
>  
>   intel_display_set_init_power(dev_priv, false);
>  
> + if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> + hsw_enable_pc8(dev_priv);
> +
>   return 0;
>  }
>  
> @@ -618,6 +621,9 @@ static int __i915_drm_thaw(struct drm_device *dev, bool 
> restore_gtt_mappings)
>  {
>   struct drm_i915_private *dev_priv = dev->dev_private;
>  
> + if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> + hsw_disable_pc8(dev_priv);

I would put this before we access any of the HW regs in thaw_early() and
correspondingly the above call to hsw_enable_pc8() to suspend_late()
before we call pci_disable_device().

With that change this is:
Reviewed-by: Imre Deak 

> +
>   if (drm_core_check_feature(dev, DRIVER_MODESET) &&
>   restore_gtt_mappings) {
>   mutex_lock(&dev->struct_mutex);



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

Re: [Intel-gfx] [PATCH 3/4] drm/i915: send proper opregion notifications on suspend/resume

On Thu, 2014-05-29 at 14:11 -0700, Jesse Barnes wrote:
> From: Kristen Carlson Accardi 
> 
> This indicates to the firmware that it can power down various other
> components or bring them back up, depending on the target system state.
> 
> Signed-off-by: Kristen Carlson Accardi 
> Signed-off-by: Jesse Barnes 
> ---
>  drivers/acpi/sleep.c|  1 +
>  drivers/gpu/drm/i915/i915_drv.c | 10 ++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
> index c40fb2e..807f333 100644
> --- a/drivers/acpi/sleep.c
> +++ b/drivers/acpi/sleep.c
> @@ -89,6 +89,7 @@ u32 acpi_target_system_state(void)
>  {
>   return acpi_target_sleep_state;
>  }
> +EXPORT_SYMBOL(acpi_target_system_state);
>  
>  static bool pwr_btn_event_pending;
>  
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 433bdfa..b6211d7 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -28,6 +28,7 @@
>   */
>  
>  #include 
> +#include 
>  #include 
>  #include 
>  #include "i915_drv.h"
> @@ -491,6 +492,7 @@ static int i915_drm_freeze(struct drm_device *dev)
>  {
>   struct drm_i915_private *dev_priv = dev->dev_private;
>   struct drm_crtc *crtc;
> + pci_power_t opregion_target_state;
>  
>   intel_runtime_pm_get(dev_priv);
>  
> @@ -540,6 +542,12 @@ static int i915_drm_freeze(struct drm_device *dev)
>  
>   i915_save_state(dev);
>  
> + if (acpi_target_system_state() >= ACPI_STATE_S3)
> + opregion_target_state = PCI_D3cold;
> + else
> + opregion_target_state = PCI_D1;
> + intel_opregion_notify_adapter(dev, opregion_target_state);
> +
>   intel_opregion_fini(dev);
>  
>   console_lock();
> @@ -671,6 +679,8 @@ static int __i915_drm_thaw(struct drm_device *dev, bool 
> restore_gtt_mappings)
>   dev_priv->modeset_restore = MODESET_DONE;
>   mutex_unlock(&dev_priv->modeset_restore_lock);
>  
> + intel_opregion_notify_adapter(dev, PCI_D0);
> +

This could be moved after intel_opregion_init() just for clarity, but
I'm also fine keeping it here.

This patch depends on Rafael's change in his PM tree to export
acpi_target_system_state(), other than that this is:
Reviewed-by: Imre Deak 
 
>   intel_runtime_pm_put(dev_priv);
>   return 0;
>  }



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

[Intel-gfx] [PATCH 2/3] drm/i915: Change Mipi register definitions

2014-05-30 Thread Shashank Sharma

Re-define MIPI register definitions in such a way that most of
the existing DSI code can be re-used for future platforms. Register
definitions are re-written using MMIO offset variable, so that without
changing the existing sequence, same code can be generically applied.

V4: Addressing review comments by Damien and Ville, splitting into two patches
This patch removes all the un-necessary formatting changes from previous patch.

Signed-off-by: Shashank Sharma 
---
 drivers/gpu/drm/i915/i915_reg.h | 342 +++-
 1 file changed, 196 insertions(+), 146 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c12a858..5513124 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5659,7 +5659,8 @@ enum punit_power_well {
 
 #define _MIPIA_PORT_CTRL   (VLV_DISPLAY_BASE + 0x61190)
 #define _MIPIB_PORT_CTRL   (VLV_DISPLAY_BASE + 0x61700)
-#define MIPI_PORT_CTRL(pipe)   _PIPE(pipe, _MIPIA_PORT_CTRL, 
_MIPIB_PORT_CTRL)
+#define MIPI_PORT_CTRL(tc) _PIPE(tc, _MIPIA_PORT_CTRL, \
+   _MIPIB_PORT_CTRL)
 #define  DPI_ENABLE(1 << 31) /* A + B */
 #define  MIPIA_MIPI4DPHY_DELAY_COUNT_SHIFT 27
 #define  MIPIA_MIPI4DPHY_DELAY_COUNT_MASK  (0xf << 27)
@@ -5701,18 +5702,20 @@ enum punit_power_well {
 
 #define _MIPIA_TEARING_CTRL(VLV_DISPLAY_BASE + 0x61194)
 #define _MIPIB_TEARING_CTRL(VLV_DISPLAY_BASE + 0x61704)
-#define MIPI_TEARING_CTRL(pipe)_PIPE(pipe, 
_MIPIA_TEARING_CTRL, _MIPIB_TEARING_CTRL)
+#define MIPI_TEARING_CTRL(tc)  _PIPE(tc, _MIPIA_TEARING_CTRL, \
+   _MIPIB_TEARING_CTRL)
 #define  TEARING_EFFECT_DELAY_SHIFT0
 #define  TEARING_EFFECT_DELAY_MASK (0x << 0)
 
 /* XXX: all bits reserved */
-#define _MIPIA_AUTOPWG (VLV_DISPLAY_BASE + 0x611a0)
+#define _MIPIA_AUTOPWG (dev_priv->mipi_mmio_base + 0x611a0)
 
 /* MIPI DSI Controller and D-PHY registers */
 
-#define _MIPIA_DEVICE_READY(VLV_DISPLAY_BASE + 0xb000)
-#define _MIPIB_DEVICE_READY(VLV_DISPLAY_BASE + 0xb800)
-#define MIPI_DEVICE_READY(pipe)_PIPE(pipe, 
_MIPIA_DEVICE_READY, _MIPIB_DEVICE_READY)
+#define _MIPIA_DEVICE_READY(dev_priv->mipi_mmio_base + 0xb000)
+#define _MIPIB_DEVICE_READY(dev_priv->mipi_mmio_base + 0xb800)
+#define MIPI_DEVICE_READY(tc)  _PIPE(tc, _MIPIA_DEVICE_READY, \
+   _MIPIB_DEVICE_READY)
 #define  BUS_POSSESSION(1 << 3) /* set 
to give bus to receiver */
 #define  ULPS_STATE_MASK   (3 << 1)
 #define  ULPS_STATE_ENTER  (2 << 1)
@@ -5720,12 +5723,14 @@ enum punit_power_well {
 #define  ULPS_STATE_NORMAL_OPERATION   (0 << 1)
 #define  DEVICE_READY  (1 << 0)
 
-#define _MIPIA_INTR_STAT   (VLV_DISPLAY_BASE + 0xb004)
-#define _MIPIB_INTR_STAT   (VLV_DISPLAY_BASE + 0xb804)
-#define MIPI_INTR_STAT(pipe)   _PIPE(pipe, _MIPIA_INTR_STAT, 
_MIPIB_INTR_STAT)
-#define _MIPIA_INTR_EN (VLV_DISPLAY_BASE + 0xb008)
-#define _MIPIB_INTR_EN (VLV_DISPLAY_BASE + 0xb808)
-#define MIPI_INTR_EN(pipe) _PIPE(pipe, _MIPIA_INTR_EN, 
_MIPIB_INTR_EN)
+#define _MIPIA_INTR_STAT   (dev_priv->mipi_mmio_base + 0xb004)
+#define _MIPIB_INTR_STAT   (dev_priv->mipi_mmio_base + 0xb804)
+#define MIPI_INTR_STAT(tc) _PIPE(tc, _MIPIA_INTR_STAT, \
+   _MIPIB_INTR_STAT)
+#define _MIPIA_INTR_EN (dev_priv->mipi_mmio_base + 0xb008)
+#define _MIPIB_INTR_EN (dev_priv->mipi_mmio_base + 0xb808)
+#define MIPI_INTR_EN(tc)   _PIPE(tc, _MIPIA_INTR_EN, \
+   _MIPIB_INTR_EN)
 #define  TEARING_EFFECT(1 << 31)
 #define  SPL_PKT_SENT_INTERRUPT(1 << 30)
 #define  GEN_READ_DATA_AVAIL   (1 << 29)
@@ -5759,9 +5764,10 @@ enum punit_power_well {
 #define  RXSOT_SYNC_ERROR  (1 << 1)
 #define  RXSOT_ERROR   (1 << 0)
 
-#define _MIPIA_DSI_FUNC_PRG(VLV_DISPLAY_BASE + 0xb00c)
-#define _MIPIB_DSI_FUNC_PRG(VLV_DISPLAY_BASE + 0xb80c)
-#define MIPI_DSI_FUNC_PRG(pipe)_PIPE(pipe, 
_MIPIA_DSI_FUNC_PRG, _MIPIB_DSI_FUNC_PRG)
+#define _MIPIA_DSI_FUNC_PRG(dev_priv->mipi_mmio_base + 0xb00c)
+#define _MIPIB_DSI_FUNC

[Intel-gfx] [PATCH 3/3] drm/i915: Use transcoder as index to MIPI regs

2014-05-30 Thread Shashank Sharma

Conceptually, the MIPI registers are addressed by the MIPI transcoder
index, not the pipe. It doesn't matter right now, because there's a
1:1 relationship between pipes and MIPI transcoders, but that change
allows us to break that link in the future

V1: Created new patch to address Damien's review comment.
Replacing _PIPE calls to _TRANSCODER calls
Signed-off-by: Shashank Sharma 
---
 drivers/gpu/drm/i915/i915_reg.h | 132 
 1 file changed, 66 insertions(+), 66 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 5513124..702ac6b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5659,8 +5659,8 @@ enum punit_power_well {
 
 #define _MIPIA_PORT_CTRL   (VLV_DISPLAY_BASE + 0x61190)
 #define _MIPIB_PORT_CTRL   (VLV_DISPLAY_BASE + 0x61700)
-#define MIPI_PORT_CTRL(tc) _PIPE(tc, _MIPIA_PORT_CTRL, \
-   _MIPIB_PORT_CTRL)
+#define MIPI_PORT_CTRL(tc) _TRANSCODER(tc, \
+   _MIPIA_PORT_CTRL, _MIPIB_PORT_CTRL)
 #define  DPI_ENABLE(1 << 31) /* A + B */
 #define  MIPIA_MIPI4DPHY_DELAY_COUNT_SHIFT 27
 #define  MIPIA_MIPI4DPHY_DELAY_COUNT_MASK  (0xf << 27)
@@ -5702,8 +5702,8 @@ enum punit_power_well {
 
 #define _MIPIA_TEARING_CTRL(VLV_DISPLAY_BASE + 0x61194)
 #define _MIPIB_TEARING_CTRL(VLV_DISPLAY_BASE + 0x61704)
-#define MIPI_TEARING_CTRL(tc)  _PIPE(tc, _MIPIA_TEARING_CTRL, \
-   _MIPIB_TEARING_CTRL)
+#define MIPI_TEARING_CTRL(tc)  _TRANSCODER(tc, \
+   _MIPIA_TEARING_CTRL, _MIPIB_TEARING_CTRL)
 #define  TEARING_EFFECT_DELAY_SHIFT0
 #define  TEARING_EFFECT_DELAY_MASK (0x << 0)
 
@@ -5714,7 +5714,7 @@ enum punit_power_well {
 
 #define _MIPIA_DEVICE_READY(dev_priv->mipi_mmio_base + 0xb000)
 #define _MIPIB_DEVICE_READY(dev_priv->mipi_mmio_base + 0xb800)
-#define MIPI_DEVICE_READY(tc)  _PIPE(tc, _MIPIA_DEVICE_READY, \
+#define MIPI_DEVICE_READY(tc)  _TRANSCODER(tc, _MIPIA_DEVICE_READY, \
_MIPIB_DEVICE_READY)
 #define  BUS_POSSESSION(1 << 3) /* set 
to give bus to receiver */
 #define  ULPS_STATE_MASK   (3 << 1)
@@ -5725,11 +5725,11 @@ enum punit_power_well {
 
 #define _MIPIA_INTR_STAT   (dev_priv->mipi_mmio_base + 0xb004)
 #define _MIPIB_INTR_STAT   (dev_priv->mipi_mmio_base + 0xb804)
-#define MIPI_INTR_STAT(tc) _PIPE(tc, _MIPIA_INTR_STAT, \
+#define MIPI_INTR_STAT(tc) _TRANSCODER(tc, _MIPIA_INTR_STAT, \
_MIPIB_INTR_STAT)
 #define _MIPIA_INTR_EN (dev_priv->mipi_mmio_base + 0xb008)
 #define _MIPIB_INTR_EN (dev_priv->mipi_mmio_base + 0xb808)
-#define MIPI_INTR_EN(tc)   _PIPE(tc, _MIPIA_INTR_EN, \
+#define MIPI_INTR_EN(tc)   _TRANSCODER(tc, _MIPIA_INTR_EN, \
_MIPIB_INTR_EN)
 #define  TEARING_EFFECT(1 << 31)
 #define  SPL_PKT_SENT_INTERRUPT(1 << 30)
@@ -5766,7 +5766,7 @@ enum punit_power_well {
 
 #define _MIPIA_DSI_FUNC_PRG(dev_priv->mipi_mmio_base + 0xb00c)
 #define _MIPIB_DSI_FUNC_PRG(dev_priv->mipi_mmio_base + 0xb80c)
-#define MIPI_DSI_FUNC_PRG(tc)  _PIPE(tc, _MIPIA_DSI_FUNC_PRG, \
+#define MIPI_DSI_FUNC_PRG(tc)  _TRANSCODER(tc, _MIPIA_DSI_FUNC_PRG, \
_MIPIB_DSI_FUNC_PRG)
 #define  CMD_MODE_DATA_WIDTH_MASK  (7 << 13)
 #define  CMD_MODE_NOT_SUPPORTED(0 << 13)
@@ -5790,32 +5790,32 @@ enum punit_power_well {
 
 #define _MIPIA_HS_TX_TIMEOUT   (dev_priv->mipi_mmio_base + 0xb010)
 #define _MIPIB_HS_TX_TIMEOUT   (dev_priv->mipi_mmio_base + 0xb810)
-#define MIPI_HS_TX_TIMEOUT(tc) _PIPE(tc, _MIPIA_HS_TX_TIMEOUT, \
+#define MIPI_HS_TX_TIMEOUT(tc) _TRANSCODER(tc, _MIPIA_HS_TX_TIMEOUT, \
_MIPIB_HS_TX_TIMEOUT)
 #define  HIGH_SPEED_TX_TIMEOUT_COUNTER_MASK0xff
 
 #define _MIPIA_LP_RX_TIMEOUT   (dev_priv->mipi_mmio_base + 0xb014)
 #define _MIPIB_LP_RX_TIMEOUT   (dev_priv->mipi_mmio_base + 0xb814)
-#define MIPI_LP_RX_TIMEOUT(tc) _PIPE(tc, _MIPIA_LP_RX_TIMEOUT, \
+#define MIPI_LP_RX_TIMEOUT(tc) _TRANSCODER(tc, _MIPIA_LP_RX_TIMEOUT, \
_MIPIB_LP_RX_TIMEOUT)
 #define  LOW_POWER_RX_TIMEOUT_COUNTER_MASK 0xff

Re: [Intel-gfx] [PATCH 2/3] drm/i915: Change Mipi register definitions

2014-05-30 Thread Damien Lespiau

On Fri, May 30, 2014 at 08:12:34PM +0530, Shashank Sharma wrote:
> Re-define MIPI register definitions in such a way that most of
> the existing DSI code can be re-used for future platforms. Register
> definitions are re-written using MMIO offset variable, so that without
> changing the existing sequence, same code can be generically applied.
> 
> V4: Addressing review comments by Damien and Ville, splitting into two patches
> This patch removes all the un-necessary formatting changes from previous 
> patch.

> Signed-off-by: Shashank Sharma 

Almost :) you still have th s/pipe/tc/ change in there.

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

Re: [Intel-gfx] [PATCH 2/4] drm/i915: leave rc6 enabled at suspend time

On Fri, 30 May 2014 15:54:37 +0300
Imre Deak  wrote:

> On Thu, 2014-05-29 at 14:11 -0700, Jesse Barnes wrote:
> > From: Kristen Carlson Accardi 
> > 
> > This allows the system to enter the lowest power mode during system freeze.
> > 
> > Signed-off-by: Jesse Barnes 
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c  |  3 ---
> >  drivers/gpu/drm/i915/intel_drv.h |  1 +
> >  drivers/gpu/drm/i915/intel_pm.c  | 16 +++-
> >  3 files changed, 12 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c 
> > b/drivers/gpu/drm/i915/i915_drv.c
> > index 66c6ffb..433bdfa 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -521,8 +521,6 @@ static int i915_drm_freeze(struct drm_device *dev)
> > drm_irq_uninstall(dev);
> > dev_priv->enable_hotplug_processing = false;
> >  
> > -   intel_disable_gt_powersave(dev);
> > -
> 
> I wonder what was the reason for this call. One possibility is that
> i915_save_state() depends on it to save the correct registers, but it
> would be good to clarify this.
> 
> It also cancels some deferred works which we do need here. But we could
> also add that to intel_enable_gt_powersave_sync() in this patch.

Yeah I was worried about that too, but then we do the reset on resume
anyway, and I didn't see anything in my logs in testing...

But I can split that out if there's a reason to.  Seems like we do a
bit too much teardown at suspend these days (like tearing down opregion
state), I'd like to trim it back if possible and share between runtime
and system suspend/freeze.

I'll look into the forcewake bits.

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] [PATCH 2/4] drm/i915: leave rc6 enabled at suspend time

On Fri, May 30, 2014 at 08:32:20AM -0700, Jesse Barnes wrote:
> But I can split that out if there's a reason to.  Seems like we do a
> bit too much teardown at suspend these days (like tearing down opregion
> state), I'd like to trim it back if possible and share between runtime
> and system suspend/freeze.

I guess the answer is to move all the paranoid bits from suspend into
hibernate-resume.
-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

[Intel-gfx] [PATCH 1/3] drm/i915: Check for a stalled page flip after each vblank

Long ago, back in the racy haydays of 915gm interrupt handling, page
flips would occasionally go astray and leave the hardware stuck, and the
display not updating. This annoyed people who relied on their systems
being able to display continuously updating information 24/7, and so
some code to detect when the driver missed the page flip completion
signal was added. Until recently, it was presumed that the interrupt
handling was now flawless, but once again Simon Farnsworth has found a
system whose display will stall. Reinstate the pageflip stall detection,
which works by checking to see if the hardware has been updated to the
new framebuffer address following each vblank. If the hardware is
scanning out from the new framebuffer, but we still think the flip is
pending, then we kick our driver into submision.

This is a continuation of the effort started with
commit 4e5359cd053bfb7d8dabe4a63624a5726848ffbc
Author: Simon Farnsworth 
Date:   Wed Sep 1 17:47:52 2010 +0100

drm/i915: Avoid pageflipping freeze when we miss the flip prepare interrupt

This now includes a belt-and-braces approach to make sure the driver
(or the hardware) doesn't miss an interrupt and cause us to stop
updating the display should the unthinkable happen and the pageflip fail - i.e.
that the user is able to continue submitting flips.

Reported-by: Simon Farnsworth 
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=75502
Signed-off-by: Chris Wilson 
---
 drivers/gpu/drm/i915/i915_debugfs.c  |  33 ---
 drivers/gpu/drm/i915/i915_irq.c  |  85 ---
 drivers/gpu/drm/i915/intel_display.c | 109 ++-
 drivers/gpu/drm/i915/intel_drv.h |   2 +
 4 files changed, 144 insertions(+), 85 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 0b063c03d188..a05a33ab4b33 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -581,6 +581,7 @@ static int i915_gem_pageflip_info(struct seq_file *m, void 
*data)
 {
struct drm_info_node *node = m->private;
struct drm_device *dev = node->minor->dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
unsigned long flags;
struct intel_crtc *crtc;
 
@@ -602,23 +603,37 @@ static int i915_gem_pageflip_info(struct seq_file *m, 
void *data)
seq_printf(m, "Flip pending (waiting for vsync) 
on pipe %c (plane %c)\n",
   pipe, plane);
}
+   seq_printf(m, "Flip queued on frame %d, now %d\n",
+  work->sbc, 
atomic_read(&dev->vblank[crtc->pipe].count));
if (work->enable_stall_check)
seq_puts(m, "Stall check enabled, ");
else
seq_puts(m, "Stall check waiting for page flip 
ioctl, ");
seq_printf(m, "%d prepares\n", 
atomic_read(&work->pending));
 
-   if (work->old_fb_obj) {
-   struct drm_i915_gem_object *obj = 
work->old_fb_obj;
-   if (obj)
-   seq_printf(m, "Old framebuffer 
gtt_offset 0x%08lx\n",
-  
i915_gem_obj_ggtt_offset(obj));
+   {
+   u32 addr;
+
+   if (INTEL_INFO(dev)->gen >= 4)
+   addr = DSPSURF(crtc->plane);
+   else
+   addr = DSPADDR(crtc->plane);
+
+   seq_printf(m, "Current scanout address 
0x%08x\n", 
+  I915_READ(addr));
}
+
if (work->pending_flip_obj) {
-   struct drm_i915_gem_object *obj = 
work->pending_flip_obj;
-   if (obj)
-   seq_printf(m, "New framebuffer 
gtt_offset 0x%08lx\n",
-  
i915_gem_obj_ggtt_offset(obj));
+   bool complete;
+
+   seq_printf(m, "New framebuffer address 
0x%08lx\n", (long)work->gtt_offset);
+
+   if (INTEL_INFO(dev)->gen >= 4)
+   complete = 
I915_HI_DISPBASE(I915_READ(DSPSURF(crtc->plane))) == work->gtt_offset;
+   else
+   complete = 
I915_READ(DSPADDR(crtc->plane)) == work->gtt_offset;
+
+   seq_printf(m, "MMIO update completed? %d\n", 
complete);
}
}
spin_unlock_irqrestore(&dev->event_lock, flags);
diff --git a/drivers/gpu/drm/i915/i915_i

[Intel-gfx] [PATCH 3/3] drm/i915: Boost GPU frequency if we detect outstanding pageflips

If we hit a vblank and see that have a pageflip queue but not yet
processed, ensure that the GPU is running at maximum in order to clear
the backlog. Pageflips are only queued for the following vblank, if we
miss it, there will be a visible stutter. Boosting the GPU frequency
doesn't prevent us from missing the target vblank, but it should help
the subsequent frames hitting theirs.

v2: Reorder vblank vs flip-complete so that we only check for a missed
flip after processing the completion events, and avoid spurious boosts.

Signed-off-by: Chris Wilson 
---
 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_display.c |  6 ++
 drivers/gpu/drm/i915/intel_drv.h |  1 +
 drivers/gpu/drm/i915/intel_pm.c  | 15 +++
 4 files changed, 23 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f9450045a532..af050e439168 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -910,6 +910,7 @@ struct intel_gen6_power_mgmt {
 
bool enabled;
struct delayed_work delayed_resume_work;
+   struct work_struct boost_work;
 
/*
 * Protects RPS/RC6 register access and PCU communication.
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 54b69838e2de..3ad1529e74df 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9258,6 +9258,7 @@ void intel_check_page_flip(struct drm_device *dev, int 
pipe)
struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
unsigned long flags;
+   bool outstanding;
 
if (crtc == NULL)
return;
@@ -9270,7 +9271,12 @@ void intel_check_page_flip(struct drm_device *dev, int 
pipe)
page_flip_completed(dev_priv, intel_crtc, 
intel_crtc->unpin_work);
intel_crtc->unpin_work = NULL;
}
+   outstanding = (intel_crtc->unpin_work != NULL &&
+  crtc_sbc(intel_crtc) - intel_crtc->unpin_work->sbc > 1);
spin_unlock_irqrestore(&dev->event_lock, flags);
+
+   if (outstanding)
+   intel_queue_rps_boost(dev);
 }
 
 static int intel_crtc_page_flip(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index b17c295fe967..a77f104db366 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -964,6 +964,7 @@ void ironlake_teardown_rc6(struct drm_device *dev);
 void gen6_update_ring_freq(struct drm_device *dev);
 void gen6_rps_idle(struct drm_i915_private *dev_priv);
 void gen6_rps_boost(struct drm_i915_private *dev_priv);
+void intel_queue_rps_boost(struct drm_device *dev);
 void intel_aux_display_runtime_get(struct drm_i915_private *dev_priv);
 void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv);
 void intel_runtime_pm_get(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 00ab3c7a282d..dce2c268851f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6707,6 +6707,19 @@ int vlv_freq_opcode(struct drm_i915_private *dev_priv, 
int val)
return DIV_ROUND_CLOSEST(4 * mul * val, dev_priv->mem_freq) + 0xbd - 6;
 }
 
+static void __intel_rps_boost_work(struct work_struct *work)
+{
+   gen6_rps_boost(container_of(work, struct drm_i915_private, 
rps.boost_work));
+}
+
+void intel_queue_rps_boost(struct drm_device *dev)
+{
+   struct drm_i915_private *dev_priv = to_i915(dev);
+
+   if (INTEL_INFO(dev)->gen >= 6)
+   queue_work(dev_priv->wq, &dev_priv->rps.boost_work);
+}
+
 void intel_pm_setup(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -6715,6 +6728,8 @@ void intel_pm_setup(struct drm_device *dev)
 
INIT_DELAYED_WORK(&dev_priv->rps.delayed_resume_work,
  intel_gen6_powersave_work);
+   INIT_WORK(&dev_priv->rps.boost_work,
+ __intel_rps_boost_work);
 
dev_priv->pm.suspended = false;
dev_priv->pm.irqs_disabled = false;
-- 
2.0.0.rc4

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

[Intel-gfx] [PATCH 2/3] drm/i915: Decouple the stuck pageflip on modeset

If we successfully confuse the hardware, and cause it to drop a queued
pageflip, we wait for 60s and issue a warning before continuing on with
the modeset. However, this leaves the pending pageflip still stuck
indefinitely. Pretend to userspace that it does complete, and let us
start afresh following the modeset.

Signed-off-by: Chris Wilson 
Cc: Daniel Vetter 
Cc: Ville Syrjälä 
---
 drivers/gpu/drm/i915/intel_display.c | 18 +++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 764b277e5937..54b69838e2de 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3329,9 +3329,21 @@ void intel_crtc_wait_for_pending_flips(struct drm_crtc 
*crtc)
 
WARN_ON(waitqueue_active(&dev_priv->pending_flip_queue));
 
-   WARN_ON(wait_event_timeout(dev_priv->pending_flip_queue,
-  !intel_crtc_has_pending_flip(crtc),
-  60*HZ) == 0);
+   if (WARN_ON(wait_event_timeout(dev_priv->pending_flip_queue,
+  !intel_crtc_has_pending_flip(crtc),
+  60*HZ) == 0)) {
+   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+   unsigned long flags;
+
+   spin_lock_irqsave(&dev->event_lock, flags);
+   if (intel_crtc->unpin_work) {
+   WARN_ONCE(1, "Removing stuck page flip\n");
+   drm_vblank_put(dev, intel_crtc->pipe);
+   page_flip_completed(dev_priv, intel_crtc, 
intel_crtc->unpin_work);
+   intel_crtc->unpin_work = NULL;
+   }
+   spin_unlock_irqrestore(&dev->event_lock, flags);
+   }
 
mutex_lock(&dev->struct_mutex);
intel_finish_fb(crtc->primary->fb);
-- 
2.0.0.rc4

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

Re: [Intel-gfx] [PATCH] drm/i915: Only mark the ctx as initialised after a SET_CONTEXT operation

2014-05-30 Thread Ben Widawsky

On Fri, May 30, 2014 at 02:16:30PM +0100, Chris Wilson wrote:
> Fallout from
> 
> commit 46470fc932ac8a0e8317a220b3f4ea4ed903338e
> Author: Mika Kuoppala 
> Date:   Wed May 21 19:01:06 2014 +0300
> 
> drm/i915: Add null state batch to active list
> 
> undid the earlier fix of only marking the ctx as initialised after it is
> saved by the hardware during a SET_CONTEXT operation.
> 
> Signed-off-by: Chris Wilson 
> Cc: Ville Syrjälä 
> Cc: Damien Lespiau 
> Cc: Mika Kuoppala 
> Cc: Ben Widawsky 
> ---
>  drivers/gpu/drm/i915/i915_gem_context.c | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
> b/drivers/gpu/drm/i915/i915_gem_context.c
> index 5a71ef1975b3..34a0b49e6add 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -641,6 +641,7 @@ static int do_switch(struct intel_engine_cs *ring,
>   struct intel_context *from = ring->last_context;
>   struct i915_hw_ppgtt *ppgtt = ctx_to_ppgtt(to);
>   u32 hw_flags = 0;
> + bool uninitialized = false;
>   int ret, i;
>  
>   if (from != NULL && ring == &dev_priv->ring[RCS]) {
> @@ -739,18 +740,19 @@ static int do_switch(struct intel_engine_cs *ring,
>   i915_gem_context_unreference(from);
>   }
>  
> + uninitialized = !to->is_initialized && from == NULL;
> + to->is_initialized = true;

Aren't you missing some error paths if you do this?  I think we need to
set the state after we've successfully emitted the MI_SET_CONTEXT
command (though really, until we move the ringbuffer tail, it's all
lies, on that note, I'm scared to look at if reset dtrt).

> +
>  done:
>   i915_gem_context_reference(to);
>   ring->last_context = to;
>  
> - if (ring->id == RCS && !to->is_initialized && from == NULL) {
> + if (uninitialized) {
>   ret = i915_gem_render_state_init(ring);
>   if (ret)
>   DRM_ERROR("init render state: %d\n", ret);
>   }
>  
> - to->is_initialized = true;
> -
>   return 0;
>  
>  unpin_out:

I just realized the from == NULL check is something which seems fragile
to me given how much churn we might see with init/reset paths. It'd
probably be good to eventually switch to some global state in dev_priv
which we can track across init/reset/fini

Otherwise it looks correct to me.

-- 
Ben Widawsky, 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: Only mark the ctx as initialised after a SET_CONTEXT operation

On Fri, May 30, 2014 at 10:44:53AM -0700, Ben Widawsky wrote:
> On Fri, May 30, 2014 at 02:16:30PM +0100, Chris Wilson wrote:
> > Fallout from
> > 
> > commit 46470fc932ac8a0e8317a220b3f4ea4ed903338e
> > Author: Mika Kuoppala 
> > Date:   Wed May 21 19:01:06 2014 +0300
> > 
> > drm/i915: Add null state batch to active list
> > 
> > undid the earlier fix of only marking the ctx as initialised after it is
> > saved by the hardware during a SET_CONTEXT operation.
> > 
> > Signed-off-by: Chris Wilson 
> > Cc: Ville Syrjälä 
> > Cc: Damien Lespiau 
> > Cc: Mika Kuoppala 
> > Cc: Ben Widawsky 
> > ---
> >  drivers/gpu/drm/i915/i915_gem_context.c | 8 +---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
> > b/drivers/gpu/drm/i915/i915_gem_context.c
> > index 5a71ef1975b3..34a0b49e6add 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_context.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> > @@ -641,6 +641,7 @@ static int do_switch(struct intel_engine_cs *ring,
> > struct intel_context *from = ring->last_context;
> > struct i915_hw_ppgtt *ppgtt = ctx_to_ppgtt(to);
> > u32 hw_flags = 0;
> > +   bool uninitialized = false;
> > int ret, i;
> >  
> > if (from != NULL && ring == &dev_priv->ring[RCS]) {
> > @@ -739,18 +740,19 @@ static int do_switch(struct intel_engine_cs *ring,
> > i915_gem_context_unreference(from);
> > }
> >  
> > +   uninitialized = !to->is_initialized && from == NULL;
> > +   to->is_initialized = true;
> 
> Aren't you missing some error paths if you do this?  I think we need to
> set the state after we've successfully emitted the MI_SET_CONTEXT
> command (though really, until we move the ringbuffer tail, it's all
> lies, on that note, I'm scared to look at if reset dtrt).

I also think that until we successfully emit the MI_SET_CONTEXT and
allow the execbuffer to proceed, we can continue to treat the context as
garbage and force-inhibit on the next attempt to run the execbuffer.
 
> > +
> >  done:
> > i915_gem_context_reference(to);
> > ring->last_context = to;
> >  
> > -   if (ring->id == RCS && !to->is_initialized && from == NULL) {
> > +   if (uninitialized) {
> > ret = i915_gem_render_state_init(ring);
> > if (ret)
> > DRM_ERROR("init render state: %d\n", ret);
> > }
> >  
> > -   to->is_initialized = true;
> > -
> > return 0;
> >  
> >  unpin_out:
> 
> I just realized the from == NULL check is something which seems fragile
> to me given how much churn we might see with init/reset paths. It'd
> probably be good to eventually switch to some global state in dev_priv
> which we can track across init/reset/fini

It's as fragile as the bug we are papering over. Presumably we do need
to apply fresh paper after a GPU reset, but it is hard to tell without
knowing the bug.
-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

[Intel-gfx] [PATCH 2/2] drm/i915: make userspace mode sets asynchronous

Now that we can queue CRTC enable/disable calls for later, we can allow
userspace mode sets to return immediately.  This may mean that userspace
will draw into a buffer that's not yet displayed (which is fine) or that
it may draw into a buffer it thinks is no longer displayed (which could
lead to some visual artifacts until the mode set completes, but is
otherwise harmless).  Page flip and cursor activity will synchronize
with any outstanding activity to avoid problems with the display being
off for those operations.

It should be possible to queue those ops as well though and further
de-couple driver updates of the hw state from userspace queueing of
commands.

Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/intel_display.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 8c52038..74310b5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10545,10 +10545,6 @@ static int intel_set_mode(struct drm_crtc *crtc,
 
ret = __intel_set_mode(crtc, mode, x, y, fb);
 
-   intel_sync_crtcs(crtc->dev->dev_private);
-   if (ret == 0)
-   intel_modeset_check_state(crtc->dev);
-
return ret;
 }
 
-- 
1.9.1

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

[Intel-gfx] [PATCH 1/2] drm/i915: make CRTC enable/disable asynchronous v2

This lets us return to userspace more quickly and should improve init
and suspend/resume times as well, allowing us to return to userspace
sooner.

This was initially motivated by slow resume time on some machines with
very long panel power sequencing times, and it should also improve boot
time when a full mode set is required.

v2: use a single enable/disable queue (Jesse/Chris)
fixup locking, test with lockdep (Jesse)
move hw state checks to sync_crtcs (Jesse)
make userspace initiated mode sets stay synchronous (Chris)

Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/i915_drv.c  |   2 +-
 drivers/gpu/drm/i915/i915_drv.h  |  12 ++-
 drivers/gpu/drm/i915/intel_display.c | 170 +++
 drivers/gpu/drm/i915/intel_drv.h |   2 +-
 4 files changed, 165 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index e2bfdda..e7fa84f 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -530,7 +530,7 @@ static int i915_drm_freeze(struct drm_device *dev)
mutex_lock(&dev->mode_config.mutex);
for_each_crtc(dev, crtc) {
mutex_lock(&crtc->mutex);
-   dev_priv->display.crtc_disable(crtc);
+   dev_priv->display._crtc_disable(crtc);
mutex_unlock(&crtc->mutex);
}
mutex_unlock(&dev->mode_config.mutex);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index bea9ab40..bbfe402 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -447,8 +447,8 @@ struct drm_i915_display_funcs {
int (*crtc_mode_set)(struct drm_crtc *crtc,
 int x, int y,
 struct drm_framebuffer *old_fb);
-   void (*crtc_enable)(struct drm_crtc *crtc);
-   void (*crtc_disable)(struct drm_crtc *crtc);
+   void (*_crtc_enable)(struct drm_crtc *crtc);
+   void (*_crtc_disable)(struct drm_crtc *crtc);
void (*off)(struct drm_crtc *crtc);
void (*write_eld)(struct drm_connector *connector,
  struct drm_crtc *crtc,
@@ -1432,6 +1432,14 @@ struct drm_i915_private {
/* Display functions */
struct drm_i915_display_funcs display;
 
+   /**
+* CRTC work queue handling.  Enable/disable calls are queued
+* into the list and processed by the CRTC work function at some
+* later time, or inline by a call to intel_sync_crtcs().
+*/
+   struct list_head crtc_work_queue;
+   struct work_struct crtc_work;
+
/* PCH chipset type */
enum intel_pch pch_type;
unsigned short pch_id;
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 731cd01..8c52038 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1973,6 +1973,135 @@ static void lpt_disable_pch_transcoder(struct 
drm_i915_private *dev_priv)
I915_WRITE(_TRANSA_CHICKEN2, val);
 }
 
+struct intel_crtc_work {
+   /**
+* Whether to enable or disable the given CRTC
+*/
+   bool enable;
+   /**
+* CRTC to operate on
+*/
+   struct intel_crtc *intel_crtc;
+   /**
+* Used to link to dev_priv->crtc_work_queue, protected
+* by mode_config mutex.
+*/
+   struct list_head head;
+};
+
+/**
+ * intel_sync_crtcs - complete any pending CRTC enable/disable calls
+ * @dev_priv: i915 driver struct
+ *
+ * Walk the CRTC work queue and perform the enable/disable calls in the
+ * order they were added.
+ *
+ * This function will return when the enable/disable calls have been completed,
+ * and so may take many milliseconds before returning.
+ */
+static void intel_sync_crtcs(struct drm_i915_private *dev_priv)
+{
+   struct drm_device *dev = dev_priv->dev;
+   struct intel_crtc_work *crtc_work, *tmp;
+
+   WARN(!mutex_is_locked(&dev->mode_config.mutex),
+"need mode_config mutex\n");
+
+   list_for_each_entry_safe(crtc_work, tmp, &dev_priv->crtc_work_queue,
+head) {
+   struct drm_crtc *crtc = &crtc_work->intel_crtc->base;
+
+   if (crtc_work->enable)
+   dev_priv->display._crtc_enable(crtc);
+   else
+   dev_priv->display._crtc_disable(crtc);
+   list_del(&crtc_work->head);
+   kfree(crtc_work);
+   }
+
+   intel_modeset_check_state(dev);
+}
+
+/**
+ * intel_crtc_work - CRTC queue processing function
+ * @work: crtc_work struct from drm_i915_private
+ *
+ * Just calls intel_sync_crtcs() to take the lock and process the list if any
+ * entries are present.
+ */
+static void intel_crtc_work(struct work_struct *work)
+{
+   struct drm_i915_private *dev_priv =
+

Re: [Intel-gfx] [PATCH 2/4] drm/i915: leave rc6 enabled at suspend time

On Fri, 30 May 2014 16:40:27 +0100
Chris Wilson  wrote:

> On Fri, May 30, 2014 at 08:32:20AM -0700, Jesse Barnes wrote:
> > But I can split that out if there's a reason to.  Seems like we do a
> > bit too much teardown at suspend these days (like tearing down opregion
> > state), I'd like to trim it back if possible and share between runtime
> > and system suspend/freeze.
> 
> I guess the answer is to move all the paranoid bits from suspend into
> hibernate-resume.

Well and probably move a bunch of stuff out of _freeze and into a
shared _suspend function for use at runtime suspend, suspend, and
hibernate.

Separate patches for all of that though so all the breakage is easily
bisected. :)

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

Re: [Intel-gfx] [PATCH 4/4] drm/i915: make sure PC8 is enabled on suspend and disabled on resume

On Fri, 30 May 2014 16:37:53 +0300
Imre Deak  wrote:

> On Thu, 2014-05-29 at 14:11 -0700, Jesse Barnes wrote:
> > From: Kristen Carlson Accardi 
> > 
> > This matches the runtime suspend paths and allows the system to enter
> > the lowest power mode at freeze time.
> > 
> > Signed-off-by: Kristen Carlson Accardi 
> > Signed-off-by: Jesse Barnes 
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c | 6 ++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c 
> > b/drivers/gpu/drm/i915/i915_drv.c
> > index b6211d7..24dc856 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -558,6 +558,9 @@ static int i915_drm_freeze(struct drm_device *dev)
> >  
> > intel_display_set_init_power(dev_priv, false);
> >  
> > +   if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> > +   hsw_enable_pc8(dev_priv);
> > +
> > return 0;
> >  }
> >  
> > @@ -618,6 +621,9 @@ static int __i915_drm_thaw(struct drm_device *dev, bool 
> > restore_gtt_mappings)
> >  {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> >  
> > +   if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> > +   hsw_disable_pc8(dev_priv);
> 
> I would put this before we access any of the HW regs in thaw_early() and
> correspondingly the above call to hsw_enable_pc8() to suspend_late()
> before we call pci_disable_device().
> 
> With that change this is:
> Reviewed-by: Imre Deak 

For the thaw side I think that makes sense.

But for the freeze side, putting it in suspend_late won't get us the
freeze behavior we want.  I think Rafael and Kristen are thinking of
re-using the freeze infrastructure for some kind of connected standby
feature, where most stuff is frozen, but the system isn't in S3 or S4,
so we need the enable_pc8 call in the _freeze path as well.

Rafael, is that correct?

I'll add a late_freeze and put it there instead, so it doesn't pollute
the S3 suspend path.

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: make sure PC8 is enabled on suspend and disabled on resume v2

From: Kristen Carlson Accardi 

This matches the runtime suspend paths and allows the system to enter
the lowest power mode at freeze time.

v2: move disable_pc8 call to thaw_early (Imre)
move enable_pc8 to freeze_late (Imre/Jesse)

Signed-off-by: Kristen Carlson Accardi 
Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/i915_drv.c | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index a573f5a..0a8925e 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -559,6 +559,9 @@ static int i915_drm_freeze(struct drm_device *dev)
 
intel_display_set_init_power(dev_priv, false);
 
+   if (IS_HASWELL(dev) || IS_BROADWELL(dev))
+   hsw_enable_pc8(dev_priv);
+
return 0;
 }
 
@@ -608,6 +611,9 @@ static int i915_drm_thaw_early(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = dev->dev_private;
 
+   if (IS_HASWELL(dev) || IS_BROADWELL(dev))
+   hsw_disable_pc8(dev_priv);
+
intel_uncore_early_sanitize(dev);
intel_uncore_sanitize(dev);
intel_power_domains_init_hw(dev_priv);
@@ -939,6 +945,21 @@ static int i915_pm_freeze(struct device *dev)
return i915_drm_freeze(drm_dev);
 }
 
+static int i915_pm_freeze_late(struct device *dev)
+{
+   struct pci_dev *pdev = to_pci_dev(dev);
+   struct drm_device *drm_dev = pci_get_drvdata(pdev);
+   struct drm_i915_private *dev_priv = drm_dev->dev_private;
+
+   if (IS_HASWELL(drm_dev) || IS_BROADWELL(drm_dev))
+   hsw_enable_pc8(dev_priv);
+
+   pci_disable_device(pdev);
+   pci_set_power_state(pdev, PCI_D3hot);
+
+   return 0;
+}
+
 static int i915_pm_thaw_early(struct device *dev)
 {
struct pci_dev *pdev = to_pci_dev(dev);
@@ -1476,6 +1497,7 @@ static const struct dev_pm_ops i915_pm_ops = {
.resume_early = i915_pm_resume_early,
.resume = i915_pm_resume,
.freeze = i915_pm_freeze,
+   .freeze_late = i915_pm_freeze_late,
.thaw_early = i915_pm_thaw_early,
.thaw = i915_pm_thaw,
.poweroff = i915_pm_poweroff,
-- 
1.9.1

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

[Intel-gfx] [PATCH] drm/i915: leave rc6 enabled at suspend time v2

From: Kristen Carlson Accardi 

This allows the system to enter the lowest power mode during system freeze.

v2: delete force wake timer at suspend (Imre)

Signed-off-by: Kristen Carlson Accardi 
Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/i915_drv.c  |  4 +---
 drivers/gpu/drm/i915/intel_drv.h |  1 +
 drivers/gpu/drm/i915/intel_pm.c  | 16 +++-
 3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 66c6ffb..c65fc68 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -521,8 +521,6 @@ static int i915_drm_freeze(struct drm_device *dev)
drm_irq_uninstall(dev);
dev_priv->enable_hotplug_processing = false;
 
-   intel_disable_gt_powersave(dev);
-
/*
 * Disable CRTCs directly since we want to preserve sw state
 * for _thaw.
@@ -542,8 +540,8 @@ static int i915_drm_freeze(struct drm_device *dev)
 
i915_save_state(dev);
 
+   del_timer_sync(&dev_priv->uncore.force_wake_timer);
intel_opregion_fini(dev);
-   intel_uncore_fini(dev);
 
console_lock();
intel_fbdev_set_suspend(dev, FBINFO_STATE_SUSPENDED);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index c597b0d..bf90e7d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -956,6 +956,7 @@ void intel_power_domains_init_hw(struct drm_i915_private 
*dev_priv);
 void intel_init_gt_powersave(struct drm_device *dev);
 void intel_cleanup_gt_powersave(struct drm_device *dev);
 void intel_enable_gt_powersave(struct drm_device *dev);
+void intel_enable_gt_powersave_sync(struct drm_device *dev);
 void intel_disable_gt_powersave(struct drm_device *dev);
 void intel_reset_gt_powersave(struct drm_device *dev);
 void ironlake_teardown_rc6(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 1840d15..8d9e036 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -4891,12 +4891,9 @@ void intel_disable_gt_powersave(struct drm_device *dev)
}
 }
 
-static void intel_gen6_powersave_work(struct work_struct *work)
+void intel_enable_gt_powersave_sync(struct drm_device *dev)
 {
-   struct drm_i915_private *dev_priv =
-   container_of(work, struct drm_i915_private,
-rps.delayed_resume_work.work);
-   struct drm_device *dev = dev_priv->dev;
+   struct drm_i915_private *dev_priv = dev->dev_private;
 
mutex_lock(&dev_priv->rps.hw_lock);
 
@@ -4917,6 +4914,15 @@ static void intel_gen6_powersave_work(struct work_struct 
*work)
intel_runtime_pm_put(dev_priv);
 }
 
+static void intel_gen6_powersave_work(struct work_struct *work)
+{
+   struct drm_i915_private *dev_priv =
+   container_of(work, struct drm_i915_private,
+rps.delayed_resume_work.work);
+
+   intel_enable_gt_powersave_sync(dev_priv->dev);
+}
+
 void intel_enable_gt_powersave(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = dev->dev_private;
-- 
1.9.1

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

[Intel-gfx] [PATCH] drm/i915: make sure PC8 is enabled on suspend and disabled on resume v3

From: Kristen Carlson Accardi 

This matches the runtime suspend paths and allows the system to enter
the lowest power mode at freeze time.

v2: move disable_pc8 call to thaw_early (Imre)
move enable_pc8 to freeze_late (Imre/Jesse)
v3: drop spurious hunk from _freeze now that we have freeze_late (Jesse)

Signed-off-by: Kristen Carlson Accardi 
Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/i915_drv.c | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index a573f5a..ff291c0 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -608,6 +608,9 @@ static int i915_drm_thaw_early(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = dev->dev_private;
 
+   if (IS_HASWELL(dev) || IS_BROADWELL(dev))
+   hsw_disable_pc8(dev_priv);
+
intel_uncore_early_sanitize(dev);
intel_uncore_sanitize(dev);
intel_power_domains_init_hw(dev_priv);
@@ -939,6 +942,21 @@ static int i915_pm_freeze(struct device *dev)
return i915_drm_freeze(drm_dev);
 }
 
+static int i915_pm_freeze_late(struct device *dev)
+{
+   struct pci_dev *pdev = to_pci_dev(dev);
+   struct drm_device *drm_dev = pci_get_drvdata(pdev);
+   struct drm_i915_private *dev_priv = drm_dev->dev_private;
+
+   if (IS_HASWELL(drm_dev) || IS_BROADWELL(drm_dev))
+   hsw_enable_pc8(dev_priv);
+
+   pci_disable_device(pdev);
+   pci_set_power_state(pdev, PCI_D3hot);
+
+   return 0;
+}
+
 static int i915_pm_thaw_early(struct device *dev)
 {
struct pci_dev *pdev = to_pci_dev(dev);
@@ -1476,6 +1494,7 @@ static const struct dev_pm_ops i915_pm_ops = {
.resume_early = i915_pm_resume_early,
.resume = i915_pm_resume,
.freeze = i915_pm_freeze,
+   .freeze_late = i915_pm_freeze_late,
.thaw_early = i915_pm_thaw_early,
.thaw = i915_pm_thaw,
.poweroff = i915_pm_poweroff,
-- 
1.9.1

___
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: make CRTC enable/disable asynchronous v2

On Fri, May 30, 2014 at 11:05:21AM -0700, Jesse Barnes wrote:
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index e2bfdda..e7fa84f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -530,7 +530,7 @@ static int i915_drm_freeze(struct drm_device *dev)
>   mutex_lock(&dev->mode_config.mutex);
>   for_each_crtc(dev, crtc) {
>   mutex_lock(&crtc->mutex);
> - dev_priv->display.crtc_disable(crtc);
> + dev_priv->display._crtc_disable(crtc);

Don't you want to cancel the pending work here or it will be run on
resume - but on resume, we just want to send the hotplug event and let
userspace set itself up in the new configuration.
-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

[Intel-gfx] [PATCH] drm/i915: make sure PC8 is enabled on suspend and disabled on resume v4

From: Kristen Carlson Accardi 

This matches the runtime suspend paths and allows the system to enter
the lowest power mode at freeze time.

v2: move disable_pc8 call to thaw_early (Imre)
move enable_pc8 to freeze_late (Imre/Jesse)
v3: drop spurious hunk from _freeze now that we have freeze_late (Jesse)
v4: move back to suspend_late (Imre was right)

Signed-off-by: Kristen Carlson Accardi 
Signed-off-by: Jesse Barnes 
---
 drivers/gpu/drm/i915/i915_drv.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index a573f5a..2583442 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -608,6 +608,9 @@ static int i915_drm_thaw_early(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = dev->dev_private;
 
+   if (IS_HASWELL(dev) || IS_BROADWELL(dev))
+   hsw_disable_pc8(dev_priv);
+
intel_uncore_early_sanitize(dev);
intel_uncore_sanitize(dev);
intel_power_domains_init_hw(dev_priv);
@@ -891,6 +894,7 @@ static int i915_pm_suspend_late(struct device *dev)
 {
struct pci_dev *pdev = to_pci_dev(dev);
struct drm_device *drm_dev = pci_get_drvdata(pdev);
+   struct drm_i915_private *dev_priv = drm_dev->dev_private;
 
/*
 * We have a suspedn ordering issue with the snd-hda driver also
@@ -904,6 +908,9 @@ static int i915_pm_suspend_late(struct device *dev)
if (drm_dev->switch_power_state == DRM_SWITCH_POWER_OFF)
return 0;
 
+   if (IS_HASWELL(drm_dev) || IS_BROADWELL(drm_dev))
+   hsw_enable_pc8(dev_priv);
+
pci_disable_device(pdev);
pci_set_power_state(pdev, PCI_D3hot);
 
-- 
1.9.1

___
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: make CRTC enable/disable asynchronous v2

On Fri, 30 May 2014 19:47:56 +0100
Chris Wilson  wrote:

> On Fri, May 30, 2014 at 11:05:21AM -0700, Jesse Barnes wrote:
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c 
> > b/drivers/gpu/drm/i915/i915_drv.c
> > index e2bfdda..e7fa84f 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -530,7 +530,7 @@ static int i915_drm_freeze(struct drm_device *dev)
> > mutex_lock(&dev->mode_config.mutex);
> > for_each_crtc(dev, crtc) {
> > mutex_lock(&crtc->mutex);
> > -   dev_priv->display.crtc_disable(crtc);
> > +   dev_priv->display._crtc_disable(crtc);
> 
> Don't you want to cancel the pending work here or it will be run on
> resume - but on resume, we just want to send the hotplug event and let
> userspace set itself up in the new configuration.

No you're right I need to cancel it here and in unload, 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] [PATCH 1/2] drm/i915: make CRTC enable/disable asynchronous v2

On Fri, May 30, 2014 at 11:05:21AM -0700, Jesse Barnes wrote:
> @@ -8166,6 +8296,8 @@ static int intel_crtc_cursor_move(struct drm_crtc 
> *crtc, int x, int y)
>   intel_crtc->cursor_x = clamp_t(int, x, SHRT_MIN, SHRT_MAX);
>   intel_crtc->cursor_y = clamp_t(int, y, SHRT_MIN, SHRT_MAX);
>  
> + intel_sync_crtcs(crtc->dev->dev_private);
> +
>   if (intel_crtc->active)
>   intel_crtc_update_cursor(crtc, intel_crtc->cursor_bo != NULL);
>  

Since the pending CRTC enable/disable will set the cursor anyway, this
sync could be avoided if intel_crtc->active was accurate.
-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: make CRTC enable/disable asynchronous v2

On Fri, May 30, 2014 at 11:05:21AM -0700, Jesse Barnes wrote:
> +static void intel_queue_crtc_enable(struct drm_crtc *crtc)
> +{
> + struct drm_device *dev = crtc->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + struct intel_crtc_work *work;
> +
> + WARN(!mutex_is_locked(&dev->mode_config.mutex),
> +  "need mode_config mutex\n");
> +
> + work = kmalloc(sizeof(*work), GFP_KERNEL);
> + if (!work) {
> + dev_priv->display._crtc_disable(&intel_crtc->base);
> + return;
> + }
> +
> + work->enable = true;
> + work->intel_crtc = intel_crtc;
> + INIT_LIST_HEAD(&work->head);
(redundant, list_add doesn't care)

> +
> + list_add_tail(&dev_priv->crtc_work_queue, &work->head);
> + schedule_work(&dev_priv->crtc_work);
> +}

If we tracked one queued item per crtc, we could avoid the allocation
and allow for elision of pending operations.
-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/4] drm/i915: disable power wells on suspend

2014-05-30 Thread Kristen Carlson Accardi

On Fri, 30 May 2014 15:48:23 +0300
Imre Deak  wrote:

> On Thu, 2014-05-29 at 14:11 -0700, Jesse Barnes wrote:
> > From: Kristen Carlson Accardi 
> > 
> > We want to make sure everything is disabled and at its lowest power when
> > freezing.
> > 
> > Signed-off-by: Kristen Carlson Accardi 
> > Signed-off-by: Jesse Barnes 
> 
> Looks ok to me:
> Reviewed-by: Imre Deak 
> 
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c 
> > b/drivers/gpu/drm/i915/i915_drv.c
> > index e2bfdda..66c6ffb 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -551,6 +551,8 @@ static int i915_drm_freeze(struct drm_device *dev)
> >  
> > dev_priv->suspend_count++;
> >  
> > +   intel_display_set_init_power(dev_priv, false);
> > +
> > return 0;
> >  }
> >  
> 

This was actually Imre's patch not mine.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/4] drm/i915: leave rc6 enabled at suspend time

2014-05-30 Thread Kristen Carlson Accardi

On Thu, 29 May 2014 14:11:35 -0700
Jesse Barnes  wrote:

> From: Kristen Carlson Accardi 

Imre is the author.

> 
> This allows the system to enter the lowest power mode during system freeze.
> 
> Signed-off-by: Jesse Barnes 
> ---
>  drivers/gpu/drm/i915/i915_drv.c  |  3 ---
>  drivers/gpu/drm/i915/intel_drv.h |  1 +
>  drivers/gpu/drm/i915/intel_pm.c  | 16 +++-
>  3 files changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 66c6ffb..433bdfa 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -521,8 +521,6 @@ static int i915_drm_freeze(struct drm_device *dev)
>   drm_irq_uninstall(dev);
>   dev_priv->enable_hotplug_processing = false;Linux-3.16
>  
> - intel_disable_gt_powersave(dev);
> -
>   /*
>* Disable CRTCs directly since we want to preserve sw state
>* for _thaw.
> @@ -543,7 +541,6 @@ static int i915_drm_freeze(struct drm_device *dev)
>   i915_save_state(dev);
>  
>   intel_opregion_fini(dev);
> - intel_uncore_fini(dev);
>  
>   console_lock();
>   intel_fbdev_set_suspend(dev, FBINFO_STATE_SUSPENDED);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> b/drivers/gpu/drm/i915/intel_drv.h
> index c597b0d..bf90e7d 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -956,6 +956,7 @@ void intel_power_domains_init_hw(struct drm_i915_private 
> *dev_priv);
>  void intel_init_gt_powersave(struct drm_device *dev);
>  void intel_cleanup_gt_powersave(struct drm_device *dev);
>  void intel_enable_gt_powersave(struct drm_device *dev);
> +void intel_enable_gt_powersave_sync(struct drm_device *dev);
>  void intel_disable_gt_powersave(struct drm_device *dev);
>  void intel_reset_gt_powersave(struct drm_device *dev);
>  void ironlake_teardown_rc6(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 1840d15..8d9e036 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4891,12 +4891,9 @@ void intel_disable_gt_powersave(struct drm_device *dev)
>   }
>  }
>  
> -static void intel_gen6_powersave_work(struct work_struct *work)
> +void intel_enable_gt_powersave_sync(struct drm_device *dev)
>  {
> - struct drm_i915_private *dev_priv =
> - container_of(work, struct drm_i915_private,
> -  rps.delayed_resume_work.work);
> - struct drm_device *dev = dev_priv->dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
>  
>   mutex_lock(&dev_priv->rps.hw_lock);
>  
> @@ -4917,6 +4914,15 @@ static void intel_gen6_powersave_work(struct 
> work_struct *work)
>   intel_runtime_pm_put(dev_priv);
>  }
>  
> +static void intel_gen6_powersave_work(struct work_struct *work)
> +{
> + struct drm_i915_private *dev_priv =
> + container_of(work, struct drm_i915_private,
> +  rps.delayed_resume_work.work);
> +
> + intel_enable_gt_powersave_sync(dev_priv->dev);
> +}
> +
>  void intel_enable_gt_powersave(struct drm_device *dev)
>  {
>   struct drm_i915_private *dev_priv = dev->dev_private;

___
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: make CRTC enable/disable asynchronous v2

On Fri, 30 May 2014 19:56:22 +0100
Chris Wilson  wrote:

> On Fri, May 30, 2014 at 11:05:21AM -0700, Jesse Barnes wrote:
> > +static void intel_queue_crtc_enable(struct drm_crtc *crtc)
> > +{
> > +   struct drm_device *dev = crtc->dev;
> > +   struct drm_i915_private *dev_priv = dev->dev_private;
> > +   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > +   struct intel_crtc_work *work;
> > +
> > +   WARN(!mutex_is_locked(&dev->mode_config.mutex),
> > +"need mode_config mutex\n");
> > +
> > +   work = kmalloc(sizeof(*work), GFP_KERNEL);
> > +   if (!work) {
> > +   dev_priv->display._crtc_disable(&intel_crtc->base);
> > +   return;
> > +   }
> > +
> > +   work->enable = true;
> > +   work->intel_crtc = intel_crtc;
> > +   INIT_LIST_HEAD(&work->head);
> (redundant, list_add doesn't care)

Will fix.

> > +
> > +   list_add_tail(&dev_priv->crtc_work_queue, &work->head);
> > +   schedule_work(&dev_priv->crtc_work);
> > +}
> 
> If we tracked one queued item per crtc, we could avoid the allocation
> and allow for elision of pending operations.

Yeah I thought about that too, might make for a good optimization, but
I figured this was simplest to start with.

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

Re: [Intel-gfx] [PATCH 3/4] drm/i915: send proper opregion notifications on suspend/resume

2014-05-30 Thread Kristen Carlson Accardi

On Thu, 29 May 2014 14:11:36 -0700
Jesse Barnes  wrote:

> From: Kristen Carlson Accardi 

This one was based on a patch from Imre - I just added the D0 on
the resume path.

> 
> This indicates to the firmware that it can power down various other
> components or bring them back up, depending on the target system state.
> 
> Signed-off-by: Kristen Carlson Accardi 
> Signed-off-by: Jesse Barnes 
> ---
>  drivers/acpi/sleep.c|  1 +
>  drivers/gpu/drm/i915/i915_drv.c | 10 ++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
> index c40fb2e..807f333 100644
> --- a/drivers/acpi/sleep.c
> +++ b/drivers/acpi/sleep.c
> @@ -89,6 +89,7 @@ u32 acpi_target_system_state(void)
>  {
>   return acpi_target_sleep_state;
>  }
> +EXPORT_SYMBOL(acpi_target_system_state);
>  
>  static bool pwr_btn_event_pending;
>  
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 433bdfa..b6211d7 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -28,6 +28,7 @@
>   */
>  
>  #include 
> +#include 
>  #include 
>  #include 
>  #include "i915_drv.h"
> @@ -491,6 +492,7 @@ static int i915_drm_freeze(struct drm_device *dev)
>  {
>   struct drm_i915_private *dev_priv = dev->dev_private;
>   struct drm_crtc *crtc;
> + pci_power_t opregion_target_state;
>  
>   intel_runtime_pm_get(dev_priv);
>  
> @@ -540,6 +542,12 @@ static int i915_drm_freeze(struct drm_device *dev)
>  
>   i915_save_state(dev);
>  
> + if (acpi_target_system_state() >= ACPI_STATE_S3)
> + opregion_target_state = PCI_D3cold;
> + else
> + opregion_target_state = PCI_D1;
> + intel_opregion_notify_adapter(dev, opregion_target_state);
> +
>   intel_opregion_fini(dev);
>  
>   console_lock();
> @@ -671,6 +679,8 @@ static int __i915_drm_thaw(struct drm_device *dev, bool 
> restore_gtt_mappings)
>   dev_priv->modeset_restore = MODESET_DONE;
>   mutex_unlock(&dev_priv->modeset_restore_lock);
>  
> + intel_opregion_notify_adapter(dev, PCI_D0);
> +
>   intel_runtime_pm_put(dev_priv);
>   return 0;
>  }

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

Re: [Intel-gfx] [PATCH 4/4] drm/i915: make sure PC8 is enabled on suspend and disabled on resume

2014-05-30 Thread Rafael J. Wysocki

On Friday, May 30, 2014 11:29:15 AM Jesse Barnes wrote:
> On Fri, 30 May 2014 16:37:53 +0300
> Imre Deak  wrote:
> 
> > On Thu, 2014-05-29 at 14:11 -0700, Jesse Barnes wrote:
> > > From: Kristen Carlson Accardi 
> > > 
> > > This matches the runtime suspend paths and allows the system to enter
> > > the lowest power mode at freeze time.
> > > 
> > > Signed-off-by: Kristen Carlson Accardi 
> > > Signed-off-by: Jesse Barnes 
> > > ---
> > >  drivers/gpu/drm/i915/i915_drv.c | 6 ++
> > >  1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.c 
> > > b/drivers/gpu/drm/i915/i915_drv.c
> > > index b6211d7..24dc856 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > @@ -558,6 +558,9 @@ static int i915_drm_freeze(struct drm_device *dev)
> > >  
> > >   intel_display_set_init_power(dev_priv, false);
> > >  
> > > + if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> > > + hsw_enable_pc8(dev_priv);
> > > +
> > >   return 0;
> > >  }
> > >  
> > > @@ -618,6 +621,9 @@ static int __i915_drm_thaw(struct drm_device *dev, 
> > > bool restore_gtt_mappings)
> > >  {
> > >   struct drm_i915_private *dev_priv = dev->dev_private;
> > >  
> > > + if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> > > + hsw_disable_pc8(dev_priv);
> > 
> > I would put this before we access any of the HW regs in thaw_early() and
> > correspondingly the above call to hsw_enable_pc8() to suspend_late()
> > before we call pci_disable_device().
> > 
> > With that change this is:
> > Reviewed-by: Imre Deak 
> 
> For the thaw side I think that makes sense.
> 
> But for the freeze side, putting it in suspend_late won't get us the
> freeze behavior we want.  I think Rafael and Kristen are thinking of
> re-using the freeze infrastructure for some kind of connected standby
> feature, where most stuff is frozen, but the system isn't in S3 or S4,
> so we need the enable_pc8 call in the _freeze path as well.
> 
> Rafael, is that correct?

No, it isn't.  The .freeze()/.thaw() callbacks are hibernation-specific.
There are no plans for using this in PM beyond hibernation.

What we're going to use are .suspend/_late/_noirq() and the corresponding
resume callbacks and runtime PM.

> I'll add a late_freeze and put it there instead, so it doesn't pollute
> the S3 suspend path.

The freeze/thaw stuff need not do any PM BTW, it just needs to quiesce the
device to prevent it from doing DMA etc and then bring it back to life.

Rafael

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

Re: [Intel-gfx] [PATCH 4/4] drm/i915: make sure PC8 is enabled on suspend and disabled on resume

On Fri, 30 May 2014 23:12:45 +0200
"Rafael J. Wysocki"  wrote:

> On Friday, May 30, 2014 11:29:15 AM Jesse Barnes wrote:
> > On Fri, 30 May 2014 16:37:53 +0300
> > Imre Deak  wrote:
> > 
> > > On Thu, 2014-05-29 at 14:11 -0700, Jesse Barnes wrote:
> > > > From: Kristen Carlson Accardi 
> > > > 
> > > > This matches the runtime suspend paths and allows the system to enter
> > > > the lowest power mode at freeze time.
> > > > 
> > > > Signed-off-by: Kristen Carlson Accardi 
> > > > Signed-off-by: Jesse Barnes 
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_drv.c | 6 ++
> > > >  1 file changed, 6 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c 
> > > > b/drivers/gpu/drm/i915/i915_drv.c
> > > > index b6211d7..24dc856 100644
> > > > --- a/drivers/gpu/drm/i915/i915_drv.c
> > > > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > > > @@ -558,6 +558,9 @@ static int i915_drm_freeze(struct drm_device *dev)
> > > >  
> > > > intel_display_set_init_power(dev_priv, false);
> > > >  
> > > > +   if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> > > > +   hsw_enable_pc8(dev_priv);
> > > > +
> > > > return 0;
> > > >  }
> > > >  
> > > > @@ -618,6 +621,9 @@ static int __i915_drm_thaw(struct drm_device *dev, 
> > > > bool restore_gtt_mappings)
> > > >  {
> > > > struct drm_i915_private *dev_priv = dev->dev_private;
> > > >  
> > > > +   if (IS_HASWELL(dev) || IS_BROADWELL(dev))
> > > > +   hsw_disable_pc8(dev_priv);
> > > 
> > > I would put this before we access any of the HW regs in thaw_early() and
> > > correspondingly the above call to hsw_enable_pc8() to suspend_late()
> > > before we call pci_disable_device().
> > > 
> > > With that change this is:
> > > Reviewed-by: Imre Deak 
> > 
> > For the thaw side I think that makes sense.
> > 
> > But for the freeze side, putting it in suspend_late won't get us the
> > freeze behavior we want.  I think Rafael and Kristen are thinking of
> > re-using the freeze infrastructure for some kind of connected standby
> > feature, where most stuff is frozen, but the system isn't in S3 or S4,
> > so we need the enable_pc8 call in the _freeze path as well.
> > 
> > Rafael, is that correct?
> 
> No, it isn't.  The .freeze()/.thaw() callbacks are hibernation-specific.
> There are no plans for using this in PM beyond hibernation.
> 
> What we're going to use are .suspend/_late/_noirq() and the corresponding
> resume callbacks and runtime PM.
> 
> > I'll add a late_freeze and put it there instead, so it doesn't pollute
> > the S3 suspend path.
> 
> The freeze/thaw stuff need not do any PM BTW, it just needs to quiesce the
> device to prevent it from doing DMA etc and then bring it back to life.

Ok thanks.  Kristen corrected me on IRC too.  The latest patch I sent
should do what we want then, now that I've removed the freeze_late
function and put our PC8 enable in suspend_late like Imre suggested
initially.

-- 
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: make CRTC enable/disable asynchronous v3

This lets us return to userspace more quickly and should improve init
and suspend/resume times as well, allowing us to return to userspace
sooner.

This was initially motivated by slow resume time on some machines with
very long panel power sequencing times, and it should also improve boot
time when a full mode set is required.

v2: use a single enable/disable queue (Jesse/Chris)
fixup locking, test with lockdep (Jesse)
move hw state checks to sync_crtcs (Jesse)
make userspace initiated mode sets stay synchronous (Chris)
v3: take crtc lock around enable/disable (Jesse)
cancel work on suspend & unload (Chris)
complete work if alloc fails (Jesse)
drop unneeded list head init (Chris)
drop unneeded sync in cusor movement, rely on intel_crtc->active (Chris)
take mode_config mutex around cursor_set sync call (Jesse)
fix order of list_add_tail parameters (Jesse)

Signed-off-by: Jesse Barnes 

fix order of list add
take mutex around sync in cursor_set
---
 drivers/gpu/drm/i915/i915_drv.c  |   3 +-
 drivers/gpu/drm/i915/i915_drv.h  |  12 ++-
 drivers/gpu/drm/i915/intel_display.c | 177 +++
 drivers/gpu/drm/i915/intel_drv.h |   2 +-
 4 files changed, 173 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index e2bfdda..59a583f 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -527,10 +527,11 @@ static int i915_drm_freeze(struct drm_device *dev)
 * Disable CRTCs directly since we want to preserve sw state
 * for _thaw.
 */
+   cancel_work_sync(&dev_priv->crtc_work);
mutex_lock(&dev->mode_config.mutex);
for_each_crtc(dev, crtc) {
mutex_lock(&crtc->mutex);
-   dev_priv->display.crtc_disable(crtc);
+   dev_priv->display._crtc_disable(crtc);
mutex_unlock(&crtc->mutex);
}
mutex_unlock(&dev->mode_config.mutex);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index bea9ab40..bbfe402 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -447,8 +447,8 @@ struct drm_i915_display_funcs {
int (*crtc_mode_set)(struct drm_crtc *crtc,
 int x, int y,
 struct drm_framebuffer *old_fb);
-   void (*crtc_enable)(struct drm_crtc *crtc);
-   void (*crtc_disable)(struct drm_crtc *crtc);
+   void (*_crtc_enable)(struct drm_crtc *crtc);
+   void (*_crtc_disable)(struct drm_crtc *crtc);
void (*off)(struct drm_crtc *crtc);
void (*write_eld)(struct drm_connector *connector,
  struct drm_crtc *crtc,
@@ -1432,6 +1432,14 @@ struct drm_i915_private {
/* Display functions */
struct drm_i915_display_funcs display;
 
+   /**
+* CRTC work queue handling.  Enable/disable calls are queued
+* into the list and processed by the CRTC work function at some
+* later time, or inline by a call to intel_sync_crtcs().
+*/
+   struct list_head crtc_work_queue;
+   struct work_struct crtc_work;
+
/* PCH chipset type */
enum intel_pch pch_type;
unsigned short pch_id;
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 731cd01..d9e5d36 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1973,6 +1973,141 @@ static void lpt_disable_pch_transcoder(struct 
drm_i915_private *dev_priv)
I915_WRITE(_TRANSA_CHICKEN2, val);
 }
 
+struct intel_crtc_work {
+   /**
+* Whether to enable or disable the given CRTC
+*/
+   bool enable;
+   /**
+* CRTC to operate on
+*/
+   struct intel_crtc *intel_crtc;
+   /**
+* Used to link to dev_priv->crtc_work_queue, protected
+* by mode_config mutex.
+*/
+   struct list_head head;
+};
+
+/**
+ * intel_sync_crtcs - complete any pending CRTC enable/disable calls
+ * @dev_priv: i915 driver struct
+ *
+ * Walk the CRTC work queue and perform the enable/disable calls in the
+ * order they were added.
+ *
+ * This function will return when the enable/disable calls have been completed,
+ * and so may take many milliseconds before returning.
+ */
+static void intel_sync_crtcs(struct drm_i915_private *dev_priv)
+{
+   struct drm_device *dev = dev_priv->dev;
+   struct intel_crtc_work *crtc_work, *tmp;
+
+   WARN(!mutex_is_locked(&dev->mode_config.mutex),
+"need mode_config mutex\n");
+
+   list_for_each_entry_safe(crtc_work, tmp, &dev_priv->crtc_work_queue,
+head) {
+   struct drm_crtc *crtc = &crtc_work->intel_crtc->base;
+
+   mutex_lock(&crtc->mutex);
+

Re: [Intel-gfx] [PATCH] drm/i915: make CRTC enable/disable asynchronous v3

On Fri, May 30, 2014 at 02:28:52PM -0700, Jesse Barnes wrote:
> @@ -10326,7 +10466,7 @@ static int __intel_set_mode(struct drm_crtc *crtc,
>  
>   for_each_intel_crtc_masked(dev, prepare_pipes, intel_crtc) {
>   if (intel_crtc->base.enabled)
> - dev_priv->display.crtc_disable(&intel_crtc->base);
> + intel_queue_crtc_disable(&intel_crtc->base);
>   }

This one looks odd. prepare_pipes are the pipes we have to turn off in
order to perform the modeset - which needs to be synchronous.

intel_sync_crtcs(prepare_pipes) ?
-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 CRTC enable/disable asynchronous v3