linux-2.6-block.git
10 years agodrm/i915: re-add locking around hw state readout
Jesse Barnes [Fri, 21 Feb 2014 21:13:39 +0000 (13:13 -0800)]
drm/i915: re-add locking around hw state readout

To silence locking complaints.  This was a rebase failure on my part in

commit fa9fa083d0606cb323f6105c17702460ea0a6780
Author: Jesse Barnes <jbarnes@virtuousgeek.org>
Date:   Tue Feb 11 15:28:56 2014 -0800

    drm/i915: read out hw state earlier v2

Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: honor forced connector modes v2
Jesse Barnes [Thu, 20 Feb 2014 20:28:07 +0000 (12:28 -0800)]
drm/i915: honor forced connector modes v2

In the move over to use BIOS connector configs, we lost the ability to
force a specific set of connectors on or off.  Try to remedy that by
dropping back to the old behavior if we detect a hard coded connector
config.

v2: don't deref connector state for disabled connectors (Jesse)

Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Revert workaround for disabling L3 cache aging on IVB
Chris Wilson [Fri, 14 Feb 2014 22:34:43 +0000 (22:34 +0000)]
drm/i915: Revert workaround for disabling L3 cache aging on IVB

In commit e4e0c058a19c41150d12ad2d3023b3cf09c5de67
Author: Eugeni Dodonov <eugeni.dodonov@intel.com>
Date:   Wed Feb 8 12:53:50 2012 -0800

    drm/i915: gen7: Implement an L3 caching workaround.

the L3 cache aging was disabled. This was part of a shotgun response
to a number of GPU hang bugs, but there appears to be no documentation
to suggest that disabling the L3 cache age was ever required (to prevent
the GPU hangs).

Restoring the L3 cache age is a minor performance win of around 2%
on IVB:GT2. (Note that this value seems to be consistent across a number
of tests and so appears to be above the usual noise.)

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Revert workaround for disabling L3 cache aging on BYT
Sinclair Yeh [Wed, 19 Feb 2014 21:09:31 +0000 (13:09 -0800)]
drm/i915: Revert workaround for disabling L3 cache aging on BYT

V2:  edit the commit message to contain more info
The W/A spreadsheet says this is still required, but the b-spec says
it's not for BYT-T.  So the documentation is not clear.  However,
our experience with the other SKUs of BYT-I/M on Android and Linux
suggests that setting this bit actually causes GPU hang for certain
OGL benchmark applications.

Removing this bit completely resolves the GPU hangs.

Signed-off-by: Sinclair Yeh <sinclair.yeh@intel.com>
Acked-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915/bdw: Kill ppgtt->num_pt_pages
Ben Widawsky [Fri, 21 Feb 2014 21:06:34 +0000 (13:06 -0800)]
drm/i915/bdw: Kill ppgtt->num_pt_pages

With the original PPGTT implementation if the number of PDPs was not a
power of two, the number of pages for the page tables would end up being
rounded up. The code actually had a bug here afaict, but this is a
theoretical bug as I don't believe this can actually occur with the
current code/HW..

With the rework of the page table allocations, there is no longer a
distinction between number of page table pages, and number of page
directory entries. To avoid confusion, kill the redundant (and newer)
struct member.

Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Reviewed-by: Imre Deak <imre.deak@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Split GEN6 PPGTT initialization up
Ben Widawsky [Thu, 20 Feb 2014 06:05:49 +0000 (22:05 -0800)]
drm/i915: Split GEN6 PPGTT initialization up

Simply to match the GEN8 style of PPGTT initialization, split up the
allocations and mappings. Unlike GEN8, we skip a separate dma_addr_t
allocation function, as it is much simpler pre-gen8.

With this code it would be easy to make a more general PPGTT
initialization function with per GEN alloc/map/etc. or use a common
helper, similar to the ringbuffer code. I don't see a benefit to doing
this just yet, but who knows...

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Split GEN6 PPGTT cleanup
Ben Widawsky [Thu, 20 Feb 2014 06:05:48 +0000 (22:05 -0800)]
drm/i915: Split GEN6 PPGTT cleanup

This cleanup is similar to the GEN8 cleanup (though less necessary).
Having everything split will make cleaning the initialization path error
paths easier to understand.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Update i915_gem_gtt.c copyright
Ben Widawsky [Thu, 20 Feb 2014 06:05:47 +0000 (22:05 -0800)]
drm/i915: Update i915_gem_gtt.c copyright

I keep meaning to do this... by now almost the entire file has been
written by an Intel employee (including Daniel post-2010).

Reviewed-by: Imre Deak <imre.deak@intel.com>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agoRevert "drm/i915/bdw: Limit GTT to 2GB"
Ben Widawsky [Thu, 20 Feb 2014 06:05:46 +0000 (22:05 -0800)]
Revert "drm/i915/bdw: Limit GTT to 2GB"

This reverts commit 3a2ffb65eec6dbda2fd8151894f51c18b42c8d41.

Now that the code is fixed to use smaller allocations, it should be safe
to let the full GGTT be used on BDW.

The testcase for this is anything which uses more than half of the GTT,
thus eclipsing the old limit.

Reviewed-by: Imre Deak <imre.deak@intel.com>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915/bdw: Reorganize PT allocations
Ben Widawsky [Thu, 20 Feb 2014 19:51:21 +0000 (11:51 -0800)]
drm/i915/bdw: Reorganize PT allocations

The previous allocation mechanism would get 2 contiguous allocations,
one for the page directories, and one for the page tables. As each page
table is 1 page, and there are 512 of these per page directory, this
goes to 2MB. An unfriendly request at best. Worse still, our HW now
supports 4 page directories, and a 2MB allocation is not allowed.

In order to fix this, this patch attempts to split up each page table
allocation into a single, discrete allocation. There is nothing really
fancy about the patch itself, it just has to manage an extra pointer
indirection, and have a fancier bit of logic to free up the pages.

To accommodate some of the added complexity, two new helpers are
introduced to allocate, and free the page table pages.

NOTE: I really wanted to split the way we do allocations, and the way in
which we identify the page table/page directory being used. I found
splitting this functionality up to be too unwieldy. I apologize in
advance to the reviewer. I'd recommend looking at the result, rather
than the diff.

v2/NOTE2: This patch predated commit:
6f1cc993518462ccf039e195fabd47e7aa5bfd13
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Tue Dec 31 15:50:31 2013 +0000

    drm/i915: Avoid dereference past end of page arr

It fixed the same issue as that patch, but because of the limbo state of
PPGTT, Chris patch was merged instead. The excess churn is a result of
my using my original patch, which has my preferred naming. Primarily
act_* is changed to which_*, but it's mostly the same otherwise. I've
kept the convention Chris used for the pte wrap (I had something
slightly different, and broken - but fixable)

v3: Rename which_p[..]e to drop which_ (Chris)
Remove BUG_ON in inner loop (Chris)
Redo the pde/pdpe wrap logic (Chris)

v4: s/1MB/2MB in commit message (Imre)
Plug leaking gen8_pt_pages in both the error path, as well as general
free case (Imre)

v5: Rename leftover "which_" variables (Imre)
Add the pde = 0 wrap that was missed from v3 (Imre)

