Re: [Intel-gfx] [PATCH 6/6] drm/i915: implement HSW display sequences for package C8+
On Wed, Jun 05, 2013 at 02:21:56PM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com This patch implements Display Sequences for Package C8, from the Display Mode Set Sequence section from the Haswell documentation. Notice that even if we allow PC8+, there's no guarantee that we will actually enter PC8+, since the other devices also need to allow it. Also notice that we need i915.disable_power_well=1 in order to test this feature: we don't allow PC8+ if the power well is still enabled. v2: - Rebase - Implement many review comments form Daniel Vetter - Call intel_prepare_ddi and i915_gem_init_swizzling when returning from C8+ so we can actually see the screen contents Really don't like the colour you've chosen in places, and there seem to be a lot of placeholder code. Highlights: - intel_aux_display_shutdown_forbid / allow - intel_aux_display_get / put Not convinced by aux there either. Perhaps, intel_display_wakelock_get() instead. Or we could go with intel_display_forcewake_get(). Hmm, forcewake is better as we already have that concept in our code (and wakelock may end up being confused with the system-wide wakelocks.) - move your c8 state into a struct: struct i915_package_c8 { struct mutex mutex; union { struct hsw_package_c8_registers hsw_regfile; }; bool enabled; /* was allowing_package_c8 */ int forcewake; /* was c8_forbid_refcnt */ }; - The interrupt register handling is messy. We already track the values of the ones that we change, the others should be static. Looks mostly correct (barring a race with an interrupt!). - Hooking into modeset_global_resources seems strange and never explained. -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 6/6] drm/i915: implement HSW display sequences for package C8+
On Thu, Jun 06, 2013 at 07:49:49AM +0100, Chris Wilson wrote: On Wed, Jun 05, 2013 at 02:21:56PM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com This patch implements Display Sequences for Package C8, from the Display Mode Set Sequence section from the Haswell documentation. Notice that even if we allow PC8+, there's no guarantee that we will actually enter PC8+, since the other devices also need to allow it. Also notice that we need i915.disable_power_well=1 in order to test this feature: we don't allow PC8+ if the power well is still enabled. v2: - Rebase - Implement many review comments form Daniel Vetter - Call intel_prepare_ddi and i915_gem_init_swizzling when returning from C8+ so we can actually see the screen contents Really don't like the colour you've chosen in places, and there seem to be a lot of placeholder code. Highlights: - intel_aux_display_shutdown_forbid / allow - intel_aux_display_get / put Not convinced by aux there either. Perhaps, intel_display_wakelock_get() instead. Or we could go with intel_display_forcewake_get(). Hmm, forcewake is better as we already have that concept in our code (and wakelock may end up being confused with the system-wide wakelocks.) aux_display is my bikeshed ;-) The idea was that we have a russian doll stack of display power knobs: - the power well to shut of everything but edp on pipe A - the power well for all display pipes (not yet on haswell) - power control for aux stuff like gmbus/dp aux - the real pci device In reality it's probably more a tree since rc6 forcewake also hangs in there somewhere. Maybe we could also track a few more things explicitly, e.g. the refclock plls could probably be refcounted by their users (e.g. ddi plls). In the end I guess most of these need to be refcounted, and child power domains need to also grab a reference on their parents on the 0-1 transition. One idea I'm pondering is whether we should make all those power domains explicit by mapping them to platform devices we create. Upsides: - Someone clearly thought about funy races and locking issues when writing the pm runtime stuff. Using it would avoid us reinventing that wheel. - struct device is the main object for pm in linux, so if we can make our aux device (like i2c) childs of the correct power domain (what I've called aux display) instead of the pci device we might gain a bit in integration. - Full runtime PM at the pci level seems to be on the table anyway. - move your c8 state into a struct: struct i915_package_c8 { struct mutex mutex; union { struct hsw_package_c8_registers hsw_regfile; }; bool enabled; /* was allowing_package_c8 */ int forcewake; /* was c8_forbid_refcnt */ }; - The interrupt register handling is messy. We already track the values of the ones that we change, the others should be static. Looks mostly correct (barring a race with an interrupt!). I've inked in today for starting my irq review (thanks to vecs), I think we should do that first and then look again at the irq prep patches. But I agree that in theory we should only need to clear out the unused interrupts once in preinstall. For paranoia we could add a few WARNs in the uninstall hooks. - Hooking into modeset_global_resources seems strange and never explained. Modeset is a bit ugly for power domain refcounting (it's the same mess for the power well as for pc8+ here). The issue I think is that we don't have a resource acquire/release stage where platform code could hook in. crtc_disable would be at the right spot, but is also called with the dpms off stuff (so not suitable for everything). Off isn't called unconditionally when shutting down a pipe for real (if we reuse the pipe the cleanup is done in -mode_set). And crtc_enable is too late for some cases since global_modeset_resources and crtc mode_set already need to touch the hw. Maybe we could use the for_each_crtc loop we already have and grab a refcount for each crtc. If we switch the power well code to refcounting that would nicely unify things, too. Another issue I'm not yet clear on is dpms handling. Atm we ignore all that stuff for it (since neither modeset_global_resources nor mode_set hooks are called, and the hw seems to completely forget register contents, so this must be done). Probably the simplest solution is to call these hooks in the dpms on path, too. But atm that plan runs afoul of the resource accounting we still have in there (pll sharing and refcounting mostly). Tbh I don't have a clear overall idea for this yet. -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/2] drm/crtc-helper: clamp unknown connector status in the poll work
On Thu, Jun 06, 2013 at 12:17:26AM +0200, Daniel Vetter wrote: On some chipset we try to avoid possibly invasive output detection methods (like load detect which can cause flickering elsewhere) in the output poll work. Drivers could hence return unknown when a previous full -detect call returned a different state. This change will generate a hotplug event, forcing userspace to do a full scan. This in turn updates the connector-status field so that we will _again_ get a state change when the hotplug work re-runs in 10 seconds. To avoid this ping-pong loop detect this situation and clamp the connector state to the old value. Patch is inspired by a patch from Knut Peterson. Knut's patch completely ignored connector state changes if either the old or new status was unknown, which seemed to be a bit too agressive to me. References: http://lists.freedesktop.org/archives/dri-devel/2012-August/025975.html Cc: Knut Petersen knut_peter...@t-online.de Cc: Alex Deucher alexander.deuc...@amd.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch Also Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk I don't think this has any effect for i915, the circumstances where it might we return connector-status rather than Unknown, so we need some input from nouveau/radeon/architect as to our sanity. -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/crtc-helper: clamp unknown connector status in the poll work
On Thu, Jun 6, 2013 at 9:49 AM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Thu, Jun 06, 2013 at 12:17:26AM +0200, Daniel Vetter wrote: On some chipset we try to avoid possibly invasive output detection methods (like load detect which can cause flickering elsewhere) in the output poll work. Drivers could hence return unknown when a previous full -detect call returned a different state. This change will generate a hotplug event, forcing userspace to do a full scan. This in turn updates the connector-status field so that we will _again_ get a state change when the hotplug work re-runs in 10 seconds. To avoid this ping-pong loop detect this situation and clamp the connector state to the old value. Patch is inspired by a patch from Knut Peterson. Knut's patch completely ignored connector state changes if either the old or new status was unknown, which seemed to be a bit too agressive to me. References: http://lists.freedesktop.org/archives/dri-devel/2012-August/025975.html Cc: Knut Petersen knut_peter...@t-online.de Cc: Alex Deucher alexander.deuc...@amd.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch Also Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk I don't think this has any effect for i915, the circumstances where it might we return connector-status rather than Unknown, so we need some input from nouveau/radeon/architect as to our sanity. Hm, now I'm confused. The case I've had in mind indeed already works on i915.ko, but Knut's bug report was on a i915gm. The only place where we return unknown is when force==true and we can't get a load detect pipe. That shouldn't be able to ping-pong ... Knut, can you please shed some clue on me 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
[Intel-gfx] [PATCH] drm/i915: clean up vlv -pre_pll_enable and pll enable sequence
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. Cc: Jani Nikula jani.nik...@intel.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_display.c | 45 +++- 1 file changed, 18 insertions(+), 27 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 5e43b9a..6e4d666 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -1268,32 +1268,38 @@ static void assert_pch_ports_disabled(struct drm_i915_private *dev_priv, assert_pch_hdmi_disabled(dev_priv, pipe, PCH_HDMID); } -static void vlv_enable_pll(struct drm_i915_private *dev_priv, enum pipe pipe) +static void vlv_enable_pll(struct intel_crtc *crtc) { - int reg; - u32 val; + struct drm_device *dev = crtc-base.dev; + struct drm_i915_private *dev_priv = dev-dev_private; + int reg = DPLL(crtc-pipe); + u32 dpll = crtc-config.dpll_hw_state.dpll; - assert_pipe_disabled(dev_priv, pipe); + assert_pipe_disabled(dev_priv, crtc-pipe); /* No really, not for ILK+ */ BUG_ON(!IS_VALLEYVIEW(dev_priv-dev)); /* PLL is protected by panel, make sure we can write it */ if (IS_MOBILE(dev_priv-dev) !IS_I830(dev_priv-dev)) - assert_panel_unlocked(dev_priv, pipe); + assert_panel_unlocked(dev_priv, crtc-pipe); + + I915_WRITE(reg, dpll); + POSTING_READ(reg); + udelay(150); + + if (wait_for(((I915_READ(reg) DPLL_LOCK_VLV) == DPLL_LOCK_VLV), 1)) + DRM_ERROR(DPLL %d failed to lock\n, crtc-pipe); - reg = DPLL(pipe); - val = I915_READ(reg); - val |= DPLL_VCO_ENABLE; /* We do this three times for luck */ - I915_WRITE(reg, val); + I915_WRITE(reg, dpll); POSTING_READ(reg); udelay(150); /* wait for warmup */ - I915_WRITE(reg, val); + I915_WRITE(reg, dpll); POSTING_READ(reg); udelay(150); /* wait for warmup */ - I915_WRITE(reg, val); + I915_WRITE(reg, dpll); POSTING_READ(reg); udelay(150); /* wait for warmup */ } @@ -3561,7 +3567,7 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc) if (encoder-pre_pll_enable) encoder-pre_pll_enable(encoder); - vlv_enable_pll(dev_priv, pipe); + vlv_enable_pll(intel_crtc); for_each_encoder_on_crtc(dev, crtc, encoder) if (encoder-pre_enable) @@ -4315,7 +4321,6 @@ static void vlv_update_pll(struct intel_crtc *crtc) { struct drm_device *dev = crtc-base.dev; struct drm_i915_private *dev_priv = dev-dev_private; - struct intel_encoder *encoder; int pipe = crtc-pipe; u32 dpll, mdiv; u32 bestn, bestm1, bestm2, bestp1, bestp2; @@ -4403,10 +4408,6 @@ static void vlv_update_pll(struct intel_crtc *crtc) vlv_dpio_write(dev_priv, DPIO_PLL_CML(pipe), 0x87871000); - for_each_encoder_on_crtc(dev, crtc-base, encoder) - if (encoder-pre_pll_enable) - encoder-pre_pll_enable(encoder); - /* Enable DPIO clock input */ dpll = DPLL_EXT_BUFFER_ENABLE_VLV | DPLL_REFA_CLK_ENABLE_VLV | DPLL_VGA_MODE_DIS | DPLL_INTEGRATED_CLOCK_VLV; @@ -4416,20 +4417,10 @@ static void vlv_update_pll(struct intel_crtc *crtc) dpll |= DPLL_VCO_ENABLE; crtc-config.dpll_hw_state.dpll = dpll; - I915_WRITE(DPLL(pipe), dpll); - POSTING_READ(DPLL(pipe)); - udelay(150); - - if (wait_for(((I915_READ(DPLL(pipe)) DPLL_LOCK_VLV) == DPLL_LOCK_VLV), 1)) - DRM_ERROR(DPLL %d failed to lock\n, pipe); - dpll_md = (crtc-config.pixel_multiplier - 1) DPLL_MD_UDI_MULTIPLIER_SHIFT; crtc-config.dpll_hw_state.dpll_md = dpll_md; - I915_WRITE(DPLL_MD(pipe), dpll_md); - POSTING_READ(DPLL_MD(pipe)); - if (crtc-config.has_dp_encoder) intel_dp_set_m_n(crtc); -- 1.7.11.7 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: Don't count semaphore waits towards a stuck ring
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. 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 | 66 +++ drivers/gpu/drm/i915/intel_ringbuffer.h |1 + 2 files changed, 51 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 3b3f080..c046980 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2341,21 +2341,21 @@ static bool i915_hangcheck_ring_idle(struct intel_ring_buffer *ring, return false; } -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); @@ -2364,12 +2364,40 @@ 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 i915_hangcheck_ring_hung(struct intel_ring_buffer *ring) @@ -2394,13 +2422,17 @@ static bool i915_hangcheck_ring_hung(struct intel_ring_buffer *ring) return false; } - if (INTEL_INFO(dev)-gen = 6 - tmp RING_WAIT_SEMAPHORE - semaphore_passed(ring)) { - DRM_ERROR(Kicking stuck semaphore on %s\n, - ring-name); - I915_WRITE_CTL(ring, tmp); - return false; + if (INTEL_INFO(dev)-gen = 6 tmp RING_WAIT_SEMAPHORE) { + switch (semaphore_passed(ring)) { + default: + return true; + case 1: + DRM_ERROR(Kicking stuck semaphore on %s\n, + ring-name); + I915_WRITE_CTL(ring, tmp); + case 0: + return false; + } } return true; @@ -2430,6 +2462,8 @@ void
[Intel-gfx] [PATCH 1/2] drm/i915: Reset hangcheck score if we succesfully kick a ring
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. 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 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 | 47 --- 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 85694d7..3b3f080 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2372,16 +2372,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 i915_hangcheck_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 +2400,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; } /** @@ -2445,15 +2443,18 @@ void i915_hangcheck_elapsed(unsigned long data) } else { busy_count++; + err = false; if (ring-hangcheck.seqno == seqno) { - ring-hangcheck.score++; - - /* Kick ring if stuck*/ + /* Kick ring if stuck */ + err = true; if (stuck[i]) - i915_hangcheck_ring_hung(ring); - } else { - ring-hangcheck.score = 0; + err = i915_hangcheck_ring_hung(ring); } + + if (err) + ring-hangcheck.score++; + else + ring-hangcheck.score = 0; } ring-hangcheck.seqno = seqno; -- 1.7.10.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: VGA also requires the power well
On Wed, Jun 05, 2013 at 06:05:51PM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com So add a power domain and check for it before we try to read VGA_CONTROL. This fixes unclaimed register messages that happen on suspend/resume. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_display.c | 3 +++ drivers/gpu/drm/i915/intel_pm.c | 1 + 3 files changed, 5 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 46b1f70..d51ce13 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -89,6 +89,7 @@ enum port { #define port_name(p) ((p) + 'A') enum intel_display_power_domain { + POWER_DOMAIN_VGA, POWER_DOMAIN_PIPE_A, POWER_DOMAIN_PIPE_B, POWER_DOMAIN_PIPE_C, diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 4c8fcec..3719d99 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9950,6 +9950,9 @@ void i915_redisable_vga(struct drm_device *dev) struct drm_i915_private *dev_priv = dev-dev_private; u32 vga_reg = i915_vgacntrl_reg(dev); + if (!intel_display_power_enabled(dev, POWER_DOMAIN_VGA)) + return; + So it looks like you're essentially making intel_redisable_vga() a nop for HSW. Shouldn't we instead enable the power well during resume? if (I915_READ(vga_reg) != VGA_DISP_DISABLE) { DRM_DEBUG_KMS(Something enabled VGA plane, disabling it\n); i915_disable_vga(dev); diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 50fe3d7..47ef4a6 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5000,6 +5000,7 @@ bool intel_display_power_enabled(struct drm_device *dev, case POWER_DOMAIN_PIPE_A: case POWER_DOMAIN_TRANSCODER_EDP: return true; + case POWER_DOMAIN_VGA: case POWER_DOMAIN_PIPE_B: case POWER_DOMAIN_PIPE_C: case POWER_DOMAIN_PIPE_A_PANEL_FITTER: -- 1.8.1.2 ___ 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
Re: [Intel-gfx] [PATCH] drm/i915: Track clients and print their object usage in debugfs
On Tue, Jun 04, 2013 at 04:24:48PM -0700, Ben Widawsky wrote: On Tue, Jun 04, 2013 at 11:49:08PM +0100, Chris Wilson wrote: By stashing a pointer of who opened the device and keeping a list of open fd, we can then walk each client and inspect how many objects they have open. For example, i915_gem_objects: 1102 objects, 613646336 bytes 663 [662] objects, 468783104 [468750336] bytes in gtt 37 [37] active objects, 46874624 [46874624] bytes 626 [625] inactive objects, 421908480 [421875712] bytes 282 unbound objects, 6512640 bytes 85 purgeable objects, 6787072 bytes 28 pinned mappable objects, 3686400 bytes 40 fault mappable objects, 27783168 bytes 2145386496 [536870912] gtt total Xorg: 43 objects, 32243712 bytes (10223616 active, 16683008 inactive, 4096 unbound) gnome-shell: 30 objects, 28381184 bytes (0 active, 28336128 inactive, 0 unbound) xonotic-linux64: 1032 objects, 569933824 bytes (46874624 active, 383545344 inactive, 6508544 unbound) v2: Use existing drm-filelist as pointed out by Ben. v3: Not even stashing the task_struct is required as Ben pointed out drm_file-pid. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Reviewed-by: Ben Widawsky b...@bwidawsk.net Queued for -next, thanks for the patch. Also, I'll make a mental note that dev-struct_mutex seems to protect even more than what I've expected ;-) -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
[Intel-gfx] [PATCH 2/2] drm/i915: Don't count semaphore waits towards a stuck ring
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. 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 | 66 +++ drivers/gpu/drm/i915/intel_ringbuffer.h |1 + 2 files changed, 51 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 2d1890d..948eb9f 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,12 +2354,40 @@ 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) @@ -2384,13 +2412,17 @@ static bool ring_hung(struct intel_ring_buffer *ring) return false; } - if (INTEL_INFO(dev)-gen = 6 - tmp RING_WAIT_SEMAPHORE - semaphore_passed(ring)) { - DRM_ERROR(Kicking stuck semaphore on %s\n, - ring-name); - I915_WRITE_CTL(ring, tmp); - return false; + if (INTEL_INFO(dev)-gen = 6 tmp RING_WAIT_SEMAPHORE) { + switch (semaphore_passed(ring)) { + default: + return true; + case 1: + DRM_ERROR(Kicking stuck semaphore on %s\n, + ring-name); + I915_WRITE_CTL(ring, tmp); + case 0: + return false; + } } return true; @@ -2422,6 +2454,8 @@ void i915_hangcheck_elapsed(unsigned
[Intel-gfx] [PATCH 1/2] drm/i915: Only slightly increment hangcheck score if we succesfully kick a ring
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. 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 | 105 --- 1 file changed, 53 insertions(+), 52 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 85694d7..2d1890d 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,37 +2411,50 @@ 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 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; seqno = ring-get_seqno(ring, false); acthd = intel_ring_get_active_head(ring); - idle = i915_hangcheck_ring_idle(ring, seqno, err); - stuck[i] = ring-hangcheck.acthd == acthd; - - if (idle) { - if (err) -
[Intel-gfx] [PATCH] Revert drm/i915: Include display_mmio_offset in sequencer index/data registers
From: Ville Syrjälä ville.syrj...@linux.intel.com We use port I/O for VGA register access, so adding display_mmio_offset is just wrong. This reverts commit 56a12a509296c87d6f149be86c6694d312b21d35. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/i915_reg.h | 10 ++ 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 5a593d2..acb61a4 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -147,15 +147,9 @@ #define VGA_MSR_MEM_EN (11) #define VGA_MSR_CGA_MODE (10) -/* - * SR01 is the only VGA register touched on non-UMS setups. - * VLV doesn't do UMS, so the sequencer index/data registers - * are the only VGA registers which need to include - * display_mmio_offset. - */ -#define VGA_SR_INDEX (dev_priv-info-display_mmio_offset + 0x3c4) +#define VGA_SR_INDEX 0x3c4 #define SR01 1 -#define VGA_SR_DATA (dev_priv-info-display_mmio_offset + 0x3c5) +#define VGA_SR_DATA 0x3c5 #define VGA_AR_INDEX 0x3c0 #define VGA_AR_VID_EN (15) -- 1.8.1.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [igt PATCH 5/5] tests: add kms_render
On Wed, 2013-06-05 at 15:28 -0300, Rodrigo Vivi wrote: nice tests, only now I understood why Daniel randomly volunteered me to review this series ;) Reviewed-by: Rodrigo Vivi rodrigo.v...@gmail.com Thanks for the review, the patchset is pushed now to igt. --Imre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: hw state readout support for pixel_multiplier
Incomplete since ilk+ support needs proper pch dpll tracking first. SDVO get_config parts based on a patch from Jesse Barnes, but fixed up to actually work. v2: Make sure that we call encoder-get_config _after_ we get_pipe_config to be consistent in both setup_hw_state and the modeset state checker. Otherwise the clever trick with handling the pixel mutliplier on i915G/GM where the encoder overrides the default value of 1 from the crtc get_pipe_config function doesn't work. Spotted by Imre Deak. v3: Actually cross-check the pixel mutliplier (but not on pch split platforms for now). Now actually also tested on a i915G with a sdvo encoder plugged in. Cc: imre.d...@intel.com Cc: Jesse Barnes jbar...@virtuousgeek.org Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_display.c | 38 ++-- drivers/gpu/drm/i915/intel_sdvo.c| 30 +++- 2 files changed, 65 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 40d047e..a1a81b4 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4962,6 +4962,23 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc, i9xx_get_pfit_config(crtc, pipe_config); + if (INTEL_INFO(dev)-gen = 4) { + tmp = I915_READ(DPLL_MD(crtc-pipe)); + pipe_config-pixel_multiplier = + ((tmp DPLL_MD_UDI_MULTIPLIER_MASK) + DPLL_MD_UDI_MULTIPLIER_SHIFT) + 1; + } else if (IS_I945G(dev) || IS_I945GM(dev) || IS_G33(dev)) { + tmp = I915_READ(DPLL(crtc-pipe)); + pipe_config-pixel_multiplier = + ((tmp SDVO_MULTIPLIER_MASK) + SDVO_MULTIPLIER_SHIFT_HIRES) + 1; + } else { + /* Note that on i915G/GM the pixel multiplier is in the sdvo +* port and will be fixed up in the encoder-get_config +* function. */ + pipe_config-pixel_multiplier = 1; + } + return true; } @@ -5825,6 +5842,12 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc, FDI_DP_PORT_WIDTH_SHIFT) + 1; ironlake_get_fdi_m_n_config(crtc, pipe_config); + + /* XXX: Can't properly read out the pch dpll pixel multiplier +* since we don't have state tracking for pch clocks yet. */ + pipe_config-pixel_multiplier = 1; + } else { + pipe_config-pixel_multiplier = 1; } intel_get_pipe_timings(crtc, pipe_config); @@ -5959,6 +5982,8 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc, pipe_config-ips_enabled = hsw_crtc_supports_ips(crtc) (I915_READ(IPS_CTL) IPS_ENABLE); + pipe_config-pixel_multiplier = 1; + return true; } @@ -8048,6 +8073,9 @@ intel_pipe_config_compare(struct drm_device *dev, PIPE_CONF_CHECK_I(adjusted_mode.crtc_vsync_start); PIPE_CONF_CHECK_I(adjusted_mode.crtc_vsync_end); + if (!HAS_PCH_SPLIT(dev)) + PIPE_CONF_CHECK_I(pixel_multiplier); + PIPE_CONF_CHECK_FLAGS(adjusted_mode.flags, DRM_MODE_FLAG_INTERLACE); @@ -8169,9 +8197,8 @@ intel_modeset_check_state(struct drm_device *dev) enabled = true; if (encoder-connectors_active) active = true; - if (encoder-get_config) - encoder-get_config(encoder, pipe_config); } + WARN(active != crtc-active, crtc's computed active state doesn't match tracked active state (expected %i, found %i)\n, active, crtc-active); @@ -8181,6 +8208,13 @@ intel_modeset_check_state(struct drm_device *dev) active = dev_priv-display.get_pipe_config(crtc, pipe_config); + list_for_each_entry(encoder, dev-mode_config.encoder_list, + base.head) { + if (encoder-base.crtc != crtc-base) + continue; + if (encoder-get_config) + encoder-get_config(encoder, pipe_config); + } /* hw state is inconsistent with the pipe A quirk */ if (crtc-pipe == PIPE_A dev_priv-quirks QUIRK_PIPEA_FORCE) diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c index 820f1ae..d4560ad 100644 --- a/drivers/gpu/drm/i915/intel_sdvo.c +++ b/drivers/gpu/drm/i915/intel_sdvo.c @@ -1313,9 +1313,13 @@ static bool intel_sdvo_get_hw_state(struct intel_encoder *encoder, static void intel_sdvo_get_config(struct
Re: [Intel-gfx] [PATCH] Revert drm/i915: Include display_mmio_offset in sequencer index/data registers
On Thu, Jun 06, 2013 at 01:09:32PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com We use port I/O for VGA register access, so adding display_mmio_offset is just wrong. This reverts commit 56a12a509296c87d6f149be86c6694d312b21d35. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com Queued for -next, thanks for the patch. Since vlv is only considered useful in 3.11 no cc: stable for this. -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 1/6] drm/i915: add ibx_irq_preinstall
On Wed, Jun 05, 2013 at 02:21:51PM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com So we can remove some duplicate code. All the PCHs are very similar and right now the code is the same. I plan to add more code, so we would have more duplicated code. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com Queued for -next, thanks for the patch. For the other prep patches I'd like to review our interrupt code first a bit, like describe in my reply to the vecs enabling patches. -Daniel --- drivers/gpu/drm/i915/i915_irq.c | 44 - 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 63996aa..c482e8a 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2472,6 +2472,25 @@ void i915_hangcheck_elapsed(unsigned long data) DRM_I915_HANGCHECK_JIFFIES)); } +static void ibx_irq_preinstall(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev-dev_private; + + if (HAS_PCH_NOP(dev)) + return; + + /* south display irq */ + I915_WRITE(SDEIMR, 0x); + /* + * SDEIER is also touched by the interrupt handler to work around missed + * PCH interrupts. Hence we can't update it after the interrupt handler + * is enabled - instead we unconditionally enable all PCH interrupt + * sources here, but then only unmask them as needed with SDEIMR. + */ + I915_WRITE(SDEIER, 0x); + POSTING_READ(SDEIER); +} + /* drm_dma.h hooks */ static void ironlake_irq_preinstall(struct drm_device *dev) @@ -2493,16 +2512,7 @@ static void ironlake_irq_preinstall(struct drm_device *dev) I915_WRITE(GTIER, 0x0); POSTING_READ(GTIER); - /* south display irq */ - I915_WRITE(SDEIMR, 0x); - /* - * SDEIER is also touched by the interrupt handler to work around missed - * PCH interrupts. Hence we can't update it after the interrupt handler - * is enabled - instead we unconditionally enable all PCH interrupt - * sources here, but then only unmask them as needed with SDEIMR. - */ - I915_WRITE(SDEIER, 0x); - POSTING_READ(SDEIER); + ibx_irq_preinstall(dev); } static void ivybridge_irq_preinstall(struct drm_device *dev) @@ -2529,19 +2539,7 @@ static void ivybridge_irq_preinstall(struct drm_device *dev) I915_WRITE(GEN6_PMIER, 0x0); POSTING_READ(GEN6_PMIER); - if (HAS_PCH_NOP(dev)) - return; - - /* south display irq */ - I915_WRITE(SDEIMR, 0x); - /* - * SDEIER is also touched by the interrupt handler to work around missed - * PCH interrupts. Hence we can't update it after the interrupt handler - * is enabled - instead we unconditionally enable all PCH interrupt - * sources here, but then only unmask them as needed with SDEIMR. - */ - I915_WRITE(SDEIER, 0x); - POSTING_READ(SDEIER); + ibx_irq_preinstall(dev); } static void valleyview_irq_preinstall(struct drm_device *dev) -- 1.8.1.2 ___ 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] drm/i915: hw state readout support for pixel_multiplier
On Thu, 2013-06-06 at 12:45 +0200, Daniel Vetter wrote: Incomplete since ilk+ support needs proper pch dpll tracking first. SDVO get_config parts based on a patch from Jesse Barnes, but fixed up to actually work. v2: Make sure that we call encoder-get_config _after_ we get_pipe_config to be consistent in both setup_hw_state and the modeset state checker. Otherwise the clever trick with handling the pixel mutliplier on i915G/GM where the encoder overrides the default value of 1 from the crtc get_pipe_config function doesn't work. Spotted by Imre Deak. v3: Actually cross-check the pixel mutliplier (but not on pch split platforms for now). Now actually also tested on a i915G with a sdvo encoder plugged in. Cc: imre.d...@intel.com Cc: Jesse Barnes jbar...@virtuousgeek.org Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch Looks ok, Reviewed-by: Imre Deak imre.d...@intel.com ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 8/9] drm/i915: Spruce up assert_sprites_disabled()
On Wed, Jun 05, 2013 at 04:39:58PM -0300, Rodrigo Vivi wrote: Reviewed-by: Rodrigo Vivi rodrigo.v...@gmail.com On Tue, Jun 4, 2013 at 7:49 AM, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com Make assert_sprites_disabled() operational on all platforms where we currently have sprite support enabled. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com I guess we eventually need some more abstract hw state readout for planes, similar to what we have on the output routing side. But this is good enough for now, so merged. -Daniel --- drivers/gpu/drm/i915/intel_display.c | 27 +++ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 9c89ddf..90d02c7 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -1132,19 +1132,30 @@ static void assert_planes_disabled(struct drm_i915_private *dev_priv, static void assert_sprites_disabled(struct drm_i915_private *dev_priv, enum pipe pipe) { + struct drm_device *dev = dev_priv-dev; int reg, i; u32 val; - if (!IS_VALLEYVIEW(dev_priv-dev)) - return; - - /* Need to check both planes against the pipe */ - for (i = 0; i dev_priv-num_plane; i++) { - reg = SPCNTR(pipe, i); + if (IS_VALLEYVIEW(dev)) { + for (i = 0; i dev_priv-num_plane; i++) { + reg = SPCNTR(pipe, i); + val = I915_READ(reg); + WARN((val SP_ENABLE), +sprite %c assertion failure, should be off on pipe %c but is still active\n, +sprite_name(pipe, i), pipe_name(pipe)); + } + } else if (INTEL_INFO(dev)-gen = 7) { + reg = SPRCTL(pipe); + val = I915_READ(reg); + WARN((val SPRITE_ENABLE), +sprite %c assertion failure, should be off on pipe %c but is still active\n, +plane_name(pipe), pipe_name(pipe)); + } else if (INTEL_INFO(dev)-gen = 5) { + reg = DVSCNTR(pipe); val = I915_READ(reg); - WARN((val SP_ENABLE), + WARN((val DVS_ENABLE), sprite %c assertion failure, should be off on pipe %c but is still active\n, -sprite_name(pipe, i), pipe_name(pipe)); +plane_name(pipe), pipe_name(pipe)); } } -- 1.8.1.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Rodrigo Vivi Blog: http://blog.vivi.eng.br ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- 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 9/9] drm/i915: Assert dpll running in intel_crtc_load_lut() on pre-PCH platforms
On Wed, Jun 05, 2013 at 10:58:06PM +0300, Ville Syrjälä wrote: On Wed, Jun 05, 2013 at 04:41:54PM -0300, Rodrigo Vivi wrote: why is this needed? The spec says that on some hardware you need to PLL running before you can poke at the palette registers. I didn't actually try to anger the hardware so I'm not really sure what would happen otherwise, but IIRC Jesse said something about a hard system hang... I've added this to the commit message and merged the entire pile. Thanks for patchesreview. -Daniel anyways: Reviewed-by: Rodrigo Vivi rodrigo.v...@gmail.com On Tue, Jun 4, 2013 at 7:49 AM, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_display.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 90d02c7..3be69bc 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6340,6 +6340,9 @@ void intel_crtc_load_lut(struct drm_crtc *crtc) if (!crtc-enabled || !intel_crtc-active) return; + if (!HAS_PCH_SPLIT(dev_priv-dev)) + assert_pll_enabled(dev_priv, pipe); + /* use legacy palette for Ironlake */ if (HAS_PCH_SPLIT(dev)) palreg = LGC_PALETTE(pipe); -- 1.8.1.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Rodrigo Vivi Blog: http://blog.vivi.eng.br -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/4] drm/i915: export error state to string conversion
In preparation for accessing error state from sysfs, export error state to string conversion function. Also tuck buffer error handling inside the function. Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com --- drivers/gpu/drm/i915/i915_debugfs.c | 24 drivers/gpu/drm/i915/i915_drv.h |7 +++ 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 0e7e3c0..77bfd51 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -739,15 +739,8 @@ static void i915_ring_error_state(struct drm_i915_error_state_buf *m, err_printf(m, ring-tail: 0x%08x\n, error-cpu_ring_tail[ring]); } -struct i915_error_state_file_priv { - struct drm_device *dev; - struct drm_i915_error_state *error; -}; - - -static int i915_error_state(struct i915_error_state_file_priv *error_priv, - struct drm_i915_error_state_buf *m) - +int i915_error_state_to_str(struct drm_i915_error_state_buf *m, + const struct i915_error_state_file_priv *error_priv) { struct drm_device *dev = error_priv-dev; drm_i915_private_t *dev_priv = dev-dev_private; @@ -757,7 +750,7 @@ static int i915_error_state(struct i915_error_state_file_priv *error_priv, if (!error) { err_printf(m, no error state collected\n); - return 0; + goto out; } err_printf(m, Time: %ld s %ld us\n, error-time.tv_sec, @@ -867,6 +860,10 @@ static int i915_error_state(struct i915_error_state_file_priv *error_priv, if (error-display) intel_display_print_error_state(m, dev, error-display); +out: + if (m-bytes == 0 m-err) + return m-err; + return 0; } @@ -960,15 +957,10 @@ static ssize_t i915_error_state_read(struct file *file, char __user *userbuf, error_str.start = *pos; - ret = i915_error_state(error_priv, error_str); + ret = i915_error_state_to_str(error_str, error_priv); if (ret) goto out; - if (error_str.bytes == 0 error_str.err) { - ret = error_str.err; - goto out; - } - ret_count = simple_read_from_buffer(userbuf, count, tmp_pos, error_str.buf, error_str.bytes); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 215aa63..538b36b 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -857,6 +857,11 @@ struct drm_i915_error_state_buf { loff_t pos; }; +struct i915_error_state_file_priv { + struct drm_device *dev; + struct drm_i915_error_state *error; +}; + struct i915_gpu_error { /* For hangcheck timer */ #define DRM_I915_HANGCHECK_PERIOD 1500 /* in ms */ @@ -1850,6 +1855,8 @@ int i915_debugfs_init(struct drm_minor *minor); void i915_debugfs_cleanup(struct drm_minor *minor); __printf(2, 3) void i915_error_printf(struct drm_i915_error_state_buf *e, const char *f, ...); +int i915_error_state_to_str(struct drm_i915_error_state_buf *estr, + const struct i915_error_state_file_priv *error); /* i915_suspend.c */ extern int i915_save_state(struct drm_device *dev); -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/4] drm/i915: add i915_error_state sysfs entry
As getting error state doesn't anymore require big kmallocs, make error state accessible also from sysfs. Suggested-by: Daniel Vetter daniel.vet...@ffwll.ch Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com --- drivers/gpu/drm/i915/i915_sysfs.c | 49 + 1 file changed, 49 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c index 6875b56..d4c754b 100644 --- a/drivers/gpu/drm/i915/i915_sysfs.c +++ b/drivers/gpu/drm/i915/i915_sysfs.c @@ -409,6 +409,49 @@ static const struct attribute *gen6_attrs[] = { NULL, }; +static ssize_t error_state_read(struct file *filp, struct kobject *kobj, + struct bin_attribute *attr, char *buf, + loff_t off, size_t count) +{ + + struct device *kdev = container_of(kobj, struct device, kobj); + struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev); + struct drm_device *dev = minor-dev; + struct i915_error_state_file_priv error_priv; + struct drm_i915_error_state_buf error_str; + ssize_t ret_count = 0; + int ret; + + memset(error_priv, 0, sizeof(error_priv)); + + ret = i915_error_state_buf_init(error_str, count, off); + if (ret) + return ret; + + error_priv.dev = dev; + i915_error_state_get(dev, error_priv); + + ret = i915_error_state_to_str(error_str, error_priv); + if (ret) + goto out; + + ret_count = count error_str.bytes ? count : error_str.bytes; + + memcpy(buf, error_str.buf, ret_count); +out: + i915_error_state_buf_release(error_str); + i915_error_state_put(error_priv); + + return ret ?: ret_count; +} + +static struct bin_attribute error_state_attr = { + .attr.name = i915_error_state, + .attr.mode = 0444, + .size = 0, + .read = error_state_read, +}; + void i915_setup_sysfs(struct drm_device *dev) { int ret; @@ -432,10 +475,16 @@ void i915_setup_sysfs(struct drm_device *dev) if (ret) DRM_ERROR(gen6 sysfs setup failed\n); } + + ret = sysfs_create_bin_file(dev-primary-kdev.kobj, + error_state_attr); + if (ret) + DRM_ERROR(error_state sysfs setup failed\n); } void i915_teardown_sysfs(struct drm_device *dev) { + sysfs_remove_bin_file(dev-primary-kdev.kobj, error_state_attr); sysfs_remove_files(dev-primary-kdev.kobj, gen6_attrs); device_remove_bin_file(dev-primary-kdev, dpf_attrs); #ifdef CONFIG_PM -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/4] drm/i915: export error state ref handling
In preparation for sysfs error state access, export ref error state ref counting interface. Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com --- drivers/gpu/drm/i915/i915_debugfs.c | 31 ++- drivers/gpu/drm/i915/i915_drv.h |3 +++ 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 77bfd51..1e22820 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -889,12 +889,30 @@ i915_error_state_write(struct file *filp, return cnt; } +void i915_error_state_get(struct drm_device *dev, + struct i915_error_state_file_priv *error_priv) +{ + struct drm_i915_private *dev_priv = dev-dev_private; + unsigned long flags; + + spin_lock_irqsave(dev_priv-gpu_error.lock, flags); + error_priv-error = dev_priv-gpu_error.first_error; + if (error_priv-error) + kref_get(error_priv-error-ref); + spin_unlock_irqrestore(dev_priv-gpu_error.lock, flags); + +} + +void i915_error_state_put(struct i915_error_state_file_priv *error_priv) +{ + if (error_priv-error) + kref_put(error_priv-error-ref, i915_error_state_free); +} + static int i915_error_state_open(struct inode *inode, struct file *file) { struct drm_device *dev = inode-i_private; - drm_i915_private_t *dev_priv = dev-dev_private; struct i915_error_state_file_priv *error_priv; - unsigned long flags; error_priv = kzalloc(sizeof(*error_priv), GFP_KERNEL); if (!error_priv) @@ -902,11 +920,7 @@ static int i915_error_state_open(struct inode *inode, struct file *file) error_priv-dev = dev; - spin_lock_irqsave(dev_priv-gpu_error.lock, flags); - error_priv-error = dev_priv-gpu_error.first_error; - if (error_priv-error) - kref_get(error_priv-error-ref); - spin_unlock_irqrestore(dev_priv-gpu_error.lock, flags); + i915_error_state_get(dev, error_priv); file-private_data = error_priv; @@ -917,8 +931,7 @@ static int i915_error_state_release(struct inode *inode, struct file *file) { struct i915_error_state_file_priv *error_priv = file-private_data; - if (error_priv-error) - kref_put(error_priv-error-ref, i915_error_state_free); + i915_error_state_put(error_priv); kfree(error_priv); return 0; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 538b36b..adc2841 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1857,6 +1857,9 @@ __printf(2, 3) void i915_error_printf(struct drm_i915_error_state_buf *e, const char *f, ...); int i915_error_state_to_str(struct drm_i915_error_state_buf *estr, const struct i915_error_state_file_priv *error); +void i915_error_state_get(struct drm_device *dev, + struct i915_error_state_file_priv *error_priv); +void i915_error_state_put(struct i915_error_state_file_priv *error_priv); /* i915_suspend.c */ extern int i915_save_state(struct drm_device *dev); -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/4] drm/i915: introduce i915_error_state_buf_init
Make function for struct i915_error_state_buf initialization and export it, for sysfs and debugfs. Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com --- drivers/gpu/drm/i915/i915_debugfs.c | 50 +-- drivers/gpu/drm/i915/i915_drv.h |7 + 2 files changed, 37 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 1e22820..6ea6747 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -937,38 +937,48 @@ static int i915_error_state_release(struct inode *inode, struct file *file) return 0; } -static ssize_t i915_error_state_read(struct file *file, char __user *userbuf, -size_t count, loff_t *pos) +int i915_error_state_buf_init(struct drm_i915_error_state_buf *ebuf, + size_t count, loff_t pos) { - struct i915_error_state_file_priv *error_priv = file-private_data; - struct drm_i915_error_state_buf error_str; - loff_t tmp_pos = 0; - ssize_t ret_count = 0; - int ret = 0; - - memset(error_str, 0, sizeof(error_str)); + memset(ebuf, 0, sizeof(*ebuf)); /* We need to have enough room to store any i915_error_state printf * so that we can move it to start position. */ - error_str.size = count + 1 PAGE_SIZE ? count + 1 : PAGE_SIZE; - error_str.buf = kmalloc(error_str.size, + ebuf-size = count + 1 PAGE_SIZE ? count + 1 : PAGE_SIZE; + ebuf-buf = kmalloc(ebuf-size, GFP_TEMPORARY | __GFP_NORETRY | __GFP_NOWARN); - if (error_str.buf == NULL) { - error_str.size = PAGE_SIZE; - error_str.buf = kmalloc(error_str.size, GFP_TEMPORARY); + if (ebuf-buf == NULL) { + ebuf-size = PAGE_SIZE; + ebuf-buf = kmalloc(ebuf-size, GFP_TEMPORARY); } - if (error_str.buf == NULL) { - error_str.size = 128; - error_str.buf = kmalloc(error_str.size, GFP_TEMPORARY); + if (ebuf-buf == NULL) { + ebuf-size = 128; + ebuf-buf = kmalloc(ebuf-size, GFP_TEMPORARY); } - if (error_str.buf == NULL) + if (ebuf-buf == NULL) return -ENOMEM; - error_str.start = *pos; + ebuf-start = pos; + + return 0; +} + +static ssize_t i915_error_state_read(struct file *file, char __user *userbuf, +size_t count, loff_t *pos) +{ + struct i915_error_state_file_priv *error_priv = file-private_data; + struct drm_i915_error_state_buf error_str; + loff_t tmp_pos = 0; + ssize_t ret_count = 0; + int ret; + + ret = i915_error_state_buf_init(error_str, count, *pos); + if (ret) + return ret; ret = i915_error_state_to_str(error_str, error_priv); if (ret) @@ -983,7 +993,7 @@ static ssize_t i915_error_state_read(struct file *file, char __user *userbuf, else *pos = error_str.start + ret_count; out: - kfree(error_str.buf); + i915_error_state_buf_release(error_str); return ret ?: ret_count; } diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index adc2841..b8e16d1 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1860,6 +1860,13 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *estr, void i915_error_state_get(struct drm_device *dev, struct i915_error_state_file_priv *error_priv); void i915_error_state_put(struct i915_error_state_file_priv *error_priv); +int i915_error_state_buf_init(struct drm_i915_error_state_buf *eb, + size_t count, loff_t pos); +static inline void i915_error_state_buf_release( + struct drm_i915_error_state_buf *eb) +{ + kfree(eb-buf); +} /* i915_suspend.c */ extern int i915_save_state(struct drm_device *dev); -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/4] drm/i915: add i915_error_state sysfs entry
On Thu, Jun 06, 2013 at 03:18:42PM +0300, Mika Kuoppala wrote: As getting error state doesn't anymore require big kmallocs, make error state accessible also from sysfs. Suggested-by: Daniel Vetter daniel.vet...@ffwll.ch Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com Don't forget the ability to clear an error-state in order to capture a fresh one. -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 4/4] drm/i915: add i915_error_state sysfs entry
On Thu, Jun 06, 2013 at 03:18:42PM +0300, Mika Kuoppala wrote: As getting error state doesn't anymore require big kmallocs, make error state accessible also from sysfs. Suggested-by: Daniel Vetter daniel.vet...@ffwll.ch Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com --- drivers/gpu/drm/i915/i915_sysfs.c | 49 + 1 file changed, 49 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c index 6875b56..d4c754b 100644 --- a/drivers/gpu/drm/i915/i915_sysfs.c +++ b/drivers/gpu/drm/i915/i915_sysfs.c @@ -409,6 +409,49 @@ static const struct attribute *gen6_attrs[] = { NULL, }; +static ssize_t error_state_read(struct file *filp, struct kobject *kobj, + struct bin_attribute *attr, char *buf, + loff_t off, size_t count) +{ + + struct device *kdev = container_of(kobj, struct device, kobj); + struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev); + struct drm_device *dev = minor-dev; + struct i915_error_state_file_priv error_priv; + struct drm_i915_error_state_buf error_str; + ssize_t ret_count = 0; + int ret; + + memset(error_priv, 0, sizeof(error_priv)); + + ret = i915_error_state_buf_init(error_str, count, off); + if (ret) + return ret; + + error_priv.dev = dev; + i915_error_state_get(dev, error_priv); + + ret = i915_error_state_to_str(error_str, error_priv); + if (ret) + goto out; + + ret_count = count error_str.bytes ? count : error_str.bytes; + + memcpy(buf, error_str.buf, ret_count); +out: + i915_error_state_buf_release(error_str); + i915_error_state_put(error_priv); + + return ret ?: ret_count; +} + +static struct bin_attribute error_state_attr = { + .attr.name = i915_error_state, + .attr.mode = 0444, Error state could easily leak sensitive information, so I think we should only allow root to read it. Also maybe a new name for it, i915_ is kinda meh ... I vote for gpu_error_state. -Daniel + .size = 0, + .read = error_state_read, +}; + void i915_setup_sysfs(struct drm_device *dev) { int ret; @@ -432,10 +475,16 @@ void i915_setup_sysfs(struct drm_device *dev) if (ret) DRM_ERROR(gen6 sysfs setup failed\n); } + + ret = sysfs_create_bin_file(dev-primary-kdev.kobj, + error_state_attr); + if (ret) + DRM_ERROR(error_state sysfs setup failed\n); } void i915_teardown_sysfs(struct drm_device *dev) { + sysfs_remove_bin_file(dev-primary-kdev.kobj, error_state_attr); sysfs_remove_files(dev-primary-kdev.kobj, gen6_attrs); device_remove_bin_file(dev-primary-kdev, dpf_attrs); #ifdef CONFIG_PM -- 1.7.9.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 2/2] drm/crtc-helper: clamp unknown connector status in the poll work
On Thu, Jun 6, 2013 at 3:49 AM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Thu, Jun 06, 2013 at 12:17:26AM +0200, Daniel Vetter wrote: On some chipset we try to avoid possibly invasive output detection methods (like load detect which can cause flickering elsewhere) in the output poll work. Drivers could hence return unknown when a previous full -detect call returned a different state. This change will generate a hotplug event, forcing userspace to do a full scan. This in turn updates the connector-status field so that we will _again_ get a state change when the hotplug work re-runs in 10 seconds. To avoid this ping-pong loop detect this situation and clamp the connector state to the old value. Patch is inspired by a patch from Knut Peterson. Knut's patch completely ignored connector state changes if either the old or new status was unknown, which seemed to be a bit too agressive to me. References: http://lists.freedesktop.org/archives/dri-devel/2012-August/025975.html Cc: Knut Petersen knut_peter...@t-online.de Cc: Alex Deucher alexander.deuc...@amd.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch Also Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk I don't think this has any effect for i915, the circumstances where it might we return connector-status rather than Unknown, so we need some input from nouveau/radeon/architect as to our sanity. It shouldn't affect radeon at all. We shouldn't ever see the unknown status in practice. The only time we return unknown is as a sanity check in our load detection functions (e.g., if the vbios is missing the load detect command table or if we somehow end up in dac load detect on a digital encoder). So, FWIW, Acked-by: Alex Deucher alexander.deuc...@amd.com Alex ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [ANNOUNCE] xf86-video-intel 2.21.9
Release 2.21.9 (2013-06-06) === Consolidating the copy-on-write support, hopefully cleaning up the last of the regressions. * Restore vsync on textured videos. [regression from 2.21.8] https://bugs.freedesktop.org/show_bug.cgi?id=65048 * Fix incorrect ordering of possible_clones with certain outputs, which can lead to attempting to incorrectly clone 2 outputs and failing to light them up. [regression from 2.20.10] * Fix performance regression from not promoting large fills to the GPU [regression from 2.21.7] * Undo the pixmap clone before performing a DRI2CopyRegion [regression from 2.21.7] https://bugs.freedesktop.org/show_bug.cgi?id=65250 Complete list of changes since 2.21.8 - Chris Wilson (23): sna/video: Correct interpretation of 'sync' test: Add an easily visible tearing test for video playback sna: Call mode update after disabling outputs upon VT switch sna: Make the backend identifier more informative Add the known marketing names for the performance Haswell parts sna: Sanity check that CRTC / output combination is valid sna: Log which outputs are being configured during a modeset sna: Report allocations failures during Screen initialisation sna: Cleanup up error reporting after failure to init KMS interface sna: Assert that an existing scanout is the desired size sna: Restore GPU promotion for large fills sna: Compile fix for non-debug builds sna/dri: Reorder assert not to fail on a pageflip deferred to after a modeset sna: Prevent adding damage to an already all-damaged GPU bo sna: Add some more DBG hints to copy-on-write cloning sna/dri: Undo any COW before performing a copy with DRI2CopyRegion sna: Make copying the glyph size more compact sna: Always populate the CPU features string sna: Do not conflate ignoring an output with an allocation failure sna/video: Fix redundant initialisation of video-clip sna: Include the GT details in the backend name for a chipset sna: Only emit an error for terminal mmap failures 2.21.9 release Daniel Vetter (1): sna: fixup up possible_clones kms-X impedance mismatch Rodrigo Vivi (1): Add more correct names for Haswell. git tag: 2.21.9 http://xorg.freedesktop.org/archive/individual/driver/xf86-video-intel-2.21.9.tar.bz2 MD5: fd96651b0d90dc7e977b23fd54f2c3e8 xf86-video-intel-2.21.9.tar.bz2 SHA1: e2f0b75929ba90c0abdbe2233ecfb5b3ccefc323 xf86-video-intel-2.21.9.tar.bz2 SHA256: 1359cbc9e494a284faa52d1db83e7388cb8ab590b660e29e78e6e7f5ee7ff189 xf86-video-intel-2.21.9.tar.bz2 http://xorg.freedesktop.org/archive/individual/driver/xf86-video-intel-2.21.9.tar.gz MD5: f56fc5a53d730c5b7a037cfce2b71196 xf86-video-intel-2.21.9.tar.gz SHA1: a7b6c36c84fb7336384bfffb6cfb3c8a4b5e6946 xf86-video-intel-2.21.9.tar.gz SHA256: 35690e66ed5a99864b66e7abd86ca4bb2b077949c5e0716a8bc03cbd12623a6a xf86-video-intel-2.21.9.tar.gz -- Chris Wilson, Intel Open Source Technology Centre signature.asc Description: Digital signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: hw state readout support for pixel_multiplier
On Thu, Jun 06, 2013 at 02:50:15PM +0300, Imre Deak wrote: On Thu, 2013-06-06 at 12:45 +0200, Daniel Vetter wrote: Incomplete since ilk+ support needs proper pch dpll tracking first. SDVO get_config parts based on a patch from Jesse Barnes, but fixed up to actually work. v2: Make sure that we call encoder-get_config _after_ we get_pipe_config to be consistent in both setup_hw_state and the modeset state checker. Otherwise the clever trick with handling the pixel mutliplier on i915G/GM where the encoder overrides the default value of 1 from the crtc get_pipe_config function doesn't work. Spotted by Imre Deak. v3: Actually cross-check the pixel mutliplier (but not on pch split platforms for now). Now actually also tested on a i915G with a sdvo encoder plugged in. Cc: imre.d...@intel.com Cc: Jesse Barnes jbar...@virtuousgeek.org Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch Looks ok, Reviewed-by: Imre Deak imre.d...@intel.com Queued for -next, thanks for the review. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: pipe config quirk infrastructure plus sdvo mode.flags fix
For various reasons the hw state readout might not be able to faithfully match the hw state: - broken hw (like the case which motivated this patch here where the sdvo encoder does not implemented mandatory functionality correctly). - platforms which are not supported fully with the pipe config infrastructure - if our code doesn't support a given hw configuration natively, e.g. special restrictions on the per-pipe panel fitters when they're used in high-quality scaling modes. In all these cases both fastboot and the hw state cross checker need to be aware of these cases and act accordingly. To be able to do this add a new quirk flag to the pipe config structure. The specific case at hand is an sdvo encoder which doesn't implement the get_timings function, so adjusted_mode flags will be wrong. The strange thing though is that the encoder _does_ work, even though it doesn't implement any of the timings functions (so neither get nor set, neither for input nor output timings). Not that non-compliant sdvo encoder are any surprise at all ... v2: - Don't read random garbage from the dtd if the get_timings call failed (suggested by Chris). - Still check the interlaced flag, that's read out from someplace else. We want maximal paranoia, after all. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_display.c | 22 ++ drivers/gpu/drm/i915/intel_drv.h | 11 +++ drivers/gpu/drm/i915/intel_sdvo.c| 24 +--- 3 files changed, 38 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 372c649..b6b6164 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -7986,6 +7986,9 @@ intel_pipe_config_compare(struct drm_device *dev, return false; \ } +#define PIPE_CONF_QUIRK(quirk) \ + ((current_config-quirks | pipe_config-quirks) (quirk)) + PIPE_CONF_CHECK_I(cpu_transcoder); PIPE_CONF_CHECK_I(has_pch_encoder); @@ -8013,14 +8016,16 @@ intel_pipe_config_compare(struct drm_device *dev, PIPE_CONF_CHECK_FLAGS(adjusted_mode.flags, DRM_MODE_FLAG_INTERLACE); - PIPE_CONF_CHECK_FLAGS(adjusted_mode.flags, - DRM_MODE_FLAG_PHSYNC); - PIPE_CONF_CHECK_FLAGS(adjusted_mode.flags, - DRM_MODE_FLAG_NHSYNC); - PIPE_CONF_CHECK_FLAGS(adjusted_mode.flags, - DRM_MODE_FLAG_PVSYNC); - PIPE_CONF_CHECK_FLAGS(adjusted_mode.flags, - DRM_MODE_FLAG_NVSYNC); + if (!PIPE_CONF_QUIRK(PIPE_CONFIG_QUIRK_MODE_SYNC_FLAGS)) { + PIPE_CONF_CHECK_FLAGS(adjusted_mode.flags, + DRM_MODE_FLAG_PHSYNC); + PIPE_CONF_CHECK_FLAGS(adjusted_mode.flags, + DRM_MODE_FLAG_NHSYNC); + PIPE_CONF_CHECK_FLAGS(adjusted_mode.flags, + DRM_MODE_FLAG_PVSYNC); + PIPE_CONF_CHECK_FLAGS(adjusted_mode.flags, + DRM_MODE_FLAG_NVSYNC); + } PIPE_CONF_CHECK_I(requested_mode.hdisplay); PIPE_CONF_CHECK_I(requested_mode.vdisplay); @@ -8044,6 +8049,7 @@ intel_pipe_config_compare(struct drm_device *dev, #undef PIPE_CONF_CHECK_X #undef PIPE_CONF_CHECK_I #undef PIPE_CONF_CHECK_FLAGS +#undef PIPE_CONF_QUIRK return true; } diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index ea8aa5e..1bf0b40 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -193,6 +193,17 @@ typedef struct dpll { } intel_clock_t; struct intel_crtc_config { + /** +* quirks - bitfield with hw state readout quirks +* +* For various reasons the hw state readout code might not be able to +* completely faithfully read out the current state. These cases are +* tracked with quirk flags so that fastboot and state checker can act +* accordingly. +*/ +#define PIPE_CONFIG_QUIRK_MODE_SYNC_FLAGS (10) /* unreliable sync mode.flags */ + unsigned long quirks; + struct drm_display_mode requested_mode; struct drm_display_mode adjusted_mode; /* This flag must be set by the encoder's compute_config callback if it diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c index d4560ad..67710e1 100644 --- a/drivers/gpu/drm/i915/intel_sdvo.c +++ b/drivers/gpu/drm/i915/intel_sdvo.c @@ -1324,19 +1324,21 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder, ret = intel_sdvo_get_input_timing(intel_sdvo, dtd); if (!ret) { + /* Some sdvo encoders are not spec compliant and don't +* implement the mandatory get_timings function. */
Re: [Intel-gfx] [PATCH] drm/i915: update FBC maximum fb sizes
On Tue, Jun 04, 2013 at 04:53:39PM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com CTG/ILK/SNB/IVB support 4kx2k surfaces. HSW supports 4kx4k, but without proper front buffer invalidation on the last 2k lines, so don't enable FBC on these cases for now. v2: Use gen = 5, not gen 4 (Daniel). Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com Queued for -next, thanks for the patch. -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
[Intel-gfx] [PATCH 4/4] drm/i915: add error_state sysfs entry
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) Suggested-by: Daniel Vetter daniel.vet...@ffwll.ch Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com --- drivers/gpu/drm/i915/i915_irq.c |3 +- drivers/gpu/drm/i915/i915_sysfs.c | 71 + 2 files changed, 72 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 63996aa..22ed519 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1881,8 +1881,7 @@ static void i915_capture_error_state(struct drm_device *dev) } DRM_INFO(capturing error event; look for more information in -/sys/kernel/debug/dri/%d/i915_error_state\n, -dev-primary-index); +/sys/class/drm/card%d/error_state\n, dev-primary-index); kref_init(error-ref); error-eir = I915_READ(EIR); diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c index 6875b56..50416d1 100644 --- a/drivers/gpu/drm/i915/i915_sysfs.c +++ b/drivers/gpu/drm/i915/i915_sysfs.c @@ -409,6 +409,71 @@ static const struct attribute *gen6_attrs[] = { NULL, }; +static ssize_t error_state_read(struct file *filp, struct kobject *kobj, + struct bin_attribute *attr, char *buf, + loff_t off, size_t count) +{ + + struct device *kdev = container_of(kobj, struct device, kobj); + struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev); + struct drm_device *dev = minor-dev; + struct i915_error_state_file_priv error_priv; + struct drm_i915_error_state_buf error_str; + ssize_t ret_count = 0; + int ret; + + memset(error_priv, 0, sizeof(error_priv)); + + ret = i915_error_state_buf_init(error_str, count, off); + if (ret) + return ret; + + error_priv.dev = dev; + i915_error_state_get(dev, error_priv); + + ret = i915_error_state_to_str(error_str, error_priv); + if (ret) + goto out; + + ret_count = count error_str.bytes ? count : error_str.bytes; + + memcpy(buf, error_str.buf, ret_count); +out: + i915_error_state_buf_release(error_str); + i915_error_state_put(error_priv); + + return ret ?: ret_count; +} + +static ssize_t error_state_write(struct file *file, struct kobject *kobj, +struct bin_attribute *attr, char *buf, +loff_t off, size_t count) +{ + struct device *kdev = container_of(kobj, struct device, kobj); + struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev); + struct drm_device *dev = minor-dev; + int ret; + + DRM_DEBUG_DRIVER(Resetting error state\n); + + ret = mutex_lock_interruptible(dev-struct_mutex); + if (ret) + return ret; + + i915_destroy_error_state(dev); + mutex_unlock(dev-struct_mutex); + + return count; +} + +static struct bin_attribute error_state_attr = { + .attr.name = error_state, + .attr.mode = S_IRUSR | S_IWUSR, + .size = 0, + .read = error_state_read, + .write = error_state_write, +}; + void i915_setup_sysfs(struct drm_device *dev) { int ret; @@ -432,10 +497,16 @@ void i915_setup_sysfs(struct drm_device *dev) if (ret) DRM_ERROR(gen6 sysfs setup failed\n); } + + ret = sysfs_create_bin_file(dev-primary-kdev.kobj, + error_state_attr); + if (ret) + DRM_ERROR(error_state sysfs setup failed\n); } void i915_teardown_sysfs(struct drm_device *dev) { + sysfs_remove_bin_file(dev-primary-kdev.kobj, error_state_attr); sysfs_remove_files(dev-primary-kdev.kobj, gen6_attrs); device_remove_bin_file(dev-primary-kdev, dpf_attrs); #ifdef CONFIG_PM -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/4] drm/i915: add error_state sysfs entry
On Thu, Jun 06, 2013 at 04:33:06PM +0300, Mika Kuoppala wrote: 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) Suggested-by: Daniel Vetter daniel.vet...@ffwll.ch Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com --- +static ssize_t error_state_read(struct file *filp, struct kobject *kobj, + struct bin_attribute *attr, char *buf, + loff_t off, size_t count) +{ + + struct device *kdev = container_of(kobj, struct device, kobj); + struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev); + struct drm_device *dev = minor-dev; + struct i915_error_state_file_priv error_priv; + struct drm_i915_error_state_buf error_str; + ssize_t ret_count = 0; + int ret; + + memset(error_priv, 0, sizeof(error_priv)); + + ret = i915_error_state_buf_init(error_str, count, off); + if (ret) + return ret; + + error_priv.dev = dev; + i915_error_state_get(dev, error_priv); + + ret = i915_error_state_to_str(error_str, error_priv); + if (ret) + goto out; + + ret_count = count error_str.bytes ? count : error_str.bytes; + + memcpy(buf, error_str.buf, ret_count); +out: + i915_error_state_buf_release(error_str); + i915_error_state_put(error_priv); Not quite oniony enough. -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: pipe config quirk infrastructure plus sdvo mode.flags fix
On Thu, Jun 06, 2013 at 02:55:52PM +0200, Daniel Vetter wrote: For various reasons the hw state readout might not be able to faithfully match the hw state: - broken hw (like the case which motivated this patch here where the sdvo encoder does not implemented mandatory functionality correctly). - platforms which are not supported fully with the pipe config infrastructure - if our code doesn't support a given hw configuration natively, e.g. special restrictions on the per-pipe panel fitters when they're used in high-quality scaling modes. In all these cases both fastboot and the hw state cross checker need to be aware of these cases and act accordingly. To be able to do this add a new quirk flag to the pipe config structure. The specific case at hand is an sdvo encoder which doesn't implement the get_timings function, so adjusted_mode flags will be wrong. The strange thing though is that the encoder _does_ work, even though it doesn't implement any of the timings functions (so neither get nor set, neither for input nor output timings). Not that non-compliant sdvo encoder are any surprise at all ... v2: - Don't read random garbage from the dtd if the get_timings call failed (suggested by Chris). - Still check the interlaced flag, that's read out from someplace else. We want maximal paranoia, after all. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk -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
[Intel-gfx] [PATCH 0/3]
Hi Greg, Andrew - Patch 1 is for DMI, bugfixes in patches 2-3 for i915 and included for completeness. After a tested-by they should be good for stable. I'll leave it to Daniel to sort out how the last two get in. BR, Jani. Chris Wilson (1): drm/i915: Quirk away phantom LVDS on Intel's D510MO mainboard Jani Nikula (2): dmi: add support for exact DMI matches in addition to substring matching drm/i915: Quirk away phantom LVDS on Intel's D525MW mainboard drivers/firmware/dmi_scan.c | 12 +--- drivers/gpu/drm/i915/intel_lvds.c | 16 include/linux/mod_devicetable.h |6 -- 3 files changed, 29 insertions(+), 5 deletions(-) -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] drm/i915: Quirk away phantom LVDS on Intel's D510MO mainboard
From: Chris Wilson ch...@chris-wilson.co.uk This replaceable mainboard only has a VGA-out, yet it claims to also have a connected LVDS header. Reported-by: annndd...@gmail.com Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=63860 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk [Jani: Use DMI_EXACT_MATCH for board name.] Signed-off-by: Jani Nikula jani.nik...@intel.com --- drivers/gpu/drm/i915/intel_lvds.c |8 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c index 10c3d56..76213e4 100644 --- a/drivers/gpu/drm/i915/intel_lvds.c +++ b/drivers/gpu/drm/i915/intel_lvds.c @@ -690,6 +690,14 @@ static const struct dmi_system_id intel_no_lvds[] = { DMI_MATCH(DMI_PRODUCT_NAME, ESPRIMO Q900), }, }, + { + .callback = intel_no_lvds_dmi_callback, + .ident = Intel D510MO, + .matches = { + DMI_MATCH(DMI_BOARD_VENDOR, Intel), + DMI_EXACT_MATCH(DMI_BOARD_NAME, D510MO), + }, + }, { } /* terminating entry */ }; -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] dmi: add support for exact DMI matches in addition to substring matching
dmi_match() considers a substring match to be a successful match. This is not always sufficient to distinguish between DMI data for different systems. Add support for exact string matching using strcmp() in addition to the substring matching using strstr(). The specific use case in the i915 driver is to allow us to use an exact match for D510MO, without also incorrectly matching D510MOV: { .ident = Intel D510MO, .matches = { DMI_MATCH(DMI_BOARD_VENDOR, Intel), DMI_EXACT_MATCH(DMI_BOARD_NAME, D510MO), }, } Signed-off-by: Jani Nikula jani.nik...@intel.com --- drivers/firmware/dmi_scan.c | 12 +--- include/linux/mod_devicetable.h |6 -- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c index b95159b..eb760a2 100644 --- a/drivers/firmware/dmi_scan.c +++ b/drivers/firmware/dmi_scan.c @@ -551,9 +551,15 @@ static bool dmi_matches(const struct dmi_system_id *dmi) int s = dmi-matches[i].slot; if (s == DMI_NONE) break; - if (dmi_ident[s] -strstr(dmi_ident[s], dmi-matches[i].substr)) - continue; + if (dmi_ident[s]) { + if (!dmi-matches[i].exact_match + strstr(dmi_ident[s], dmi-matches[i].substr)) + continue; + else if (dmi-matches[i].exact_match +!strcmp(dmi_ident[s], dmi-matches[i].substr)) + continue; + } + /* No match */ return false; } diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h index b508016..b3bd7e7 100644 --- a/include/linux/mod_devicetable.h +++ b/include/linux/mod_devicetable.h @@ -456,7 +456,8 @@ enum dmi_field { }; struct dmi_strmatch { - unsigned char slot; + unsigned char slot:7; + unsigned char exact_match:1; char substr[79]; }; @@ -474,7 +475,8 @@ struct dmi_system_id { */ #define dmi_device_id dmi_system_id -#define DMI_MATCH(a, b){ a, b } +#define DMI_MATCH(a, b){ .slot = a, .substr = b } +#define DMI_EXACT_MATCH(a, b) { .slot = a, .substr = b, .exact_match = 1 } #define PLATFORM_NAME_SIZE 20 #define PLATFORM_MODULE_PREFIX platform: -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] drm/i915: Quirk away phantom LVDS on Intel's D525MW mainboard
This replaceable mainboard only has a VGA-out, yet it claims to also have a connected LVDS header. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=65256 Reported-by: Cornel Panceac cpanc...@gmail.com Signed-off-by: Jani Nikula jani.nik...@intel.com --- drivers/gpu/drm/i915/intel_lvds.c |8 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c index 76213e4..607c06e 100644 --- a/drivers/gpu/drm/i915/intel_lvds.c +++ b/drivers/gpu/drm/i915/intel_lvds.c @@ -698,6 +698,14 @@ static const struct dmi_system_id intel_no_lvds[] = { DMI_EXACT_MATCH(DMI_BOARD_NAME, D510MO), }, }, + { + .callback = intel_no_lvds_dmi_callback, + .ident = Intel D525MW, + .matches = { + DMI_MATCH(DMI_BOARD_VENDOR, Intel), + DMI_EXACT_MATCH(DMI_BOARD_NAME, D525MW), + }, + }, { } /* terminating entry */ }; -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/3]
With Greg's address fixed. Please drop the old one from any replies. Sorry for the noise. On Thu, 06 Jun 2013, Jani Nikula jani.nik...@intel.com wrote: Hi Greg, Andrew - Patch 1 is for DMI, bugfixes in patches 2-3 for i915 and included for completeness. After a tested-by they should be good for stable. I'll leave it to Daniel to sort out how the last two get in. BR, Jani. Chris Wilson (1): drm/i915: Quirk away phantom LVDS on Intel's D510MO mainboard Jani Nikula (2): dmi: add support for exact DMI matches in addition to substring matching drm/i915: Quirk away phantom LVDS on Intel's D525MW mainboard drivers/firmware/dmi_scan.c | 12 +--- drivers/gpu/drm/i915/intel_lvds.c | 16 include/linux/mod_devicetable.h |6 -- 3 files changed, 29 insertions(+), 5 deletions(-) -- 1.7.9.5 -- Jani Nikula, 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] drm/i915: VGA also requires the power well
2013/6/6 Ville Syrjälä ville.syrj...@linux.intel.com: On Wed, Jun 05, 2013 at 06:05:51PM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com So add a power domain and check for it before we try to read VGA_CONTROL. This fixes unclaimed register messages that happen on suspend/resume. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_display.c | 3 +++ drivers/gpu/drm/i915/intel_pm.c | 1 + 3 files changed, 5 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 46b1f70..d51ce13 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -89,6 +89,7 @@ enum port { #define port_name(p) ((p) + 'A') enum intel_display_power_domain { + POWER_DOMAIN_VGA, POWER_DOMAIN_PIPE_A, POWER_DOMAIN_PIPE_B, POWER_DOMAIN_PIPE_C, diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 4c8fcec..3719d99 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9950,6 +9950,9 @@ void i915_redisable_vga(struct drm_device *dev) struct drm_i915_private *dev_priv = dev-dev_private; u32 vga_reg = i915_vgacntrl_reg(dev); + if (!intel_display_power_enabled(dev, POWER_DOMAIN_VGA)) + return; + So it looks like you're essentially making intel_redisable_vga() a nop for HSW. It's not a nop for HSW, it's only a nop if the power well is disabled, which means VGA is disabled, so it's a nop if VGA is disabled. But if you look at the current function it's also a nop if VGA is disabled. So we're keeping the same behavior, but checking the power well before checking vga_reg. VGA mode requires the power well to be enabled, we can be sure that if the power well is disabled, then VGA is disabled, so you don't need to do the check for VGA_DISP_DISABLE. Shouldn't we instead enable the power well during resume? So far we don't need it. I hope it stays this way. if (I915_READ(vga_reg) != VGA_DISP_DISABLE) { DRM_DEBUG_KMS(Something enabled VGA plane, disabling it\n); i915_disable_vga(dev); diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 50fe3d7..47ef4a6 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5000,6 +5000,7 @@ bool intel_display_power_enabled(struct drm_device *dev, case POWER_DOMAIN_PIPE_A: case POWER_DOMAIN_TRANSCODER_EDP: return true; + case POWER_DOMAIN_VGA: case POWER_DOMAIN_PIPE_B: case POWER_DOMAIN_PIPE_C: case POWER_DOMAIN_PIPE_A_PANEL_FITTER: -- 1.8.1.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/3]
On Thu, Jun 06, 2013 at 04:53:01PM +0300, Jani Nikula wrote: Hi Greg, Andrew - Patch 1 is for DMI, bugfixes in patches 2-3 for i915 and included for completeness. After a tested-by they should be good for stable. I'll leave it to Daniel to sort out how the last two get in. I'd prefer all to go through the same tree (to avoid tracking them), and conflicts around lvds quirks will be trivial at most. So no problem for me if this doesn't go in through drm-next. So Acked-by: Daniel Vetter daniel.vet...@ffwll.ch on the i915 patches for merging through whatever tree the drm stuff goes through. -Daniel BR, Jani. Chris Wilson (1): drm/i915: Quirk away phantom LVDS on Intel's D510MO mainboard Jani Nikula (2): dmi: add support for exact DMI matches in addition to substring matching drm/i915: Quirk away phantom LVDS on Intel's D525MW mainboard drivers/firmware/dmi_scan.c | 12 +--- drivers/gpu/drm/i915/intel_lvds.c | 16 include/linux/mod_devicetable.h |6 -- 3 files changed, 29 insertions(+), 5 deletions(-) -- 1.7.9.5 -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/4] drm/i915: add error_state sysfs entry
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: Daniel Vetter daniel.vet...@ffwll.ch Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com --- drivers/gpu/drm/i915/i915_irq.c |3 +- drivers/gpu/drm/i915/i915_sysfs.c | 71 + 2 files changed, 72 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 63996aa..22ed519 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1881,8 +1881,7 @@ static void i915_capture_error_state(struct drm_device *dev) } DRM_INFO(capturing error event; look for more information in -/sys/kernel/debug/dri/%d/i915_error_state\n, -dev-primary-index); +/sys/class/drm/card%d/error_state\n, dev-primary-index); kref_init(error-ref); error-eir = I915_READ(EIR); diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c index 6875b56..ffaf5da 100644 --- a/drivers/gpu/drm/i915/i915_sysfs.c +++ b/drivers/gpu/drm/i915/i915_sysfs.c @@ -409,6 +409,71 @@ static const struct attribute *gen6_attrs[] = { NULL, }; +static ssize_t error_state_read(struct file *filp, struct kobject *kobj, + struct bin_attribute *attr, char *buf, + loff_t off, size_t count) +{ + + struct device *kdev = container_of(kobj, struct device, kobj); + struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev); + struct drm_device *dev = minor-dev; + struct i915_error_state_file_priv error_priv; + struct drm_i915_error_state_buf error_str; + ssize_t ret_count = 0; + int ret; + + memset(error_priv, 0, sizeof(error_priv)); + + ret = i915_error_state_buf_init(error_str, count, off); + if (ret) + return ret; + + error_priv.dev = dev; + i915_error_state_get(dev, error_priv); + + ret = i915_error_state_to_str(error_str, error_priv); + if (ret) + goto out; + + ret_count = count error_str.bytes ? count : error_str.bytes; + + memcpy(buf, error_str.buf, ret_count); +out: + i915_error_state_put(error_priv); + i915_error_state_buf_release(error_str); + + return ret ?: ret_count; +} + +static ssize_t error_state_write(struct file *file, struct kobject *kobj, +struct bin_attribute *attr, char *buf, +loff_t off, size_t count) +{ + struct device *kdev = container_of(kobj, struct device, kobj); + struct drm_minor *minor = container_of(kdev, struct drm_minor, kdev); + struct drm_device *dev = minor-dev; + int ret; + + DRM_DEBUG_DRIVER(Resetting error state\n); + + ret = mutex_lock_interruptible(dev-struct_mutex); + if (ret) + return ret; + + i915_destroy_error_state(dev); + mutex_unlock(dev-struct_mutex); + + return count; +} + +static struct bin_attribute error_state_attr = { + .attr.name = error_state, + .attr.mode = S_IRUSR | S_IWUSR, + .size = 0, + .read = error_state_read, + .write = error_state_write, +}; + void i915_setup_sysfs(struct drm_device *dev) { int ret; @@ -432,10 +497,16 @@ void i915_setup_sysfs(struct drm_device *dev) if (ret) DRM_ERROR(gen6 sysfs setup failed\n); } + + ret = sysfs_create_bin_file(dev-primary-kdev.kobj, + error_state_attr); + if (ret) + DRM_ERROR(error_state sysfs setup failed\n); } void i915_teardown_sysfs(struct drm_device *dev) { + sysfs_remove_bin_file(dev-primary-kdev.kobj, error_state_attr); sysfs_remove_files(dev-primary-kdev.kobj, gen6_attrs); device_remove_bin_file(dev-primary-kdev, dpf_attrs); #ifdef CONFIG_PM -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Only slightly increment hangcheck score if we succesfully kick a ring
Chris Wilson ch...@chris-wilson.co.uk writes: 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. 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 Hopefully Ben will also take a look as he saw exactly what was coming. Reviewed-by: Mika Kuoppala mika.kuopp...@intel.com ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: VGA also requires the power well
On Thu, Jun 06, 2013 at 11:35:15AM -0300, Paulo Zanoni wrote: 2013/6/6 Ville Syrjälä ville.syrj...@linux.intel.com: On Wed, Jun 05, 2013 at 06:05:51PM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com So add a power domain and check for it before we try to read VGA_CONTROL. This fixes unclaimed register messages that happen on suspend/resume. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_display.c | 3 +++ drivers/gpu/drm/i915/intel_pm.c | 1 + 3 files changed, 5 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 46b1f70..d51ce13 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -89,6 +89,7 @@ enum port { #define port_name(p) ((p) + 'A') enum intel_display_power_domain { + POWER_DOMAIN_VGA, POWER_DOMAIN_PIPE_A, POWER_DOMAIN_PIPE_B, POWER_DOMAIN_PIPE_C, diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 4c8fcec..3719d99 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9950,6 +9950,9 @@ void i915_redisable_vga(struct drm_device *dev) struct drm_i915_private *dev_priv = dev-dev_private; u32 vga_reg = i915_vgacntrl_reg(dev); + if (!intel_display_power_enabled(dev, POWER_DOMAIN_VGA)) + return; + So it looks like you're essentially making intel_redisable_vga() a nop for HSW. It's not a nop for HSW, it's only a nop if the power well is disabled, which means VGA is disabled, so it's a nop if VGA is disabled. But if you look at the current function it's also a nop if VGA is disabled. So we're keeping the same behavior, but checking the power well before checking vga_reg. VGA mode requires the power well to be enabled, we can be sure that if the power well is disabled, then VGA is disabled, so you don't need to do the check for VGA_DISP_DISABLE. Rigth, but intel_display_power_enabled() only checks the driver power well register. If BIOS can leave VGA enabled, then I guess it could've left the power well on too. So I'm thinking we should check for that rather than the what the driver requested. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: WA: FBC Render Nuke.
WaFbcNukeOn3DBlt for IVB, HSW. According BSPec: Workaround: Do not enable Render Command Streamer tracking for FBC. Instead insert a LRI to address 0x50380 with data 0x0004 after the PIPE_CONTROL that follows each render submission. v2: Chris noticed that flush_domains check was missing here and also suggested to do LRI only when fbc is enabled. To avoid do a I915_READ on every flush lets use the module parameter check. v3: Adding Wa name as Damien suggested. v4: Ville noticed VLV doesn't support fbc at all and comment came wrong from spec. v5: Ville noticed than on blt a Cache Clean LRI should be used instead the Nuke one. v6: Check for flush domain on blt (by Ville). Check for scanout dirty (by Chris). Cc: Ville Syrjälä ville.syrj...@linux.intel.com Cc: Chris Wilson ch...@chris-wilson.co.uk Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com --- drivers/gpu/drm/i915/i915_reg.h | 4 drivers/gpu/drm/i915/intel_pm.c | 2 +- drivers/gpu/drm/i915/intel_ringbuffer.c | 36 + 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 47a9de0..757bbb7 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -1026,6 +1026,10 @@ #define IPS_CTL0x43408 #define IPS_ENABLE (1 31) +#define MSG_FBC_REND_STATE 0x50380 +#define FBC_REND_NUKE(12) +#define FBC_REND_CACHE_CLEAN (11) + #define _HSW_PIPE_SLICE_CHICKEN_1_A0x420B0 #define _HSW_PIPE_SLICE_CHICKEN_1_B0x420B4 #define HSW_BYPASS_FBC_QUEUE (122) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 50fe3d7..324d1ce 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -274,7 +274,7 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, unsigned long interval) struct drm_i915_gem_object *obj = intel_fb-obj; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - I915_WRITE(IVB_FBC_RT_BASE, obj-gtt_offset | ILK_FBC_RT_VALID); + I915_WRITE(IVB_FBC_RT_BASE, obj-gtt_offset); I915_WRITE(ILK_DPFC_CONTROL, DPFC_CTL_EN | DPFC_CTL_LIMIT_1X | IVB_DPFC_CTL_FENCE_EN | diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 0e72da6..cec5246 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -280,6 +280,34 @@ gen7_render_ring_cs_stall_wa(struct intel_ring_buffer *ring) return 0; } +static int gen7_ring_fbc_flush(struct intel_ring_buffer *ring, u32 value) +{ + struct drm_device *dev = ring-dev; + struct drm_i915_gem_object *obj = ring-obj; + int ret; + + if (i915_enable_fbc == 0) + return 0; + + if (i915_enable_fbc 0 !IS_HASWELL(dev)) + return 0; + + if (obj-base.write_domain != I915_GEM_DOMAIN_GTT) + return 0; + + ret = intel_ring_begin(ring, 4); + if (ret) + return ret; + intel_ring_emit(ring, MI_NOOP); + /* WaFbcNukeOn3DBlt:ivb/hsw */ + intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1)); + intel_ring_emit(ring, MSG_FBC_REND_STATE); + intel_ring_emit(ring, value); + intel_ring_advance(ring); + + return 0; +} + static int gen7_render_ring_flush(struct intel_ring_buffer *ring, u32 invalidate_domains, u32 flush_domains) @@ -336,6 +364,9 @@ gen7_render_ring_flush(struct intel_ring_buffer *ring, intel_ring_emit(ring, 0); intel_ring_advance(ring); + if (flush_domains) + return gen7_ring_fbc_flush(ring, FBC_REND_NUKE); + return 0; } @@ -1685,6 +1716,7 @@ gen6_ring_dispatch_execbuffer(struct intel_ring_buffer *ring, static int gen6_ring_flush(struct intel_ring_buffer *ring, u32 invalidate, u32 flush) { + struct drm_device *dev = ring-dev; uint32_t cmd; int ret; @@ -1707,6 +1739,10 @@ static int gen6_ring_flush(struct intel_ring_buffer *ring, intel_ring_emit(ring, 0); intel_ring_emit(ring, MI_NOOP); intel_ring_advance(ring); + + if (IS_GEN7(dev) flush) + return gen7_ring_fbc_flush(ring, FBC_REND_CACHE_CLEAN); + return 0; } -- 1.7.11.7 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: WA: FBC Render Nuke.
On Thu, Jun 06, 2013 at 11:49:56AM -0300, Rodrigo Vivi wrote: WaFbcNukeOn3DBlt for IVB, HSW. According BSPec: Workaround: Do not enable Render Command Streamer tracking for FBC. Instead insert a LRI to address 0x50380 with data 0x0004 after the PIPE_CONTROL that follows each render submission. v2: Chris noticed that flush_domains check was missing here and also suggested to do LRI only when fbc is enabled. To avoid do a I915_READ on every flush lets use the module parameter check. v3: Adding Wa name as Damien suggested. v4: Ville noticed VLV doesn't support fbc at all and comment came wrong from spec. v5: Ville noticed than on blt a Cache Clean LRI should be used instead the Nuke one. v6: Check for flush domain on blt (by Ville). Check for scanout dirty (by Chris). Note quite what I had in mind, see https://patchwork.kernel.org/patch/2606131/ -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: WA: FBC Render Nuke.
please just ignore this version... going to try fbc_dirty and other changes here... On Thu, Jun 6, 2013 at 12:00 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: On Thu, Jun 06, 2013 at 11:49:56AM -0300, Rodrigo Vivi wrote: WaFbcNukeOn3DBlt for IVB, HSW. According BSPec: Workaround: Do not enable Render Command Streamer tracking for FBC. Instead insert a LRI to address 0x50380 with data 0x0004 after the PIPE_CONTROL that follows each render submission. v2: Chris noticed that flush_domains check was missing here and also suggested to do LRI only when fbc is enabled. To avoid do a I915_READ on every flush lets use the module parameter check. v3: Adding Wa name as Damien suggested. v4: Ville noticed VLV doesn't support fbc at all and comment came wrong from spec. v5: Ville noticed than on blt a Cache Clean LRI should be used instead the Nuke one. v6: Check for flush domain on blt (by Ville). Check for scanout dirty (by Chris). Note quite what I had in mind, see https://patchwork.kernel.org/patch/2606131/ -Chris -- Chris Wilson, Intel Open Source Technology Centre -- Rodrigo Vivi Blog: http://blog.vivi.eng.br ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/4 V7] Power-well API implementation for Haswell
On Thu, May 30, 2013 at 10:07:07PM +0800, Wang Xingchao wrote: Hi all, This is V7 and here're some changes notes: change from V6--V7: - rename variable - use HAS_POWER_WELL instead of IS_HASWELL - put structure inside drm_i915_private - use WARN_ON for global pointer check change from V5--V6: - Remove duplication code in new introduced probe work - move duplication code in azx_probe_continue - remove unused #ifdef - replace request_module with symbol_request - replace spin_lock_irq with spin_lock_irqsave in gfx side - other typo fixes (review by Takashi) change from V4--V5: - fix reference count bug - new patch on general runtime pm support for audio pci device - new patch to avoid request_module() deadlock change between V3--V4: - add new structure i915_power_well - initialize drm_device pointer at module init time - change function name change between V2--V3: - make SND_HDA_I915 selectable - use snd_printdd to output message - add return error code check - use symbol_request to replace symbol_get - release power_well at azx_free - some typo fixes changes between V1--V2: - use reference count to track power-well usage - remove external module, compiled into snd-hda-intel instead - manage symbols and module loading properly - remove IS_HSW macro, use flag instead - remove audio callback for gfx driver to avoid dependency - split whole patch into two pieces for easy review - more typo fixes Takashi Iwai (1): ALSA: hda - Move azx_first_init() into azx_probe_continue() Wang Xingchao (3): ALSA: hda - Fix runtime PM check ALSA: hda - Add power-welll support for haswell HDA i915/drm: Add private api for power well usage After discussion with Dave and Takashi I've merged the entire series to drm-intel-next. I'll show up in the next linux-next and I'll forward it to Dave for mergin into drm-next in roughly 2 weeks. Thanks, Daniel drivers/gpu/drm/i915/i915_dma.c |6 +++ drivers/gpu/drm/i915/i915_drv.h | 12 ++ drivers/gpu/drm/i915/intel_drv.h |4 ++ drivers/gpu/drm/i915/intel_pm.c | 81 --- include/drm/i915_powerwell.h | 36 sound/pci/hda/Kconfig| 10 + sound/pci/hda/Makefile |2 + sound/pci/hda/hda_i915.c | 75 sound/pci/hda/hda_i915.h | 35 +++ sound/pci/hda/hda_intel.c| 87 ++ 10 files changed, 324 insertions(+), 24 deletions(-) create mode 100644 include/drm/i915_powerwell.h create mode 100644 sound/pci/hda/hda_i915.c create mode 100644 sound/pci/hda/hda_i915.h -- 1.7.9.5 -- 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/6] drm/i915: Always enable the cursor right after the primary plane
On Tue, May 21, 2013 at 05:17:27PM +0200, Egbert Eich wrote: On Wed, May 08, 2013 at 12:50:24PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com Follow the same sequence when enabling the cursor plane during modeset. No point in doing this stuff in different order on different generations. This should also avoid a needless wait for vblank for the g4x cursor workaround when the cursor gets enabled anyway. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_display.c | 10 -- 1 file changed, 4 insertions(+), 6 deletions(-) @@ -3727,6 +3725,7 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc) intel_enable_pipe(dev_priv, pipe, false); intel_enable_plane(dev_priv, plane, pipe); + intel_crtc_update_cursor(crtc, true); if (IS_G4X(dev)) g4x_fixup_plane(dev_priv, pipe); As discussed on IRC this may interfere with commit 61bc95c1fbbb6a08b55bbe161fdf1ea5493fc595 Author: Egbert Eich e...@suse.com Date: Mon Mar 4 09:24:38 2013 -0500 DRM/i915: On G45 enable cursor plane briefly after enabling the display plane. described in https://bugs.freedesktop.org/show_bug.cgi?id=61457 Today I had the chance to test this. First I tried if I can still reproduce the blank with this patch added when I disable my voodoo g4x_fixup_plane(): It turned out it still happens however very rarely (like 1 out of 20 tries). When I reenabled my voodoo the issue still occurred. I had to switch two lines around, ie: intel_enable_plane(dev_priv, plane, pipe); if (IS_G4X(dev)) g4x_fixup_plane(dev_priv, pipe); + intel_crtc_update_cursor(crtc, true); to avoid the blank screen issue - which is it didn't happen in ~75 tries. With this change: Acked-by: Egbert Eich e...@suse.com Cheers, Egbert. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/6] drm/i915: Always enable the cursor right after the primary plane
On Thu, Jun 06, 2013 at 06:30:52PM +0200, Egbert Eich wrote: On Tue, May 21, 2013 at 05:17:27PM +0200, Egbert Eich wrote: On Wed, May 08, 2013 at 12:50:24PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com Follow the same sequence when enabling the cursor plane during modeset. No point in doing this stuff in different order on different generations. This should also avoid a needless wait for vblank for the g4x cursor workaround when the cursor gets enabled anyway. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_display.c | 10 -- 1 file changed, 4 insertions(+), 6 deletions(-) @@ -3727,6 +3725,7 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc) intel_enable_pipe(dev_priv, pipe, false); intel_enable_plane(dev_priv, plane, pipe); + intel_crtc_update_cursor(crtc, true); if (IS_G4X(dev)) g4x_fixup_plane(dev_priv, pipe); As discussed on IRC this may interfere with commit 61bc95c1fbbb6a08b55bbe161fdf1ea5493fc595 Author: Egbert Eich e...@suse.com Date: Mon Mar 4 09:24:38 2013 -0500 DRM/i915: On G45 enable cursor plane briefly after enabling the display plane. described in https://bugs.freedesktop.org/show_bug.cgi?id=61457 Today I had the chance to test this. First I tried if I can still reproduce the blank with this patch added when I disable my voodoo g4x_fixup_plane(): It turned out it still happens however very rarely (like 1 out of 20 tries). When I reenabled my voodoo the issue still occurred. I had to switch two lines around, ie: intel_enable_plane(dev_priv, plane, pipe); if (IS_G4X(dev)) g4x_fixup_plane(dev_priv, pipe); + intel_crtc_update_cursor(crtc, true); to avoid the blank screen issue - which is it didn't happen in ~75 tries. With this change: Acked-by: Egbert Eich e...@suse.com Hmm. That means the cursor itself isn't perhaps really relevant, and it's maybe more about the self refresh. BTW did you ever try w/ just the self refresh disable but w/o the cursor trick? We seem to enable self refresh already before we enable any planes (in intel_update_watermarks()). So maybe something like this would work: i9xx_crtc_enable() ... intel_update_watermarks() ... SR off intel_enable_pipe() intel_enable_plane() enable other planes [ maybe intel_wait_for_vblank(), but we seem to have too many of those already... ] SR back on BTW the spec lists an SR workaround, which we don't follow apparently. It's only relevant for switching between 1 and 2 pipes though. But maybe there's a more general issue enabling even 1 pipe w/ SR enabled... I suspect I need to dig more into the g4x watermark/SR stuff at some point, so this is good practice for me :) I just wish I was able to reproduce the bug. Maybe I need to find a ctg instead of the elk I have... -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915: Track when we dirty the scanout with render commands
From: Chris Wilson ch...@chris-wilson.co.uk This is required for tracking render damage for use with FBC and will be used in subsequent patches. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Reviewed-by: Rodrigo Vivi rodrigo.v...@gmail.com --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +- drivers/gpu/drm/i915/intel_display.c | 13 + drivers/gpu/drm/i915/intel_drv.h | 3 ++- drivers/gpu/drm/i915/intel_ringbuffer.h| 1 + 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index a8bb62c..c98333d 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -786,7 +786,7 @@ i915_gem_execbuffer_move_to_active(struct list_head *objects, obj-dirty = 1; obj-last_write_seqno = intel_ring_get_seqno(ring); if (obj-pin_count) /* check for potential scanout */ - intel_mark_fb_busy(obj); + intel_mark_fb_busy(obj, ring); } trace_i915_gem_object_change_domain(obj, old_read, old_write); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 40d047e..657f9a1 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -7042,7 +7042,8 @@ void intel_mark_idle(struct drm_device *dev) } } -void intel_mark_fb_busy(struct drm_i915_gem_object *obj) +void intel_mark_fb_busy(struct drm_i915_gem_object *obj, + struct intel_ring_buffer *ring) { struct drm_device *dev = obj-base.dev; struct drm_crtc *crtc; @@ -7054,8 +7055,12 @@ void intel_mark_fb_busy(struct drm_i915_gem_object *obj) if (!crtc-fb) continue; - if (to_intel_framebuffer(crtc-fb)-obj == obj) - intel_increase_pllclock(crtc); + if (to_intel_framebuffer(crtc-fb)-obj != obj) + continue; + + intel_increase_pllclock(crtc); + if (ring intel_fbc_enabled(dev)) + ring-fbc_dirty = true; } } @@ -7505,7 +7510,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, goto cleanup_pending; intel_disable_fbc(dev); - intel_mark_fb_busy(obj); + intel_mark_fb_busy(obj, NULL); mutex_unlock(dev-struct_mutex); trace_i915_flip_request(intel_crtc-plane, obj); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index eae3dbc..2f87652 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -563,7 +563,8 @@ extern bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, extern void intel_dvo_init(struct drm_device *dev); extern void intel_tv_init(struct drm_device *dev); extern void intel_mark_busy(struct drm_device *dev); -extern void intel_mark_fb_busy(struct drm_i915_gem_object *obj); +extern void intel_mark_fb_busy(struct drm_i915_gem_object *obj, + struct intel_ring_buffer *ring); extern void intel_mark_idle(struct drm_device *dev); extern bool intel_lvds_init(struct drm_device *dev); extern bool intel_is_dual_link_lvds(struct drm_device *dev); diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 4c7e103..efc403d 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -140,6 +140,7 @@ struct intel_ring_buffer { */ u32 outstanding_lazy_request; bool gpu_caches_dirty; + bool fbc_dirty; wait_queue_head_t irq_queue; -- 1.7.11.7 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915: Track when we dirty the scanout with render commands
From: Chris Wilson ch...@chris-wilson.co.uk This is required for tracking render damage for use with FBC and will be used in subsequent patches. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Reviewed-by: Rodrigo Vivi rodrigo.v...@gmail.com --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +- drivers/gpu/drm/i915/intel_display.c | 13 + drivers/gpu/drm/i915/intel_drv.h | 3 ++- drivers/gpu/drm/i915/intel_ringbuffer.h| 1 + 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index a8bb62c..c98333d 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -786,7 +786,7 @@ i915_gem_execbuffer_move_to_active(struct list_head *objects, obj-dirty = 1; obj-last_write_seqno = intel_ring_get_seqno(ring); if (obj-pin_count) /* check for potential scanout */ - intel_mark_fb_busy(obj); + intel_mark_fb_busy(obj, ring); } trace_i915_gem_object_change_domain(obj, old_read, old_write); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 40d047e..657f9a1 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -7042,7 +7042,8 @@ void intel_mark_idle(struct drm_device *dev) } } -void intel_mark_fb_busy(struct drm_i915_gem_object *obj) +void intel_mark_fb_busy(struct drm_i915_gem_object *obj, + struct intel_ring_buffer *ring) { struct drm_device *dev = obj-base.dev; struct drm_crtc *crtc; @@ -7054,8 +7055,12 @@ void intel_mark_fb_busy(struct drm_i915_gem_object *obj) if (!crtc-fb) continue; - if (to_intel_framebuffer(crtc-fb)-obj == obj) - intel_increase_pllclock(crtc); + if (to_intel_framebuffer(crtc-fb)-obj != obj) + continue; + + intel_increase_pllclock(crtc); + if (ring intel_fbc_enabled(dev)) + ring-fbc_dirty = true; } } @@ -7505,7 +7510,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, goto cleanup_pending; intel_disable_fbc(dev); - intel_mark_fb_busy(obj); + intel_mark_fb_busy(obj, NULL); mutex_unlock(dev-struct_mutex); trace_i915_flip_request(intel_crtc-plane, obj); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index eae3dbc..2f87652 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -563,7 +563,8 @@ extern bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, extern void intel_dvo_init(struct drm_device *dev); extern void intel_tv_init(struct drm_device *dev); extern void intel_mark_busy(struct drm_device *dev); -extern void intel_mark_fb_busy(struct drm_i915_gem_object *obj); +extern void intel_mark_fb_busy(struct drm_i915_gem_object *obj, + struct intel_ring_buffer *ring); extern void intel_mark_idle(struct drm_device *dev); extern bool intel_lvds_init(struct drm_device *dev); extern bool intel_is_dual_link_lvds(struct drm_device *dev); diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 4c7e103..efc403d 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -140,6 +140,7 @@ struct intel_ring_buffer { */ u32 outstanding_lazy_request; bool gpu_caches_dirty; + bool fbc_dirty; wait_queue_head_t irq_queue; -- 1.7.11.7 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915: WA: FBC Render Nuke.
WaFbcNukeOn3DBlt for IVB, HSW. According BSPec: Workaround: Do not enable Render Command Streamer tracking for FBC. Instead insert a LRI to address 0x50380 with data 0x0004 after the PIPE_CONTROL that follows each render submission. v2: Chris noticed that flush_domains check was missing here and also suggested to do LRI only when fbc is enabled. To avoid do a I915_READ on every flush lets use the module parameter check. v3: Adding Wa name as Damien suggested. v4: Ville noticed VLV doesn't support fbc at all and comment came wrong from spec. v5: Ville noticed than on blt a Cache Clean LRI should be used instead the Nuke one. v6: Check for flush domain on blt (by Ville). Check for scanout dirty (by Chris). v7: Apply proper fbc_dirty implemented by Chris. Cc: Ville Syrjälä ville.syrj...@linux.intel.com Cc: Chris Wilson ch...@chris-wilson.co.uk Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com --- drivers/gpu/drm/i915/i915_reg.h | 4 drivers/gpu/drm/i915/intel_pm.c | 2 +- drivers/gpu/drm/i915/intel_ringbuffer.c | 31 +++ 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 47a9de0..757bbb7 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -1026,6 +1026,10 @@ #define IPS_CTL0x43408 #define IPS_ENABLE (1 31) +#define MSG_FBC_REND_STATE 0x50380 +#define FBC_REND_NUKE(12) +#define FBC_REND_CACHE_CLEAN (11) + #define _HSW_PIPE_SLICE_CHICKEN_1_A0x420B0 #define _HSW_PIPE_SLICE_CHICKEN_1_B0x420B4 #define HSW_BYPASS_FBC_QUEUE (122) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 50fe3d7..324d1ce 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -274,7 +274,7 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, unsigned long interval) struct drm_i915_gem_object *obj = intel_fb-obj; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - I915_WRITE(IVB_FBC_RT_BASE, obj-gtt_offset | ILK_FBC_RT_VALID); + I915_WRITE(IVB_FBC_RT_BASE, obj-gtt_offset); I915_WRITE(ILK_DPFC_CONTROL, DPFC_CTL_EN | DPFC_CTL_LIMIT_1X | IVB_DPFC_CTL_FENCE_EN | diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 0e72da6..85f06a8 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -280,6 +280,29 @@ gen7_render_ring_cs_stall_wa(struct intel_ring_buffer *ring) return 0; } +static int gen7_ring_fbc_flush(struct intel_ring_buffer *ring, u32 value) +{ + struct drm_device *dev = ring-dev; + struct drm_i915_gem_object *obj = ring-obj; + int ret; + + if (!ring-fbc_dirty) + return 0; + + ret = intel_ring_begin(ring, 4); + if (ret) + return ret; + intel_ring_emit(ring, MI_NOOP); + /* WaFbcNukeOn3DBlt:ivb/hsw */ + intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1)); + intel_ring_emit(ring, MSG_FBC_REND_STATE); + intel_ring_emit(ring, value); + intel_ring_advance(ring); + + ring-fbc_dirty = false; + return 0; +} + static int gen7_render_ring_flush(struct intel_ring_buffer *ring, u32 invalidate_domains, u32 flush_domains) @@ -336,6 +359,9 @@ gen7_render_ring_flush(struct intel_ring_buffer *ring, intel_ring_emit(ring, 0); intel_ring_advance(ring); + if (flush_domains) + return gen7_ring_fbc_flush(ring, FBC_REND_NUKE); + return 0; } @@ -1685,6 +1711,7 @@ gen6_ring_dispatch_execbuffer(struct intel_ring_buffer *ring, static int gen6_ring_flush(struct intel_ring_buffer *ring, u32 invalidate, u32 flush) { + struct drm_device *dev = ring-dev; uint32_t cmd; int ret; @@ -1707,6 +1734,10 @@ static int gen6_ring_flush(struct intel_ring_buffer *ring, intel_ring_emit(ring, 0); intel_ring_emit(ring, MI_NOOP); intel_ring_advance(ring); + + if (IS_GEN7(dev) flush) + return gen7_ring_fbc_flush(ring, FBC_REND_CACHE_CLEAN); + return 0; } -- 1.7.11.7 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: WA: FBC Render Nuke.
WaFbcNukeOn3DBlt for IVB, HSW. According BSPec: Workaround: Do not enable Render Command Streamer tracking for FBC. Instead insert a LRI to address 0x50380 with data 0x0004 after the PIPE_CONTROL that follows each render submission. v2: Chris noticed that flush_domains check was missing here and also suggested to do LRI only when fbc is enabled. To avoid do a I915_READ on every flush lets use the module parameter check. v3: Adding Wa name as Damien suggested. v4: Ville noticed VLV doesn't support fbc at all and comment came wrong from spec. v5: Ville noticed than on blt a Cache Clean LRI should be used instead the Nuke one. v6: Check for flush domain on blt (by Ville). Check for scanout dirty (by Chris). v7: Apply proper fbc_dirty implemented by Chris. v8: remove unused variables. Cc: Ville Syrjälä ville.syrj...@linux.intel.com Cc: Chris Wilson ch...@chris-wilson.co.uk Signed-off-by: Rodrigo Vivi rodrigo.v...@gmail.com --- drivers/gpu/drm/i915/i915_reg.h | 4 drivers/gpu/drm/i915/intel_pm.c | 2 +- drivers/gpu/drm/i915/intel_ringbuffer.c | 29 + 3 files changed, 34 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 47a9de0..757bbb7 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -1026,6 +1026,10 @@ #define IPS_CTL0x43408 #define IPS_ENABLE (1 31) +#define MSG_FBC_REND_STATE 0x50380 +#define FBC_REND_NUKE(12) +#define FBC_REND_CACHE_CLEAN (11) + #define _HSW_PIPE_SLICE_CHICKEN_1_A0x420B0 #define _HSW_PIPE_SLICE_CHICKEN_1_B0x420B4 #define HSW_BYPASS_FBC_QUEUE (122) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 50fe3d7..324d1ce 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -274,7 +274,7 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, unsigned long interval) struct drm_i915_gem_object *obj = intel_fb-obj; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - I915_WRITE(IVB_FBC_RT_BASE, obj-gtt_offset | ILK_FBC_RT_VALID); + I915_WRITE(IVB_FBC_RT_BASE, obj-gtt_offset); I915_WRITE(ILK_DPFC_CONTROL, DPFC_CTL_EN | DPFC_CTL_LIMIT_1X | IVB_DPFC_CTL_FENCE_EN | diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 0e72da6..1ef081c 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -280,6 +280,27 @@ gen7_render_ring_cs_stall_wa(struct intel_ring_buffer *ring) return 0; } +static int gen7_ring_fbc_flush(struct intel_ring_buffer *ring, u32 value) +{ + int ret; + + if (!ring-fbc_dirty) + return 0; + + ret = intel_ring_begin(ring, 4); + if (ret) + return ret; + intel_ring_emit(ring, MI_NOOP); + /* WaFbcNukeOn3DBlt:ivb/hsw */ + intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1)); + intel_ring_emit(ring, MSG_FBC_REND_STATE); + intel_ring_emit(ring, value); + intel_ring_advance(ring); + + ring-fbc_dirty = false; + return 0; +} + static int gen7_render_ring_flush(struct intel_ring_buffer *ring, u32 invalidate_domains, u32 flush_domains) @@ -336,6 +357,9 @@ gen7_render_ring_flush(struct intel_ring_buffer *ring, intel_ring_emit(ring, 0); intel_ring_advance(ring); + if (flush_domains) + return gen7_ring_fbc_flush(ring, FBC_REND_NUKE); + return 0; } @@ -1685,6 +1709,7 @@ gen6_ring_dispatch_execbuffer(struct intel_ring_buffer *ring, static int gen6_ring_flush(struct intel_ring_buffer *ring, u32 invalidate, u32 flush) { + struct drm_device *dev = ring-dev; uint32_t cmd; int ret; @@ -1707,6 +1732,10 @@ static int gen6_ring_flush(struct intel_ring_buffer *ring, intel_ring_emit(ring, 0); intel_ring_emit(ring, MI_NOOP); intel_ring_advance(ring); + + if (IS_GEN7(dev) flush) + return gen7_ring_fbc_flush(ring, FBC_REND_CACHE_CLEAN); + return 0; } -- 1.7.11.7 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: stop killing pfit on i9xx
Nowadays (i.e. with Valleyview) we also have edp on non-PCH_SPLIT platforms, so just checking for LVDS is not good enough. Secondly we have full pfit pipe config tracking, so we'll correctly disable the pfit as part of the initial modeset. For fastboot we need a bit of work here to correctly kill unsupported configs (if e.g. the pfit is used on anything else than the built-in panel). But since that's not yet supported we don't need to worry. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_display.c |7 +-- drivers/gpu/drm/i915/intel_drv.h |2 +- drivers/gpu/drm/i915/intel_lvds.c| 20 ++-- 3 files changed, 12 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 421b7e2..3358851 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8815,13 +8815,8 @@ static void intel_setup_outputs(struct drm_device *dev) struct drm_i915_private *dev_priv = dev-dev_private; struct intel_encoder *encoder; bool dpd_is_edp = false; - bool has_lvds; - has_lvds = intel_lvds_init(dev); - if (!has_lvds !HAS_PCH_SPLIT(dev)) { - /* disable the panel fitter on everything but LVDS */ - I915_WRITE(PFIT_CONTROL, 0); - } + intel_lvds_init(dev); if (!IS_ULT(dev)) intel_crt_init(dev); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 1735cdc..8d178cf 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -565,7 +565,7 @@ extern void intel_tv_init(struct drm_device *dev); extern void intel_mark_busy(struct drm_device *dev); extern void intel_mark_fb_busy(struct drm_i915_gem_object *obj); extern void intel_mark_idle(struct drm_device *dev); -extern bool intel_lvds_init(struct drm_device *dev); +extern void intel_lvds_init(struct drm_device *dev); extern bool intel_is_dual_link_lvds(struct drm_device *dev); extern void intel_dp_init(struct drm_device *dev, int output_reg, enum port port); diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c index 10c3d56..eeff28e 100644 --- a/drivers/gpu/drm/i915/intel_lvds.c +++ b/drivers/gpu/drm/i915/intel_lvds.c @@ -877,7 +877,7 @@ static bool intel_lvds_supported(struct drm_device *dev) * Create the connector, register the LVDS DDC bus, and try to figure out what * modes we can display on the LVDS panel (if present). */ -bool intel_lvds_init(struct drm_device *dev) +void intel_lvds_init(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev-dev_private; struct intel_lvds_encoder *lvds_encoder; @@ -895,35 +895,35 @@ bool intel_lvds_init(struct drm_device *dev) u8 pin; if (!intel_lvds_supported(dev)) - return false; + return; /* Skip init on machines we know falsely report LVDS */ if (dmi_check_system(intel_no_lvds)) - return false; + return; pin = GMBUS_PORT_PANEL; if (!lvds_is_present_in_vbt(dev, pin)) { DRM_DEBUG_KMS(LVDS is not present in VBT\n); - return false; + return; } if (HAS_PCH_SPLIT(dev)) { if ((I915_READ(PCH_LVDS) LVDS_DETECTED) == 0) - return false; + return; if (dev_priv-vbt.edp_support) { DRM_DEBUG_KMS(disable LVDS for eDP support\n); - return false; + return; } } lvds_encoder = kzalloc(sizeof(struct intel_lvds_encoder), GFP_KERNEL); if (!lvds_encoder) - return false; + return; lvds_connector = kzalloc(sizeof(struct intel_lvds_connector), GFP_KERNEL); if (!lvds_connector) { kfree(lvds_encoder); - return false; + return; } lvds_encoder-attached_connector = lvds_connector; @@ -1094,7 +1094,7 @@ out: intel_panel_init(intel_connector-panel, fixed_mode); intel_panel_setup_backlight(connector); - return true; + return; failed: DRM_DEBUG_KMS(No LVDS modes found, disabling.\n); @@ -1104,5 +1104,5 @@ failed: drm_mode_destroy(dev, fixed_mode); kfree(lvds_encoder); kfree(lvds_connector); - return false; + return; } -- 1.7.10.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: pipe config quirk infrastructure plus sdvo mode.flags fix
On Thu, Jun 06, 2013 at 02:51:37PM +0100, Chris Wilson wrote: On Thu, Jun 06, 2013 at 02:55:52PM +0200, Daniel Vetter wrote: For various reasons the hw state readout might not be able to faithfully match the hw state: - broken hw (like the case which motivated this patch here where the sdvo encoder does not implemented mandatory functionality correctly). - platforms which are not supported fully with the pipe config infrastructure - if our code doesn't support a given hw configuration natively, e.g. special restrictions on the per-pipe panel fitters when they're used in high-quality scaling modes. In all these cases both fastboot and the hw state cross checker need to be aware of these cases and act accordingly. To be able to do this add a new quirk flag to the pipe config structure. The specific case at hand is an sdvo encoder which doesn't implement the get_timings function, so adjusted_mode flags will be wrong. The strange thing though is that the encoder _does_ work, even though it doesn't implement any of the timings functions (so neither get nor set, neither for input nor output timings). Not that non-compliant sdvo encoder are any surprise at all ... v2: - Don't read random garbage from the dtd if the get_timings call failed (suggested by Chris). - Still check the interlaced flag, that's read out from someplace else. We want maximal paranoia, after all. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk Queued for -next, thanks for the review. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Only slightly increment hangcheck score if we succesfully kick a ring
On Thu, Jun 06, 2013 at 10:45:42AM +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. 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 | 105 --- 1 file changed, 53 insertions(+), 52 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 85694d7..2d1890d 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,37 +2411,50 @@ 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 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; seqno = ring-get_seqno(ring, false); acthd = intel_ring_get_active_head(ring); - idle = i915_hangcheck_ring_idle(ring, seqno, err); - stuck[i] = ring-hangcheck.acthd == acthd; - - if (idle) { -
Re: [Intel-gfx] [PATCH 2/2] drm/i915: enable 30bpp for DP outputs
On Sat, Jun 01, 2013 at 07:45:56PM +0200, Daniel Vetter wrote: We always limited the link bw calculations to 24bpp. Tested with my shiny new high-bpc screen, seems to work as advertised. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=65280 Tested-by: shui yangwei yangweix.s...@intel.com --- drivers/gpu/drm/i915/intel_dp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index c92eeeb..9868600 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -704,7 +704,7 @@ intel_dp_compute_config(struct intel_encoder *encoder, /* Walk through all bpp values. Luckily they're all nicely spaced with 2 * bpc in between. */ - bpp = min_t(int, 8*3, pipe_config-pipe_bpp); + bpp = pipe_config-pipe_bpp; if (is_edp(intel_dp) dev_priv-vbt.edp_bpp) bpp = min_t(int, bpp, dev_priv-vbt.edp_bpp); -- 1.7.11.7 -- 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