[Intel-gfx] [PATCH 3/4] drm/i915/contexts: Serialize default context init
This is possible with the new force paramter in do_switch. As stated in that patch, the goal is to get a real context stored at the time of initialization. References: https://bugs.freedesktop.org/show_bug.cgi?id=52429 Tested-by: Guang A Yang guang.a.y...@intel.com Signed-off-by: Ben Widawsky b...@bwidawsk.net --- drivers/gpu/drm/i915/i915_gem_context.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 3945e79..c96d6f2 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -229,6 +229,10 @@ static int create_default_context(struct drm_i915_private *dev_priv) if (ret) goto err_unpin; + ret = do_switch(ctx, true); + if (ret) + goto err_unpin; + DRM_DEBUG_DRIVER(Default HW context loaded\n); return 0; -- 1.7.11.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/4] drm/i915/contexts: Add forced switches
A force parameter for switch currently only has one use, the first time we load the default context. Slightly hand-wavy explanation: We want to get the default context actually loaded so that the GPU has some real state to load (instead of garbage) after a reset or resume Therefore, the benefit to adding a parameter instead of trying to determine when to force is that we can strictly limit when such switches occur. As a +1 to me: This existed in earlier versions of the patch series, but got removed as part of the review process. The reason before was the same, and the same person who convinced me to remove it before has convinced me to re-add it. :-) References: https://bugs.freedesktop.org/show_bug.cgi?id=52429 Tested-by: Guang A Yang guang.a.y...@intel.com Signed-off-by: Ben Widawsky b...@bwidawsk.net --- drivers/gpu/drm/i915/i915_gem_context.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 5c2d354..3945e79 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -97,7 +97,7 @@ static struct i915_hw_context * i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id); -static int do_switch(struct i915_hw_context *to); +static int do_switch(struct i915_hw_context *to, bool force); static int get_context_size(struct drm_device *dev) { @@ -225,7 +225,7 @@ static int create_default_context(struct drm_i915_private *dev_priv) if (ret) goto err_destroy; - ret = do_switch(ctx); + ret = do_switch(ctx, false); if (ret) goto err_unpin; @@ -362,7 +362,7 @@ mi_set_context(struct intel_ring_buffer *ring, return ret; } -static int do_switch(struct i915_hw_context *to) +static int do_switch(struct i915_hw_context *to, bool force) { struct intel_ring_buffer *ring = to-ring; struct drm_i915_gem_object *from_obj = ring-last_context_obj; @@ -371,7 +371,7 @@ static int do_switch(struct i915_hw_context *to) BUG_ON(from_obj != NULL from_obj-pin_count == 0); - if (from_obj == to-obj) + if (!force (from_obj == to-obj)) return 0; ret = i915_gem_object_pin(to-obj, CONTEXT_ALIGN, false); @@ -392,10 +392,10 @@ static int do_switch(struct i915_hw_context *to) if (!to-obj-has_global_gtt_mapping) i915_gem_gtt_bind_object(to-obj, to-obj-cache_level); - if (!to-is_initialized || is_default_context(to)) - hw_flags |= MI_RESTORE_INHIBIT; - else if (WARN_ON_ONCE(from_obj == to-obj)) /* not yet expected */ + if (force) { hw_flags |= MI_FORCE_RESTORE; + } else if (!to-is_initialized || is_default_context(to)) + hw_flags |= MI_RESTORE_INHIBIT; ret = mi_set_context(ring, to, hw_flags); if (ret) { @@ -471,7 +471,7 @@ int i915_switch_context(struct intel_ring_buffer *ring, return -ENOENT; } - return do_switch(to); + return do_switch(to, false); } int i915_gem_context_create_ioctl(struct drm_device *dev, void *data, -- 1.7.11.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/4] drm/i915: Cleanup instdone state when idle
The previous state is bogus when we've gone into idle. Actually there would be a known state of idle (ie. all units are done), but since it differs for every platform, we can just set 0, and let the hangcheck progress as normal. References: https://bugs.freedesktop.org/show_bug.cgi?id=52429 Tested-by: Guang A Yang guang.a.y...@intel.com Signed-off-by: Ben Widawsky b...@bwidawsk.net --- drivers/gpu/drm/i915/i915_gem.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 31054fa..5f116a1 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3624,6 +3624,9 @@ i915_gem_idle(struct drm_device *dev) /* Cancel the retire work handler, which should be idle now. */ cancel_delayed_work_sync(dev_priv-mm.retire_work); + dev_priv-last_instdone = 0; + dev_priv-last_instdone1 = 0; + return 0; } -- 1.7.11.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/1] hopefully fix null pointer dereference on i915 load
On Mon, 13 Aug 2012, Mihai Moldovan io...@ionic.de wrote: Had another look at the code and would like to apologize for the confusion... No worries Mihai, thanks for testing! BR, Jani. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/4] drm/i915/contexts: fix list corruption
On Mon, 13 Aug 2012 22:41:08 -0700, Ben Widawsky b...@bwidawsk.net wrote: After reset we unconditionally reinitialize lists. If the context switch hasn't yet completed before the suspend the default context object will end up on lists that are going to go away when we resume. The patch forces the context switch to be synchronous before suspend assuring that the active/inactive tracking is correct at the time of resume. References: https://bugs.freedesktop.org/show_bug.cgi?id=52429 Tested-by: Guang A Yang guang.a.y...@intel.com Signed-off-by: Ben Widawsky b...@bwidawsk.net Reviewd-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
Re: [Intel-gfx] [PATCH] drm/i915: reorder edp disabling to fix ivb MacBook Air
On Mon, Aug 13, 2012 at 10:35:26PM -0700, Keith Packard wrote: Daniel Vetter daniel.vet...@ffwll.ch writes: Cc: Keith Packard kei...@keithp.com I tried this on top of v3.5, appears to work just fine. Thanks! Tested-by: Keith Packard kei...@keithp.com Merged to -fixes, thanks for testing. -Daniel -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ 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: Cleanup instdone state when idle
On Mon, 13 Aug 2012 22:41:11 -0700, Ben Widawsky b...@bwidawsk.net wrote: The previous state is bogus when we've gone into idle. Actually there would be a known state of idle (ie. all units are done), but since it differs for every platform, we can just set 0, and let the hangcheck progress as normal. References: https://bugs.freedesktop.org/show_bug.cgi?id=52429 Tested-by: Guang A Yang guang.a.y...@intel.com Signed-off-by: Ben Widawsky b...@bwidawsk.net I think you have a point, but I don't think this should be just on idle. Everytime we kick off the hangcheck, we should not be comparing to stale state. -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 3/4] drm/i915/contexts: Serialize default context init
On Mon, 13 Aug 2012 22:41:10 -0700, Ben Widawsky b...@bwidawsk.net wrote: This is possible with the new force paramter in do_switch. As stated in that patch, the goal is to get a real context stored at the time of initialization. References: https://bugs.freedesktop.org/show_bug.cgi?id=52429 Tested-by: Guang A Yang guang.a.y...@intel.com Signed-off-by: Ben Widawsky b...@bwidawsk.net I'm missing the rationalisation for this pair of patches... For instance, I can't see how this closes the hole we have upon resume where ring-context_obj == DEFAULT_CONTEXT but CCID is 0. -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: Apply post-sync write for pipe control invalidates
jk Sat, Aug 11, 2012 at 12:20:19PM -0700, Ben Widawsky wrote: On Fri, 10 Aug 2012 10:18:10 +0100 Chris Wilson ch...@chris-wilson.co.uk wrote: When invalidating the TLBs it is documentated as requiring a post-sync write. Failure to do so seems to result in a GPU hang. Exposure to this hang on IVB seems to be a result of removing the extra stalls required for SNB pipecontrol workarounds: commit 6c6cf5aa9c583478b19e23149feaa92d01fb8c2d Author: Chris Wilson ch...@chris-wilson.co.uk Date: Fri Jul 20 18:02:28 2012 +0100 drm/i915: Only apply the SNB pipe control w/a to gen6 Reported-by: yex.t...@intel.com Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=53322 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk This is the moral equivalent of my patch to make the simulator happy on IVB. Daniel, I'll settle for either patch. Therefore, Acked-by: Ben Widawsky b...@bwidawsk.net Patch merged to -fixes, with some manual frobbery to ensure we get a load conflict instead of a silent one. -Daniel --- drivers/gpu/drm/i915/intel_ringbuffer.c | 35 ++- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 13318a0..7608bc2 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -213,20 +213,27 @@ gen6_render_ring_flush(struct intel_ring_buffer *ring, * number of bits based on the write domains has little performance * impact. */ - flags |= PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH; - flags |= PIPE_CONTROL_TLB_INVALIDATE; - flags |= PIPE_CONTROL_INSTRUCTION_CACHE_INVALIDATE; - flags |= PIPE_CONTROL_TEXTURE_CACHE_INVALIDATE; - flags |= PIPE_CONTROL_DEPTH_CACHE_FLUSH; - flags |= PIPE_CONTROL_VF_CACHE_INVALIDATE; - flags |= PIPE_CONTROL_CONST_CACHE_INVALIDATE; - flags |= PIPE_CONTROL_STATE_CACHE_INVALIDATE; - /* -* Ensure that any following seqno writes only happen when the render -* cache is indeed flushed (but only if the caller actually wants that). -*/ - if (flush_domains) + if (flush_domains) { + flags |= PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH; + flags |= PIPE_CONTROL_DEPTH_CACHE_FLUSH; + /* +* Ensure that any following seqno writes only happen +* when the render cache is indeed flushed. +*/ flags |= PIPE_CONTROL_CS_STALL; + } + if (invalidate_domains) { + flags |= PIPE_CONTROL_TLB_INVALIDATE; + flags |= PIPE_CONTROL_INSTRUCTION_CACHE_INVALIDATE; + flags |= PIPE_CONTROL_TEXTURE_CACHE_INVALIDATE; + flags |= PIPE_CONTROL_VF_CACHE_INVALIDATE; + flags |= PIPE_CONTROL_CONST_CACHE_INVALIDATE; + flags |= PIPE_CONTROL_STATE_CACHE_INVALIDATE; + /* +* TLB invalidate requires a post-sync write. +*/ + flags |= PIPE_CONTROL_QW_WRITE; + } ret = intel_ring_begin(ring, 4); if (ret) @@ -234,7 +241,7 @@ gen6_render_ring_flush(struct intel_ring_buffer *ring, intel_ring_emit(ring, GFX_OP_PIPE_CONTROL(4)); intel_ring_emit(ring, flags); - intel_ring_emit(ring, 0); + intel_ring_emit(ring, (u32)ring-status_page.gfx_addr+2048); intel_ring_emit(ring, 0); intel_ring_advance(ring); -- Ben Widawsky, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [pull] -fixes for 3.6
Meh, I've again forgotten about all the cc's ... /me needs coffee On Tue, Aug 14, 2012 at 10:28 AM, Daniel Vetter dan...@ffwll.ch wrote: Hi Dave, A few important fixers: - fix various lvds backlight issues, regressed in 3.6 (Takashi Iwai) - make the retina mbp work (ignore bogus edp bpc value in vbt) - fix a gmbus regression introduced in (iirc) 3.4 (Jani Nikula) - fix an edp panel power sequence regression, fixes the new macbook air - apply the tlb invalidate w/a Otherwise we still have another gmbus regression (patches are awaiting tested-bys) and there's something odd going with some rare systems not entering rc6 often enough (and hence blowing through too much power). It seems to be a timing-related issue and can be mitigated by frobbing the magic tuning parameters. We're still working on that one. Also, we still have some fallout from the hw context support, but you can only hit that with mesa master. Yours, Daniel The following changes since commit 9830605d4c070b16ec5c24a75503877cc7698409: drm/mgag200: fix G200ER pll picking algorithm (2012-08-10 20:31:37 +1000) are available in the git repository at: git://people.freedesktop.org/~danvet/drm-intel drm-intel-fixes for you to fetch changes up to 7d54a904285b6e780291b91a518267bec5591913: drm/i915: Apply post-sync write for pipe control invalidates (2012-08-14 09:47:45 +0200) Chris Wilson (1): drm/i915: Apply post-sync write for pipe control invalidates Daniel Vetter (2): drm/i915: ignore eDP bpc settings from vbt drm/i915: reorder edp disabling to fix ivb MacBook Air Jani Nikula (1): drm/i915: ensure i2c adapter is all set before adding it Takashi Iwai (1): drm/i915: Fix blank panel at reopening lid drivers/gpu/drm/i915/intel_display.c| 11 - drivers/gpu/drm/i915/intel_dp.c | 14 +-- drivers/gpu/drm/i915/intel_i2c.c|7 +++--- drivers/gpu/drm/i915/intel_panel.c | 13 +++--- drivers/gpu/drm/i915/intel_ringbuffer.c | 41 +-- 5 files changed, 43 insertions(+), 43 deletions(-) -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 -- Daniel Vetter daniel.vet...@ffwll.ch - +41 (0) 79 364 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 v6 4/4] drm/i915: ironlake_write_eld code cleanup
On Thu, Aug 09, 2012 at 04:52:18PM +0800, Wang Xingchao wrote: Use _PIPE macro to get correct register definition for IBX/CPT, discard old variable i way. Signed-off-by: Wang Xingchao xingchao.w...@intel.com Ok, I've slurped in patches 1,24 for -next (with a bit of frobbing since one of the #defines used in patch 4 is introduced in patch 3). I'll pick up patch 3 as soon as the revised version is postedreviewed. Thanks for the patchesreview. -Daniel -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 3/4] drm/i915: Haswell HDMI audio initialization
On Fri, Aug 10, 2012 at 5:52 PM, Wang, Xingchao xingchao.w...@intel.com wrote: HI Deak, -Original Message- From: Imre Deak [mailto:imre.d...@gmail.com] Sent: Friday, August 10, 2012 9:15 PM To: Wang, Xingchao Cc: dan...@ffwll.ch; przan...@gmail.com; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH v6 3/4] drm/i915: Haswell HDMI audio initialization Hi, couple of comments inlined: On Thu, Aug 9, 2012 at 11:52 AM, Wang Xingchao xingchao.w...@intel.com wrote: Added new haswell_write_eld() to initialize Haswell HDMI audio registers to generate an unsolicited response to the audio controller driver to indicate that the controller sequence should start. Signed-off-by: Wang Xingchao xingchao.w...@intel.com --- drivers/gpu/drm/i915/i915_reg.h |1 + drivers/gpu/drm/i915/intel_display.c | 94 +- 2 files changed, 94 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 55aa10e..08f8b65 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4294,6 +4294,7 @@ #define AUD_DIG_CNVT(pipe) _PIPE(pipe, \ HSW_AUD_DIG_CNVT_1, \ HSW_AUD_DIG_CNVT_2) +#define DIP_PORT_SEL_MASK0x3 #define HSW_AUD_EDID_DATA_A 0x65050 #define HSW_AUD_EDID_DATA_B 0x65150 diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 70d30fc..be0950d 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5067,6 +5067,98 @@ static void g4x_write_eld(struct drm_connector *connector, I915_WRITE(G4X_AUD_CNTL_ST, i); } +static void haswell_write_eld(struct drm_connector *connector, +struct drm_crtc *crtc) { + struct drm_i915_private *dev_priv = connector-dev-dev_private; + uint8_t *eld = connector-eld; + uint32_t eldv; + uint32_t i; + int len; + int pipe = to_intel_crtc(crtc)-pipe; + int tmp; + + int hdmiw_hdmiedid = HSW_AUD_EDID_DATA(pipe); + int aud_cntl_st = HSW_AUD_DIP_ELD_CTRL(pipe); + int aud_config = HSW_AUD_CFG(pipe); + int aud_cntrl_st2 = HSW_AUD_PIN_ELD_CP_VLD; + + + DRM_DEBUG_DRIVER(HDMI: Haswell Audio initialize\n); + + /* Audio output enable */ + DRM_DEBUG_DRIVER(HDMI audio: enable codec\n); + tmp = I915_READ(aud_cntrl_st2); uint32_t for tmp would be better, in case you happen to right shift tmp with bit31 set. Agree, will change it to u32 type in next version. + tmp |= (AUDIO_OUTPUT_ENABLE_A | AUDIO_OUTPUT_ENABLE_B | AUDIO_OUTPUT_ENABLE_C); + I915_WRITE(aud_cntrl_st2, tmp); I don't know much about HDMI, but according to the spec you'd need to wait for one vblank here. At least a comment why we don't do it would be nice. Yeah, according to audio programming sequence, wait 1 vertical blank here. I left it empty here because I did not found the proper api and did not meet issue during test. Is intel_wait_for_vblank() the proper one I should call ? Yes, although it will work only in non-atomic context. Also the relevant pipe needs to be enabled otherwise you'll get a timeout. + + /* Set ELD valid state */ + tmp = I915_READ(aud_cntrl_st2); + DRM_DEBUG_DRIVER(HDMI audio: pin eld vld status=0x%8x\n, tmp); + tmp |= (AUDIO_ELD_VALID_A | AUDIO_ELD_VALID_B | AUDIO_ELD_VALID_C); + I915_WRITE(aud_cntrl_st2, tmp); + tmp = I915_READ(aud_cntrl_st2); + DRM_DEBUG_DRIVER(HDMI audio: eld vld status=0x%8x\n, tmp); + + /* Enable HDMI mode */ + tmp = I915_READ(aud_config); + DRM_DEBUG_DRIVER(HDMI audio: audio conf: 0x%8x\n, tmp); + /* clear N_programing_enable and N_value_index */ + tmp = ~(AUD_CONFIG_N_VALUE_INDEX | AUD_CONFIG_N_PROG_ENABLE); + I915_WRITE(aud_config, tmp); + + DRM_DEBUG_DRIVER(ELD on pipe %c\n, pipe_name(pipe)); + + i = I915_READ(aud_cntl_st); + i = (i 29) DIP_PORT_SEL_MASK; /* DIP_Port_Select, 0x1 = PortB */ + if (!i) { + DRM_DEBUG_DRIVER(Audio directed to unknown port\n); + /* operate blindly on all ports */ + eldv = AUDIO_ELD_VALID_A; + eldv |= AUDIO_ELD_VALID_B; + eldv |= AUDIO_ELD_VALID_C; + } else { + DRM_DEBUG_DRIVER(ELD on port %c\n, 'A' + i); + eldv = AUDIO_ELD_VALID_A ((i - 1) * 4); This is confusing. Reading the spec I undersand i=1 means port B, but in that case we'll set eldv to AUDIO_ELD_VALID_A. Is it intentional? Bit 0 is for ELD Valid A in Project DevHSW and for ELD Valid B for HSW-X0, and HSW-X0 is old
[Intel-gfx] [PATCH] drm/i915: fix hsw uncached pte
From: Daniel Vetter daniel.vet...@ffwll.ch They've changed it ... for no apparent reason. Meh. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/char/agp/intel-agp.h| 1 + drivers/char/agp/intel-gtt.c| 107 drivers/gpu/drm/i915/i915_gem_gtt.c | 5 +- drivers/gpu/drm/i915/i915_reg.h | 1 + 4 files changed, 77 insertions(+), 37 deletions(-) Applies to -fixes. diff --git a/drivers/char/agp/intel-agp.h b/drivers/char/agp/intel-agp.h index 6f007b6..6ec0fff 100644 --- a/drivers/char/agp/intel-agp.h +++ b/drivers/char/agp/intel-agp.h @@ -64,6 +64,7 @@ #define I830_PTE_SYSTEM_CACHED 0x0006 /* GT PTE cache control fields */ #define GEN6_PTE_UNCACHED 0x0002 +#define HSW_PTE_UNCACHED 0x #define GEN6_PTE_LLC 0x0004 #define GEN6_PTE_LLC_MLC 0x0006 #define GEN6_PTE_GFDT 0x0008 diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c index 08fc5cb..8f956db 100644 --- a/drivers/char/agp/intel-gtt.c +++ b/drivers/char/agp/intel-gtt.c @@ -44,6 +44,7 @@ struct intel_gtt_driver { unsigned int is_g33 : 1; unsigned int is_pineview : 1; unsigned int is_ironlake : 1; + unsigned int is_hsw : 1; unsigned int has_pgtbl_enable : 1; unsigned int dma_mask_size : 8; /* Chipset specific GTT setup */ @@ -1156,6 +1157,30 @@ static bool gen6_check_flags(unsigned int flags) return true; } +static void haswell_write_entry(dma_addr_t addr, unsigned int entry, + unsigned int flags) +{ + unsigned int type_mask = flags ~AGP_USER_CACHED_MEMORY_GFDT; + unsigned int gfdt = flags AGP_USER_CACHED_MEMORY_GFDT; + u32 pte_flags; + + if (type_mask == AGP_USER_MEMORY) + pte_flags = HSW_PTE_UNCACHED | I810_PTE_VALID; + else if (type_mask == AGP_USER_CACHED_MEMORY_LLC_MLC) { + pte_flags = GEN6_PTE_LLC_MLC | I810_PTE_VALID; + if (gfdt) + pte_flags |= GEN6_PTE_GFDT; + } else { /* set 'normal'/'cached' to LLC by default */ + pte_flags = GEN6_PTE_LLC | I810_PTE_VALID; + if (gfdt) + pte_flags |= GEN6_PTE_GFDT; + } + + /* gen6 has bit11-4 for physical addr bit39-32 */ + addr |= (addr 28) 0xff0; + writel(addr | pte_flags, intel_private.gtt + entry); +} + static void gen6_write_entry(dma_addr_t addr, unsigned int entry, unsigned int flags) { @@ -1382,6 +1407,16 @@ static const struct intel_gtt_driver sandybridge_gtt_driver = { .check_flags = gen6_check_flags, .chipset_flush = i9xx_chipset_flush, }; +static const struct intel_gtt_driver haswell_gtt_driver = { + .gen = 6, + .setup = i9xx_setup, + .cleanup = gen6_cleanup, + .write_entry = haswell_write_entry, + .dma_mask_size = 40, + .is_hsw = 1, + .check_flags = gen6_check_flags, + .chipset_flush = i9xx_chipset_flush, +}; static const struct intel_gtt_driver valleyview_gtt_driver = { .gen = 7, .setup = i9xx_setup, @@ -1499,77 +1534,77 @@ static const struct intel_gtt_driver_description { { PCI_DEVICE_ID_INTEL_VALLEYVIEW_IG, ValleyView, valleyview_gtt_driver }, { PCI_DEVICE_ID_INTEL_HASWELL_D_GT1_IG, - Haswell, sandybridge_gtt_driver }, + Haswell, haswell_gtt_driver }, { PCI_DEVICE_ID_INTEL_HASWELL_D_GT2_IG, - Haswell, sandybridge_gtt_driver }, + Haswell, haswell_gtt_driver }, { PCI_DEVICE_ID_INTEL_HASWELL_D_GT2_PLUS_IG, - Haswell, sandybridge_gtt_driver }, + Haswell, haswell_gtt_driver }, { PCI_DEVICE_ID_INTEL_HASWELL_M_GT1_IG, - Haswell, sandybridge_gtt_driver }, + Haswell, haswell_gtt_driver }, { PCI_DEVICE_ID_INTEL_HASWELL_M_GT2_IG, - Haswell, sandybridge_gtt_driver }, + Haswell, haswell_gtt_driver }, { PCI_DEVICE_ID_INTEL_HASWELL_M_GT2_PLUS_IG, - Haswell, sandybridge_gtt_driver }, + Haswell, haswell_gtt_driver }, { PCI_DEVICE_ID_INTEL_HASWELL_S_GT1_IG, - Haswell, sandybridge_gtt_driver }, + Haswell, haswell_gtt_driver }, { PCI_DEVICE_ID_INTEL_HASWELL_S_GT2_IG, - Haswell, sandybridge_gtt_driver }, + Haswell, haswell_gtt_driver }, { PCI_DEVICE_ID_INTEL_HASWELL_S_GT2_PLUS_IG, - Haswell, sandybridge_gtt_driver }, + Haswell, haswell_gtt_driver }, { PCI_DEVICE_ID_INTEL_HASWELL_SDV_D_GT1_IG, - Haswell, sandybridge_gtt_driver }, + Haswell, haswell_gtt_driver }, { PCI_DEVICE_ID_INTEL_HASWELL_SDV_D_GT2_IG, - Haswell, sandybridge_gtt_driver }, + Haswell, haswell_gtt_driver }, { PCI_DEVICE_ID_INTEL_HASWELL_SDV_D_GT2_PLUS_IG, - Haswell,
Re: [Intel-gfx] [PATCH] drm/i915: fix hsw uncached pte
On Tue, 14 Aug 2012, Paulo Zanoni przan...@gmail.com wrote: From: Daniel Vetter daniel.vet...@ffwll.ch They've changed it ... for no apparent reason. Meh. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/char/agp/intel-agp.h| 1 + drivers/char/agp/intel-gtt.c| 107 drivers/gpu/drm/i915/i915_gem_gtt.c | 5 +- drivers/gpu/drm/i915/i915_reg.h | 1 + 4 files changed, 77 insertions(+), 37 deletions(-) Applies to -fixes. diff --git a/drivers/char/agp/intel-agp.h b/drivers/char/agp/intel-agp.h index 6f007b6..6ec0fff 100644 --- a/drivers/char/agp/intel-agp.h +++ b/drivers/char/agp/intel-agp.h @@ -64,6 +64,7 @@ #define I830_PTE_SYSTEM_CACHED 0x0006 /* GT PTE cache control fields */ #define GEN6_PTE_UNCACHED0x0002 +#define HSW_PTE_UNCACHED 0x #define GEN6_PTE_LLC 0x0004 #define GEN6_PTE_LLC_MLC 0x0006 #define GEN6_PTE_GFDT0x0008 diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c index 08fc5cb..8f956db 100644 --- a/drivers/char/agp/intel-gtt.c +++ b/drivers/char/agp/intel-gtt.c @@ -44,6 +44,7 @@ struct intel_gtt_driver { unsigned int is_g33 : 1; unsigned int is_pineview : 1; unsigned int is_ironlake : 1; + unsigned int is_hsw : 1; This bit is set in haswell_gtt_driver, but it's not referenced anywhere, is it? Why do you add it? Am I missing something? BR, Jani. unsigned int has_pgtbl_enable : 1; unsigned int dma_mask_size : 8; /* Chipset specific GTT setup */ @@ -1156,6 +1157,30 @@ static bool gen6_check_flags(unsigned int flags) return true; } +static void haswell_write_entry(dma_addr_t addr, unsigned int entry, + unsigned int flags) +{ + unsigned int type_mask = flags ~AGP_USER_CACHED_MEMORY_GFDT; + unsigned int gfdt = flags AGP_USER_CACHED_MEMORY_GFDT; + u32 pte_flags; + + if (type_mask == AGP_USER_MEMORY) + pte_flags = HSW_PTE_UNCACHED | I810_PTE_VALID; + else if (type_mask == AGP_USER_CACHED_MEMORY_LLC_MLC) { + pte_flags = GEN6_PTE_LLC_MLC | I810_PTE_VALID; + if (gfdt) + pte_flags |= GEN6_PTE_GFDT; + } else { /* set 'normal'/'cached' to LLC by default */ + pte_flags = GEN6_PTE_LLC | I810_PTE_VALID; + if (gfdt) + pte_flags |= GEN6_PTE_GFDT; + } + + /* gen6 has bit11-4 for physical addr bit39-32 */ + addr |= (addr 28) 0xff0; + writel(addr | pte_flags, intel_private.gtt + entry); +} + static void gen6_write_entry(dma_addr_t addr, unsigned int entry, unsigned int flags) { @@ -1382,6 +1407,16 @@ static const struct intel_gtt_driver sandybridge_gtt_driver = { .check_flags = gen6_check_flags, .chipset_flush = i9xx_chipset_flush, }; +static const struct intel_gtt_driver haswell_gtt_driver = { + .gen = 6, + .setup = i9xx_setup, + .cleanup = gen6_cleanup, + .write_entry = haswell_write_entry, + .dma_mask_size = 40, + .is_hsw = 1, + .check_flags = gen6_check_flags, + .chipset_flush = i9xx_chipset_flush, +}; static const struct intel_gtt_driver valleyview_gtt_driver = { .gen = 7, .setup = i9xx_setup, @@ -1499,77 +1534,77 @@ static const struct intel_gtt_driver_description { { PCI_DEVICE_ID_INTEL_VALLEYVIEW_IG, ValleyView, valleyview_gtt_driver }, { PCI_DEVICE_ID_INTEL_HASWELL_D_GT1_IG, - Haswell, sandybridge_gtt_driver }, + Haswell, haswell_gtt_driver }, { PCI_DEVICE_ID_INTEL_HASWELL_D_GT2_IG, - Haswell, sandybridge_gtt_driver }, + Haswell, haswell_gtt_driver }, { PCI_DEVICE_ID_INTEL_HASWELL_D_GT2_PLUS_IG, - Haswell, sandybridge_gtt_driver }, + Haswell, haswell_gtt_driver }, { PCI_DEVICE_ID_INTEL_HASWELL_M_GT1_IG, - Haswell, sandybridge_gtt_driver }, + Haswell, haswell_gtt_driver }, { PCI_DEVICE_ID_INTEL_HASWELL_M_GT2_IG, - Haswell, sandybridge_gtt_driver }, + Haswell, haswell_gtt_driver }, { PCI_DEVICE_ID_INTEL_HASWELL_M_GT2_PLUS_IG, - Haswell, sandybridge_gtt_driver }, + Haswell, haswell_gtt_driver }, { PCI_DEVICE_ID_INTEL_HASWELL_S_GT1_IG, - Haswell, sandybridge_gtt_driver }, + Haswell, haswell_gtt_driver }, { PCI_DEVICE_ID_INTEL_HASWELL_S_GT2_IG, - Haswell, sandybridge_gtt_driver }, + Haswell, haswell_gtt_driver }, { PCI_DEVICE_ID_INTEL_HASWELL_S_GT2_PLUS_IG, - Haswell, sandybridge_gtt_driver }, + Haswell, haswell_gtt_driver }, { PCI_DEVICE_ID_INTEL_HASWELL_SDV_D_GT1_IG, - Haswell, sandybridge_gtt_driver }, + Haswell, haswell_gtt_driver }, { PCI_DEVICE_ID_INTEL_HASWELL_SDV_D_GT2_IG, -
[Intel-gfx] [PATCH] drm/i915: fix hsw uncached pte
From: Daniel Vetter daniel.vet...@ffwll.ch They've changed it ... for no apparent reason. Meh. V2: remove unused 'is_hsw' field. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/char/agp/intel-agp.h|1 + drivers/char/agp/intel-gtt.c| 105 +++ drivers/gpu/drm/i915/i915_gem_gtt.c |5 +- drivers/gpu/drm/i915/i915_reg.h |1 + 4 files changed, 75 insertions(+), 37 deletions(-) diff --git a/drivers/char/agp/intel-agp.h b/drivers/char/agp/intel-agp.h index 6f007b6..6ec0fff 100644 --- a/drivers/char/agp/intel-agp.h +++ b/drivers/char/agp/intel-agp.h @@ -64,6 +64,7 @@ #define I830_PTE_SYSTEM_CACHED 0x0006 /* GT PTE cache control fields */ #define GEN6_PTE_UNCACHED 0x0002 +#define HSW_PTE_UNCACHED 0x #define GEN6_PTE_LLC 0x0004 #define GEN6_PTE_LLC_MLC 0x0006 #define GEN6_PTE_GFDT 0x0008 diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c index 08fc5cb..58e32f7 100644 --- a/drivers/char/agp/intel-gtt.c +++ b/drivers/char/agp/intel-gtt.c @@ -1156,6 +1156,30 @@ static bool gen6_check_flags(unsigned int flags) return true; } +static void haswell_write_entry(dma_addr_t addr, unsigned int entry, + unsigned int flags) +{ + unsigned int type_mask = flags ~AGP_USER_CACHED_MEMORY_GFDT; + unsigned int gfdt = flags AGP_USER_CACHED_MEMORY_GFDT; + u32 pte_flags; + + if (type_mask == AGP_USER_MEMORY) + pte_flags = HSW_PTE_UNCACHED | I810_PTE_VALID; + else if (type_mask == AGP_USER_CACHED_MEMORY_LLC_MLC) { + pte_flags = GEN6_PTE_LLC_MLC | I810_PTE_VALID; + if (gfdt) + pte_flags |= GEN6_PTE_GFDT; + } else { /* set 'normal'/'cached' to LLC by default */ + pte_flags = GEN6_PTE_LLC | I810_PTE_VALID; + if (gfdt) + pte_flags |= GEN6_PTE_GFDT; + } + + /* gen6 has bit11-4 for physical addr bit39-32 */ + addr |= (addr 28) 0xff0; + writel(addr | pte_flags, intel_private.gtt + entry); +} + static void gen6_write_entry(dma_addr_t addr, unsigned int entry, unsigned int flags) { @@ -1382,6 +1406,15 @@ static const struct intel_gtt_driver sandybridge_gtt_driver = { .check_flags = gen6_check_flags, .chipset_flush = i9xx_chipset_flush, }; +static const struct intel_gtt_driver haswell_gtt_driver = { + .gen = 6, + .setup = i9xx_setup, + .cleanup = gen6_cleanup, + .write_entry = haswell_write_entry, + .dma_mask_size = 40, + .check_flags = gen6_check_flags, + .chipset_flush = i9xx_chipset_flush, +}; static const struct intel_gtt_driver valleyview_gtt_driver = { .gen = 7, .setup = i9xx_setup, @@ -1499,77 +1532,77 @@ static const struct intel_gtt_driver_description { { PCI_DEVICE_ID_INTEL_VALLEYVIEW_IG, ValleyView, valleyview_gtt_driver }, { PCI_DEVICE_ID_INTEL_HASWELL_D_GT1_IG, - Haswell, sandybridge_gtt_driver }, + Haswell, haswell_gtt_driver }, { PCI_DEVICE_ID_INTEL_HASWELL_D_GT2_IG, - Haswell, sandybridge_gtt_driver }, + Haswell, haswell_gtt_driver }, { PCI_DEVICE_ID_INTEL_HASWELL_D_GT2_PLUS_IG, - Haswell, sandybridge_gtt_driver }, + Haswell, haswell_gtt_driver }, { PCI_DEVICE_ID_INTEL_HASWELL_M_GT1_IG, - Haswell, sandybridge_gtt_driver }, + Haswell, haswell_gtt_driver }, { PCI_DEVICE_ID_INTEL_HASWELL_M_GT2_IG, - Haswell, sandybridge_gtt_driver }, + Haswell, haswell_gtt_driver }, { PCI_DEVICE_ID_INTEL_HASWELL_M_GT2_PLUS_IG, - Haswell, sandybridge_gtt_driver }, + Haswell, haswell_gtt_driver }, { PCI_DEVICE_ID_INTEL_HASWELL_S_GT1_IG, - Haswell, sandybridge_gtt_driver }, + Haswell, haswell_gtt_driver }, { PCI_DEVICE_ID_INTEL_HASWELL_S_GT2_IG, - Haswell, sandybridge_gtt_driver }, + Haswell, haswell_gtt_driver }, { PCI_DEVICE_ID_INTEL_HASWELL_S_GT2_PLUS_IG, - Haswell, sandybridge_gtt_driver }, + Haswell, haswell_gtt_driver }, { PCI_DEVICE_ID_INTEL_HASWELL_SDV_D_GT1_IG, - Haswell, sandybridge_gtt_driver }, + Haswell, haswell_gtt_driver }, { PCI_DEVICE_ID_INTEL_HASWELL_SDV_D_GT2_IG, - Haswell, sandybridge_gtt_driver }, + Haswell, haswell_gtt_driver }, { PCI_DEVICE_ID_INTEL_HASWELL_SDV_D_GT2_PLUS_IG, - Haswell, sandybridge_gtt_driver }, + Haswell, haswell_gtt_driver }, { PCI_DEVICE_ID_INTEL_HASWELL_SDV_M_GT1_IG, - Haswell, sandybridge_gtt_driver }, + Haswell, haswell_gtt_driver }, { PCI_DEVICE_ID_INTEL_HASWELL_SDV_M_GT2_IG, -
Re: [Intel-gfx] [PATCH] drm/i915: fix hsw uncached pte
Paulo Zanoni przan...@gmail.com writes: +#define HSW_PTE_UNCACHED 0x Are you sure this value should be zero? It seems pretty unlikely to me. -- keith.pack...@intel.com pgptzbyqd415W.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/4] drm/i915/contexts: Serialize default context init
On 2012-08-14 00:41, Chris Wilson wrote: On Mon, 13 Aug 2012 22:41:10 -0700, Ben Widawsky b...@bwidawsk.net wrote: This is possible with the new force paramter in do_switch. As stated in that patch, the goal is to get a real context stored at the time of initialization. References: https://bugs.freedesktop.org/show_bug.cgi?id=52429 Tested-by: Guang A Yang guang.a.y...@intel.com Signed-off-by: Ben Widawsky b...@bwidawsk.net I'm missing the rationalisation for this pair of patches... For instance, I can't see how this closes the hole we have upon resume where ring-context_obj == DEFAULT_CONTEXT but CCID is 0. -Chris Yeah this doesn't fix that problem. The problem this is trying to solve is suspend/resume before any context switch actually occurs. Basically jam the default context obj in, and this allows us to force restore it on resume. However, as you point out, I guess that force restore is missing. Let me think a bit more/chat on IRC. -- Ben Widawsky, 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 4/4] drm/i915: Cleanup instdone state when idle
On 2012-08-14 00:39, Chris Wilson wrote: On Mon, 13 Aug 2012 22:41:11 -0700, Ben Widawsky b...@bwidawsk.net wrote: The previous state is bogus when we've gone into idle. Actually there would be a known state of idle (ie. all units are done), but since it differs for every platform, we can just set 0, and let the hangcheck progress as normal. References: https://bugs.freedesktop.org/show_bug.cgi?id=52429 Tested-by: Guang A Yang guang.a.y...@intel.com Signed-off-by: Ben Widawsky b...@bwidawsk.net I think you have a point, but I don't think this should be just on idle. Everytime we kick off the hangcheck, we should not be comparing to stale state. -Chris Are you suggesting this is a patch which already exists somewhere? -- Ben Widawsky, 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 4/4] drm/i915: Cleanup instdone state when idle
On Tue, 14 Aug 2012 09:42:07 -0700, Ben Widawsky b...@bwidawsk.net wrote: On 2012-08-14 00:39, Chris Wilson wrote: On Mon, 13 Aug 2012 22:41:11 -0700, Ben Widawsky b...@bwidawsk.net wrote: The previous state is bogus when we've gone into idle. Actually there would be a known state of idle (ie. all units are done), but since it differs for every platform, we can just set 0, and let the hangcheck progress as normal. References: https://bugs.freedesktop.org/show_bug.cgi?id=52429 Tested-by: Guang A Yang guang.a.y...@intel.com Signed-off-by: Ben Widawsky b...@bwidawsk.net I think you have a point, but I don't think this should be just on idle. Everytime we kick off the hangcheck, we should not be comparing to stale state. Are you suggesting this is a patch which already exists somewhere? Not yet. :) Just that I agree with you that this a problem, and it is a bigger problem than first thought. -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/5] Some HW context fixes
Daniel, the first 3 patches I believe are all that's required to fix https://bugs.freedesktop.org/show_bug.cgi?id=52429 The latter 2 are things we've discussed sort of. I've managed to go back and forth several times on whether or not we actually need them, so I'll just let you decide if you want them. I'm not completely certain they aren't required to fix #52429, but it seems likely. Ben Widawsky (5): drm/i915: Cleanup instdone state drm/i915/contexts: fix list corruption drm/i915/contexts: Switch to default on resume drm/i915/contexts: Add forced switches drm/i915/contexts: Serialize default context init drivers/gpu/drm/i915/i915_gem.c | 5 +++-- drivers/gpu/drm/i915/i915_gem_context.c | 37 +++-- 2 files changed, 29 insertions(+), 13 deletions(-) -- 1.7.11.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/5] drm/i915: Cleanup instdone state
Clear the cached instdone state to match what we expect from hardware and prevent us from comparing stale values. Actually, clearing the state is not the same as setting idle state. There would be a known state of idle (ie. all units are done), but since it differs for every platform, we can just set 0, and let the hangcheck progress as normal. By putting the clear into add_request we are essentially initializing the cached instdone to a known state before we start the hangcheck timer. v2: clear instdone in more place (Chris) Rewrote the commit message References: https://bugs.freedesktop.org/show_bug.cgi?id=52429 Tested-by: Guang A Yang guang.a.y...@intel.com Signed-off-by: Ben Widawsky b...@bwidawsk.net --- drivers/gpu/drm/i915/i915_gem.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 0514593..0d992e6 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1601,6 +1601,7 @@ i915_add_request(struct intel_ring_buffer *ring, if (!dev_priv-mm.suspended) { if (i915_enable_hangcheck) { + dev_priv-last_instdone = dev_priv-last_instdone1 = 0; mod_timer(dev_priv-hangcheck_timer, jiffies + msecs_to_jiffies(DRM_I915_HANGCHECK_PERIOD)); -- 1.7.11.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/5] drm/i915/contexts: fix list corruption
After reset we unconditionally reinitialize lists. If the context switch hasn't yet completed before the suspend, the default context object will end up on lists that are going to go away when we resume. The patch forces the context switch to be synchronous before suspend assuring that the active/inactive tracking is correct at the time of resume. References: https://bugs.freedesktop.org/show_bug.cgi?id=52429 Tested-by: Guang A Yang guang.a.y...@intel.com Signed-off-by: Ben Widawsky b...@bwidawsk.net Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_gem.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 0d992e6..0edec12 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2274,11 +2274,11 @@ int i915_gpu_idle(struct drm_device *dev) /* Flush everything onto the inactive list. */ for_each_ring(ring, dev_priv, i) { - ret = i915_ring_idle(ring); + ret = i915_switch_context(ring, NULL, DEFAULT_CONTEXT_ID); if (ret) return ret; - ret = i915_switch_context(ring, NULL, DEFAULT_CONTEXT_ID); + ret = i915_ring_idle(ring); if (ret) return ret; } -- 1.7.11.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/5] drm/i915/contexts: Switch to default on resume
In order to make the HW state CCID match with what we think it should be, we must order a switch to the default context. The really sad thing here is that the switch can potentially fail, and as such we have to assume contexts no longer work. There is likely room for improvement but until we actually start seeing the case occur, I think it should be fine. This was accidentally left this out of the first series, noticed by Chris Wilson. Signed-off-by: Ben Widawsky b...@bwidawsk.net --- drivers/gpu/drm/i915/i915_gem_context.c | 17 ++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 5c2d354..3e884dc 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -244,15 +244,26 @@ void i915_gem_context_init(struct drm_device *dev) struct drm_i915_private *dev_priv = dev-dev_private; uint32_t ctx_size; + if (dev_priv-hw_contexts_disabled) + return; + if (!HAS_HW_CONTEXTS(dev)) { dev_priv-hw_contexts_disabled = true; return; } - /* If called from reset, or thaw... we've been here already */ - if (dev_priv-hw_contexts_disabled || - dev_priv-ring[RCS].default_context) + /* If called from reset, or thaw we want to switch back to the default +* context so our HW CCID matches what we think it should be in software. +*/ + if (dev_priv-ring[RCS].default_context) { + int ret = do_switch(dev_priv-ring[RCS].default_context); + if (ret) { + DRM_ERROR(HW contexts were broken after resume\n); + i915_gem_context_fini(dev); + dev_priv-hw_contexts_disabled = true; + } return; + } ctx_size = get_context_size(dev); dev_priv-hw_context_size = get_context_size(dev); -- 1.7.11.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/5] drm/i915/contexts: Add forced switches
A force parameter for switch currently only has one use, the first time we load the default context. Slightly hand-wavy explanation: We want to get the default context actually loaded so that the GPU has some real state to load (instead of garbage) after a reset or resume. The reason it's hand-wavy is because the context we load is still somewhat random. Certain things may be initialized, and certain other things may not. However it should definitely be better than he random state that HW comes up in (and better than all 0's). Therefore, the benefit to adding a parameter instead of trying to determine when to force is that we can strictly limit when such switches occur. References: https://bugs.freedesktop.org/show_bug.cgi?id=52429 Tested-by: Guang A Yang guang.a.y...@intel.com Signed-off-by: Ben Widawsky b...@bwidawsk.net --- drivers/gpu/drm/i915/i915_gem_context.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 3e884dc..3db84e2 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -97,7 +97,7 @@ static struct i915_hw_context * i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id); -static int do_switch(struct i915_hw_context *to); +static int do_switch(struct i915_hw_context *to, bool force); static int get_context_size(struct drm_device *dev) { @@ -225,7 +225,7 @@ static int create_default_context(struct drm_i915_private *dev_priv) if (ret) goto err_destroy; - ret = do_switch(ctx); + ret = do_switch(ctx, false); if (ret) goto err_unpin; @@ -256,7 +256,7 @@ void i915_gem_context_init(struct drm_device *dev) * context so our HW CCID matches what we think it should be in software. */ if (dev_priv-ring[RCS].default_context) { - int ret = do_switch(dev_priv-ring[RCS].default_context); + int ret = do_switch(dev_priv-ring[RCS].default_context, false); if (ret) { DRM_ERROR(HW contexts were broken after resume\n); i915_gem_context_fini(dev); @@ -373,7 +373,7 @@ mi_set_context(struct intel_ring_buffer *ring, return ret; } -static int do_switch(struct i915_hw_context *to) +static int do_switch(struct i915_hw_context *to, bool force) { struct intel_ring_buffer *ring = to-ring; struct drm_i915_gem_object *from_obj = ring-last_context_obj; @@ -382,7 +382,7 @@ static int do_switch(struct i915_hw_context *to) BUG_ON(from_obj != NULL from_obj-pin_count == 0); - if (from_obj == to-obj) + if (!force (from_obj == to-obj)) return 0; ret = i915_gem_object_pin(to-obj, CONTEXT_ALIGN, false); @@ -403,10 +403,10 @@ static int do_switch(struct i915_hw_context *to) if (!to-obj-has_global_gtt_mapping) i915_gem_gtt_bind_object(to-obj, to-obj-cache_level); - if (!to-is_initialized || is_default_context(to)) - hw_flags |= MI_RESTORE_INHIBIT; - else if (WARN_ON_ONCE(from_obj == to-obj)) /* not yet expected */ + if (force) { hw_flags |= MI_FORCE_RESTORE; + } else if (!to-is_initialized || is_default_context(to)) + hw_flags |= MI_RESTORE_INHIBIT; ret = mi_set_context(ring, to, hw_flags); if (ret) { @@ -482,7 +482,7 @@ int i915_switch_context(struct intel_ring_buffer *ring, return -ENOENT; } - return do_switch(to); + return do_switch(to, false); } int i915_gem_context_create_ioctl(struct drm_device *dev, void *data, -- 1.7.11.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 5/5] drm/i915/contexts: Serialize default context init
This is possible with the new force parameter in do_switch. As stated in that patch, the goal is to get a real context stored at the time of initialization. References: https://bugs.freedesktop.org/show_bug.cgi?id=52429 Tested-by: Guang A Yang guang.a.y...@intel.com Signed-off-by: Ben Widawsky b...@bwidawsk.net --- drivers/gpu/drm/i915/i915_gem_context.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 3db84e2..eed00da 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -229,6 +229,10 @@ static int create_default_context(struct drm_i915_private *dev_priv) if (ret) goto err_unpin; + ret = do_switch(ctx, true); + if (ret) + goto err_unpin; + DRM_DEBUG_DRIVER(Default HW context loaded\n); return 0; -- 1.7.11.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 3/4] drm/i915: Haswell HDMI audio initialization
-Original Message- From: Imre Deak [mailto:imre.d...@gmail.com] Sent: Tuesday, August 14, 2012 8:36 PM To: Wang, Xingchao Cc: dan...@ffwll.ch; przan...@gmail.com; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH v6 3/4] drm/i915: Haswell HDMI audio initialization On Fri, Aug 10, 2012 at 5:52 PM, Wang, Xingchao xingchao.w...@intel.com wrote: HI Deak, -Original Message- From: Imre Deak [mailto:imre.d...@gmail.com] Sent: Friday, August 10, 2012 9:15 PM To: Wang, Xingchao Cc: dan...@ffwll.ch; przan...@gmail.com; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH v6 3/4] drm/i915: Haswell HDMI audio initialization Hi, couple of comments inlined: On Thu, Aug 9, 2012 at 11:52 AM, Wang Xingchao xingchao.w...@intel.com wrote: Added new haswell_write_eld() to initialize Haswell HDMI audio registers to generate an unsolicited response to the audio controller driver to indicate that the controller sequence should start. Signed-off-by: Wang Xingchao xingchao.w...@intel.com --- drivers/gpu/drm/i915/i915_reg.h |1 + drivers/gpu/drm/i915/intel_display.c | 94 +- 2 files changed, 94 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 55aa10e..08f8b65 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4294,6 +4294,7 @@ #define AUD_DIG_CNVT(pipe) _PIPE(pipe, \ HSW_AUD_DIG_CNVT_1, \ HSW_AUD_DIG_CNVT_2) +#define DIP_PORT_SEL_MASK0x3 #define HSW_AUD_EDID_DATA_A 0x65050 #define HSW_AUD_EDID_DATA_B 0x65150 diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 70d30fc..be0950d 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5067,6 +5067,98 @@ static void g4x_write_eld(struct drm_connector *connector, I915_WRITE(G4X_AUD_CNTL_ST, i); } +static void haswell_write_eld(struct drm_connector *connector, +struct drm_crtc *crtc) { + struct drm_i915_private *dev_priv = connector-dev-dev_private; + uint8_t *eld = connector-eld; + uint32_t eldv; + uint32_t i; + int len; + int pipe = to_intel_crtc(crtc)-pipe; + int tmp; + + int hdmiw_hdmiedid = HSW_AUD_EDID_DATA(pipe); + int aud_cntl_st = HSW_AUD_DIP_ELD_CTRL(pipe); + int aud_config = HSW_AUD_CFG(pipe); + int aud_cntrl_st2 = HSW_AUD_PIN_ELD_CP_VLD; + + + DRM_DEBUG_DRIVER(HDMI: Haswell Audio initialize\n); + + /* Audio output enable */ + DRM_DEBUG_DRIVER(HDMI audio: enable codec\n); + tmp = I915_READ(aud_cntrl_st2); uint32_t for tmp would be better, in case you happen to right shift tmp with bit31 set. Agree, will change it to u32 type in next version. + tmp |= (AUDIO_OUTPUT_ENABLE_A | AUDIO_OUTPUT_ENABLE_B | AUDIO_OUTPUT_ENABLE_C); + I915_WRITE(aud_cntrl_st2, tmp); I don't know much about HDMI, but according to the spec you'd need to wait for one vblank here. At least a comment why we don't do it would be nice. Yeah, according to audio programming sequence, wait 1 vertical blank here. I left it empty here because I did not found the proper api and did not meet issue during test. Is intel_wait_for_vblank() the proper one I should call ? Yes, although it will work only in non-atomic context. Also the relevant pipe needs to be enabled otherwise you'll get a timeout. Okay, I made the change. Haswell_write_eld() is called after ironlake_crtc_enable() which pipe was enabled there, so I think it's safe. + + /* Set ELD valid state */ + tmp = I915_READ(aud_cntrl_st2); + DRM_DEBUG_DRIVER(HDMI audio: pin eld vld status=0x%8x\n, tmp); + tmp |= (AUDIO_ELD_VALID_A | AUDIO_ELD_VALID_B | AUDIO_ELD_VALID_C); + I915_WRITE(aud_cntrl_st2, tmp); + tmp = I915_READ(aud_cntrl_st2); + DRM_DEBUG_DRIVER(HDMI audio: eld vld status=0x%8x\n, tmp); + + /* Enable HDMI mode */ + tmp = I915_READ(aud_config); + DRM_DEBUG_DRIVER(HDMI audio: audio conf: 0x%8x\n, tmp); + /* clear N_programing_enable and N_value_index */ + tmp = ~(AUD_CONFIG_N_VALUE_INDEX | AUD_CONFIG_N_PROG_ENABLE); + I915_WRITE(aud_config, tmp); + + DRM_DEBUG_DRIVER(ELD on pipe %c\n, pipe_name(pipe)); + + i = I915_READ(aud_cntl_st); + i = (i 29) DIP_PORT_SEL_MASK; /* DIP_Port_Select, 0x1 = PortB */ + if (!i) { + DRM_DEBUG_DRIVER(Audio directed to unknown port\n); + /* operate blindly on all ports */ + eldv
Re: [Intel-gfx] [PATCH v7 3/4] drm/i915: Haswell HDMI audio initialization
Hi Daniel/Imre, This revised version changelog: - add Wait for 1 vertical blank after enable audio output port - configure pipe related transcoder instead of operate all transcoders blindly Thanks --xingchao -Original Message- From: Wang, Xingchao Sent: Wednesday, August 15, 2012 11:11 AM To: dan...@ffwll.ch; imre.d...@gmail.com Cc: intel-gfx@lists.freedesktop.org; przan...@gmail.com; Wang, Xingchao Subject: [PATCH v7 3/4] drm/i915: Haswell HDMI audio initialization Added new haswell_write_eld() to initialize Haswell HDMI audio registers to generate an unsolicited response to the audio controller driver to indicate that the controller sequence should start. Signed-off-by: Wang Xingchao xingchao.w...@intel.com --- drivers/gpu/drm/i915/i915_reg.h |1 + drivers/gpu/drm/i915/intel_display.c | 98 +- 2 files changed, 98 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 55aa10e..08f8b65 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -4294,6 +4294,7 @@ #define AUD_DIG_CNVT(pipe) _PIPE(pipe, \ HSW_AUD_DIG_CNVT_1, \ HSW_AUD_DIG_CNVT_2) +#define DIP_PORT_SEL_MASK 0x3 #define HSW_AUD_EDID_DATA_A0x65050 #define HSW_AUD_EDID_DATA_B0x65150 diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 70d30fc..aad0a1b 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -5067,6 +5067,102 @@ static void g4x_write_eld(struct drm_connector *connector, I915_WRITE(G4X_AUD_CNTL_ST, i); } +static void haswell_write_eld(struct drm_connector *connector, + struct drm_crtc *crtc) +{ + struct drm_i915_private *dev_priv = connector-dev-dev_private; + uint8_t *eld = connector-eld; + struct drm_device *dev = crtc-dev; + uint32_t eldv; + uint32_t i; + int len; + int pipe = to_intel_crtc(crtc)-pipe; + int tmp; + + int hdmiw_hdmiedid = HSW_AUD_EDID_DATA(pipe); + int aud_cntl_st = HSW_AUD_DIP_ELD_CTRL(pipe); + int aud_config = HSW_AUD_CFG(pipe); + int aud_cntrl_st2 = HSW_AUD_PIN_ELD_CP_VLD; + + + DRM_DEBUG_DRIVER(HDMI: Haswell Audio initialize\n); + + /* Audio output enable */ + DRM_DEBUG_DRIVER(HDMI audio: enable codec\n); + tmp = I915_READ(aud_cntrl_st2); + tmp |= (AUDIO_OUTPUT_ENABLE_A | AUDIO_OUTPUT_ENABLE_B | AUDIO_OUTPUT_ENABLE_C); + I915_WRITE(aud_cntrl_st2, tmp); + + /* Wait for 1 vertical blank */ + intel_wait_for_vblank(dev, pipe); + + /* Set ELD valid state */ + tmp = I915_READ(aud_cntrl_st2); + DRM_DEBUG_DRIVER(HDMI audio: pin eld vld status=0x%8x\n, tmp); + tmp |= (AUDIO_ELD_VALID_A (pipe * 4)); + I915_WRITE(aud_cntrl_st2, tmp); + tmp = I915_READ(aud_cntrl_st2); + DRM_DEBUG_DRIVER(HDMI audio: eld vld status=0x%8x\n, tmp); + + /* Enable HDMI mode */ + tmp = I915_READ(aud_config); + DRM_DEBUG_DRIVER(HDMI audio: audio conf: 0x%8x\n, tmp); + /* clear N_programing_enable and N_value_index */ + tmp = ~(AUD_CONFIG_N_VALUE_INDEX | AUD_CONFIG_N_PROG_ENABLE); + I915_WRITE(aud_config, tmp); + + DRM_DEBUG_DRIVER(ELD on pipe %c\n, pipe_name(pipe)); + + i = I915_READ(aud_cntl_st); + i = (i 29) DIP_PORT_SEL_MASK; /* DIP_Port_Select, 0x1 = PortB */ + if (!i) { + DRM_DEBUG_DRIVER(Audio directed to unknown port\n); + /* operate blindly on all ports */ + eldv = AUDIO_ELD_VALID_A; + eldv |= AUDIO_ELD_VALID_B; + eldv |= AUDIO_ELD_VALID_C; + } else { + DRM_DEBUG_DRIVER(ELD on port %c\n, 'A' + i); + eldv = AUDIO_ELD_VALID_A ((i - 1) * 4); + } + + if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT)) { + DRM_DEBUG_DRIVER(ELD: DisplayPort detected\n); + eld[5] |= (1 2); /* Conn_Type, 0x1 = DisplayPort */ + I915_WRITE(aud_config, AUD_CONFIG_N_VALUE_INDEX); /* 0x1 = DP */ + } else + I915_WRITE(aud_config, 0); + + if (intel_eld_uptodate(connector, +aud_cntrl_st2, eldv, +aud_cntl_st, IBX_ELD_ADDRESS, +hdmiw_hdmiedid)) + return; + + i = I915_READ(aud_cntrl_st2); + i = ~eldv; + I915_WRITE(aud_cntrl_st2, i); + + if (!eld[0]) + return; + + i = I915_READ(aud_cntl_st); + i = ~IBX_ELD_ADDRESS; + I915_WRITE(aud_cntl_st, i); + i = (i 29) DIP_PORT_SEL_MASK; /* DIP_Port_Select, 0x1 = PortB */ + DRM_DEBUG_DRIVER(port num:%d\n, i); + + len =
Re: [Intel-gfx] [PATCH v6 4/4] drm/i915: ironlake_write_eld code cleanup
-Original Message- From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter Sent: Tuesday, August 14, 2012 7:49 PM To: Wang, Xingchao Cc: dan...@ffwll.ch; przan...@gmail.com; intel-gfx@lists.freedesktop.org Subject: Re: [PATCH v6 4/4] drm/i915: ironlake_write_eld code cleanup On Thu, Aug 09, 2012 at 04:52:18PM +0800, Wang Xingchao wrote: Use _PIPE macro to get correct register definition for IBX/CPT, discard old variable i way. Signed-off-by: Wang Xingchao xingchao.w...@intel.com Ok, I've slurped in patches 1,24 for -next (with a bit of frobbing since one of the #defines used in patch 4 is introduced in patch 3). I'll pick up patch 3 as soon as the revised version is postedreviewed. Thanks, Daniel. For patch 3 at version 7 , I had send it out separately. Thanks --xingchao ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx