[Intel-gfx] [PATCH 05/48] drm/i915: Takedown drm_mm on failed gtt setup

2013-12-06 Thread Ben Widawsky
This was found by code inspection. If the GTT setup fails then we are
left without properly tearing down the drm_mm.

Hopefully this never happens.

Signed-off-by: Ben Widawsky 
---
 drivers/gpu/drm/i915/i915_gem.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 998f165..61f88a8 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4507,6 +4507,7 @@ int i915_gem_init(struct drm_device *dev)
mutex_unlock(&dev->struct_mutex);
if (ret) {
i915_gem_cleanup_aliasing_ppgtt(dev);
+   drm_mm_takedown(&dev_priv->gtt.base.mm);
return ret;
}
 
-- 
1.8.4.2

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


[Intel-gfx] [PATCH 03/48] drm/i915: Don't unconditionally try to deref aliasing ppgtt

2013-12-06 Thread Ben Widawsky
Since the beginning, the functions which try to properly reference the
aliasing PPGTT have deferences a potentially null aliasing_ppgtt member.
Since the accessors are meant to be global, this will not do.

Introduced originally in:
commit a70a3148b0c61cb7c588ea650db785b261b378a3
Author: Ben Widawsky 
Date:   Wed Jul 31 16:59:56 2013 -0700

drm/i915: Make proper functions for VMs

Signed-off-by: Ben Widawsky 
---
 drivers/gpu/drm/i915/i915_gem.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 8b18c2a..5278b67 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4974,7 +4974,8 @@ unsigned long i915_gem_obj_offset(struct 
drm_i915_gem_object *o,
struct drm_i915_private *dev_priv = o->base.dev->dev_private;
struct i915_vma *vma;
 
-   if (vm == &dev_priv->mm.aliasing_ppgtt->base)
+   if (!dev_priv->mm.aliasing_ppgtt ||
+   vm == &dev_priv->mm.aliasing_ppgtt->base)
vm = &dev_priv->gtt.base;
 
BUG_ON(list_empty(&o->vma_list));
@@ -5015,7 +5016,8 @@ unsigned long i915_gem_obj_size(struct 
drm_i915_gem_object *o,
struct drm_i915_private *dev_priv = o->base.dev->dev_private;
struct i915_vma *vma;
 
-   if (vm == &dev_priv->mm.aliasing_ppgtt->base)
+   if (!dev_priv->mm.aliasing_ppgtt ||
+   vm == &dev_priv->mm.aliasing_ppgtt->base)
vm = &dev_priv->gtt.base;
 
BUG_ON(list_empty(&o->vma_list));
-- 
1.8.4.2

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


[Intel-gfx] [PATCH 02/48] drm/i915: Provide PDP updates via MMIO

2013-12-06 Thread Ben Widawsky
The initial implementation of this function used MMIO to write the PDPs.
Upon review it was determined (correctly) that the docs say to use LRI.
The issue is there are times where we want to do a synchronous write
(GPU reset).

I've tested this, and it works. I've verified with as many people as
possible that it should work.

This should fix the failing reset problems.

Signed-off-by: Ben Widawsky 
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index a54eaab..81420e1 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -199,12 +199,19 @@ static gen6_gtt_pte_t iris_pte_encode(dma_addr_t addr,
 
 /* Broadwell Page Directory Pointer Descriptors */
 static int gen8_write_pdp(struct intel_ring_buffer *ring, unsigned entry,
-  uint64_t val)
+  uint64_t val, bool synchronous)
 {
+   struct drm_i915_private *dev_priv = ring->dev->dev_private;
int ret;
 
BUG_ON(entry >= 4);
 
+   if (synchronous) {
+   I915_WRITE(GEN8_RING_PDP_UDW(ring, entry), val >> 32);
+   I915_WRITE(GEN8_RING_PDP_LDW(ring, entry), (u32)val);
+   return 0;
+   }
+
ret = intel_ring_begin(ring, 6);
if (ret)
return ret;
@@ -238,7 +245,8 @@ static int gen8_ppgtt_enable(struct drm_device *dev)
for (i = used_pd - 1; i >= 0; i--) {
dma_addr_t addr = ppgtt->pd_dma_addr[i];
for_each_ring(ring, dev_priv, j) {
-   ret = gen8_write_pdp(ring, i, addr);
+   ret = gen8_write_pdp(ring, i, addr,
+
i915_reset_in_progress(&dev_priv->gpu_error));
if (ret)
goto err_out;
}
-- 
1.8.4.2

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


[Intel-gfx] [PATCH 01/48] drm/i915: Fix bad refcounting on execbuf failures

2013-12-06 Thread Ben Widawsky
eb_destroy currently cleans up the refcounts for all the VMAs done at
lookup. Currently eb_lookup_vmas cleans up all the *objects* we've
looked up. There exists a period of time when we under severe memory
pressure, the VMA creation will fail, and fall into our exit path.

When the above event occurs, the object list, and eb->vma list are not
equal, the latter being a subset of the former. As we attempt to clean
up the refcounts on the error path we have the potential to decrement
the refcount by one extra here.

commit 27173f1f95db5e74ceb35fe9a2f2f348ea11bac9
Author: Ben Widawsky 
Date:   Wed Aug 14 11:38:36 2013 +0200

drm/i915: Convert execbuf code to use vmas

NOTE: A patch purporting the same results exists from Chris written to
address a quibble he had with something or other in this patch. I have
not tested that patch, but if it does the same thing I don't care
which is used.

Cc: sta...@vger.kernel.org
Cc: Chris Wilson 
Signed-off-by: Ben Widawsky 
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index f0c590e..cdab6e4 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -157,6 +157,13 @@ eb_lookup_vmas(struct eb_vmas *eb,
 
 
 out:
+   /* eb_vmas are cleaned up by destroy. Others are not */
+   if (ret) {
+   struct i915_vma *vma;
+   list_for_each_entry(vma, &eb->vmas, exec_list)
+   list_del(&vma->obj->obj_exec_link);
+   }
+
while (!list_empty(&objects)) {
obj = list_first_entry(&objects,
   struct drm_i915_gem_object,
-- 
1.8.4.2

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


[Intel-gfx] [PULL] PPGTT

2013-12-06 Thread Ben Widawsky
The following changes since commit 1ce477c917c75ce398e0def49f480327c9d0bab0:

  drm-intel-nightly: 2013y-12m-06d-13h-13m-07s integration manifest (2013-12-06 
13:13:33 +0100)

are available in the git repository at:

  git://people.freedesktop.org/~bwidawsk/drm-intel ppgtt

for you to fetch changes up to 7eae4c845ee80ad2ade3461f0291b83edd1207d9:

  page allocator: Tmp OOM deadlock w/a from Chris (2013-12-06 11:00:25 -0800)


This branch implemented full PPGTT support as has been discussed several times,
but most thoroughly:
http://lists.freedesktop.org/archives/intel-gfx/2013-June/029249.html

Quick recap: These patches enable GPU process isolation through the use of the
Per Process Graphics Translation Tables (PPGTT). These patches support the
feature on IVB, and HSW, with support coming next for BDW. Our existing
aliasing PPGTT support remains on SNB, and can be configured via modparam for
IVB and HSW. I have noted one TODO on the shared per fd default context, which
does inherit whatever ran last, and this potentially lets a bit of information
slip out from the last run context. Unlike the status listed in the
aforementioned mail, I think the patches are now quite stable.

Some tests need to be modified, and additionally there is one test from Oscar
Mateo Lozano. By the way, Oscar has done an outstanding job in helping me with
tests, and debug during my recent development.
I have those stored here:
http://cgit.freedesktop.org/~bwidawsk/intel-gpu-tools/log/?h=ppgtt

My overall feel on the patches is pretty good. Through the course of
development, I have had to work with a lot of OOM killer problems, many of
which were not introduced by my patches - simply exacerbated by. My gut tells
me (as well as intermittent dmesg spew) that problems still exist. Since other
OOM fixes are going in quite rapidly at the moment, I'm hopeful a merge now
will help to both capture the benefit of those other fixes, as well as perhaps
bring more problems to the surface while people are hunting in that area.

NOTES:
The drm patch needs to be sent to dri-devel.
The page allocator patch simply makes OOM inevitably angrier, but did help
  unblock things enough to find a couple of bugs.
One immediate TODO is a flag on context creation to allow users to opt-in to
  page faulting.


Ben Widawsky (47):
  drm/i915: Fix bad refcounting on execbuf failures
  drm/i915: Provide PDP updates via MMIO
  drm/i915: Don't unconditionally try to deref aliasing ppgtt
  drm/i915: Allow ggtt lookups to not WARN
  drm/i915: Takedown drm_mm on failed gtt setup
  drm/i915: Handle inactivating objects for all VMAs
  drm/i915: Add vm to error BO capture
  drm/i915: Don't use gtt mapping for !gtt error objects
  drm/i915: Identify active VM for batchbuffer capture
  drm/i915: Make pin count per VMA
  drm/i915: Create bind/unbind abstraction for VMAs
  drm/i915: Remove vm arg from relocate entry
  drm/i915: Add a context open function
  drm/i915: relax context alignment
  drm/i915: Simplify ring handling in execbuf
  drm/i915: Permit contexts on all rings
  drm/i915: Track which ring a context ran on
  drm/i915: Better reset handling for contexts
  drm/i915: Split context enabling from init
  drm/i915: Generalize default context setup
  drm/i915: PPGTT vfuncs should take a ppgtt argument
  drm/i915: Use drm_mm for PPGTT PDEs
  drm/i915: One hopeful eviction on PPGTT alloc
  drm/i915: Use platform specific ppgtt enable
  drm/i915: Extract mm switching to function
  drm/i915: Use LRI for switching PP_DIR_BASE
  drm/i915: Flush TLBs after !RCS PP_DIR_BASE
  drm/i915: Generalize PPGTT init
  drm/i915: Reorganize intel_enable_ppgtt
  drm/i915: Add VM to context
  drm/i915: Write PDEs at init instead of enable
  drm/i915: Restore PDEs for all VMs
  drm/i915: Do aliasing PPGTT init with contexts
  drm/i915: Create a per file_priv default context
  drm/i915: Piggy back hangstats off of contexts
  drm/i915: Get context early in execbuf
  drm/i915: Defer request freeing
  drm/i915: Clean up VMAs before freeing
  drm/i915: Do not allow buffers at offset 0
  drm/i915: Add a tracepoint for new VMs
  drm/i915: Use multiple VMs -- the point of no return
  drm/i915: Remove extraneous mm_switch in ppgtt enable
  drm/i915: Warn on gem_pin usage
  drm/i915: Add PPGTT dumper
  drm/i915: Dump all ppgtt
  drm/i915: Use topdown allocation for PPGTT
  page allocator: Tmp OOM deadlock w/a from Chris

Chris Wilson (1):
  drm: Optionally create mm blocks from top-to-bottom

 drivers/gpu/drm/drm_mm.c   |  56 ++-
 drivers/gpu/drm/i915/i915_debugfs.c|  40 +-
 drivers/gpu/drm/i915/i915_dma.c|   7 +-
 drivers/gpu/drm/i915/i915_drv.c|   3 +-
 drivers/gpu/drm/i915/i915_drv.h| 22

Re: [Intel-gfx] [PATCH 4/5] drm/i915: save some time when waiting the eDP timings

2013-12-06 Thread Jesse Barnes
On Fri,  6 Dec 2013 17:32:43 -0200
Paulo Zanoni  wrote:

> From: Paulo Zanoni 
> 
> The eDP spec defines some points where after you do action A, you have
> to wait some time before action B. The thing is that in our driver
> action B does not happen exactly after action A, but we still use
> msleep() calls directly. What this patch does is that we record the
> timestamp of when action A happened, then, just before action B, we
> look at how much time has passed and only sleep the remaining amount
> needed.
> 
> With this change, I am able to save about 5-20ms (out of the total
> 200ms) of the backlight_off delay and completely skip the 1ms
> backlight_on delay. The 600ms vdd_off delay doesn't happen during
> normal usage anymore due to a previous patch.
> 
> v2: - Rename ironlake_wait_jiffies_delay to intel_wait_until_after and
>   move it to intel_display.c
> - Fix the msleep call: diff is in jiffies
> v3: - Use "tmp_jiffies" so we don't need to worry about the value of
>   "jiffies" advancing while we're doing the math.
> 
> Signed-off-by: Paulo Zanoni 
> ---
>  drivers/gpu/drm/i915/intel_display.c | 18 ++
>  drivers/gpu/drm/i915/intel_dp.c  | 26 +++---
>  drivers/gpu/drm/i915/intel_drv.h |  4 
>  3 files changed, 45 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 3c59b67..0c238dd 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -806,6 +806,24 @@ void intel_wait_for_vblank(struct drm_device *dev, int 
> pipe)
>   DRM_DEBUG_KMS("vblank wait timed out\n");
>  }
>  
> +/* If you need to wait X ms between events A and B, but event B doesn't 
> happen
> + * exactly after event A, you record the timestamp (jiffies) of when event A
> + * happened, then just before event B you call intel_wait_until_after and 
> pass
> + * the timestamp as the first argument, and X as the second argument. */
> +void intel_wait_until_after(unsigned long timestamp, int to_wait_ms)
> +{
> + unsigned long target = timestamp + msecs_to_jiffies(to_wait_ms);
> + unsigned long diff;
> + /* Don't re-read the value of "jiffies" every time since it may change
> +  * behind our back and break the math. */
> + unsigned long tmp_jiffies = jiffies;
> +
> + if (time_after(target, tmp_jiffies)) {
> + diff = (long)target - (long)tmp_jiffies;
> + msleep(jiffies_to_msecs(diff));
> + }
> +}
> +
>  static bool pipe_dsl_stopped(struct drm_device *dev, enum pipe pipe)
>  {
>   struct drm_i915_private *dev_priv = dev->dev_private;
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index a2aace2..79f7ec2 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1056,9 +1056,26 @@ static void ironlake_wait_panel_off(struct intel_dp 
> *intel_dp)
>  static void ironlake_wait_panel_power_cycle(struct intel_dp *intel_dp)
>  {
>   DRM_DEBUG_KMS("Wait for panel power cycle\n");
> +
> + /* When we disable the VDD override bit last we have to do the manual
> +  * wait. */
> + intel_wait_until_after(intel_dp->last_power_cycle,
> +intel_dp->panel_power_cycle_delay);
> +
>   ironlake_wait_panel_status(intel_dp, IDLE_CYCLE_MASK, IDLE_CYCLE_VALUE);
>  }
>  
> +static void ironlake_wait_backlight_on(struct intel_dp *intel_dp)
> +{
> + intel_wait_until_after(intel_dp->last_power_on,
> +intel_dp->backlight_on_delay);
> +}
> +
> +static void ironlake_edp_wait_backlight_off(struct intel_dp *intel_dp)
> +{
> + intel_wait_until_after(intel_dp->last_backlight_off,
> +intel_dp->backlight_off_delay);
> +}
>  
>  /* Read the current pp_control value, unlocking the register if it
>   * is locked
> @@ -1144,7 +1161,7 @@ static void ironlake_panel_vdd_off_sync(struct intel_dp 
> *intel_dp)
>   I915_READ(pp_stat_reg), I915_READ(pp_ctrl_reg));
>  
>   if ((pp & POWER_TARGET_ON) == 0)
> - msleep(intel_dp->panel_power_cycle_delay);
> + intel_dp->last_power_cycle = jiffies;
>   }
>  }
>  
> @@ -1217,6 +1234,7 @@ void ironlake_edp_panel_on(struct intel_dp *intel_dp)
>   POSTING_READ(pp_ctrl_reg);
>  
>   ironlake_wait_panel_on(intel_dp);
> + intel_dp->last_power_on = jiffies;
>  
>   if (IS_GEN5(dev)) {
>   pp |= PANEL_POWER_RESET; /* restore panel reset bit */
> @@ -1237,6 +1255,8 @@ void ironlake_edp_panel_off(struct intel_dp *intel_dp)
>  
>   DRM_DEBUG_KMS("Turn eDP power off\n");
>  
> + ironlake_edp_wait_backlight_off(intel_dp);
> +
>   pp = ironlake_get_pp_control(intel_dp);
>   /* We need to switch off panel power _and_ force vdd, for otherwise some
>* panels get very unhappy and cease to work. */
> @@ -1268,7 +1288,7 @@ v

Re: [Intel-gfx] [PATCH 3/5] drm/i915: fix VDD override off wait

2013-12-06 Thread Jesse Barnes
On Fri,  6 Dec 2013 17:32:42 -0200
Paulo Zanoni  wrote:

> From: Paulo Zanoni 
> 
> If we're disabling the VDD override bit and the panel is enabled, we
> don't need to wait for anything. If the panel is disabled, then we
> need to actually wait for panel_power_cycle_delay, not
> panel_power_down_delay, because the power down delay was already
> respected when we disabled the panel.
> 
> Signed-off-by: Paulo Zanoni 
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index fe327ce..a2aace2 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1142,7 +1142,9 @@ static void ironlake_panel_vdd_off_sync(struct intel_dp 
> *intel_dp)
>   /* Make sure sequencer is idle before allowing subsequent 
> activity */
>   DRM_DEBUG_KMS("PP_STATUS: 0x%08x PP_CONTROL: 0x%08x\n",
>   I915_READ(pp_stat_reg), I915_READ(pp_ctrl_reg));
> - msleep(intel_dp->panel_power_down_delay);
> +
> + if ((pp & POWER_TARGET_ON) == 0)
> + msleep(intel_dp->panel_power_cycle_delay);
>   }
>  }
>  

Lemme check the eDP docs on this one...  it's supposed to be T12, which
is the time between power cycles.  Yeah that matches what we're using
elsewhere, so:

Reviewed-by: Jesse Barnes 

-- 
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 3/5] drm/i915: fix VDD override off wait

2013-12-06 Thread Paulo Zanoni
From: Paulo Zanoni 

If we're disabling the VDD override bit and the panel is enabled, we
don't need to wait for anything. If the panel is disabled, then we
need to actually wait for panel_power_cycle_delay, not
panel_power_down_delay, because the power down delay was already
respected when we disabled the panel.

Signed-off-by: Paulo Zanoni 
---
 drivers/gpu/drm/i915/intel_dp.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index fe327ce..a2aace2 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1142,7 +1142,9 @@ static void ironlake_panel_vdd_off_sync(struct intel_dp 
*intel_dp)
/* Make sure sequencer is idle before allowing subsequent 
activity */
DRM_DEBUG_KMS("PP_STATUS: 0x%08x PP_CONTROL: 0x%08x\n",
I915_READ(pp_stat_reg), I915_READ(pp_ctrl_reg));
-   msleep(intel_dp->panel_power_down_delay);
+
+   if ((pp & POWER_TARGET_ON) == 0)
+   msleep(intel_dp->panel_power_cycle_delay);
}
 }
 
-- 
1.8.3.1

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


[Intel-gfx] [PATCH 5/5] drm/i915: init the DP panel power seq variables earlier

2013-12-06 Thread Paulo Zanoni
From: Paulo Zanoni 

Our driver has two different ways of waiting for panel power
sequencing delays. One of these ways is through
ironlake_wait_panel_status, which implicitly uses the values written
to our registers. The other way is through the functions that call
intel_wait_until_after, and on this case we do direct msleep() calls
on the intel_dp->xxx_delay variables.

Function intel_dp_init_panel_power_sequencer is responsible for
initializing the _delay variables and deciding which values we need to
write to the registers, but it does not write these values to the
registers. Only at intel_dp_init_panel_power_sequencer_registers we
actually do this write.

Then problem is that when we call intel_dp_i2c_init, we will get some
I2C calls, which will trigger a VDD enable, which will make use of the
panel power sequencing registers and the _delay variables, so we need
to have both ready by this time. Today, when this happens, the _delay
variables are zero (because they were not computed) and the panel
power sequence registers contain whatever values were written by the
BIOS (which are usually correct).

What this patch does is to make sure that function
intel_dp_init_panel_power_sequencer is called earlier, so by the time
we call intel_dp_i2c_init, the _delay variables will already be
initialized. The actual registers won't contain their final values,
but at least they will contain the values set by the BIOS.

The good side is that we were reading the values, but were not using
them for anything (because we were just skipping the msleep(0) calls),
so this "fix" shouldn't fix any real existing bugs. I was only able to
identify the problem because I added some debug code to check how much
time time we were saving with my previous patch.

Regression introduced by:
commit ed92f0b239ac971edc509169ae3d6955fbe0a188
Author: Paulo Zanoni 
Date:   Wed Jun 12 17:27:24 2013 -0300
drm/i915: extract intel_edp_init_connector

v2: - Rewrite commit message.

Signed-off-by: Paulo Zanoni 
---
 drivers/gpu/drm/i915/intel_dp.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 79f7ec2..9cc5819 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3540,14 +3540,14 @@ intel_dp_init_panel_power_sequencer_registers(struct 
drm_device *dev,
 }
 
 static bool intel_edp_init_connector(struct intel_dp *intel_dp,
-struct intel_connector *intel_connector)
+struct intel_connector *intel_connector,
+struct edp_power_seq *power_seq)
 {
struct drm_connector *connector = &intel_connector->base;
struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
struct drm_device *dev = intel_dig_port->base.base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_display_mode *fixed_mode = NULL;
-   struct edp_power_seq power_seq = { 0 };
bool has_dpcd;
struct drm_display_mode *scan;
struct edid *edid;
@@ -3555,8 +3555,6 @@ static bool intel_edp_init_connector(struct intel_dp 
*intel_dp,
if (!is_edp(intel_dp))
return true;
 
-   intel_dp_init_panel_power_sequencer(dev, intel_dp, &power_seq);
-
/* Cache DPCD and EDID for edp. */
ironlake_edp_panel_vdd_on(intel_dp);
has_dpcd = intel_dp_get_dpcd(intel_dp);
@@ -3574,8 +3572,7 @@ static bool intel_edp_init_connector(struct intel_dp 
*intel_dp,
}
 
/* We now know it's not a ghost, init power sequence regs. */
-   intel_dp_init_panel_power_sequencer_registers(dev, intel_dp,
- &power_seq);
+   intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, power_seq);
 
edid = drm_get_edid(connector, &intel_dp->adapter);
if (edid) {
@@ -3624,6 +3621,7 @@ intel_dp_init_connector(struct intel_digital_port 
*intel_dig_port,
struct drm_device *dev = intel_encoder->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private;
enum port port = intel_dig_port->port;
+   struct edp_power_seq power_seq = { 0 };
const char *name = NULL;
int type, error;
 
@@ -3707,13 +3705,16 @@ intel_dp_init_connector(struct intel_digital_port 
*intel_dig_port,
BUG();
}
 
+   if (is_edp(intel_dp))
+   intel_dp_init_panel_power_sequencer(dev, intel_dp, &power_seq);
+
error = intel_dp_i2c_init(intel_dp, intel_connector, name);
WARN(error, "intel_dp_i2c_init failed with error %d for port %c\n",
 error, port_name(port));
 
intel_dp->psr_setup_done = false;
 
-   if (!intel_edp_init_connector(intel_dp, intel_connector)) {
+   if (!intel_edp_init_connector(intel_dp, intel_connector, &power_seq)) {
i2c_del

[Intel-gfx] [PATCH 4/5] drm/i915: save some time when waiting the eDP timings

2013-12-06 Thread Paulo Zanoni
From: Paulo Zanoni 

The eDP spec defines some points where after you do action A, you have
to wait some time before action B. The thing is that in our driver
action B does not happen exactly after action A, but we still use
msleep() calls directly. What this patch does is that we record the
timestamp of when action A happened, then, just before action B, we
look at how much time has passed and only sleep the remaining amount
needed.

With this change, I am able to save about 5-20ms (out of the total
200ms) of the backlight_off delay and completely skip the 1ms
backlight_on delay. The 600ms vdd_off delay doesn't happen during
normal usage anymore due to a previous patch.

v2: - Rename ironlake_wait_jiffies_delay to intel_wait_until_after and
  move it to intel_display.c
- Fix the msleep call: diff is in jiffies
v3: - Use "tmp_jiffies" so we don't need to worry about the value of
  "jiffies" advancing while we're doing the math.

Signed-off-by: Paulo Zanoni 
---
 drivers/gpu/drm/i915/intel_display.c | 18 ++
 drivers/gpu/drm/i915/intel_dp.c  | 26 +++---
 drivers/gpu/drm/i915/intel_drv.h |  4 
 3 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 3c59b67..0c238dd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -806,6 +806,24 @@ void intel_wait_for_vblank(struct drm_device *dev, int 
pipe)
DRM_DEBUG_KMS("vblank wait timed out\n");
 }
 
+/* If you need to wait X ms between events A and B, but event B doesn't happen
+ * exactly after event A, you record the timestamp (jiffies) of when event A
+ * happened, then just before event B you call intel_wait_until_after and pass
+ * the timestamp as the first argument, and X as the second argument. */
+void intel_wait_until_after(unsigned long timestamp, int to_wait_ms)
+{
+   unsigned long target = timestamp + msecs_to_jiffies(to_wait_ms);
+   unsigned long diff;
+   /* Don't re-read the value of "jiffies" every time since it may change
+* behind our back and break the math. */
+   unsigned long tmp_jiffies = jiffies;
+
+   if (time_after(target, tmp_jiffies)) {
+   diff = (long)target - (long)tmp_jiffies;
+   msleep(jiffies_to_msecs(diff));
+   }
+}
+
 static bool pipe_dsl_stopped(struct drm_device *dev, enum pipe pipe)
 {
struct drm_i915_private *dev_priv = dev->dev_private;
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index a2aace2..79f7ec2 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1056,9 +1056,26 @@ static void ironlake_wait_panel_off(struct intel_dp 
*intel_dp)
 static void ironlake_wait_panel_power_cycle(struct intel_dp *intel_dp)
 {
DRM_DEBUG_KMS("Wait for panel power cycle\n");
+
+   /* When we disable the VDD override bit last we have to do the manual
+* wait. */
+   intel_wait_until_after(intel_dp->last_power_cycle,
+  intel_dp->panel_power_cycle_delay);
+
ironlake_wait_panel_status(intel_dp, IDLE_CYCLE_MASK, IDLE_CYCLE_VALUE);
 }
 
+static void ironlake_wait_backlight_on(struct intel_dp *intel_dp)
+{
+   intel_wait_until_after(intel_dp->last_power_on,
+  intel_dp->backlight_on_delay);
+}
+
+static void ironlake_edp_wait_backlight_off(struct intel_dp *intel_dp)
+{
+   intel_wait_until_after(intel_dp->last_backlight_off,
+  intel_dp->backlight_off_delay);
+}
 
 /* Read the current pp_control value, unlocking the register if it
  * is locked
@@ -1144,7 +1161,7 @@ static void ironlake_panel_vdd_off_sync(struct intel_dp 
*intel_dp)
I915_READ(pp_stat_reg), I915_READ(pp_ctrl_reg));
 
if ((pp & POWER_TARGET_ON) == 0)
-   msleep(intel_dp->panel_power_cycle_delay);
+   intel_dp->last_power_cycle = jiffies;
}
 }
 
@@ -1217,6 +1234,7 @@ void ironlake_edp_panel_on(struct intel_dp *intel_dp)
POSTING_READ(pp_ctrl_reg);
 
ironlake_wait_panel_on(intel_dp);
+   intel_dp->last_power_on = jiffies;
 
if (IS_GEN5(dev)) {
pp |= PANEL_POWER_RESET; /* restore panel reset bit */
@@ -1237,6 +1255,8 @@ void ironlake_edp_panel_off(struct intel_dp *intel_dp)
 
DRM_DEBUG_KMS("Turn eDP power off\n");
 
+   ironlake_edp_wait_backlight_off(intel_dp);
+
pp = ironlake_get_pp_control(intel_dp);
/* We need to switch off panel power _and_ force vdd, for otherwise some
 * panels get very unhappy and cease to work. */
@@ -1268,7 +1288,7 @@ void ironlake_edp_backlight_on(struct intel_dp *intel_dp)
 * link.  So delay a bit to make sure the image is solid before
 * allowing it to appear.
 */
-   msleep(intel_dp->backlight_on_delay);
+   

[Intel-gfx] [PATCH 1/5] drm/i915: don't enable VDD just to enable the panel

2013-12-06 Thread Paulo Zanoni
From: Paulo Zanoni 

We just don't need this. This saves 250ms from every modeset on my
machine.

Reviewed-by: Rodrigo Vivi 
Signed-off-by: Paulo Zanoni 
---
 drivers/gpu/drm/i915/intel_ddi.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index c8a5fb7..432ead5 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1122,9 +1122,7 @@ static void intel_ddi_pre_enable(struct intel_encoder 
*intel_encoder)
 
if (type == INTEL_OUTPUT_EDP) {
struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
-   ironlake_edp_panel_vdd_on(intel_dp);
ironlake_edp_panel_on(intel_dp);
-   ironlake_edp_panel_vdd_off(intel_dp, true);
}
 
WARN_ON(intel_crtc->ddi_pll_sel == PORT_CLK_SEL_NONE);
-- 
1.8.3.1

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


[Intel-gfx] [PATCH 2/5] drm/i915: don't touch the VDD when disabling the panel

2013-12-06 Thread Paulo Zanoni
From: Paulo Zanoni 

I don't see a reason to touch VDD when we're disabling the panel:
since the panel is enabled, we don't need VDD. This saves a few sleep
calls from the vdd_on and vdd_off functions at every modeset.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=69693
Reviewed-by: Rodrigo Vivi 
Signed-off-by: Paulo Zanoni 
---
 drivers/gpu/drm/i915/intel_ddi.c | 1 -
 drivers/gpu/drm/i915/intel_dp.c  | 7 +--
 2 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 432ead5..b1b1d9d 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1165,7 +1165,6 @@ static void intel_ddi_post_disable(struct intel_encoder 
*intel_encoder)
 
if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP) {
struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
-   ironlake_edp_panel_vdd_on(intel_dp);
intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
ironlake_edp_panel_off(intel_dp);
}
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index bda2e1f..fe327ce 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1235,20 +1235,16 @@ void ironlake_edp_panel_off(struct intel_dp *intel_dp)
 
DRM_DEBUG_KMS("Turn eDP power off\n");
 
-   WARN(!intel_dp->want_panel_vdd, "Need VDD to turn off panel\n");
-
pp = ironlake_get_pp_control(intel_dp);
/* We need to switch off panel power _and_ force vdd, for otherwise some
 * panels get very unhappy and cease to work. */
-   pp &= ~(POWER_TARGET_ON | EDP_FORCE_VDD | PANEL_POWER_RESET | 
EDP_BLC_ENABLE);
+   pp &= ~(POWER_TARGET_ON | PANEL_POWER_RESET | EDP_BLC_ENABLE);
 
pp_ctrl_reg = _pp_ctrl_reg(intel_dp);
 
I915_WRITE(pp_ctrl_reg, pp);
POSTING_READ(pp_ctrl_reg);
 
-   intel_dp->want_panel_vdd = false;
-
ironlake_wait_panel_off(intel_dp);
 }
 
@@ -1774,7 +1770,6 @@ static void intel_disable_dp(struct intel_encoder 
*encoder)
 
/* Make sure the panel is off before trying to change the mode. But also
 * ensure that we have vdd while we switch off the panel. */
-   ironlake_edp_panel_vdd_on(intel_dp);
ironlake_edp_backlight_off(intel_dp);
intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
ironlake_edp_panel_off(intel_dp);
-- 
1.8.3.1

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


[Intel-gfx] [PATCH 0/5] Panel power sequencing improvements

2013-12-06 Thread Paulo Zanoni
From: Paulo Zanoni 

Hi

This series was already posted to the mailing list as part of the Runtime PM
series. Since the runtime PM code touches some eDP code, I originally made this
series on top of runtime PM because I was hoping runtime PM would be merged
earlier, so I wouldn't need to rebase this series.

It seems that these patches actually fix real bugs, so Jesse asked me to send
this series on top of -nightly so we could try to merge it earlier. Here it is.

Besides rebasing, the differences are on patch 4 and 5: they now should address
the review comments from Chris, Ben and Jani.

Looks like this series fixes freedesktop.org bug 69693, and it also helps us
getting closer to fix 72211.

Thanks,
Paulo

Paulo Zanoni (5):
  drm/i915: don't enable VDD just to enable the panel
  drm/i915: don't touch the VDD when disabling the panel
  drm/i915: fix VDD override off wait
  drm/i915: save some time when waiting the eDP timings
  drm/i915: init the DP panel power seq variables earlier

 drivers/gpu/drm/i915/intel_ddi.c |  3 ---
 drivers/gpu/drm/i915/intel_display.c | 18 ++
 drivers/gpu/drm/i915/intel_dp.c  | 48 +---
 drivers/gpu/drm/i915/intel_drv.h |  4 +++
 4 files changed, 55 insertions(+), 18 deletions(-)

-- 
1.8.3.1

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


Re: [Intel-gfx] [PATCH 19/19] drm/i915: init the DP panel power seq regs earlier

2013-12-06 Thread Paulo Zanoni
2013/12/5 Jani Nikula :
> On Thu, 21 Nov 2013, Paulo Zanoni  wrote:
>> From: Paulo Zanoni 
>>
>> When we call intel_dp_i2c_init we already get some I2C calls, which
>> will trigger a VDD enable, which will make use of the panel power
>> sequencing registers, so we need to have them ready by this time.
>
> But this patch won't make the registers ready either! The only thing
> this one does is initialize the delays in intel_dp. (And unlock the
> registers, but that's done in the vdd enable function too.)

Facepalm. You're right. I was mainly looking at the msleep() calls,
which are fixed by this patch. Besides this, we also need the
registers to be correct, but I guess this should be a separate patch.

>
> And you actually can't trivially initialize the registers either,
> because we will only later figure out whether this is a ghost eDP, and
> such initialization might botch up LVDS.

I wonder if we shouldn't at least try to do the right thing on HSW+,
since there's no LVDS there. But then there's the "eDP on port D" case
which we can't forget.


>
> See
> commit f30d26e468322b50d5e376bec40658683aff8ece
> Author: Jani Nikula 
> Date:   Wed Jan 16 10:53:40 2013 +0200
>
> drm/i915/eDP: do not write power sequence registers for ghost eDP
>
> I'm sorry I don't readily have any suggestions.
>
> If you are only interested in fixing the delays for msleep, then please
> fix the commit message!

Yeah, I'll fix the commit message for now, but I'll also think about
what to do with the registers.

Thanks,
Paulo

>
> BR,
> Jani.
>
>
>
>>
>> The good side is that we were reading the values, but were not using
>> them for anything (because we were just skipping the msleep(0) calls),
>> so this "fix" shouldn't fix any real existing bugs. I was only able to
>> identify the problem because I added some debug code to check how much
>> time time we were saving with my previous patch.
>>
>> Regression introduced by:
>> commit ed92f0b239ac971edc509169ae3d6955fbe0a188
>> Author: Paulo Zanoni 
>> Date:   Wed Jun 12 17:27:24 2013 -0300
>> drm/i915: extract intel_edp_init_connector
>>
>> Signed-off-by: Paulo Zanoni 
>> ---
>>  drivers/gpu/drm/i915/intel_dp.c | 15 ---
>>  1 file changed, 8 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c 
>> b/drivers/gpu/drm/i915/intel_dp.c
>> index 3a1ca80..23927a0 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -3565,14 +3565,14 @@ intel_dp_init_panel_power_sequencer_registers(struct 
>> drm_device *dev,
>>  }
>>
>>  static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>> -  struct intel_connector *intel_connector)
>> +  struct intel_connector *intel_connector,
>> +  struct edp_power_seq *power_seq)
>>  {
>>   struct drm_connector *connector = &intel_connector->base;
>>   struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>>   struct drm_device *dev = intel_dig_port->base.base.dev;
>>   struct drm_i915_private *dev_priv = dev->dev_private;
>>   struct drm_display_mode *fixed_mode = NULL;
>> - struct edp_power_seq power_seq = { 0 };
>>   bool has_dpcd;
>>   struct drm_display_mode *scan;
>>   struct edid *edid;
>> @@ -3580,8 +3580,6 @@ static bool intel_edp_init_connector(struct intel_dp 
>> *intel_dp,
>>   if (!is_edp(intel_dp))
>>   return true;
>>
>> - intel_dp_init_panel_power_sequencer(dev, intel_dp, &power_seq);
>> -
>>   /* Cache DPCD and EDID for edp. */
>>   ironlake_edp_panel_vdd_on(intel_dp);
>>   has_dpcd = intel_dp_get_dpcd(intel_dp);
>> @@ -3599,8 +3597,7 @@ static bool intel_edp_init_connector(struct intel_dp 
>> *intel_dp,
>>   }
>>
>>   /* We now know it's not a ghost, init power sequence regs. */
>> - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp,
>> -   &power_seq);
>> + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, 
>> power_seq);
>>
>>   edid = drm_get_edid(connector, &intel_dp->adapter);
>>   if (edid) {
>> @@ -3649,6 +3646,7 @@ intel_dp_init_connector(struct intel_digital_port 
>> *intel_dig_port,
>>   struct drm_device *dev = intel_encoder->base.dev;
>>   struct drm_i915_private *dev_priv = dev->dev_private;
>>   enum port port = intel_dig_port->port;
>> + struct edp_power_seq power_seq = { 0 };
>>   const char *name = NULL;
>>   int type, error;
>>
>> @@ -3748,13 +3746,16 @@ intel_dp_init_connector(struct intel_digital_port 
>> *intel_dig_port,
>>   BUG();
>>   }
>>
>> + if (is_edp(intel_dp))
>> + intel_dp_init_panel_power_sequencer(dev, intel_dp, &power_seq);
>> +
>>   error = intel_dp_i2c_init(intel_dp, intel_connector, name);
>>   WARN(error, "intel_dp_i2c_init failed with error %d for port %c\n",
>>  

Re: [Intel-gfx] [PATCH] drm/i915: Introduce vblank work function

2013-12-06 Thread Jesse Barnes
On Fri, 6 Dec 2013 14:42:29 +0100
Daniel Vetter  wrote:

> On Fri, Dec 06, 2013 at 12:12:21PM +, Bloomfield, Jon wrote:
> > Ok thanks. 
> > 
> > To add weight to it becoming official in some form, we're using it for 
> > various deferred operations:
> > drm/i915: Make plane switching asynchronous
> > drm/i915: Asynchronously unpin the old framebuffer after the next 
> > vblank
> > 
> > They aren't my patches but I believe they should be upstreamed in the near 
> > future. The claim is that these give a noticeable performance boost.
> > 
> > I'll leave it in and hope it becomes official.
> 
> For this stuff the upstream plane is to merge Ville's nuclear pageflip
> code, which is the full deal solution for all these issues. I haven't read
> his latest wip code to see what exactly he's using for all the vblank
> work.

I don't think that should block getting the vblank worker code in along
with the trivial change to the sprite code to avoid a wait_vblank in
the buffer switching case.  We've needed that since this code went in,
but for some reason Chris's code has been stalled the whole time (and
didn't we have a wq patch for the sprite unpin even before that?).

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


Re: [Intel-gfx] [PATCH] tests/gem_media_fill: Remove unnecessary include

2013-12-06 Thread Daniel Vetter
On Fri, Dec 06, 2013 at 12:38:49PM +, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin 
> 
> Causes trouble for Android builds.
> 
> Signed-off-by: Tvrtko Ursulin 

Applied, thanks.
-Daniel

> ---
>  tests/gem_media_fill.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/tests/gem_media_fill.c b/tests/gem_media_fill.c
> index 40b391d..b458ded 100644
> --- a/tests/gem_media_fill.c
> +++ b/tests/gem_media_fill.c
> @@ -32,7 +32,6 @@
>  
>  #include 
>  #include 
> -#include 
>  
>  #include "media_fill.h"
>  
> -- 
> 1.8.4.3
> 

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


Re: [Intel-gfx] [PATCH] tests/gem_evict_everything: Use bo_count instead of count where intended

2013-12-06 Thread Tvrtko Ursulin
On Fri, 2013-12-06 at 14:46 +0100, Daniel Vetter wrote:
> On Fri, Dec 06, 2013 at 12:33:28PM +, Tvrtko Ursulin wrote:
> > On Fri, 2013-12-06 at 13:12 +0100, Daniel Vetter wrote:
> > > On Fri, Dec 06, 2013 at 11:37:49AM +, Tvrtko Ursulin wrote:
> > > > From: Tvrtko Ursulin 
> > > > 
> > > > Don't see that it causes a problem but it looks it was intended
> > > > to use bo_count at these places.
> > > > 
> > > > Also using count to determine number of processes does not make
> > > > sense unless thousands of cores.
> > > > 
> > > > Signed-off-by: Tvrtko Ursulin 
> > > > ---
> > > >  tests/gem_evict_everything.c | 12 +---
> > > >  1 file changed, 5 insertions(+), 7 deletions(-)
> > > > 
> > > > diff --git a/tests/gem_evict_everything.c b/tests/gem_evict_everything.c
> > > > index 41abef7..90c3ae1 100644
> > > > --- a/tests/gem_evict_everything.c
> > > > +++ b/tests/gem_evict_everything.c
> > > > @@ -135,8 +135,6 @@ static void exchange_uint32_t(void *array, unsigned 
> > > > i, unsigned j)
> > > > i_arr[j] = i_tmp;
> > > >  }
> > > >  
> > > > -#define min(a, b) ((a) < (b) ? (a) : (b))
> > > > -
> > > >  #define INTERRUPTIBLE  (1 << 0)
> > > >  #define SWAPPING   (1 << 1)
> > > >  #define DUP_DRMFD  (1 << 2)
> > > > @@ -168,7 +166,7 @@ static void forked_evictions(int fd, int size, int 
> > > > count,
> > > > for (n = 0; n < bo_count; n++)
> > > > bo[n] = gem_create(fd, size);
> > > >  
> > > > -   igt_fork(i, min(count, min(num_threads * 5, 12))) {
> > > > +   igt_fork(i, num_threads * 4) {
> > > 
> > > You've killed the min( , 12) here ... that'll hurt on big desktops.
> > > Otherwise patch looks good.
> > 
> > It was hard for me to know what kind of stress was desired there.
> > Thinking of typical cases, single core single thread gives five
> > "stressers", more typical 2x1 gives 10. So it seems the whole
> > calculation will typically be between 10 and 12, 5 and 12 conditionally.
> > Which almost sounds a bit pointless.. I mean to have the calculation as
> > it was at all.
> 
> Well, igt stresstests are mostly random whacking until I'm fairly happy on
> a set of machines. But If you kill that max 12 runtime on bigger stuff
> will go through the roof for sure. And even on my really old single-core
> machines it's still ok. I suspect due to the thrashing the depency is
> fairly non-linear.

OK, I'll send a version with that clamp put back in.

Tvrtko


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


Re: [Intel-gfx] [Intel gfx][assembler][i-g-t PATCH] assembler/bdw: Update write(...)

2013-12-06 Thread Damien Lespiau
On Fri, Dec 06, 2013 at 09:16:58AM +0800, Xiang, Haihao wrote:
> From: "Xiang, Haihao" 
> 
> write(...) is used for Render Target Write and Media Block Write.
> The two message types no longer share the same cache agent on GEN8,
> So a parameter is needed for cache agent. The 4th parameter of write()
> is used for write commit bit which has been removed since GEN7. Hence
> we can re-use the 4th parameter as cache agent on GEN8
> 
> Signed-off-by: Xiang, Haihao 

Looks good to me, pushed. Thanks for the patch.

-- 
Damien

> ---
>  assembler/gram.y |   30 --
>  1 file changed, 28 insertions(+), 2 deletions(-)
> 
> diff --git a/assembler/gram.y b/assembler/gram.y
> index bdcfe79..ad4cb29 100644
> --- a/assembler/gram.y
> +++ b/assembler/gram.y
> @@ -1651,7 +1651,20 @@ msgtarget: NULL_TOKEN
>   INTEGER RPAREN
>   {
> if (IS_GENp(8)) {
> -  gen8_set_sfid(GEN8(&$$), 
> GEN6_SFID_DATAPORT_RENDER_CACHE);
> +  if ($9 != 0 &&
> +   $9 != GEN6_SFID_DATAPORT_SAMPLER_CACHE &&
> +   $9 != GEN6_SFID_DATAPORT_RENDER_CACHE &&
> +   $9 != GEN6_SFID_DATAPORT_CONSTANT_CACHE &&
> +   $9 != GEN7_SFID_DATAPORT_DATA_CACHE &&
> +   $9 != HSW_SFID_DATAPORT_DATA_CACHE1) {
> +   error (&@9, "error: wrong cache type\n");
> +   }
> +
> +   if ($9 == 0)
> +   gen8_set_sfid(GEN8(&$$), 
> GEN6_SFID_DATAPORT_RENDER_CACHE);
> +   else
> +   gen8_set_sfid(GEN8(&$$), $9);
> +
>gen8_set_header_present(GEN8(&$$), 1);
>gen8_set_dp_binding_table_index(GEN8(&$$), $3);
>gen8_set_dp_message_control(GEN8(&$$), $5);
> @@ -1701,7 +1714,20 @@ msgtarget: NULL_TOKEN
>   INTEGER COMMA INTEGER RPAREN
>   {
> if (IS_GENp(8)) {
> -  gen8_set_sfid(GEN8(&$$), 
> GEN6_SFID_DATAPORT_RENDER_CACHE);
> +  if ($9 != 0 &&
> +   $9 != GEN6_SFID_DATAPORT_SAMPLER_CACHE &&
> +   $9 != GEN6_SFID_DATAPORT_RENDER_CACHE &&
> +   $9 != GEN6_SFID_DATAPORT_CONSTANT_CACHE &&
> +   $9 != GEN7_SFID_DATAPORT_DATA_CACHE &&
> +   $9 != HSW_SFID_DATAPORT_DATA_CACHE1) {
> +   error (&@9, "error: wrong cache type\n");
> +   }
> +
> +   if ($9 == 0)
> +   gen8_set_sfid(GEN8(&$$), 
> GEN6_SFID_DATAPORT_RENDER_CACHE);
> +   else
> +   gen8_set_sfid(GEN8(&$$), $9);
> +
>gen8_set_header_present(GEN8(&$$), ($11 != 0));
>gen8_set_dp_binding_table_index(GEN8(&$$), $3);
>gen8_set_dp_message_control(GEN8(&$$), $5);
> -- 
> 1.7.9.5
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] tests/gem_evict_everything: Use bo_count instead of count where intended

2013-12-06 Thread Daniel Vetter
On Fri, Dec 06, 2013 at 12:33:28PM +, Tvrtko Ursulin wrote:
> On Fri, 2013-12-06 at 13:12 +0100, Daniel Vetter wrote:
> > On Fri, Dec 06, 2013 at 11:37:49AM +, Tvrtko Ursulin wrote:
> > > From: Tvrtko Ursulin 
> > > 
> > > Don't see that it causes a problem but it looks it was intended
> > > to use bo_count at these places.
> > > 
> > > Also using count to determine number of processes does not make
> > > sense unless thousands of cores.
> > > 
> > > Signed-off-by: Tvrtko Ursulin 
> > > ---
> > >  tests/gem_evict_everything.c | 12 +---
> > >  1 file changed, 5 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/tests/gem_evict_everything.c b/tests/gem_evict_everything.c
> > > index 41abef7..90c3ae1 100644
> > > --- a/tests/gem_evict_everything.c
> > > +++ b/tests/gem_evict_everything.c
> > > @@ -135,8 +135,6 @@ static void exchange_uint32_t(void *array, unsigned 
> > > i, unsigned j)
> > >   i_arr[j] = i_tmp;
> > >  }
> > >  
> > > -#define min(a, b) ((a) < (b) ? (a) : (b))
> > > -
> > >  #define INTERRUPTIBLE(1 << 0)
> > >  #define SWAPPING (1 << 1)
> > >  #define DUP_DRMFD(1 << 2)
> > > @@ -168,7 +166,7 @@ static void forked_evictions(int fd, int size, int 
> > > count,
> > >   for (n = 0; n < bo_count; n++)
> > >   bo[n] = gem_create(fd, size);
> > >  
> > > - igt_fork(i, min(count, min(num_threads * 5, 12))) {
> > > + igt_fork(i, num_threads * 4) {
> > 
> > You've killed the min( , 12) here ... that'll hurt on big desktops.
> > Otherwise patch looks good.
> 
> It was hard for me to know what kind of stress was desired there.
> Thinking of typical cases, single core single thread gives five
> "stressers", more typical 2x1 gives 10. So it seems the whole
> calculation will typically be between 10 and 12, 5 and 12 conditionally.
> Which almost sounds a bit pointless.. I mean to have the calculation as
> it was at all.

Well, igt stresstests are mostly random whacking until I'm fairly happy on
a set of machines. But If you kill that max 12 runtime on bigger stuff
will go through the roof for sure. And even on my really old single-core
machines it's still ok. I suspect due to the thrashing the depency is
fairly non-linear.

Longer-term I want to speed up all these memory thrashing tests by
mlocking most of main memory and so removing it from consideration. But
that's a bit of work to set up and roll out across all tests.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Introduce vblank work function

2013-12-06 Thread Daniel Vetter
On Fri, Dec 06, 2013 at 12:12:21PM +, Bloomfield, Jon wrote:
> Ok thanks. 
> 
> To add weight to it becoming official in some form, we're using it for 
> various deferred operations:
>   drm/i915: Make plane switching asynchronous
>   drm/i915: Asynchronously unpin the old framebuffer after the next 
> vblank
> 
> They aren't my patches but I believe they should be upstreamed in the near 
> future. The claim is that these give a noticeable performance boost.
> 
> I'll leave it in and hope it becomes official.

For this stuff the upstream plane is to merge Ville's nuclear pageflip
code, which is the full deal solution for all these issues. I haven't read
his latest wip code to see what exactly he's using for all the vblank
work.
-Daniel

> 
> > -Original Message-
> > From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel
> > Vetter
> > Sent: Friday, December 06, 2013 12:07 PM
> > To: Bloomfield, Jon
> > Cc: Chris Wilson; intel-gfx@lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Introduce vblank work function
> > 
> > On Fri, Dec 06, 2013 at 10:44:15AM +, Bloomfield, Jon wrote:
> > > What's the status of this patch ? I can't find any subsequent mention
> > > of it, but we currently use it in one of our Android development
> > > trees. I'm trying to work out whether to retain it or replace it.
> > >
> > > Was it killed off, or is it still in the pipeline ?
> > 
> > Stalled atm I think. The overall concept of a vblank worker/timer support
> > code is still useful imo. I think I've written up all the bikesheds Chris&I
> > discussed on irc in my other reply.
> > -Daniel
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > +41 (0) 79 365 57 48 - http://blog.ffwll.ch

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


Re: [Intel-gfx] [Intel gfx][i-g-t PATCH 3/4] rendercopy/bdw: A workaround for 3D pipeline

2013-12-06 Thread Damien Lespiau
On Fri, Dec 06, 2013 at 04:54:46PM +0800, Xiang, Haihao wrote:
> From: "Xiang, Haihao" 
> 
> Emit PIPELINE_SELECT twice and make sure the commands in
> the first batch buffer have been done.
> 
> However I don't know why this works !!!

Hum :) on one hand, it's great that you found this w/a, on the other
hand, I'm not comfortable with not understanding why this works. So far
what we know (I don't have Silicon that can't test anything):

 - Ken was saying that mesa doesn't need this.
 - There are a bunch of W/A around FF units clock gating, might worth
   checking that we're not hiting WaDisableFfDopClockGating or one of
   those 3D Vs GPGPU pipelines ones.
   This could happen to you but not to Ken because you have been
   switching between 3D and media pipeline with the 2 igt tests.
 - In any case, doing a pass on the W/A sounds like a good idea
 - I'd be interested to know if there a even more minimal batch that
   works (say an empty batch), or if the active ingredient is the
   pipeline switch.

If people want to push the patch to make progress on other parts, I
guess that's fine, but we'll need to dig deeper here.

-- 
Damien

> 
> Signed-off-by: Xiang, Haihao 
> ---
>  lib/rendercopy_gen8.c |   19 +--
>  1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/rendercopy_gen8.c b/lib/rendercopy_gen8.c
> index 1a137dd..6eb1051 100644
> --- a/lib/rendercopy_gen8.c
> +++ b/lib/rendercopy_gen8.c
> @@ -148,7 +148,8 @@ batch_copy(struct intel_batchbuffer *batch, const void 
> *ptr, uint32_t size, uint
>  
>  static void
>  gen6_render_flush(struct intel_batchbuffer *batch,
> -   drm_intel_context *context, uint32_t batch_end)
> + drm_intel_context *context, uint32_t batch_end,
> + int waiting)
>  {
>   int ret;
>  
> @@ -157,6 +158,11 @@ gen6_render_flush(struct intel_batchbuffer *batch,
>   ret = drm_intel_gem_bo_context_exec(batch->bo, context,
>   batch_end, 0);
>   assert(ret == 0);
> +
> + if (waiting) {
> + dri_bo_map(batch->bo, 0);
> + dri_bo_unmap(batch->bo);
> + }
>  }
>  
>  /* Mostly copy+paste from gen6, except height, width, pitch moved */
> @@ -880,6 +886,15 @@ void gen8_render_copyfunc(struct intel_batchbuffer 
> *batch,
>  
>   intel_batchbuffer_flush_with_context(batch, context);
>  
> + /* I don't know why it works !!! */
> + batch->ptr = batch->buffer;
> + OUT_BATCH(GEN6_PIPELINE_SELECT | PIPELINE_SELECT_3D);
> + OUT_BATCH(MI_BATCH_BUFFER_END);
> + batch_end = batch_align(batch, 8);
> + assert(batch_end < BATCH_STATE_SPLIT);
> + gen6_render_flush(batch, context, batch_end, 1);
> + intel_batchbuffer_reset(batch);
> +
>   batch_align(batch, 8);
>  
>   batch->ptr = &batch->buffer[BATCH_STATE_SPLIT];
> @@ -968,6 +983,6 @@ void gen8_render_copyfunc(struct intel_batchbuffer *batch,
>  
>   annotation_flush(&aub_annotations, batch);
>  
> - gen6_render_flush(batch, context, batch_end);
> + gen6_render_flush(batch, context, batch_end, 0);
>   intel_batchbuffer_reset(batch);
>  }
> -- 
> 1.7.9.5
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [Intel gfx][i-g-t PATCH 2/4] rendercopy/bdw: Set Instruction Buffer size Modify Enable to 1

2013-12-06 Thread Damien Lespiau
On Fri, Dec 06, 2013 at 02:15:18AM -0800, Kenneth Graunke wrote:
> On 12/06/2013 12:54 AM, Xiang, Haihao wrote:
> > From: "Xiang, Haihao" 
> > 
> > Otherwise it may result in GPU hang
> > 
> > Signed-off-by: Xiang, Haihao 
> > ---
> >  lib/rendercopy_gen8.c |2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/lib/rendercopy_gen8.c b/lib/rendercopy_gen8.c
> > index 43e962c..1a137dd 100644
> > --- a/lib/rendercopy_gen8.c
> > +++ b/lib/rendercopy_gen8.c
> > @@ -526,7 +526,7 @@ gen8_emit_state_base_address(struct intel_batchbuffer 
> > *batch) {
> > /* indirect object buffer size */
> > OUT_BATCH(0xf000 | 1);
> > /* intruction buffer size */
> > -   OUT_BATCH(1 << 12);
> > +   OUT_BATCH(1 << 12 | 1);
> >  }
> >  
> >  static void
> > 
> 
> Thanks a ton for finding this!
> 
> Reviewed-by: Kenneth Graunke 

Nice catch! Thanks for the patch and review, pushed.

-- 
Damien

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


Re: [Intel-gfx] [Intel gfx][i-g-t PATCH 1/4] lib: Clean the batch buffer store after reset

2013-12-06 Thread Damien Lespiau
On Fri, Dec 06, 2013 at 02:14:52AM -0800, Kenneth Graunke wrote:
> On 12/06/2013 12:54 AM, Xiang, Haihao wrote:
> > From: "Xiang, Haihao" 
> > 
> > Otherwise the stale data in the buffer
> > 
> > Signed-off-by: Xiang, Haihao 
> > ---
> >  lib/intel_batchbuffer.c |2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/lib/intel_batchbuffer.c b/lib/intel_batchbuffer.c
> > index 06a5437..9ce7424 100644
> > --- a/lib/intel_batchbuffer.c
> > +++ b/lib/intel_batchbuffer.c
> > @@ -50,6 +50,8 @@ intel_batchbuffer_reset(struct intel_batchbuffer *batch)
> > batch->bo = drm_intel_bo_alloc(batch->bufmgr, "batchbuffer",
> >BATCH_SZ, 4096);
> >  
> > +   memset(batch->buffer, 0, sizeof(batch->buffer));
> > +
> > batch->ptr = batch->buffer;
> >  }
> >  
> > 
> 
> I don't think that should be harmful, but this would definitely make
> debugging nicer.  For intel-gpu-tools, I think it makes sense.
> 
> Reviewed-by: Kenneth Graunke 

Indeed, I think it makes sense as well, thanks for the patch and review, pushed.

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


Re: [Intel-gfx] [PATCH 1/3] tests: drm_open_any doesn't fail

2013-12-06 Thread Damien Lespiau
On Fri, Dec 06, 2013 at 10:48:42AM +0100, Daniel Vetter wrote:
> Or more precisely: It already has an igt_require. So we cant ditch it
> from tests.
> 
> Signed-off-by: Daniel Vetter 

Reviewed-by: Damien Lespiau 

> ---
>  tests/kms_cursor_crc.c | 1 -
>  tests/kms_fbc_crc.c| 1 -
>  tests/kms_pipe_crc_basic.c | 1 -
>  3 files changed, 3 deletions(-)
> 
> diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
> index 74da32eb7497..b78ea7863585 100644
> --- a/tests/kms_cursor_crc.c
> +++ b/tests/kms_cursor_crc.c
> @@ -338,7 +338,6 @@ igt_main
>   const char *cmd = "pipe A none";
>  
>   data.drm_fd = drm_open_any();
> - igt_require(data.drm_fd >= 0);
>  
>   igt_set_vt_graphics_mode();
>  
> diff --git a/tests/kms_fbc_crc.c b/tests/kms_fbc_crc.c
> index 660086290f28..7a7f3903667b 100644
> --- a/tests/kms_fbc_crc.c
> +++ b/tests/kms_fbc_crc.c
> @@ -492,7 +492,6 @@ igt_main
>   FILE *status;
>  
>   data.drm_fd = drm_open_any();
> - igt_require(data.drm_fd);
>   igt_set_vt_graphics_mode();
>  
>   data.devid = intel_get_drm_devid(data.drm_fd);
> diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c
> index 3bc9eb0ee361..0e793cdf617d 100644
> --- a/tests/kms_pipe_crc_basic.c
> +++ b/tests/kms_pipe_crc_basic.c
> @@ -219,7 +219,6 @@ igt_main
>   const char *cmd = "pipe A none";
>  
>   data.drm_fd = drm_open_any();
> - igt_require(data.drm_fd >= 0);
>  
>   igt_set_vt_graphics_mode();
>  
> -- 
> 1.8.4.3
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 2/3] lib: add igt_pipe_crc_check

2013-12-06 Thread Damien Lespiau
On Fri, Dec 06, 2013 at 10:48:43AM +0100, Daniel Vetter wrote:
> No need to duplicate this all over the place.
> 
> Signed-off-by: Daniel Vetter 

(note that the use of buffered fopen/fwrite operations and the need to
flush the internal buffer could be just replaced with raw open/write and
was a left over from the initial implementation, but who cares).

Reviewed-by: Damien Lespiau 

> ---
>  lib/igt_debugfs.c  | 18 ++
>  lib/igt_debugfs.h  |  1 +
>  tests/kms_cursor_crc.c | 18 ++
>  tests/kms_fbc_crc.c| 14 +-
>  tests/kms_pipe_crc_basic.c | 25 -
>  5 files changed, 30 insertions(+), 46 deletions(-)
> 
> diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
> index 1ceaf59db11f..139be893f75b 100644
> --- a/lib/igt_debugfs.c
> +++ b/lib/igt_debugfs.c
> @@ -205,6 +205,24 @@ static void pipe_crc_exit_handler(int sig)
>   igt_pipe_crc_reset();
>  }
>  
> +void igt_pipe_crc_check(igt_debugfs_t *debugfs)
> +{
> + const char *cmd = "pipe A none";
> + FILE *ctl;
> + size_t written;
> + int ret;
> +
> + ctl = igt_debugfs_fopen(debugfs, "i915_display_crc_ctl", "r+");
> + igt_require_f(ctl,
> +   "No display_crc_ctl found, kernel too old\n");
> + written = fwrite(cmd, 1, strlen(cmd), ctl);
> + ret = fflush(ctl);
> + igt_require_f((written == strlen(cmd) && ret == 0) || errno != ENODEV,
> +   "CRCs not supported on this platform\n");
> +
> + fclose(ctl);
> +}
> +
>  igt_pipe_crc_t *
>  igt_pipe_crc_new(igt_debugfs_t *debugfs, int drm_fd, enum pipe pipe,
>enum intel_pipe_crc_source source)
> diff --git a/lib/igt_debugfs.h b/lib/igt_debugfs.h
> index 40d9d28fd49b..393b5767adbe 100644
> --- a/lib/igt_debugfs.h
> +++ b/lib/igt_debugfs.h
> @@ -70,6 +70,7 @@ bool igt_crc_is_null(igt_crc_t *crc);
>  bool igt_crc_equal(igt_crc_t *a, igt_crc_t *b);
>  char *igt_crc_to_string(igt_crc_t *crc);
>  
> +void igt_pipe_crc_check(igt_debugfs_t *debugfs);
>  igt_pipe_crc_t *
>  igt_pipe_crc_new(igt_debugfs_t *debugfs, int drm_fd, enum pipe pipe,
>enum intel_pipe_crc_source source);
> diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
> index b78ea7863585..d80695f67e2f 100644
> --- a/tests/kms_cursor_crc.c
> +++ b/tests/kms_cursor_crc.c
> @@ -53,7 +53,6 @@ typedef struct {
>   int drm_fd;
>   igt_debugfs_t debugfs;
>   drmModeRes *resources;
> - FILE *ctl;
>   uint32_t fb_id[NUM_CURSOR_TYPES];
>   struct kmstest_fb fb[NUM_CURSOR_TYPES];
>   igt_pipe_crc_t **pipe_crc;
> @@ -333,23 +332,12 @@ igt_main
>   igt_skip_on_simulation();
>  
>   igt_fixture {
> - size_t written;
> - int ret;
> - const char *cmd = "pipe A none";
> -
>   data.drm_fd = drm_open_any();
>  
>   igt_set_vt_graphics_mode();
>  
>   igt_debugfs_init(&data.debugfs);
> - data.ctl = igt_debugfs_fopen(&data.debugfs,
> -  "i915_display_crc_ctl", "r+");
> - igt_require_f(data.ctl,
> -   "No display_crc_ctl found, kernel too old\n");
> - written = fwrite(cmd, 1, strlen(cmd), data.ctl);
> - ret = fflush(data.ctl);
> - igt_require_f((written == strlen(cmd) && ret == 0) || errno != 
> ENODEV,
> -   "CRCs not supported on this platform\n");
> + igt_pipe_crc_check(&data.debugfs);
>  
>   display_init(&data);
>  
> @@ -376,8 +364,6 @@ igt_main
>   igt_subtest("cursor-black-invisible-offscreen")
>   run_test(&data, BLACK_INVISIBLE, false);
>  
> - igt_fixture {
> + igt_fixture
>   display_fini(&data);
> - fclose(data.ctl);
> - }
>  }
> diff --git a/tests/kms_fbc_crc.c b/tests/kms_fbc_crc.c
> index 7a7f3903667b..4cddd27428d0 100644
> --- a/tests/kms_fbc_crc.c
> +++ b/tests/kms_fbc_crc.c
> @@ -60,7 +60,6 @@ typedef struct {
>   int drm_fd;
>   igt_debugfs_t debugfs;
>   drmModeRes *resources;
> - FILE *ctl;
>   igt_crc_t ref_crc[2];
>   igt_pipe_crc_t **pipe_crc;
>   drm_intel_bufmgr *bufmgr;
> @@ -485,9 +484,6 @@ igt_main
>   igt_skip_on_simulation();
>  
>   igt_fixture {
> - size_t written;
> - int ret;
> - const char *cmd = "pipe A none";
>   char buf[64];
>   FILE *status;
>  
> @@ -497,14 +493,7 @@ igt_main
>   data.devid = intel_get_drm_devid(data.drm_fd);
>  
>   igt_debugfs_init(&data.debugfs);
> - data.ctl = igt_debugfs_fopen(&data.debugfs,
> -  "i915_display_crc_ctl", "r+");
> - igt_require_f(data.ctl,
> -   "No display_crc_ctl found, kernel too old\n");
> - written = fwrite(cmd, 1, strlen(cmd), data.ctl);
> - ret 

Re: [Intel-gfx] [PATCH 3/3] lib: make igt_pipe_crc_start never fail

2013-12-06 Thread Damien Lespiau
On Fri, Dec 06, 2013 at 10:48:44AM +0100, Daniel Vetter wrote:
> It's what callers expect - pipe_crc_new is the function where
> we pass a potential failure back to callers.
> 
> Signed-off-by: Daniel Vetter 

Oh, yes, initially ono had to specify the point at _start() time, but I
moved it to _new() mid-development and forgot to remove the return
value.

Reviewed-by: Damien Lespiau 

> ---
>  lib/igt_debugfs.c  | 7 ++-
>  lib/igt_debugfs.h  | 2 +-
>  tests/kms_pipe_crc_basic.c | 2 +-
>  3 files changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
> index 139be893f75b..4b96521331af 100644
> --- a/lib/igt_debugfs.c
> +++ b/lib/igt_debugfs.c
> @@ -269,12 +269,11 @@ void igt_pipe_crc_free(igt_pipe_crc_t *pipe_crc)
>   free(pipe_crc);
>  }
>  
> -bool igt_pipe_crc_start(igt_pipe_crc_t *pipe_crc)
> +void igt_pipe_crc_start(igt_pipe_crc_t *pipe_crc)
>  {
>   igt_crc_t *crcs = NULL;
>  
> - if (!igt_pipe_crc_do_start(pipe_crc))
> - return false;
> + igt_assert(igt_pipe_crc_do_start(pipe_crc));
>  
>   /*
>* For some no yet identified reason, the first CRC is bonkers. So
> @@ -282,8 +281,6 @@ bool igt_pipe_crc_start(igt_pipe_crc_t *pipe_crc)
>*/
>   igt_pipe_crc_get_crcs(pipe_crc, 1, &crcs);
>   free(crcs);
> -
> - return true;
>  }
>  
>  void igt_pipe_crc_stop(igt_pipe_crc_t *pipe_crc)
> diff --git a/lib/igt_debugfs.h b/lib/igt_debugfs.h
> index 393b5767adbe..075e44625213 100644
> --- a/lib/igt_debugfs.h
> +++ b/lib/igt_debugfs.h
> @@ -75,7 +75,7 @@ igt_pipe_crc_t *
>  igt_pipe_crc_new(igt_debugfs_t *debugfs, int drm_fd, enum pipe pipe,
>enum intel_pipe_crc_source source);
>  void igt_pipe_crc_free(igt_pipe_crc_t *pipe_crc);
> -bool igt_pipe_crc_start(igt_pipe_crc_t *pipe_crc);
> +void igt_pipe_crc_start(igt_pipe_crc_t *pipe_crc);
>  void igt_pipe_crc_stop(igt_pipe_crc_t *pipe_crc);
>  void igt_pipe_crc_get_crcs(igt_pipe_crc_t *pipe_crc, int n_crcs,
>  igt_crc_t **out_crcs);
> diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c
> index 90d9b9404877..3fc59344d90d 100644
> --- a/tests/kms_pipe_crc_basic.c
> +++ b/tests/kms_pipe_crc_basic.c
> @@ -180,7 +180,7 @@ static void test_read_crc(data_t *data, int pipe, 
> unsigned flags)
>   continue;
>   valid_connectors++;
>  
> - igt_assert(igt_pipe_crc_start(pipe_crc));
> + igt_pipe_crc_start(pipe_crc);
>  
>   /* wait for 3 vblanks and the corresponding 3 CRCs */
>   igt_pipe_crc_get_crcs(pipe_crc, 3, &crcs);
> -- 
> 1.8.4.3
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 5/6] drm/i915: parse backlight modulation frequency from the BIOS VBT

2013-12-06 Thread Rodrigo Vivi
Hi Jani,

On Mon, Dec 2, 2013 at 11:26 AM, Rodrigo Vivi  wrote:
> From: Jani Nikula 
>
> We don't actually do anything with the information yet, but parse and
> log what's in the VBT.
>
> Signed-off-by: Jani Nikula 
> Signed-off-by: Rodrigo Vivi 
> ---
>  drivers/gpu/drm/i915/i915_drv.h   |  5 +
>  drivers/gpu/drm/i915/intel_bios.c | 29 +
>  drivers/gpu/drm/i915/intel_bios.h | 16 
>  3 files changed, 50 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 780f815..687eee2 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1185,6 +1185,11 @@ struct intel_vbt_data {
> int edp_bpp;
> struct edp_power_seq edp_pps;
>
> +   struct {
> +   u16 pwm_freq_hz;
> +   bool active_low_pwm;
> +   } backlight;
> +
> /* MIPI DSI */
> struct {
> u16 panel_id;
> diff --git a/drivers/gpu/drm/i915/intel_bios.c 
> b/drivers/gpu/drm/i915/intel_bios.c
> index e4fba39..720ce55 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -281,6 +281,34 @@ parse_lfp_panel_data(struct drm_i915_private *dev_priv,
> }
>  }
>
> +static void
> +parse_lfp_backlight(struct drm_i915_private *dev_priv, struct bdb_header 
> *bdb)
> +{
> +   const struct bdb_lfp_backlight_data *backlight_data;
> +   const struct bdb_lfp_backlight_data_entry *entry;
> +
> +   backlight_data = find_section(bdb, BDB_LVDS_BACKLIGHT);
> +   if (!backlight_data)
> +   return;
> +
> +   if (backlight_data->entry_size != sizeof(backlight_data->data[0])) {
> +   DRM_DEBUG_KMS("Unsupported backlight data entry size %u\n",
> + backlight_data->entry_size);
> +   return;
> +   }
> +
> +   entry = &backlight_data->data[panel_type];
> +
> +   dev_priv->vbt.backlight.pwm_freq_hz = entry->pwm_freq_hz;
> +   dev_priv->vbt.backlight.active_low_pwm = entry->active_low_pwm;
> +   DRM_DEBUG_KMS("VBT backlight PWM modulation frequency %u Hz, "
> + "active %s, min brightness %u, level %u\n",
> + dev_priv->vbt.backlight.pwm_freq_hz,
> + dev_priv->vbt.backlight.active_low_pwm ? "low" : "high",
> + entry->min_brightness,
> + backlight_data->level[panel_type]);
> +}
> +
>  /* Try to find sdvo panel data */
>  static void
>  parse_sdvo_panel_data(struct drm_i915_private *dev_priv,
> @@ -894,6 +922,7 @@ intel_parse_bios(struct drm_device *dev)
> parse_general_features(dev_priv, bdb);
> parse_general_definitions(dev_priv, bdb);
> parse_lfp_panel_data(dev_priv, bdb);
> +   parse_lfp_backlight(dev_priv, bdb);
> parse_sdvo_panel_data(dev_priv, bdb);
> parse_sdvo_device_mapping(dev_priv, bdb);
> parse_device_mapping(dev_priv, bdb);
> diff --git a/drivers/gpu/drm/i915/intel_bios.h 
> b/drivers/gpu/drm/i915/intel_bios.h
> index 81ed58c..282de5e 100644
> --- a/drivers/gpu/drm/i915/intel_bios.h
> +++ b/drivers/gpu/drm/i915/intel_bios.h
> @@ -373,6 +373,22 @@ struct bdb_lvds_lfp_data {
> struct bdb_lvds_lfp_data_entry data[16];
>  } __packed;
>
> +struct bdb_lfp_backlight_data_entry {
> +   u8 type:2;
> +   u8 active_low_pwm:1;

I'd prefer inverted_polarity for this bit.

> +   u8 obsolete1:5;
> +   u16 pwm_freq_hz;
> +   u8 min_brightness;
> +   u8 obsolete2;
> +   u8 obsolete3;
> +} __packed;
> +
> +struct bdb_lfp_backlight_data {
> +   u8 entry_size;
> +   struct bdb_lfp_backlight_data_entry data[16];
> +   u8 level[16];
> +} __packed;
> +
>  struct aimdb_header {
> char signature[16];
> char oem_device[20];
> --
> 1.8.3.1
>

with or without my bikeshed,
Reviewed-by: Rodrigo Vivi 

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


[Intel-gfx] [PATCH] tests/gem_media_fill: Remove unnecessary include

2013-12-06 Thread Tvrtko Ursulin
From: Tvrtko Ursulin 

Causes trouble for Android builds.

Signed-off-by: Tvrtko Ursulin 
---
 tests/gem_media_fill.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tests/gem_media_fill.c b/tests/gem_media_fill.c
index 40b391d..b458ded 100644
--- a/tests/gem_media_fill.c
+++ b/tests/gem_media_fill.c
@@ -32,7 +32,6 @@
 
 #include 
 #include 
-#include 
 
 #include "media_fill.h"
 
-- 
1.8.4.3

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


Re: [Intel-gfx] [PATCH] tests/gem_evict_everything: Use bo_count instead of count where intended

2013-12-06 Thread Tvrtko Ursulin
On Fri, 2013-12-06 at 13:12 +0100, Daniel Vetter wrote:
> On Fri, Dec 06, 2013 at 11:37:49AM +, Tvrtko Ursulin wrote:
> > From: Tvrtko Ursulin 
> > 
> > Don't see that it causes a problem but it looks it was intended
> > to use bo_count at these places.
> > 
> > Also using count to determine number of processes does not make
> > sense unless thousands of cores.
> > 
> > Signed-off-by: Tvrtko Ursulin 
> > ---
> >  tests/gem_evict_everything.c | 12 +---
> >  1 file changed, 5 insertions(+), 7 deletions(-)
> > 
> > diff --git a/tests/gem_evict_everything.c b/tests/gem_evict_everything.c
> > index 41abef7..90c3ae1 100644
> > --- a/tests/gem_evict_everything.c
> > +++ b/tests/gem_evict_everything.c
> > @@ -135,8 +135,6 @@ static void exchange_uint32_t(void *array, unsigned i, 
> > unsigned j)
> > i_arr[j] = i_tmp;
> >  }
> >  
> > -#define min(a, b) ((a) < (b) ? (a) : (b))
> > -
> >  #define INTERRUPTIBLE  (1 << 0)
> >  #define SWAPPING   (1 << 1)
> >  #define DUP_DRMFD  (1 << 2)
> > @@ -168,7 +166,7 @@ static void forked_evictions(int fd, int size, int 
> > count,
> > for (n = 0; n < bo_count; n++)
> > bo[n] = gem_create(fd, size);
> >  
> > -   igt_fork(i, min(count, min(num_threads * 5, 12))) {
> > +   igt_fork(i, num_threads * 4) {
> 
> You've killed the min( , 12) here ... that'll hurt on big desktops.
> Otherwise patch looks good.

It was hard for me to know what kind of stress was desired there.
Thinking of typical cases, single core single thread gives five
"stressers", more typical 2x1 gives 10. So it seems the whole
calculation will typically be between 10 and 12, 5 and 12 conditionally.
Which almost sounds a bit pointless.. I mean to have the calculation as
it was at all.

Tvrtko


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


Re: [Intel-gfx] [PATCH] meh

2013-12-06 Thread Chris Wilson
On Fri, Dec 06, 2013 at 12:11:47PM +, Chris Wilson wrote:

Ah, must have been another machine with the verbose commit msg!
-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: Introduce vblank work function

2013-12-06 Thread Bloomfield, Jon
Ok thanks. 

To add weight to it becoming official in some form, we're using it for various 
deferred operations:
drm/i915: Make plane switching asynchronous
drm/i915: Asynchronously unpin the old framebuffer after the next 
vblank

They aren't my patches but I believe they should be upstreamed in the near 
future. The claim is that these give a noticeable performance boost.

I'll leave it in and hope it becomes official.

> -Original Message-
> From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel
> Vetter
> Sent: Friday, December 06, 2013 12:07 PM
> To: Bloomfield, Jon
> Cc: Chris Wilson; intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915: Introduce vblank work function
> 
> On Fri, Dec 06, 2013 at 10:44:15AM +, Bloomfield, Jon wrote:
> > What's the status of this patch ? I can't find any subsequent mention
> > of it, but we currently use it in one of our Android development
> > trees. I'm trying to work out whether to retain it or replace it.
> >
> > Was it killed off, or is it still in the pipeline ?
> 
> Stalled atm I think. The overall concept of a vblank worker/timer support
> code is still useful imo. I think I've written up all the bikesheds Chris&I
> discussed on irc in my other reply.
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] tests/gem_evict_everything: Use bo_count instead of count where intended

2013-12-06 Thread Daniel Vetter
On Fri, Dec 06, 2013 at 11:37:49AM +, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin 
> 
> Don't see that it causes a problem but it looks it was intended
> to use bo_count at these places.
> 
> Also using count to determine number of processes does not make
> sense unless thousands of cores.
> 
> Signed-off-by: Tvrtko Ursulin 
> ---
>  tests/gem_evict_everything.c | 12 +---
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/gem_evict_everything.c b/tests/gem_evict_everything.c
> index 41abef7..90c3ae1 100644
> --- a/tests/gem_evict_everything.c
> +++ b/tests/gem_evict_everything.c
> @@ -135,8 +135,6 @@ static void exchange_uint32_t(void *array, unsigned i, 
> unsigned j)
>   i_arr[j] = i_tmp;
>  }
>  
> -#define min(a, b) ((a) < (b) ? (a) : (b))
> -
>  #define INTERRUPTIBLE(1 << 0)
>  #define SWAPPING (1 << 1)
>  #define DUP_DRMFD(1 << 2)
> @@ -168,7 +166,7 @@ static void forked_evictions(int fd, int size, int count,
>   for (n = 0; n < bo_count; n++)
>   bo[n] = gem_create(fd, size);
>  
> - igt_fork(i, min(count, min(num_threads * 5, 12))) {
> + igt_fork(i, num_threads * 4) {

You've killed the min( , 12) here ... that'll hurt on big desktops.
Otherwise patch looks good.
-Daniel

>   int realfd = fd;
>   int num_passes = flags & SWAPPING ? 10 : 100;
>  
> @@ -184,7 +182,7 @@ static void forked_evictions(int fd, int size, int count,
>   realfd = drm_open_any();
>  
>   /* We can overwrite the bo array since we're forked. */
> - for (l = 0; l < count; l++) {
> + for (l = 0; l < bo_count; l++) {
>   uint32_t flink;
>  
>   flink = gem_flink(fd, bo[l]);
> @@ -194,9 +192,9 @@ static void forked_evictions(int fd, int size, int count,
>   }
>  
>   for (pass = 0; pass < num_passes; pass++) {
> - copy(realfd, bo[0], bo[1], bo, count, 0);
> + copy(realfd, bo[0], bo[1], bo, bo_count, 0);
>  
> - for (l = 0; l < count && (flags & MEMORY_PRESSURE); 
> l++) {
> + for (l = 0; l < bo_count && (flags & MEMORY_PRESSURE); 
> l++) {
>   uint32_t *base = gem_mmap__cpu(realfd, bo[l],
>  size,
>  PROT_READ | 
> PROT_WRITE);
> @@ -244,7 +242,7 @@ static void swapping_evictions(int fd, int size, int 
> count)
>   igt_permute_array(bo, bo_count, exchange_uint32_t);
>  
>   for (pass = 0; pass < 100; pass++) {
> - copy(fd, bo[0], bo[1], bo, count, 0);
> + copy(fd, bo[0], bo[1], bo, bo_count, 0);
>   }
>   }
>  
> -- 
> 1.8.4.3
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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


[Intel-gfx] [PATCH] meh

2013-12-06 Thread Chris Wilson
---
 drivers/gpu/drm/i915/i915_gem_evict.c | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c 
b/drivers/gpu/drm/i915/i915_gem_evict.c
index b7376533633d..8f3adc7d0dc8 100644
--- a/drivers/gpu/drm/i915/i915_gem_evict.c
+++ b/drivers/gpu/drm/i915/i915_gem_evict.c
@@ -88,6 +88,7 @@ i915_gem_evict_something(struct drm_device *dev, struct 
i915_address_space *vm,
} else
drm_mm_init_scan(&vm->mm, min_size, alignment, cache_level);
 
+search_again:
/* First see if there is a large enough contiguous idle region... */
list_for_each_entry(vma, &vm->inactive_list, mm_list) {
if (mark_free(vma, &unwind_list))
@@ -115,10 +116,17 @@ none:
list_del_init(&vma->exec_list);
}
 
-   /* We expect the caller to unpin, evict all and try again, or give up.
-* So calling i915_gem_evict_vm() is unnecessary.
+   /* Can we unpin some objects such as idle hw contents,
+* or pending flips?
 */
-   return -ENOSPC;
+   ret = nonblocking ? -ENOSPC : i915_gpu_idle(dev);
+   if (ret)
+   return ret;
+
+   /* Only idle the GPU and repeat the search once */
+   i915_gem_retire_requests(dev);
+   nonblocking = true;
+   goto search_again;
 
 found:
/* drm_mm doesn't allow any other other operations while
-- 
1.8.5.1

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


Re: [Intel-gfx] [PATCH] drm/i915: Fix use-after-free in do_switch

2013-12-06 Thread Daniel Vetter
On Fri, Dec 06, 2013 at 10:05:51AM +, Lister, Ian wrote:
> 
> 
> > -Original Message-
> > From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch]
> > Sent: Thursday, December 05, 2013 2:43 PM
> > To: Intel Graphics Development
> > Cc: Daniel Vetter; Lister, Ian; sta...@vger.kernel.org; Widawsky, Benjamin;
> > Stéphane Marchesin; Bloomfield, Jon
> > Subject: [PATCH] drm/i915: Fix use-after-free in do_switch
> > 
> > So apparently under ridiculous amounts of memory pressure we can get into
> > trouble in do_switch when we try to move the old hw context backing
> > storage object onto the active lists.
> > 
> > With list debugging enabled that usually results in us chasing a poisoned
> > pointer - which means we've hit upon a vma that has been removed from all
> > lrus with list_del (and then deallocated, so it's a real use-after free).
> > 
> > Ian Lister has done some great callchain chasing and noticed that we can
> > reenter do_switch:
> > 
> > i915_gem_do_execbuffer()
> > 
> > i915_switch_context()
> > 
> > do_switch()
> >from = ring->last_context;
> >i915_gem_object_pin()
> > 
> >   i915_gem_object_bind_to_gtt()
> >  ret = drm_mm_insert_node_in_range_generic();
> >  // If the above call fails then it will try 
> > i915_gem_evict_something()
> >  // If that fails it will call i915_gem_evict_everything() ...
> >  i915_gem_evict_everything()
> > i915_gpu_idle()
> >i915_switch_context(DEFAULT_CONTEXT)
> > 
> > Like with everything else where the shrinker or eviction code can invalidate
> > pointers we need to reload relevant state.
> > 
> > Note that there's no need to recheck whether a context switch is still
> > required because:
> > 
> > - Doing a switch to the same context is harmless (besides wasting a
> >   bit of energy).
> > 
> > - This can only happen with the default context. But since that one's
> >   pinned we'll never call down into evict_everything under normal
> >   circumstances. Note that there's a little driver bringup fun
> >   involved namely that we could recourse into do_switch for the
> >   initial switch. Atm we're fine since we assign the context pointer
> >   only after the call to do_switch at driver load or resume time. And
> >   in the gpu reset case we skip the entire setup sequence (which might
> >   be a bug on its own, but definitely not this one here).
> > 
> > Cc'ing stable since apparently ChromeOS guys are seeing this in the wild 
> > (and
> > not just on artificial stress tests), see the reference.
> > 
> > Note that in upstream code doesn't calle evict_everything directly from
> > evict_something, that's an extension in this product branch. But we can 
> > still
> > hit upon this bug (and apparently we do, see the linked backtraces). I've
> > noticed this while trying to construct a testcase for this bug and utterly 
> > failed
> > to provoke it. It looks like we need to driver the system squarly into the
> > lowmem wall and provoke the shrinker to evict the context object by doing
> > the last-ditch evict_everything call.
> > 
> > Aside: There's currently no means to get a badly-fragmenting hw context
> > object away from a bad spot in the upstream code. We should fix this by at
> > least adding some code to evict_something to handle hw contexts.
> > 
> > References: https://code.google.com/p/chromium/issues/detail?id=248191
> > Reported-by: Ian Lister 
> > Cc: Ian Lister 
> > Cc: sta...@vger.kernel.org
> > Cc: Ben Widawsky 
> > Cc: Stéphane Marchesin 
> > Cc: Bloomfield, Jon 
> > Signed-off-by: Daniel Vetter 
> 
> Reviewed-by: Ian Lister 

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


Re: [Intel-gfx] [PATCH] drm/i915: fix pm init ordering

2013-12-06 Thread Daniel Vetter
On Fri, Dec 06, 2013 at 10:17:53AM +0100, Daniel Vetter wrote:
> Shovel a bit more of the the code into the setup function, and call
> it earlier. Otherwise lockdep is unhappy since we cancel the delayed
> resume work before it's initialized.
> 
> While at it also shovel the pc8 setup code into the same functions.
> I wanted to also ditch the header declaration of the hws pc8 functions,
> but for unfathomable reasons that stuff is in intel_display.c instead
> of intel_pm.c.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=71980
> Tested-by: Guo Jinxian 
> Signed-off-by: Daniel Vetter 

Merged to -fixes with Damien's irc r-b.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Introduce vblank work function

2013-12-06 Thread Daniel Vetter
On Fri, Dec 06, 2013 at 10:44:15AM +, Bloomfield, Jon wrote:
> What's the status of this patch ? I can't find any subsequent mention of
> it, but we currently use it in one of our Android development trees. I'm
> trying to work out whether to retain it or replace it.
> 
> Was it killed off, or is it still in the pipeline ?

Stalled atm I think. The overall concept of a vblank worker/timer support
code is still useful imo. I think I've written up all the bikesheds
Chris&I discussed on irc in my other reply.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [Intel gfx][i-g-t PATCH (v3) 1/4] tests: add gem_media_fill

2013-12-06 Thread Tvrtko Ursulin

Hi,

On Fri, 2013-12-06 at 08:55 +0800, Xiang, Haihao wrote:

> +++ b/tests/gem_media_fill.c
...
> +#include 

It does not look like this include is needed and it is troublesome on
Android.

Regards,

Tvrtko


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


[Intel-gfx] [PATCH] tests/gem_evict_everything: Use bo_count instead of count where intended

2013-12-06 Thread Tvrtko Ursulin
From: Tvrtko Ursulin 

Don't see that it causes a problem but it looks it was intended
to use bo_count at these places.

Also using count to determine number of processes does not make
sense unless thousands of cores.

Signed-off-by: Tvrtko Ursulin 
---
 tests/gem_evict_everything.c | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/tests/gem_evict_everything.c b/tests/gem_evict_everything.c
index 41abef7..90c3ae1 100644
--- a/tests/gem_evict_everything.c
+++ b/tests/gem_evict_everything.c
@@ -135,8 +135,6 @@ static void exchange_uint32_t(void *array, unsigned i, 
unsigned j)
i_arr[j] = i_tmp;
 }
 
-#define min(a, b) ((a) < (b) ? (a) : (b))
-
 #define INTERRUPTIBLE  (1 << 0)
 #define SWAPPING   (1 << 1)
 #define DUP_DRMFD  (1 << 2)
@@ -168,7 +166,7 @@ static void forked_evictions(int fd, int size, int count,
for (n = 0; n < bo_count; n++)
bo[n] = gem_create(fd, size);
 
-   igt_fork(i, min(count, min(num_threads * 5, 12))) {
+   igt_fork(i, num_threads * 4) {
int realfd = fd;
int num_passes = flags & SWAPPING ? 10 : 100;
 
@@ -184,7 +182,7 @@ static void forked_evictions(int fd, int size, int count,
realfd = drm_open_any();
 
/* We can overwrite the bo array since we're forked. */
-   for (l = 0; l < count; l++) {
+   for (l = 0; l < bo_count; l++) {
uint32_t flink;
 
flink = gem_flink(fd, bo[l]);
@@ -194,9 +192,9 @@ static void forked_evictions(int fd, int size, int count,
}
 
for (pass = 0; pass < num_passes; pass++) {
-   copy(realfd, bo[0], bo[1], bo, count, 0);
+   copy(realfd, bo[0], bo[1], bo, bo_count, 0);
 
-   for (l = 0; l < count && (flags & MEMORY_PRESSURE); 
l++) {
+   for (l = 0; l < bo_count && (flags & MEMORY_PRESSURE); 
l++) {
uint32_t *base = gem_mmap__cpu(realfd, bo[l],
   size,
   PROT_READ | 
PROT_WRITE);
@@ -244,7 +242,7 @@ static void swapping_evictions(int fd, int size, int count)
igt_permute_array(bo, bo_count, exchange_uint32_t);
 
for (pass = 0; pass < 100; pass++) {
-   copy(fd, bo[0], bo[1], bo, count, 0);
+   copy(fd, bo[0], bo[1], bo, bo_count, 0);
}
}
 
-- 
1.8.4.3

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


Re: [Intel-gfx] [PATCH v2 5/7] drm/i915: Reorganize the DSI enable/disable sequence

2013-12-06 Thread Shobhit Kumar

On Friday 15 November 2013 01:57 PM, Jani Nikula wrote:

On Sat, 09 Nov 2013, Shobhit Kumar  wrote:

Basically ULPS handling during enable/disable has been moved to
pre_enable and post_disable phases. PLL and panel power disable
also has been moved to post_disable phase. The ULPS entry/exit
sequneces as suggested by HW team is as follows -

During enable time -
set DEVICE_READY --> Clear DEVICE_READY --> set DEVICE_READY

And during disable time to flush all FIFOs -
set ENTER_SLEEP --> EXIT_SLEEP --> ENTER_SLEEP

Also during disbale sequnece sub-encoder disable is moved to the end
after port is disabled.

v2: Based on comments from Ville
 - Detailed epxlaination in the commit messgae
 - Moved parameter changes out into another patch
 - Backlight enabling will be a new patch

Signed-off-by: Yogesh Mohan Marimuthu 
Signed-off-by: Shobhit Kumar 
---
  drivers/gpu/drm/i915/i915_drv.h  |   11 
  drivers/gpu/drm/i915/intel_dsi.c |  111 ++
  drivers/gpu/drm/i915/intel_dsi.h |2 +
  3 files changed, 91 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a2bbff9..55c16cb 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2433,6 +2433,17 @@ int vlv_freq_opcode(int ddr_freq, int val);
  #define POSTING_READ(reg) (void)I915_READ_NOTRACE(reg)
  #define POSTING_READ16(reg)   (void)I915_READ16_NOTRACE(reg)

+#define I915_WRITE_BITS(reg, val, mask) \
+do { \
+   u32 tmp, data; \
+   tmp = I915_READ((reg)); \
+   tmp &= ~(mask); \
+   data = (val) & (mask); \
+   data = data | tmp; \
+   I915_WRITE((reg), data); \
+} while(0)


I would still prefer the explicit read, modify, and write in the code
instead of this, but it's a matter of taste I'll leave for Daniel to
call the shots on.

One reason for my dislike is how easy it will be to accidentally get the
val and mask parameters mixed up. I would instinctively put the mask
before the value (common convention of context before the rest), which
would be wrong here. I am not saying changing the order would make me
like this.


As mentioned in another mail, this is not required and I will remove it




+
+
  /* "Broadcast RGB" property */
  #define INTEL_BROADCAST_RGB_AUTO 0
  #define INTEL_BROADCAST_RGB_FULL 1
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 8dc9a38..9e67f78 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -101,46 +101,59 @@ static void intel_dsi_pre_pll_enable(struct intel_encoder 
*encoder)
vlv_enable_dsi_pll(encoder);
  }

-static void intel_dsi_pre_enable(struct intel_encoder *encoder)
-{
-   DRM_DEBUG_KMS("\n");
-}
-
-static void intel_dsi_enable(struct intel_encoder *encoder)
+void intel_dsi_device_ready(struct intel_encoder *encoder)
  {
struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
int pipe = intel_crtc->pipe;
-   u32 temp;

DRM_DEBUG_KMS("\n");

if (intel_dsi->dev.dev_ops->panel_reset)
intel_dsi->dev.dev_ops->panel_reset(&intel_dsi->dev);

-   temp = I915_READ(MIPI_DEVICE_READY(pipe));
-   if ((temp & DEVICE_READY) == 0) {
-   temp &= ~ULPS_STATE_MASK;
-   I915_WRITE(MIPI_DEVICE_READY(pipe), temp | DEVICE_READY);
-   } else if (temp & ULPS_STATE_MASK) {
-   temp &= ~ULPS_STATE_MASK;
-   I915_WRITE(MIPI_DEVICE_READY(pipe), temp | ULPS_STATE_EXIT);
-   /*
-* We need to ensure that there is a minimum of 1 ms time
-* available before clearing the UPLS exit state.
-*/
-   msleep(2);
-   I915_WRITE(MIPI_DEVICE_READY(pipe), temp);
-   }
+   I915_WRITE_BITS(MIPI_PORT_CTRL(pipe), LP_OUTPUT_HOLD, LP_OUTPUT_HOLD);
+   usleep_range(1000, 1500);
+   I915_WRITE_BITS(MIPI_DEVICE_READY(pipe), DEVICE_READY |
+   ULPS_STATE_EXIT, DEVICE_READY | ULPS_STATE_MASK);
+   usleep_range(2000, 2500);
+   I915_WRITE_BITS(MIPI_DEVICE_READY(pipe), DEVICE_READY,
+   DEVICE_READY | ULPS_STATE_MASK);
+   usleep_range(2000, 2500);
+   I915_WRITE_BITS(MIPI_DEVICE_READY(pipe), 0x00,
+   DEVICE_READY | ULPS_STATE_MASK);
+   usleep_range(2000, 2500);
+   I915_WRITE_BITS(MIPI_DEVICE_READY(pipe), DEVICE_READY,
+   DEVICE_READY | ULPS_STATE_MASK);


It seems like an odd dance, but if that's what the hw folks say, I guess
we'll just have to take their word for it...


Yeah, but I reconfirmed from HW team and this is also now updated in 
documents





+   usleep_range(2000, 2500);

if (intel_dsi->dev.dev_ops->send_otp_cmds)
   

Re: [Intel-gfx] [PATCH v2 5/7] drm/i915: Reorganize the DSI enable/disable sequence

2013-12-06 Thread Shobhit Kumar

On Wednesday 20 November 2013 07:09 AM, Shobhit Kumar wrote:

On Friday 15 November 2013 02:25 PM, Daniel Vetter wrote:

On Fri, Nov 15, 2013 at 10:27:25AM +0200, Jani Nikula wrote:

On Sat, 09 Nov 2013, Shobhit Kumar  wrote:

Basically ULPS handling during enable/disable has been moved to
pre_enable and post_disable phases. PLL and panel power disable
also has been moved to post_disable phase. The ULPS entry/exit
sequneces as suggested by HW team is as follows -

During enable time -
set DEVICE_READY --> Clear DEVICE_READY --> set DEVICE_READY

And during disable time to flush all FIFOs -
set ENTER_SLEEP --> EXIT_SLEEP --> ENTER_SLEEP

Also during disbale sequnece sub-encoder disable is moved to the end
after port is disabled.

v2: Based on comments from Ville
 - Detailed epxlaination in the commit messgae
 - Moved parameter changes out into another patch
 - Backlight enabling will be a new patch

Signed-off-by: Yogesh Mohan Marimuthu

Signed-off-by: Shobhit Kumar 
---
  drivers/gpu/drm/i915/i915_drv.h  |   11 
  drivers/gpu/drm/i915/intel_dsi.c |  111
++
  drivers/gpu/drm/i915/intel_dsi.h |2 +
  3 files changed, 91 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h
b/drivers/gpu/drm/i915/i915_drv.h
index a2bbff9..55c16cb 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2433,6 +2433,17 @@ int vlv_freq_opcode(int ddr_freq, int val);
  #define POSTING_READ(reg)(void)I915_READ_NOTRACE(reg)
  #define POSTING_READ16(reg)(void)I915_READ16_NOTRACE(reg)

+#define I915_WRITE_BITS(reg, val, mask) \
+do { \
+u32 tmp, data; \
+tmp = I915_READ((reg)); \
+tmp &= ~(mask); \
+data = (val) & (mask); \
+data = data | tmp; \
+I915_WRITE((reg), data); \
+} while(0)


I would still prefer the explicit read, modify, and write in the code
instead of this, but it's a matter of taste I'll leave for Daniel to
call the shots on.


Yeah, this looks a bit funny. We could compute the tmp value once (where
the mask is mutliple times the same thing) and then just or in the right
bits.  That should make the I915_WRITE calls fit ont on line, too, which
helps readability.

Also we put POSTING_READs before any waits to ensure the write has
actually landed. It's mostly documentation.

And while I'm at it: We generally frown upon readbacks of register value
and prefer to just keep track of things in software well enough. The
reason for that is that register readbacks allows us too much flexibility
in adding subtile state-depencies. Which long-term makes the code a real
pain to maintain.


Ok. Will work on updating the patch accordingly and take care of other
comments as well in next patch set update soon.


Sorry took me more time than I anticipated to rework on this due to 
other critical stuff. Looking at the code and doing more testing I now 
confirmed that there is no READ/Modify/WRITE needed for ULPS and hence I 
will convert I915_WRITE_BITS to normal I915_WRITE. Will be sending 
updated patches on Monday


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


Re: [Intel-gfx] [PATCH] drm/i915: Introduce vblank work function

2013-12-06 Thread Bloomfield, Jon
What's the status of this patch ? I can't find any subsequent mention of it, 
but we currently use it in one of our Android development trees. I'm trying to 
work out whether to retain it or replace it.

Was it killed off, or is it still in the pipeline ?

Thanks.

> -Original Message-
> From: intel-gfx-bounces+jon.bloomfield=intel@lists.freedesktop.org
> [mailto:intel-gfx-bounces+jon.bloomfield=intel@lists.freedesktop.org]
> On Behalf Of Chris Wilson
> Sent: Friday, July 05, 2013 9:49 AM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH] drm/i915: Introduce vblank work function
> 
> Throughout the driver, we have a number of pieces of code that must wait
> for a vblank before we can update some state. Often these could be run
> asynchronously since they are merely freeing a resource no long referenced
> by a double-buffered registered. So we introduce a vblank worker upon
> which we queue various tasks to be run after the next vvblank.
> 
> This will be used in the next patches to avoid unnecessary stalls when
> updating registers and for freeing resources.
> 
> Signed-off-by: Chris Wilson 
> ---
>  drivers/gpu/drm/i915/intel_display.c | 79
> 
>  drivers/gpu/drm/i915/intel_drv.h |  8 +++-
>  2 files changed, 86 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index b70ce4e..dff3b93 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -795,6 +795,74 @@ void intel_wait_for_vblank(struct drm_device *dev,
> int pipe)
>   DRM_DEBUG_KMS("vblank wait timed out\n");  }
> 
> +struct intel_crtc_vblank_task {
> + struct list_head list;
> + void (*func)(struct intel_crtc *, void *data);
> + void *data;
> +};
> +
> +static void intel_crtc_vblank_work_fn(struct work_struct *_work) {
> + struct intel_crtc_vblank_work *work =
> + container_of(_work, struct intel_crtc_vblank_work, work);
> + struct intel_crtc *crtc =
> + container_of(work, struct intel_crtc, vblank_work);
> + struct list_head tasks;
> +
> + intel_wait_for_vblank(crtc->base.dev, crtc->pipe);
> +
> + mutex_lock(&crtc->vblank_work.mutex);
> + list_replace_init(&work->tasks, &tasks);
> + mutex_unlock(&crtc->vblank_work.mutex);
> +
> + while (!list_empty(&tasks)) {
> + struct intel_crtc_vblank_task *task
> + = list_first_entry(&tasks,
> +struct intel_crtc_vblank_task,
> +list);
> +
> + task->func(crtc, task->data);
> + list_del(&task->list);
> + kfree(task);
> + }
> +}
> +
> +static int intel_crtc_add_vblank_task(struct intel_crtc *crtc,
> +   bool single,
> +   void (*func)(struct intel_crtc *,
> +void *data),
> +   void *data)
> +{
> + struct intel_crtc_vblank_task *task;
> + struct intel_crtc_vblank_work *work = &crtc->vblank_work;
> +
> + task = kzalloc(sizeof *task, GFP_KERNEL);
> + if (task == NULL)
> + return -ENOMEM;
> + task->func = func;
> + task->data = data;
> +
> + mutex_lock(&work->mutex);
> + if (list_empty(&work->tasks)) {
> + schedule_work(&work->work);
> + } else if (single) {
> + struct intel_crtc_vblank_task *old;
> + list_for_each_entry(old, &work->tasks, list) {
> + if (task->func == func && task->data == data) {
> + func = NULL;
> + break;
> + }
> + }
> + }
> + if (func)
> + list_add(&task->list, &work->tasks);
> + else
> + kfree(task);
> + mutex_unlock(&work->mutex);
> +
> + return 0;
> +}
> +
>  /*
>   * intel_wait_for_pipe_off - wait for pipe to turn off
>   * @dev: drm device
> @@ -2835,6 +2903,8 @@ static void intel_crtc_wait_for_pending_flips(struct
> drm_crtc *crtc)
>   if (crtc->fb == NULL)
>   return;
> 
> + flush_work(&to_intel_crtc(crtc)->vblank_work.work);
> +
>   WARN_ON(waitqueue_active(&dev_priv->pending_flip_queue));
> 
>   wait_event(dev_priv->pending_flip_queue,
> @@ -9097,6 +9167,10 @@ static void intel_crtc_init(struct drm_device *dev,
> int pipe)
>   if (intel_crtc == NULL)
>   return;
> 
> + mutex_init(&intel_crtc->vblank_work.mutex);
> + INIT_LIST_HEAD(&intel_crtc->vblank_work.tasks);
> + INIT_WORK(&intel_crtc->vblank_work.work,
> intel_crtc_vblank_work_fn);
> +
>   drm_crtc_init(dev, &intel_crtc->base, &intel_crtc_funcs);
> 
>   if (drm_mode_crtc_set_gamma_size(&intel_crtc->base, 256)) @@ -
> 10296,11 +10370,16 @@ void intel_modeset_cleanup(struct drm_de

Re: [Intel-gfx] [Intel gfx][i-g-t PATCH 2/4] rendercopy/bdw: Set Instruction Buffer size Modify Enable to 1

2013-12-06 Thread Kenneth Graunke
On 12/06/2013 12:54 AM, Xiang, Haihao wrote:
> From: "Xiang, Haihao" 
> 
> Otherwise it may result in GPU hang
> 
> Signed-off-by: Xiang, Haihao 
> ---
>  lib/rendercopy_gen8.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/rendercopy_gen8.c b/lib/rendercopy_gen8.c
> index 43e962c..1a137dd 100644
> --- a/lib/rendercopy_gen8.c
> +++ b/lib/rendercopy_gen8.c
> @@ -526,7 +526,7 @@ gen8_emit_state_base_address(struct intel_batchbuffer 
> *batch) {
>   /* indirect object buffer size */
>   OUT_BATCH(0xf000 | 1);
>   /* intruction buffer size */
> - OUT_BATCH(1 << 12);
> + OUT_BATCH(1 << 12 | 1);
>  }
>  
>  static void
> 

Thanks a ton for finding this!

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


Re: [Intel-gfx] [Intel gfx][i-g-t PATCH 1/4] lib: Clean the batch buffer store after reset

2013-12-06 Thread Kenneth Graunke
On 12/06/2013 12:54 AM, Xiang, Haihao wrote:
> From: "Xiang, Haihao" 
> 
> Otherwise the stale data in the buffer
> 
> Signed-off-by: Xiang, Haihao 
> ---
>  lib/intel_batchbuffer.c |2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/lib/intel_batchbuffer.c b/lib/intel_batchbuffer.c
> index 06a5437..9ce7424 100644
> --- a/lib/intel_batchbuffer.c
> +++ b/lib/intel_batchbuffer.c
> @@ -50,6 +50,8 @@ intel_batchbuffer_reset(struct intel_batchbuffer *batch)
>   batch->bo = drm_intel_bo_alloc(batch->bufmgr, "batchbuffer",
>  BATCH_SZ, 4096);
>  
> + memset(batch->buffer, 0, sizeof(batch->buffer));
> +
>   batch->ptr = batch->buffer;
>  }
>  
> 

I don't think that should be harmful, but this would definitely make
debugging nicer.  For intel-gpu-tools, I think it makes sense.

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


Re: [Intel-gfx] [PATCH] drm/i915: Fix use-after-free in do_switch

2013-12-06 Thread Lister, Ian


> -Original Message-
> From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch]
> Sent: Thursday, December 05, 2013 2:43 PM
> To: Intel Graphics Development
> Cc: Daniel Vetter; Lister, Ian; sta...@vger.kernel.org; Widawsky, Benjamin;
> Stéphane Marchesin; Bloomfield, Jon
> Subject: [PATCH] drm/i915: Fix use-after-free in do_switch
> 
> So apparently under ridiculous amounts of memory pressure we can get into
> trouble in do_switch when we try to move the old hw context backing
> storage object onto the active lists.
> 
> With list debugging enabled that usually results in us chasing a poisoned
> pointer - which means we've hit upon a vma that has been removed from all
> lrus with list_del (and then deallocated, so it's a real use-after free).
> 
> Ian Lister has done some great callchain chasing and noticed that we can
> reenter do_switch:
> 
> i915_gem_do_execbuffer()
> 
> i915_switch_context()
> 
> do_switch()
>from = ring->last_context;
>i915_gem_object_pin()
> 
>   i915_gem_object_bind_to_gtt()
>  ret = drm_mm_insert_node_in_range_generic();
>  // If the above call fails then it will try 
> i915_gem_evict_something()
>  // If that fails it will call i915_gem_evict_everything() ...
>i915_gem_evict_everything()
>   i915_gpu_idle()
>  i915_switch_context(DEFAULT_CONTEXT)
> 
> Like with everything else where the shrinker or eviction code can invalidate
> pointers we need to reload relevant state.
> 
> Note that there's no need to recheck whether a context switch is still
> required because:
> 
> - Doing a switch to the same context is harmless (besides wasting a
>   bit of energy).
> 
> - This can only happen with the default context. But since that one's
>   pinned we'll never call down into evict_everything under normal
>   circumstances. Note that there's a little driver bringup fun
>   involved namely that we could recourse into do_switch for the
>   initial switch. Atm we're fine since we assign the context pointer
>   only after the call to do_switch at driver load or resume time. And
>   in the gpu reset case we skip the entire setup sequence (which might
>   be a bug on its own, but definitely not this one here).
> 
> Cc'ing stable since apparently ChromeOS guys are seeing this in the wild (and
> not just on artificial stress tests), see the reference.
> 
> Note that in upstream code doesn't calle evict_everything directly from
> evict_something, that's an extension in this product branch. But we can still
> hit upon this bug (and apparently we do, see the linked backtraces). I've
> noticed this while trying to construct a testcase for this bug and utterly 
> failed
> to provoke it. It looks like we need to driver the system squarly into the
> lowmem wall and provoke the shrinker to evict the context object by doing
> the last-ditch evict_everything call.
> 
> Aside: There's currently no means to get a badly-fragmenting hw context
> object away from a bad spot in the upstream code. We should fix this by at
> least adding some code to evict_something to handle hw contexts.
> 
> References: https://code.google.com/p/chromium/issues/detail?id=248191
> Reported-by: Ian Lister 
> Cc: Ian Lister 
> Cc: sta...@vger.kernel.org
> Cc: Ben Widawsky 
> Cc: Stéphane Marchesin 
> Cc: Bloomfield, Jon 
> Signed-off-by: Daniel Vetter 

Reviewed-by: Ian Lister 

> ---
>  drivers/gpu/drm/i915/i915_gem_context.c | 14 --
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
> b/drivers/gpu/drm/i915/i915_gem_context.c
> index 41877045a1a0..2d2877493f61 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -421,11 +421,21 @@ static int do_switch(struct i915_hw_context *to)
>   if (ret)
>   return ret;
> 
> - /* Clear this page out of any CPU caches for coherent swap-in/out.
> Note
> + /*
> +  * Pin can switch back to the default context if we end up calling into
> +  * evict_everything - as a last ditch gtt defrag effort that also
> +  * switches to the default context. Hence we need to reload from
> here.
> +  */
> + from = ring->last_context;
> +
> + /*
> +  * Clear this page out of any CPU caches for coherent swap-in/out.
> +Note
>* that thanks to write = false in this call and us not setting any gpu
>* write domains when putting a context object onto the active list
>* (when switching away from it), this won't block.
> -  * XXX: We need a real interface to do this instead of trickery. */
> +  *
> +  * XXX: We need a real interface to do this instead of trickery.
> +  */
>   ret = i915_gem_object_set_to_gtt_domain(to->obj, false);
>   if (ret) {
>   i915_gem_object_unpin(to->obj);
> --
> 1.8.4.3

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedeskt

[Intel-gfx] [PATCH 3/3] lib: make igt_pipe_crc_start never fail

2013-12-06 Thread Daniel Vetter
It's what callers expect - pipe_crc_new is the function where
we pass a potential failure back to callers.

Signed-off-by: Daniel Vetter 
---
 lib/igt_debugfs.c  | 7 ++-
 lib/igt_debugfs.h  | 2 +-
 tests/kms_pipe_crc_basic.c | 2 +-
 3 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
index 139be893f75b..4b96521331af 100644
--- a/lib/igt_debugfs.c
+++ b/lib/igt_debugfs.c
@@ -269,12 +269,11 @@ void igt_pipe_crc_free(igt_pipe_crc_t *pipe_crc)
free(pipe_crc);
 }
 
-bool igt_pipe_crc_start(igt_pipe_crc_t *pipe_crc)
+void igt_pipe_crc_start(igt_pipe_crc_t *pipe_crc)
 {
igt_crc_t *crcs = NULL;
 
-   if (!igt_pipe_crc_do_start(pipe_crc))
-   return false;
+   igt_assert(igt_pipe_crc_do_start(pipe_crc));
 
/*
 * For some no yet identified reason, the first CRC is bonkers. So
@@ -282,8 +281,6 @@ bool igt_pipe_crc_start(igt_pipe_crc_t *pipe_crc)
 */
igt_pipe_crc_get_crcs(pipe_crc, 1, &crcs);
free(crcs);
-
-   return true;
 }
 
 void igt_pipe_crc_stop(igt_pipe_crc_t *pipe_crc)
diff --git a/lib/igt_debugfs.h b/lib/igt_debugfs.h
index 393b5767adbe..075e44625213 100644
--- a/lib/igt_debugfs.h
+++ b/lib/igt_debugfs.h
@@ -75,7 +75,7 @@ igt_pipe_crc_t *
 igt_pipe_crc_new(igt_debugfs_t *debugfs, int drm_fd, enum pipe pipe,
 enum intel_pipe_crc_source source);
 void igt_pipe_crc_free(igt_pipe_crc_t *pipe_crc);
-bool igt_pipe_crc_start(igt_pipe_crc_t *pipe_crc);
+void igt_pipe_crc_start(igt_pipe_crc_t *pipe_crc);
 void igt_pipe_crc_stop(igt_pipe_crc_t *pipe_crc);
 void igt_pipe_crc_get_crcs(igt_pipe_crc_t *pipe_crc, int n_crcs,
   igt_crc_t **out_crcs);
diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c
index 90d9b9404877..3fc59344d90d 100644
--- a/tests/kms_pipe_crc_basic.c
+++ b/tests/kms_pipe_crc_basic.c
@@ -180,7 +180,7 @@ static void test_read_crc(data_t *data, int pipe, unsigned 
flags)
continue;
valid_connectors++;
 
-   igt_assert(igt_pipe_crc_start(pipe_crc));
+   igt_pipe_crc_start(pipe_crc);
 
/* wait for 3 vblanks and the corresponding 3 CRCs */
igt_pipe_crc_get_crcs(pipe_crc, 3, &crcs);
-- 
1.8.4.3

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


[Intel-gfx] [PATCH 2/3] lib: add igt_pipe_crc_check

2013-12-06 Thread Daniel Vetter
No need to duplicate this all over the place.

Signed-off-by: Daniel Vetter 
---
 lib/igt_debugfs.c  | 18 ++
 lib/igt_debugfs.h  |  1 +
 tests/kms_cursor_crc.c | 18 ++
 tests/kms_fbc_crc.c| 14 +-
 tests/kms_pipe_crc_basic.c | 25 -
 5 files changed, 30 insertions(+), 46 deletions(-)

diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
index 1ceaf59db11f..139be893f75b 100644
--- a/lib/igt_debugfs.c
+++ b/lib/igt_debugfs.c
@@ -205,6 +205,24 @@ static void pipe_crc_exit_handler(int sig)
igt_pipe_crc_reset();
 }
 
+void igt_pipe_crc_check(igt_debugfs_t *debugfs)
+{
+   const char *cmd = "pipe A none";
+   FILE *ctl;
+   size_t written;
+   int ret;
+
+   ctl = igt_debugfs_fopen(debugfs, "i915_display_crc_ctl", "r+");
+   igt_require_f(ctl,
+ "No display_crc_ctl found, kernel too old\n");
+   written = fwrite(cmd, 1, strlen(cmd), ctl);
+   ret = fflush(ctl);
+   igt_require_f((written == strlen(cmd) && ret == 0) || errno != ENODEV,
+ "CRCs not supported on this platform\n");
+
+   fclose(ctl);
+}
+
 igt_pipe_crc_t *
 igt_pipe_crc_new(igt_debugfs_t *debugfs, int drm_fd, enum pipe pipe,
 enum intel_pipe_crc_source source)
diff --git a/lib/igt_debugfs.h b/lib/igt_debugfs.h
index 40d9d28fd49b..393b5767adbe 100644
--- a/lib/igt_debugfs.h
+++ b/lib/igt_debugfs.h
@@ -70,6 +70,7 @@ bool igt_crc_is_null(igt_crc_t *crc);
 bool igt_crc_equal(igt_crc_t *a, igt_crc_t *b);
 char *igt_crc_to_string(igt_crc_t *crc);
 
+void igt_pipe_crc_check(igt_debugfs_t *debugfs);
 igt_pipe_crc_t *
 igt_pipe_crc_new(igt_debugfs_t *debugfs, int drm_fd, enum pipe pipe,
 enum intel_pipe_crc_source source);
diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
index b78ea7863585..d80695f67e2f 100644
--- a/tests/kms_cursor_crc.c
+++ b/tests/kms_cursor_crc.c
@@ -53,7 +53,6 @@ typedef struct {
int drm_fd;
igt_debugfs_t debugfs;
drmModeRes *resources;
-   FILE *ctl;
uint32_t fb_id[NUM_CURSOR_TYPES];
struct kmstest_fb fb[NUM_CURSOR_TYPES];
igt_pipe_crc_t **pipe_crc;
@@ -333,23 +332,12 @@ igt_main
igt_skip_on_simulation();
 
igt_fixture {
-   size_t written;
-   int ret;
-   const char *cmd = "pipe A none";
-
data.drm_fd = drm_open_any();
 
igt_set_vt_graphics_mode();
 
igt_debugfs_init(&data.debugfs);
-   data.ctl = igt_debugfs_fopen(&data.debugfs,
-"i915_display_crc_ctl", "r+");
-   igt_require_f(data.ctl,
- "No display_crc_ctl found, kernel too old\n");
-   written = fwrite(cmd, 1, strlen(cmd), data.ctl);
-   ret = fflush(data.ctl);
-   igt_require_f((written == strlen(cmd) && ret == 0) || errno != 
ENODEV,
- "CRCs not supported on this platform\n");
+   igt_pipe_crc_check(&data.debugfs);
 
display_init(&data);
 
@@ -376,8 +364,6 @@ igt_main
igt_subtest("cursor-black-invisible-offscreen")
run_test(&data, BLACK_INVISIBLE, false);
 
-   igt_fixture {
+   igt_fixture
display_fini(&data);
-   fclose(data.ctl);
-   }
 }
diff --git a/tests/kms_fbc_crc.c b/tests/kms_fbc_crc.c
index 7a7f3903667b..4cddd27428d0 100644
--- a/tests/kms_fbc_crc.c
+++ b/tests/kms_fbc_crc.c
@@ -60,7 +60,6 @@ typedef struct {
int drm_fd;
igt_debugfs_t debugfs;
drmModeRes *resources;
-   FILE *ctl;
igt_crc_t ref_crc[2];
igt_pipe_crc_t **pipe_crc;
drm_intel_bufmgr *bufmgr;
@@ -485,9 +484,6 @@ igt_main
igt_skip_on_simulation();
 
igt_fixture {
-   size_t written;
-   int ret;
-   const char *cmd = "pipe A none";
char buf[64];
FILE *status;
 
@@ -497,14 +493,7 @@ igt_main
data.devid = intel_get_drm_devid(data.drm_fd);
 
igt_debugfs_init(&data.debugfs);
-   data.ctl = igt_debugfs_fopen(&data.debugfs,
-"i915_display_crc_ctl", "r+");
-   igt_require_f(data.ctl,
- "No display_crc_ctl found, kernel too old\n");
-   written = fwrite(cmd, 1, strlen(cmd), data.ctl);
-   ret = fflush(data.ctl);
-   igt_require_f((written == strlen(cmd) && ret == 0) || errno != 
ENODEV,
- "CRCs not supported on this platform\n");
+   igt_pipe_crc_check(&data.debugfs);
 
status = igt_debugfs_fopen(&data.debugfs, "i915_fbc_status", 
"r");
igt_require_f(status, "No i915_fbc_status found\n");
@@ -532,6 +521,5 @@ igt_main
igt_fix

[Intel-gfx] [PATCH 1/3] tests: drm_open_any doesn't fail

2013-12-06 Thread Daniel Vetter
Or more precisely: It already has an igt_require. So we cant ditch it
from tests.

Signed-off-by: Daniel Vetter 
---
 tests/kms_cursor_crc.c | 1 -
 tests/kms_fbc_crc.c| 1 -
 tests/kms_pipe_crc_basic.c | 1 -
 3 files changed, 3 deletions(-)

diff --git a/tests/kms_cursor_crc.c b/tests/kms_cursor_crc.c
index 74da32eb7497..b78ea7863585 100644
--- a/tests/kms_cursor_crc.c
+++ b/tests/kms_cursor_crc.c
@@ -338,7 +338,6 @@ igt_main
const char *cmd = "pipe A none";
 
data.drm_fd = drm_open_any();
-   igt_require(data.drm_fd >= 0);
 
igt_set_vt_graphics_mode();
 
diff --git a/tests/kms_fbc_crc.c b/tests/kms_fbc_crc.c
index 660086290f28..7a7f3903667b 100644
--- a/tests/kms_fbc_crc.c
+++ b/tests/kms_fbc_crc.c
@@ -492,7 +492,6 @@ igt_main
FILE *status;
 
data.drm_fd = drm_open_any();
-   igt_require(data.drm_fd);
igt_set_vt_graphics_mode();
 
data.devid = intel_get_drm_devid(data.drm_fd);
diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c
index 3bc9eb0ee361..0e793cdf617d 100644
--- a/tests/kms_pipe_crc_basic.c
+++ b/tests/kms_pipe_crc_basic.c
@@ -219,7 +219,6 @@ igt_main
const char *cmd = "pipe A none";
 
data.drm_fd = drm_open_any();
-   igt_require(data.drm_fd >= 0);
 
igt_set_vt_graphics_mode();
 
-- 
1.8.4.3

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


[Intel-gfx] [PATCH] drm/i915: fix pm init ordering

2013-12-06 Thread Daniel Vetter
Shovel a bit more of the the code into the setup function, and call
it earlier. Otherwise lockdep is unhappy since we cancel the delayed
resume work before it's initialized.

While at it also shovel the pc8 setup code into the same functions.
I wanted to also ditch the header declaration of the hws pc8 functions,
but for unfathomable reasons that stuff is in intel_display.c instead
of intel_pm.c.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=71980
Tested-by: Guo Jinxian 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/i915/i915_dma.c  | 10 +-
 drivers/gpu/drm/i915/i915_drv.h  |  2 --
 drivers/gpu/drm/i915/intel_drv.h |  1 +
 drivers/gpu/drm/i915/intel_pm.c  | 11 ++-
 4 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 0cab2d045135..adec2e41aa6e 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1490,16 +1490,9 @@ int i915_driver_load(struct drm_device *dev, unsigned 
long flags)
spin_lock_init(&dev_priv->uncore.lock);
spin_lock_init(&dev_priv->mm.object_stat_lock);
mutex_init(&dev_priv->dpio_lock);
-   mutex_init(&dev_priv->rps.hw_lock);
mutex_init(&dev_priv->modeset_restore_lock);
 
-   mutex_init(&dev_priv->pc8.lock);
-   dev_priv->pc8.requirements_met = false;
-   dev_priv->pc8.gpu_idle = false;
-   dev_priv->pc8.irqs_disabled = false;
-   dev_priv->pc8.enabled = false;
-   dev_priv->pc8.disable_count = 2; /* requirements_met + gpu_idle */
-   INIT_DELAYED_WORK(&dev_priv->pc8.enable_work, hsw_enable_pc8_work);
+   intel_pm_setup(dev);
 
intel_display_crc_init(dev);
 
@@ -1603,7 +1596,6 @@ int i915_driver_load(struct drm_device *dev, unsigned 
long flags)
}
 
intel_irq_init(dev);
-   intel_pm_init(dev);
intel_uncore_sanitize(dev);
 
/* Try to make sure MCHBAR is enabled before poking at it */
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ccdbecca070d..6f04fa4b31fd 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1901,9 +1901,7 @@ void i915_queue_hangcheck(struct drm_device *dev);
 void i915_handle_error(struct drm_device *dev, bool wedged);
 
 extern void intel_irq_init(struct drm_device *dev);
-extern void intel_pm_init(struct drm_device *dev);
 extern void intel_hpd_init(struct drm_device *dev);
-extern void intel_pm_init(struct drm_device *dev);
 
 extern void intel_uncore_sanitize(struct drm_device *dev);
 extern void intel_uncore_early_sanitize(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a18e88b3e425..79f91f26e288 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -821,6 +821,7 @@ void intel_update_sprite_watermarks(struct drm_plane *plane,
uint32_t sprite_width, int pixel_size,
bool enabled, bool scaled);
 void intel_init_pm(struct drm_device *dev);
+void intel_pm_setup(struct drm_device *dev);
 bool intel_fbc_enabled(struct drm_device *dev);
 void intel_update_fbc(struct drm_device *dev);
 void intel_gpu_ips_init(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 6e0d5e075b15..e0dec95c764e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6130,10 +6130,19 @@ int vlv_freq_opcode(int ddr_freq, int val)
return val;
 }
 
-void intel_pm_init(struct drm_device *dev)
+void intel_pm_setup(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = dev->dev_private;
 
+   mutex_init(&dev_priv->rps.hw_lock);
+
+   mutex_init(&dev_priv->pc8.lock);
+   dev_priv->pc8.requirements_met = false;
+   dev_priv->pc8.gpu_idle = false;
+   dev_priv->pc8.irqs_disabled = false;
+   dev_priv->pc8.enabled = false;
+   dev_priv->pc8.disable_count = 2; /* requirements_met + gpu_idle */
+   INIT_DELAYED_WORK(&dev_priv->pc8.enable_work, hsw_enable_pc8_work);
INIT_DELAYED_WORK(&dev_priv->rps.delayed_resume_work,
  intel_gen6_powersave_work);
 }
-- 
1.8.4.3

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


[Intel-gfx] [Intel gfx][i-g-t PATCH 4/4] Revert "gen8 rendercpy: temporarily disable"

2013-12-06 Thread Xiang, Haihao
From: "Xiang, Haihao" 

This reverts commit e41928e6c9bb3f24833a827903f1afeda83592d6.

Now the case no longer causes GPU hang on GEN
---
 lib/rendercopy_i830.c |6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/lib/rendercopy_i830.c b/lib/rendercopy_i830.c
index 5dd67b2..73edcfa 100644
--- a/lib/rendercopy_i830.c
+++ b/lib/rendercopy_i830.c
@@ -241,10 +241,8 @@ render_copyfunc_t get_render_copyfunc(int devid)
copy = gen6_render_copyfunc;
else if (IS_GEN7(devid))
copy = gen7_render_copyfunc;
-   else if (IS_GEN8(devid)) {
-   fprintf(stderr, "Temporarily disabled\n");
-   //copy = gen8_render_copyfunc;
-   }
+   else if (IS_GEN8(devid))
+   copy = gen8_render_copyfunc;
 
return copy;
 }
-- 
1.7.9.5

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


[Intel-gfx] [Intel gfx][i-g-t PATCH 1/4] lib: Clean the batch buffer store after reset

2013-12-06 Thread Xiang, Haihao
From: "Xiang, Haihao" 

Otherwise the stale data in the buffer

Signed-off-by: Xiang, Haihao 
---
 lib/intel_batchbuffer.c |2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/intel_batchbuffer.c b/lib/intel_batchbuffer.c
index 06a5437..9ce7424 100644
--- a/lib/intel_batchbuffer.c
+++ b/lib/intel_batchbuffer.c
@@ -50,6 +50,8 @@ intel_batchbuffer_reset(struct intel_batchbuffer *batch)
batch->bo = drm_intel_bo_alloc(batch->bufmgr, "batchbuffer",
   BATCH_SZ, 4096);
 
+   memset(batch->buffer, 0, sizeof(batch->buffer));
+
batch->ptr = batch->buffer;
 }
 
-- 
1.7.9.5

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


[Intel-gfx] [Intel gfx][i-g-t PATCH 2/4] rendercopy/bdw: Set Instruction Buffer size Modify Enable to 1

2013-12-06 Thread Xiang, Haihao
From: "Xiang, Haihao" 

Otherwise it may result in GPU hang

Signed-off-by: Xiang, Haihao 
---
 lib/rendercopy_gen8.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/rendercopy_gen8.c b/lib/rendercopy_gen8.c
index 43e962c..1a137dd 100644
--- a/lib/rendercopy_gen8.c
+++ b/lib/rendercopy_gen8.c
@@ -526,7 +526,7 @@ gen8_emit_state_base_address(struct intel_batchbuffer 
*batch) {
/* indirect object buffer size */
OUT_BATCH(0xf000 | 1);
/* intruction buffer size */
-   OUT_BATCH(1 << 12);
+   OUT_BATCH(1 << 12 | 1);
 }
 
 static void
-- 
1.7.9.5

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


[Intel-gfx] [Intel gfx][i-g-t PATCH 3/4] rendercopy/bdw: A workaround for 3D pipeline

2013-12-06 Thread Xiang, Haihao
From: "Xiang, Haihao" 

Emit PIPELINE_SELECT twice and make sure the commands in
the first batch buffer have been done.

However I don't know why this works !!!

Signed-off-by: Xiang, Haihao 
---
 lib/rendercopy_gen8.c |   19 +--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/lib/rendercopy_gen8.c b/lib/rendercopy_gen8.c
index 1a137dd..6eb1051 100644
--- a/lib/rendercopy_gen8.c
+++ b/lib/rendercopy_gen8.c
@@ -148,7 +148,8 @@ batch_copy(struct intel_batchbuffer *batch, const void 
*ptr, uint32_t size, uint
 
 static void
 gen6_render_flush(struct intel_batchbuffer *batch,
- drm_intel_context *context, uint32_t batch_end)
+   drm_intel_context *context, uint32_t batch_end,
+   int waiting)
 {
int ret;
 
@@ -157,6 +158,11 @@ gen6_render_flush(struct intel_batchbuffer *batch,
ret = drm_intel_gem_bo_context_exec(batch->bo, context,
batch_end, 0);
assert(ret == 0);
+
+   if (waiting) {
+   dri_bo_map(batch->bo, 0);
+   dri_bo_unmap(batch->bo);
+   }
 }
 
 /* Mostly copy+paste from gen6, except height, width, pitch moved */
@@ -880,6 +886,15 @@ void gen8_render_copyfunc(struct intel_batchbuffer *batch,
 
intel_batchbuffer_flush_with_context(batch, context);
 
+   /* I don't know why it works !!! */
+   batch->ptr = batch->buffer;
+   OUT_BATCH(GEN6_PIPELINE_SELECT | PIPELINE_SELECT_3D);
+   OUT_BATCH(MI_BATCH_BUFFER_END);
+   batch_end = batch_align(batch, 8);
+   assert(batch_end < BATCH_STATE_SPLIT);
+   gen6_render_flush(batch, context, batch_end, 1);
+   intel_batchbuffer_reset(batch);
+
batch_align(batch, 8);
 
batch->ptr = &batch->buffer[BATCH_STATE_SPLIT];
@@ -968,6 +983,6 @@ void gen8_render_copyfunc(struct intel_batchbuffer *batch,
 
annotation_flush(&aub_annotations, batch);
 
-   gen6_render_flush(batch, context, batch_end);
+   gen6_render_flush(batch, context, batch_end, 0);
intel_batchbuffer_reset(batch);
 }
-- 
1.7.9.5

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


[Intel-gfx] [PATCH v3 4/6] ACPI/i915: Fix wrong inclusion in i915 opregion module.

2013-12-06 Thread Lv Zheng
In Linux kernel, ACPICA is wrapped and safely exported by CONFIG_ACPI.  So
all external modules should depend on CONFIG_ACPI rather than using ACPICA
header directly for stubbing.  But if we moves  inclusions
into "#ifdef CONFIG_ACPI", build breakge can help to detect wrong ACPICA
dependent modules.

One of the build breakage is:
include/linux/acpi_io.h:7:45: error: unknown type name 'acpi_physical_address'
include/linux/acpi_io.h:8:10: error: unknown type name 'acpi_size'
include/linux/acpi_io.h:13:33: error: unknown type name 'acpi_physical_address'
include/linux/acpi_io.h:15:40: warning: 'struct acpi_generic_address' declared 
inside parameter list [enabled by default]
include/linux/acpi_io.h:15:40: warning: its scope is only this definition or 
declaration, which is probably not what you want [enabled by default]
include/linux/acpi_io.h:16:43: warning: 'struct acpi_generic_address' declared 
inside parameter list [enabled by default]
drivers/gpu/drm/i915/intel_opregion.c: In function 'intel_opregion_setup':
drivers/gpu/drm/i915/intel_opregion.c:883:2: error: implicit declaration of 
function 'acpi_os_ioremap' [-Werror=implicit-function-declaration]
drivers/gpu/drm/i915/intel_opregion.c:883:7: warning: assignment makes pointer 
from integer without a cast [enabled by default]

The root causes of this breakage are:
1. The  depends on CONFIG_ACPI=y as most of the prototypes
   exported by it are implemented in drivers/acpi/osl.c.
2. CONFIG_DRM_I915 uses the only "inline" function acpi_os_ioremap() to
   implement stubs but it shouldn't.

Since ACPI IGD OpRegion is an ACPI-based mechanism, (please refer to the
Doclink below), this patch fixes this issue by making
drivers/gpu/drm/i915/intel_opregion.c dependent on CONFIG_ACPI.  This is
identical to other Intel DRM drivers' OpRegion support (e.x.,
drivers/gpu/drm/gma500/opregion.c).

Since acpi_io.h is not safe for CONFIG_ACPI=y environment, this patch also
moves it to include/acpi, includes it in  for CONFIG_ACPI=y
build environment and cleans up its inclusions by converting them into
 inclusions.

After that, since all  inclusions are CONFIG_ACPI
dependent, the FIXME marked  inclusion in  is
also deleted in this patch.

Doclink: 
https://01.org/linuxgraphics/sites/default/files/documentation/acpi_igd_opregion_spec.pdf
Cc: Matthew Garrett 
Acked-by: Daniel Vetter 
Cc: intel-gfx@lists.freedesktop.org
Signed-off-by: Lv Zheng 
---
 drivers/acpi/apei/apei-base.c |1 -
 drivers/acpi/apei/apei-internal.h |1 -
 drivers/acpi/apei/ghes.c  |1 -
 drivers/acpi/nvs.c|1 -
 drivers/acpi/osl.c|1 -
 drivers/gpu/drm/gma500/opregion.c |1 -
 drivers/gpu/drm/i915/Makefile |3 +--
 drivers/gpu/drm/i915/i915_drv.h   |3 ++-
 drivers/gpu/drm/i915/intel_opregion.c |1 -
 include/acpi/acpi_io.h|   17 +
 include/linux/acpi.h  |1 +
 include/linux/acpi_io.h   |   18 --
 12 files changed, 21 insertions(+), 28 deletions(-)
 create mode 100644 include/acpi/acpi_io.h
 delete mode 100644 include/linux/acpi_io.h

diff --git a/drivers/acpi/apei/apei-base.c b/drivers/acpi/apei/apei-base.c
index 6d2c49b..0760b75 100644
--- a/drivers/acpi/apei/apei-base.c
+++ b/drivers/acpi/apei/apei-base.c
@@ -34,7 +34,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git a/drivers/acpi/apei/apei-internal.h 
b/drivers/acpi/apei/apei-internal.h
index 21ba34a..e5bcd91 100644
--- a/drivers/acpi/apei/apei-internal.h
+++ b/drivers/acpi/apei/apei-internal.h
@@ -8,7 +8,6 @@
 
 #include 
 #include 
-#include 
 
 struct apei_exec_context;
 
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index a30bc31..694c486 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -33,7 +33,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git a/drivers/acpi/nvs.c b/drivers/acpi/nvs.c
index 386a9fe..ef28613 100644
--- a/drivers/acpi/nvs.c
+++ b/drivers/acpi/nvs.c
@@ -12,7 +12,6 @@
 #include 
 #include 
 #include 
-#include 
 
 /* ACPI NVS regions, APEI may use it */
 
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 7e2d814..63251b6 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -39,7 +39,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git a/drivers/gpu/drm/gma500/opregion.c 
b/drivers/gpu/drm/gma500/opregion.c
index ad0d6de..13ec628 100644
--- a/drivers/gpu/drm/gma500/opregion.c
+++ b/drivers/gpu/drm/gma500/opregion.c
@@ -22,7 +22,6 @@
  *
  */
 #include 
-#include 
 #include "psb_drv.h"
 #include "psb_intel_reg.h"
 
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 41838ea..d4ae48b 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -38,7 +38,6 @@ i915-y := i915_drv.o i915_dma.o i915_irq.o \
  intel_ringbu

<    1   2