- 16 Jul, 2013 11 commits
-
-
Damien Lespiau authored
v2: Put the comment a bit closer to the actual write (Paulo Zanoni) Reviewed-by:
Paulo Zanoni <paulo.r.zanoni@intel.com> Signed-off-by:
Damien Lespiau <damien.lespiau@intel.com> [danvet: Fix space before tab.] Signed-off-by:
Daniel Vetter <daniel.vetter@ffwll.ch>
-
Damien Lespiau authored
We also wait for that blank on other platforms but the w/a doesn't apply there. Not an issue at all. Signed-off-by:
-
Daniel Vetter authored
At the moment we have the following interrupt enabling sequence: 1. irq preinstall hook 2. enabling the interrupt handler and calling irq postinstall hook 3. enable rps interrupts from the async work And the folliwing disable sequence: 1. disabling the interrupt handler and calling the uninstall hook 2. disabling the rps interrupt Since the postinstall hook now always sets up PMIIR, PMIER and PMIMR to known-good states there no way for an interrupt to sneak in in the enable sequence, so we can reinstate the WARN lost in commit eda63ffb Author: Ben Widawsky <ben@bwidawsk.net> Date: Tue May 28 19:22:26 2013 -0700 drm/i915: Add PM regs to pre/post install Note that there's some room for future cleanups since most of the interrupt register clearing in the disable function is rather redundant. But that's better done in follow-up patches, if at all. Cc: Ben Widawsky <ben@bwidawsk.net> Reviewed-by:
Ben Widawsky <ben@bwidawsk.net> Signed-off-by:
-
Daniel Vetter authored
The VECS enabling required some changes to how rps interrupts are enabled/disabled since VECS interrupts are handling with the PM interrupt registers. But now that the pre/postinstall sequences is identical for all platforms with rps support (snb, ivb, hsw, vlv) we can also use the exact same sequence to actually enable the rps interrupts. Strictly speaking using spinlocks is overkill on snb/ivb & vlv since they have no VECS ring, but imo that's more than made up by the common code. Hence this just unifies the vlv code with the snb-hsw code which matched exactly before the VECS enabling. See commit eda63ffb Author: Ben Widawsky <ben@bwidawsk.net> Date: Tue May 28 19:22:26 2013 -0700 drm/i915: Add PM regs to pre/post install and commit 4848405c Author: Ben Widawsky <ben@bwidawsk.net> Date: Tue May 28 19:22:27 2013 -0700 drm/i915: make PM interrupt writes non-destructive for why the gen6 code (shared between snb, ivb and hsw) needed to be changed originally. v3: Improve the commit message to more clearly spell out why we want to unify the code and what exactly changes. Cc: Paulo Zanoni <przanoni@gmail.com> Cc: Ben Widawsky <ben@bwidawsk.net> Reviewed-by:
-
Daniel Vetter authored
Again extract a common helper. For the postinstall hook things are a bit more complicated since we have more cases on ilk-hsw/vlv here. But since vlv was clearly broken by failing to initialize dev_priv->gt_irq_mask correctly the shared code is clearly justified. Also kill the PMIER setting in the async rps enable work. I should have been save, but also clearly looked rather fragile. PMIER setup is now all down in the irq pre/postinstall hooks. With this we now have the usual interrupt register sequence for GT/PM irq registers: - IER is setup once with all the interrupts we ever need in the postinstall hook and never touched again. Exceptions are SDEIER, which is touched in the preinstall hook (when the irq handler isn't enabled) and then only from the irq handler. And DEIER/VLV_IER with is used in the irq handler but also written to once in the postinstall hook. But since that write is essentially what enables the interrupt and we should always have MSI interrupts we should be save. In case we ever have non-MSI interrupts we'd be screwed. - IIR is cleared in the postinstall hook before we enable/unmask the respective interrupt sources. Hence we can't steal an interrupt event an accidentally trigger the spurious interrupt logic in the core kernel. Note that after some discussion with Ben Widawsky we think that we actually should clear the IIR registers in the preinstall hook. But doing that is a much larger patch series. - IMR regs are (usually) all masked off. Those are the only regs changed at runtime, which is all protected by dev_priv->irq_lock. This unification also kills the cargo-culted read-modify-write PM register setup for VECS. Interrupt setup is done without userspace being able to interfere, so we better know what values we want to put into those registers. RMW cycles otoh are really good at papering over races, until stuff magically blows up and no one has a clue why. v2: Touch the gen6+ PM interrupt registers only on gen6+. v3: Improve the commit message to more clearly spell out why we want to unify the code and what exactly changes. Cc: Ben Widawsky <ben@bwidawsk.net> Cc: Paulo Zanoni <przanoni@gmail.com> Reviewed-by:
-
Daniel Vetter authored
Since the addition of VECS we have a slightly different enable sequence for PM interrupts on ivb/hsw vs snb and vlv. Usually that will end up in hard to track down surprises. Hence unifiy things and since we have copies of this code in 3 places now, extract it into its own little helper. Note that this changes the irq preinstall sequence a bit for snb and vlv: We now also clear the PM registers in the preinstall hook, in addition to the PM register clearing/setup already done when actually enabling rps. So this doesn't fix a bug but simply unifies the code across all platforms. After the postinstall hook is similarly unified we can rip out the then redundant PM interrupt setup from the rps code. v3: Rebase on top of the retained double-GTIIR clearing. Also resurrect the masking/disabling of the gen6+ PM interrupts as spotted by Ben Widaswky. v4: Move the DE interrupt reset code out of gen5_gt_irq_preinstall back to ironlake_irq_preinstall where it really belongs. Spotted by Paulo. v3: Improve the commit message to more clearly spell out why we want to unify the code and what exactly changes. Cc: Paulo Zanoni <przanoni@gmail.com> Reviewed-by:
-
Ben Widawsky authored
To make users life a little easier figuring out what they have on their system. Ideally, I'd really like to report LLC size, but it turned out to be a bit of a pain. Maybe I'll revisit it in the future. Signed-off-by:
Rodrigo Vivi <rodrigo.vivi@gmail.com> Signed-off-by:
-
Ben Widawsky authored
DRI clients really should be using MOCS to get fine grained streaming cache controls. With that note, I *hope* that this patch doesn't improve performance overwhelmingly, because if it does - it means there is a problem elsewhere. In any case, the kernel, and old userspace should get some benefit from this, so let's do it. eLLC is always a good default, and really not using it is the special case for MOCS. References: http://www.intel.com/newsroom/kits/restricted/ha$well!/pdfs/4th_Gen_Intel_Core_PressBriefing_5-29.pdf (page 57) Signed-off-by:
-
Ben Widawsky authored
The eLLC cannot be determined by PCIID because as far as we know, even machines supporting eLLC may not have it enabled, or fused off or whatever. It's possible this isn't actually true, and at that point we can switch to a DEV_INFO flag instead. I've defined everything where the docs are clear, and left the rest as magic. But we need it before we set the pte_encode function pointers, which happens really early, in gtt_init. The problem with just doing the normal sequence earlier is we don't have the ability to use forcewake until after the pte functions have been set up. Since all solutions are somewhat ugly (barring rewriting all the init ordering), I've opted to do the detection really early, and the enabling later - since the register to detect doesn't require forcewake. Signed-off-by:
-
Ben Widawsky authored
The EDRAM present register isn't really defined in the docs. It just says check to see if it's set to 1. So I haven't defined the 1 value not knowing what it actually means. Signed-off-by:
-
Ben Widawsky authored
The cacheability controls have changed, and the bits have been rearranged in general. Note that age 0 is the oldest (most likely to get evicted) and age 3 is the youngest (most likely to stick around for a bit). We've picked 0 for no reason, but atm it shouldn't matter anyway (since we don't yet try to differentiate between different objects). v2: Remove comments for snb/ivb cache leves, that's a separate change. v3: Resolve conflicts due to patch series reordering. v4: Rebased on top of Kenneth Graunke's ->pte_encode refactoring. v5: Removed eLLC bits for separate patch. In the internal repository this was: Signed-off-by:
Kenneth Graunke <kenneth@whitecape.org> Reviewed-by:
-
- 12 Jul, 2013 5 commits
-
-
Daniel Vetter authored
Toghether with the hw state readout this should catch cases where we don't properly updated the pll state (either in sw or hw). At least for the shared dpll code the equivalent tricke helped a lot in catching bugs. Also rename the function prefix, it's not a generic piece of infrastructure. Reviewed-by:
Imre Deak <imre.deak@intel.com> Signed-off-by:
-
Daniel Vetter authored
Bspec for the "DPLL HDMI multiplier" field says: "Restriction : The DPLL must be enabled and stable before setting these bits. These bits must be programmed after DPLL_SEL is programmed." There is apparently no restriction on programming the DPLL_SEL register wrt the DPLL. So let's just move that up before we enable the pch dpll. Reviewed-by:
-
Daniel Vetter authored
No need to call the ->pre_pll_enable hook twice if we don't enable the dpll too early. This should make Jani a bit less grumpy. v2: Rebase on top of the newly-colored BUG_ONs. v3: Reinstate the lost write of the DPLL_MD register, spotted by Imre. Cc: Imre Deak <imre.deak@intel.com> Cc: Jani Nikula <jani.nikula@intel.com> Reviewed-by:
-
Mika Kuoppala authored
Move error state generation and stringification to it's own compilation unit. Sysfs also uses this so it can't be under CONFIG_DEBUG_FS This fixes a regression introduced in commit ef86ddce Author: Mika Kuoppala <mika.kuoppala@linux.intel.com> Date: Thu Jun 6 17:38:54 2013 +0300 drm/i915: add error_state sysfs entry Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=66814Signed-off-by:
Mika Kuoppala <mika.kuoppala@intel.com> Reported-by:
kbuild test robot <fengguang.wu@intel.com> Signed-off-by:
-
Damien Lespiau authored
If intel_sdvo_get_value() fails here, val is unitialized and the cross check will compare the pipe config multiplier with a bogus value. Instead, only set encoder_pixel_multiplier when the sdvo command has been successful. The cross check will compare the pipe config value with 0 otherwise. v2: Do the cross check with the initial value of encoder_pixel_multiplier (0) if the sdvo command fails (and thus keep the warning) (Daniel Vetter) Signed-off-by:
-
- 11 Jul, 2013 11 commits
-
-
Damien Lespiau authored
Came accross two open coding of for_each_pipe(), might as well use the macro. Signed-off-by:
-
Daniel Vetter authored
The code to handle it is broken - there's simply no code to clear CS parser errors on gen5+. And behold, for all the other rings we also don't enable it! Leave the handling code itself in place just to be consistent with the existing mess though. And in case someone feels like fixing it all up. This has been errornously enabled in commit 12638c57 Author: Ben Widawsky <ben@bwidawsk.net> Date: Tue May 28 19:22:31 2013 -0700 drm/i915: Enable vebox interrupts Cc: Damien Lespiau <damien.lespiau@intel.com> Cc: Ben Widawsky <ben@bwidawsk.net> Reviewed-by:
-
Daniel Vetter authored
With the simplified locking there's no reason any more to keep the refcounts seperate. v2: Readd the lost comment that ring->irq_refcount is protected by dev_priv->irq_lock. Reviewed-by:
-
Daniel Vetter authored
Now that the rps interrupt locking isn't clearly separated (at elast conceptually) from all the other interrupt locking having a different lock stopped making sense: It protects much more than just the rps workqueue it started out with. But with the addition of VECS the separation started to blurr and resulted in some more complex locking for the ring interrupt refcount. With this we can (again) unifiy the ringbuffer irq refcounts without causing a massive confusion, but that's for the next patch. v2: Explain better why the rps.lock once made sense and why no longer, requested by Ben. Reviewed-by:
-
Daniel Vetter authored
And kill the comment about it. Queueing work is a barrier type event, no amount of locking will help in ordering things (as long as we queue the work after having updated all relevant data structures). Also, the queue_work works itself as a sufficient memory barrier. Again on the surface this is just a tiny micro-optimization to reduce the hold-time of dev_priv->irq_lock. But the better reason is that it reduces superficial locking and so makes it clearer what we actually need for correctness. Reviewed-by:
-
Daniel Vetter authored
The if (pm_iir & ~GEN6_PM_RPS_EVENTS) check was redunandant. Otoh adding a check for rps events allows us to avoid the spinlock grabbing for VECS interrupts. v2: Drop misplaced hunk which now moved to the right patch. Reviewed-by:
-
Daniel Vetter authored
Since we only have one interrupt handler and interrupt handlers are non-reentrant. To drive the point really home give them all an _irq_handler suffix. This is a tiny micro-optimization but even more important it makes it clearer what locking we actually need. And in case someone screws this up: lockdep will catch hardirq vs. other context deadlocks. v2: Fix up compile fail. Reviewed-by:
-
Daniel Vetter authored
It's racy: There's no guarantee that we won't walk this code (due to a pch fifo underrun interrupt) while someone is changing the pointers around. The only reason we do this is to use the righ crtc for the pch fifo underrun accounting. But we never expose this to userspace, so essentially no one really cares if we use the "wrong" crtc. So let's just rip it out. With this patch fifo underrun code will always use crtc A for tracking underruns on the (only) pch transcoder on LPT. v2: Add a big comment explaining what's going on. Requested by Paulo. v3: Fixup spelling in comment as spotted by Paulo. Cc: Paulo Zanoni <przanoni@gmail.com> Reviewed-by:
-
Daniel Vetter authored
Same treatment as for SERR_INT: If we clear only the bit for the pipe we're enabling (but unconditionally) then we can always check for possible underruns after having disabled the interrupt. That way pipe underruns won't be lost, but at worst only get reported in a delayed fashion. v2: The same logic bug as in the SERR handling change also existed here. The same bugfix of only reporting missed underruns when the error interrupt was masked applies, too. v3: Do the same fixes as for the SERR handling that Paulo suggested in his review: - s/%i/%c/ fix in the debug output - move the DE_ERR_INT_IVB read into the respective if block Cc: Paulo Zanoni <przanoni@gmail.com> Reviewed-by:
-
Daniel Vetter authored
The current code won't report any fifo underruns on cpt if just one pipe has fifo underrun reporting disabled. We can't enable the interrupts, but we can still check the per-transcoder bits and so report the underrun delayed if: - We always clear the transcoder's bit (and none of the other bits) when enabling. - We check the transcoder's bit after disabling (to avoid racing with the interrupt handler). v2: I've forgotten to actually remove the old SERR_INT clearing. v3: Use transcoder_name as suggested by Paulo Zanoni. Paulo also noticed a logic bug: When an underrun interrupt fires we report it both in the interrupt handler and when checking for underruns when disabling it in cpt_set_fifo_underrun_reporting. But that second check is only required if the interrupt is disabled and we're switching of underrun reporting (e.g. because we're disabling the crtc). Hence check for that condition. At first I wanted to rework the code to pass that bit of information from the uppper functions down to cpt_set_fifo_underrun_reporting. But that turned out too messy. Hence the quick&dirty check whether the south error interrupt source is masked off or not. v4: Streamline the control flow a bit. v5: s/pipe/pch transcoder/ in the dmesg output, suggested by Paulo. v6: Review from Paulo: - Reorder the was_enabled assignment to only read the register when we need it. Also add a comment that we need to do that before updating the register. - s/%i/%c/ fix for the debug output. - Fix the checkpath complaint in the SERR_INT_TRANS_FIFO_UNDERRUN #define. v7: Hopefully put that elusive SERR hunk back into this patch, spotted by Paulo. Cc: Paulo Zanoni <przanoni@gmail.com> Reviewed-by:
-
Daniel Vetter authored
This way all changes to SDEIMR all go through the same function, with the exception of the (single-threaded) setup/teardown code. For paranoia again add an assert_spin_locked. v2: For even more paranoia also sprinkle a spinlock assert over cpt_can_enable_serr_int since we need to have that one there, too. v3: Fix the logic of interrupt enabling, add enable/disable macros for the simple cases in the fifo code and add a comment. All requested by Paulo. Reviewed-by:
-
- 10 Jul, 2013 2 commits
-
-
Maarten Lankhorst authored
Signed-off-by:
Maarten Lankhorst <maarten.lankhorst@canonical.com> Signed-off-by:
-
Daniel Vetter authored
In kernel modeset driver mode we're in full control of the chip, always. So there's no need at all to set mm.suspended in i915_gem_idle. Hence move that out into the leavevt ioctl. Since i915_gem_idle doesn't suspend gem any more we can also drop the re-enabling for KMS in the thaw function. Also clean up the handling of mm.suspend at driver load by coalescing all the assignments. Stumbled over while reading through our resume code for unrelated reasons. v2: Shovel mm.suspended into the (newly created) ums dungeon as suggested by Chris Wilson. The plan is that once we've completely stopped relying on the register save/restore code we could shovel even that in there. v3: Improve the locking for the entervt/leavevt ioctls a bit by moving the dev->struct_mutex locking outside of i915_gem_idle. Also don't clear dev_priv->ums.mm_suspended for the kms case, we allocate it with kzalloc. Both suggested by Chris Wilson. Cc: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> (v2) Signed-off-by:
-
- 09 Jul, 2013 4 commits
-
-
Ville Syrjälä authored
I just got confirmation that we're using some old values for the PLL LPF coefficients for DP RBR/HDMI/DAC on VLV. The VLV2A0_DP_eDP_HDMI_DPIO_driver_vbios_notes_9 document lists both values by mistake, and apparently we had picked the wrong one. Change the coefficients to the recommended values. Changing the value doesn't appear to destabilize the VGA output picture even with my sensitive HP ZR24w display. Also HDMI output to my TV still works fine. Signed-off-by:
Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by:
Jesse Barnes <jbarnes@virtuousgeek.org> Signed-off-by:
-
Daniel Vetter authored
v2: Bail out if we hit the WARN_ON to avoid fallout later on. Spotted by Chris Wilson. Suggested-by:
Chris Wilson <chris@chris-wilson.co.uk> Cc: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by:
-
Daniel Vetter authored
Originally I've thought that this fixes up the reset issues on my gm45, but that was just a red herring due to b0rked testing. Still I much prefer writing the right values (all other fields are reserved) instead of potentially dragging gunk around. Hence also clear the register to 0 after a reset. Note that Cspec is a bit confused and doesn't explicitly say that all the other bits in this register are "reserved, mbz" like usually. Instead they're marked as "r/o, default value = 0" which semantically amounts to the same thing. v2: Stop claiming this fixes anything and return 0 if successful instead of stack garbage. v3: Pimp the commit message to explain exactly why I think the docs allow us to ditch the rmw cycle, spurred by a discussion with Chris. Cc: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by:
-
Chris Wilson authored
Sanity check that the memory region found through the Graphics Base of Stolen Memory is reserved and hidden from the rest of the system through the use of the resource API. v2: "Graphics Stolen Memory" is such a more bodacious name than the lame "i915 stolen", and convert to using devres for automagical cleanup of the resource. (danvet) Signed-off-by:
-
- 08 Jul, 2013 7 commits
-
-
Daniel Vetter authored
At least for the common cases where we only need special file operations. The forcewake file is still rather more special. v2: Fix up the debugfs unregister code. v3: Actually squash in the right fixup. Acked-by:
-
Daniel Vetter authored
Section 1.5.4, "DPLL A Control Register" from Bspec about bit 23 "FPA0/A1 P2 Clock Divide": 0 = Divide by 2 1 = Divide by 4. This bit must be set in DVO non-gang mode So copy the current limits (which should be good for i8xx) and create a new set for dvo encoders. Reviewed-by:
Chris Wilson <chris@chris-wilson.oc.uk> Signed-off-by:
-
Daniel Vetter authored
I've missed that intel_dvo_mode_set changes the dpll configuration. Hence when I've reworked the sequence to only enable the dpll in the crtc_enable callback in commit 66e3d5c0 Author: Daniel Vetter <daniel.vetter@ffwll.ch> Date: Sun Jun 16 21:24:16 2013 +0200 drm/i915: move i9xx dpll enabling into crtc enable function that special DVO bit was lost. Some BSpec reading confirms that it's only needed for DVO encoders. Section 1.5.4, "DPLL A Control Register" for bit 30: "2X Clock Enable. When driving In non-gang DVO modes such as a connected flat panel or TV, a 2X" version of the clock is needed. When not using the 2X output it should be disabled. This bit cannot be set when driving the integrated LVDS port on devices such as Montara-GM." Fix this regression up. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=66516 Cc: Chris Wilson <chris@chris-wilson.co.uk> Reported-by:
-
Ben Widawsky authored
Embedding the node in the obj is more natural in the transition to VMAs which will also have embedded nodes. This change also helps transition away from put_block to remove node. Though it's quite an uncommon occurrence, it's somewhat convenient to not fail at bind time because we cannot allocate the node. Though in practice there are other allocations (like the request structure) which would probably make this point not terribly useful. Quoting Daniel: Note that the only difference between put_block and remove_node is that the former fills up the preallocation cache. Which we don't need anyway and hence is just wasted space. v2: Clean up the stolen preallocation code. Rebased on the reserve_node patches renames ggtt_ stuff to gtt_ stuff WARN_ON if the object is already bound (which doesn't mean it's in the bound list, tricky) Signed-off-by:
-
Ben Widawsky authored
With the getters in place from the previous patch this members serves no purpose other than saving one spare pointer chase, which will be killed in the next patch anyway. Moving to VMAs, this members adds unnecessary confusion since an object may exist at different offsets in different VMs. v2: Properly preserve the stolen offset. This code is a bit hacky but it all goes away when we embed the drm_mm_node and removes the need for the incorrect patch I submitted previously: "Use gtt_space->start for stolen reservation" Signed-off-by:
-
Ben Widawsky authored
Soon we want to gut a lot of our existing assumptions how many address spaces an object can live in, and in doing so, embed the drm_mm_node in the object (and later the VMA). It's possible in the future we'll want to add more getter/setter methods, but for now this is enough to enable the VMAs. v2: Reworked commit message (Ben) Added comments to the main functions (Ben) sed -i "s/i915_gem_obj_set_color/i915_gem_obj_ggtt_set_color/" drivers/gpu/drm/i915/*.[ch] sed -i "s/i915_gem_obj_bound/i915_gem_obj_ggtt_bound/" drivers/gpu/drm/i915/*.[ch] sed -i "s/i915_gem_obj_size/i915_gem_obj_ggtt_size/" drivers/gpu/drm/i915/*.[ch] sed -i "s/i915_gem_obj_offset/i915_gem_obj_ggtt_offset/" drivers/gpu/drm/i915/*.[ch] (Daniel) v3: Rebased on new reserve_node patch Changed DRM_DEBUG_KMS to actually work (will need fixing later) Signed-off-by:
-
Ben Widawsky authored
With the previous patch we no longer actually create a node, we simply find the correct hole and occupy it. This very well could have been squashed with the last patch, but since I already had David's review, I figured it's easiest to keep it distinct. Also update the users in i915. Conveniently this is the only user of the interface. CC: David Airlie <airlied@linux.ie> CC: <dri-devel@lists.freedesktop.org> Signed-off-by:
David Airlie <airlied@linux.ie> Signed-off-by:
-
