[Intel-gfx] [PATCH 05/48] drm/i915: Takedown drm_mm on failed gtt setup
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
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
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
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
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
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
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
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
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
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
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
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
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/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
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
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
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(...)
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
--- 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
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
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
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
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
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
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
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
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
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
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
> -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
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
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
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
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"
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
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
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
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.
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