Reviewed-by: Imre Deak <imre.deak@intel.com>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
[danvet: Squash in fixup from Ben.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Make clear/insert vfuncs args absolute
Ben Widawsky [Thu, 20 Feb 2014 19:50:33 +0000 (11:50 -0800)]
drm/i915: Make clear/insert vfuncs args absolute

This patch converts insert_entries and clear_range, both functions which
are specific to the VM. These functions tend to encapsulate the gen
specific PTE writes. Passing absolute addresses to the insert_entries,
and clear_range will help make the logic clearer within the functions as
to what's going on. Currently, all callers simply do the appropriate
page shift, which IMO, ends up looking weird with an upcoming change for
the gen8 page table allocations.

Up until now, the PPGTT was a funky 2 level page table. GEN8 changes
this to look more like a 3 level page table, and to that extent we need
a significant amount more memory simply for the page tables. To address
this, the allocations will be split up in finer amounts.

v2: Replace size_t with uint64_t (Chris, Imre)

v3: Fix size in gen8_ppgtt_init (Ben)
Fix Size in i915_gem_suspend_gtt_mappings/restore (Imre)

Reviewed-by: Imre Deak <imre.deak@intel.com> (v2)
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915/bdw: Split ppgtt initialization up
Ben Widawsky [Thu, 20 Feb 2014 06:05:43 +0000 (22:05 -0800)]
drm/i915/bdw: Split ppgtt initialization up

Like cleanup in an earlier patch, the code becomes much more readable,
and easier to extend if we extract out helper functions for the various
stages of init.

Note that with this patch it becomes really simple, and tempting to begin
using the 'goto out' idiom with explicit free/fini semantics. I've
kept the error path as similar as possible to the cleanup() function to
make sure cleanup is as robust as possible

v2: Remove comment "NB:From here on, ppgtt->base.cleanup() should
function properly"
Update commit message to reflect above

v3: Rebased on top of bugfixes found in the previous patch by Imre
Moved number of pd pages assertion to the proper place (Imre)

v4:
Allocate dma address space for num_pd_pages, not num_pd_entries (Ben)
Don't use gen8_pt_dma_addr after free on error path (Imre)
With new fix from v4 of the previous patch.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Reviewed-by: Imre Deak <imre.deak@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915/bdw: Reorganize PPGTT init
Ben Widawsky [Thu, 20 Feb 2014 06:05:42 +0000 (22:05 -0800)]
drm/i915/bdw: Reorganize PPGTT init

Create 3 clear stages in PPGTT init. This will help with upcoming
changes be more readable. The 3 stages are, allocation, dma mapping, and
writing the P[DT]Es

One nice benefit to the patches is that it makes 2 very clear error
points, allocation, and mapping, and avoids having to do any handling
after writing PTEs (something which was likely buggy before). This
simplified error handling I suspect will be helpful when we move to
deferred/dynamic page table allocation and mapping.

The patches also attempts to break up some of the steps into more
logical reviewable chunks, particularly when we free.

v2: Don't call cleanup on the error path since that takes down the
drm_mm and list entry, which aren't setup at this point.

v3: Fixes addressing Imre's comments from:
<1392821989.19792.13.camel@intelbox>

Don't do dynamic allocation for the page table DMA addresses. I can't
remember why I did it in the first place. This addresses one of Imre's
other issues.

Fix error path leak of page tables.

v4: Fix the fix of the error path leak. Original fix still leaked page
tables. (Imre)

Reviewed-by: Imre Deak <imre.deak@intel.com>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915/bdw: Free PPGTT struct
Ben Widawsky [Thu, 20 Feb 2014 19:47:07 +0000 (11:47 -0800)]
drm/i915/bdw: Free PPGTT struct

GEN8 never freed the PPGTT struct. As GEN8 doesn't use full PPGTT, the
leak is small and only found on a module reload. ie. I don't think this
needs to go to stable.

v2: The very naive, kfree in gen8 ppgtt cleanup, is subject to a double
free on PPGTT initialization failure. (Spotted by Imre). Instead this
patch pulls the ppgtt struct freeing out of the cleanup and leaves it to
the allocators/callers or the one doing the last kref_put as in standard
convention

Reported-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Reviewed-by: Imre Deak <imre.deak@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Move ppgtt_release out of the header
Ben Widawsky [Thu, 20 Feb 2014 19:47:06 +0000 (11:47 -0800)]
drm/i915: Move ppgtt_release out of the header

At one time it was expected to be called in multiple places by kref_put.
At the current time however, it is all contained within
i915_gem_context.c.

This patch makes an upcoming required addition a bit nicer since it too
doesn't need to be defined in a header file.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Reviewed-by: Imre Deak <imre.deak@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Add a comment about WIZ hashing vs. thread counts
Ville Syrjälä [Wed, 5 Feb 2014 10:43:47 +0000 (12:43 +0200)]
drm/i915: Add a comment about WIZ hashing vs. thread counts

Add a comment next to our WIZ hashing setup to remind people about the
link between WIZ hashing disable bit and PS/WM thread counts.

Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Change BDW WIZ hashing mode to 16x4
Ville Syrjälä [Tue, 4 Feb 2014 19:59:21 +0000 (21:59 +0200)]
drm/i915: Change BDW WIZ hashing mode to 16x4

BSpec recommends using 8x4 hashing mode when MSAA is used. But in
practice 16x4 seems to have a slight edge in performance (on IVB and
HSW at least). So just use 16x4.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Antti Koskipää <antti.koskipaa@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Change HSW WIZ hashing mode to 16x4
Ville Syrjälä [Tue, 4 Feb 2014 19:59:20 +0000 (21:59 +0200)]
drm/i915: Change HSW WIZ hashing mode to 16x4

BSpec recommends using 8x4 hashing mode when MSAA is used. But in
practice 16x4 seems to have a slight edge in performance (on IVB and
HSW at least). So just use 16x4.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Antti Koskipää <antti.koskipaa@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Change IVB WIZ hashing mode to 16x4
Ville Syrjälä [Tue, 4 Feb 2014 19:59:19 +0000 (21:59 +0200)]
drm/i915: Change IVB WIZ hashing mode to 16x4

BSpec recommends using 8x4 hashing mode when MSAA is used. But in
practice 16x4 seems to have a slight edge in performance (on IVB and
HSW at least). So just use 16x4.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Antti Koskipää <antti.koskipaa@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: There's no need to mask all 3D_CHICKEN bits on SNB
Ville Syrjälä [Tue, 4 Feb 2014 19:59:17 +0000 (21:59 +0200)]
drm/i915: There's no need to mask all 3D_CHICKEN bits on SNB

The need to set all of the mask bits for 3D_CHICKEN3 was required
only for pre-production hardware.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Antti Koskipää <antti.koskipaa@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Assume we implement WaStripsFansDisableFastClipPerformanceFix:snb
Ville Syrjälä [Tue, 4 Feb 2014 19:59:16 +0000 (21:59 +0200)]
drm/i915: Assume we implement WaStripsFansDisableFastClipPerformanceFix:snb

Based on the name, the workaround we implement is
WaStripsFansDisableFastClipPerformanceFix. Unfortunately there's no
description in the w/a database, so this is just a guess.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Antti Koskipää <antti.koskipaa@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Fix SNB GT_MODE register setup
Ville Syrjälä [Tue, 4 Feb 2014 19:59:15 +0000 (21:59 +0200)]
drm/i915: Fix SNB GT_MODE register setup

On SNB we set up WaSetupGtModeTdRowDispatch:snb early in
gen6_init_clock_gating(). That sets a bit in the GEN6_GT_MODE register.
However later we go and disable all the bits in the same register. And
then we go on to set some other bit. So apparently we never actually
implemented this workaround since the "disable all bits" part was there
already before the w/a got supposedly implemented.

These are the relevant commits:

 commit 6547fbdbfff62c99e4f7b4f985ff8b3454f33b0f
 Author: Daniel Vetter <daniel.vetter@ffwll.ch>
 Date:   Fri Dec 14 23:38:29 2012 +0100

    drm/i915: Implement WaSetupGtModeTdRowDispatch

 commit f8f2ac9a76b0f80a6763ca316116a7bab8486997
 Author: Ben Widawsky <ben@bwidawsk.net>
 Date:   Wed Oct 3 19:34:24 2012 -0700

    drm/i915: Fix GT_MODE default value

So, let's drop the "disable all bits" part, move both writes to
closer proxomity to each other, and name the WIZ hashing bits
appropriately. BSpec is still a bit confused how the bits should
actually be interpreted, but I took the the description for the
high bit since the low bit part only lists values for a single bit.

Also add a comment about our choice of WIZ hashing mode.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Antti Koskipää <antti.koskipaa@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: get/put runtime PM without holding rps.hw_lock
Paulo Zanoni [Thu, 19 Dec 2013 13:54:52 +0000 (11:54 -0200)]
drm/i915: get/put runtime PM without holding rps.hw_lock

We'll need this when we merge PC8 and Runtime PM: the PC8
enable/disable functions need that lock.

Also, it's good practice to not hold a lock for longer than strictly
needed.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: rename modeset_update_power_wells
Paulo Zanoni [Thu, 19 Dec 2013 13:54:51 +0000 (11:54 -0200)]
drm/i915: rename modeset_update_power_wells

To modeset_update_crtc_power_domains, since this function is
responsible for updating all the power domains of all CRTCs after a
modeset. In the future we should also run this function on all
platforms, not just Haswell.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Remove dead code
Thierry Reding [Fri, 21 Feb 2014 07:55:27 +0000 (08:55 +0100)]
drm/i915: Remove dead code

The i915 driver sets DRIVER_GEM unconditionally, so testing for the
feature will always fail.

Signed-off-by: Thierry Reding <treding@nvidia.com>
[danvet: Fix up conflicts.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: sprinkle static
Daniel Vetter [Sun, 2 Mar 2014 20:18:00 +0000 (21:18 +0100)]
drm/i915: sprinkle static

Apparently we've missed a few more than what Fengguang's 0-day tester
recently reported in i915_irq.c ... Makes sparse happy again (ignore
some spurious stuff about ksyms of exported functions).

Cc: kbuild test robot <fengguang.wu@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: tune down user-triggerable dmesg noise in the cursor/overlay code
Daniel Vetter [Fri, 14 Feb 2014 13:06:06 +0000 (14:06 +0100)]
drm/i915: tune down user-triggerable dmesg noise in the cursor/overlay code

Spotted while auditing the code for fencing issues.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: fix NULL deref in the load detect code
Daniel Vetter [Fri, 14 Feb 2014 15:35:54 +0000 (16:35 +0100)]
drm/i915: fix NULL deref in the load detect code

Looks like I've missed one of the potential NULL deref bugs in Jesse's
fbdev->fb embedded struct to pointer conversions. Fix it up.

This regression has been introduced in

commit 8bcd45534ddf68ab71aeed709dacd9cf65dc0f75
Author: Jesse Barnes <jbarnes@virtuousgeek.org>
Date:   Fri Feb 7 12:10:38 2014 -0800

    drm/i915: alloc intel_fb in the intel_fbdev struct

Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Only bind each object rather than for every execbuffer
Daniel Vetter [Fri, 14 Feb 2014 13:01:21 +0000 (14:01 +0100)]
drm/i915: Only bind each object rather than for every execbuffer

One side-effect of the introduction of ppgtt was that we needed to
rebind the object into the appropriate vm (and global gtt in some
peculiar cases). For simplicity this was done twice for every object on
every call to execbuffer. However, that adds a tremendous amount of CPU
overhead (rewriting all the PTE for all objects into WC memory) per
draw. The fix is to push all the decision about which vm to bind into
and when down into the low-level bind routines through hints rather than
performing the bind unconditionally in the execbuffer routine.

Note that this is a regression introduced in the full ppgtt feature
branch, before this we've only done re-bound objects when the relevant
has_(aliasing_ppgtt|global_gtt)_mapping flag was clear. But since
that's per-object and not per-vma that optimization broke.

v2: Split out prep work and unrelated changes.

v3: Bring back functional change around PIN_GLOBAL that I've
accidentally split out.

v4: Remove the temporary hack for the old binding logic to avoid
bisection issues.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=72906
Tested-by: jianx.zhou@intel.com
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> (v1)
Cc: Ben Widawsky <benjamin.widawsky@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Acked-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Directly return the vma from bind_to_vm
Daniel Vetter [Fri, 14 Feb 2014 13:01:20 +0000 (14:01 +0100)]
drm/i915: Directly return the vma from bind_to_vm

This is prep work for reworking the object_pin logic. Atm
it still does a (now redundant) lookup of the vma. The next
patch will fix this.

Split out from Chris vma-bind rework.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ben Widawsky <benjamin.widawsky@intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Simplify i915_gem_object_ggtt_unpin
Daniel Vetter [Fri, 14 Feb 2014 13:01:19 +0000 (14:01 +0100)]
drm/i915: Simplify i915_gem_object_ggtt_unpin

Split out from Chris vma-bind rework.

Jani wondered why this is save, and the reason is that i915_vma_unbind
does all these checks, too. So they're redundant.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ben Widawsky <benjamin.widawsky@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Allow blocking in the PDE alloc when running low on gtt space
Daniel Vetter [Fri, 14 Feb 2014 13:01:18 +0000 (14:01 +0100)]
drm/i915: Allow blocking in the PDE alloc when running low on gtt space

There's no need not to, really.

Split out from Chris vma-bind rework.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ben Widawsky <benjamin.widawsky@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Don't allocate context pages as mappable
Daniel Vetter [Fri, 14 Feb 2014 13:01:17 +0000 (14:01 +0100)]
drm/i915: Don't allocate context pages as mappable

Only the hardware really access them, so no need to have cpu
gtt access available.

Split out from Chris vma-bind rework.

Note that this is only possible due to the split-up of the mappable
pin flag into PIN_GLOBAL and PIN_MAPPABLE.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ben Widawsky <benjamin.widawsky@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Handle set_cache_level errors in the status page setup
Daniel Vetter [Fri, 14 Feb 2014 13:01:16 +0000 (14:01 +0100)]
drm/i915: Handle set_cache_level errors in the status page setup

Split out from Chris vma-bind rework.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ben Widawsky <benjamin.widawsky@intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Don't pin the status page as mappable
Daniel Vetter [Fri, 14 Feb 2014 13:01:15 +0000 (14:01 +0100)]
drm/i915: Don't pin the status page as mappable

We access it through the cpu window. No functional difference expected
atm since we default to a bottom-up allocation scheme. But that might
eventually change so that we prefer the unmappable range for buffers
that don't need cpu gtt access.

Split out from Chris vma-bind rework.

Note that this is only possible due to the split-up of the mappable
pin flag into PIN_GLOBAL and PIN_MAPPABLE.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ben Widawsky <benjamin.widawsky@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Don't set PIN_MAPPABLE for legacy ringbuffers
Daniel Vetter [Fri, 14 Feb 2014 13:01:14 +0000 (14:01 +0100)]
drm/i915: Don't set PIN_MAPPABLE for legacy ringbuffers

Tighter code since legacy gem has only mappable anyway.

Split out from Chris vma-bind rework.

Note that this is only possible due to the split-up of the mappable
pin flag into PIN_GLOBAL and PIN_MAPPABLE.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ben Widawsky <benjamin.widawsky@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Handle set_cache_level errors in the pipe control scratch setup
Daniel Vetter [Fri, 14 Feb 2014 13:01:13 +0000 (14:01 +0100)]
drm/i915: Handle set_cache_level errors in the pipe control scratch setup

Split out from Chris vma-bind rework.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ben Widawsky <benjamin.widawsky@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: split PIN_GLOBAL out from PIN_MAPPABLE
Daniel Vetter [Fri, 14 Feb 2014 13:01:12 +0000 (14:01 +0100)]
drm/i915: split PIN_GLOBAL out from PIN_MAPPABLE

With abitrary pin flags it makes sense to split out a "please bind
this into global gtt" from the "please allocate in the mappable
range".

Use this unconditionally in our global gtt pin helper since this is
what its callers want. Later patches will drop PIN_MAPPABLE where it's
not strictly needed.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Consolidate binding parameters into flags
Daniel Vetter [Fri, 14 Feb 2014 13:01:11 +0000 (14:01 +0100)]
drm/i915: Consolidate binding parameters into flags

Anything more than just one bool parameter is just a pain to read,
symbolic constants are much better.

Split out from Chris' vma-binding rework patch.

v2: Undo the behaviour change in object_pin that Chris spotted.

v3: Split out misplaced hunk to handle set_cache_level errors,
spotted by Jani.

v4: Keep the current over-zealous binding logic in the execbuffer code
working with a quick hack while the overall binding code gets shuffled
around.

v5: Reorder the PIN_ flags for more natural patch splitup.

v6: Pull out the PIN_GLOBAL split-up again.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ben Widawsky <benjamin.widawsky@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: sdvo: add i2c sysfs symlink to the connector's directory
Imre Deak [Tue, 11 Feb 2014 15:12:51 +0000 (17:12 +0200)]
drm/i915: sdvo: add i2c sysfs symlink to the connector's directory

This is the same what we do for DP connectors, so make things more
consistent.

Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Antti Koskipää <antti.koskipaa@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: sdvo: fix error path in sdvo_connector_init
Imre Deak [Tue, 11 Feb 2014 15:12:50 +0000 (17:12 +0200)]
drm/i915: sdvo: fix error path in sdvo_connector_init

Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Antti Koskipää <antti.koskipaa@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: dp: fix order of dp aux i2c device cleanup
Imre Deak [Tue, 11 Feb 2014 15:12:49 +0000 (17:12 +0200)]
drm/i915: dp: fix order of dp aux i2c device cleanup

Atm we set the parent of the dp i2c device to be the correspondig
connector device. During driver cleanup we first remove the connector
device through intel_modeset_cleanup()->drm_sysfs_connector_remove() and
only after that the i2c device through the encoder's destroy callback.
This order is not supported by the device core and we'll get a warning,
see the below bugzilla ticket. The proper order is to remove first any
child device and only then the parent device.

The first part of the fix changes the i2c device's parent to be the drm
device. Its logical owner is not the connector anyway, but the encoder.
Since the encoder doesn't have a device object, the next best choice is
the drm device. This is the same what we do in the case of the sdvo i2c
device and what the nouveau driver does.

The second part creates a symlink in the connector's sysfs directory
pointing to the i2c device. This is so, that we keep the current ABI,
which also makes sense in case someone wants to look up the i2c device
belonging to a specific connector.

Reference: http://lists.freedesktop.org/archives/intel-gfx/2014-January/038782.html
Reference: http://lists.freedesktop.org/archives/intel-gfx/2014-February/039427.html
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=70523
Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Antti Koskipää <antti.koskipaa@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: add unregister callback to connector
Imre Deak [Tue, 11 Feb 2014 15:12:48 +0000 (17:12 +0200)]
drm/i915: add unregister callback to connector

Since

commit d9255d57147e1dbcebdf6670409c2fa0ac3609e6
Author: Paulo Zanoni <paulo.r.zanoni@intel.com>
Date:   Thu Sep 26 20:05:59 2013 -0300

it became clear that we need to separate the unload sequence into two
parts:

1. remove all interfaces through which new operations on some object
   (crtc, encoder, connector) can be started and make sure all pending
   operations are completed
2. do the actual tear down of the internal representation of the above
   objects

The above commit achieved this separation for connectors by splitting
out the sysfs removal part from the connector's destroy callback and
doing this removal before calling drm_mode_config_cleanup() which does
the actual tear-down of all the drm objects.

Since we'll have to customize the interface removal part for different
types of connectors in the upcoming patches, add a new unregister
callback and move the interface removal part to it.

No functional change.

Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Antti Koskipää <antti.koskipaa@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: don't reference null pointer at i915_sink_crc
Paulo Zanoni [Thu, 13 Feb 2014 19:51:33 +0000 (17:51 -0200)]
drm/i915: don't reference null pointer at i915_sink_crc

Reproducible by runtime suspending a Haswell machine with eDP + HDMI
outputs connected.

[  209.600086] [drm:i915_runtime_suspend], Suspending device
[  209.688435] BUG: unable to handle kernel NULL pointer dereference at 0000000000000060
[  209.688500] IP: [<ffffffffa0109d4e>] i915_sink_crc+0x6e/0xf0 [i915]
[  209.688577] PGD 36aba067 PUD 35d7f067 PMD 0
[  209.688613] Oops: 0000 [#1] SMP
[  209.688641] Modules linked in: fuse ip6table_filter ip6_tables ebtable_nat ebtables iTCO_wdt iTCO_vendor_support x86_pkg_temp_thermal coretemp microcode serio_raw e1000e pcspkr i2c_i801 ptp mei_me mei lpc_ich mfd_core pps_core dm_crypt i915 i2c_algo_bit crc32_pclmul drm_kms_helper crc32c_intel drm ghash_clmulni_intel video
[  209.688893] CPU: 1 PID: 1797 Comm: pm_pc8 Not tainted 3.13.0+ #118
[  209.688937] Hardware name: Intel Corporation Shark Bay Client platform/WhiteTip Mountain 1, BIOS HSWLPTU1.86C.0133.R00.1309172123 09/17/2013
[  209.689023] task: ffff88007fb4b690 ti: ffff88007d9d2000 task.ti: ffff88007d9d2000
[  209.689074] RIP: 0010:[<ffffffffa0109d4e>]  [<ffffffffa0109d4e>] i915_sink_crc+0x6e/0xf0 [i915]
[  209.689169] RSP: 0018:ffff88007d9d3e68  EFLAGS: 00010246
[  209.689205] RAX: 0000000000000000 RBX: ffff880036a03478 RCX: ffff8800366c9770
[  209.689252] RDX: ffff88014325cf38 RSI: ffff88007fb4bd08 RDI: ffff88007fb4b690
[  209.689299] RBP: ffff88007d9d3e98 R08: 0000000000000000 R09: 0000000000000000
[  209.689346] R10: 0000000000000001 R11: 0000000000000000 R12: ffff8800366c9148
[  209.689393] R13: 00000000ffffffed R14: ffff88007d9d3f50 R15: ffff880036a03478
[  209.689441] FS:  00007f5a74bc29c0(0000) GS:ffff88014f240000(0000) knlGS:0000000000000000
[  209.689494] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  209.689533] CR2: 0000000000000060 CR3: 0000000079d7e000 CR4: 00000000001407e0
[  209.689580] Stack:
[  209.689594]  0000000000001000 ffff880146083980 ffff880146083980 0000000000000000
[  209.689649]  ffff880146083980 0000000000000001 ffff88007d9d3f00 ffffffff811d0744
[  209.689702]  0000000000000046 00007fff7949fe20 ffff880036a034b8 0000000000000080
[  209.689756] Call Trace:
[  209.689778]  [<ffffffff811d0744>] seq_read+0x164/0x3e0
[  209.689816]  [<ffffffff811ab165>] vfs_read+0x95/0x160
[  209.689851]  [<ffffffff811abc79>] SyS_read+0x49/0xa0
[  209.689888]  [<ffffffff810ef64c>] ? __audit_syscall_entry+0x9c/0xf0
[  209.689933]  [<ffffffff81659412>] system_call_fastpath+0x16/0x1b

Testcase: igt/pm_pc8 (do a full run, it will fail at the debugfs-read subtest)
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
[danvet: Flip around NULL check for robustness.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915/lvds: Remove dead code from failing case
Damien Lespiau [Thu, 13 Feb 2014 18:56:15 +0000 (18:56 +0000)]
drm/i915/lvds: Remove dead code from failing case

Coverity points out that, if we end up in the 'failed' label, that's
precisely because we couldn't retrieve a fixed mode (ie fixed_mode is
NULL) and then "if (fixed_mode)" is always false.

Remove that dead code.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: don't preserve inherited configs with nothing on v2
Jesse Barnes [Wed, 12 Feb 2014 23:03:40 +0000 (15:03 -0800)]
drm/i915: don't preserve inherited configs with nothing on v2

It can be corrected later and may be what was actually desired, but
generally isn't, so if we find nothing is enabled, let the core DRM fb
helper figure something out.

v2: free the array too (Jesse)

Note that this also undoes any changes in case we bail out due to hw
cloning.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915/bdw: Split up PPGTT cleanup
Ben Widawsky [Wed, 12 Feb 2014 22:28:44 +0000 (14:28 -0800)]
drm/i915/bdw: Split up PPGTT cleanup

This will make the code more readable, and extensible which is needed
for upcoming feature work. Eventually, we'll do the same for init.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Convert DIP port switch cases to a simple macro
Ville Syrjälä [Thu, 23 Jan 2014 21:15:34 +0000 (23:15 +0200)]
drm/i915: Convert DIP port switch cases to a simple macro

We have a couple of switch cases to compute the port value for the
VIDEO_DIP_CTL register. Replace them with a simple macro.

We do lose a few BUG() calls, but many people may consider that
an improvement.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: delay master/sarea deref for legacy ioctls
Daniel Vetter [Wed, 12 Feb 2014 22:50:06 +0000 (23:50 +0100)]
drm/i915: delay master/sarea deref for legacy ioctls

... past the check for DRIVER_MODESET. Avoids races with userspace
opening a master and our sarea setup.

Cc: Signed-off-by: Stéphane Marchesin <marcheu@chromium.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: protect ringbuffer sarea update behind !MODESET
Daniel Vetter [Wed, 12 Feb 2014 22:44:34 +0000 (23:44 +0100)]
drm/i915: protect ringbuffer sarea update behind !MODESET

Avoids surprises when userspace races open/closes against this.

Cc: Stéphane Marchesin <marcheu@chromium.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: kill intel_crtc_update_sarea_pos
Daniel Vetter [Wed, 12 Feb 2014 22:36:24 +0000 (23:36 +0100)]
drm/i915: kill intel_crtc_update_sarea_pos

We assign the sarea_priv pointer only in the dma ioctl, which is
disallowed when kernel modesetting is enabled. So this is dead code.

Cc: Stéphane Marchesin <marcheu@chromium.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: allow re-use BIOS connector config for initial fbdev config v3
Jesse Barnes [Wed, 12 Feb 2014 20:26:25 +0000 (12:26 -0800)]
drm/i915: allow re-use BIOS connector config for initial fbdev config v3

The BIOS or boot loader will generally create an initial display
configuration for us that includes some set of active pipes and
displays.  This routine tries to figure out which pipes and connectors
are active and stuffs them into the crtcs and modes array given to us by
the drm_fb_helper code.

The overall sequence is:
  intel_fbdev_init - from driver load
    intel_fbdev_init_bios - initialize the intel_fbdev using BIOS data
    drm_fb_helper_init - build fb helper structs
    drm_fb_helper_single_add_all_connectors - more fb helper structs
  intel_fbdev_initial_config - apply the config
    drm_fb_helper_initial_config - call ->probe then register_framebuffer()
        drm_setup_crtcs - build crtc config for fbdev
          intel_fb_initial_config - find active connectors etc
        drm_fb_helper_single_fb_probe - set up fbdev
          intelfb_create - re-use or alloc fb, build out fbdev structs

v2: use BIOS connector config unconditionally if possible (Daniel)
    check for crtc cloning and reject (Daniel)
    fix up comments (Daniel)
v3: use command line args and preferred modes first (Ville)

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Tested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
[danvet: Re-add the WARN_ON for a missing encoder crtc - the state
sanitizer should take care of this. And spell-ocd the comments.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm: export cmdline and preferred mode functions from fb helper
Jesse Barnes [Wed, 12 Feb 2014 20:26:24 +0000 (12:26 -0800)]
drm: export cmdline and preferred mode functions from fb helper

This allows drivers to use them in custom initial_config functions.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Acked-by: Dave Airlie <airlied@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Some polish for the new pipestat_irq_handler
Daniel Vetter [Wed, 12 Feb 2014 16:55:36 +0000 (17:55 +0100)]
drm/i915: Some polish for the new pipestat_irq_handler

Just a bit of polish which I hope will help me with massaging some
internal patches to use Imre's reworked pipestat handling:
- Don't check for underrun reporting or enable pipestat interrupts
  twice.
- Frob the comments a bit.
- Do the iir PIPE_EVENT to pipe mapping explicitly with a switch. We
  only have one place which does this, so better to make it explicit.

v2: Ville noticed that I've broken the logic a bit with trying to
avoid checking whether we're interested in a given pipe twice. push
the PIPESTAT read down after we've computed the mask of interesting
bits first to avoid that duplication properly.

v3: Squash in fixups from Imre on irc.

Cc: Imre Deak <imre.deak@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Pass explicit mode into mode_from_pipe_config v3
Daniel Vetter [Tue, 11 Feb 2014 23:28:57 +0000 (15:28 -0800)]
drm/i915: Pass explicit mode into mode_from_pipe_config v3

We want to reuse this in the fbdev initial config code independently
from any fastboot hacks. So allow a bit more flexibility.

v2: Forgot to git add ...
v3: make non-static (Jesse)

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: read out hw state earlier v2
Jesse Barnes [Tue, 11 Feb 2014 23:28:56 +0000 (15:28 -0800)]
drm/i915: read out hw state earlier v2

We want to do this early on before we try to fetch the plane config,
which depends on some of the pipe config state.

Note that the important part is that we do this before we initialize
gem, since otherwise we can't properly pre-reserve the stolen memory
for framebuffers inherited from the bios.

v2: split back out from get_plane_config change (Daniel)
    update for recent locking & reset changes (Jesse)

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
[danvet: Explain a bit more why we need to move this.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: unbind fbs from crtcs during driver unload
Imre Deak [Tue, 11 Feb 2014 22:01:10 +0000 (00:01 +0200)]
drm/i915: unbind fbs from crtcs during driver unload

So far during driver unload we called drm_framebuffer_cleanup() for the
fbdev fb, which only removes the fb from the drm fb list regardless of
its reference count, but leaves the fb bound on an active crtc. Since
the fb's backing storage was freed this could mean we scan some random
memory content out afterwards. It's not a big issue since the fb is
allocated from stolen memory and afaik there is no other user for that
than i915. It's still cleaner to properly unbind the fb and disable the
crtc, which is what drm_framebuffer_remove() does.

Note that after

commit 88891eb1e9eca0ba619518bed31580f91e9cf84d
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Mon Feb 10 18:00:38 2014 +0100

we call drm_framebuffer_cleanup() only after dropping the last reference
on the fb, but that won't happen since we don't unbind the fb. This
results in a drm core warn about a leaked fb.

Signed-off-by: Imre Deak <imre.deak@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915/bdw: Use centralized rc6 info print
Ben Widawsky [Wed, 29 Jan 2014 04:25:41 +0000 (20:25 -0800)]
drm/i915/bdw: Use centralized rc6 info print

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Reviewed-by: Deepak S <deepak.s@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Just print rc6 facts
Ben Widawsky [Wed, 29 Jan 2014 04:25:40 +0000 (20:25 -0800)]
drm/i915: Just print rc6 facts

Everything can be overridden by module parameters, so don't confuse the
users that are using them.

We have RC6 turned on for all platforms which support it, but Ironlake,
so the need to explain the situation is no longer pressing.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Reviewed-by: Deepak S <deepak.s@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Stop pretending VLV has rc6+
Ben Widawsky [Wed, 29 Jan 2014 04:25:39 +0000 (20:25 -0800)]
drm/i915: Stop pretending VLV has rc6+

It wasn't ever used by the caller anyway with the exception of what we
show in sysfs.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Reviewed-by: Deepak S <deepak.s@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
[danvet: Apply Deepak's suggestion.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Clarify RC6 enabling
Ben Widawsky [Wed, 29 Jan 2014 04:25:38 +0000 (20:25 -0800)]
drm/i915: Clarify RC6 enabling

At one time, we though all future platforms would have the deeper RC6
states. As it turned out, they killed it after Ivybridge, and began
using other means to achieve the power savings (the stuff we need to get
to PC7+).

The enable function was left in a weird state of odd corner cases as a
result. Since the future is now, and we also have some insight into
what's currently the future, we have an opportunity to simplify, and
future proof the function.

NOTE: VLV will be addressed in a subsequent patch. This patch was trying
not to change functionality.

NOTE2: All callers sanitize the return value anyway, so this patch is
simply to have the code make a bit more sense.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Reviewed-by: Deepak S <deepak.s@intel.com>
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: WARN in case we're enabling the pipe and it's enabled
Paulo Zanoni [Fri, 17 Jan 2014 15:51:13 +0000 (13:51 -0200)]
drm/i915: WARN in case we're enabling the pipe and it's enabled

... and QUIRK_PIPEA_FORCE is not present.

I initially thought that case was impossible and just added a WARN on
it, but then I was told this case is possible due to
QUIRK_PIPEA_FORCE. So let's add a WARN that serves two purposes:
  - tell us in case we have done something wrong;
  - document the only case where we expect this.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: remove wait_for_vblank argument form intel_enable_pipe
Paulo Zanoni [Fri, 17 Jan 2014 15:51:12 +0000 (13:51 -0200)]
drm/i915: remove wait_for_vblank argument form intel_enable_pipe

Add a nice comment explaining why we shouldn't wait for a vblank on
all cases, wait based on the HW gen, and add a comment saying we
should probably skip that wait on some of the previous HW gens.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: remove "dsi" argument form intel_enable_pipe
Paulo Zanoni [Fri, 17 Jan 2014 15:51:11 +0000 (13:51 -0200)]
drm/i915: remove "dsi" argument form intel_enable_pipe

Now that we pass struct intel_crtc as an argument, we can check for
DSI inside the function, removing one more of those confusing boolean
arguments.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: remove pch_port argument form intel_enable_pipe
Paulo Zanoni [Fri, 17 Jan 2014 15:51:10 +0000 (13:51 -0200)]
drm/i915: remove pch_port argument form intel_enable_pipe

Now that we pass struct intel_crtc as an argument, there's no need for
it.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: pass intel_crtc as argument for intel_enable_pipe
Paulo Zanoni [Fri, 17 Jan 2014 15:51:09 +0000 (13:51 -0200)]
drm/i915: pass intel_crtc as argument for intel_enable_pipe

We want to remove those 3 boolean arguments. This is the first step.
The "pipe" passed as the argument is always intel_crtc->pipe.

Also adjust the function documentation.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: remove the vblank_wait hack from HSW+
Paulo Zanoni [Thu, 19 Dec 2013 21:12:31 +0000 (19:12 -0200)]
drm/i915: remove the vblank_wait hack from HSW+

When I forked haswell_crtc_enable I copied all the code from
ironlake_crtc_enable. The last piece of the function contains a big
comment with a call to intel_wait_for_vblank. After this fork, we
rearranged the Haswell code so that it enables the planes as the very
last step of the modeset sequence, so we're sure that we call
intel_enable_primary_plane after the pipe is really running, so the
vblank waiting functions work as expected. I really believe this is
what fixes the problem described by the big comment, so let's give it
a try and get rid of that intel_wait_for_vblank, saving around 16ms
per modeset (and init/resume). We can always revert if needed :)

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: don't wait for vblank after enabling pipe on HSW
Paulo Zanoni [Thu, 19 Dec 2013 21:12:30 +0000 (19:12 -0200)]
drm/i915: don't wait for vblank after enabling pipe on HSW

Because on Haswell, the pipe is never running at this point, so we hit
the 50ms timeout waiting for nothing. We already have two other places
where we wait for vblanks on haswell_crtc_enable, so we're safe.

This gets us rid of one instance of "vblank wait timed out" for each
mode set, which means driver init and resume are also 50ms faster.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: add wait_for_vblank argument to intel_enable_pipe
Paulo Zanoni [Thu, 19 Dec 2013 21:12:29 +0000 (19:12 -0200)]
drm/i915: add wait_for_vblank argument to intel_enable_pipe

Depending on the HW gen and the connector type, the pipe won't start
running right after we call intel_enable_pipe, so that
intel_wait_for_vblank call we currently have will just sit there for
the full 50ms timeout. So this patch adds an argument that will allow
us to avoid the vblank wait in case we want. Currently all the callers
still request for the vblank wait, so the behavior should still be the
same.

We also added a POSTING_READ on the register: previously
intel_wait_for_vblank was acting as a POSTING_READ, but now if
wait_for_vblank is false we'll stkip it, so we need an explicit
POSTING_READ.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Initialize downclock mode in panel init
Vandana Kannan [Tue, 11 Feb 2014 08:56:36 +0000 (14:26 +0530)]
drm/i915: Initialize downclock mode in panel init

Instead of modifying intel_panel in lvds_init_connector/dsi_init/
edp_init_connector, making changes to move intel_panel->downclock_mode
initialization to intel_panel_init()

v2: Jani's review comments incorporated
Removed downclock_mode local variable in dsi_init and
edp_init_connector

Signed-off-by: Vandana Kannan <vandana.kannan@intel.com>
Signed-off-by: Pradeep Bhat <pradeep.bhat@intel.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: add a display info file to debugfs v2
Jesse Barnes [Fri, 7 Feb 2014 20:48:15 +0000 (12:48 -0800)]
drm/i915: add a display info file to debugfs v2

Can be expanded up on to include all sorts of things (HDMI infoframe
data, more DP status, etc).  Should be useful for bug reports to get a
baseline on the display config and info.

v2: use seq_putc (Rodrigo)
    describe mode field names (Rodrigo)

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm: expose subpixel order name routine v3
Jesse Barnes [Mon, 10 Feb 2014 23:32:44 +0000 (15:32 -0800)]
drm: expose subpixel order name routine v3

Just like we have for connector type etc.

v2: drop static array (Chris)
v3: add kdoc (Daniel)

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Acked-by: Dave Airlie <airlied@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: split aligned height calculation out v2
Jesse Barnes [Fri, 7 Feb 2014 20:10:35 +0000 (12:10 -0800)]
drm/i915: split aligned height calculation out v2

For use by get_plane_config.

v2: cleanup tile_height bits (Chris)

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Fix error path leak in fbdev fb allocation
Daniel Vetter [Mon, 10 Feb 2014 17:00:39 +0000 (18:00 +0100)]
drm/i915: Fix error path leak in fbdev fb allocation

In Jesse's patch to switch the fbdev framebuffer from an embedded
struct to a pointer the kfree in case of an error was missed. Fix this
up by using our own internal fb allocation helper directly instead of
reinventing that wheel.

We need a to_intel_framebuffer cast unfortunately since all the other
callers of _create still look better whith using a drm_framebuffer as
return pointer.

v2: Add an unlocked __intel_framebuffer_create function since our
dev->struct_mutex locking is too much a mess. With ppgtt we even need
it to take a look at the global gtt offset of pinned objects, since
the vma list might chance from underneath us. At least with the
current global gtt lookup functions. Reported by Mika.

Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Use normal fb deref for the fbcon framebuffer
Daniel Vetter [Mon, 10 Feb 2014 17:00:38 +0000 (18:00 +0100)]
drm/i915: Use normal fb deref for the fbcon framebuffer

Now that it's a normally kmalloce buffer we can use the usual cleanup
paths. The upside here is that if we get the refcounting wrong will be
able to catch it, since the drm core will complain about leftover
framebuffers and kref about underflows.

v2: Kill intel_framebuffer_fini - no longer needed now that we
refcount all fbs properly and only confusing.

v3: We actually still need to call unregister_private to remove the fb
from the idr and drop the idr reference - the final unref doesn't do
that. So much for remembering my own fb liftime rules. Reported by
Imre Deak.

Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org> (v2)
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Provide a command line option to disable display
Damien Lespiau [Mon, 10 Feb 2014 17:20:55 +0000 (17:20 +0000)]
drm/i915: Provide a command line option to disable display

If we can't actually determine at run-time we have a fused-off display,
provide at least an option to disable it.

v2: Move the i915.disable_display test in a separate check
    (Daniel Vetter)

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Disable display when fused off
Damien Lespiau [Mon, 10 Feb 2014 17:19:45 +0000 (17:19 +0000)]
drm/i915: Disable display when fused off

FUSE_STRAP has a bit to inform us that the display has been fused off.
Use it to setup the definitive number of pipes at run-time.

v2: actually tweak num_pipes, not num_planes
v3: also tests SFUSE_STRAP bit 7
v4: rebase on top of drm-nightly
    use DRM_INFO() for the message telling display is fused off
    try to read the FUSE_LOCK bit to determine if PCH display is disabled
v5: Don't read SFUSE_STRAP (register on the PCH) if num_pipes is already 0
    from the initial device info struct (to prevent hangs) (Daniel Vetter)

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com> (for v3)
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> (for v3)
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Flush GPU rendering with a lockless wait during a pagefault
Chris Wilson [Fri, 7 Feb 2014 20:37:06 +0000 (18:37 -0200)]
drm/i915: Flush GPU rendering with a lockless wait during a pagefault

Arjan van de Ven reported that on his test machine that he was seeing
stalls of greater than 1 frame greatly impacting the user experience. He
tracked this down to being the locked flush during a pagefault as being
the culprit hogging the struct_mutex and so blocking any other user from
proceeding. Stalling on a pagefault is bad behaviour on userspace's
part, for one it means that they are ignoring the coherency rules on
pointer access through the GTT, but fortunately we can apply the same
trick as the set-to-domain ioctl to do a lightweight, nonblocking flush
of outstanding rendering first.

"Prior to the patch it looks like this
(this one testrun does not show the 20ms+ I've seen occasionally)

  4.99 ms     2.36 ms    31360  __wait_seqno i915_wait_seqno i915_gem_object_wait_rendering i915_gem_object_set_to_gtt_domain i915_gem_fault __do_fault handle_
+pte_fault handle_mm_fault __do_page_fault do_page_fault page_fault
   4.99 ms     2.75 ms   107751  __wait_seqno i915_gem_wait_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
   4.99 ms     1.63 ms     1666  i915_mutex_lock_interruptible i915_gem_fault __do_fault handle_pte_fault handle_mm_fault __do_page_fault do_page_fault page_fa
+ult
   4.93 ms     2.45 ms      980  i915_mutex_lock_interruptible intel_crtc_page_flip drm_mode_page_flip_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_
+sysret
   4.89 ms     2.20 ms     3283  i915_mutex_lock_interruptible i915_gem_wait_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
   4.34 ms     1.66 ms     1715  i915_mutex_lock_interruptible i915_gem_pwrite_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
   3.73 ms     3.73 ms       49  i915_mutex_lock_interruptible i915_gem_set_domain_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
   3.17 ms     0.33 ms      931  i915_mutex_lock_interruptible i915_gem_madvise_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
   2.97 ms     0.43 ms     1029  i915_mutex_lock_interruptible i915_gem_busy_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
   2.55 ms     0.51 ms      735  i915_gem_get_tiling drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret

After the patch it looks like this:

   4.99 ms     2.14 ms    22212  __wait_seqno i915_gem_wait_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
   4.86 ms     0.99 ms    14170  __wait_seqno i915_gem_object_wait_rendering__nonblocking i915_gem_fault __do_fault handle_pte_fault handle_mm_fault __do_page_
+fault do_page_fault page_fault
   3.59 ms     1.31 ms      325  i915_gem_get_tiling drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
   3.37 ms     3.37 ms       65  i915_mutex_lock_interruptible i915_gem_wait_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
   2.58 ms     2.58 ms       65  i915_mutex_lock_interruptible i915_gem_do_execbuffer.isra.23 i915_gem_execbuffer2 drm_ioctl i915_compat_ioctl compat_sys_ioctl
+ia32_sysret
   2.19 ms     2.19 ms       65  i915_mutex_lock_interruptible intel_crtc_page_flip drm_mode_page_flip_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_
+sysret
   2.18 ms     2.18 ms       65  i915_mutex_lock_interruptible i915_gem_busy_ioctl drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret
   1.66 ms     1.66 ms       65  i915_gem_set_tiling drm_ioctl i915_compat_ioctl compat_sys_ioctl ia32_sysret

It may not look like it, but this is quite a large difference, and I've
been unable to reproduce > 5 msec delays at all, while before they do
happen (just not in the trace above)."

gem_gtt_hog on an old Pineview (GMA3150),
before: 4969.119ms
after:  4122.749ms

Reported-by: Arjan van de Ven <arjan.van.de.ven@intel.com>
Testcase: igt/gem_gtt_hog
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: vlv: handle only enabled pipestat interrupt events
Imre Deak [Mon, 10 Feb 2014 16:42:49 +0000 (18:42 +0200)]
drm/i915: vlv: handle only enabled pipestat interrupt events

Atm we call the handlers for pending pipestat interrupt events even if
they aren't explicitly enabled by i915_enable_pipestat(). This isn't an
issue for events other than the vblank start event, since those are
always enabled anyways. Otoh, we enable the vblank start event
on-demand, so we'll end up calling the vblank handler at times when they
are disabled.

I haven't checked if this causes any real problem, but for consistency
and to remove some overhead we should still fix this by clearing /
handling only the enabled interrupt events. Also this is a dependency
for the upcoming VLV power domain patchset where we need to disable all
the pipestat interrupts whenever the display power well is off.

v2:
- inline the status->enable mask mapping (Ville)
- don't check for invalid PSR bit on platforms other than VLV (Ville)

Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
[danvet: Frob conflict due to different merge order.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: vlv: fix mapping of pipestat enable to status bits
Imre Deak [Mon, 10 Feb 2014 16:42:48 +0000 (18:42 +0200)]
drm/i915: vlv: fix mapping of pipestat enable to status bits

At least on VLV we can't get at the pipestat status bits by simply right
shifting the corresponding enable bits. The mapping between enable and
status bits for the sprite0,1 flip done and the PSR events don't follow
this rule, so we need to map them separately.

The PSR enable for pipe A is DPFLIPSTAT[22], but I haven't added support
for this, since there is no user of it atm. Until support is added WARN
if someone tries to enable PSR interrupts, or tries to enable the same
(1 << 6) bit on pipe B, which MBZ.

v2:
- inline the status->enable mask mapping (Ville)
- fix bogus use of status bits in enable mask (Ville)

Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: pass status instead of enable flags to i915_enable_pipestat
Imre Deak [Mon, 10 Feb 2014 16:42:47 +0000 (18:42 +0200)]
drm/i915: pass status instead of enable flags to i915_enable_pipestat

There isn't any PSR interrupt enable bit for pipe A, so we couldn't
enable it through the current API. Passing the corresponding status bits
solves this and also makes the mapping between enable and status bits
simpler on VLV (addressed in an upcoming patch).

Except of checking for invalid status bit arguments, no functional
change.

v2: split out the low level parts of i915_enable_pipestat accepting
    separate enabled and status masks, to make the non-standard mapping
    between those masks stand out more (added in the next patch)
    (Jesse,Daniel)

Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Short-circuit no-op vga_set_state()
Chris Wilson [Fri, 7 Feb 2014 20:37:03 +0000 (18:37 -0200)]
drm/i915: Short-circuit no-op vga_set_state()

Touching the VGA registers risks a hard machine hang, at least on this
ivb machine after removing a conflicting efifb. This is more than likely
related to the discovery that VGA IO decode on the more recent PCH
platforms is terminally broken.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Propagate PCI read/write errors during vga_set_state()
Chris Wilson [Fri, 7 Feb 2014 20:37:02 +0000 (18:37 -0200)]
drm/i915: Propagate PCI read/write errors during vga_set_state()

This has very little effect other than log the errors in case of failure,
and we then hope for the best.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: alloc intel_fb in the intel_fbdev struct
Jesse Barnes [Fri, 7 Feb 2014 20:10:38 +0000 (12:10 -0800)]
drm/i915: alloc intel_fb in the intel_fbdev struct

Allocate this struct instead, so we can re-use another allocated
elsewhere if needed.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
[danvet: WARN_ON if there's no backing storage attached to an fb,
that's a bug.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Downgrade *ERROR* message for invalid user input
Chris Wilson [Mon, 10 Feb 2014 09:03:50 +0000 (09:03 +0000)]
drm/i915: Downgrade *ERROR* message for invalid user input

When we detect that the user passed along an invalid handle or object,
we emit a warning as an aide for debugging. Since these are indeed only
for debugging user triggerable errors (and the errors are reported back
to userspace by the errno), the messages should only be at the debug
level and not claiming that there is a catastrophic error in the
driver/hardware.

References: https://bugs.freedesktop.org/show_bug.cgi?id=74704
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Reorder i915_params fields to not create holes
Damien Lespiau [Fri, 7 Feb 2014 19:12:53 +0000 (19:12 +0000)]
drm/i915: Reorder i915_params fields to not create holes

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Use I915_MAX_PIPES in the pipe/plane_to_crtc_mapping definitions
Damien Lespiau [Fri, 7 Feb 2014 19:12:52 +0000 (19:12 +0000)]
drm/i915: Use I915_MAX_PIPES in the pipe/plane_to_crtc_mapping definitions

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Consolidate FUSE_STRAP in one set of defines
Damien Lespiau [Fri, 7 Feb 2014 19:12:50 +0000 (19:12 +0000)]
drm/i915: Consolidate FUSE_STRAP in one set of defines

We had 2 set of defines for the same register, so make it one.

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Move num_plane to the intel_device_info structure
Damien Lespiau [Fri, 7 Feb 2014 19:12:49 +0000 (19:12 +0000)]
drm/i915: Move num_plane to the intel_device_info structure

And rename it to num_sprites as this value doesn't count the primary
plane.

This limit lives with num_pipes really, and now that dev_priv->info is
writable we can put it there instead.

While at it, introduce a intel_device_info_runtime_init() where we'll be
able to gather the device info fields at run-time.

v2: rename num_plane to num_sprites (Ville Syrjälä)
v3: rebase on top of latest drm-nightly

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com> (for v2)
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> (for v2)
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Make the intel_device_info structure kept in dev_priv writable
Damien Lespiau [Fri, 7 Feb 2014 19:12:48 +0000 (19:12 +0000)]
drm/i915: Make the intel_device_info structure kept in dev_priv writable

Turns out it'd be nice to change some device information at run-time or simply
have some code to fill in the info struct instead of having to declare the
values in 30+ structures.

What prompted this change is handling fused out display/pipe and tweaking
num_pipes at run-time, but I'm quite sure we'll find other flags/limits to
stick into dev_priv->info.

Most of the changes were done with a sed:
sed -i -e 's/dev_priv->info->/dev_priv->info./g' drivers/gpu/drm/i915/*[ch]

with a few tweaks to make it all work:
- Change the field definition in struct drm_i915_private
- adjust i915_dump_device_info()
- adjust i915_driver_load()
- adjust the INTEL_INFO() macro

v2: cast the info pointer returned by INTEL_INFO() to be const to catch
    uses that would modify the structure post-initialization.
    (Ville Syrjälä)

v3: Redo the patch onto latest drm-nightly,
    Keep the info field const to catch post initialization writes
    instead of the v2 solution,
    Use a direct structure copy for the initial info initialization to
    use the compiler type safety (Ville Syrjälä)

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com> (for v2)
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> (for v2)
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Always use INTEL_INFO() to access the device_info structure
Damien Lespiau [Fri, 7 Feb 2014 19:12:47 +0000 (19:12 +0000)]
drm/i915: Always use INTEL_INFO() to access the device_info structure

If we make sure that all the dev_priv->info usages are wrapped by
INTEL_INFO(), we can easily modify the ->info field to be structure and
not a pointer while keeping the const protection in the INTEL_INFO()
macro.

v2: Rebased onto latest drm-nightly

Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Disable SF pipelined attribute fetch for SNB
Ville Syrjälä [Tue, 4 Feb 2014 19:59:18 +0000 (21:59 +0200)]
drm/i915: Disable SF pipelined attribute fetch for SNB

According to Bspec we need to disable SF pipelined attribute fetch
whenever SF outputs exceed 16 and normal clip mode is used. A quick
glance at Mesa suggests that these conditions could happen. So let's
just always set the magic bit.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Update rps interrupt limits
Jeff McGee [Tue, 4 Feb 2014 17:37:01 +0000 (11:37 -0600)]
drm/i915: Update rps interrupt limits

sysfs changes to rps min and max delay were only triggering an update
of the rps interrupt limits if the active delay required an update.
This change ensures that interrupt limits are always updated.

v2: correct compile issue missed on rebase
v3: add igt testcases to signed-off-by section

Testcase: igt/pm_rps/min-max-config-idle
Testcase: igt/pm_rps/min-max-config-loaded
Signed-off-by: Jeff McGee <jeff.mcgee@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Restore rps/rc6 on reset
Jeff McGee [Tue, 4 Feb 2014 17:32:31 +0000 (11:32 -0600)]
drm/i915: Restore rps/rc6 on reset

A check of rps/rc6 state after i915_reset determined that the ring
MAX_IDLE registers were returned to their hardware defaults and that
the GEN6_PMIMR register was set to mask all interrupts. This change
restores those values to their pre-reset states by re-initializing
rps/rc6 in i915_reset. A full re-initialization was opted for versus
a targeted set of restore operations for simplicity and maintain-
ability. Note that the re-initialization is not done for Ironlake,
due to a past comment that it causes problems.

Also updated the rps initialization sequence to preserve existing
min/max values in the case of a re-init. We assume the values were
validated upon being set and do not do further range checking. The
debugfs interface for changing min/max was updated with range
checking to ensure this condition (already present in sysfs
interface).

v2: fix rps logging to output hw_max and hw_min, not rps.max_delay
    and rps.min_delay which don't strictly represent hardware limits.
    Add igt testcase to signed-off-by section.

Testcase: igt/pm_rps/reset
Signed-off-by: Jeff McGee <jeff.mcgee@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Prevent recursion by retiring requests when the ring is full
Chris Wilson [Mon, 27 Jan 2014 22:43:07 +0000 (22:43 +0000)]
drm/i915: Prevent recursion by retiring requests when the ring is full

As the VM do not track activity of objects and instead use a large
hammer to forcibly idle and evict all of their associated objects when
one is released, it is possible for that to cause a recursion when we
need to wait for free space on a ring and call retire requests.
(intel_ring_begin -> intel_ring_wait_request ->
i915_gem_retire_requests_ring -> i915_gem_context_free ->
i915_gem_evict_vm -> i915_gpu_idle -> intel_ring_begin etc)

In order to remove the requirement for calling retire-requests from
intel_ring_wait_request, we have to inline a couple of steps from
retiring requests, notably we have to record the position of the request
we wait for and use that to update the available ring space.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: Generate a hang error code
Ben Widawsky [Tue, 4 Feb 2014 12:18:55 +0000 (12:18 +0000)]
drm/i915: Generate a hang error code

We get a large number of bugs which have a, "hey I have that too"
because they see a GPU hang in dmesg. While two machines of the same
model having a GPU hang is indeed a coincidence, it is far from enough
evidence to suggest they are the same.

In order to reduce this effect, and hopefully get people to file new bug
reports, clearly the error message itself has been insufficient (see ref
at the bottom for a new bug report with this characteristic).

The algorithm is purposely pretty naive. I don't think we need much in
order to avoid the problem I am trying to solve, and keeping it naive
gives us some ability to make a decent test case.

Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
References: https://bugs.freedesktop.org/show_bug.cgi?id=73276
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: unify FLIP_DONE macro names
Imre Deak [Tue, 4 Feb 2014 19:35:48 +0000 (21:35 +0200)]
drm/i915: unify FLIP_DONE macro names

s/FLIPDONE/FLIP_DONE/ to make all FLIP_DONE macro names consistent.

No functional change.

Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: vlv: s/spin_lock_irqsave/spin_lock/ in irq handler
Imre Deak [Tue, 4 Feb 2014 19:35:47 +0000 (21:35 +0200)]
drm/i915: vlv: s/spin_lock_irqsave/spin_lock/ in irq handler

Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: factor out valleyview_pipestat_irq_handler
Imre Deak [Tue, 4 Feb 2014 19:35:46 +0000 (21:35 +0200)]
drm/i915: factor out valleyview_pipestat_irq_handler

This will be used by other platforms too, so factor it out.

The only functional change is the reordeing of gmbus_irq_handler() wrt.
the hotplug handling, but since it only schedules a work, it isn't an
issue.

Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
[danvet: Don't keep on using the private_t typedef.]
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
10 years agodrm/i915: vlv: don't unmask IIR[DISPLAY_PIPE_A/B_VBLANK] interrupt
Imre Deak [Tue, 4 Feb 2014 19:35:45 +0000 (21:35 +0200)]
drm/i915: vlv: don't unmask IIR[DISPLAY_PIPE_A/B_VBLANK] interrupt

Bspec and the code suggests that the interrupt signaled by IIR[7,5]
(DISPLAY_PIPE_A/B_VBLANK) is a first level IRQ flag for the second
level PIPEA/BSTAT[2] (Start of Vertical Blank) interrupt. Measuring
the relative timings of when IIR[7] and PIPEASTAT[1,2] get set and
checking the effect of unmasking different pipestat and IIR events
shows that this isn't so:

First, ISR/IIR[7] gets set independently of PIPEASTAT[18] (Start of
Vertical Blank Enable) or any other pipestat enable bit, so it isn't
a first level IRQ bit showing the state of PIPEASTAT[2], but is
connected directly to the timing generator.

Second, setting only PIPEASTAT[18] and leaving all other pipestat events
disabled, IIR[6] (DISPLAY_PIPE_A_EVENT) gets set close to the moment when
PIPEASTAT[2] gets set, so the former is a first level interrupt flag for
the latter. The bspec is rather unclear about this, but I also assume
that IIR[6] signals all pipestat A events, except PIPEASTAT[31] (FIFO
Under-run Status).

Third, IIR[7] is set close to the moment when PIPEASTAT[1] (Framestart
Interrupt) gets set, in the mode I used about 12usec after PIPEASTAT[2]
and IIR[6] gets set. This means the IIR[7] isn't marking the start of
vblank, but rather signals the framestart event.

Based on the above, we don't need to unmask IIR[7] when waiting for
start of vblank events, but we can rely on IIR[6] being always unmasked,
which will signal when PIPEASTAT[2] gets set. Doing this will also get
rid of the overhead of getting an interrupt and servicing IIR[7], which
is atm raised always some time after IIR[6]/PIPEASTAT[2] is raised.

Signed-off-by: Imre Deak <imre.deak@intel.com>
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>