Re: [Intel-gfx] Backporting drm/i915: force full modeset if the connector is in DPMS OFF mode
On Sun, Jun 09, 2013 at 11:18:16AM +0200, Daniel Vetter wrote: Hi stable maintainers, Please backport commit e3de42b68478a8c95dd27520e9adead2af9477a5 Author: Imre Deak imre.d...@intel.com Date: Fri May 3 19:44:07 2013 +0200 drm/i915: force full modeset if the connector is in DPMS OFF mode to all supported stable kernels. Additional bugzilla on top of all the others: Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=65559 Is that a new record for the most number of Bugzilla: tags in a single patch? :) Now applied to the 3.9-stable queue, it wouldn't apply to 3.4 or 3.0. thanks, greg k-h ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: fix build warning on format specifier mismatch
On Fri, Jun 07, 2013 at 04:03:50PM +0300, Jani Nikula wrote: drivers/gpu/drm/i915/i915_gem.c: In function ‘i915_gem_object_bind_to_gtt’: drivers/gpu/drm/i915/i915_gem.c:3002:3: warning: format ‘%ld’ expects argument of type ‘long int’, but argument 5 has type ‘size_t’ [-Wformat] v2: Use %zu instead of %d. Two char patch, and 100% wrong. (Ville) Signed-off-by: Jani Nikula jani.nik...@intel.com Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/i915_gem.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 58048d4..c2b60a4 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2999,7 +2999,7 @@ i915_gem_object_bind_to_gtt(struct drm_i915_gem_object *obj, * before evicting everything in a vain attempt to find space. */ if (obj-base.size gtt_max) { - DRM_ERROR(Attempting to bind an object larger than the aperture: object=%zd %s aperture=%ld\n, + DRM_ERROR(Attempting to bind an object larger than the aperture: object=%zd %s aperture=%zu\n, obj-base.size, map_and_fenceable ? mappable : total, gtt_max); -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PULL] drm-intel-fixes
Hi Dave, Just tiny regression fixes here: - Two fixes to fix sdvo hotplug which broke in the hpd storm detection work. - One fix to patch-up the sdvo lvds regression fixer from the last pull - we need to prefer the vbt mode over edid modes. IMPORTANT: The sdvo lvds fix in this -fixes pull commit c3456fb3e4712d0448592af3c5d644c9472cd3c1 Author: Daniel Vetter daniel.vet...@ffwll.ch Date: Mon Jun 10 09:47:58 2013 +0200 drm/i915: prefer VBT modes for SVDO-LVDS over EDID has a silent functional conflict with commit 990256aec2f10800595dddf4d1c3441fcd6b2616 Author: Ville Syrjälä ville.syrj...@linux.intel.com Date: Fri May 31 12:17:07 2013 + drm: Add probed modes in probe order in drm-next. Please poke me when you do a backmerge so that I can supply the fixup (we simply need to add the vbt modes before edid modes, i.e. the other way round than now). Cheers, Daniel The following changes since commit 317ddd256b9c24b0d78fa8018f80f1e495481a10: Linux 3.10-rc5 (2013-06-08 17:41:04 -0700) are available in the git repository at: git://people.freedesktop.org/~danvet/drm-intel tags/drm-intel-fixes-2013-06-11 for you to fetch changes up to c3456fb3e4712d0448592af3c5d644c9472cd3c1: drm/i915: prefer VBT modes for SVDO-LVDS over EDID (2013-06-10 10:13:34 +0200) Chris Wilson (2): drm/i915: Fix hotplug interrupt enabling for SDVOC drm/i915: Enable hotplug interrupts after querying hw capabilities. Daniel Vetter (1): drm/i915: prefer VBT modes for SVDO-LVDS over EDID drivers/gpu/drm/i915/intel_sdvo.c | 24 ++-- 1 file changed, 14 insertions(+), 10 deletions(-) -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/4] drm/i915: Only slightly increment hangcheck score if we succesfully kick a ring
On Mon, Jun 10, 2013 at 11:20:20AM +0100, Chris Wilson wrote: After kicking a ring, it should be free to make progress again and so should not be accused of being stuck until hangcheck fires once more. In order to catch a denial-of-service within a batch or across multiple batches, we still do increment the hangcheck score - just not as severely so that it takes multiple kicks to fail. This should address part of Ben's justified criticism of commit 05407ff889ceebe383aa5907219f86582ef96b72 Author: Mika Kuoppala mika.kuopp...@linux.intel.com Date: Thu May 30 09:04:29 2013 +0300 drm/i915: detect hang using per ring hangcheck_score There's also another corner case on the kick. If the seqno = 2 (though not stuck), and on the 3rd hangcheck, the ring is stuck, and we try to kick it... we don't actually try to find out if the kick helped. v2: Make sure we catch DoS attempts with batches full of invalid WAITs. v3: Preserve the ability to detect loops by always charging the ring if it is busy on the same request. v4: Make sure we queue another check if on a new batch References: https://bugs.freedesktop.org/show_bug.cgi?id=65394 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Mika Kuoppala mika.kuopp...@linux.intel.com Cc: Ben Widawsky b...@bwidawsk.net --- drivers/gpu/drm/i915/i915_irq.c | 110 +-- 1 file changed, 58 insertions(+), 52 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index dcb5209..32b2465 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2324,21 +2324,11 @@ ring_last_seqno(struct intel_ring_buffer *ring) struct drm_i915_gem_request, list)-seqno; } -static bool i915_hangcheck_ring_idle(struct intel_ring_buffer *ring, - u32 ring_seqno, bool *err) -{ - if (list_empty(ring-request_list) || - i915_seqno_passed(ring_seqno, ring_last_seqno(ring))) { - /* Issue a wake-up to catch stuck h/w. */ - if (waitqueue_active(ring-irq_queue)) { - DRM_ERROR(Hangcheck timer elapsed... %s idle\n, - ring-name); - wake_up_all(ring-irq_queue); - *err = true; - } - return true; - } - return false; +static bool +ring_idle(struct intel_ring_buffer *ring, u32 seqno) +{ + return (list_empty(ring-request_list) || + i915_seqno_passed(seqno, ring_last_seqno(ring))); } static bool semaphore_passed(struct intel_ring_buffer *ring) @@ -2372,16 +2362,26 @@ static bool semaphore_passed(struct intel_ring_buffer *ring) ioread32(ring-virtual_start+acthd+4)+1); } -static bool kick_ring(struct intel_ring_buffer *ring) +static bool ring_hung(struct intel_ring_buffer *ring) { struct drm_device *dev = ring-dev; struct drm_i915_private *dev_priv = dev-dev_private; - u32 tmp = I915_READ_CTL(ring); + u32 tmp; + + if (IS_GEN2(dev)) + return true; + + /* Is the chip hanging on a WAIT_FOR_EVENT? + * If so we can simply poke the RB_WAIT bit + * and break the hang. This should work on + * all but the second generation chipsets. + */ + tmp = I915_READ_CTL(ring); if (tmp RING_WAIT) { DRM_ERROR(Kicking stuck wait on %s\n, ring-name); I915_WRITE_CTL(ring, tmp); - return true; + return false; } if (INTEL_INFO(dev)-gen = 6 @@ -2390,22 +2390,10 @@ static bool kick_ring(struct intel_ring_buffer *ring) DRM_ERROR(Kicking stuck semaphore on %s\n, ring-name); I915_WRITE_CTL(ring, tmp); - return true; - } - return false; -} - -static bool i915_hangcheck_ring_hung(struct intel_ring_buffer *ring) -{ - if (IS_GEN2(ring-dev)) return false; + } - /* Is the chip hanging on a WAIT_FOR_EVENT? - * If so we can simply poke the RB_WAIT bit - * and break the hang. This should work on - * all but the second generation chipsets. - */ - return !kick_ring(ring); + return true; } /** @@ -2423,45 +2411,63 @@ void i915_hangcheck_elapsed(unsigned long data) struct intel_ring_buffer *ring; int i; int busy_count = 0, rings_hung = 0; - bool stuck[I915_NUM_RINGS]; + bool stuck[I915_NUM_RINGS] = { 0 }; +#define BUSY 1 +#define KICK 5 +#define HUNG 20 +#define FIRE 30 if (!i915_enable_hangcheck) return; for_each_ring(ring, dev_priv, i) { u32 seqno, acthd; - bool idle, err = false; + bool busy = true; seqno = ring-get_seqno(ring, false);
Re: [Intel-gfx] [PATCH 3/4] drm/i915: Don't count semaphore waits towards a stuck ring
On Mon, Jun 10, 2013 at 11:20:21AM +0100, Chris Wilson wrote: If we detect a ring is in a valid wait for another, just let it be. Eventually it will either begin to progress again, or the entire system will come grinding to a halt and then hangcheck will fire as soon as the deadlock is detected. This error was foretold by Ben in commit 05407ff889ceebe383aa5907219f86582ef96b72 Author: Mika Kuoppala mika.kuopp...@linux.intel.com Date: Thu May 30 09:04:29 2013 +0300 drm/i915: detect hang using per ring hangcheck_score If ring B is waiting on ring A via semaphore, and ring A is making progress, albeit slowly - the hangcheck will fire. The check will determine that A is moving, however ring B will appear hung because the ACTHD doesn't move. I honestly can't say if that's actually a realistic problem to hit it probably implies the timeout value is too low. v2: Make sure we don't even incur the KICK cost whilst waiting. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=65394 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: Mika Kuoppala mika.kuopp...@linux.intel.com Cc: Ben Widawsky b...@bwidawsk.net --- drivers/gpu/drm/i915/i915_irq.c | 121 +++ drivers/gpu/drm/i915/intel_ringbuffer.h |1 + 2 files changed, 90 insertions(+), 32 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 32b2465..cf8584c 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2331,21 +2331,21 @@ ring_idle(struct intel_ring_buffer *ring, u32 seqno) i915_seqno_passed(seqno, ring_last_seqno(ring))); } -static bool semaphore_passed(struct intel_ring_buffer *ring) +static struct intel_ring_buffer * +semaphore_waits_for(struct intel_ring_buffer *ring, u32 *seqno) { struct drm_i915_private *dev_priv = ring-dev-dev_private; - u32 acthd = intel_ring_get_active_head(ring) HEAD_ADDR; - struct intel_ring_buffer *signaller; - u32 cmd, ipehr, acthd_min; + u32 cmd, ipehr, acthd, acthd_min; ipehr = I915_READ(RING_IPEHR(ring-mmio_base)); if ((ipehr ~(0x3 16)) != (MI_SEMAPHORE_MBOX | MI_SEMAPHORE_COMPARE | MI_SEMAPHORE_REGISTER)) - return false; + return NULL; /* ACTHD is likely pointing to the dword after the actual command, * so scan backwards until we find the MBOX. */ + acthd = intel_ring_get_active_head(ring) HEAD_ADDR; acthd_min = max((int)acthd - 3 * 4, 0); do { cmd = ioread32(ring-virtual_start + acthd); @@ -2354,22 +2354,53 @@ static bool semaphore_passed(struct intel_ring_buffer *ring) acthd -= 4; if (acthd acthd_min) - return false; + return NULL; } while (1); - signaller = dev_priv-ring[(ring-id + (((ipehr 17) 1) + 1)) % 3]; - return i915_seqno_passed(signaller-get_seqno(signaller, false), - ioread32(ring-virtual_start+acthd+4)+1); + *seqno = ioread32(ring-virtual_start+acthd+4)+1; + return dev_priv-ring[(ring-id + (((ipehr 17) 1) + 1)) % 3]; +} + +static int semaphore_passed(struct intel_ring_buffer *ring) +{ + struct drm_i915_private *dev_priv = ring-dev-dev_private; + struct intel_ring_buffer *signaller; + u32 seqno, ctl; + + ring-hangcheck.deadlock = true; + + signaller = semaphore_waits_for(ring, seqno); + if (signaller == NULL || signaller-hangcheck.deadlock) + return -1; + + /* cursory check for an unkickable deadlock */ + ctl = I915_READ_CTL(signaller); + if (ctl RING_WAIT_SEMAPHORE semaphore_passed(signaller) 0) + return -1; + + return i915_seqno_passed(signaller-get_seqno(signaller, false), seqno); +} + +static void semaphore_clear_deadlocks(struct drm_i915_private *dev_priv) +{ + struct intel_ring_buffer *ring; + int i; + + for_each_ring(ring, dev_priv, i) + ring-hangcheck.deadlock = false; } -static bool ring_hung(struct intel_ring_buffer *ring) +static enum { wait, active, kick, hung } ring_stuck(struct intel_ring_buffer *ring, u32 acthd) { struct drm_device *dev = ring-dev; struct drm_i915_private *dev_priv = dev-dev_private; u32 tmp; + if (ring-hangcheck.acthd != acthd) + return active; + if (IS_GEN2(dev)) - return true; + return hung; /* Is the chip hanging on a WAIT_FOR_EVENT? * If so we can simply poke the RB_WAIT bit @@ -2381,19 +2412,24 @@ static bool ring_hung(struct intel_ring_buffer *ring) DRM_ERROR(Kicking stuck wait on %s\n, ring-name); I915_WRITE_CTL(ring, tmp); - return false; - } - - if (INTEL_INFO(dev)-gen = 6 - tmp
Re: [Intel-gfx] [PATCH 0/3] Fix backlight issues on some Windows 8 systems
On Sun, Jun 09, 2013 at 07:01:36PM -0400, Matthew Garrett wrote: Windows 8 introduced new policy for backlight control by pushing it out to graphics drivers. This appears to have coincided with a range of vendors adding Windows 8 checks to their backlight control code which trigger either awkward behaviour (Lenovo) or complete brokenness (some Dells). The simplest thing to do would be to just disable ACPI backlight control entirely if the firmware indicates Windows 8 support, but it's entirely possible that individual graphics drivers might still make use of the ACPI functionality in preference to native control. The first two patches in this series are picked from other patchesets aimed at solving similar problems. The last simply unregisters ACPI backlight control on Windows 8 systems when using an Intel GPU. Similar code could be added to other drivers, but I'm reluctant to do so without further investigation as to the behaviour of the vendor drivers under Windows. I've received some feedback from user testing of these patches (don't have an affected machine myself) and the feedback is mostly good. One user reported it didn't work, but I that machine may have discrete graphics (waiting to hear back from the user). The main drawback I see with any approach like this one is that the backlight remains broken for users of proprietary graphics drivers. Seth ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/4] drm/i915: Only slightly increment hangcheck score if we succesfully kick a ring
On Tue, Jun 11, 2013 at 11:45:00AM +0200, Daniel Vetter wrote: On Mon, Jun 10, 2013 at 11:20:20AM +0100, Chris Wilson wrote: + if (ring-hangcheck.seqno == seqno) { + if (ring_idle(ring, seqno)) { + if (waitqueue_active(ring-irq_queue)) { + /* Issue a wake-up to catch stuck h/w. */ + DRM_ERROR(Hangcheck timer elapsed... %s idle\n, + ring-name); + wake_up_all(ring-irq_queue); + ring-hangcheck.score += HUNG; Not sure whether we want to hit missed interrupts this badly, it was rather common a while back ;-) But we can fine-tune this easily now, so now reservations for merging from my side. Not sure what you mean here. The check is fairly easy and has gotten us out of many a hole before, and makes for a good defense. So how would you want to fine tune it? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/4] drm/i915: Only slightly increment hangcheck score if we succesfully kick a ring
On Tue, Jun 11, 2013 at 02:40:19PM +0100, Chris Wilson wrote: On Tue, Jun 11, 2013 at 11:45:00AM +0200, Daniel Vetter wrote: On Mon, Jun 10, 2013 at 11:20:20AM +0100, Chris Wilson wrote: + if (ring-hangcheck.seqno == seqno) { + if (ring_idle(ring, seqno)) { + if (waitqueue_active(ring-irq_queue)) { + /* Issue a wake-up to catch stuck h/w. */ + DRM_ERROR(Hangcheck timer elapsed... %s idle\n, + ring-name); + wake_up_all(ring-irq_queue); + ring-hangcheck.score += HUNG; Not sure whether we want to hit missed interrupts this badly, it was rather common a while back ;-) But we can fine-tune this easily now, so now reservations for merging from my side. Not sure what you mean here. The check is fairly easy and has gotten us out of many a hole before, and makes for a good defense. So how would you want to fine tune it? Something like the MI_WAIT hangcheck score, but like I've said as long as we don't have a real-world bug report (some poor guy disabled semaphores maybe due to the snb issue?) not worth bothering at all. I've just thought that if we're unlucky and miss the interrupt a few times in a row we don't want to accidentally declare the gpu dead. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/4] drm/i915: Only slightly increment hangcheck score if we succesfully kick a ring
On Tue, Jun 11, 2013 at 04:05:41PM +0200, Daniel Vetter wrote: On Tue, Jun 11, 2013 at 02:40:19PM +0100, Chris Wilson wrote: Not sure what you mean here. The check is fairly easy and has gotten us out of many a hole before, and makes for a good defense. So how would you want to fine tune it? Something like the MI_WAIT hangcheck score, but like I've said as long as we don't have a real-world bug report (some poor guy disabled semaphores maybe due to the snb issue?) not worth bothering at all. I've just thought that if we're unlucky and miss the interrupt a few times in a row we don't want to accidentally declare the gpu dead. I regarded it as a driver bug, that a GPU reset would not help. So the choice is between limping along with the hopefully occasional stall, or terminating the GPU with extreme prejudice. I chose the former, hence did not increment the hangcheck. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Add a hotplug connector property
On Mon, Jun 10, 2013 at 12:27 AM, Chris Wilson ch...@chris-wilson.co.uk wrote: It is these machines that require the per-connector quirk to turn off hotplug detection anyway. And there are users that need to turn off all hotplug detection (hardware and polling) on certain connectors whilst plugged into their kvm. I think it'd be useful to take one step back and lock at the intended result (i.e. use-cases) a bit first, so cut out all the other discussion. Pondering this a bit more I see a bunch of things we could aim for: - Giving userspace more rope to make even horribly broken systems useful. We have the hpd storm stuff and the connector state forcing, but atm the story is still imcomplete there I think. I guess it would be useful to expose the connector-force attribute (maybe through sysfs, tends to be easier to set correctly with scripts), so that users with really flaky hw could update the forced state at runtim. Then we could make the EDID firmware loading a bit more flexible (again maybe expose the firmware filename in sysfs per-connector) and users would be able to fake any setup. The last missing puzzle piece would be to add a new FORCE_INTEGRATED mode which also eschews the repeated -get_modes calls when -num_modes 0 or so. - Allowing some fine-tuning of the hpd storm detection engine. Since we don't yet have much real-world usage data I'm not sure what we could want here, so probably best to postpone for now. - Disabling polling. Presuming we have the pimped connector forcing infrastructure described above I'm not sure whether we really need more than what the global tunable already affords us. Thanks to locking reworks and a few other things the poll work is really unobtrusive nowadays (especially if all outputs claim to support hotplug). - Better caching of EDID data and connector state. My favourite approach for this would be some opportunistic caching for e.g. 1 second in the -fill_modes callback. Hotplug interrupts would invalidate the caching (and since we already filter hotplug interrupts to only touch relevant outputs with Egbert's latest patches, that should be fairly good). Of course caching the edid like this in a reality with horribly unreliable hpd will add new races and inconsistencies between reality and what the kernel reports. But in those cases hotplug reporting is already horribly racy, so not really a real degration. Like the poll intervall we could make that tuneable. - Full EDID caching when the hotplug interrupt is actually reliable. I think that this is the case for native DP outputs, at least I'm not aware of any bug report. And maybe we'll figure out the few holdouts for HDMI hpd handling. If we have the opportunistic caching infrastructure from the previous point we could simply disable the time-based invalidation for those, and only invalidate the edid/connector state on a hotplug event. But even for DP I think it's better if the kernel is in charge of setting this, since e.g. plugging in a DP-VGA converter means that we need to disable full caching and switch back to opportunistic caching. And then back again if a native DP monitor is plugged in. Hopefully this dump here is useful to broaden the discussion a bit. Thoughts? Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/4] drm/i915: Only slightly increment hangcheck score if we succesfully kick a ring
On Tue, Jun 11, 2013 at 4:16 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Tue, Jun 11, 2013 at 04:05:41PM +0200, Daniel Vetter wrote: On Tue, Jun 11, 2013 at 02:40:19PM +0100, Chris Wilson wrote: Not sure what you mean here. The check is fairly easy and has gotten us out of many a hole before, and makes for a good defense. So how would you want to fine tune it? Something like the MI_WAIT hangcheck score, but like I've said as long as we don't have a real-world bug report (some poor guy disabled semaphores maybe due to the snb issue?) not worth bothering at all. I've just thought that if we're unlucky and miss the interrupt a few times in a row we don't want to accidentally declare the gpu dead. I regarded it as a driver bug, that a GPU reset would not help. So the choice is between limping along with the hopefully occasional stall, or terminating the GPU with extreme prejudice. I chose the former, hence did not increment the hangcheck. Hm, maybe I'm reading the logic wrongly, but don't we add a += HUNG score now for a stuck, but idle ring? So pretty short of declaring the thing dead? Ofc there's the slow decline if the gpu isn't actually dead, but if we have more than 1 such stall every HUNG (=20) hangcheck times we'll eventually declare it dead despite the limping along. Anyway nothing to really worry about, just wanted to check my understanding here. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/4] drm/i915: Only slightly increment hangcheck score if we succesfully kick a ring
On Tue, Jun 11, 2013 at 04:37:26PM +0200, Daniel Vetter wrote: On Tue, Jun 11, 2013 at 4:16 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Tue, Jun 11, 2013 at 04:05:41PM +0200, Daniel Vetter wrote: On Tue, Jun 11, 2013 at 02:40:19PM +0100, Chris Wilson wrote: Not sure what you mean here. The check is fairly easy and has gotten us out of many a hole before, and makes for a good defense. So how would you want to fine tune it? Something like the MI_WAIT hangcheck score, but like I've said as long as we don't have a real-world bug report (some poor guy disabled semaphores maybe due to the snb issue?) not worth bothering at all. I've just thought that if we're unlucky and miss the interrupt a few times in a row we don't want to accidentally declare the gpu dead. I regarded it as a driver bug, that a GPU reset would not help. So the choice is between limping along with the hopefully occasional stall, or terminating the GPU with extreme prejudice. I chose the former, hence did not increment the hangcheck. Hm, maybe I'm reading the logic wrongly, but don't we add a += HUNG score now for a stuck, but idle ring? So pretty short of declaring the thing dead? Yeah... Didn't mean to do that, as all the time I was thinking don't hang here, this is our bug not userspace's. Ofc there's the slow decline if the gpu isn't actually dead, but if we have more than 1 such stall every HUNG (=20) hangcheck times we'll eventually declare it dead despite the limping along. Anyway nothing to really worry about, just wanted to check my understanding here. Looks like my fingers mutinied; and I am the one confused. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 01/10] drm/i915: We implement WaFbcWaitForVBlankBeforeEnable for ilk and snb
2013/6/7 Damien Lespiau damien.lesp...@intel.com: 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: Damien Lespiau damien.lesp...@intel.com Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/intel_pm.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 50fe3d7..57e99b1 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -404,6 +404,8 @@ void intel_enable_fbc(struct drm_crtc *crtc, unsigned long interval) * following the termination of the page-flipping sequence * and indeed performing the enable as a co-routine and not * waiting synchronously upon the vblank. +* +* WaFbcWaitForVBlankBeforeEnable:ilk,snb */ schedule_delayed_work(work-work, msecs_to_jiffies(50)); } -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/10] drm/i915: We implement WaFbcAsynchFlipDisableFbcQueue on ilk and snb
2013/6/7 Damien Lespiau damien.lesp...@intel.com: Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- drivers/gpu/drm/i915/intel_pm.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 57e99b1..eb3c2c4 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4420,6 +4420,8 @@ static void ironlake_init_clock_gating(struct drm_device *dev) * The bit 22 of 0x42000 * The bit 22 of 0x42004 * The bit 7,8,9 of 0x42020. +* +* WaFbcAsynchFlipDisableFbcQueue:ilk */ if (IS_IRONLAKE_M(dev)) { I915_WRITE(ILK_DISPLAY_CHICKEN1, Inside the IS_IRONLAKE_M check we set 2 chicken bits. Does the WA apply to both? It seem it only applies to ILK_DISPLAY_CHICKEN1, so my suggestion would be to move the comment down to inside the if statement, just above the ILK_DISPLAY_CHICKEN1 line. With the WA comment above the if statement it seems the WA applies to both i915_writes. Anyway, Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com @@ -4557,6 +4559,8 @@ static void gen6_init_clock_gating(struct drm_device *dev) * The bit5 and bit7 of 0x42020 * The bit14 of 0x70180 * The bit14 of 0x71180 +* +* WaFbcAsynchFlipDisableFbcQueue:snb */ I915_WRITE(ILK_DISPLAY_CHICKEN1, I915_READ(ILK_DISPLAY_CHICKEN1) | -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 03/10] drm/i915: We implement WaFbcDisableDpfcClockGating on ilk
2013/6/7 Damien Lespiau damien.lesp...@intel.com: Signed-off-by: Damien Lespiau damien.lesp...@intel.com Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com If you search for the WaFbcDisableDpfcClockGating string on that file, you'll see that this workaround is implemented differently on IVB/HSW: on ILK the bits are turned on at init_clock_gating and stay like that. On IVB/HSW we enable the bits at gen7_enable_fbc and disable them at ironlake_disable_fbc. Can't se convert these ILK/SNB FBC workarounds to be implemented like IVB/HSW? Of course, this would be a separate patch. And we would have to re-check all the workarounds that only need to be enabled while FBC is enabled. Volunteers? --- drivers/gpu/drm/i915/intel_pm.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index eb3c2c4..2eb1846 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4385,7 +4385,10 @@ static void ironlake_init_clock_gating(struct drm_device *dev) struct drm_i915_private *dev_priv = dev-dev_private; uint32_t dspclk_gate = ILK_VRHUNIT_CLOCK_GATE_DISABLE; - /* Required for FBC */ + /* +* Required for FBC +* WaFbcDisableDpfcClockGating:ilk +*/ dspclk_gate |= ILK_DPFCRUNIT_CLOCK_GATE_DISABLE | ILK_DPFCUNIT_CLOCK_GATE_DISABLE | ILK_DPFDUNIT_CLOCK_GATE_ENABLE; -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/10] drm/i915: We implement WaMPhyProgramming on Haswell
2013/6/7 Damien Lespiau damien.lesp...@intel.com: Signed-off-by: Damien Lespiau damien.lesp...@intel.com Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/intel_display.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 827d7ca..9e0d69c 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5122,7 +5122,10 @@ static void ironlake_init_pch_refclk(struct drm_device *dev) BUG_ON(val != final); } -/* Sequence to enable CLKOUT_DP for FDI usage and configure PCH FDI I/O. */ +/* + * Sequence to enable CLKOUT_DP for FDI usage and configure PCH FDI I/O. + * WaMPhyProgramming:hsw + */ static void lpt_init_pch_refclk(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev-dev_private; -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 09/10] drm/i915: Implement WaRsDisableRC6Plus
2013/6/7 Damien Lespiau damien.lesp...@intel.com: Let's disable RC6+ by default. We still leave the possibility to override this with the module parameter. I've also shuffled the code around so we always log if we have RC6 enabled or disabled. Why disable RC6+ on IVB? My docs tells me it should only be disabled on the very first pre-production machines that no one has. On the other hand, it looks like that for VLV we should disable RC6+. The code shuffle looks like a nice improvement. Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- drivers/gpu/drm/i915/intel_pm.c | 25 + 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 0465cd6..e19952f 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3158,27 +3158,20 @@ int intel_enable_rc6(const struct drm_device *dev) if (INTEL_INFO(dev)-gen 5) return 0; - /* Respect the kernel parameter if it is set */ - if (i915_enable_rc6 = 0) - return i915_enable_rc6; - /* Disable RC6 on Ironlake */ - if (INTEL_INFO(dev)-gen == 5) + if (i915_enable_rc6 == 0 || INTEL_INFO(dev)-gen == 5) { + DRM_DEBUG_DRIVER(RC6 disabled\n); return 0; - - if (IS_HASWELL(dev)) { - DRM_DEBUG_DRIVER(Haswell: only RC6 available\n); - return INTEL_RC6_ENABLE; } - /* snb/ivb have more than one rc6 state. */ - if (INTEL_INFO(dev)-gen == 6) { - DRM_DEBUG_DRIVER(Sandybridge: deep RC6 disabled\n); - return INTEL_RC6_ENABLE; - } + DRM_DEBUG_DRIVER(RC6 enabled\n); + + if (i915_enable_rc6 0) + return i915_enable_rc6; - DRM_DEBUG_DRIVER(RC6 and deep RC6 enabled\n); - return (INTEL_RC6_ENABLE | INTEL_RC6p_ENABLE); + /* snb/ivb have more than one rc6 state. However we only allow RC6 +* WaRsDisableRC6Plus:snb,ivb,vlv */ + return INTEL_RC6_ENABLE; } static void gen6_enable_rps(struct drm_device *dev) -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/10] drm/i915: Don't try to calculate RC6 residency on GEN4 and before
2013/6/7 Damien Lespiau damien.lesp...@intel.com: intel_enable_rc6() is used to check if we can compute the RC6 residency in the sysfs code. Disable this for platforms older than Ironlake. Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- drivers/gpu/drm/i915/intel_pm.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 2eb1846..0465cd6 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3154,6 +3154,10 @@ static void valleyview_disable_rps(struct drm_device *dev) int intel_enable_rc6(const struct drm_device *dev) { + /* No RC6 before Ironlake */ + if (INTEL_INFO(dev)-gen 5) + return 0; Daniel recently complained about a similar check on one of my patches. Looks like the preferred way would be if (INTEL_INFO(dev)-gen = 4). I guess Daniel can do this change while applying the patch. Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com + /* Respect the kernel parameter if it is set */ if (i915_enable_rc6 = 0) return i915_enable_rc6; -- 1.8.1.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Fix VLV analog output shivers
From: Ville Syrjälä ville.syrj...@linux.intel.com The current PLL settings produce a rather unstable picture when I hook up a VLV to my HP ZR24w display via a VGA cable. Switching the PLL to hybrid mode makes the picture a lot more stable. No idea if this is truly wise though... Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_display.c | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 29f2c0d..8c30a4c 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4412,8 +4412,14 @@ static void vlv_update_pll(struct intel_crtc *crtc) vlv_dpio_write(dev_priv, DPIO_LFP_COEFF(pipe), 0x00df); - if (intel_pipe_has_type(crtc-base, INTEL_OUTPUT_EDP) || - intel_pipe_has_type(crtc-base, INTEL_OUTPUT_DISPLAYPORT)) { + if (intel_pipe_has_type(crtc-base, INTEL_OUTPUT_ANALOG)) { + /* XXX this gives a less shivery picture */ + if (!pipe) + vlv_dpio_write(dev_priv, DPIO_REFSFR(pipe), 0x08f7); + else + vlv_dpio_write(dev_priv, DPIO_REFSFR(pipe), 0x08f4); + } else if (intel_pipe_has_type(crtc-base, INTEL_OUTPUT_EDP) || + intel_pipe_has_type(crtc-base, INTEL_OUTPUT_DISPLAYPORT)) { /* Use SSC source */ if (!pipe) vlv_dpio_write(dev_priv, DPIO_REFSFR(pipe), @@ -4421,7 +4427,7 @@ static void vlv_update_pll(struct intel_crtc *crtc) else vlv_dpio_write(dev_priv, DPIO_REFSFR(pipe), 0x0df7); - } else { /* HDMI or VGA */ + } else { /* HDMI */ /* Use bend source */ if (!pipe) vlv_dpio_write(dev_priv, DPIO_REFSFR(pipe), -- 1.8.1.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 08/10] drm/i915: Don't try to calculate RC6 residency on GEN4 and before
On Tue, Jun 11, 2013 at 04:33:24PM -0300, Paulo Zanoni wrote: 2013/6/7 Damien Lespiau damien.lesp...@intel.com: intel_enable_rc6() is used to check if we can compute the RC6 residency in the sysfs code. Disable this for platforms older than Ironlake. Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- drivers/gpu/drm/i915/intel_pm.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 2eb1846..0465cd6 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3154,6 +3154,10 @@ static void valleyview_disable_rps(struct drm_device *dev) int intel_enable_rc6(const struct drm_device *dev) { + /* No RC6 before Ironlake */ + if (INTEL_INFO(dev)-gen 5) + return 0; Daniel recently complained about a similar check on one of my patches. Looks like the preferred way would be if (INTEL_INFO(dev)-gen = 4). I guess Daniel can do this change while applying the patch. Actually that way round is imo ok, since we tend to say pre-gen5 and mean that it excludes gen5. It's kinda like iterating over 0-based C arrays where you check i = start i end ;-) Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix VLV analog output shivers
On Tue, Jun 11, 2013 at 11:08:16PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com The current PLL settings produce a rather unstable picture when I hook up a VLV to my HP ZR24w display via a VGA cable. Switching the PLL to hybrid mode makes the picture a lot more stable. No idea if this is truly wise though... Ok, you've just slipped up here and mentioned that the changed bit is for hybrid mode. Can I have real register defines for this magic now please? Apparently Jesse just weaseled out of real work claiming that it's not documented at all ;-) Cheers, Daniel Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_display.c | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 29f2c0d..8c30a4c 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4412,8 +4412,14 @@ static void vlv_update_pll(struct intel_crtc *crtc) vlv_dpio_write(dev_priv, DPIO_LFP_COEFF(pipe), 0x00df); - if (intel_pipe_has_type(crtc-base, INTEL_OUTPUT_EDP) || - intel_pipe_has_type(crtc-base, INTEL_OUTPUT_DISPLAYPORT)) { + if (intel_pipe_has_type(crtc-base, INTEL_OUTPUT_ANALOG)) { + /* XXX this gives a less shivery picture */ + if (!pipe) + vlv_dpio_write(dev_priv, DPIO_REFSFR(pipe), 0x08f7); + else + vlv_dpio_write(dev_priv, DPIO_REFSFR(pipe), 0x08f4); + } else if (intel_pipe_has_type(crtc-base, INTEL_OUTPUT_EDP) || +intel_pipe_has_type(crtc-base, INTEL_OUTPUT_DISPLAYPORT)) { /* Use SSC source */ if (!pipe) vlv_dpio_write(dev_priv, DPIO_REFSFR(pipe), @@ -4421,7 +4427,7 @@ static void vlv_update_pll(struct intel_crtc *crtc) else vlv_dpio_write(dev_priv, DPIO_REFSFR(pipe), 0x0df7); - } else { /* HDMI or VGA */ + } else { /* HDMI */ /* Use bend source */ if (!pipe) vlv_dpio_write(dev_priv, DPIO_REFSFR(pipe), -- 1.8.1.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/9] drm/i915: fixup i8xx pll limits
On Tue, May 21, 2013 at 09:54:53PM +0200, Daniel Vetter wrote: So apparently the pll limits in the docs are the real values, but the formula actually seems to consistenly use register values. See commit 4f7dfb6788dd022446847fbbfbe45e13bedb5be2 Author: Patrik Jakobsson patrik.r.jakobs...@gmail.com Date: Wed Feb 13 22:20:22 2013 +0100 drm/i915: Set i9xx sdvo clock limits according to specifications On gen2 the situation is a bit more messy: We reuse the code for m1/m2/n computation and register frobbing from gen3, so those limits need to be in register values (and so need to substract 2 from the doc limits). But gen2 also stores p1/p2 with 2/1 substracted in the register! For those though i8xx_update_pll does the adjustments, but the clock computation code assumes it works like on gen3. So for divisor limits we must _not_ adjust them to the register values. I think this deserves a comment for the next one looking at that code. Also the documented computations for p are: gen2: p = (p1 + 2) * 2^(p2 + 1) gen3: p = p1 * p2 which lead me to believe we weren't computing the dot clock correctly on gen2 (but in fact it's fine as p1/p2 are never expressed as register values, which are not a direct deduction (say on gen 3, p2 = 10 is coded with 0, p2 = 5 is coded by 1)) So IIUC, we have n, m1, m2 that we express in the register domain while m, p1, p2, p are in the clock formula domain, for lack of a better wording. And so the limits follow that. Otherwise, the patch looks good: Reviewed-by: Damien Lespiau damien.lesp...@intel.com -- Damien v2: Extract the common i8xx m/n limits into a single define. This was motivated by the mess for the g4x limits where they've all been off in different directions, apparently to fix specific bugs ... Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_display.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 959c2ae..ea8eb0c 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -84,13 +84,16 @@ intel_fdi_link_freq(struct drm_device *dev) return 27; } +#define I8XX_DPLL_M_N_LIMITS \ + .n = { .min = 1, .max = 14 }, \ + .m = { .min = 96, .max = 140 }, \ + .m1 = { .min = 16, .max = 24 }, \ + .m2 = { .min = 4, .max = 14 }, + static const intel_limit_t intel_limits_i8xx_dvo = { .dot = { .min = 25000, .max = 35 }, .vco = { .min = 93, .max = 140 }, - .n = { .min = 3, .max = 16 }, - .m = { .min = 96, .max = 140 }, - .m1 = { .min = 18, .max = 26 }, - .m2 = { .min = 6, .max = 16 }, + I8XX_DPLL_M_N_LIMITS .p = { .min = 4, .max = 128 }, .p1 = { .min = 2, .max = 33 }, .p2 = { .dot_limit = 165000, @@ -100,10 +103,7 @@ static const intel_limit_t intel_limits_i8xx_dvo = { static const intel_limit_t intel_limits_i8xx_lvds = { .dot = { .min = 25000, .max = 35 }, .vco = { .min = 93, .max = 140 }, - .n = { .min = 3, .max = 16 }, - .m = { .min = 96, .max = 140 }, - .m1 = { .min = 18, .max = 26 }, - .m2 = { .min = 6, .max = 16 }, + I8XX_DPLL_M_N_LIMITS .p = { .min = 4, .max = 128 }, .p1 = { .min = 1, .max = 6 }, .p2 = { .dot_limit = 165000, -- 1.7.11.7 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915: tune the RC6 threshold for stability
It's basically the same deal as the RC6+ issues on ivy bridge except this time with RC6 on sandy bridge. Like last time the core of the issue is that the timings don't work 100% with our voltage regulator. So from time to time, the kernel will print a warning message about the GPU not getting out of RC6. In particular, I found this fairly easy to reproduce during suspend/resume. Changing the threshold to 15 instead of 5 seems to fix the issue. I also measured the idle power usage before/after this patch and didn't see a difference on a sandy bridge laptop. Signed-off-by: Stéphane Marchesin marc...@chromium.org --- drivers/gpu/drm/i915/intel_pm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index aa01128..52fe8f7 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -2577,7 +2577,7 @@ static void gen6_enable_rps(struct drm_device *dev) I915_WRITE(GEN6_RC_SLEEP, 0); I915_WRITE(GEN6_RC1e_THRESHOLD, 1000); - I915_WRITE(GEN6_RC6_THRESHOLD, 5); + I915_WRITE(GEN6_RC6_THRESHOLD, 15); I915_WRITE(GEN6_RC6p_THRESHOLD, 15); I915_WRITE(GEN6_RC6pp_THRESHOLD, 64000); /* unused */ -- 1.8.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: repin bound framebuffers on resume
During suspend all fences are reset, including their pin_count which is reset to 0. However a framebuffer can be bound across suspend/resume, which means that when the buffer is unbound after resume, the pin count for the buffer will be negative. Since the fence pin count is now negative when available and zero when in use, the buffer's fence will get recycled when the fence is in use which is the opposite of what we want. The visible effect is that since the fence is recycled the tiling mode goes away while the buffer is being displayed and we get lines/screens of garbage. To fix this, we repin the fences for all bound fbs on resume, which ensures the pin count is right. Signed-off-by: Stéphane Marchesin marc...@chromium.org --- drivers/gpu/drm/i915/i915_drv.c | 2 ++ drivers/gpu/drm/i915/intel_display.c | 32 drivers/gpu/drm/i915/intel_drv.h | 1 + 3 files changed, 35 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index a2e4953..b8e82ab 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -643,6 +643,8 @@ static int __i915_drm_thaw(struct drm_device *dev) /* We need working interrupts for modeset enabling ... */ drm_irq_install(dev); + /* Repin all live fences before resuming */ + intel_repin_fences(dev); intel_modeset_init_hw(dev); drm_modeset_lock_all(dev); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 56746dc..4bf3240 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2051,6 +2051,38 @@ void intel_unpin_fb_obj(struct drm_i915_gem_object *obj) i915_gem_object_unpin(obj); } +/* + * Repin the fences for all currently bound fbs. During suspend i915_drm_freeze + * calls i915_gem_reset, which in turn calls i915_gem_reset_fences, which + * resets the fence pin_counts to 0. So on resume we can call this function to + * restore the fences on bound framebuffers. + */ +void intel_repin_fences(struct drm_device *dev) +{ + struct drm_crtc *crtc; + struct drm_i915_gem_object *obj; + int ret; + + mutex_lock(mode_config-mutex); + list_for_each_entry(crtc, dev-mode_config.crtc_list, head) { + if (!crtc || !crtc-fb) + continue; + obj = to_intel_framebuffer(crtc-fb)-obj; + if (!obj) + continue; + + /* Install a fence for tiled scan-out. */ + if (obj-tiling_mode != I915_TILING_NONE) { + ret = i915_gem_object_get_fence(obj); + if (ret) + DRM_ERROR(Couldn't get a fence\n); + else + i915_gem_object_pin_fence(obj); + } + } + mutex_unlock(mode_config-mutex); +} + /* Computes the linear offset to the base tile and adjusts x, y. bytes per pixel * is assumed to be a power-of-two. */ unsigned long intel_gen4_compute_page_offset(int *x, int *y, diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 624a9e6..b5395ed 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -630,6 +630,7 @@ extern int intel_pin_and_fence_fb_obj(struct drm_device *dev, struct drm_i915_gem_object *obj, struct intel_ring_buffer *pipelined); extern void intel_unpin_fb_obj(struct drm_i915_gem_object *obj); +extern void intel_repin_fences(struct drm_device *dev); extern int intel_framebuffer_init(struct drm_device *dev, struct intel_framebuffer *ifb, -- 1.8.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: repin bound framebuffers on resume
On Tue, Jun 11, 2013 at 03:49:27PM -0700, Stéphane Marchesin wrote: During suspend all fences are reset, including their pin_count which is reset to 0. However a framebuffer can be bound across suspend/resume, which means that when the buffer is unbound after resume, the pin count for the buffer will be negative. Since the fence pin count is now negative when available and zero when in use, the buffer's fence will get recycled when the fence is in use which is the opposite of what we want. The visible effect is that since the fence is recycled the tiling mode goes away while the buffer is being displayed and we get lines/screens of garbage. To fix this, we repin the fences for all bound fbs on resume, which ensures the pin count is right. Yikes. So why do we not just keep the fences alive during suspend (not touching their pin_count), and then just iterate over the list of fences rewriting the register as required upon resume? That would seem less error prone than trying to reconstruct the lost pin_count. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: repin bound framebuffers on resume
On Tue, Jun 11, 2013 at 3:57 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Tue, Jun 11, 2013 at 03:49:27PM -0700, Stéphane Marchesin wrote: During suspend all fences are reset, including their pin_count which is reset to 0. However a framebuffer can be bound across suspend/resume, which means that when the buffer is unbound after resume, the pin count for the buffer will be negative. Since the fence pin count is now negative when available and zero when in use, the buffer's fence will get recycled when the fence is in use which is the opposite of what we want. The visible effect is that since the fence is recycled the tiling mode goes away while the buffer is being displayed and we get lines/screens of garbage. To fix this, we repin the fences for all bound fbs on resume, which ensures the pin count is right. Yikes. So why do we not just keep the fences alive during suspend (not touching their pin_count), and then just iterate over the list of fences rewriting the register as required upon resume? That would seem less error prone than trying to reconstruct the lost pin_count. I suspect they'd need to be saved/restored at the hw level as well, which AFAICS isn't happening today... Stéphane ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/9] drm/i915: fixup g4x pll limits
On Tue, May 21, 2013 at 09:54:54PM +0200, Daniel Vetter wrote: Again the same confusion that our code expects m1/m2 in register values. This time around with the added fun that many of the existing values have been all off by a bit in different directions. Hence extract a common #define. Note that n limits differ between lvds and other outputs. Strangely they've all been correct already. v2: Rebased on top of the DP pll rework, which makes it even more obvious that we can do this ... I'm a bit confused by this one: - I don't seem to find the special LVDS limits for n - Those limits are gated by a IS_G4X(). IS_G4X() is true for eaglelake and cantiga. The docs single out Cantiga and we seem to need a different set of limits for that platform (compared to the rest of gen4) platforms? - The limits for n are 3-6 or 3-5 in this code but I read 3-8 and 5-6 (cantiga) - m doesn't seem to match what I have, it seems like it could be the cantiga values (except that m is in formula space so we don't substract 2) - m1 and m2 seem to match what I have for cantiga, are we supposed to have those limits for eaglelake as well? Well, the only conclusion is that I must be reading the wrong docs, or? Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_display.c | 20 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index ea8eb0c..cb54131 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -136,14 +136,16 @@ static const intel_limit_t intel_limits_i9xx_lvds = { .p2_slow = 14, .p2_fast = 7 }, }; +#define G4X_DPLL_M_LIMITS \ + .m = { .min = 104, .max = 138 },\ + .m1 = { .min = 15, .max = 21 },\ + .m2 = { .min = 3, .max = 11 }, static const intel_limit_t intel_limits_g4x_sdvo = { .dot = { .min = 25000, .max = 27 }, .vco = { .min = 175, .max = 350}, .n = { .min = 1, .max = 4 }, - .m = { .min = 104, .max = 138 }, - .m1 = { .min = 17, .max = 23 }, - .m2 = { .min = 5, .max = 11 }, + G4X_DPLL_M_LIMITS .p = { .min = 10, .max = 30 }, .p1 = { .min = 1, .max = 3}, .p2 = { .dot_limit = 27, @@ -156,9 +158,7 @@ static const intel_limit_t intel_limits_g4x_hdmi = { .dot = { .min = 22000, .max = 40 }, .vco = { .min = 175, .max = 350}, .n = { .min = 1, .max = 4 }, - .m = { .min = 104, .max = 138 }, - .m1 = { .min = 16, .max = 23 }, - .m2 = { .min = 5, .max = 11 }, + G4X_DPLL_M_LIMITS .p = { .min = 5, .max = 80 }, .p1 = { .min = 1, .max = 8}, .p2 = { .dot_limit = 165000, @@ -169,9 +169,7 @@ static const intel_limit_t intel_limits_g4x_single_channel_lvds = { .dot = { .min = 2, .max = 115000 }, .vco = { .min = 175, .max = 350 }, .n = { .min = 1, .max = 3 }, - .m = { .min = 104, .max = 138 }, - .m1 = { .min = 17, .max = 23 }, - .m2 = { .min = 5, .max = 11 }, + G4X_DPLL_M_LIMITS .p = { .min = 28, .max = 112 }, .p1 = { .min = 2, .max = 8 }, .p2 = { .dot_limit = 0, @@ -183,9 +181,7 @@ static const intel_limit_t intel_limits_g4x_dual_channel_lvds = { .dot = { .min = 8, .max = 224000 }, .vco = { .min = 175, .max = 350 }, .n = { .min = 1, .max = 3 }, - .m = { .min = 104, .max = 138 }, - .m1 = { .min = 17, .max = 23 }, - .m2 = { .min = 5, .max = 11 }, + G4X_DPLL_M_LIMITS .p = { .min = 14, .max = 42 }, .p1 = { .min = 2, .max = 6 }, .p2 = { .dot_limit = 0, -- 1.7.11.7 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix VLV analog output shivers
On Tue, 11 Jun 2013 23:06:59 +0200 Daniel Vetter dan...@ffwll.ch wrote: On Tue, Jun 11, 2013 at 11:08:16PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com The current PLL settings produce a rather unstable picture when I hook up a VLV to my HP ZR24w display via a VGA cable. Switching the PLL to hybrid mode makes the picture a lot more stable. No idea if this is truly wise though... Ok, you've just slipped up here and mentioned that the changed bit is for hybrid mode. Can I have real register defines for this magic now please? Apparently Jesse just weaseled out of real work claiming that it's not documented at all ;-) There are some bits we could use, but we'd be making up the name. On top of that, the hex value is used in the docs, so if we make up bit field names, we'll end up double taking everytime we look at these bits. So there's no good answer here... :/ -- Jesse Barnes, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/9] drm/i915: pnv dpll doesn't use m1!
On Tue, May 21, 2013 at 09:54:55PM +0200, Daniel Vetter wrote: So don't try to store it in the DPLL_FP register. Otherwise it looks like the limits for pineview are correct: It has it's own clock computation code, which doesn't use an offset for n divisors, and the register value based m limits look sane enough. - n can vary between 2 and 6, but we declare the 3-6 as limits. - p1 seems to be able to go up to 9 - the m upper limit seems a bit big, but the docs are a bit shy on that values for pnv. Otherwise, the change itself seems good: Reviewed-by: Damien Lespiau damien.lesp...@intel.com v2: Rebase on top of the pineview clock refactor and fixup up the commit message: It's m1 pnv doens't care about, not m2! Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_display.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index cb54131..520e340 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4300,7 +4300,7 @@ static int i9xx_get_refclk(struct drm_crtc *crtc, int num_connectors) static uint32_t pnv_dpll_compute_fp(struct dpll *dpll) { - return (1 dpll-n) 16 | dpll-m1 8 | dpll-m2; + return (1 dpll-n) 16 | dpll-m2; } static uint32_t i9xx_dpll_compute_fp(struct dpll *dpll) -- 1.7.11.7 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: repin bound framebuffers on resume
On Tue, Jun 11, 2013 at 04:01:21PM -0700, Stéphane Marchesin wrote: On Tue, Jun 11, 2013 at 3:57 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Tue, Jun 11, 2013 at 03:49:27PM -0700, Stéphane Marchesin wrote: During suspend all fences are reset, including their pin_count which is reset to 0. However a framebuffer can be bound across suspend/resume, which means that when the buffer is unbound after resume, the pin count for the buffer will be negative. Since the fence pin count is now negative when available and zero when in use, the buffer's fence will get recycled when the fence is in use which is the opposite of what we want. The visible effect is that since the fence is recycled the tiling mode goes away while the buffer is being displayed and we get lines/screens of garbage. To fix this, we repin the fences for all bound fbs on resume, which ensures the pin count is right. Yikes. So why do we not just keep the fences alive during suspend (not touching their pin_count), and then just iterate over the list of fences rewriting the register as required upon resume? That would seem less error prone than trying to reconstruct the lost pin_count. I suspect they'd need to be saved/restored at the hw level as well, which AFAICS isn't happening today... Ugh, I introduced this bug 30 months ago - saved by the VT switch on resume. But we can restore the fences from dev_priv-fence_regs... Actually we have a very similar problem after a GPU reset where we should restore fences for pinned objects (i.e. the scanout). The patch to fix both looks fairly straightforward. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx