- 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 <daniel.vetter@ffwll.ch>
-
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:
Damien Lespiau <damien.lespiau@intel.com> 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:
Ben Widawsky <ben@bwidawsk.net> Signed-off-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:
Paulo Zanoni <paulo.r.zanoni@intel.com> 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 8 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:
-
Ben Widawsky authored
For an upcoming patch where we introduce the i915 VMA, it's ideal to have the drm_mm_node as part of the VMA struct (ie. it's pre-allocated). Part of the conversion to VMAs is to kill off obj->gtt_space. Doing this will break a bunch of code, but amongst them are 2 callers of drm_mm_create_block(), both related to stolen memory. It also allows us to embed the drm_mm_node into the object currently which provides a nice transition over to the new code. v2: Reordered to do before ripping out obj->gtt_offset. Some minor cleanups made available because of reordering. v3: s/continue/break on failed stolen node allocation (David) Set obj->gtt_space on failed node allocation (David) Only unref stolen (fix double free) on failed create_stolen (David) Free node, and NULL it in failed create_stolen (David) Add back accidentally removed newline (David) CC: <dri-devel@lists.freedesktop.org> Reviewed-by:
David Herrmann <dh.herrmann@gmail.com> Signed-off-by:
-
- 05 Jul, 2013 4 commits
-
-
Daniel Vetter authored
Just to keep the paranoia equal also sprinkle locking asserts over the pipestat interrupt enable/disable functions. Again this results in false positives in the interrupt setup. Add bogo-locking for these and a big comment explaining why it's there and that it's indeed unnecessary. v2: Fix up the spelling fail Paulo spotted in comments. Reviewed-by:
-
Daniel Vetter authored
A magic -1 is a obscure, especially since it's actually passed as an unsigned, so depends upon the magic sign extension rules in C. This has been added in commit 3727d55e Author: Jesse Barnes <jbarnes@virtuousgeek.org> Date: Wed May 8 10:45:14 2013 -0700 drm/i915: allow stolen, pre-allocated objects to avoid GTT allocation v2 Use a proper #define instead. Spotted while reviewing Ben's drm_mm_create_block changes. v2: Cast the constant to u32 since otherwise we again have a type mismatch. Suggested by Chris Wilson. Cc: Ben Widawsky <ben@bwidawsk.net> Cc: Jesse Barnes <jbarnes@virtuousgeek.org> Reviewed-by:
-
Daniel Vetter authored
We only do this on IBX where there's a fixed pch dpll to pipe assignment. Being explicit about it can't really hurt and makes sparse happy. Reviewed-by:
-
Daniel Vetter authored
This makes sparse happy and also makes it a bit more obvious where we pull off this trick - after all we're only allowed to do it eithe as a default or on platforms where there is no disdinction between the pipe and the cpu transcoder. Reviewed-by:
-
- 02 Jul, 2013 2 commits
-
-
Ville Syrjälä authored
Use wait_for() instead of the open coded loop to avoid spreading the same old timeout related bugs. This changes the loop to use msleep(1) instead of udelay(10) when the Punit had not yet completed the frequency change. In practice that doesn't seem to hurt performance as the Punit appears to be ready pretty much always. Also give the status bit a name, instead of using the magic number 1. Signed-off-by:
-
Jesse Barnes authored
This should help on HSW, where we don't currently have a get_clock call. Reported-by:
Paulo Zanoni <przanoni@gmail.com> Signed-off-by:
-
- 01 Jul, 2013 4 commits
-
-
Jesse Barnes authored
Print out the flag that failed and fix up a mismatched paren. Signed-off-by:
-
Mika Kuoppala authored
As getting error state doesn't anymore require big kmallocs, make error state accessible also from sysfs. v2: - error state clearing (Chris Wilson) - user hint, proper access mode bits and name (Daniel Vetter) v3: release resources in proper order (Chris Wilson) Suggested-by: -
Mika Kuoppala authored
Make function for struct i915_error_state_buf initialization and export it, for sysfs and debugfs. Signed-off-by:
-
Mika Kuoppala authored
In preparation for sysfs error state access, export ref error state ref counting interface. Signed-off-by:
-
