Re: [Intel-gfx] [PATCH] drm/i915/bdw: Render moot context reset and switch with Execlists
On Mon, Aug 25, 2014 at 10:39:39PM +0200, Daniel Vetter wrote: On Wed, Aug 20, 2014 at 04:36:05PM +0100, Chris Wilson wrote: On Wed, Aug 20, 2014 at 04:29:24PM +0100, Thomas Daniel wrote: These two functions make no sense in an Logical Ring Context Execlists world. v2: We got rid of lrc_enabled and centralized everything in the sanitized i915.enable_execlists instead. Signed-off-by: Oscar Mateo oscar.ma...@intel.com v3: Rebased. Corrected a typo in comment for i915_switch_context and added a comment that it should not be called in execlist mode. Added WARN_ON if i915_switch_context is called in execlist mode. Moved check for execlist mode out of i915_switch_context and into callers. Added comment in context_reset explaining why nothing is done in execlist mode. No, this is not the way. The requirement is to reduce the number of special cases not increase them. These should be evaluated to be no-ops when execlists is used. I think it's ok-ish for now. Maybe we need to reconsider when we wire up lrc reclaim - which is the real user of the switch_context in gpu_idle. The problem I have though is that I can't parse the subject of the patch, someone please translate that to simplified English for me. I can do the replacement while applying. No, it is not. execlists is badly designed and this is a further symptom of that. -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 v3] drm/i915: Rework GPU reset sequence to match driver load thaw
On Mon, Aug 25, 2014 at 10:18:09PM +0200, Daniel Vetter wrote: On Wed, Aug 20, 2014 at 04:56:41PM +0100, Chris Wilson wrote: On Wed, Aug 20, 2014 at 03:21:55PM +, Mcaulay, Alistair wrote: It is not the same. This is a special case when re-initialising the hw. This flag is to allow gem_init_hw() to complete successfully during reset. At any other point during reset, -EAGAIN should be returned. Indeed. You've missed the point. Look closer at the reset counter and reset ordering. We could try to mark the gpu as reset again before starting the reinit to avoid this kludge. But that has the problem that if the ring init fails we have a bit a mess in marking the gpu terminally wedged. So I think overall not prettier than what we have here ... And if I'm mistaken I guess I can put on my idiot hat and merge the fixup ;-) See other patches during the week. Marking the reset as complete after performing the reset and before resuming the hardware is not ugly. If the reset fails, you still have the wedge | in-progress. If the reset succeeds but resume fails, you then have wedge | !in-progress. Seriously read the request patch to see how it straightens out ring access, full stop. -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: Disallow any pinning when kms is enabled
On Mon, Aug 25, 2014 at 09:59:33PM +0200, Daniel Vetter wrote: So apparently userspace managed to get itself into a corner with tricky tricks and the kernel can't work around because the batch buffer ends up being pinned. For simplicity let's just take the toys away. No. Same nak as for the last pin_ioctl change. But I don't think you are listening. -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] Merge two subtests for pm_rc6_residency IGT case
Combine two subtests(rc6_residency_check and rc6_residency_counter) into one subtest(residency_accuracy) diff --git a/tests/pm_rc6_residency.c b/tests/pm_rc6_residency.c index 8cf7be3..3f3363a 100644 --- a/tests/pm_rc6_residency.c +++ b/tests/pm_rc6_residency.c @@ -87,13 +87,22 @@ static void read_rc6_residency( int value[], const char *name_of_rc6_residency[] free(path); } -static void rc6_residency_counter(int value[],const char *name_of_rc6_residency[]) +static void residency_accuracy(int value[],const char *name_of_rc6_residency[]) { int flag; unsigned int flag_counter,flag_support; double counter_result = 0; + unsigned int diff; flag_counter = 0; flag_support = 0; + diff = (value[3] - value[0]) + + (value[4] - value[1]) + + (value[5] - value[2]); + + igt_assert_f(diff = (SLEEP_DURATION + RC6_FUDGE),Diff was too high. That is unpossible\n); + igt_assert_f(diff = (SLEEP_DURATION - RC6_FUDGE),GPU was not in RC6 long enough. Check that + the GPU is as idle as possible(ie. no X, + running and running no other tests)\n); for(flag = NUMBER_OF_RC6_RESIDENCY-1; flag = 0; flag --) { @@ -123,26 +132,11 @@ static void rc6_residency_counter(int value[],const char *name_of_rc6_residency[ } igt_info(The residency counter: %f \n, counter_result); - igt_skip_on_f(flag_support == 0 , This machine didn't entry any RC6 state.\n); igt_assert_f((flag_counter != 0) (counter_result =1) , Sysfs RC6 residency counter is inaccurate.\n); - igt_info(This machine entry %s state.\n, name_of_rc6_residency[(flag_counter / 2) - 1]); } -static void rc6_residency_check(int value[]) -{ - unsigned int diff; - diff = (value[3] - value[0]) + - (value[4] - value[1]) + - (value[5] - value[2]); - - igt_assert_f(diff = (SLEEP_DURATION + RC6_FUDGE),Diff was too high. That is unpossible\n); - igt_assert_f(diff = (SLEEP_DURATION - RC6_FUDGE),GPU was not in RC6 long enough. Check that - the GPU is as idle as possible(ie. no X, - running and running no other tests)\n); -} - igt_main { int fd; @@ -159,9 +153,6 @@ igt_main read_rc6_residency(value, name_of_rc6_residency); } - igt_subtest(rc6-residency-check) - rc6_residency_check(value); - - igt_subtest(rc6-residency-counter) - rc6_residency_counter(value, name_of_rc6_residency); + igt_subtest(residency-accuracy) + residency_accuracy(value, name_of_rc6_residency); } -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: don't check for i830 in vlv specific code
On Fri, Aug 22, 2014 at 03:44:30PM +0300, Ville Syrjälä wrote: On Fri, Aug 22, 2014 at 03:06:35PM +0300, Jani Nikula wrote: Signed-off-by: Jani Nikula jani.nik...@intel.com --- drivers/gpu/drm/i915/intel_display.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 51b4cd29f932..83eabd758ed9 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -1546,7 +1546,7 @@ static void vlv_enable_pll(struct intel_crtc *crtc) 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)) + if (IS_MOBILE(dev_priv-dev)) assert_panel_unlocked(dev_priv, crtc-pipe); My gut feeling is that the IS_MOBILE check could also be dropped. Not quite sure though since VLV_D is not mentioned anywhere in the docs AFAICS. I think in the old gens we've pretty much just used IS_MOBILE as HAS_LVDS. vlv/chv having edp panels instead makes that a bit more complicated, but generally I think a new IS_MOBILE check is bogus. Anyway dropping the 830 check definitely makes sense so: Reviewed-by: Ville Syrjälä ville.syrj...@linux.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
Re: [Intel-gfx] Ping: Status of the i830 code?
On Mon, Aug 25, 2014 at 03:13:46PM +0200, Thomas Richter wrote: Hi Ville, hi Daniel, this is again a ping on the status of the i830 code. I reviewed the code I received, at least the code I could review (I do not know enough about the internal wirings of the intel_display source, only on the ns2501 dvo source), but I haven't heart anything back. Is there anything I can do to help to move Ville's modifications for i380 into the kernel? Me getting back to working internet essentially so that I can give them a spin on my own i830m. -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] Fwd: i915 (possibly): Suspend regression Macbook Pro 15 (late 2013)
On Mon, Aug 25, 2014 at 08:33:02PM -0700, Eric Rannaud wrote: [Cross-post from LKML; apologies if you've all already seen this] Hi, Between 3.13.5 and 3.15.4, suspend became partially broken on Macbook Pro 15 (late 2013): a few minutes after wakeup from the *second* suspend following a fresh boot, the screen will randomly turn itself off at shorter and shorter time intervals, and turn itself back on after a few seconds and/or user interaction. Eventually, after 4 or 5 such cycles, the screen refuses to turn back on altogether, and the system must be rebooted (closing the lid again has no effect). When the screen turns back on, there is usually some transient corruption (full screen is displayed shifted to the right by 1/10th of its full width, cutting off the right side, which wraps back to the left). This corruption disappears after additional user interaction that forces a re-render. The issue is still present in Linus' tree (v3.17-rc1-22-g480cadc2b7e0). Before I get started trying to bisect this slow-to-reproduce problem, has anyone any idea of possible causes? Sounds new. Anything interesting going on in dmesg while the on/off flickering goes on if you crank up output with drm.debug=0xe? Otherwise I guess bisect it is. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/ilk: special case enabling of PCU_EVENT interrupt
On Mon, Aug 25, 2014 at 04:24:55PM -0700, Jesse Barnes wrote: This happens in irq_postinstall before we've set the pm._irqs_disabled flag, but shouldn't warn. So add a nowarn variant to allow this to happen w/o a backtrace and keep the rest of the IRQ tracking code happy. Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org Shouldn't we instead just move the pm._irqs_disabled = false in i915_dma.c right above the drm_irq_install call? In intel_runtime_pm_restore_interrupts we also set it to false before we call the various hooks. Also the commit message is a bit thin on the usual details like which commits introduced this regression, so that maintainers know where to apply this to. -Daniel --- drivers/gpu/drm/i915/i915_irq.c | 18 -- 1 files changed, 12 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index d5445e7..ec1d9fe 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -132,6 +132,16 @@ static const u32 hpd_status_i915[] = { /* i915 and valleyview are the same */ /* For display hotplug interrupt */ static void +ironlake_enable_display_irq_nowarn(struct drm_i915_private *dev_priv, u32 mask) +{ + if ((dev_priv-irq_mask mask) != 0) { + dev_priv-irq_mask = ~mask; + I915_WRITE(DEIMR, dev_priv-irq_mask); + POSTING_READ(DEIMR); + } +} + +static void ironlake_enable_display_irq(struct drm_i915_private *dev_priv, u32 mask) { assert_spin_locked(dev_priv-irq_lock); @@ -139,11 +149,7 @@ ironlake_enable_display_irq(struct drm_i915_private *dev_priv, u32 mask) if (WARN_ON(!intel_irqs_enabled(dev_priv))) return; - if ((dev_priv-irq_mask mask) != 0) { - dev_priv-irq_mask = ~mask; - I915_WRITE(DEIMR, dev_priv-irq_mask); - POSTING_READ(DEIMR); - } + ironlake_enable_display_irq_nowarn(dev_priv, mask); } static void @@ -3665,7 +3671,7 @@ static int ironlake_irq_postinstall(struct drm_device *dev) * setup is guaranteed to run in single-threaded context. But we * need it to make the assert_spin_locked happy. */ spin_lock_irqsave(dev_priv-irq_lock, irqflags); - ironlake_enable_display_irq(dev_priv, DE_PCU_EVENT); + ironlake_enable_display_irq_nowarn(dev_priv, DE_PCU_EVENT); spin_unlock_irqrestore(dev_priv-irq_lock, irqflags); } -- 1.7.5.4 ___ 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: Use dev_priv as first argument of for_each_pipe()
On Mon, Aug 18, 2014 at 02:13:06PM +0100, Chris Wilson wrote: On Mon, Aug 18, 2014 at 02:07:40PM +0100, Damien Lespiau wrote: On Mon, Aug 18, 2014 at 01:58:06PM +0100, Chris Wilson wrote: On Mon, Aug 18, 2014 at 01:49:10PM +0100, Damien Lespiau wrote: Chris has decided that enough is enough. It's time to fixup dev Vs dev_priv. This is a modest contribution to the crusade. v2: Still use INTEL_INFO(), for the (mythical!) case we want to hardcode the info struct with defines (Chris) Rename the macro argument from 'dev' to 'dev_priv' (Jani) v3: Use names unlikely to be used as macro arguments (Chris) I can be annoying! These macros typically take the iter as the first argument... How typical is your typical? list_for_each and everything derived from them that paid attention... #define for_each_pipe(__dev_priv, __p) #define for_each_crtc(dev, crtc) #define for_each_intel_crtc(dev, intel_crtc) #define for_each_intel_encoder(dev, intel_encoder) #define for_each_encoder_on_crtc(dev, __crtc, intel_encoder) #define for_each_connector_on_encoder(dev, __encoder, intel_connector) Sounds like a lot of churn for no good reason this time. Or that I forgot to tell you about this detail last time. I think a lot of those are on me ;-) Patch merged to dinq, thanks. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: fix suspend/resume for GENs w/o runtime PM support
On Mon, Aug 18, 2014 at 01:20:06PM +0300, Imre Deak wrote: Before sharing common parts between the system and runtime s/r handlers we WARNed if the runtime s/r handlers were called on GENs that didn't support RPM. But this WARN is not correct if the same handler is called from the system s/r path, since that can happen on any platform. This also broke system s/r on old platforms. The issue was introduced in commit 016970beb05da6285c2f3ed2bee1c676cb75972e Author: Sagar Kamble sagar.a.kam...@intel.com Date: Wed Aug 13 23:07:06 2014 +0530 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82751 Signed-off-by: Imre Deak imre.d...@intel.com Adding boolean arguments to control warnings always feels a bit too much like just shutting up the warnings. Can't we instead wrap the relevant calls into HAS_RUNTIME_PM checks? Imo that would also lead to clearer code by making the intention clear - with this you essentially have to git blame to figure out why we sometimes disable the warning. -Daniel --- drivers/gpu/drm/i915/i915_drv.c | 18 ++ 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 117f5c1..f646f34 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -499,7 +499,8 @@ bool i915_semaphore_is_enabled(struct drm_device *dev) } -static int intel_suspend_complete(struct drm_i915_private *dev_priv); +static int intel_suspend_complete(struct drm_i915_private *dev_priv, + bool rpm_suspend); static int intel_resume_prepare(struct drm_i915_private *dev_priv, bool rpm_resume); @@ -909,7 +910,7 @@ static int i915_pm_suspend_late(struct device *dev) if (drm_dev-switch_power_state == DRM_SWITCH_POWER_OFF) return 0; - ret = intel_suspend_complete(dev_priv); + ret = intel_suspend_complete(dev_priv, false); if (ret) DRM_ERROR(Suspend complete failed: %d\n, ret); @@ -1410,7 +1411,7 @@ static int intel_runtime_suspend(struct device *device) cancel_work_sync(dev_priv-rps.work); intel_runtime_pm_disable_interrupts(dev); - ret = intel_suspend_complete(dev_priv); + ret = intel_suspend_complete(dev_priv, true); if (ret) { DRM_ERROR(Runtime suspend failed, disabling it (%d)\n, ret); intel_runtime_pm_restore_interrupts(dev); @@ -1471,10 +1472,11 @@ static int intel_runtime_resume(struct device *device) * This function implements common functionality of runtime and system * suspend sequence. */ -static int intel_suspend_complete(struct drm_i915_private *dev_priv) +static int intel_suspend_complete(struct drm_i915_private *dev_priv, + bool rpm_suspend) { struct drm_device *dev = dev_priv-dev; - int ret; + int ret = 0; if (IS_GEN6(dev)) { ret = 0; @@ -1482,7 +1484,7 @@ static int intel_suspend_complete(struct drm_i915_private *dev_priv) ret = hsw_suspend_complete(dev_priv); } else if (IS_VALLEYVIEW(dev)) { ret = vlv_suspend_complete(dev_priv); - } else { + } else if (rpm_suspend) { ret = -ENODEV; WARN_ON(1); } @@ -1499,7 +1501,7 @@ static int intel_resume_prepare(struct drm_i915_private *dev_priv, bool rpm_resume) { struct drm_device *dev = dev_priv-dev; - int ret; + int ret = 0; if (IS_GEN6(dev)) { ret = snb_resume_prepare(dev_priv, rpm_resume); @@ -1507,7 +1509,7 @@ static int intel_resume_prepare(struct drm_i915_private *dev_priv, ret = hsw_resume_prepare(dev_priv, rpm_resume); } else if (IS_VALLEYVIEW(dev)) { ret = vlv_resume_prepare(dev_priv, rpm_resume); - } else { + } else if (rpm_resume) { WARN_ON(1); ret = -ENODEV; } -- 1.8.4 ___ 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: fix suspend/resume for GENs w/o runtime PM support
On Tue, Aug 26, 2014 at 09:44:42AM +0200, Daniel Vetter wrote: On Mon, Aug 18, 2014 at 01:20:06PM +0300, Imre Deak wrote: Before sharing common parts between the system and runtime s/r handlers we WARNed if the runtime s/r handlers were called on GENs that didn't support RPM. But this WARN is not correct if the same handler is called from the system s/r path, since that can happen on any platform. This also broke system s/r on old platforms. The issue was introduced in commit 016970beb05da6285c2f3ed2bee1c676cb75972e Author: Sagar Kamble sagar.a.kam...@intel.com Date: Wed Aug 13 23:07:06 2014 +0530 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82751 Signed-off-by: Imre Deak imre.d...@intel.com Adding boolean arguments to control warnings always feels a bit too much like just shutting up the warnings. Can't we instead wrap the relevant calls into HAS_RUNTIME_PM checks? Imo that would also lead to clearer code by making the intention clear - with this you essentially have to git blame to figure out why we sometimes disable the warning. Also the patch subject is a bit misleading - we only shut up a wrong warning, it's not a code fix. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Print the pipe on which the vblank wait times out
On Wed, Aug 20, 2014 at 02:45:37PM +0100, Thomas Wood wrote: On 18 August 2014 13:51, Damien Lespiau damien.lesp...@intel.com wrote: Pimp up the debug message that tells us we've been waiting for a vblank that never arrived. Printing the pipe could lead a doh! moment where we've been waiting for a vblank on a pipe that was off for instance. Signed-off-by: Damien Lespiau damien.lesp...@intel.com Reviewed-by: Thomas Wood thomas.w...@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
Re: [Intel-gfx] [PATCH] drm/i915: FBC flush nuke for BDW
On Tue, Aug 26, 2014 at 2:39 AM, Rodrigo Vivi rodrigo.v...@gmail.com wrote: So I prefer to continue using the HW/ring version we have already working for HSW and merge this v3 to get FBC working at BDW. Well for that we first need to fix up the psr testcase. I really want that. And then I also want fbc enabled by default, which means you need to rebase/review Ville's patch series to make that work. Also I really think the fb frontbuffer tracking can be made to work for fbc - if you do it right you should actually end up with fewer frontbuffer flushes than what we currently do by submitting them through rings. -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 4/4] drm/i915/bdw: Map unused PDPs to a scratch page
On Mon, Aug 18, 2014 at 10:35:30AM -0700, Rodrigo Vivi wrote: From: Bob Beckett robert.beck...@intel.com Create a scratch page for the two unused PDPs and set all the PTEs for them to point to it. This patch addresses a page fault, and subsequent hang in pipe control flush. In these cases, the Main Graphic Arbiter Error register [0x40A0] showed a TLB Page Fault error, and a high memory address (higher than the size of our PPGTT) was reported in the Fault TLB RD Data0 register (0x4B10). PDP2 PDP3 were not set because, in theory, they aren't required for our PPGTT size, but they should be mapped to a scratch page anyway. v2: Rebase on latest nightly. Signed-off-by: Michel Thierry michel.thie...@intel.com (v1) Signed-off-by: Dave Gordon david.s.gor...@intel.com (v2) Signed-off-by: Oscar Mateo oscar.ma...@intel.com Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com No idea about this one, especially since there's tons of other bdw ppgtt patches in-flight. I've merged all the others though. Aside: We need to figure out how to make people review their -collector assignments actually, it seems to totally not work :( -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/bdw: Populate lrc with aliasing ppgtt if required
On Tue, Aug 19, 2014 at 10:44:55AM +0100, Damien Lespiau wrote: On Tue, Aug 19, 2014 at 10:13:36AM +0100, Thomas Daniel wrote: A previous commit broke aliasing PPGTT for lrc, resulting in a kernel oops on boot. Add a check so that is full PPGTT is not in use the context is populated with the aliasing PPGTT. Usually, we add a bit of history to the patch (even for trivial things) something like: v2: blah blah blah blah But doesn't really matter much this time. Reviewed-by: Damien Lespiau damien.lesp...@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
Re: [Intel-gfx] [PATCH] drm/edid: Reduce horizontal timings for pixel replicated modes
On Tue, Aug 19, 2014 at 02:21:21PM -0700, clinton.a.tay...@intel.com wrote: From: Clint Taylor clinton.a.tay...@intel.com Pixel replicated modes should be 720 horizontal pixel and pixel replicated by the HW across the HDMI cable at 2X pixel clock. Current horizontal resolution of 1440 does not allow pixel duplication to occur and scaling artifacts occur on the TV. HDMI certification 7-26 currently fails for all pixel replicated modes. This change fizes the HDMI certification issues with 480i/576i. V2: Removed interlace flag from VICs 44 and 45. Will be submitted in another patch. Various other formatting fixes. V3: 576i@200 htotal fixed. Check min and max pixel clocks. Signed-off-by: Clint Taylor clinton.a.tay...@intel.com --- drivers/gpu/drm/drm_edid.c| 96 ++--- drivers/gpu/drm/i915/intel_hdmi.c | 15 -- Still plea to split out the i915 change. Thanks, Daniel 2 files changed, 60 insertions(+), 51 deletions(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index f905c63..dc25999 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -632,27 +632,27 @@ static const struct drm_display_mode edid_cea_modes[] = { DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC | DRM_MODE_FLAG_INTERLACE), .vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9, }, - /* 6 - 1440x480i@60Hz */ - { DRM_MODE(1440x480i, DRM_MODE_TYPE_DRIVER, 27000, 1440, 1478, -1602, 1716, 0, 480, 488, 494, 525, 0, + /* 6 - 720(1440)x480i@60Hz */ + { DRM_MODE(720x480i, DRM_MODE_TYPE_DRIVER, 13500, 720, 739, +801, 858, 0, 480, 488, 494, 525, 0, DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_INTERLACE | DRM_MODE_FLAG_DBLCLK), .vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_4_3, }, - /* 7 - 1440x480i@60Hz */ - { DRM_MODE(1440x480i, DRM_MODE_TYPE_DRIVER, 27000, 1440, 1478, -1602, 1716, 0, 480, 488, 494, 525, 0, + /* 7 - 720(1440)x480i@60Hz */ + { DRM_MODE(720x480i, DRM_MODE_TYPE_DRIVER, 13500, 720, 739, +801, 858, 0, 480, 488, 494, 525, 0, DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_INTERLACE | DRM_MODE_FLAG_DBLCLK), .vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9, }, - /* 8 - 1440x240@60Hz */ - { DRM_MODE(1440x240, DRM_MODE_TYPE_DRIVER, 27000, 1440, 1478, -1602, 1716, 0, 240, 244, 247, 262, 0, + /* 8 - 720(1440)x240@60Hz */ + { DRM_MODE(720x240, DRM_MODE_TYPE_DRIVER, 13500, 720, 739, +801, 858, 0, 240, 244, 247, 262, 0, DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_DBLCLK), .vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_4_3, }, - /* 9 - 1440x240@60Hz */ - { DRM_MODE(1440x240, DRM_MODE_TYPE_DRIVER, 27000, 1440, 1478, -1602, 1716, 0, 240, 244, 247, 262, 0, + /* 9 - 720(1440)x240@60Hz */ + { DRM_MODE(720x240, DRM_MODE_TYPE_DRIVER, 13500, 720, 739, +801, 858, 0, 240, 244, 247, 262, 0, DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_DBLCLK), .vrefresh = 60, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9, }, @@ -714,27 +714,27 @@ static const struct drm_display_mode edid_cea_modes[] = { DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC | DRM_MODE_FLAG_INTERLACE), .vrefresh = 50, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9, }, - /* 21 - 1440x576i@50Hz */ - { DRM_MODE(1440x576i, DRM_MODE_TYPE_DRIVER, 27000, 1440, 1464, -1590, 1728, 0, 576, 580, 586, 625, 0, + /* 21 - 720(1440)x576i@50Hz */ + { DRM_MODE(720x576i, DRM_MODE_TYPE_DRIVER, 13500, 720, 732, +795, 864, 0, 576, 580, 586, 625, 0, DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_INTERLACE | DRM_MODE_FLAG_DBLCLK), .vrefresh = 50, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_4_3, }, - /* 22 - 1440x576i@50Hz */ - { DRM_MODE(1440x576i, DRM_MODE_TYPE_DRIVER, 27000, 1440, 1464, -1590, 1728, 0, 576, 580, 586, 625, 0, + /* 22 - 720(1440)x576i@50Hz */ + { DRM_MODE(720x576i, DRM_MODE_TYPE_DRIVER, 13500, 720, 732, +795, 864, 0, 576, 580, 586, 625, 0, DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_INTERLACE | DRM_MODE_FLAG_DBLCLK), .vrefresh = 50, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9, }, - /* 23 - 1440x288@50Hz */ - { DRM_MODE(1440x288, DRM_MODE_TYPE_DRIVER, 27000, 1440, 1464, -1590, 1728, 0, 288, 290, 293, 312, 0,
Re: [Intel-gfx] [PATCH] drm/i915: fix suspend/resume for GENs w/o runtime PM support
On Tue, 2014-08-26 at 09:45 +0200, Daniel Vetter wrote: On Tue, Aug 26, 2014 at 09:44:42AM +0200, Daniel Vetter wrote: On Mon, Aug 18, 2014 at 01:20:06PM +0300, Imre Deak wrote: Before sharing common parts between the system and runtime s/r handlers we WARNed if the runtime s/r handlers were called on GENs that didn't support RPM. But this WARN is not correct if the same handler is called from the system s/r path, since that can happen on any platform. This also broke system s/r on old platforms. The issue was introduced in commit 016970beb05da6285c2f3ed2bee1c676cb75972e Author: Sagar Kamble sagar.a.kam...@intel.com Date: Wed Aug 13 23:07:06 2014 +0530 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82751 Signed-off-by: Imre Deak imre.d...@intel.com Adding boolean arguments to control warnings always feels a bit too much like just shutting up the warnings. Can't we instead wrap the relevant calls into HAS_RUNTIME_PM checks? I could instead remove the WARN from intel_suspend_complete/resume, and do an early return from intel_runtime_suspend/resume for !HAS_RUNTIME_PM(). Atm we only WARN there. Imo that would also lead to clearer code by making the intention clear - with this you essentially have to git blame to figure out why we sometimes disable the warning. Also the patch subject is a bit misleading - we only shut up a wrong warning, it's not a code fix. We return -ENODEV for old GENs which breaks system suspend for them. --Imre signature.asc Description: This is a digitally signed message part ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 07/14] drm/i915: Warn about want_panel_vdd in edp_panel_vdd_off_sync()
On Tue, Aug 19, 2014 at 04:37:03PM +0300, Jani Nikula wrote: On Tue, 19 Aug 2014, Ville Syrjälä ville.syrj...@linux.intel.com wrote: On Tue, Aug 19, 2014 at 10:36:52AM +0300, Jani Nikula wrote: On Mon, 18 Aug 2014, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com If we force vdd off warn if someone is still using it. With this change the delayed vdd off work needs to check want_panel_vdd itself to make sure it doesn't try to turn vdd off when someone is using it. I think this calls for a prep cleanup patch to check and fix the uses of edp_panel_vdd_off(intel_dp, true) vs. edp_panel_vdd_off_sync(intel_dp). In particular, why are there direct calls to the latter all over the place? Seems wrong. edp_panel_vdd_off() should always be paired with a edp_panel_vdd_on(). If we were to call edp_panel_vdd_off() without the correct pairing we would get a warning due to want_panel_vdd==false, whereas edp_panel_vdd_off_sync() will now warn when want_panel_vdd==true. The direct calls to edp_panel_vdd_off_sync() are in places where we should not have want_panel_vdd==true (eg. system suspend) but we just want to force vdd off even if the delayed off work has alrady been scheduled. Okay, care to add some of that as brief documentation comments for the functions in question, as follow-up? IMO detailed kernel-docs here won't be read by anyone and will just get stale. Hm, imo a wrappe for vdd_off_sync or would be clearer than piles of comments. Or maybe just vdd_sync, akin to all the work/time _sync functions. Our system suspend/resume code is splattered with such functions, so the code pattern should be clear with just that. Perhaps as a follow-up patch on top of all of this? -Daniel BR, Jani. BR, Jani. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_dp.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index e6b4d4d..0fb510c 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1241,7 +1241,9 @@ static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp) WARN_ON(!drm_modeset_is_locked(dev-mode_config.connection_mutex)); -if (intel_dp-want_panel_vdd || !edp_have_panel_vdd(intel_dp)) +WARN_ON(intel_dp-want_panel_vdd); + +if (!edp_have_panel_vdd(intel_dp)) return; DRM_DEBUG_KMS(Turning eDP VDD off\n); @@ -1273,7 +1275,8 @@ static void edp_panel_vdd_work(struct work_struct *__work) struct drm_device *dev = intel_dp_to_dev(intel_dp); drm_modeset_lock(dev-mode_config.connection_mutex, NULL); -edp_panel_vdd_off_sync(intel_dp); +if (!intel_dp-want_panel_vdd) +edp_panel_vdd_off_sync(intel_dp); drm_modeset_unlock(dev-mode_config.connection_mutex); } -- 1.8.5.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center -- Ville Syrjälä Intel OTC -- Jani Nikula, Intel Open Source Technology Center ___ 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 1/2] drm/i915/bdw: Apply workarounds in render ring init function
On 25/08/2014 13:18, Ville Syrjälä wrote: On Fri, Aug 22, 2014 at 08:39:11PM +0100, Arun Siluvery wrote: For BDW workarounds are currently initialized in init_clock_gating() but they are lost during reset, suspend/resume etc; this patch moves the WAs that are part of register state context to render ring init fn otherwise default context ends up with incorrect values as they don't get initialized until init_clock_gating fn. v2: Add workarounds to golden render state This method has its own issues, first of all this is different for each gen and it is generated using a tool so adding new workaround and mainitaining them across gens is not a straightforward process. v3: Use LRIs to emit these workarounds (Ville) Instead of modifying the golden render state the same LRIs are emitted from within the driver. For: VIZ-4092 Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com --- drivers/gpu/drm/i915/i915_gem_context.c | 6 +++ drivers/gpu/drm/i915/intel_pm.c | 48 -- drivers/gpu/drm/i915/intel_ringbuffer.c | 70 + drivers/gpu/drm/i915/intel_ringbuffer.h | 7 4 files changed, 83 insertions(+), 48 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 9683e62..2debce4 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -631,20 +631,26 @@ static int do_switch(struct intel_engine_cs *ring, } uninitialized = !to-legacy_hw_ctx.initialized from == NULL; to-legacy_hw_ctx.initialized = true; done: i915_gem_context_reference(to); ring-last_context = to; if (uninitialized) { + if (IS_BROADWELL(ring-dev)) { + ret = bdw_init_workarounds(ring); + if (ret) + DRM_ERROR(init workarounds: %d\n, ret); + } + ret = i915_gem_render_state_init(ring); if (ret) DRM_ERROR(init render state: %d\n, ret); } return 0; unpin_out: if (ring-id == RCS) i915_gem_object_ggtt_unpin(to-legacy_hw_ctx.rcs_state); diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index c8f744c..668acd9 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5507,101 +5507,53 @@ static void gen8_init_clock_gating(struct drm_device *dev) struct drm_i915_private *dev_priv = dev-dev_private; enum pipe pipe; I915_WRITE(WM3_LP_ILK, 0); I915_WRITE(WM2_LP_ILK, 0); I915_WRITE(WM1_LP_ILK, 0); /* FIXME(BDW): Check all the w/a, some might only apply to * pre-production hw. */ - /* WaDisablePartialInstShootdown:bdw */ - I915_WRITE(GEN8_ROW_CHICKEN, - _MASKED_BIT_ENABLE(PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE)); - - /* WaDisableThreadStallDopClockGating:bdw */ - /* FIXME: Unclear whether we really need this on production bdw. */ - I915_WRITE(GEN8_ROW_CHICKEN, - _MASKED_BIT_ENABLE(STALL_DOP_GATING_DISABLE)); - /* -* This GEN8_CENTROID_PIXEL_OPT_DIS W/A is only needed for -* pre-production hardware -*/ - I915_WRITE(HALF_SLICE_CHICKEN3, - _MASKED_BIT_ENABLE(GEN8_CENTROID_PIXEL_OPT_DIS)); - I915_WRITE(HALF_SLICE_CHICKEN3, - _MASKED_BIT_ENABLE(GEN8_SAMPLER_POWER_BYPASS_DIS)); I915_WRITE(GAMTARBMODE, _MASKED_BIT_ENABLE(ARB_MODE_BWGTLB_DISABLE)); I915_WRITE(_3D_CHICKEN3, _MASKED_BIT_ENABLE(_3D_CHICKEN_SDE_LIMIT_FIFO_POLY_DEPTH(2))); - I915_WRITE(COMMON_SLICE_CHICKEN2, - _MASKED_BIT_ENABLE(GEN8_CSC2_SBE_VUE_CACHE_CONSERVATIVE)); - - I915_WRITE(GEN7_HALF_SLICE_CHICKEN1, - _MASKED_BIT_ENABLE(GEN7_SINGLE_SUBSCAN_DISPATCH_ENABLE)); - - /* WaDisableDopClockGating:bdw May not be needed for production */ - I915_WRITE(GEN7_ROW_CHICKEN2, - _MASKED_BIT_ENABLE(DOP_CLOCK_GATING_DISABLE)); /* WaSwitchSolVfFArbitrationPriority:bdw */ I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | HSW_ECOCHK_ARB_PRIO_SOL); /* WaPsrDPAMaskVBlankInSRD:bdw */ I915_WRITE(CHICKEN_PAR1_1, I915_READ(CHICKEN_PAR1_1) | DPA_MASK_VBLANK_SRD); /* WaPsrDPRSUnmaskVBlankInSRD:bdw */ for_each_pipe(pipe) { I915_WRITE(CHICKEN_PIPESL_1(pipe), I915_READ(CHICKEN_PIPESL_1(pipe)) | BDW_DPRS_MASK_VBLANK_SRD); } - /* Use Force Non-Coherent whenever executing a 3D context. This is a -* workaround for for a possible hang in the unlikely event a TLB -* invalidation occurs during a PSD flush. -*/ - I915_WRITE(HDC_CHICKEN0, -
[Intel-gfx] [PATCH 2/2] drm/i915/bdw: Export workaround data to debugfs
The workarounds that are applied are exported to a debugfs file; this is used to verify their state after the test case (reset or suspend/resume etc). This patch is only required to support i-g-t. Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com --- drivers/gpu/drm/i915/i915_debugfs.c | 40 + drivers/gpu/drm/i915/i915_drv.h | 14 drivers/gpu/drm/i915/intel_ringbuffer.c | 23 +++ 3 files changed, 77 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index d42db6b..f0d63f6 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2451,20 +2451,59 @@ static int i915_shared_dplls_info(struct seq_file *m, void *unused) seq_printf(m, dpll_md: 0x%08x\n, pll-hw_state.dpll_md); seq_printf(m, fp0: 0x%08x\n, pll-hw_state.fp0); seq_printf(m, fp1: 0x%08x\n, pll-hw_state.fp1); seq_printf(m, wrpll: 0x%08x\n, pll-hw_state.wrpll); } drm_modeset_unlock_all(dev); return 0; } +static int intel_wa_registers(struct seq_file *m, void *unused) +{ + int i; + int ret; + struct drm_info_node *node = (struct drm_info_node *) m-private; + struct drm_device *dev = node-minor-dev; + struct drm_i915_private *dev_priv = dev-dev_private; + + if (!IS_BROADWELL(dev)) { + DRM_DEBUG_DRIVER(Workaround table not available !!\n); + return -EINVAL; + } + + ret = mutex_lock_interruptible(dev-struct_mutex); + if (ret) + return ret; + + intel_runtime_pm_get(dev_priv); + + seq_printf(m, Workarounds applied: %d\n, dev_priv-num_wa_regs); + for (i = 0; i dev_priv-num_wa_regs; ++i) { + u32 addr, mask; + + addr = dev_priv-intel_wa_regs[i].addr; + mask = dev_priv-intel_wa_regs[i].mask; + dev_priv-intel_wa_regs[i].value = I915_READ(addr) | mask; + if (dev_priv-intel_wa_regs[i].addr) + seq_printf(m, 0x%X: 0x%08X, mask: 0x%08X\n, + dev_priv-intel_wa_regs[i].addr, + dev_priv-intel_wa_regs[i].value, + dev_priv-intel_wa_regs[i].mask); + } + + intel_runtime_pm_put(dev_priv); + mutex_unlock(dev-struct_mutex); + + return 0; +} + struct pipe_crc_info { const char *name; struct drm_device *dev; enum pipe pipe; }; static int i915_dp_mst_info(struct seq_file *m, void *unused) { struct drm_info_node *node = (struct drm_info_node *) m-private; struct drm_device *dev = node-minor-dev; @@ -3980,20 +4019,21 @@ static const struct drm_info_list i915_debugfs_list[] = { {i915_llc, i915_llc, 0}, {i915_edp_psr_status, i915_edp_psr_status, 0}, {i915_sink_crc_eDP1, i915_sink_crc, 0}, {i915_energy_uJ, i915_energy_uJ, 0}, {i915_pc8_status, i915_pc8_status, 0}, {i915_power_domain_info, i915_power_domain_info, 0}, {i915_display_info, i915_display_info, 0}, {i915_semaphore_status, i915_semaphore_status, 0}, {i915_shared_dplls_info, i915_shared_dplls_info, 0}, {i915_dp_mst_info, i915_dp_mst_info, 0}, + {intel_wa_registers, intel_wa_registers, 0} }; #define I915_DEBUGFS_ENTRIES ARRAY_SIZE(i915_debugfs_list) static const struct i915_debugfs_files { const char *name; const struct file_operations *fops; } i915_debugfs_files[] = { {i915_wedged, i915_wedged_fops}, {i915_max_freq, i915_max_freq_fops}, {i915_min_freq, i915_min_freq_fops}, diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index bcf79f0..49b7be7 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1546,20 +1546,34 @@ struct drm_i915_private { wait_queue_head_t pending_flip_queue; #ifdef CONFIG_DEBUG_FS struct intel_pipe_crc pipe_crc[I915_MAX_PIPES]; #endif int num_shared_dpll; struct intel_shared_dpll shared_dplls[I915_NUM_PLLS]; int dpio_phy_iosf_port[I915_NUM_PHYS_VLV]; + /* +* workarounds are currently applied at different places and +* changes are being done to consolidate them so exact count is +* not clear at this point, use a max value for now. +*/ +#define I915_MAX_WA_REGS 16 + struct { + u32 addr; + u32 value; + /* bitmask representing WA bits */ + u32 mask; + } intel_wa_regs[I915_MAX_WA_REGS]; + u32 num_wa_regs; + /* Reclocking support */ bool render_reclock_avail; bool lvds_downclock_avail; /* indicates the reduced downclock for LVDS*/ int lvds_downclock; struct
[Intel-gfx] [PATCH 0/2] Apply BDW workarounds using LRIs in render init fn
Workarounds for BDW are applied using LRIs during render ring initialization. Only those WA registers that are part of register state are initialized in this fn, remaining are still in its current place init_clock_gating() which are not affected by a gpu reset. I can send another patch where they can be moved to render ring init function but during testing I found their state doesn't change after reset. Arun Siluvery (2): drm/i915/bdw: Apply workarounds in render ring init function drm/i915/bdw: Export workaround data to debugfs drivers/gpu/drm/i915/i915_debugfs.c | 40 + drivers/gpu/drm/i915/i915_drv.h | 14 + drivers/gpu/drm/i915/i915_gem_context.c | 6 ++ drivers/gpu/drm/i915/intel_pm.c | 48 --- drivers/gpu/drm/i915/intel_ringbuffer.c | 101 drivers/gpu/drm/i915/intel_ringbuffer.h | 1 + 6 files changed, 162 insertions(+), 48 deletions(-) -- 2.0.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915/bdw: Apply workarounds in render ring init function
For BDW workarounds are currently initialized in init_clock_gating() but they are lost during reset, suspend/resume etc; this patch moves the WAs that are part of register state context to render ring init fn otherwise default context ends up with incorrect values as they don't get initialized until init_clock_gating fn. v2: Add workarounds to golden render state This method has its own issues, first of all this is different for each gen and it is generated using a tool so adding new workaround and mainitaining them across gens is not a straightforward process. v3: Use LRIs to emit these workarounds (Ville) Instead of modifying the golden render state the same LRIs are emitted from within the driver. For: VIZ-4092 Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com --- drivers/gpu/drm/i915/i915_gem_context.c | 6 +++ drivers/gpu/drm/i915/intel_pm.c | 48 drivers/gpu/drm/i915/intel_ringbuffer.c | 78 + drivers/gpu/drm/i915/intel_ringbuffer.h | 1 + 4 files changed, 85 insertions(+), 48 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 9683e62..2debce4 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -631,20 +631,26 @@ static int do_switch(struct intel_engine_cs *ring, } uninitialized = !to-legacy_hw_ctx.initialized from == NULL; to-legacy_hw_ctx.initialized = true; done: i915_gem_context_reference(to); ring-last_context = to; if (uninitialized) { + if (IS_BROADWELL(ring-dev)) { + ret = bdw_init_workarounds(ring); + if (ret) + DRM_ERROR(init workarounds: %d\n, ret); + } + ret = i915_gem_render_state_init(ring); if (ret) DRM_ERROR(init render state: %d\n, ret); } return 0; unpin_out: if (ring-id == RCS) i915_gem_object_ggtt_unpin(to-legacy_hw_ctx.rcs_state); diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index c8f744c..668acd9 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5507,101 +5507,53 @@ static void gen8_init_clock_gating(struct drm_device *dev) struct drm_i915_private *dev_priv = dev-dev_private; enum pipe pipe; I915_WRITE(WM3_LP_ILK, 0); I915_WRITE(WM2_LP_ILK, 0); I915_WRITE(WM1_LP_ILK, 0); /* FIXME(BDW): Check all the w/a, some might only apply to * pre-production hw. */ - /* WaDisablePartialInstShootdown:bdw */ - I915_WRITE(GEN8_ROW_CHICKEN, - _MASKED_BIT_ENABLE(PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE)); - - /* WaDisableThreadStallDopClockGating:bdw */ - /* FIXME: Unclear whether we really need this on production bdw. */ - I915_WRITE(GEN8_ROW_CHICKEN, - _MASKED_BIT_ENABLE(STALL_DOP_GATING_DISABLE)); - /* -* This GEN8_CENTROID_PIXEL_OPT_DIS W/A is only needed for -* pre-production hardware -*/ - I915_WRITE(HALF_SLICE_CHICKEN3, - _MASKED_BIT_ENABLE(GEN8_CENTROID_PIXEL_OPT_DIS)); - I915_WRITE(HALF_SLICE_CHICKEN3, - _MASKED_BIT_ENABLE(GEN8_SAMPLER_POWER_BYPASS_DIS)); I915_WRITE(GAMTARBMODE, _MASKED_BIT_ENABLE(ARB_MODE_BWGTLB_DISABLE)); I915_WRITE(_3D_CHICKEN3, _MASKED_BIT_ENABLE(_3D_CHICKEN_SDE_LIMIT_FIFO_POLY_DEPTH(2))); - I915_WRITE(COMMON_SLICE_CHICKEN2, - _MASKED_BIT_ENABLE(GEN8_CSC2_SBE_VUE_CACHE_CONSERVATIVE)); - - I915_WRITE(GEN7_HALF_SLICE_CHICKEN1, - _MASKED_BIT_ENABLE(GEN7_SINGLE_SUBSCAN_DISPATCH_ENABLE)); - - /* WaDisableDopClockGating:bdw May not be needed for production */ - I915_WRITE(GEN7_ROW_CHICKEN2, - _MASKED_BIT_ENABLE(DOP_CLOCK_GATING_DISABLE)); /* WaSwitchSolVfFArbitrationPriority:bdw */ I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | HSW_ECOCHK_ARB_PRIO_SOL); /* WaPsrDPAMaskVBlankInSRD:bdw */ I915_WRITE(CHICKEN_PAR1_1, I915_READ(CHICKEN_PAR1_1) | DPA_MASK_VBLANK_SRD); /* WaPsrDPRSUnmaskVBlankInSRD:bdw */ for_each_pipe(pipe) { I915_WRITE(CHICKEN_PIPESL_1(pipe), I915_READ(CHICKEN_PIPESL_1(pipe)) | BDW_DPRS_MASK_VBLANK_SRD); } - /* Use Force Non-Coherent whenever executing a 3D context. This is a -* workaround for for a possible hang in the unlikely event a TLB -* invalidation occurs during a PSD flush. -*/ - I915_WRITE(HDC_CHICKEN0, - I915_READ(HDC_CHICKEN0) | - _MASKED_BIT_ENABLE(HDC_FORCE_NON_COHERENT)); - /*
Re: [Intel-gfx] [PATCH 00/14] drm/i915: edp vdd locking and prep for power sequencer kick
On Tue, Aug 19, 2014 at 01:46:58PM +0300, Ville Syrjälä wrote: On Tue, Aug 19, 2014 at 11:08:33AM +0300, Jani Nikula wrote: On Mon, 18 Aug 2014, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com While wrestling with the VLV/CHV panel power sequencer I noticed the locking in our edp vdd code was rather broken. This series aims to fix that by introducing a power seqeuencer mutex. I was already thinking about using the aux.hw_mutex for this since it's already locked around the aux -transfer() function, but the VLV/CHV multiple power sequencer issue requires a single lock instead of per-port. For extra kicks, see i915_save_display() and i915_restore_display(). Why are we doing this to ourselves? Yeah, crap all around. I suppose someone needs to frob the lvds code a bit before we can kill the power sequencer stuff from those two functions. I think with Jani's reworked backlight code we can ditch most of them for kms. For the panel power sequencer I guess we just need to hook up a -reset function to lvds/edp. That would also have the benefit of us being able to ditch the lvds restore crap. -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 00/14] drm/i915: edp vdd locking and prep for power sequencer kick
On Tue, Aug 19, 2014 at 10:45:20AM +0300, Jani Nikula wrote: On Mon, 18 Aug 2014, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com While wrestling with the VLV/CHV panel power sequencer I noticed the locking in our edp vdd code was rather broken. This series aims to fix that by introducing a power seqeuencer mutex. I was already thinking about using the aux.hw_mutex for this since it's already locked around the aux -transfer() function, but the VLV/CHV multiple power sequencer issue requires a single lock instead of per-port. At the end of the series there's a bit of reordering of the DP port enable/disable sequences to make subsequent power sequencer kick patches easier. The last patch fixes the wait_pipe_off() timeouts on my ILK. Strictly speaking it shouldn't be part of this series, but I couldn't really test this on my ILK without suffering tons of warnings so I included it here anyway. This is what I'd like to happen here: 1) Patches [1] from me and [2] from Clinton get reviewed and merged first, through -fixes. 2) The rest of the patches in my series after [1] get reviewed and merged, through dinq. 3) The first part of this series, up to the locking bits, mostly reviewed now, get refreshed on top the above, should not conflict much, and we can start merging them while... I've just merged them all to dinq. I guess you could simply cherry-pick the -fixes material to, that's imo saner than backmerges here. I think. -Daniel 4) The second part of this series, from about the locking on, get reviewed and merged. I want this order to make backporting steps 1 and 2 as painless as possible. Please help review them, and I'll follow through with this series. BR, Jani. [1] http://mid.gmane.org/e7a47b50ed0f25cafdc26711fc09561ea8af3b81.1407849872.git.jani.nik...@intel.com [2] http://mid.gmane.org/1408394915-21882-1-git-send-email-clinton.a.tay...@intel.com Ville Syrjälä (14): drm/i915: Parametrize PANEL_PORT_SELECT_VLV drm/i915: Reorganize vlv eDP reboot notifier drm/i915: Use intel_edp_panel_vdd_on() in intel_dp_probe_mst() drm/i915: Rename edp vdd funcs for consistency drm/i915: Add a note explaining vdd on/off handling in intel_dp_aux_ch() drm/i915: Replace big nested if block with early return drm/i915: Warn about want_panel_vdd in edp_panel_vdd_off_sync() drm/i915: Flatten intel_edp_panel_vdd_on() drm/i915: Fix edp vdd locking drm/i915: Track which port is using which pipe's power sequencer drm/i915: Be more careful when picking the initial power sequencer pipe drm/i915: Turn on panel power before doing aux transfers drm/i915: Enable DP port earlier drm/i915: Move DP port disable to post_disable for pch platforms drivers/gpu/drm/i915/i915_drv.h | 3 + drivers/gpu/drm/i915/i915_reg.h | 3 +- drivers/gpu/drm/i915/intel_display.c | 2 + drivers/gpu/drm/i915/intel_dp.c | 619 +-- drivers/gpu/drm/i915/intel_drv.h | 6 + 5 files changed, 463 insertions(+), 170 deletions(-) -- 1.8.5.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center ___ 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 14/14] drm/i915: Move DP port disable to post_disable for pch platforms
On Mon, Aug 18, 2014 at 10:16:09PM +0300, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com We need to turn the DP port off after the pipe, otherwise the pipe won't turn off properly on certain pch platforms at least (happens on my ILK for example). This also matches the BSpec modeset sequence better. We still don't match the spec exactly though (eg. audio disable should happen much earlier), but at last this eliminates the nasty wait_for_pipe_off() timeouts. We already did the port disable after the pipe for VLV/CHV and for CPU eDP. For g4x leave the port disable where it is since that matches the modeset sequence in the documentation and I don't have a suitable machine to test if the other order would work. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com We have a pile of bug reports on this topic: https://bugs.freedesktop.org/show_bug.cgi?id=67462 https://bugs.freedesktop.org/show_bug.cgi?id=54687 https://bugzilla.kernel.org/show_bug.cgi?id=52061 https://bugzilla.kernel.org/show_bug.cgi?id=52061 Can you please run them by these reports and collect tested-bys? If it checks out it's imo good to go. -Daniel --- drivers/gpu/drm/i915/intel_dp.c | 16 +++- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 12925be..915d4ec 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -2194,7 +2194,6 @@ void intel_edp_psr_init(struct drm_device *dev) static void intel_disable_dp(struct intel_encoder *encoder) { struct intel_dp *intel_dp = enc_to_intel_dp(encoder-base); - enum port port = dp_to_dig_port(intel_dp)-port; struct drm_device *dev = encoder-base.dev; /* Make sure the panel is off before trying to change the mode. But also @@ -2204,21 +2203,19 @@ static void intel_disable_dp(struct intel_encoder *encoder) intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF); intel_edp_panel_off(intel_dp); - /* cpu edp my only be disable _after_ the cpu pipe/plane is disabled. */ - if (!(port == PORT_A || IS_VALLEYVIEW(dev))) + /* disable the port before the pipe on g4x */ + if (INTEL_INFO(dev)-gen 5) intel_dp_link_down(intel_dp); } -static void g4x_post_disable_dp(struct intel_encoder *encoder) +static void ilk_post_disable_dp(struct intel_encoder *encoder) { struct intel_dp *intel_dp = enc_to_intel_dp(encoder-base); enum port port = dp_to_dig_port(intel_dp)-port; - if (port != PORT_A) - return; - intel_dp_link_down(intel_dp); - ironlake_edp_pll_off(intel_dp); + if (port == PORT_A) + ironlake_edp_pll_off(intel_dp); } static void vlv_post_disable_dp(struct intel_encoder *encoder) @@ -5044,7 +5041,8 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port) } else { intel_encoder-pre_enable = g4x_pre_enable_dp; intel_encoder-enable = g4x_enable_dp; - intel_encoder-post_disable = g4x_post_disable_dp; + if (INTEL_INFO(dev)-gen = 5) + intel_encoder-post_disable = ilk_post_disable_dp; } intel_dig_port-port = port; -- 1.8.5.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] drm/i915/dp: Backlight PWM enable before BL Enable assert
On Fri, Aug 22, 2014 at 05:12:25PM +, Runyan, Arthur J wrote: I'll check it out. I haven't heard any complaint about this before, but it may be one of those things that started back on i745 and never got documented. Only i855 and later started to have native lvds with all the surrounding stuff, before there's only bridge chips that did all the magic. So won't be quite _that_ old ;-) Cheers, Daniel -Original Message- From: Jani Nikula [mailto:jani.nik...@linux.intel.com] Sent: Friday, August 22, 2014 6:07 AM To: Taylor, Clinton A; Ville Syrjälä; Runyan, Arthur J Cc: Intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH] drm/i915/dp: Backlight PWM enable before BL Enable assert +Art On Thu, 21 Aug 2014, Clint Taylor clinton.a.tay...@intel.com wrote: There is also a need to add this delay when turning off the PWM enable bit through intel_panel_disable_backlight(). If the PWM enable bit is turned off while the PWM signal is high then the signal remains high. If the bit is turned off when the signal is low the PWM will remain low. Since we don't know the current state of the PWM signal we must set the duty_cycle to 0, wait PWM_CYCLE_DELAY, and then turn off the enable bit. [citation needed] Really, I want this in the specs if this is true. Actually it might be better if we never turn off the PWM enable bit in intel_panel_disable_backlight() and we just use the duty cycle register to control the PWM. Art, any feedback on these two? BR, Jani. So I guess your approach is the simplest option here. _intel_edp_backlight_on(intel_dp); } @@ -4256,9 +4257,10 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev, assign_final(t11_t12); #undef assign_final +#define PWM_CYCLE_DELAY 5 Shoduln't this be calculated from the PWM frequqncy? Not sure what a PWM cycle here is exactly. Just one full period of the signal? The PWM cycle in this case turns out to be 1 full cycle of the PWM waveform though it depends on which display core clock (128 or 25Mhz(S0ix) is being used. Since we only care about the minimum elapsed time to meet the panel specification a value of 5ms will work even though more or less PWM cycles would take place before the BL_ENABLE is asserted. I would prefer not to add complexity to switch between panel timings for normal and S0ix display clock modes. How often does the backlight get enabled while in S0ix? Also would be nice to have a comment in the code explaining what this is and why we need to add it to the delay. Sure, As you may have noticed in other patches I really like comments. #define get_delay(field)(DIV_ROUND_UP(final.field, 10)) intel_dp-panel_power_up_delay = get_delay(t1_t3); - intel_dp-backlight_on_delay = get_delay(t8); + intel_dp-backlight_on_delay = get_delay(t8) + PWM_CYCLE_DELAY; intel_dp-backlight_off_delay = get_delay(t9); intel_dp-panel_power_down_delay = get_delay(t10); intel_dp-panel_power_cycle_delay = get_delay(t11_t12); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 3abc915..ad6fcc1 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -556,6 +556,7 @@ struct intel_dp { bool want_panel_vdd; unsigned long last_power_cycle; unsigned long last_power_on; + unsigned long last_backlight_on; unsigned long last_backlight_off; struct notifier_block edp_notifier; -- 1.8.3.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center ___ 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 1/2] drm/i915/bdw: Apply workarounds in render ring init function
On Tue, Aug 26, 2014 at 10:33:16AM +0100, Arun Siluvery wrote: For BDW workarounds are currently initialized in init_clock_gating() but they are lost during reset, suspend/resume etc; this patch moves the WAs that are part of register state context to render ring init fn otherwise default context ends up with incorrect values as they don't get initialized until init_clock_gating fn. v2: Add workarounds to golden render state This method has its own issues, first of all this is different for each gen and it is generated using a tool so adding new workaround and mainitaining them across gens is not a straightforward process. v3: Use LRIs to emit these workarounds (Ville) Instead of modifying the golden render state the same LRIs are emitted from within the driver. For: VIZ-4092 Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com --- drivers/gpu/drm/i915/i915_gem_context.c | 6 +++ drivers/gpu/drm/i915/intel_pm.c | 48 drivers/gpu/drm/i915/intel_ringbuffer.c | 78 + drivers/gpu/drm/i915/intel_ringbuffer.h | 1 + 4 files changed, 85 insertions(+), 48 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 9683e62..2debce4 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -631,20 +631,26 @@ static int do_switch(struct intel_engine_cs *ring, } uninitialized = !to-legacy_hw_ctx.initialized from == NULL; to-legacy_hw_ctx.initialized = true; done: i915_gem_context_reference(to); ring-last_context = to; if (uninitialized) { + if (IS_BROADWELL(ring-dev)) { + ret = bdw_init_workarounds(ring); + if (ret) + DRM_ERROR(init workarounds: %d\n, ret); A good rule of thumb is that if you are exporting gen specific routines, the layering and abstraction is fishy. -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 1/2] drm/i915/bdw: Apply workarounds in render ring init function
On 26/08/2014 11:09, Chris Wilson wrote: On Tue, Aug 26, 2014 at 10:33:16AM +0100, Arun Siluvery wrote: For BDW workarounds are currently initialized in init_clock_gating() but they are lost during reset, suspend/resume etc; this patch moves the WAs that are part of register state context to render ring init fn otherwise default context ends up with incorrect values as they don't get initialized until init_clock_gating fn. v2: Add workarounds to golden render state This method has its own issues, first of all this is different for each gen and it is generated using a tool so adding new workaround and mainitaining them across gens is not a straightforward process. v3: Use LRIs to emit these workarounds (Ville) Instead of modifying the golden render state the same LRIs are emitted from within the driver. For: VIZ-4092 Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com --- drivers/gpu/drm/i915/i915_gem_context.c | 6 +++ drivers/gpu/drm/i915/intel_pm.c | 48 drivers/gpu/drm/i915/intel_ringbuffer.c | 78 + drivers/gpu/drm/i915/intel_ringbuffer.h | 1 + 4 files changed, 85 insertions(+), 48 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 9683e62..2debce4 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -631,20 +631,26 @@ static int do_switch(struct intel_engine_cs *ring, } uninitialized = !to-legacy_hw_ctx.initialized from == NULL; to-legacy_hw_ctx.initialized = true; done: i915_gem_context_reference(to); ring-last_context = to; if (uninitialized) { + if (IS_BROADWELL(ring-dev)) { + ret = bdw_init_workarounds(ring); + if (ret) + DRM_ERROR(init workarounds: %d\n, ret); A good rule of thumb is that if you are exporting gen specific routines, the layering and abstraction is fishy. -Chris ok, so something like i915_init_workarounds() is ok? with a check for bdw/gen8 done inside that function. regards Arun ___ 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: reorganize the unclaimed register detection code
On Wed, Jul 16, 2014 at 05:49:29PM -0300, Paulo Zanoni wrote: static void -hsw_unclaimed_reg_check(struct drm_i915_private *dev_priv, u32 reg) +hsw_unclaimed_reg_detect(struct drm_i915_private *dev_priv) { + if (i915.mmio_debug) + return; + if (__raw_i915_read32(dev_priv, FPGA_DBG) FPGA_DBG_RM_NOCLAIM) { - DRM_ERROR(Unclaimed write to %x\n, reg); + DRM_ERROR(Unclaimed register detected. Please use the i915.mmio_debug=1 to debug this problem.); __raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM); } What was the point here? You still add an extra read to every register write and then repeat the request to enable mmio_debug ad infinitum. And you still check for illegal writes in the irq handler. Just kill this code. -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 v2] drm/i915: fix suspend/resume for GENs w/o runtime PM support
Before sharing common parts between the system and runtime s/r handlers we WARNed if the runtime s/r handlers were called on GENs that didn't support RPM. But this WARN is not correct if the same handler is called from the system s/r path, since that can happen on any platform. This also broke system s/r on old platforms. The issue was introduced in commit 016970beb05da6285c2f3ed2bee1c676cb75972e Author: Sagar Kamble sagar.a.kam...@intel.com Date: Wed Aug 13 23:07:06 2014 +0530 v2: - remove the WARN and depend on the HAS_RUNTIME_PM check in rutime_suspend/resume instead (Daniel) Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82751 Signed-off-by: Imre Deak imre.d...@intel.com --- drivers/gpu/drm/i915/i915_drv.c | 31 ++- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index de1b664..b5fd1c5 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1412,7 +1412,9 @@ static int intel_runtime_suspend(struct device *device) if (WARN_ON_ONCE(!(dev_priv-rps.enabled intel_enable_rc6(dev return -ENODEV; - WARN_ON(!HAS_RUNTIME_PM(dev)); + if (WARN_ON_ONCE(!HAS_RUNTIME_PM(dev))) + return -ENODEV; + assert_force_wake_inactive(dev_priv); DRM_DEBUG_KMS(Suspending device\n); @@ -1480,7 +1482,8 @@ static int intel_runtime_resume(struct device *device) struct drm_i915_private *dev_priv = dev-dev_private; int ret; - WARN_ON(!HAS_RUNTIME_PM(dev)); + if (WARN_ON_ONCE(!HAS_RUNTIME_PM(dev))) + return -ENODEV; DRM_DEBUG_KMS(Resuming device\n); @@ -1515,16 +1518,12 @@ static int intel_suspend_complete(struct drm_i915_private *dev_priv) struct drm_device *dev = dev_priv-dev; int ret; - if (IS_GEN6(dev)) { - ret = 0; - } else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) { + if (IS_HASWELL(dev) || IS_BROADWELL(dev)) ret = hsw_suspend_complete(dev_priv); - } else if (IS_VALLEYVIEW(dev)) { + else if (IS_VALLEYVIEW(dev)) ret = vlv_suspend_complete(dev_priv); - } else { - ret = -ENODEV; - WARN_ON(1); - } + else + ret = 0; return ret; } @@ -1540,16 +1539,14 @@ static int intel_resume_prepare(struct drm_i915_private *dev_priv, struct drm_device *dev = dev_priv-dev; int ret; - if (IS_GEN6(dev)) { + if (IS_GEN6(dev)) ret = snb_resume_prepare(dev_priv, rpm_resume); - } else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) { + else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) ret = hsw_resume_prepare(dev_priv, rpm_resume); - } else if (IS_VALLEYVIEW(dev)) { + else if (IS_VALLEYVIEW(dev)) ret = vlv_resume_prepare(dev_priv, rpm_resume); - } else { - WARN_ON(1); - ret = -ENODEV; - } + else + ret = 0; return ret; } -- 1.8.4 ___ 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/bdw: Apply workarounds in render ring init function
On Tue, Aug 26, 2014 at 11:16:29AM +0100, Siluvery, Arun wrote: On 26/08/2014 11:09, Chris Wilson wrote: On Tue, Aug 26, 2014 at 10:33:16AM +0100, Arun Siluvery wrote: For BDW workarounds are currently initialized in init_clock_gating() but they are lost during reset, suspend/resume etc; this patch moves the WAs that are part of register state context to render ring init fn otherwise default context ends up with incorrect values as they don't get initialized until init_clock_gating fn. v2: Add workarounds to golden render state This method has its own issues, first of all this is different for each gen and it is generated using a tool so adding new workaround and mainitaining them across gens is not a straightforward process. v3: Use LRIs to emit these workarounds (Ville) Instead of modifying the golden render state the same LRIs are emitted from within the driver. For: VIZ-4092 Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com --- drivers/gpu/drm/i915/i915_gem_context.c | 6 +++ drivers/gpu/drm/i915/intel_pm.c | 48 drivers/gpu/drm/i915/intel_ringbuffer.c | 78 + drivers/gpu/drm/i915/intel_ringbuffer.h | 1 + 4 files changed, 85 insertions(+), 48 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 9683e62..2debce4 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -631,20 +631,26 @@ static int do_switch(struct intel_engine_cs *ring, } uninitialized = !to-legacy_hw_ctx.initialized from == NULL; to-legacy_hw_ctx.initialized = true; done: i915_gem_context_reference(to); ring-last_context = to; if (uninitialized) { + if (IS_BROADWELL(ring-dev)) { + ret = bdw_init_workarounds(ring); + if (ret) + DRM_ERROR(init workarounds: %d\n, ret); A good rule of thumb is that if you are exporting gen specific routines, the layering and abstraction is fishy. -Chris ok, so something like i915_init_workarounds() is ok? with a check for bdw/gen8 done inside that function. Except for init_workarounds is quite useless as a function name and we already have a structure that is already customised per-engine and per-gen that you could hook into. engine-ring_init_context() ? -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: don't warn if backlight unexpectedly enabled
On Thu, Aug 21, 2014 at 07:12:59AM +, Scot Doyle wrote: On Tue, 19 Aug 2014, Daniel Vetter wrote: On Tue, Aug 19, 2014 at 4:07 AM, Scot Doyle lkm...@scotdoyle.com wrote: BIOS or firmware can modify hardware state during suspend/resume, for example on the Toshiba CB35 or Lenovo T400, so log a debug message instead of a warning if the backlight is unexpectedly enabled. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=80930 Cc: Jani Nikula jani.nik...@intel.com Signed-off-by: Scot Doyle lkm...@scotdoyle.com This should get cleaned up in the modeset state sanitization we do upon resume, so without someone digging into this bug a bit and coming up with an explanation for why that fails I'm reluctant to merge this. -Daniel When we enter intel_modeset_setup_hw_state during resume - BLC_PWM_CPU_CTL2 == BLM_PWM_ENABLE - the physical backlight is off Hm, this is actually interesting - we have some other evidence that the best way to shut off the backlight is actually to just set the pwm duty cycle to 0. Can you please check that this is the case for your system? Maybe we just need to extend the check to look for !PWM_ENABLE || duty_cycle == 0. In general if we hit upon a WARN it's not a good idea to just shut it up, but to dig a bit deeper and figure out why exactly something doesn't work as we expected it to work. Thanks, Daniel - readout_hw_state says crtcs, encoders and connectors are disabled The sanitizations done before reaching the force_restore block are - drm_vblank_off for each crtc - sarea_priv-pipeA_w=0 - sarea_priv-pipeA_h=0 - ilk_wm_get_hw_state(dev) Then the warning is logged inside the force_restore block [11.651495] [8106fc5c] warn_slowpath_fmt+0x5c/0x80 [11.651508] [a0576d0a] pch_enable_backlight+0x1ba/0x200 [11.651518] [a0577474] intel_panel_enable_backlight+0xa4/0x100 [11.651529] [a0568514] intel_edp_backlight_on+0x54/0x140 [11.651540] [a0560c23] intel_enable_ddi+0xb3/0x100 [11.651552] [a054b869] haswell_crtc_enable+0x559/0xe80 [11.651564] [a0545ea4] __intel_set_mode+0xf94/0x1c00 [11.651575] [a055062b] intel_modeset_setup_hw_state+0x55b/0xd80 [11.651609] [a04eb3a9] __i915_drm_thaw+0x159/0x1d0 [11.651617] [a04ebcd8] i915_resume+0x28/0x50 I see no code in the execution path of intel_modeset_setup_hw_state to either note the state of BLC_PWM_CPU_CTL2 or reset it, apart from the pch_enable_backlight function. Which would be one explanation :-) -- 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 v2] drm/i915: fix suspend/resume for GENs w/o runtime PM support
On Tue, Aug 26, 2014 at 01:26:56PM +0300, Imre Deak wrote: Before sharing common parts between the system and runtime s/r handlers we WARNed if the runtime s/r handlers were called on GENs that didn't support RPM. But this WARN is not correct if the same handler is called from the system s/r path, since that can happen on any platform. This also broke system s/r on old platforms. The issue was introduced in commit 016970beb05da6285c2f3ed2bee1c676cb75972e Author: Sagar Kamble sagar.a.kam...@intel.com Date: Wed Aug 13 23:07:06 2014 +0530 Aside: I just realized that this patch adds an rpm_resume paramter to the platform resume functions. Somehow I've been blind and didn't spot that one. Not a good idea imo, since that still means we have piles of arbitrary differences between the different platforms and code paths. v2: - remove the WARN and depend on the HAS_RUNTIME_PM check in rutime_suspend/resume instead (Daniel) Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82751 Signed-off-by: Imre Deak imre.d...@intel.com Queued for -next, thanks for the patch. -Daniel --- drivers/gpu/drm/i915/i915_drv.c | 31 ++- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index de1b664..b5fd1c5 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1412,7 +1412,9 @@ static int intel_runtime_suspend(struct device *device) if (WARN_ON_ONCE(!(dev_priv-rps.enabled intel_enable_rc6(dev return -ENODEV; - WARN_ON(!HAS_RUNTIME_PM(dev)); + if (WARN_ON_ONCE(!HAS_RUNTIME_PM(dev))) + return -ENODEV; + assert_force_wake_inactive(dev_priv); DRM_DEBUG_KMS(Suspending device\n); @@ -1480,7 +1482,8 @@ static int intel_runtime_resume(struct device *device) struct drm_i915_private *dev_priv = dev-dev_private; int ret; - WARN_ON(!HAS_RUNTIME_PM(dev)); + if (WARN_ON_ONCE(!HAS_RUNTIME_PM(dev))) + return -ENODEV; DRM_DEBUG_KMS(Resuming device\n); @@ -1515,16 +1518,12 @@ static int intel_suspend_complete(struct drm_i915_private *dev_priv) struct drm_device *dev = dev_priv-dev; int ret; - if (IS_GEN6(dev)) { - ret = 0; - } else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) { + if (IS_HASWELL(dev) || IS_BROADWELL(dev)) ret = hsw_suspend_complete(dev_priv); - } else if (IS_VALLEYVIEW(dev)) { + else if (IS_VALLEYVIEW(dev)) ret = vlv_suspend_complete(dev_priv); - } else { - ret = -ENODEV; - WARN_ON(1); - } + else + ret = 0; return ret; } @@ -1540,16 +1539,14 @@ static int intel_resume_prepare(struct drm_i915_private *dev_priv, struct drm_device *dev = dev_priv-dev; int ret; - if (IS_GEN6(dev)) { + if (IS_GEN6(dev)) ret = snb_resume_prepare(dev_priv, rpm_resume); - } else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) { + else if (IS_HASWELL(dev) || IS_BROADWELL(dev)) ret = hsw_resume_prepare(dev_priv, rpm_resume); - } else if (IS_VALLEYVIEW(dev)) { + else if (IS_VALLEYVIEW(dev)) ret = vlv_resume_prepare(dev_priv, rpm_resume); - } else { - WARN_ON(1); - ret = -ENODEV; - } + else + ret = 0; return ret; } -- 1.8.4 ___ 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] i915: Regression: +4W in idle power use on Macbook Pro 15 (late 2013)
On Mon, Aug 25, 2014 at 05:19:49AM -0700, Eric Rannaud wrote: Hi Jani, Is there a way to restore the prior lower power consumption when idling? As I understand it, FBC should not have a direct effect on power consumption when idle, only when the FB is actively refreshed. Is it understood why no FBC would have such a dramatic impact (+4W) on a system sitting idle? FBC works best when the screen contents don't change. The more activity on the screen the less effective FBC becomes. 4W sounds way too much for FBC however. 0.4W is closer to what one might expect from FBC based on my observations. 4W sounds more like the difference between min vs. max display brightness to me. I'm aware of i915.i915_enable_rc6=1 and i915.lvds_downclock=1, which I haven't yet tried. Is there anything else I should also try? Thanks, Eric On Mon, Aug 25, 2014 at 3:22 AM, Jani Nikula jani.nik...@intel.com wrote: [just moving from lkml to intel-gfx for a better fitting audience] On Mon, 25 Aug 2014, Jani Nikula jani.nik...@intel.com wrote: On Fri, 22 Aug 2014, Eric Rannaud eric.rann...@gmail.com wrote: Hi, Between 3.15.4 and 3.15.8, there was an increase in idle power consumption on Apple Macbook Pro 15 (late 2013) on a freshly booted system (no wifi driver loaded; brightness set to 4/100; X running; no desktop environment, except Awesome), from 6.5W to about 10.5W, as reported by powertop. In the stable tree, it bisects to: commit f4db98240ac2c6d9d2118c6f82d483ff5293f1ed Author: Chris Wilson ch...@chris-wilson.co.uk Date: Fri Jun 6 10:37:11 2014 +0100 drm/i915: Disable FBC by default also on Haswell and later commit 0368920e51ae0cded0eb518c340a4dd17764d461 upstream. It causes black screen on bootup and is approximately 100x slower than running with FBC disabled, so the GPU runs at a high frequency for much longer - completely contrary to the power saving claims. It also still has mutex deadlocks in multi-head scenarios, which can lead to a system/X lockup. These bugs were known before FBC was enabled by default on Haswell and still have not been fixed. The issue is still present in Linus' tree (v3.17-rc1-22-g480cadc2b7e0). With a 75Wh battery, that's a significant loss in battery life in normal use. I'll be happy to help test any potential fix. The earlier regression trumps, and in this case it was enabling FBC by default on Haswell. Sorry. You can enable FBC with i915.enable_fbc=1 module parameter, but all bets are off. See the commit message you quoted above. I don't recommend. BR, Jani. -- Jani Nikula, Intel Open Source Technology Center -- Jani Nikula, Intel Open Source Technology Center ___ 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] 3.17.0-rc1: WARNING: CPU: 1 PID: 1 at drivers/gpu/drm/i915/i915_irq.c:139 ironlake_enable_display_irq+0x7f/0x90()
On Mon, Aug 25, 2014 at 03:36:15PM -0700, Jesse Barnes wrote: On Mon, 25 Aug 2014 20:29:27 +0200 Oliver Hartkopp socket...@hartkopp.net wrote: Hi Jesse, since the i915 stuff for 3.17 was merged I always get this warning on my core i7 with internal Intel HD graphics. Intel(R) Core(TM) i7 CPU M 640 @ 2.80GHz As this warning is triggered by the code which was changed by you and obviously the latest drm-fixes did not solve the issue, it's now time to post it :-) My kernel is the latest pull from Linus' mainline tree. Any idea? My .config is attached. I just dug out my ILK to see if I can reproduce this... must be related to the irq warning changes we moved around; maybe I missed an init path on the ILK side. It's the PCU irq enable. I think I have a patch somewhere for it. Just forgot to send it out. -- 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 1/7] drm: Renaming DP training vswing pre emph defines
On Fri, Aug 08, 2014 at 04:23:40PM +0530, sonika.jin...@intel.com wrote: From: Sonika Jindal sonika.jin...@intel.com Adding new defines, older one will be removed in the last patch in the series. This is to rename the defines to have levels instead of values for vswing and pre-emph levels as the values may differ in other scenarios like low vswing of eDP1.4 where the values are different. Done using following cocci patch for each define: @@ @@ # define DP_TRAIN_VOLTAGE_SWING_400 (0 0) + # define DP_TRAIN_VOLTAGE_SWING_LEVEL_0 (0 0) Could this perhaps be simply: #define DP_TRAIN_VOLTAGE_SWING(x) ((x) 0) As it is, there's no information about the value within the symbolic name anyway, so _LEVEL_* really isn't that useful and keeping several macros for each value seems isn't either. An alternative would be to provide a second set of defines for eDP 1.4 where the name implies the meaning and then use them as appropriate. Thierry pgpA_r_4Q22mZ.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 1/2] drm/i915/bdw: Apply workarounds in render ring init function
On 26/08/2014 11:34, Chris Wilson wrote: On Tue, Aug 26, 2014 at 11:16:29AM +0100, Siluvery, Arun wrote: On 26/08/2014 11:09, Chris Wilson wrote: On Tue, Aug 26, 2014 at 10:33:16AM +0100, Arun Siluvery wrote: For BDW workarounds are currently initialized in init_clock_gating() but they are lost during reset, suspend/resume etc; this patch moves the WAs that are part of register state context to render ring init fn otherwise default context ends up with incorrect values as they don't get initialized until init_clock_gating fn. v2: Add workarounds to golden render state This method has its own issues, first of all this is different for each gen and it is generated using a tool so adding new workaround and mainitaining them across gens is not a straightforward process. v3: Use LRIs to emit these workarounds (Ville) Instead of modifying the golden render state the same LRIs are emitted from within the driver. For: VIZ-4092 Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com --- drivers/gpu/drm/i915/i915_gem_context.c | 6 +++ drivers/gpu/drm/i915/intel_pm.c | 48 drivers/gpu/drm/i915/intel_ringbuffer.c | 78 + drivers/gpu/drm/i915/intel_ringbuffer.h | 1 + 4 files changed, 85 insertions(+), 48 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 9683e62..2debce4 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -631,20 +631,26 @@ static int do_switch(struct intel_engine_cs *ring, } uninitialized = !to-legacy_hw_ctx.initialized from == NULL; to-legacy_hw_ctx.initialized = true; done: i915_gem_context_reference(to); ring-last_context = to; if (uninitialized) { + if (IS_BROADWELL(ring-dev)) { + ret = bdw_init_workarounds(ring); + if (ret) + DRM_ERROR(init workarounds: %d\n, ret); A good rule of thumb is that if you are exporting gen specific routines, the layering and abstraction is fishy. -Chris ok, so something like i915_init_workarounds() is ok? with a check for bdw/gen8 done inside that function. Except for init_workarounds is quite useless as a function name and we already have a structure that is already customised per-engine and per-gen that you could hook into. engine-ring_init_context() ? -Chris Ok thanks, I can create a new fn ring_init_context(). regards Arun ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Make wait-for-pending-flips more defensive
On Wed, 20 Aug 2014, Chris Wilson ch...@chris-wilson.co.uk wrote: Be sure to always flush a stuck pageflip even if we couldn't possibly expect one to be there. References: https://bugs.freedesktop.org/show_bug.cgi?id=82612 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk --- drivers/gpu/drm/i915/intel_display.c | 12 +--- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index a7582a46e82e..5898e7157c4c 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3359,11 +3359,7 @@ void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc) struct drm_device *dev = crtc-dev; struct drm_i915_private *dev_priv = dev-dev_private; - if (crtc-primary-fb == NULL) - return; - WARN_ON(waitqueue_active(dev_priv-pending_flip_queue)); - if (WARN_ON(wait_event_timeout(dev_priv-pending_flip_queue, !intel_crtc_has_pending_flip(crtc), 60*HZ) == 0)) { @@ -3378,9 +3374,11 @@ void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc) spin_unlock_irqrestore(dev-event_lock, flags); } Chris, the patch context has changed above, in fact I can't find such context anywhere. Is the patch otherwise valid against current -fixes? BR, Jani. - mutex_lock(dev-struct_mutex); - intel_finish_fb(crtc-primary-fb); - mutex_unlock(dev-struct_mutex); + if (crtc-primary-fb) { + mutex_lock(dev-struct_mutex); + intel_finish_fb(crtc-primary-fb); + mutex_unlock(dev-struct_mutex); + } } /* Program iCLKIP clock to the desired frequency */ -- 2.1.0.rc1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- 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: Make wait-for-pending-flips more defensive
On Tue, Aug 26, 2014 at 02:49:07PM +0300, Jani Nikula wrote: On Wed, 20 Aug 2014, Chris Wilson ch...@chris-wilson.co.uk wrote: Be sure to always flush a stuck pageflip even if we couldn't possibly expect one to be there. References: https://bugs.freedesktop.org/show_bug.cgi?id=82612 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk --- drivers/gpu/drm/i915/intel_display.c | 12 +--- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index a7582a46e82e..5898e7157c4c 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3359,11 +3359,7 @@ void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc) struct drm_device *dev = crtc-dev; struct drm_i915_private *dev_priv = dev-dev_private; - if (crtc-primary-fb == NULL) - return; - WARN_ON(waitqueue_active(dev_priv-pending_flip_queue)); - if (WARN_ON(wait_event_timeout(dev_priv-pending_flip_queue, !intel_crtc_has_pending_flip(crtc), 60*HZ) == 0)) { @@ -3378,9 +3374,11 @@ void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc) spin_unlock_irqrestore(dev-event_lock, flags); } Chris, the patch context has changed above, in fact I can't find such context anywhere. Is the patch otherwise valid against current -fixes? The context is from earlier patches to decouple stuck pageflips before a modeset. They have been on the list for many months - a very useful fixup in cases like this. This patch itself does not depend upon those changes. -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: Move intel_ddi_set_vc_payload_alloc(false) to haswell_crtc_disable()
On Mon, 18 Aug 2014, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com Somehow the intel_ddi_set_vc_payload_alloc(false) call has ended up in ironlake_crtc_disable() rather than haswell_crtc_disable(). Move it to the correct place. intel_ddi_disable_transcoder_func() already disables the vc payload allocation so this doesn't actually do anything more. The spec says we should wait for some kind of ack after frobbing the bit. We don't appear to do that currently, but if and when someone decides that we should do it, intel_ddi_set_vc_payload_alloc() would appear to be be the right place for it. So having the function call in haswell_crtc_disable() seems like the right thing for the future even if it does nothing currently. Cc: Dave Airlie airl...@redhat.com Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com Pushed to drm-intel-fixes, thanks for the patch. BR, Jani. --- drivers/gpu/drm/i915/intel_display.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 09c7298..121024e 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4169,10 +4169,6 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc) intel_set_pch_fifo_underrun_reporting(dev, pipe, false); intel_disable_pipe(dev_priv, pipe); - - if (intel_crtc-config.dp_encoder_is_mst) - intel_ddi_set_vc_payload_alloc(crtc, false); - ironlake_pfit_disable(intel_crtc); for_each_encoder_on_crtc(dev, crtc, encoder) @@ -4240,6 +4236,9 @@ static void haswell_crtc_disable(struct drm_crtc *crtc) intel_set_pch_fifo_underrun_reporting(dev, TRANSCODER_A, false); intel_disable_pipe(dev_priv, pipe); + if (intel_crtc-config.dp_encoder_is_mst) + intel_ddi_set_vc_payload_alloc(crtc, false); + intel_ddi_disable_transcoder_func(dev_priv, cpu_transcoder); ironlake_pfit_disable(intel_crtc); -- 1.8.5.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- 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 1/2] drm/i915: reorganize the unclaimed register detection code
2014-08-26 7:22 GMT-03:00 Chris Wilson ch...@chris-wilson.co.uk: On Wed, Jul 16, 2014 at 05:49:29PM -0300, Paulo Zanoni wrote: static void -hsw_unclaimed_reg_check(struct drm_i915_private *dev_priv, u32 reg) +hsw_unclaimed_reg_detect(struct drm_i915_private *dev_priv) { + if (i915.mmio_debug) + return; + if (__raw_i915_read32(dev_priv, FPGA_DBG) FPGA_DBG_RM_NOCLAIM) { - DRM_ERROR(Unclaimed write to %x\n, reg); + DRM_ERROR(Unclaimed register detected. Please use the i915.mmio_debug=1 to debug this problem.); __raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM); } What was the point here? You still add an extra read to every register write Well, we previously had 2 extra reads instead of 1, so with this patch we're in a better position :) and then repeat the request to enable mmio_debug ad infinitum. Yeah, this could be avoided. OTOH, on most cases it's not gonna happen frequently enough to annoy the user. And you still check for illegal writes in the irq handler. That just happens on HSW, not BDW+. Just kill this code. If we do it, we won't be checking for unclaimed registers on BDW+ without i915.mmio_debug=1. -Chris -- Chris Wilson, Intel Open Source Technology Centre -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: fix plane/cursor handling when runtime suspended
On Fri, 15 Aug 2014, Paulo Zanoni przan...@gmail.com wrote: From: Paulo Zanoni paulo.r.zan...@intel.com If we're runtime suspended and try to use the plane interfaces, we will get a lot of WARNs saying we did the wrong thing. We need to get runtime PM references to pin the objects, and to change the fences. The pin functions are the ideal places for this, but intel_crtc_cursor_set_obj() doesn't call them, so we also have to add get/put calls inside it. There is no problem if we runtime suspend right after these functions are finished, because the registers written are forwarded to system memory. Note: for a complete fix of the cursor-dpms test case, we also need the patch named drm/i915: Don't try to enable cursor from setplane when crtc is disabled. v2: - Narrow the put/get calls on intel_crtc_cursor_set_obj() (Daniel) v3: - Make get/put also surround the fence and unpin calls (Daniel and Ville). - Merge all the plane changes into a single patch since they're the same fix. - Add the comment requested by Daniel. v4: - Remove spurious whitespace (Ville). v5: - Remove intel_crtc_update_cursor() chunk since Ville did an equivalent fix in another patch (Ville). v6: - Remove unpin chunk: it will be on a separate patch (Ville, Chris, Daniel). v7: - Same thing, new color. Testcase: igt/pm_rpm/cursor Testcase: igt/pm_rpm/cursor-dpms Testcase: igt/pm_rpm/legacy-planes Testcase: igt/pm_rpm/legacy-planes-dpms Testcase: igt/pm_rpm/universal-planes Testcase: igt/pm_rpm/universal-planes-dpms Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=81645 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=82603 Cc: sta...@vger.kernel.org Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com Pushed to drm-intel-fixes, thanks for the patch and review. BR, Jani. --- drivers/gpu/drm/i915/intel_display.c | 25 + 1 file changed, 25 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 3f8e037..15fe3eb 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2201,6 +2201,15 @@ intel_pin_and_fence_fb_obj(struct drm_device *dev, if (need_vtd_wa(dev) alignment 256 * 1024) alignment = 256 * 1024; + /* + * Global gtt pte registers are special registers which actually forward + * writes to a chunk of system memory. Which means that there is no risk + * that the register values disappear as soon as we call + * intel_runtime_pm_put(), so it is correct to wrap only the + * pin/unpin/fence and not more. + */ + intel_runtime_pm_get(dev_priv); + dev_priv-mm.interruptible = false; ret = i915_gem_object_pin_to_display_plane(obj, alignment, pipelined); if (ret) @@ -2218,12 +2227,14 @@ intel_pin_and_fence_fb_obj(struct drm_device *dev, i915_gem_object_pin_fence(obj); dev_priv-mm.interruptible = true; + intel_runtime_pm_put(dev_priv); return 0; err_unpin: i915_gem_object_unpin_from_display_plane(obj); err_interruptible: dev_priv-mm.interruptible = true; + intel_runtime_pm_put(dev_priv); return ret; } @@ -8253,6 +8264,7 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc, uint32_t width, uint32_t height) { struct drm_device *dev = crtc-dev; + struct drm_i915_private *dev_priv = dev-dev_private; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); enum pipe pipe = intel_crtc-pipe; unsigned old_width, stride; @@ -8292,6 +8304,15 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc, goto fail_locked; } + /* + * Global gtt pte registers are special registers which actually + * forward writes to a chunk of system memory. Which means that + * there is no risk that the register values disappear as soon + * as we call intel_runtime_pm_put(), so it is correct to wrap + * only the pin/unpin/fence and not more. + */ + intel_runtime_pm_get(dev_priv); + /* Note that the w/a also requires 2 PTE of padding following * the bo. We currently fill all unused PTE with the shadow * page and so we should always have valid PTE following the @@ -8304,16 +8325,20 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc *crtc, ret = i915_gem_object_pin_to_display_plane(obj, alignment, NULL); if (ret) { DRM_DEBUG_KMS(failed to move cursor bo into the GTT\n); + intel_runtime_pm_put(dev_priv); goto fail_locked; } ret = i915_gem_object_put_fence(obj); if (ret) {
Re: [Intel-gfx] [PATCH] drm/i915: Ignore VBT backlight presence check on Acer C720 (4005U)
On Thu, 21 Aug 2014, Scot Doyle lkm...@scotdoyle.com wrote: commit c675949ec58ca50d5a3ae3c757892f1560f6e896 drm/i915: do not setup backlight if not available according to VBT prevents backlight setup on the Acer C720 (Core i3 4005U CPU), which has a misconfigured VBT. Apply quirk to ignore the VBT backlight presence check during backlight setup. Signed-off-by: Scot Doyle lkm...@scotdoyle.com Tested-by: Tyler Cleveland siraluca...@openmailbox.org Cc: Jani Nikula jani.nik...@intel.com Cc: Daniel Vetter daniel.vet...@ffwll.ch Pushed to drm-intel-fixes, thanks for the patch. BR, Jani. --- 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 0b327eb..3d4a4eb 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12553,6 +12553,9 @@ static struct intel_quirk intel_quirks[] = { /* Acer C720 and C720P Chromebooks (Celeron 2955U) have backlights */ { 0x0a06, 0x1025, 0x0a11, quirk_backlight_present }, + /* Acer C720 Chromebook (Core i3 4005U) */ + { 0x0a16, 0x1025, 0x0a11, quirk_backlight_present }, + /* Toshiba CB35 Chromebook (Celeron 2955U) */ { 0x0a06, 0x1179, 0x0a88, quirk_backlight_present }, -- 1.7.10.4 -- 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 v2] drm/i915: Handle i915_ppgtt_put correctly
On Tue, Aug 19, 2014 at 03:49:41PM +0100, Michel Thierry wrote: Unfortunately, the gem_obj/vma relationship is not symmetrical; a gem_obj can look up for the same vma more than once (where the ppgtt refcount is incremented), but will free the vma only once (i915_gem_free_object). This difference in refcount get/put means that the ppgtt is not removed after the context and vma are destroyed, because sometimes the refcount will never go back to zero. v2: Just move the ppgtt refcount into vma_create. OTC-Jira: VIZ-3719 Signed-off-by: Michel Thierry michel.thie...@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
Re: [Intel-gfx] [PATCH 12/14] drm/i915: Turn on panel power before doing aux transfers
On Tue, Aug 19, 2014 at 01:57:57PM +0300, Ville Syrjälä wrote: On Tue, Aug 19, 2014 at 10:33:15AM +0300, Jani Nikula wrote: On Mon, 18 Aug 2014, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com On VLV/CHV the panel power sequencer may need to be kicked a bit to lock onto the new port, and that needs to happen before any aux transfers are attempted if we want the aux transfers to actaully succeed. So turn on panel power (part of the kick) before aux transfers (DPMS_ON + link training). This also matches the documented modeset sequence better for pch platforms. The documentation doesn't explicitly state anything about the DPMS or link training DPCD writes, but the panel power on step is always listed before link training is mentioned. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_dp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 4952783..28bc652 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -2275,10 +2275,10 @@ static void intel_enable_dp(struct intel_encoder *encoder) return; intel_edp_panel_vdd_on(intel_dp); - intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); - intel_dp_start_link_train(intel_dp); intel_edp_panel_on(intel_dp); intel_edp_panel_vdd_off(intel_dp, true); + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); + intel_dp_start_link_train(intel_dp); Please dig into the git history in this area. I fear this may regress. We've juggled this too many times... I did. But I couldn't spot much solid analysis of the problems in the earlier patches/reverts. It's mostly been guesswork AFAICS. Most of it seems to be back and forth with the force vdd on/off vs. panel power on, but this patch doesn't change that order. Also: a) we need this patch on VLV/CHV at the very least b) this agrees with the bspec modeset sequence for pch platforms better c) my ILK with CPU eDP seems happy with the new order There have been bug reports about ivb/snb cpu edp panels not lighting up iirc. Especially when the bios didn't light up the panel already (e.g. when an external screen is plugged in). Might be worth a try to haggle this patch to those bugs. But I seem to have a hard time finding them right now :( -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: reorganize the unclaimed register detection code
On Tue, Aug 26, 2014 at 09:17:11AM -0300, Paulo Zanoni wrote: 2014-08-26 7:22 GMT-03:00 Chris Wilson ch...@chris-wilson.co.uk: On Wed, Jul 16, 2014 at 05:49:29PM -0300, Paulo Zanoni wrote: static void -hsw_unclaimed_reg_check(struct drm_i915_private *dev_priv, u32 reg) +hsw_unclaimed_reg_detect(struct drm_i915_private *dev_priv) { + if (i915.mmio_debug) + return; + if (__raw_i915_read32(dev_priv, FPGA_DBG) FPGA_DBG_RM_NOCLAIM) { - DRM_ERROR(Unclaimed write to %x\n, reg); + DRM_ERROR(Unclaimed register detected. Please use the i915.mmio_debug=1 to debug this problem.); __raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM); } What was the point here? You still add an extra read to every register write Well, we previously had 2 extra reads instead of 1, so with this patch we're in a better position :) and then repeat the request to enable mmio_debug ad infinitum. Yeah, this could be avoided. OTOH, on most cases it's not gonna happen frequently enough to annoy the user. How do you think I came across this? And you still check for illegal writes in the irq handler. That just happens on HSW, not BDW+. Even on hsw, it should be killed. Checking inside the irq handler is just insane. Just move it to one of the periodic checks since we can't get any more information than an error occurred, and ask to be re-run with mmio_debug (and then shut up) - heck you could even automatically enable it for a one-shot operation. Just kill this code. If we do it, we won't be checking for unclaimed registers on BDW+ without i915.mmio_debug=1. Then do as above. -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: Make wait-for-pending-flips more defensive
On Tue, Aug 26, 2014 at 02:49:07PM +0300, Jani Nikula wrote: On Wed, 20 Aug 2014, Chris Wilson ch...@chris-wilson.co.uk wrote: Be sure to always flush a stuck pageflip even if we couldn't possibly expect one to be there. References: https://bugs.freedesktop.org/show_bug.cgi?id=82612 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk --- drivers/gpu/drm/i915/intel_display.c | 12 +--- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index a7582a46e82e..5898e7157c4c 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3359,11 +3359,7 @@ void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc) struct drm_device *dev = crtc-dev; struct drm_i915_private *dev_priv = dev-dev_private; - if (crtc-primary-fb == NULL) - return; - WARN_ON(waitqueue_active(dev_priv-pending_flip_queue)); - if (WARN_ON(wait_event_timeout(dev_priv-pending_flip_queue, !intel_crtc_has_pending_flip(crtc), 60*HZ) == 0)) { @@ -3378,9 +3374,11 @@ void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc) spin_unlock_irqrestore(dev-event_lock, flags); } Chris, the patch context has changed above, in fact I can't find such context anywhere. Is the patch otherwise valid against current -fixes? Until we have confirmation that it fixes a real bug I don't think this is material for -fixes. 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
Re: [Intel-gfx] [PATCH 1/2] drm/i915/bdw: Apply workarounds using the golden render state
On Fri, Aug 22, 2014 at 01:10:26PM +0100, Siluvery, Arun wrote: On 22/08/2014 12:06, Mika Kuoppala wrote: Ville Syrjälä ville.syrj...@linux.intel.com writes: On Wed, Aug 20, 2014 at 03:19:17PM +0100, Arun Siluvery wrote: Workarounds for bdw are currently applied in init_clock_gating() but they are lost following a gpu reset. Some of the WA registers are part of register state context and they are restored with every context switch so initializing them in golden render state ensures that they are applied even when we start with an uninitialized context or during hw initlialization followed by a reset. v2: Add comments corresponding to WAs in golden render state (Chris). The generation of render state is not a straighforward process, it would be ideal to augment WA values from during the setup state as opposed to using a tool but that would be a follow up patch. I'd still prefer just emitting the LRIs from code rather tha mucking about with null batch. Less hoops to jump through when adding a new w/a. I agree with this. We should aim to keep null state as per gen. Workaround set is different for gtX inside particular gen so we would need then multiple null states per gen. After brief chat with Ville, I think that the correct spot to init the context specific workarounds is after MI_SET_CONTEXT to default and right before null batch is run. If we do these with emitting LRIs to ring, we should be safe as they are then saved with default ctx. The default ctx is then used as a 'parent' for newly created contexts. Ofcource if registers get globbered, then we inherit crap. If we have the per gen null state and the ring is initializing workarounds for the default context, then in future we can save this state as 'read only golden context'. And use it as the initial state for all newly created contexts. Then the full plan how to init would look like this: #1 reset the gpu (on driver load, on resume or on hang recovery) #2 if we have 'read only golden context', copy it to default ctx #3 switch to default context #4 if we had 'read only golden context' we are done with the init. --- #5 if this is driver load thus there is no 'read only golden context' yet. #6 init workarounds through ring LRIs #7 run null/golden state batch #8 save this state as a 'read only golden context' --- #9 for each new context, initialize ctx obj with 'read only golden context' (either by memcpy or restoring from it when switching to new) I understand applying WAs using null batch has its issues but as I mentioned in the commit msg I will fix this as a follow up patch. It is going to take some time though to change the patch as per the new sequence. The patch in its current state helps fix WA issues after reset; so it can only be accepted if it is updated as per the new sequence? We already have a lot of let's fix it later experiments running, so I don't want to overload the ship. So I highly prefer to merge the revised version directly. -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] igt/gem_workarounds: igt to test workaround registers
On Wed, Aug 20, 2014 at 03:52:12PM +0100, Arun Siluvery wrote: Some of the workarounds are lost followed by a gpu reset, suspend/resume; this patch adds a test which compares register state before and after the test scenario. This test currently verifies only bdw workarounds. Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com On top of Thomas' comments about using igt infrastructure some more below. --- tests/Makefile.sources | 1 + tests/gem_workarounds.c | 238 2 files changed, 239 insertions(+) create mode 100644 tests/gem_workarounds.c diff --git a/tests/Makefile.sources b/tests/Makefile.sources index 0eb9369..a17acd1 100644 --- a/tests/Makefile.sources +++ b/tests/Makefile.sources @@ -127,20 +127,21 @@ TESTS_progs = \ gem_storedw_loop_vebox \ gem_threaded_access_tiled \ gem_tiled_fence_blits \ gem_tiled_pread \ gem_tiled_pread_pwrite \ gem_tiled_swapping \ gem_tiling_max_stride \ gem_unfence_active_buffers \ gem_unref_active_buffers \ gem_wait_render_timeout \ + gem_workarounds \ gen3_mixed_blits \ gen3_render_linear_blits \ gen3_render_mixed_blits \ gen3_render_tiledx_blits \ gen3_render_tiledy_blits \ gen7_forcewake_mt \ kms_force_connector \ kms_sink_crc_basic \ kms_fence_pin_leak \ pm_psr \ diff --git a/tests/gem_workarounds.c b/tests/gem_workarounds.c new file mode 100644 index 000..56bf4b1 --- /dev/null +++ b/tests/gem_workarounds.c @@ -0,0 +1,238 @@ +/* + * Copyright © 2014 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the Software), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + * + * Authors: + * Arun Siluvery arun.siluv...@linux.intel.com + * + */ + +#define _GNU_SOURCE +#include stdbool.h +#include unistd.h +#include stdlib.h +#include stdio.h +#include string.h +#include fcntl.h +#include inttypes.h +#include errno.h +#include sys/stat.h +#include sys/ioctl.h +#include sys/mman.h +#include time.h +#include signal.h + +#include ioctl_wrappers.h +#include drmtest.h +#include igt_debugfs.h +#include igt_aux.h +#include intel_chipset.h +#include intel_io.h + +enum operation { + GPU_RESET = 0x01, + SUSPEND_RESUME = 0x02, +}; + +struct intel_wa_reg { + uint32_t addr; + uint32_t value; + uint32_t mask; +}; + +int drm_fd; +uint32_t devid; +static drm_intel_bufmgr *bufmgr; +struct intel_batchbuffer *batch; +int num_wa; +struct intel_wa_reg *wa_regs; + + +static void test_hang_gpu(void) +{ + int retry_count = 30; + enum stop_ring_flags flags; + struct drm_i915_gem_execbuffer2 execbuf; + struct drm_i915_gem_exec_object2 gem_exec; + uint32_t b[2] = {MI_BATCH_BUFFER_END}; + + igt_assert(retry_count); + igt_set_stop_rings(STOP_RING_DEFAULTS); + + memset(gem_exec, 0, sizeof(gem_exec)); + gem_exec.handle = gem_create(drm_fd, 4096); + gem_write(drm_fd, gem_exec.handle, 0, b, sizeof(b)); + + memset(execbuf, 0, sizeof(execbuf)); + execbuf.buffers_ptr = (uintptr_t)gem_exec; + execbuf.buffer_count = 1; + execbuf.batch_len = sizeof(b); + + drmIoctl(drm_fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, execbuf); + + while(retry_count--) { + flags = igt_get_stop_rings(); + if (flags == 0) + break; + printf(gpu hang not yet cleared, retries left %d\n, retry_count); + sleep(1); + } + + flags = igt_get_stop_rings(); + if (flags) + igt_set_stop_rings(STOP_RING_NONE); +} + +static void test_suspend_resume(void) +{ + printf(Suspending the device ...\n); + igt_system_suspend_autoresume(); +} + +static void get_current_wa_data(struct intel_wa_reg **curr, int num) +{ + int i; +
Re: [Intel-gfx] [PATCH 1/2] drm/i915/bdw: Apply workarounds using the golden render state
On 26/08/2014 13:53, Daniel Vetter wrote: On Fri, Aug 22, 2014 at 01:10:26PM +0100, Siluvery, Arun wrote: On 22/08/2014 12:06, Mika Kuoppala wrote: Ville Syrjälä ville.syrj...@linux.intel.com writes: On Wed, Aug 20, 2014 at 03:19:17PM +0100, Arun Siluvery wrote: Workarounds for bdw are currently applied in init_clock_gating() but they are lost following a gpu reset. Some of the WA registers are part of register state context and they are restored with every context switch so initializing them in golden render state ensures that they are applied even when we start with an uninitialized context or during hw initlialization followed by a reset. v2: Add comments corresponding to WAs in golden render state (Chris). The generation of render state is not a straighforward process, it would be ideal to augment WA values from during the setup state as opposed to using a tool but that would be a follow up patch. I'd still prefer just emitting the LRIs from code rather tha mucking about with null batch. Less hoops to jump through when adding a new w/a. I agree with this. We should aim to keep null state as per gen. Workaround set is different for gtX inside particular gen so we would need then multiple null states per gen. After brief chat with Ville, I think that the correct spot to init the context specific workarounds is after MI_SET_CONTEXT to default and right before null batch is run. If we do these with emitting LRIs to ring, we should be safe as they are then saved with default ctx. The default ctx is then used as a 'parent' for newly created contexts. Ofcource if registers get globbered, then we inherit crap. If we have the per gen null state and the ring is initializing workarounds for the default context, then in future we can save this state as 'read only golden context'. And use it as the initial state for all newly created contexts. Then the full plan how to init would look like this: #1 reset the gpu (on driver load, on resume or on hang recovery) #2 if we have 'read only golden context', copy it to default ctx #3 switch to default context #4 if we had 'read only golden context' we are done with the init. --- #5 if this is driver load thus there is no 'read only golden context' yet. #6 init workarounds through ring LRIs #7 run null/golden state batch #8 save this state as a 'read only golden context' --- #9 for each new context, initialize ctx obj with 'read only golden context' (either by memcpy or restoring from it when switching to new) I understand applying WAs using null batch has its issues but as I mentioned in the commit msg I will fix this as a follow up patch. It is going to take some time though to change the patch as per the new sequence. The patch in its current state helps fix WA issues after reset; so it can only be accepted if it is updated as per the new sequence? We already have a lot of let's fix it later experiments running, so I don't want to overload the ship. So I highly prefer to merge the revised version directly. -Daniel I understand, a revised version with LRIs emitting from the driver is already submitted and is being reviewed. regards Arun ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/14] drm/i915: Reorganize vlv eDP reboot notifier
On Tue, Aug 19, 2014 at 10:00:55AM +0300, Jani Nikula wrote: On Mon, 18 Aug 2014, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com Move the vlv_power_sequencer_pipe() after the IS_VALLEYVIEW() check and flatten the rest of the function. Please imagine adding another platform there, and realize this just adds unnecessary churn. I'd just add another reboot notifier then. Frankly I don't understand the current one either. Why does it need to set the delay to max for instance? And does this mean that the PANEL_POWER_RESET bit doesn't actually work as advertised in the docs? BR, Jani. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_dp.c | 24 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 43dd226..a9ed2a6 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -347,22 +347,22 @@ static int edp_notify_handler(struct notifier_block *this, unsigned long code, struct drm_i915_private *dev_priv = dev-dev_private; u32 pp_div; u32 pp_ctrl_reg, pp_div_reg; - enum pipe pipe = vlv_power_sequencer_pipe(intel_dp); + enum pipe pipe; - if (!is_edp(intel_dp) || code != SYS_RESTART) + if (!IS_VALLEYVIEW(dev) || !is_edp(intel_dp) || code != SYS_RESTART) return 0; - if (IS_VALLEYVIEW(dev)) { - pp_ctrl_reg = VLV_PIPE_PP_CONTROL(pipe); - pp_div_reg = VLV_PIPE_PP_DIVISOR(pipe); - pp_div = I915_READ(pp_div_reg); - pp_div = PP_REFERENCE_DIVIDER_MASK; + pipe = vlv_power_sequencer_pipe(intel_dp); - /* 0x1F write to PP_DIV_REG sets max cycle delay */ - I915_WRITE(pp_div_reg, pp_div | 0x1F); - I915_WRITE(pp_ctrl_reg, PANEL_UNLOCK_REGS | PANEL_POWER_OFF); - msleep(intel_dp-panel_power_cycle_delay); - } + pp_ctrl_reg = VLV_PIPE_PP_CONTROL(pipe); + pp_div_reg = VLV_PIPE_PP_DIVISOR(pipe); + pp_div = I915_READ(pp_div_reg); + pp_div = PP_REFERENCE_DIVIDER_MASK; + + /* 0x1F write to PP_DIV_REG sets max cycle delay */ + I915_WRITE(pp_div_reg, pp_div | 0x1F); + I915_WRITE(pp_ctrl_reg, PANEL_UNLOCK_REGS | PANEL_POWER_OFF); + msleep(intel_dp-panel_power_cycle_delay); return 0; } -- 1.8.5.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center -- 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 1/2] drm/i915: reorganize the unclaimed register detection code
2014-08-26 9:42 GMT-03:00 Chris Wilson ch...@chris-wilson.co.uk: On Tue, Aug 26, 2014 at 09:17:11AM -0300, Paulo Zanoni wrote: 2014-08-26 7:22 GMT-03:00 Chris Wilson ch...@chris-wilson.co.uk: On Wed, Jul 16, 2014 at 05:49:29PM -0300, Paulo Zanoni wrote: static void -hsw_unclaimed_reg_check(struct drm_i915_private *dev_priv, u32 reg) +hsw_unclaimed_reg_detect(struct drm_i915_private *dev_priv) { + if (i915.mmio_debug) + return; + if (__raw_i915_read32(dev_priv, FPGA_DBG) FPGA_DBG_RM_NOCLAIM) { - DRM_ERROR(Unclaimed write to %x\n, reg); + DRM_ERROR(Unclaimed register detected. Please use the i915.mmio_debug=1 to debug this problem.); __raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM); } What was the point here? You still add an extra read to every register write Well, we previously had 2 extra reads instead of 1, so with this patch we're in a better position :) and then repeat the request to enable mmio_debug ad infinitum. Yeah, this could be avoided. OTOH, on most cases it's not gonna happen frequently enough to annoy the user. How do you think I came across this? And you still check for illegal writes in the irq handler. That just happens on HSW, not BDW+. Even on hsw, it should be killed. Checking inside the irq handler is just insane. Just move it to one of the periodic checks since we can't get any more information than an error occurred, and ask to be re-run with mmio_debug (and then shut up) - heck you could even automatically enable it for a one-shot operation. Yeah, looking at how the IRQ handler situation is _today_, I agree it doesn't really make too much sense. I know it was different in the past, so I wonder how we ended up reaching this point :) Anyway, if we just remove the call to intel_uncore_check_errors() that happens on the irq handler, we'll still end up checking for errors as soon as we I915_WRITE(DEIER), so that won't be too much helpful. One thing we could do would be to remove the check from the I915_WRITE macros and put it on a real periodic check that could be executed at other specific points of our code (less frequent than every i915_write), or put it at its own workqueue that gets run every X jiffies. Or perhaps change the implementation of hsw_unclaimed_reg_detect() to not do anything when we're in an interrupt/spinlock context. Which one do you think is better to do? Of course, we can also implement the one-shot thing on top of the above, but it won't really help us reducing the amount of reads on the happy case where we never got the error before. All I have to say in my defense is that I did ask you to look at this patch before it was merged :) Just kill this code. If we do it, we won't be checking for unclaimed registers on BDW+ without i915.mmio_debug=1. Then do as above. -Chris -- Chris Wilson, Intel Open Source Technology Centre -- Paulo Zanoni ___ 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: reorganize the unclaimed register detection code
On Tue, Aug 26, 2014 at 10:04:22AM -0300, Paulo Zanoni wrote: 2014-08-26 9:42 GMT-03:00 Chris Wilson ch...@chris-wilson.co.uk: On Tue, Aug 26, 2014 at 09:17:11AM -0300, Paulo Zanoni wrote: 2014-08-26 7:22 GMT-03:00 Chris Wilson ch...@chris-wilson.co.uk: On Wed, Jul 16, 2014 at 05:49:29PM -0300, Paulo Zanoni wrote: static void -hsw_unclaimed_reg_check(struct drm_i915_private *dev_priv, u32 reg) +hsw_unclaimed_reg_detect(struct drm_i915_private *dev_priv) { + if (i915.mmio_debug) + return; + if (__raw_i915_read32(dev_priv, FPGA_DBG) FPGA_DBG_RM_NOCLAIM) { - DRM_ERROR(Unclaimed write to %x\n, reg); + DRM_ERROR(Unclaimed register detected. Please use the i915.mmio_debug=1 to debug this problem.); __raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM); } What was the point here? You still add an extra read to every register write Well, we previously had 2 extra reads instead of 1, so with this patch we're in a better position :) and then repeat the request to enable mmio_debug ad infinitum. Yeah, this could be avoided. OTOH, on most cases it's not gonna happen frequently enough to annoy the user. How do you think I came across this? And you still check for illegal writes in the irq handler. That just happens on HSW, not BDW+. Even on hsw, it should be killed. Checking inside the irq handler is just insane. Just move it to one of the periodic checks since we can't get any more information than an error occurred, and ask to be re-run with mmio_debug (and then shut up) - heck you could even automatically enable it for a one-shot operation. Yeah, looking at how the IRQ handler situation is _today_, I agree it doesn't really make too much sense. I know it was different in the past, so I wonder how we ended up reaching this point :) Anyway, if we just remove the call to intel_uncore_check_errors() that happens on the irq handler, we'll still end up checking for errors as soon as we I915_WRITE(DEIER), so that won't be too much helpful. One thing we could do would be to remove the check from the I915_WRITE macros and put it on a real periodic check that could be executed at other specific points of our code (less frequent than every i915_write), or put it at its own workqueue that gets run every X jiffies. Or perhaps change the implementation of hsw_unclaimed_reg_detect() to not do anything when we're in an interrupt/spinlock context. Which one do you think is better to do? From the irq handler, we can use just use raw mmio interfaces and skip all the debug and forcewake. I think the solution I prefer is to instrument modesetting (say check_state) and warn if an error had occurred outside of mmio_debug. Of course, we can also implement the one-shot thing on top of the above, but it won't really help us reducing the amount of reads on the happy case where we never got the error before. Actually I am tempted to dynamically patch the mmio vfuncs to avoid even the forcewake spinlock when we already hold it. So there won't be any such logic except when enabled by the user. -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 02/14] drm/i915: Reorganize vlv eDP reboot notifier
On Tue, 26 Aug 2014, Ville Syrjälä ville.syrj...@linux.intel.com wrote: On Tue, Aug 19, 2014 at 10:00:55AM +0300, Jani Nikula wrote: On Mon, 18 Aug 2014, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com Move the vlv_power_sequencer_pipe() after the IS_VALLEYVIEW() check and flatten the rest of the function. Please imagine adding another platform there, and realize this just adds unnecessary churn. I'd just add another reboot notifier then. Fair enough; it should be vlv_edp_notify_handler then. (No, don't send a patch to change that! ;) Frankly I don't understand the current one either. Why does it need to set the delay to max for instance? And does this mean that the PANEL_POWER_RESET bit doesn't actually work as advertised in the docs? *shrug* experimental evidence? commit 01527b3127997ef6370d5ad4fa25d96847fbf12a Author: Clint Taylor clinton.a.tay...@intel.com Date: Mon Jul 7 13:01:46 2014 -0700 drm/i915/vlv: T12 eDP panel timing enforcement during reboot The panel power sequencer on vlv doesn't appear to accept changes to its T12 power down duration during warm reboots. This change forces a delay for warm reboots to the T12 panel timing as defined in the VBT table for the connected panel. BR, Jani. Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_dp.c | 24 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 43dd226..a9ed2a6 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -347,22 +347,22 @@ static int edp_notify_handler(struct notifier_block *this, unsigned long code, struct drm_i915_private *dev_priv = dev-dev_private; u32 pp_div; u32 pp_ctrl_reg, pp_div_reg; - enum pipe pipe = vlv_power_sequencer_pipe(intel_dp); + enum pipe pipe; - if (!is_edp(intel_dp) || code != SYS_RESTART) + if (!IS_VALLEYVIEW(dev) || !is_edp(intel_dp) || code != SYS_RESTART) return 0; - if (IS_VALLEYVIEW(dev)) { - pp_ctrl_reg = VLV_PIPE_PP_CONTROL(pipe); - pp_div_reg = VLV_PIPE_PP_DIVISOR(pipe); - pp_div = I915_READ(pp_div_reg); - pp_div = PP_REFERENCE_DIVIDER_MASK; + pipe = vlv_power_sequencer_pipe(intel_dp); - /* 0x1F write to PP_DIV_REG sets max cycle delay */ - I915_WRITE(pp_div_reg, pp_div | 0x1F); - I915_WRITE(pp_ctrl_reg, PANEL_UNLOCK_REGS | PANEL_POWER_OFF); - msleep(intel_dp-panel_power_cycle_delay); - } + pp_ctrl_reg = VLV_PIPE_PP_CONTROL(pipe); + pp_div_reg = VLV_PIPE_PP_DIVISOR(pipe); + pp_div = I915_READ(pp_div_reg); + pp_div = PP_REFERENCE_DIVIDER_MASK; + + /* 0x1F write to PP_DIV_REG sets max cycle delay */ + I915_WRITE(pp_div_reg, pp_div | 0x1F); + I915_WRITE(pp_ctrl_reg, PANEL_UNLOCK_REGS | PANEL_POWER_OFF); + msleep(intel_dp-panel_power_cycle_delay); return 0; } -- 1.8.5.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Jani Nikula, Intel Open Source Technology Center -- Ville Syrjälä Intel OTC -- 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/bdw: Do not initialize PPGTT in the legacy way for execlists
On Wed, Aug 20, 2014 at 04:24:50PM +0100, Thomas Daniel wrote: A pending commit removes synchronous mode from switch_mm. This breaks execlists because switch_mm will always try to write to the legacy ring buffer. Return immediately from i915_ppgtt_init_gw in execlists mode. No longer check for execlists mode in gen8_ppgtt_enable() because this will no longer be called in execlists mode. Signed-off-by: Thomas Daniel thomas.dan...@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
Re: [Intel-gfx] [PATCH 1/2] drm/i915: reorganize the unclaimed register detection code
2014-08-26 10:18 GMT-03:00 Chris Wilson ch...@chris-wilson.co.uk: On Tue, Aug 26, 2014 at 10:04:22AM -0300, Paulo Zanoni wrote: 2014-08-26 9:42 GMT-03:00 Chris Wilson ch...@chris-wilson.co.uk: On Tue, Aug 26, 2014 at 09:17:11AM -0300, Paulo Zanoni wrote: 2014-08-26 7:22 GMT-03:00 Chris Wilson ch...@chris-wilson.co.uk: On Wed, Jul 16, 2014 at 05:49:29PM -0300, Paulo Zanoni wrote: static void -hsw_unclaimed_reg_check(struct drm_i915_private *dev_priv, u32 reg) +hsw_unclaimed_reg_detect(struct drm_i915_private *dev_priv) { + if (i915.mmio_debug) + return; + if (__raw_i915_read32(dev_priv, FPGA_DBG) FPGA_DBG_RM_NOCLAIM) { - DRM_ERROR(Unclaimed write to %x\n, reg); + DRM_ERROR(Unclaimed register detected. Please use the i915.mmio_debug=1 to debug this problem.); __raw_i915_write32(dev_priv, FPGA_DBG, FPGA_DBG_RM_NOCLAIM); } What was the point here? You still add an extra read to every register write Well, we previously had 2 extra reads instead of 1, so with this patch we're in a better position :) and then repeat the request to enable mmio_debug ad infinitum. Yeah, this could be avoided. OTOH, on most cases it's not gonna happen frequently enough to annoy the user. How do you think I came across this? And you still check for illegal writes in the irq handler. That just happens on HSW, not BDW+. Even on hsw, it should be killed. Checking inside the irq handler is just insane. Just move it to one of the periodic checks since we can't get any more information than an error occurred, and ask to be re-run with mmio_debug (and then shut up) - heck you could even automatically enable it for a one-shot operation. Yeah, looking at how the IRQ handler situation is _today_, I agree it doesn't really make too much sense. I know it was different in the past, so I wonder how we ended up reaching this point :) Anyway, if we just remove the call to intel_uncore_check_errors() that happens on the irq handler, we'll still end up checking for errors as soon as we I915_WRITE(DEIER), so that won't be too much helpful. One thing we could do would be to remove the check from the I915_WRITE macros and put it on a real periodic check that could be executed at other specific points of our code (less frequent than every i915_write), or put it at its own workqueue that gets run every X jiffies. Or perhaps change the implementation of hsw_unclaimed_reg_detect() to not do anything when we're in an interrupt/spinlock context. Which one do you think is better to do? From the irq handler, we can use just use raw mmio interfaces and skip all the debug and forcewake. I think the solution I prefer is to instrument modesetting (say check_state) and warn if an error had occurred outside of mmio_debug. My only problem with checking at modesetting is that we often spend hours and hours without doing a modeset, so it could lead to problems not ever being detected. So maybe there's a better place, but if that's what we want I won't block any patches. Of course, we can also implement the one-shot thing on top of the above, but it won't really help us reducing the amount of reads on the happy case where we never got the error before. Actually I am tempted to dynamically patch the mmio vfuncs to avoid even the forcewake spinlock when we already hold it. So there won't be any such logic except when enabled by the user. Should I expect a patch from you, or should I go and write the patch based on what we already discussed? -Chris -- Chris Wilson, Intel Open Source Technology Centre -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/bdw: Add BDW support in the i915 debugfs entry
On Wed, Aug 20, 2014 at 12:04:31PM -0700, vedang.pa...@intel.com wrote: From: Vedang Patel vedang.pa...@intel.com The patch introduces fixes for the debugfs attributes emitted by the i915 driver for GEN8. Currently, it is not emitting the correct attributes which include the status of RC6 states. Change-Id: Ib2068a0cac9a5wq3f228e547fa1a097ad369d242df Signed-off-by: Vedang Patel vedang.pa...@intel.com --- drivers/gpu/drm/i915/i915_debugfs.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 850fa59..2296a22 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1329,7 +1329,7 @@ static int i915_drpc_info(struct seq_file *m, void *unused) if (IS_VALLEYVIEW(dev)) return vlv_drpc_info(m); - else if (IS_GEN6(dev) || IS_GEN7(dev)) + else if (IS_GEN6(dev) || IS_GEN7(dev) || IS_GEN8(dev)) I think an INTEL_INFO(dev)-gen = 6 check here looks better and is more future proof. Can you please update the patch accordingly? Thanks, Daniel return gen6_drpc_info(m); else return ironlake_drpc_info(m); -- 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 02/14] drm/i915: Reorganize vlv eDP reboot notifier
On Tue, Aug 26, 2014 at 04:21:00PM +0300, Jani Nikula wrote: On Tue, 26 Aug 2014, Ville Syrjälä ville.syrj...@linux.intel.com wrote: On Tue, Aug 19, 2014 at 10:00:55AM +0300, Jani Nikula wrote: On Mon, 18 Aug 2014, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com Move the vlv_power_sequencer_pipe() after the IS_VALLEYVIEW() check and flatten the rest of the function. Please imagine adding another platform there, and realize this just adds unnecessary churn. I'd just add another reboot notifier then. Fair enough; it should be vlv_edp_notify_handler then. (No, don't send a patch to change that! ;) Frankly I don't understand the current one either. Why does it need to set the delay to max for instance? And does this mean that the PANEL_POWER_RESET bit doesn't actually work as advertised in the docs? *shrug* experimental evidence? commit 01527b3127997ef6370d5ad4fa25d96847fbf12a Author: Clint Taylor clinton.a.tay...@intel.com Date: Mon Jul 7 13:01:46 2014 -0700 drm/i915/vlv: T12 eDP panel timing enforcement during reboot The panel power sequencer on vlv doesn't appear to accept changes to its T12 power down duration during warm reboots. This change forces a delay for warm reboots to the T12 panel timing as defined in the VBT table for the connected panel. That explanation doesn't really make it any more clear to me. But if the reboot notifier helps someone somehow I can live with it. -- 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 1/2] drm/i915: reorganize the unclaimed register detection code
On Tue, Aug 26, 2014 at 10:29:23AM -0300, Paulo Zanoni wrote: 2014-08-26 10:18 GMT-03:00 Chris Wilson ch...@chris-wilson.co.uk: On Tue, Aug 26, 2014 at 10:04:22AM -0300, Paulo Zanoni wrote: Of course, we can also implement the one-shot thing on top of the above, but it won't really help us reducing the amount of reads on the happy case where we never got the error before. Actually I am tempted to dynamically patch the mmio vfuncs to avoid even the forcewake spinlock when we already hold it. So there won't be any such logic except when enabled by the user. Should I expect a patch from you, or should I go and write the patch based on what we already discussed? Imo this is crazy - we have no control over what the compiler does and when exactly it loads vtable entries, so patching them at runtime would be an interesting excercise at best. Adding a special version of I915_READ/WRITE for the irq hotpath makes sense, but only if we can actually show a benefit in benchmarks. -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 02/14] drm/i915: Reorganize vlv eDP reboot notifier
On Tue, Aug 26, 2014 at 04:21:00PM +0300, Jani Nikula wrote: On Tue, 26 Aug 2014, Ville Syrjälä ville.syrj...@linux.intel.com wrote: On Tue, Aug 19, 2014 at 10:00:55AM +0300, Jani Nikula wrote: On Mon, 18 Aug 2014, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com Move the vlv_power_sequencer_pipe() after the IS_VALLEYVIEW() check and flatten the rest of the function. Please imagine adding another platform there, and realize this just adds unnecessary churn. I'd just add another reboot notifier then. Fair enough; it should be vlv_edp_notify_handler then. (No, don't send a patch to change that! ;) Frankly I don't understand the current one either. Why does it need to set the delay to max for instance? And does this mean that the PANEL_POWER_RESET bit doesn't actually work as advertised in the docs? *shrug* experimental evidence? commit 01527b3127997ef6370d5ad4fa25d96847fbf12a Author: Clint Taylor clinton.a.tay...@intel.com Date: Mon Jul 7 13:01:46 2014 -0700 drm/i915/vlv: T12 eDP panel timing enforcement during reboot The panel power sequencer on vlv doesn't appear to accept changes to its T12 power down duration during warm reboots. This change forces a delay for warm reboots to the T12 panel timing as defined in the VBT table for the connected panel. So if I remember this piece of lore correctly in the past the pp was pessimistic, and enforced this delay on resume/boot-up, assuming you've shut down _right_ before the machine was lit up again. Apparently people where unhappy with that enforced delay and it was ditched on vlv, but then it broke panels if you actually managed to reboot quickly enough. -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/bdw: Apply workarounds using the golden render state
On Tue, Aug 26, 2014 at 01:57:19PM +0100, Siluvery, Arun wrote: On 26/08/2014 13:53, Daniel Vetter wrote: On Fri, Aug 22, 2014 at 01:10:26PM +0100, Siluvery, Arun wrote: On 22/08/2014 12:06, Mika Kuoppala wrote: Ville Syrjälä ville.syrj...@linux.intel.com writes: On Wed, Aug 20, 2014 at 03:19:17PM +0100, Arun Siluvery wrote: Workarounds for bdw are currently applied in init_clock_gating() but they are lost following a gpu reset. Some of the WA registers are part of register state context and they are restored with every context switch so initializing them in golden render state ensures that they are applied even when we start with an uninitialized context or during hw initlialization followed by a reset. v2: Add comments corresponding to WAs in golden render state (Chris). The generation of render state is not a straighforward process, it would be ideal to augment WA values from during the setup state as opposed to using a tool but that would be a follow up patch. I'd still prefer just emitting the LRIs from code rather tha mucking about with null batch. Less hoops to jump through when adding a new w/a. I agree with this. We should aim to keep null state as per gen. Workaround set is different for gtX inside particular gen so we would need then multiple null states per gen. After brief chat with Ville, I think that the correct spot to init the context specific workarounds is after MI_SET_CONTEXT to default and right before null batch is run. If we do these with emitting LRIs to ring, we should be safe as they are then saved with default ctx. The default ctx is then used as a 'parent' for newly created contexts. Ofcource if registers get globbered, then we inherit crap. If we have the per gen null state and the ring is initializing workarounds for the default context, then in future we can save this state as 'read only golden context'. And use it as the initial state for all newly created contexts. Then the full plan how to init would look like this: #1 reset the gpu (on driver load, on resume or on hang recovery) #2 if we have 'read only golden context', copy it to default ctx #3 switch to default context #4 if we had 'read only golden context' we are done with the init. --- #5 if this is driver load thus there is no 'read only golden context' yet. #6 init workarounds through ring LRIs #7 run null/golden state batch #8 save this state as a 'read only golden context' --- #9 for each new context, initialize ctx obj with 'read only golden context' (either by memcpy or restoring from it when switching to new) I understand applying WAs using null batch has its issues but as I mentioned in the commit msg I will fix this as a follow up patch. It is going to take some time though to change the patch as per the new sequence. The patch in its current state helps fix WA issues after reset; so it can only be accepted if it is updated as per the new sequence? We already have a lot of let's fix it later experiments running, so I don't want to overload the ship. So I highly prefer to merge the revised version directly. -Daniel I understand, a revised version with LRIs emitting from the driver is already submitted and is being reviewed. Ah, still catching up from my unusable network connection from last week. Please ignore me ;-) -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] i915: Regression: +4W in idle power use on Macbook Pro 15 (late 2013)
On Tue, Aug 26, 2014 at 4:06 AM, Ville Syrjälä ville.syrj...@linux.intel.com wrote: FBC works best when the screen contents don't change. The more activity on the screen the less effective FBC becomes. 4W sounds way too much for FBC however. 0.4W is closer to what one might expect from FBC based on my observations. 4W sounds more like the difference between min vs. max display brightness to me. That's why I'm here, because it is such a dramatic change. All measurements were taken at constant brightness (4/100) and the difference is indeed a full 4W, as reported by the battery (and seen in: powertop(1), /sys/bus/acpi/drivers/battery/PNP0C0A\:00/power_supply/BAT0/power_now, and a noticeably faster drop in battery charge over time). Could the FBC commit somehow also disable RC6, or similar? ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t 3/3] tests: add kms_3d test
On Wed, Aug 20, 2014 at 11:54:09AM +0100, Thomas Wood wrote: Add a test to verify creation and use of 3D stereo modes. Signed-off-by: Thomas Wood thomas.w...@intel.com --- lib/igt_fb.c | 4 +- tests/.gitignore | 1 + tests/Android.mk | 1 + tests/Makefile.sources | 1 + tests/kms_3d.c | 118 + 5 files changed, 123 insertions(+), 2 deletions(-) create mode 100644 tests/kms_3d.c diff --git a/lib/igt_fb.c b/lib/igt_fb.c index 9a13969..0096836 100644 --- a/lib/igt_fb.c +++ b/lib/igt_fb.c @@ -614,10 +614,10 @@ unsigned int igt_create_stereo_fb(int drm_fd, drmModeModeInfo *mode) enable_tiling, fb); cr = igt_get_cairo_ctx(drm_fd, fb); - igt_paint_image(cr, IGT_DATADIR1080p-left.png, + igt_paint_image(cr, IGT_DATADIR/1080p-left.png, layout.left.x, layout.left.y, layout.left.width, layout.left.height); - igt_paint_image(cr, IGT_DATADIR1080p-right.png, + igt_paint_image(cr, IGT_DATADIR/1080p-right.png, layout.right.x, layout.right.y, layout.right.width, layout.right.height); I guess this hunk really belongs to the previous commit, but for i-g-t, whatever really. diff --git a/tests/.gitignore b/tests/.gitignore index 3da061e..9b5cf7d 100644 --- a/tests/.gitignore +++ b/tests/.gitignore @@ -116,6 +116,7 @@ igt_no_exit igt_no_exit_list_only igt_no_subtest igt_simulation +kms_3d kms_addfb kms_cursor_crc kms_fbc_crc diff --git a/tests/Android.mk b/tests/Android.mk index 3644aa1..f28b400 100644 --- a/tests/Android.mk +++ b/tests/Android.mk @@ -55,6 +55,7 @@ ifeq (${ANDROID_HAS_CAIRO}, 1) else # the following tests depend on cairo, so skip them skip_tests_list += \ +kms_3d \ kms_plane \ kms_addfb \ kms_cursor_crc \ diff --git a/tests/Makefile.sources b/tests/Makefile.sources index 698e290..078df57 100644 --- a/tests/Makefile.sources +++ b/tests/Makefile.sources @@ -141,6 +141,7 @@ TESTS_progs = \ gen3_render_tiledx_blits \ gen3_render_tiledy_blits \ gen7_forcewake_mt \ + kms_3d \ kms_force_connector \ kms_sink_crc_basic \ kms_fence_pin_leak \ diff --git a/tests/kms_3d.c b/tests/kms_3d.c new file mode 100644 index 000..c593f34 --- /dev/null +++ b/tests/kms_3d.c @@ -0,0 +1,118 @@ +/* + * Copyright © 2014 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the Software), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + * + */ + +#include igt_core.h +#include igt_kms.h +#include drmtest.h +#include igt_edid.h + +igt_simple_main +{ + int drm_fd; + drmModeRes *res; + drmModeConnector *connector; + unsigned char *edid; + size_t length; + int mode_count, connector_id; + + drm_fd = drm_open_any(); + res = drmModeGetResources(drm_fd); + + igt_assert(drmSetClientCap(drm_fd, DRM_CLIENT_CAP_STEREO_3D, 1) = 0); + + /* find an hdmi connector */ + for (int i = 0; i res-count_connectors; i++) { + + connector = drmModeGetConnector(drm_fd, res-connectors[i]); + + if (connector-connector_type == DRM_MODE_CONNECTOR_HDMIA + connector-connection == DRM_MODE_DISCONNECTED) + break; + + drmModeFreeConnector(connector); + + connector = NULL; + } + igt_require(connector); + + kmstest_edid_add_3d(generic_edid[EDID_FHD], EDID_LENGTH, edid, + length); + + kmstest_force_edid(drm_fd, connector, edid, length); + kmstest_force_connector(drm_fd, connector, FORCE_CONNECTOR_ON); + + connector_id = connector-connector_id; + + /* check for 3D modes */ + mode_count = 0; + connector =
Re: [Intel-gfx] [PATCH i-g-t 1/3] lib: add kmstest_edid_add_3d
On Wed, Aug 20, 2014 at 11:54:07AM +0100, Thomas Wood wrote: kmstest_edid_add_3d adds an EDID extension block with 3D support to a copy of the specified EDID. Signed-off-by: Thomas Wood thomas.w...@intel.com The series looks reasonable, ship it! -- Damien --- lib/igt_kms.c | 80 +++ lib/igt_kms.h | 1 + 2 files changed, 81 insertions(+) diff --git a/lib/igt_kms.c b/lib/igt_kms.c index a414d96..eb898f8 100644 --- a/lib/igt_kms.c +++ b/lib/igt_kms.c @@ -657,6 +657,86 @@ kmstest_get_property(int drm_fd, uint32_t object_id, uint32_t object_type, } /** + * kmstest_edid_add_3d: + * @edid: an existing valid edid block + * @length: length of @edid + * @new_edid_ptr: pointer to where the new edid will be placed + * @new_length: pointer to the size of the new edid + * + * Makes a copy of an existing edid block and adds an extension indicating + * stereo 3D capabilities. + */ +void kmstest_edid_add_3d(const unsigned char *edid, size_t length, + unsigned char *new_edid_ptr[], size_t *new_length) +{ + unsigned char *new_edid; + int n_extensions; + char sum = 0; + int pos; + int i; + char cea_header_len = 4, video_block_len = 6, vsdb_block_len = 11; + + igt_assert(new_edid_ptr != NULL new_length != NULL); + + *new_length = length + 128; + + new_edid = calloc(*new_length, sizeof(char)); + memcpy(new_edid, edid, length); + *new_edid_ptr = new_edid; + + n_extensions = new_edid[126]; + n_extensions++; + new_edid[126] = n_extensions; + + /* recompute checksum */ + for (i = 0; i 127; i++) { + sum = sum + new_edid[i]; + } + new_edid[127] = 256 - sum; + + /* add a cea-861 extension block */ + pos = length; + new_edid[pos++] = 0x2; + new_edid[pos++] = 0x3; + new_edid[pos++] = cea_header_len + video_block_len + vsdb_block_len; + new_edid[pos++] = 0x0; + + /* video block (id | length) */ + new_edid[pos++] = 2 5 | (video_block_len - 1); + new_edid[pos++] = 32 | 0x80; /* 1080p @ 24Hz | (native)*/ + new_edid[pos++] = 5; /* 1080i @ 60Hz */ + new_edid[pos++] = 20;/* 1080i @ 50Hz */ + new_edid[pos++] = 4; /* 720p @ 60Hz*/ + new_edid[pos++] = 19;/* 720p @ 50Hz*/ + + /* vsdb block ( id | length ) */ + new_edid[pos++] = 3 5 | (vsdb_block_len - 1); + /* registration id */ + new_edid[pos++] = 0x3; + new_edid[pos++] = 0xc; + new_edid[pos++] = 0x0; + /* source physical address */ + new_edid[pos++] = 0x0; + new_edid[pos++] = 0x0; + /* Supports_AI ... etc */ + new_edid[pos++] = 0x00; + /* Max TMDS Clock */ + new_edid[pos++] = 0x00; + /* Latency present, HDMI Video Present */ + new_edid[pos++] = 0x20; + /* HDMI Video */ + new_edid[pos++] = 0x80; + new_edid[pos++] = 0x00; + + /* checksum */ + sum = 0; + for (i = 0; i 127; i++) { + sum = sum + new_edid[length + i]; + } + new_edid[length + 127] = 256 - sum; +} + +/** * kmstest_unset_all_crtcs: * @drm_fd: the DRM fd * @resources: libdrm resources pointer diff --git a/lib/igt_kms.h b/lib/igt_kms.h index 4263a01..921afef 100644 --- a/lib/igt_kms.h +++ b/lib/igt_kms.h @@ -149,6 +149,7 @@ enum kmstest_generic_edid { bool kmstest_force_connector(int fd, drmModeConnector *connector, enum kmstest_force_connector_state state); +void kmstest_edid_add_3d(const unsigned char *edid, size_t length, unsigned char *new_edid_ptr[], size_t *new_length); void kmstest_force_edid(int drm_fd, drmModeConnector *connector, const unsigned char *edid, size_t length); -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915/bdw: Apply workarounds in render ring init function
For BDW workarounds are currently initialized in init_clock_gating() but they are lost during reset, suspend/resume etc; this patch moves the WAs that are part of register state context to render ring init fn otherwise default context ends up with incorrect values as they don't get initialized until init_clock_gating fn. v2: Add workarounds to golden render state This method has its own issues, first of all this is different for each gen and it is generated using a tool so adding new workaround and mainitaining them across gens is not a straightforward process. v3: Use LRIs to emit these workarounds (Ville) Instead of modifying the golden render state the same LRIs are emitted from within the driver. v4: Use abstract name when exporting gen specific routines (Chris) For: VIZ-4092 Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com --- drivers/gpu/drm/i915/i915_gem_context.c | 6 +++ drivers/gpu/drm/i915/intel_pm.c | 48 drivers/gpu/drm/i915/intel_ringbuffer.c | 79 + drivers/gpu/drm/i915/intel_ringbuffer.h | 2 + 4 files changed, 87 insertions(+), 48 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 9683e62..0a9bb0e 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -631,20 +631,26 @@ static int do_switch(struct intel_engine_cs *ring, } uninitialized = !to-legacy_hw_ctx.initialized from == NULL; to-legacy_hw_ctx.initialized = true; done: i915_gem_context_reference(to); ring-last_context = to; if (uninitialized) { + if (ring-init_context) { + ret = ring-init_context(ring); + if (ret) + DRM_ERROR(ring init context: %d\n, ret); + } + ret = i915_gem_render_state_init(ring); if (ret) DRM_ERROR(init render state: %d\n, ret); } return 0; unpin_out: if (ring-id == RCS) i915_gem_object_ggtt_unpin(to-legacy_hw_ctx.rcs_state); diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index c8f744c..668acd9 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5507,101 +5507,53 @@ static void gen8_init_clock_gating(struct drm_device *dev) struct drm_i915_private *dev_priv = dev-dev_private; enum pipe pipe; I915_WRITE(WM3_LP_ILK, 0); I915_WRITE(WM2_LP_ILK, 0); I915_WRITE(WM1_LP_ILK, 0); /* FIXME(BDW): Check all the w/a, some might only apply to * pre-production hw. */ - /* WaDisablePartialInstShootdown:bdw */ - I915_WRITE(GEN8_ROW_CHICKEN, - _MASKED_BIT_ENABLE(PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE)); - - /* WaDisableThreadStallDopClockGating:bdw */ - /* FIXME: Unclear whether we really need this on production bdw. */ - I915_WRITE(GEN8_ROW_CHICKEN, - _MASKED_BIT_ENABLE(STALL_DOP_GATING_DISABLE)); - /* -* This GEN8_CENTROID_PIXEL_OPT_DIS W/A is only needed for -* pre-production hardware -*/ - I915_WRITE(HALF_SLICE_CHICKEN3, - _MASKED_BIT_ENABLE(GEN8_CENTROID_PIXEL_OPT_DIS)); - I915_WRITE(HALF_SLICE_CHICKEN3, - _MASKED_BIT_ENABLE(GEN8_SAMPLER_POWER_BYPASS_DIS)); I915_WRITE(GAMTARBMODE, _MASKED_BIT_ENABLE(ARB_MODE_BWGTLB_DISABLE)); I915_WRITE(_3D_CHICKEN3, _MASKED_BIT_ENABLE(_3D_CHICKEN_SDE_LIMIT_FIFO_POLY_DEPTH(2))); - I915_WRITE(COMMON_SLICE_CHICKEN2, - _MASKED_BIT_ENABLE(GEN8_CSC2_SBE_VUE_CACHE_CONSERVATIVE)); - - I915_WRITE(GEN7_HALF_SLICE_CHICKEN1, - _MASKED_BIT_ENABLE(GEN7_SINGLE_SUBSCAN_DISPATCH_ENABLE)); - - /* WaDisableDopClockGating:bdw May not be needed for production */ - I915_WRITE(GEN7_ROW_CHICKEN2, - _MASKED_BIT_ENABLE(DOP_CLOCK_GATING_DISABLE)); /* WaSwitchSolVfFArbitrationPriority:bdw */ I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | HSW_ECOCHK_ARB_PRIO_SOL); /* WaPsrDPAMaskVBlankInSRD:bdw */ I915_WRITE(CHICKEN_PAR1_1, I915_READ(CHICKEN_PAR1_1) | DPA_MASK_VBLANK_SRD); /* WaPsrDPRSUnmaskVBlankInSRD:bdw */ for_each_pipe(pipe) { I915_WRITE(CHICKEN_PIPESL_1(pipe), I915_READ(CHICKEN_PIPESL_1(pipe)) | BDW_DPRS_MASK_VBLANK_SRD); } - /* Use Force Non-Coherent whenever executing a 3D context. This is a -* workaround for for a possible hang in the unlikely event a TLB -* invalidation occurs during a PSD flush. -*/ - I915_WRITE(HDC_CHICKEN0, - I915_READ(HDC_CHICKEN0) | -
[Intel-gfx] [PATCH 0/2] Apply BDW workarounds using LRIs in render init fn
Workarounds for BDW are applied using LRIs during render ring initialization. Only those WA registers that are part of register state are initialized in this fn, remaining are still in its current place init_clock_gating() which are not affected by a gpu reset. I can send another patch where they can be moved to render ring init function but during testing I found their state doesn't change after reset. Arun Siluvery (2): drm/i915/bdw: Apply workarounds in render ring init function drm/i915/bdw: Export workaround data to debugfs drivers/gpu/drm/i915/i915_debugfs.c | 40 + drivers/gpu/drm/i915/i915_drv.h | 14 + drivers/gpu/drm/i915/i915_gem_context.c | 6 ++ drivers/gpu/drm/i915/intel_pm.c | 48 --- drivers/gpu/drm/i915/intel_ringbuffer.c | 102 drivers/gpu/drm/i915/intel_ringbuffer.h | 2 + 6 files changed, 164 insertions(+), 48 deletions(-) -- 2.0.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915/bdw: Export workaround data to debugfs
The workarounds that are applied are exported to a debugfs file; this is used to verify their state after the test case (reset or suspend/resume etc). This patch is only required to support i-g-t. Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com --- drivers/gpu/drm/i915/i915_debugfs.c | 40 + drivers/gpu/drm/i915/i915_drv.h | 14 drivers/gpu/drm/i915/intel_ringbuffer.c | 23 +++ 3 files changed, 77 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index d42db6b..f0d63f6 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2451,20 +2451,59 @@ static int i915_shared_dplls_info(struct seq_file *m, void *unused) seq_printf(m, dpll_md: 0x%08x\n, pll-hw_state.dpll_md); seq_printf(m, fp0: 0x%08x\n, pll-hw_state.fp0); seq_printf(m, fp1: 0x%08x\n, pll-hw_state.fp1); seq_printf(m, wrpll: 0x%08x\n, pll-hw_state.wrpll); } drm_modeset_unlock_all(dev); return 0; } +static int intel_wa_registers(struct seq_file *m, void *unused) +{ + int i; + int ret; + struct drm_info_node *node = (struct drm_info_node *) m-private; + struct drm_device *dev = node-minor-dev; + struct drm_i915_private *dev_priv = dev-dev_private; + + if (!IS_BROADWELL(dev)) { + DRM_DEBUG_DRIVER(Workaround table not available !!\n); + return -EINVAL; + } + + ret = mutex_lock_interruptible(dev-struct_mutex); + if (ret) + return ret; + + intel_runtime_pm_get(dev_priv); + + seq_printf(m, Workarounds applied: %d\n, dev_priv-num_wa_regs); + for (i = 0; i dev_priv-num_wa_regs; ++i) { + u32 addr, mask; + + addr = dev_priv-intel_wa_regs[i].addr; + mask = dev_priv-intel_wa_regs[i].mask; + dev_priv-intel_wa_regs[i].value = I915_READ(addr) | mask; + if (dev_priv-intel_wa_regs[i].addr) + seq_printf(m, 0x%X: 0x%08X, mask: 0x%08X\n, + dev_priv-intel_wa_regs[i].addr, + dev_priv-intel_wa_regs[i].value, + dev_priv-intel_wa_regs[i].mask); + } + + intel_runtime_pm_put(dev_priv); + mutex_unlock(dev-struct_mutex); + + return 0; +} + struct pipe_crc_info { const char *name; struct drm_device *dev; enum pipe pipe; }; static int i915_dp_mst_info(struct seq_file *m, void *unused) { struct drm_info_node *node = (struct drm_info_node *) m-private; struct drm_device *dev = node-minor-dev; @@ -3980,20 +4019,21 @@ static const struct drm_info_list i915_debugfs_list[] = { {i915_llc, i915_llc, 0}, {i915_edp_psr_status, i915_edp_psr_status, 0}, {i915_sink_crc_eDP1, i915_sink_crc, 0}, {i915_energy_uJ, i915_energy_uJ, 0}, {i915_pc8_status, i915_pc8_status, 0}, {i915_power_domain_info, i915_power_domain_info, 0}, {i915_display_info, i915_display_info, 0}, {i915_semaphore_status, i915_semaphore_status, 0}, {i915_shared_dplls_info, i915_shared_dplls_info, 0}, {i915_dp_mst_info, i915_dp_mst_info, 0}, + {intel_wa_registers, intel_wa_registers, 0} }; #define I915_DEBUGFS_ENTRIES ARRAY_SIZE(i915_debugfs_list) static const struct i915_debugfs_files { const char *name; const struct file_operations *fops; } i915_debugfs_files[] = { {i915_wedged, i915_wedged_fops}, {i915_max_freq, i915_max_freq_fops}, {i915_min_freq, i915_min_freq_fops}, diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index bcf79f0..49b7be7 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1546,20 +1546,34 @@ struct drm_i915_private { wait_queue_head_t pending_flip_queue; #ifdef CONFIG_DEBUG_FS struct intel_pipe_crc pipe_crc[I915_MAX_PIPES]; #endif int num_shared_dpll; struct intel_shared_dpll shared_dplls[I915_NUM_PLLS]; int dpio_phy_iosf_port[I915_NUM_PHYS_VLV]; + /* +* workarounds are currently applied at different places and +* changes are being done to consolidate them so exact count is +* not clear at this point, use a max value for now. +*/ +#define I915_MAX_WA_REGS 16 + struct { + u32 addr; + u32 value; + /* bitmask representing WA bits */ + u32 mask; + } intel_wa_regs[I915_MAX_WA_REGS]; + u32 num_wa_regs; + /* Reclocking support */ bool render_reclock_avail; bool lvds_downclock_avail; /* indicates the reduced downclock for LVDS*/ int lvds_downclock; struct
Re: [Intel-gfx] [PATCH 1/2] drm/i915: reorganize the unclaimed register detection code
On Tue, Aug 26, 2014 at 03:34:07PM +0200, Daniel Vetter wrote: On Tue, Aug 26, 2014 at 10:29:23AM -0300, Paulo Zanoni wrote: 2014-08-26 10:18 GMT-03:00 Chris Wilson ch...@chris-wilson.co.uk: On Tue, Aug 26, 2014 at 10:04:22AM -0300, Paulo Zanoni wrote: Of course, we can also implement the one-shot thing on top of the above, but it won't really help us reducing the amount of reads on the happy case where we never got the error before. Actually I am tempted to dynamically patch the mmio vfuncs to avoid even the forcewake spinlock when we already hold it. So there won't be any such logic except when enabled by the user. Should I expect a patch from you, or should I go and write the patch based on what we already discussed? Imo this is crazy - we have no control over what the compiler does and when exactly it loads vtable entries, so patching them at runtime would be an interesting excercise at best. Wtf? -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] igt/gem_workarounds: igt to test workaround registers
Some of the workarounds are lost followed by a gpu reset, suspend/resume; this patch adds a test which compares register state before and after the test scenario. This test currently verifies only bdw workarounds. v2: address patch cleanup comments (ThomasW) Add binary to ignore list and use igt_debugfs helper fns to read debugfs file and igt_info for printing debug info. v2.1: address minor comments from Daniel use igt_main as opposed to normal main Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com --- tests/.gitignore| 1 + tests/Makefile.sources | 1 + tests/gem_workarounds.c | 233 3 files changed, 235 insertions(+) create mode 100644 tests/gem_workarounds.c diff --git a/tests/.gitignore b/tests/.gitignore index 985afbd..1103e2e 100644 --- a/tests/.gitignore +++ b/tests/.gitignore @@ -97,20 +97,21 @@ gem_tiled_fence_blits gem_tiled_partial_pwrite_pread gem_tiled_pread gem_tiled_pread_pwrite gem_tiled_swapping gem_tiling_max_stride gem_unfence_active_buffers gem_unref_active_buffers gem_userptr_blits gem_wait_render_timeout gem_write_read_ring_switch +gem_workarounds gen3_mixed_blits gen3_render_linear_blits gen3_render_mixed_blits gen3_render_tiledx_blits gen3_render_tiledy_blits gen7_forcewake_mt igt_fork_helper igt_list_only igt_no_exit igt_no_exit_list_only diff --git a/tests/Makefile.sources b/tests/Makefile.sources index 0eb9369..a17acd1 100644 --- a/tests/Makefile.sources +++ b/tests/Makefile.sources @@ -127,20 +127,21 @@ TESTS_progs = \ gem_storedw_loop_vebox \ gem_threaded_access_tiled \ gem_tiled_fence_blits \ gem_tiled_pread \ gem_tiled_pread_pwrite \ gem_tiled_swapping \ gem_tiling_max_stride \ gem_unfence_active_buffers \ gem_unref_active_buffers \ gem_wait_render_timeout \ + gem_workarounds \ gen3_mixed_blits \ gen3_render_linear_blits \ gen3_render_mixed_blits \ gen3_render_tiledx_blits \ gen3_render_tiledy_blits \ gen7_forcewake_mt \ kms_force_connector \ kms_sink_crc_basic \ kms_fence_pin_leak \ pm_psr \ diff --git a/tests/gem_workarounds.c b/tests/gem_workarounds.c new file mode 100644 index 000..91c2aeb --- /dev/null +++ b/tests/gem_workarounds.c @@ -0,0 +1,233 @@ +/* + * Copyright © 2014 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the Software), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + * + * Authors: + * Arun Siluvery arun.siluv...@linux.intel.com + * + */ + +#define _GNU_SOURCE +#include stdbool.h +#include unistd.h +#include stdlib.h +#include stdio.h +#include string.h +#include fcntl.h +#include inttypes.h +#include errno.h +#include sys/stat.h +#include sys/ioctl.h +#include sys/mman.h +#include time.h +#include signal.h + +#include ioctl_wrappers.h +#include drmtest.h +#include igt_debugfs.h +#include igt_aux.h +#include intel_chipset.h +#include intel_io.h + +enum operation { + GPU_RESET = 0x01, + SUSPEND_RESUME = 0x02, +}; + +struct intel_wa_reg { + uint32_t addr; + uint32_t value; + uint32_t mask; +}; + +int drm_fd; +uint32_t devid; +static drm_intel_bufmgr *bufmgr; +struct intel_batchbuffer *batch; +int num_wa_regs; +struct intel_wa_reg *wa_regs; + + +static void test_hang_gpu(void) +{ + int retry_count = 30; + enum stop_ring_flags flags; + struct drm_i915_gem_execbuffer2 execbuf; + struct drm_i915_gem_exec_object2 gem_exec; + uint32_t b[2] = {MI_BATCH_BUFFER_END}; + + igt_assert(retry_count); + igt_set_stop_rings(STOP_RING_DEFAULTS); + + memset(gem_exec, 0, sizeof(gem_exec)); + gem_exec.handle = gem_create(drm_fd, 4096); + gem_write(drm_fd, gem_exec.handle, 0, b, sizeof(b)); + + memset(execbuf, 0, sizeof(execbuf)); + execbuf.buffers_ptr = (uintptr_t)gem_exec; +
Re: [Intel-gfx] [PATCH v2] drm/i915: Fix to Enable GT/PM Interrupts for cherryview.
On Fri, Aug 22, 2014 at 08:32:40AM +0530, deepa...@linux.intel.com wrote: From: Deepak S deepa...@linux.intel.com Programing GT IER interrupts was fumbled while enabling Interrupts for gen8 This is a regression from commit abd58f0175915bed644aa67c8f69dc571b8280e0 Author: Ben Widawsky benjamin.widaw...@intel.com Date: Sat Nov 2 21:07:09 2013 -0700 drm/i915/bdw: Implement interrupt changes _Really_ unlikely that this is a regression from this commit, since that introduced all the gen8 interrupt handling in the first place. I think this is because of the reworked interrupt handling over gpu resets, where we want to keep rps interrupts enabled. But I'm not terribly sure. I think a more convincing story is that this is an oversight from commit a6706b45a57a23a613b34793e1414991b60a09c1 Author: Deepak S deepa...@linux.intel.com Date: Sat Mar 15 20:23:22 2014 +0530 drm/i915: Track the enabled PM interrupts in dev_priv or more precisely (since chv wasn't public back then iirc) we forgot to add the corresponding patch to -internal. In any case please clarify the commit message and please also add a few words about why exactly we need this - I had to dig through git history to figure this all out. I've applied the patch already, so you can just reply with the revised commit message that I should put in. Thanks, Daniel v2: Kill the loop and init GT interrupts (Ville) Signed-off-by: Deepak S deepa...@linux.intel.com --- drivers/gpu/drm/i915/i915_irq.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index d5445e7..c33cf89 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -3799,8 +3799,6 @@ static int valleyview_irq_postinstall(struct drm_device *dev) static void gen8_gt_irq_postinstall(struct drm_i915_private *dev_priv) { - int i; - /* These are interrupts we'll toggle with the ring mask register */ uint32_t gt_interrupts[] = { GT_RENDER_USER_INTERRUPT GEN8_RCS_IRQ_SHIFT | @@ -3817,10 +3815,11 @@ static void gen8_gt_irq_postinstall(struct drm_i915_private *dev_priv) GT_CONTEXT_SWITCH_INTERRUPT GEN8_VECS_IRQ_SHIFT }; - for (i = 0; i ARRAY_SIZE(gt_interrupts); i++) - GEN8_IRQ_INIT_NDX(GT, i, ~gt_interrupts[i], gt_interrupts[i]); - dev_priv-pm_irq_mask = 0x; + GEN8_IRQ_INIT_NDX(GT, 0, ~gt_interrupts[0], gt_interrupts[0]); + GEN8_IRQ_INIT_NDX(GT, 1, ~gt_interrupts[1], gt_interrupts[1]); + GEN8_IRQ_INIT_NDX(GT, 2, dev_priv-pm_irq_mask, dev_priv-pm_rps_events); + GEN8_IRQ_INIT_NDX(GT, 3, ~gt_interrupts[3], gt_interrupts[3]); } static void gen8_de_irq_postinstall(struct drm_i915_private *dev_priv) -- 1.9.1 ___ 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/bdw: Render moot context reset and switch with Execlists
On 26/08/2014 06:59, Chris Wilson wrote: On Mon, Aug 25, 2014 at 10:39:39PM +0200, Daniel Vetter wrote: On Wed, Aug 20, 2014 at 04:36:05PM +0100, Chris Wilson wrote: On Wed, Aug 20, 2014 at 04:29:24PM +0100, Thomas Daniel wrote: These two functions make no sense in an Logical Ring Context Execlists world. v2: We got rid of lrc_enabled and centralized everything in the sanitized i915.enable_execlists instead. Signed-off-by: Oscar Mateo oscar.ma...@intel.com v3: Rebased. Corrected a typo in comment for i915_switch_context and added a comment that it should not be called in execlist mode. Added WARN_ON if i915_switch_context is called in execlist mode. Moved check for execlist mode out of i915_switch_context and into callers. Added comment in context_reset explaining why nothing is done in execlist mode. No, this is not the way. The requirement is to reduce the number of special cases not increase them. These should be evaluated to be no-ops when execlists is used. I think it's ok-ish for now. Maybe we need to reconsider when we wire up lrc reclaim - which is the real user of the switch_context in gpu_idle. The problem I have though is that I can't parse the subject of the patch, someone please translate that to simplified English for me. I can do the replacement while applying. No, it is not. execlists is badly designed and this is a further symptom of that. -Chris Thomas is not available and I am replying on his behalf. Is the following subject is good for this patch? Don't execute context reset and switch when using Execlists regards Arun ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 02/14] drm/i915: Reorganize vlv eDP reboot notifier
On Tue, Aug 26, 2014 at 03:36:07PM +0200, Daniel Vetter wrote: On Tue, Aug 26, 2014 at 04:21:00PM +0300, Jani Nikula wrote: On Tue, 26 Aug 2014, Ville Syrjälä ville.syrj...@linux.intel.com wrote: On Tue, Aug 19, 2014 at 10:00:55AM +0300, Jani Nikula wrote: On Mon, 18 Aug 2014, ville.syrj...@linux.intel.com wrote: From: Ville Syrjälä ville.syrj...@linux.intel.com Move the vlv_power_sequencer_pipe() after the IS_VALLEYVIEW() check and flatten the rest of the function. Please imagine adding another platform there, and realize this just adds unnecessary churn. I'd just add another reboot notifier then. Fair enough; it should be vlv_edp_notify_handler then. (No, don't send a patch to change that! ;) Frankly I don't understand the current one either. Why does it need to set the delay to max for instance? And does this mean that the PANEL_POWER_RESET bit doesn't actually work as advertised in the docs? *shrug* experimental evidence? commit 01527b3127997ef6370d5ad4fa25d96847fbf12a Author: Clint Taylor clinton.a.tay...@intel.com Date: Mon Jul 7 13:01:46 2014 -0700 drm/i915/vlv: T12 eDP panel timing enforcement during reboot The panel power sequencer on vlv doesn't appear to accept changes to its T12 power down duration during warm reboots. This change forces a delay for warm reboots to the T12 panel timing as defined in the VBT table for the connected panel. So if I remember this piece of lore correctly in the past the pp was pessimistic, and enforced this delay on resume/boot-up, assuming you've shut down _right_ before the machine was lit up again. Apparently people where unhappy with that enforced delay and it was ditched on vlv, but then it broke panels if you actually managed to reboot quickly enough. IIRC the way the reset bit is documented the hardware itself is supposed to initiate the power off cycle when it gets some reset notification and it should enforce the timing before allowing the panel power to be re-enabled. Although it does seem that it would also reset the power cycle delay so it would maybe only enforce some default delay in that case (300ms based on the documented default value of 0x4). So if the panel requires more than the 300ms then I understand the msleep() here. I guess use of the VDD force bit just after reset might also require that we do the power down + wait before reset. So that part does make sense to me, but I still don't understand the power cycle delay=0x1f part. -- 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 1/2] drm/i915: reorganize the unclaimed register detection code
On Tue, Aug 26, 2014 at 02:46:25PM +0100, Chris Wilson wrote: On Tue, Aug 26, 2014 at 03:34:07PM +0200, Daniel Vetter wrote: On Tue, Aug 26, 2014 at 10:29:23AM -0300, Paulo Zanoni wrote: 2014-08-26 10:18 GMT-03:00 Chris Wilson ch...@chris-wilson.co.uk: On Tue, Aug 26, 2014 at 10:04:22AM -0300, Paulo Zanoni wrote: Of course, we can also implement the one-shot thing on top of the above, but it won't really help us reducing the amount of reads on the happy case where we never got the error before. Actually I am tempted to dynamically patch the mmio vfuncs to avoid even the forcewake spinlock when we already hold it. So there won't be any such logic except when enabled by the user. Should I expect a patch from you, or should I go and write the patch based on what we already discussed? Imo this is crazy - we have no control over what the compiler does and when exactly it loads vtable entries, so patching them at runtime would be an interesting excercise at best. Wtf? I've assumed you'd runtime patch the vtables or something like that. And I didn't see any other less crazy way to ... -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/bdw: Apply workarounds in render ring init function
On 26/08/2014 15:37, Ville Syrjälä wrote: On Tue, Aug 26, 2014 at 02:44:50PM +0100, Arun Siluvery wrote: For BDW workarounds are currently initialized in init_clock_gating() but they are lost during reset, suspend/resume etc; this patch moves the WAs that are part of register state context to render ring init fn otherwise default context ends up with incorrect values as they don't get initialized until init_clock_gating fn. v2: Add workarounds to golden render state This method has its own issues, first of all this is different for each gen and it is generated using a tool so adding new workaround and mainitaining them across gens is not a straightforward process. v3: Use LRIs to emit these workarounds (Ville) Instead of modifying the golden render state the same LRIs are emitted from within the driver. v4: Use abstract name when exporting gen specific routines (Chris) For: VIZ-4092 Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com This one looks good as far as I'm concerned. Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Do you plan to give other platforms the same treatment? We need at least CHV converted ASAP. But if you don't have a test machine I can take care of that myself. I don't have hardware for CHV, I can borrow and try to do but since it is required at the earliest could you please modify it for CHV? regards Arun --- drivers/gpu/drm/i915/i915_gem_context.c | 6 +++ drivers/gpu/drm/i915/intel_pm.c | 48 drivers/gpu/drm/i915/intel_ringbuffer.c | 79 + drivers/gpu/drm/i915/intel_ringbuffer.h | 2 + 4 files changed, 87 insertions(+), 48 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 9683e62..0a9bb0e 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -631,20 +631,26 @@ static int do_switch(struct intel_engine_cs *ring, } uninitialized = !to-legacy_hw_ctx.initialized from == NULL; to-legacy_hw_ctx.initialized = true; done: i915_gem_context_reference(to); ring-last_context = to; if (uninitialized) { + if (ring-init_context) { + ret = ring-init_context(ring); + if (ret) + DRM_ERROR(ring init context: %d\n, ret); + } + ret = i915_gem_render_state_init(ring); if (ret) DRM_ERROR(init render state: %d\n, ret); } return 0; unpin_out: if (ring-id == RCS) i915_gem_object_ggtt_unpin(to-legacy_hw_ctx.rcs_state); diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index c8f744c..668acd9 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5507,101 +5507,53 @@ static void gen8_init_clock_gating(struct drm_device *dev) struct drm_i915_private *dev_priv = dev-dev_private; enum pipe pipe; I915_WRITE(WM3_LP_ILK, 0); I915_WRITE(WM2_LP_ILK, 0); I915_WRITE(WM1_LP_ILK, 0); /* FIXME(BDW): Check all the w/a, some might only apply to * pre-production hw. */ - /* WaDisablePartialInstShootdown:bdw */ - I915_WRITE(GEN8_ROW_CHICKEN, - _MASKED_BIT_ENABLE(PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE)); - - /* WaDisableThreadStallDopClockGating:bdw */ - /* FIXME: Unclear whether we really need this on production bdw. */ - I915_WRITE(GEN8_ROW_CHICKEN, - _MASKED_BIT_ENABLE(STALL_DOP_GATING_DISABLE)); - /* -* This GEN8_CENTROID_PIXEL_OPT_DIS W/A is only needed for -* pre-production hardware -*/ - I915_WRITE(HALF_SLICE_CHICKEN3, - _MASKED_BIT_ENABLE(GEN8_CENTROID_PIXEL_OPT_DIS)); - I915_WRITE(HALF_SLICE_CHICKEN3, - _MASKED_BIT_ENABLE(GEN8_SAMPLER_POWER_BYPASS_DIS)); I915_WRITE(GAMTARBMODE, _MASKED_BIT_ENABLE(ARB_MODE_BWGTLB_DISABLE)); I915_WRITE(_3D_CHICKEN3, _MASKED_BIT_ENABLE(_3D_CHICKEN_SDE_LIMIT_FIFO_POLY_DEPTH(2))); - I915_WRITE(COMMON_SLICE_CHICKEN2, - _MASKED_BIT_ENABLE(GEN8_CSC2_SBE_VUE_CACHE_CONSERVATIVE)); - - I915_WRITE(GEN7_HALF_SLICE_CHICKEN1, - _MASKED_BIT_ENABLE(GEN7_SINGLE_SUBSCAN_DISPATCH_ENABLE)); - - /* WaDisableDopClockGating:bdw May not be needed for production */ - I915_WRITE(GEN7_ROW_CHICKEN2, - _MASKED_BIT_ENABLE(DOP_CLOCK_GATING_DISABLE)); /* WaSwitchSolVfFArbitrationPriority:bdw */ I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | HSW_ECOCHK_ARB_PRIO_SOL); /* WaPsrDPAMaskVBlankInSRD:bdw */ I915_WRITE(CHICKEN_PAR1_1, I915_READ(CHICKEN_PAR1_1) | DPA_MASK_VBLANK_SRD); /* WaPsrDPRSUnmaskVBlankInSRD:bdw */
Re: [Intel-gfx] [PATCH 1/2] drm/i915/bdw: Apply workarounds in render ring init function
On Tue, Aug 26, 2014 at 02:44:50PM +0100, Arun Siluvery wrote: For BDW workarounds are currently initialized in init_clock_gating() but they are lost during reset, suspend/resume etc; this patch moves the WAs that are part of register state context to render ring init fn otherwise default context ends up with incorrect values as they don't get initialized until init_clock_gating fn. v2: Add workarounds to golden render state This method has its own issues, first of all this is different for each gen and it is generated using a tool so adding new workaround and mainitaining them across gens is not a straightforward process. v3: Use LRIs to emit these workarounds (Ville) Instead of modifying the golden render state the same LRIs are emitted from within the driver. v4: Use abstract name when exporting gen specific routines (Chris) For: VIZ-4092 Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com This one looks good as far as I'm concerned. Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Do you plan to give other platforms the same treatment? We need at least CHV converted ASAP. But if you don't have a test machine I can take care of that myself. --- drivers/gpu/drm/i915/i915_gem_context.c | 6 +++ drivers/gpu/drm/i915/intel_pm.c | 48 drivers/gpu/drm/i915/intel_ringbuffer.c | 79 + drivers/gpu/drm/i915/intel_ringbuffer.h | 2 + 4 files changed, 87 insertions(+), 48 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 9683e62..0a9bb0e 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -631,20 +631,26 @@ static int do_switch(struct intel_engine_cs *ring, } uninitialized = !to-legacy_hw_ctx.initialized from == NULL; to-legacy_hw_ctx.initialized = true; done: i915_gem_context_reference(to); ring-last_context = to; if (uninitialized) { + if (ring-init_context) { + ret = ring-init_context(ring); + if (ret) + DRM_ERROR(ring init context: %d\n, ret); + } + ret = i915_gem_render_state_init(ring); if (ret) DRM_ERROR(init render state: %d\n, ret); } return 0; unpin_out: if (ring-id == RCS) i915_gem_object_ggtt_unpin(to-legacy_hw_ctx.rcs_state); diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index c8f744c..668acd9 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5507,101 +5507,53 @@ static void gen8_init_clock_gating(struct drm_device *dev) struct drm_i915_private *dev_priv = dev-dev_private; enum pipe pipe; I915_WRITE(WM3_LP_ILK, 0); I915_WRITE(WM2_LP_ILK, 0); I915_WRITE(WM1_LP_ILK, 0); /* FIXME(BDW): Check all the w/a, some might only apply to * pre-production hw. */ - /* WaDisablePartialInstShootdown:bdw */ - I915_WRITE(GEN8_ROW_CHICKEN, -_MASKED_BIT_ENABLE(PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE)); - - /* WaDisableThreadStallDopClockGating:bdw */ - /* FIXME: Unclear whether we really need this on production bdw. */ - I915_WRITE(GEN8_ROW_CHICKEN, -_MASKED_BIT_ENABLE(STALL_DOP_GATING_DISABLE)); - /* - * This GEN8_CENTROID_PIXEL_OPT_DIS W/A is only needed for - * pre-production hardware - */ - I915_WRITE(HALF_SLICE_CHICKEN3, -_MASKED_BIT_ENABLE(GEN8_CENTROID_PIXEL_OPT_DIS)); - I915_WRITE(HALF_SLICE_CHICKEN3, -_MASKED_BIT_ENABLE(GEN8_SAMPLER_POWER_BYPASS_DIS)); I915_WRITE(GAMTARBMODE, _MASKED_BIT_ENABLE(ARB_MODE_BWGTLB_DISABLE)); I915_WRITE(_3D_CHICKEN3, _MASKED_BIT_ENABLE(_3D_CHICKEN_SDE_LIMIT_FIFO_POLY_DEPTH(2))); - I915_WRITE(COMMON_SLICE_CHICKEN2, -_MASKED_BIT_ENABLE(GEN8_CSC2_SBE_VUE_CACHE_CONSERVATIVE)); - - I915_WRITE(GEN7_HALF_SLICE_CHICKEN1, -_MASKED_BIT_ENABLE(GEN7_SINGLE_SUBSCAN_DISPATCH_ENABLE)); - - /* WaDisableDopClockGating:bdw May not be needed for production */ - I915_WRITE(GEN7_ROW_CHICKEN2, -_MASKED_BIT_ENABLE(DOP_CLOCK_GATING_DISABLE)); /* WaSwitchSolVfFArbitrationPriority:bdw */ I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | HSW_ECOCHK_ARB_PRIO_SOL); /* WaPsrDPAMaskVBlankInSRD:bdw */ I915_WRITE(CHICKEN_PAR1_1, I915_READ(CHICKEN_PAR1_1) | DPA_MASK_VBLANK_SRD); /* WaPsrDPRSUnmaskVBlankInSRD:bdw */ for_each_pipe(pipe) { I915_WRITE(CHICKEN_PIPESL_1(pipe), I915_READ(CHICKEN_PIPESL_1(pipe)) | BDW_DPRS_MASK_VBLANK_SRD); }
Re: [Intel-gfx] Fwd: i915 (possibly): Suspend regression Macbook Pro 15 (late 2013)
On Tue, Aug 26, 2014 at 12:17 AM, Daniel Vetter dan...@ffwll.ch wrote: Sounds new. Anything interesting going on in dmesg while the on/off flickering goes on if you crank up output with drm.debug=0xe? I actually get some traces. There appears to be 2 different traces that repeat whenever the screen goes through that turn off-turn on cycle (but at first, only the first trace is shown). I believe the screen turns itself off at the time this message is printed: Aug 26 07:25:04 nc052 kernel: [drm:i915_runtime_suspend] Suspending device First trace: Aug 26 07:25:04 nc052 kernel: [drm:i915_runtime_suspend] Suspending device Aug 26 07:25:04 nc052 kernel: [drm:hsw_enable_pc8] Enabling package C8+ Aug 26 07:25:04 nc052 kernel: [ cut here ] Aug 26 07:25:04 nc052 kernel: WARNING: CPU: 0 PID: 60 at drivers/gpu/drm/i915/intel_display.c:6888 hsw_enable_pc8+0xae/0x6f0() Aug 26 07:25:04 nc052 kernel: CRTC for pipe A enabled Aug 26 07:25:04 nc052 kernel: Modules linked in: Aug 26 07:25:04 nc052 kernel: CPU: 0 PID: 60 Comm: kworker/0:1 Not tainted 3.15.4-ARCH-00041-gf4db98240ac2 #21 Aug 26 07:25:04 nc052 kernel: Hardware name: Apple Inc. MacBookPro11,2/Mac-3CBD00234E554E41, BIOS MBP112.88Z.0138.B02.1310181745 10/18/2013 Aug 26 07:25:04 nc052 kernel: Workqueue: pm pm_runtime_work Aug 26 07:25:04 nc052 kernel: 2a8a3008 8802732a3c20 81872013 Aug 26 07:25:04 nc052 kernel: 8802732a3c68 8802732a3c58 810d650d 880272298000 Aug 26 07:25:04 nc052 kernel: 8802737ea000 880273050b80 81a8d140 88027311d098 Aug 26 07:25:04 nc052 kernel: Call Trace: Aug 26 07:25:04 nc052 kernel: [81872013] dump_stack+0x4d/0x6f Aug 26 07:25:04 nc052 kernel: [810d650d] warn_slowpath_common+0x7d/0xa0 Aug 26 07:25:04 nc052 kernel: [810d658c] warn_slowpath_fmt+0x5c/0x80 Aug 26 07:25:04 nc052 kernel: [814ea3fd] ? hsw_runtime_pm_disable_interrupts+0xed/0x100 Aug 26 07:25:04 nc052 kernel: [8150a95e] hsw_enable_pc8+0xae/0x6f0 Aug 26 07:25:04 nc052 kernel: [814b6ad8] i915_runtime_suspend+0x88/0xf0 Aug 26 07:25:04 nc052 kernel: [813caa7f] pci_pm_runtime_suspend+0x5f/0x150 Aug 26 07:25:04 nc052 kernel: [813caa20] ? pci_legacy_suspend_late+0xf0/0xf0 Aug 26 07:25:04 nc052 kernel: [8154e8e2] __rpm_callback+0x32/0x70 Aug 26 07:25:04 nc052 kernel: [8154e946] rpm_callback+0x26/0xa0 Aug 26 07:25:04 nc052 kernel: [8154ee91] rpm_suspend+0x121/0x680 Aug 26 07:25:04 nc052 kernel: [810e46f8] ? add_timer+0x18/0x30 Aug 26 07:25:04 nc052 kernel: [810f1d5b] ? __queue_delayed_work+0x8b/0x1c0 Aug 26 07:25:04 nc052 kernel: [8155070a] pm_runtime_work+0x7a/0xd0 Aug 26 07:25:04 nc052 kernel: [810f2bc8] process_one_work+0x168/0x450 Aug 26 07:25:04 nc052 kernel: [810f3622] worker_thread+0x132/0x3e0 Aug 26 07:25:04 nc052 kernel: [810f34f0] ? manage_workers.isra.23+0x2d0/0x2d0 Aug 26 07:25:04 nc052 kernel: [810f9e2a] kthread+0xea/0x100 Aug 26 07:25:04 nc052 kernel: [810f9d40] ? kthread_create_on_node+0x1b0/0x1b0 Aug 26 07:25:04 nc052 kernel: [81881d3c] ret_from_fork+0x7c/0xb0 Aug 26 07:25:04 nc052 kernel: [810f9d40] ? kthread_create_on_node+0x1b0/0x1b0 Aug 26 07:25:04 nc052 kernel: ---[ end trace 51deaa0c7e786a61 ]--- Second trace: Aug 26 07:30:51 nc052 kernel: [drm:i915_runtime_suspend] Device suspended Aug 26 07:31:32 nc052 kernel: [drm:drm_ioctl] pid=405, dev=0xe200, auth=1, DRM_IOCTL_MODE_CURSOR Aug 26 07:31:32 nc052 kernel: [ cut here ] Aug 26 07:31:32 nc052 kernel: WARNING: CPU: 0 PID: 405 at drivers/gpu/drm/i915/intel_uncore.c:47 assert_device_not_suspended.isra.6+0x32/0x40() Aug 26 07:31:32 nc052 kernel: Device suspended Aug 26 07:31:32 nc052 kernel: Modules linked in: Aug 26 07:31:32 nc052 kernel: CPU: 0 PID: 405 Comm: Xorg.bin Tainted: GW 3.15.4-ARCH-00041-gf4db98240ac2 #21 Aug 26 07:31:32 nc052 kernel: Hardware name: Apple Inc. MacBookPro11,2/Mac-3CBD00234E554E41, BIOS MBP112.88Z.0138.B02.1310181745 10/18/2013 Aug 26 07:31:32 nc052 kernel: 2f20b3eb 880272beb980 81872013 Aug 26 07:31:32 nc052 kernel: 880272beb9c8 880272beb9b8 810d650d 880272298000 Aug 26 07:31:32 nc052 kernel: 00130040 880273050800 880272298070 0001 Aug 26 07:31:32 nc052 kernel: Call Trace: Aug 26 07:31:32 nc052 kernel: [81872013] dump_stack+0x4d/0x6f Aug 26 07:31:32 nc052 kernel: [810d650d] warn_slowpath_common+0x7d/0xa0 Aug 26 07:31:32 nc052 kernel: [810d658c] warn_slowpath_fmt+0x5c/0x80 Aug 26 07:31:32 nc052 kernel: [814f1bf2] assert_device_not_suspended.isra.6+0x32/0x40 Aug 26 07:31:32 nc052 kernel: [814f3222] gen6_read32+0x32/0x120 Aug 26 07:31:32 nc052 kernel: [8151fe74] intel_ddi_get_cdclk_freq+0x24/0x100 Aug 26 07:31:32 nc052 kernel:
Re: [Intel-gfx] i915: Regression: +4W in idle power use on Macbook Pro 15 (late 2013)
On Tue, Aug 26, 2014 at 6:38 AM, Eric Rannaud eric.rann...@gmail.com wrote: Could the FBC commit somehow also disable RC6, or similar? I just tried these kernel arguments with no significant effect on power consumption (i.e. still around 11W in idle, instead of under 7W): i915.enable_rc6=1 i915.lvds_downclock=1 pcie_aspm=force Forcing FBC with i915.enable_fbc=1 brings the idle power consumption back to under 7W, however. This is all on 3.15.4-ARCH-00041-gf4db98240ac2. ___ 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/bdw: Apply workarounds in render ring init function
On Tue, Aug 26, 2014 at 03:47:52PM +0100, Siluvery, Arun wrote: On 26/08/2014 15:37, Ville Syrjälä wrote: On Tue, Aug 26, 2014 at 02:44:50PM +0100, Arun Siluvery wrote: For BDW workarounds are currently initialized in init_clock_gating() but they are lost during reset, suspend/resume etc; this patch moves the WAs that are part of register state context to render ring init fn otherwise default context ends up with incorrect values as they don't get initialized until init_clock_gating fn. v2: Add workarounds to golden render state This method has its own issues, first of all this is different for each gen and it is generated using a tool so adding new workaround and mainitaining them across gens is not a straightforward process. v3: Use LRIs to emit these workarounds (Ville) Instead of modifying the golden render state the same LRIs are emitted from within the driver. v4: Use abstract name when exporting gen specific routines (Chris) For: VIZ-4092 Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com This one looks good as far as I'm concerned. Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Do you plan to give other platforms the same treatment? We need at least CHV converted ASAP. But if you don't have a test machine I can take care of that myself. I don't have hardware for CHV, I can borrow and try to do but since it is required at the earliest could you please modify it for CHV? Sure, I can take care of it. -- 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 1/2] drm/i915/bdw: Apply workarounds in render ring init function
On Tue, Aug 26, 2014 at 05:37:05PM +0300, Ville Syrjälä wrote: On Tue, Aug 26, 2014 at 02:44:50PM +0100, Arun Siluvery wrote: For BDW workarounds are currently initialized in init_clock_gating() but they are lost during reset, suspend/resume etc; this patch moves the WAs that are part of register state context to render ring init fn otherwise default context ends up with incorrect values as they don't get initialized until init_clock_gating fn. I am dubious here. How does the default context end up with incorrect values if the registers are loaded afterwards and the default context is *never* restored from memory? Could you explain how this exactly fails? Could you explain why only gen8 is affected? -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: don't warn if backlight unexpectedly enabled
On Tue, 26 Aug 2014, Daniel Vetter wrote: On Thu, Aug 21, 2014 at 07:12:59AM +, Scot Doyle wrote: When we enter intel_modeset_setup_hw_state during resume - BLC_PWM_CPU_CTL2 == BLM_PWM_ENABLE - the physical backlight is off Hm, this is actually interesting - we have some other evidence that the best way to shut off the backlight is actually to just set the pwm duty cycle to 0. Can you please check that this is the case for your system? /sys/class/backlight/intel_backlight/brightness 0 - backlight not visible 1 - backlight visible 937 - max backlight Setting /sys/class/backlight/intel_backlight/brightness to 0 updates BLC_PWM_CPU_CTL, but BLC_PWM_CPU_CTL2 remains 0xe000. Maybe we just need to extend the check to look for !PWM_ENABLE || duty_cycle == 0. The following measurements hold true no matter the duty cycle before suspend: When entering hsw_enable_pc8 during suspend - the physical backlight is off - BLC_PWM_CPU_CTL == 0x3a90 (BACKLIGHT_DUTY_CYCLE_MASK == ) - BLC_PWM_CPU_CTL2 == 0x6000 (BLM_PWM_ENABLE) When exiting hsw_disable_pc8 during resume - the physical backlight is off - BLC_PWM_CPU_CTL == 0x200 - BLC_PWM_CPU_CTL2 == 0x8000 (BLM_PWM_ENABLE | BLM_TRANSCODER_EDP) When entering pch_enable_backlight during resume - the physical backlight is off - BLC_PWM_CPU_CTL == 0x200 - BLC_PWM_CPU_CTL2 == 0x8000 (BLM_PWM_ENABLE) When exiting pch_enable_backlight during resume - the physical backlight is off - BLC_PWM_CPU_CTL == duty cycle prior to suspend - BLC_PWM_CPU_CTL2 == 0xe000 (BLM_PWM_ENABLE | BLM_TRANSCODER_EDP) So the BIOS is setting BLC_PWM_CPU_CTL=0x200 and BLC_PWM_CPU_CTL2=0x8000 ? ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] i915.fastboot bug report - not working on coreboot
Hello I'm trying to use i915.fastboot on a Thinkpad X60t. The bios has been replaced by coreboot, which supports native video init. The goal is to boot to a console on a debian in less than 2 seconds (kernel + systemd), systemd is just fine in 0.6s but the kernel takes a long time, with apparently 1 full second spend on the video mode initialization, just as if fastboot was ignored. Coreboot is starting a grub2 payload, which is in the appropriate vesa mode, and has option set gfxpayload=keep to pass it to the kernel. The 3.14.16 kernel start in the appropriate vesa mode, but there is some flickering at one time, which indicates the drivers tries to reinitialize the video card, before returning to this same mode. The kernel arguments are: nohz=on nmi_watchdog=0 pcie_aspm=force i915.semaphores=1 i915.fastboot=1 i915.i915_enable_rc6=7 i915.i915_enable_fbc=1 i915.lvds_downclock=1 thinkpad_acpi.force_load=1 thinkpad_acpi.brightness_enable=0 snd-hda-intel.index=0 snd_hda_intel.power_save=10 snd_hda_intel.model=thinkpad snd-hda-intel.probe_mask=0x103 snd-pcsp.index=1 btusb.reset=1 quiet root=/dev/sda1 The kernel .config has: CONFIG_DRM_I915=y CONFIG_DRM_I915_KMS=y CONFIG_DRM_I915_FBDEV=y # CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT is not set # CONFIG_DRM_I915_UMS is not set Is it possible to enable some kind of debugging of i915.fastboot to see why exactly the video is reinitialized? I didn't see any module option to do that in drivers/gpu/drm/i915/i915_drv.c Here are the dmesg showing the time spent. In a 3.14.16 kernel : [0.498511] [drm] Initialized drm 1.1.0 20060810 [0.498967] [drm] Memory usable by graphics device = 256M [0.500365] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013). [0.500370] [drm] Driver supports precise vblank timestamp query. [0.500377] i915 :00:02.0: Invalid ROM contents [0.500384] [drm] failed to find VBIOS tables [0.500449] vgaarb: device changed decodes: PCI::00:02.0,olddecodes=io+me m,decodes=io+mem:owns=io+mem [0.532092] [drm] initialized overlay support [0.799483] fbcon: inteldrmfb (fb0) is primary device [1.452009] Console: switching to colour frame buffer device 128x48 [1.480975] i915 :00:02.0: fb0: inteldrmfb frame buffer device [1.480978] i915 :00:02.0: registered panic notifier [1.480990] [drm] Initialized i915 1.6.0 20080730 for :00:02.0 on minor 0 In a 3.10.45 kernel: (no fastboot support) [0.213937] [drm] Initialized drm 1.1.0 20060810 [0.214643] [drm] Memory usable by graphics device = 256M [0.214650] i915 :00:02.0: setting latency timer to 64 [0.216252] [drm] Supports vblank timestamp caching Rev 1 (10.10.2010). [0.216284] [drm] Driver supports precise vblank timestamp query. [0.216291] i915 :00:02.0: Invalid ROM contents [0.216298] [drm] failed to find VBIOS tables [0.216351] vgaarb: device changed decodes: PCI::00:02.0,olddecodes=io+mem,decodes=io+mem:owns=io+mem [0.226207] ACPI: Deprecated procfs I/F for battery is loaded, please retry with CONFIG_ACPI_PROCFS_POWER cleared [0.226218] ACPI: Battery Slot [BAT0] (battery present) [0.226378] ACPI: Deprecated procfs I/F for battery is loaded, please retry with CONFIG_ACPI_PROCFS_POWER cleared [0.226386] ACPI: Battery Slot [BAT1] (battery absent) [0.267013] [drm] GMBUS [i915 gmbus panel] timed out, falling back to bit banging on pin 3 [0.297928] [drm] initialized overlay support [0.360013] [drm] GMBUS [i915 gmbus vga] timed out, falling back to bit banging on pin 2 [0.546070] fbcon: inteldrmfb (fb0) is primary device [1.198025] Console: switching to colour frame buffer device 128x48 [1.226996] i915 :00:02.0: fb0: inteldrmfb frame buffer device [1.227000] i915 :00:02.0: registered panic notifier [1.227015] [drm] Initialized i915 1.6.0 20080730 for :00:02.0 on minor 0 [1.228648] loop: module loaded [1.279013] [drm] GMBUS [i915 gmbus ssc] timed out, falling back to bit banging on pin 1 [1.398013] [drm] GMBUS [i915 gmbus dpc] timed out, falling back to bit banging on pin 4 [1.513013] [drm] GMBUS [i915 gmbus dpb] timed out, falling back to bit banging on pin 5 [1.586013] [drm] GMBUS [i915 gmbus dpd] timed out, falling back to bit banging on pin 6 Thanks Charles ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: call lpt_init_clock_gating on BDW too
On Thu, Aug 21, 2014 at 10:50:38PM +0100, Damien Lespiau wrote: On Thu, Aug 21, 2014 at 05:09:36PM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com Because BDW has WPT, which is equivalent to LPT. This is just like the CPT/PPT case. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com It'd be probably good to have drm/i915/bdw: in the subject to ease back porting for product groups, and add those patches to a list someone maintains (Rodrigo?) Forgotten your r-b tag or intentionally left blank? -Daniel -- Damien --- drivers/gpu/drm/i915/intel_pm.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index c8f744c..b3e948f 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5595,6 +5595,8 @@ static void gen8_init_clock_gating(struct drm_device *dev) /* Wa4x4STCOptimizationDisable:bdw */ I915_WRITE(CACHE_MODE_1, _MASKED_BIT_ENABLE(GEN8_4x4_STC_OPTIMIZATION_DISABLE)); + + lpt_init_clock_gating(dev); } static void haswell_init_clock_gating(struct drm_device *dev) -- 2.0.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- 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/3] drm/i915: call lpt_init_clock_gating on BDW too
On Tue, Aug 26, 2014 at 07:16:34PM +0200, Daniel Vetter wrote: On Thu, Aug 21, 2014 at 10:50:38PM +0100, Damien Lespiau wrote: On Thu, Aug 21, 2014 at 05:09:36PM -0300, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com Because BDW has WPT, which is equivalent to LPT. This is just like the CPT/PPT case. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com It'd be probably good to have drm/i915/bdw: in the subject to ease back porting for product groups, and add those patches to a list someone maintains (Rodrigo?) Forgotten your r-b tag or intentionally left blank? It's in 20140821214159.ga29...@strange.ger.corp.intel.com -- Damien ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: don't warn if backlight unexpectedly enabled
On Tue, Aug 26, 2014 at 04:15:53PM +, Scot Doyle wrote: On Tue, 26 Aug 2014, Daniel Vetter wrote: On Thu, Aug 21, 2014 at 07:12:59AM +, Scot Doyle wrote: When we enter intel_modeset_setup_hw_state during resume - BLC_PWM_CPU_CTL2 == BLM_PWM_ENABLE - the physical backlight is off Hm, this is actually interesting - we have some other evidence that the best way to shut off the backlight is actually to just set the pwm duty cycle to 0. Can you please check that this is the case for your system? /sys/class/backlight/intel_backlight/brightness 0 - backlight not visible 1 - backlight visible 937 - max backlight Setting /sys/class/backlight/intel_backlight/brightness to 0 updates BLC_PWM_CPU_CTL, but BLC_PWM_CPU_CTL2 remains 0xe000. Maybe we just need to extend the check to look for !PWM_ENABLE || duty_cycle == 0. The following measurements hold true no matter the duty cycle before suspend: When entering hsw_enable_pc8 during suspend - the physical backlight is off - BLC_PWM_CPU_CTL == 0x3a90 (BACKLIGHT_DUTY_CYCLE_MASK == ) - BLC_PWM_CPU_CTL2 == 0x6000 (BLM_PWM_ENABLE) When exiting hsw_disable_pc8 during resume - the physical backlight is off - BLC_PWM_CPU_CTL == 0x200 - BLC_PWM_CPU_CTL2 == 0x8000 (BLM_PWM_ENABLE | BLM_TRANSCODER_EDP) When entering pch_enable_backlight during resume - the physical backlight is off - BLC_PWM_CPU_CTL == 0x200 - BLC_PWM_CPU_CTL2 == 0x8000 (BLM_PWM_ENABLE) When exiting pch_enable_backlight during resume - the physical backlight is off - BLC_PWM_CPU_CTL == duty cycle prior to suspend - BLC_PWM_CPU_CTL2 == 0xe000 (BLM_PWM_ENABLE | BLM_TRANSCODER_EDP) So the BIOS is setting BLC_PWM_CPU_CTL=0x200 and BLC_PWM_CPU_CTL2=0x8000 ? Indeed the bios seems to just but gunk into that register. And if we add in all the knobs there's piles of them (you have semi-duplicated backlight registers on hsw on the PCH), so I guess it doesn't make sense to combine them all and warn if something goes awry, at least not in a -fixes patch. So Reviewed-by: Daniel Vetter daniel.vet...@ffwll.ch on your original patch. Jani can decide whether he wants to save this WARN_ON (imo it's useful to have such sanity-checks) in -next by taking all the various bits and duty cycles into account. But maybe just on the latest platforms, that still should give is good coverage, but with a lot less fuss. Thanks for tracking this all down. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: don't warn if backlight unexpectedly enabled
On Tue, Aug 26, 2014 at 07:33:25PM +0200, Daniel Vetter wrote: On Tue, Aug 26, 2014 at 04:15:53PM +, Scot Doyle wrote: On Tue, 26 Aug 2014, Daniel Vetter wrote: On Thu, Aug 21, 2014 at 07:12:59AM +, Scot Doyle wrote: When we enter intel_modeset_setup_hw_state during resume - BLC_PWM_CPU_CTL2 == BLM_PWM_ENABLE - the physical backlight is off Hm, this is actually interesting - we have some other evidence that the best way to shut off the backlight is actually to just set the pwm duty cycle to 0. Can you please check that this is the case for your system? /sys/class/backlight/intel_backlight/brightness 0 - backlight not visible 1 - backlight visible 937 - max backlight Setting /sys/class/backlight/intel_backlight/brightness to 0 updates BLC_PWM_CPU_CTL, but BLC_PWM_CPU_CTL2 remains 0xe000. Maybe we just need to extend the check to look for !PWM_ENABLE || duty_cycle == 0. The following measurements hold true no matter the duty cycle before suspend: When entering hsw_enable_pc8 during suspend - the physical backlight is off - BLC_PWM_CPU_CTL == 0x3a90 (BACKLIGHT_DUTY_CYCLE_MASK == ) - BLC_PWM_CPU_CTL2 == 0x6000 (BLM_PWM_ENABLE) When exiting hsw_disable_pc8 during resume - the physical backlight is off - BLC_PWM_CPU_CTL == 0x200 - BLC_PWM_CPU_CTL2 == 0x8000 (BLM_PWM_ENABLE | BLM_TRANSCODER_EDP) When entering pch_enable_backlight during resume - the physical backlight is off - BLC_PWM_CPU_CTL == 0x200 - BLC_PWM_CPU_CTL2 == 0x8000 (BLM_PWM_ENABLE) When exiting pch_enable_backlight during resume - the physical backlight is off - BLC_PWM_CPU_CTL == duty cycle prior to suspend - BLC_PWM_CPU_CTL2 == 0xe000 (BLM_PWM_ENABLE | BLM_TRANSCODER_EDP) So the BIOS is setting BLC_PWM_CPU_CTL=0x200 and BLC_PWM_CPU_CTL2=0x8000 ? Indeed the bios seems to just but gunk into that register. And if we add in all the knobs there's piles of them (you have semi-duplicated backlight registers on hsw on the PCH), so I guess it doesn't make sense to combine them all and warn if something goes awry, at least not in a -fixes patch. So Reviewed-by: Daniel Vetter daniel.vet...@ffwll.ch on your original patch. Jani can decide whether he wants to save this WARN_ON (imo it's useful to have such sanity-checks) in -next by taking all the various bits and duty cycles into account. But maybe just on the latest platforms, that still should give is good coverage, but with a lot less fuss. Out of curiosity: What are the PCH copies of the backlight registers doing after resume? -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/4] lib/intel_* Use igt checks and macros
Various stuff all over. Most done with the igt.cocci spatch, but with a few fixups by hand. And add igt_core.h includes where needed. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- lib/igt_fb.c| 2 +- lib/intel_chipset.c | 17 ++--- lib/intel_iosf.c| 8 lib/intel_mmio.c| 42 -- lib/intel_os.c | 16 lib/intel_reg_map.c | 6 +++--- 6 files changed, 38 insertions(+), 53 deletions(-) diff --git a/lib/igt_fb.c b/lib/igt_fb.c index b8448c86d5b5..71d9a26a24c5 100644 --- a/lib/igt_fb.c +++ b/lib/igt_fb.c @@ -455,7 +455,7 @@ igt_create_fb_with_bo_size(int fd, int width, int height, * The kms id of the created framebuffer. */ unsigned int igt_create_fb(int fd, int width, int height, uint32_t format, - unsigned int tiling, struct igt_fb *fb) + unsigned tiling, struct igt_fb *fb) { return igt_create_fb_with_bo_size(fd, width, height, format, tiling, fb, 0); } diff --git a/lib/intel_chipset.c b/lib/intel_chipset.c index 54d55ac00811..0828e444b7b0 100644 --- a/lib/intel_chipset.c +++ b/lib/intel_chipset.c @@ -39,6 +39,7 @@ #include i915_drm.h #include intel_chipset.h +#include igt_core.h /** * SECTION:intel_chipset @@ -74,11 +75,8 @@ intel_get_pci_device(void) int error; error = pci_system_init(); - if (error != 0) { - fprintf(stderr, Couldn't initialize PCI system: %s\n, - strerror(error)); - exit(1); - } + igt_fail_on_f(error != 0, + Couldn't initialize PCI system\n); /* Grab the graphics card. Try the canonical slot first, then * walk the entire PCI bus for a matching device. */ @@ -105,11 +103,8 @@ intel_get_pci_device(void) errx(1, Couldn't find graphics card); error = pci_device_probe(pci_dev); - if (error != 0) { - fprintf(stderr, Couldn't probe graphics card: %s\n, - strerror(error)); - exit(1); - } + igt_fail_on_f(error != 0, + Couldn't probe graphics card\n); if (pci_dev-vendor_id != 0x8086) errx(1, Graphics card is non-intel); @@ -145,7 +140,7 @@ intel_get_drm_devid(int fd) gp.value = (int *)devid; ret = ioctl(fd, DRM_IOCTL_I915_GETPARAM, gp, sizeof(gp)); - assert(ret == 0); + igt_assert(ret == 0); errno = 0; } diff --git a/lib/intel_iosf.c b/lib/intel_iosf.c index 2f1ef90cdd3d..27134a07319e 100644 --- a/lib/intel_iosf.c +++ b/lib/intel_iosf.c @@ -3,8 +3,10 @@ #include stdio.h #include err.h #include errno.h + #include intel_io.h #include intel_reg.h +#include igt_core.h #define TIMEOUT_US 50 @@ -33,8 +35,7 @@ static int vlv_sideband_rw(uint32_t port, uint8_t opcode, uint32_t addr, (bar IOSF_BAR_SHIFT); if (intel_register_read(VLV_IOSF_DOORBELL_REQ) IOSF_SB_BUSY) { - fprintf(stderr, warning: pcode (%s) mailbox access failed\n, - is_read ? read : write); + igt_warn(warning: pcode (%s) mailbox access failed\n, is_read ? read : write); return -EAGAIN; } @@ -51,8 +52,7 @@ static int vlv_sideband_rw(uint32_t port, uint8_t opcode, uint32_t addr, timeout TIMEOUT_US); if (timeout = TIMEOUT_US) { - fprintf(stderr, timeout waiting for pcode %s (%d) to finish\n, - is_read ? read : write, addr); + igt_warn(timeout waiting for pcode %s (%d) to finish\n, is_read ? read : write, addr); return -ETIMEDOUT; } diff --git a/lib/intel_mmio.c b/lib/intel_mmio.c index 45f39a46aff9..c95ae583619f 100644 --- a/lib/intel_mmio.c +++ b/lib/intel_mmio.c @@ -42,6 +42,7 @@ #include sys/mman.h #include intel_io.h +#include igt_core.h #include igt_debugfs.h #include intel_chipset.h @@ -93,18 +94,13 @@ intel_mmio_use_dump_file(char *file) struct stat st; fd = open(file, O_RDWR); - if (fd == -1) { - fprintf(stderr, Couldn't open %s: %s\n, file, - strerror(errno)); - exit(1); - } + igt_fail_on_f(fd == -1, + Couldn't open %s\n, file); + fstat(fd, st); mmio = mmap(NULL, st.st_size, PROT_READ|PROT_WRITE, MAP_PRIVATE, fd, 0); - if (mmio == MAP_FAILED) { - fprintf(stderr, Couldn't mmap %s: %s\n, file, - strerror(errno)); - exit(1); - } + igt_fail_on_f(mmio == MAP_FAILED, + Couldn't mmap %s\n, file); close(fd); } @@ -145,11 +141,8 @@ intel_mmio_use_pci_bar(struct pci_device *pci_dev) PCI_DEV_MAP_FLAG_WRITABLE,
[Intel-gfx] [PATCH 4/4] lib: Use igt macros more
Stragglers. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- lib/instdone.c | 5 +++-- lib/intel_batchbuffer.c | 32 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/lib/instdone.c b/lib/instdone.c index 99857e2033bd..51cdff9bd22b 100644 --- a/lib/instdone.c +++ b/lib/instdone.c @@ -30,6 +30,7 @@ #include intel_chipset.h #include intel_reg.h +#include igt_core.h /* INSTDONE */ # define IDCT_DONE (1 30) @@ -279,7 +280,7 @@ int num_instdone_bits = 0; static void add_instdone_bit(uint32_t reg, uint32_t bit, const char *name) { - assert(num_instdone_bits MAX_INSTDONE_BITS); + igt_assert(num_instdone_bits MAX_INSTDONE_BITS); instdone_bits[num_instdone_bits].reg = reg; instdone_bits[num_instdone_bits].bit = bit; instdone_bits[num_instdone_bits].name = name; @@ -587,7 +588,7 @@ init_instdone_definitions(uint32_t devid) gen3_instdone_bit(MAP_FILTER_DONE, Map filter); gen3_instdone_bit(MAP_L2_IDLE, Map L2); } else { - assert(IS_GEN2(devid)); + igt_assert(IS_GEN2(devid)); gen3_instdone_bit(I830_GMBUS_DONE, GMBUS); gen3_instdone_bit(I830_FBC_DONE, FBC); gen3_instdone_bit(I830_BINNER_DONE, BINNER); diff --git a/lib/intel_batchbuffer.c b/lib/intel_batchbuffer.c index 2c253d5a7171..46e0b33eb6dc 100644 --- a/lib/intel_batchbuffer.c +++ b/lib/intel_batchbuffer.c @@ -193,13 +193,13 @@ intel_batchbuffer_flush_with_context(struct intel_batchbuffer *batch, return; ret = drm_intel_bo_subdata(batch-bo, 0, used, batch-buffer); - assert(ret == 0); + igt_assert(ret == 0); batch-ptr = NULL; ret = drm_intel_gem_bo_context_exec(batch-bo, context, used, I915_EXEC_RENDER); - assert(ret == 0); + igt_assert(ret == 0); intel_batchbuffer_reset(batch); } @@ -246,10 +246,9 @@ intel_batchbuffer_emit_reloc(struct intel_batchbuffer *batch, int ret; if (batch-ptr - batch-buffer BATCH_SZ) - printf(bad relocation ptr %p map %p offset %d size %d\n, - batch-ptr, batch-buffer, - (int)(batch-ptr - batch-buffer), - BATCH_SZ); + igt_info(bad relocation ptr %p map %p offset %d size %d\n, +batch-ptr, batch-buffer, +(int)(batch-ptr - batch-buffer), BATCH_SZ); if (fenced) ret = drm_intel_bo_emit_reloc_fence(batch-bo, batch-ptr - batch-buffer, @@ -260,7 +259,7 @@ intel_batchbuffer_emit_reloc(struct intel_batchbuffer *batch, buffer, delta, read_domains, write_domain); intel_batchbuffer_emit_dword(batch, buffer-offset + delta); - assert(ret == 0); + igt_assert(ret == 0); } /** @@ -276,7 +275,7 @@ void intel_batchbuffer_data(struct intel_batchbuffer *batch, const void *data, unsigned int bytes) { - assert((bytes 3) == 0); + igt_assert((bytes 3) == 0); intel_batchbuffer_require_space(batch, bytes); memcpy(batch-ptr, data, bytes); batch-ptr += bytes; @@ -336,16 +335,17 @@ intel_blt_copy(struct intel_batchbuffer *batch, XY_SRC_COPY_BLT_WRITE_RGB; break; default: - abort(); + igt_fail(1); } #define CHECK_RANGE(x) ((x) = 0 (x) (1 15)) - assert(CHECK_RANGE(src_x1) CHECK_RANGE(src_y1) - CHECK_RANGE(dst_x1) CHECK_RANGE(dst_y1) - CHECK_RANGE(width) CHECK_RANGE(height) - CHECK_RANGE(src_x1 + width) CHECK_RANGE(src_y1 + height) - CHECK_RANGE(dst_x1 + width) CHECK_RANGE(dst_y1 + height) - CHECK_RANGE(src_pitch) CHECK_RANGE(dst_pitch)); + igt_assert(CHECK_RANGE(src_x1) CHECK_RANGE(src_y1) + CHECK_RANGE(dst_x1) CHECK_RANGE(dst_y1) + CHECK_RANGE(width) CHECK_RANGE(height) + CHECK_RANGE(src_x1 + width) CHECK_RANGE(src_y1 + height) + CHECK_RANGE(dst_x1 + width) CHECK_RANGE(dst_y1 + +height) + CHECK_RANGE(src_pitch) CHECK_RANGE(dst_pitch)); #undef CHECK_RANGE BEGIN_BATCH(intel_gen(batch-devid) = 8 ? 10 : 8); @@ -383,7 +383,7 @@ intel_copy_bo(struct intel_batchbuffer *batch, drm_intel_bo *dst_bo, drm_intel_bo *src_bo, long int size) { - assert(size % 4096 == 0); + igt_assert(size % 4096 == 0); intel_blt_copy(batch, src_bo, 0, 0, 4096, -- 2.0.1 ___ Intel-gfx mailing list
[Intel-gfx] [PATCH 3/4] lib/igt_* Use igt macros in igt libaries
Except in igt_core since that would lead to some hilarious recursions. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- lib/igt_aux.c | 9 - lib/igt_debugfs.c | 14 +++--- lib/igt_kms.c | 43 ++- 3 files changed, 29 insertions(+), 37 deletions(-) diff --git a/lib/igt_aux.c b/lib/igt_aux.c index 3357f1f09942..5ddc8b610895 100644 --- a/lib/igt_aux.c +++ b/lib/igt_aux.c @@ -233,7 +233,7 @@ void igt_progress(const char *header, uint64_t i, uint64_t total) return; if (i+1 = total) { - fprintf(stderr, \r%s100%%\n, header); + igt_warn(\r%s100%%\n, header); return; } @@ -241,10 +241,9 @@ void igt_progress(const char *header, uint64_t i, uint64_t total) divider = 1; /* only bother updating about every 0.5% */ - if (i % (total / divider) == 0 || i+1 = total) { - fprintf(stderr, \r%s%3llu%%, header, - (long long unsigned) i * 100 / total); - } + igt_warn_on_f(i % (total / divider) == 0 || i + 1 = total, + \r%s%3llu%%, header, + (long long unsigned)i * 100 / total); } /* mappable aperture trasher helper */ diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c index f6f509dd1875..123cf13e9b22 100644 --- a/lib/igt_debugfs.c +++ b/lib/igt_debugfs.c @@ -200,10 +200,10 @@ bool igt_crc_is_null(igt_crc_t *crc) int i; for (i = 0; i crc-n_words; i++) { - if (crc-crc[i] == 0x) - igt_warn(Suspicious CRC: it looks like the CRC -read back was from a register in a powered -down well\n); + igt_warn_on_f(crc-crc[i] == 0x, + Suspicious CRC: it looks like the CRC + read back was from a register in a powered + down well\n); if (crc-crc[i]) return false; } @@ -731,7 +731,7 @@ void igt_set_stop_rings(enum stop_ring_flags flags) stop_rings_write(flags); current = igt_get_stop_rings(); - if (current != flags) - igt_warn(i915_ring_stop readback mismatch 0x%x vs 0x%x\n, -flags, current); + igt_warn_on_f(current != flags, + i915_ring_stop readback mismatch 0x%x vs 0x%x\n, + flags, current); } diff --git a/lib/igt_kms.c b/lib/igt_kms.c index a414d960d24b..0497042eac0c 100644 --- a/lib/igt_kms.c +++ b/lib/igt_kms.c @@ -202,23 +202,15 @@ void kmstest_dump_mode(drmModeModeInfo *mode) { const char *stereo = mode_stereo_name(mode); - printf( %s %d %d %d %d %d %d %d %d %d 0x%x 0x%x %d%s%s%s\n, - mode-name, - mode-vrefresh, - mode-hdisplay, - mode-hsync_start, - mode-hsync_end, - mode-htotal, - mode-vdisplay, - mode-vsync_start, - mode-vsync_end, - mode-vtotal, - mode-flags, - mode-type, - mode-clock, - stereo ? (3D: : , - stereo ? stereo : , - stereo ? ) : ); + igt_info( %s %d %d %d %d %d %d %d %d %d 0x%x 0x%x %d%s%s%s\n, +mode-name, mode-vrefresh, +mode-hdisplay, mode-hsync_start, +mode-hsync_end, mode-htotal, +mode-vdisplay, mode-vsync_start, +mode-vsync_end, mode-vtotal, +mode-flags, mode-type, mode-clock, +stereo ? (3D: : , +stereo ? stereo : , stereo ? ) : ); fflush(stdout); } @@ -438,8 +430,8 @@ bool kmstest_get_connector_default_mode(int drm_fd, drmModeConnector *connector, int i; if (!connector-count_modes) { - fprintf(stderr, no modes for connector %d\n, - connector-connector_id); + igt_warn(no modes for connector %d\n, +connector-connector_id); return false; } @@ -476,7 +468,7 @@ bool kmstest_get_connector_config(int drm_fd, uint32_t connector_id, resources = drmModeGetResources(drm_fd); if (!resources) { - perror(drmModeGetResources failed); + igt_warn(drmModeGetResources failed); goto err1; } @@ -489,13 +481,13 @@ bool kmstest_get_connector_config(int drm_fd, uint32_t connector_id, goto err3; if (!connector-count_modes) { - fprintf(stderr, connector %d has no modes\n, connector_id); + igt_warn(connector %d has no modes\n, connector_id); goto err3; } if (connector-connector_id != connector_id) { - fprintf(stderr, connector
[Intel-gfx] [PATCH 1/4] lib/rendercopy*: Use igt_assert
--- lib/media_fill_gen7.c | 8 lib/media_fill_gen8.c | 8 lib/media_fill_gen8lp.c | 8 lib/rendercopy_gen6.c | 4 ++-- lib/rendercopy_gen7.c | 8 lib/rendercopy_gen8.c | 10 +- 6 files changed, 23 insertions(+), 23 deletions(-) diff --git a/lib/media_fill_gen7.c b/lib/media_fill_gen7.c index 82c34692a3d8..5a23b7d323c1 100644 --- a/lib/media_fill_gen7.c +++ b/lib/media_fill_gen7.c @@ -67,7 +67,7 @@ gen7_render_flush(struct intel_batchbuffer *batch, uint32_t batch_end) if (ret == 0) ret = drm_intel_bo_mrb_exec(batch-bo, batch_end, NULL, 0, 0, 0); - assert(ret == 0); + igt_assert(ret == 0); } static uint32_t @@ -118,7 +118,7 @@ gen7_fill_surface_state(struct intel_batchbuffer *batch, batch_offset(batch, ss) + 4, buf-bo, 0, read_domain, write_domain); - assert(ret == 0); + igt_assert(ret == 0); ss-ss2.height = igt_buf_height(buf) - 1; ss-ss2.width = igt_buf_width(buf) - 1; @@ -330,7 +330,7 @@ gen7_media_fillfunc(struct intel_batchbuffer *batch, curbe_buffer = gen7_fill_curbe_buffer_data(batch, color); interface_descriptor = gen7_fill_interface_descriptor(batch, dst); - assert(batch-ptr batch-buffer[4095]); + igt_assert(batch-ptr batch-buffer[4095]); /* media pipeline */ batch-ptr = batch-buffer; @@ -348,7 +348,7 @@ gen7_media_fillfunc(struct intel_batchbuffer *batch, OUT_BATCH(MI_BATCH_BUFFER_END); batch_end = batch_align(batch, 8); - assert(batch_end BATCH_STATE_SPLIT); + igt_assert(batch_end BATCH_STATE_SPLIT); gen7_render_flush(batch, batch_end); intel_batchbuffer_reset(batch); diff --git a/lib/media_fill_gen8.c b/lib/media_fill_gen8.c index 54309d59f52a..3ca1bfa7c2a8 100644 --- a/lib/media_fill_gen8.c +++ b/lib/media_fill_gen8.c @@ -67,7 +67,7 @@ gen8_render_flush(struct intel_batchbuffer *batch, uint32_t batch_end) if (ret == 0) ret = drm_intel_bo_mrb_exec(batch-bo, batch_end, NULL, 0, 0, 0); - assert(ret == 0); + igt_assert(ret == 0); } static uint32_t @@ -121,7 +121,7 @@ gen8_fill_surface_state(struct intel_batchbuffer *batch, batch_offset(batch, ss) + 8 * 4, buf-bo, 0, read_domain, write_domain); - assert(ret == 0); + igt_assert(ret == 0); ss-ss2.height = igt_buf_height(buf) - 1; ss-ss2.width = igt_buf_width(buf) - 1; @@ -353,7 +353,7 @@ gen8_media_fillfunc(struct intel_batchbuffer *batch, curbe_buffer = gen8_fill_curbe_buffer_data(batch, color); interface_descriptor = gen8_fill_interface_descriptor(batch, dst); - assert(batch-ptr batch-buffer[4095]); + igt_assert(batch-ptr batch-buffer[4095]); /* media pipeline */ batch-ptr = batch-buffer; @@ -371,7 +371,7 @@ gen8_media_fillfunc(struct intel_batchbuffer *batch, OUT_BATCH(MI_BATCH_BUFFER_END); batch_end = batch_align(batch, 8); - assert(batch_end BATCH_STATE_SPLIT); + igt_assert(batch_end BATCH_STATE_SPLIT); gen8_render_flush(batch, batch_end); intel_batchbuffer_reset(batch); diff --git a/lib/media_fill_gen8lp.c b/lib/media_fill_gen8lp.c index 74dc573cf247..7fa37a89351e 100644 --- a/lib/media_fill_gen8lp.c +++ b/lib/media_fill_gen8lp.c @@ -67,7 +67,7 @@ gen8_render_flush(struct intel_batchbuffer *batch, uint32_t batch_end) if (ret == 0) ret = drm_intel_bo_mrb_exec(batch-bo, batch_end, NULL, 0, 0, 0); - assert(ret == 0); + igt_assert(ret == 0); } static uint32_t @@ -121,7 +121,7 @@ gen8_fill_surface_state(struct intel_batchbuffer *batch, batch_offset(batch, ss) + 8 * 4, buf-bo, 0, read_domain, write_domain); - assert(ret == 0); + igt_assert(ret == 0); ss-ss2.height = igt_buf_height(buf) - 1; ss-ss2.width = igt_buf_width(buf) - 1; @@ -345,7 +345,7 @@ gen8lp_media_fillfunc(struct intel_batchbuffer *batch, curbe_buffer = gen8_fill_curbe_buffer_data(batch, color); interface_descriptor = gen8_fill_interface_descriptor(batch, dst); - assert(batch-ptr batch-buffer[4095]); + igt_assert(batch-ptr batch-buffer[4095]); /* media pipeline */ batch-ptr = batch-buffer; @@ -363,7 +363,7 @@ gen8lp_media_fillfunc(struct intel_batchbuffer *batch, OUT_BATCH(MI_BATCH_BUFFER_END); batch_end = batch_align(batch, 8); - assert(batch_end BATCH_STATE_SPLIT); + igt_assert(batch_end BATCH_STATE_SPLIT);
[Intel-gfx] [PATCH] drm/i915/bdw: Add BDW support in the i915 debugfs entry
From: Vedang Patel vedang.pa...@intel.com The patch introduces fixes for the debugfs attributes emitted by the i915 driver for GEN8. Currently, it is not emitting the correct attributes which include the status of RC6 states. Change-Id: Ib2068a0cac9a5wq3f228e547fa1a097ad369d242df Signed-off-by: Vedang Patel vedang.pa...@intel.com --- drivers/gpu/drm/i915/i915_debugfs.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 850fa59..c712566 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1329,7 +1329,7 @@ static int i915_drpc_info(struct seq_file *m, void *unused) if (IS_VALLEYVIEW(dev)) return vlv_drpc_info(m); - else if (IS_GEN6(dev) || IS_GEN7(dev)) + else if (INTEL_INFO(dev)-gen = 6) return gen6_drpc_info(m); else return ironlake_drpc_info(m); -- 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 3/4] lib/igt_* Use igt macros in igt libaries
On Tue, Aug 26, 2014 at 07:36:13PM +0200, Daniel Vetter wrote: diff --git a/lib/igt_kms.c b/lib/igt_kms.c index a414d960d24b..0497042eac0c 100644 --- a/lib/igt_kms.c +++ b/lib/igt_kms.c @@ -202,23 +202,15 @@ void kmstest_dump_mode(drmModeModeInfo *mode) { const char *stereo = mode_stereo_name(mode); - printf( %s %d %d %d %d %d %d %d %d %d 0x%x 0x%x %d%s%s%s\n, -mode-name, -mode-vrefresh, -mode-hdisplay, -mode-hsync_start, -mode-hsync_end, -mode-htotal, -mode-vdisplay, -mode-vsync_start, -mode-vsync_end, -mode-vtotal, -mode-flags, -mode-type, -mode-clock, -stereo ? (3D: : , -stereo ? stereo : , -stereo ? ) : ); + igt_info( %s %d %d %d %d %d %d %d %d %d 0x%x 0x%x %d%s%s%s\n, + mode-name, mode-vrefresh, + mode-hdisplay, mode-hsync_start, + mode-hsync_end, mode-htotal, + mode-vdisplay, mode-vsync_start, + mode-vsync_end, mode-vtotal, + mode-flags, mode-type, mode-clock, + stereo ? (3D: : , + stereo ? stereo : , stereo ? ) : ); fflush(stdout); fflush(stdout) is a non-sequitur when converting to opaque logging. -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] 3.17.0-rc1: WARNING: CPU: 1 PID: 1 at drivers/gpu/drm/i915/i915_irq.c:139 ironlake_enable_display_irq+0x7f/0x90()
On 26.08.2014 13:09, Ville Syrjälä wrote: It's the PCU irq enable. I think I have a patch somewhere for it. Just forgot to send it out. As Daniel complained about Jesse's approach you are invited to post it here so that I can test it too. Regards, Oliver ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: FBC flush nuke for BDW
On Tue, Aug 26, 2014 at 12:54 AM, Daniel Vetter dan...@ffwll.ch wrote: On Tue, Aug 26, 2014 at 2:39 AM, Rodrigo Vivi rodrigo.v...@gmail.com wrote: So I prefer to continue using the HW/ring version we have already working for HSW and merge this v3 to get FBC working at BDW. Well for that we first need to fix up the psr testcase. I really want that. fbc and psr are independend features and tasks prioritization should come from managers and program managers, right?! And then I also want fbc enabled by default, which means you need to rebase/review Ville's patch series to make that work. We have FBC working with issues and protected by parameter on all platforms. On BDW there is no fbc at all. This patch makes FBC state at least go to the same level as we already have FBC working on all other platforms. I don't see a dependency here between this fix and the big FBC rework-fix. This patch isn't enabling FBC by default. But allowing people that want and need to use FBC. Also I really think the fb frontbuffer tracking can be made to work for fbc - if you do it right you should actually end up with fewer frontbuffer flushes than what we currently do by submitting them through rings. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- 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] drm/i915/ilk: special case enabling of PCU_EVENT interrupt
On Tue, 26 Aug 2014 09:23:54 +0200 Daniel Vetter dan...@ffwll.ch wrote: On Mon, Aug 25, 2014 at 04:24:55PM -0700, Jesse Barnes wrote: This happens in irq_postinstall before we've set the pm._irqs_disabled flag, but shouldn't warn. So add a nowarn variant to allow this to happen w/o a backtrace and keep the rest of the IRQ tracking code happy. Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org Shouldn't we instead just move the pm._irqs_disabled = false in i915_dma.c right above the drm_irq_install call? In intel_runtime_pm_restore_interrupts we also set it to false before we call the various hooks. I didn't check on all the paths whether that was safe, we have a lot of warnings. And really this init time thing is a special case, so it made sense to me. Also the commit message is a bit thin on the usual details like which commits introduced this regression, so that maintainers know where to apply this to. I don't have the commit... Oliver do you have it handy? -- Jesse Barnes, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/ilk: special case enabling of PCU_EVENT interrupt
On 26.08.2014 20:52, Jesse Barnes wrote: On Tue, 26 Aug 2014 09:23:54 +0200 Daniel Vetter dan...@ffwll.ch wrote: This happens in irq_postinstall before we've set the pm._irqs_disabled flag, but shouldn't warn. So add a nowarn variant to allow this to happen w/o a backtrace and keep the rest of the IRQ tracking code happy. Also the commit message is a bit thin on the usual details like which commits introduced this regression, so that maintainers know where to apply this to. I don't have the commit... Oliver do you have it handy? Hm - I really can not tell what has been done to introduce this regression. I just saw the warning on my machine after upgrading to 3.17 ... You can ask me about linux/net/can/ but not the drm stuff ;-) Cheers, Oliver ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/ilk: special case enabling of PCU_EVENT interrupt
On Tue, 26 Aug 2014 21:03:11 +0200 Oliver Hartkopp socket...@hartkopp.net wrote: On 26.08.2014 20:52, Jesse Barnes wrote: On Tue, 26 Aug 2014 09:23:54 +0200 Daniel Vetter dan...@ffwll.ch wrote: This happens in irq_postinstall before we've set the pm._irqs_disabled flag, but shouldn't warn. So add a nowarn variant to allow this to happen w/o a backtrace and keep the rest of the IRQ tracking code happy. Also the commit message is a bit thin on the usual details like which commits introduced this regression, so that maintainers know where to apply this to. I don't have the commit... Oliver do you have it handy? Hm - I really can not tell what has been done to introduce this regression. I just saw the warning on my machine after upgrading to 3.17 ... You can ask me about linux/net/can/ but not the drm stuff ;-) I think it was this one Daniel, or the combination of patches around it: commit 95f25beddba2ec9510b249740bacc11eca70cf75 Author: Jesse Barnes jbar...@virtuousgeek.org Date: Fri Jun 20 09:29:22 2014 -0700 drm/i915: set pm._irqs_disabled at IRQ init time -- Jesse Barnes, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/4] lib/igt_* Use igt macros in igt libaries
On Tue, Aug 26, 2014 at 7:48 PM, Chris Wilson ch...@chris-wilson.co.uk wrote: fflush(stdout); fflush(stdout) is a non-sequitur when converting to opaque logging. Indeed, I missed that one. Fixed up locally. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: don't warn if backlight unexpectedly enabled
On Tue, 26 Aug 2014, Daniel Vetter wrote: On Tue, Aug 26, 2014 at 07:33:25PM +0200, Daniel Vetter wrote: Indeed the bios seems to just but gunk into that register. And if we add in all the knobs there's piles of them (you have semi-duplicated backlight registers on hsw on the PCH), so I guess it doesn't make sense to combine them all and warn if something goes awry, at least not in a -fixes patch. So Reviewed-by: Daniel Vetter daniel.vet...@ffwll.ch on your original patch. Jani can decide whether he wants to save this WARN_ON (imo it's useful to have such sanity-checks) in -next by taking all the various bits and duty cycles into account. But maybe just on the latest platforms, that still should give is good coverage, but with a lot less fuss. Out of curiosity: What are the PCH copies of the backlight registers doing after resume? Thanks Daniel, here's the info: When entering hsw_enable_pc8 during suspend - the physical backlight is off - BLC_PWM_CPU_CTL == 0x 3a9 (BACKLIGHT_DUTY_CYCLE_MASK == ) - BLC_PWM_CPU_CTL2 == 0x6000 - BLC_PWM_PCH_CTL == 0x 0 - BLC_PWM_PCH_CTL2 == 0x 3a9 When exiting hsw_disable_pc8 during resume - the physical backlight is off - BLC_PWM_CPU_CTL == 0x 200 - BLC_PWM_CPU_CTL2 == 0x8000 (BLM_PWM_ENABLE) - BLC_PWM_PCH_CTL == 0x8000 - BLC_PWM_PCH_CTL2 == 0x 400 When entering pch_enable_backlight during resume - the physical backlight is off - BLC_PWM_CPU_CTL == 0x 200 - BLC_PWM_CPU_CTL2 == 0x8000 - BLC_PWM_PCH_CTL == 0x8000 - BLC_PWM_PCH_CTL2 == 0x 400 When exiting pch_enable_backlight during resume - the physical backlight is off - BLC_PWM_CPU_CTL == duty cycle prior to suspend - BLC_PWM_CPU_CTL2 == 0xe000 (BLM_PWM_ENABLE | BLM_TRANSCODER_EDP) - BLC_PWM_PCH_CTL == 0x8000 - BLC_PWM_PCH_CTL2 == 0x 3a9 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 1/1] scripts: Add capability to resume interrupted run-tests.sh session
Piglit provides a 'resume' feature that can restart an interrupted test run at the point where it stopped. This patch adds that feature to run_tests.sh. Signed-off-by: Mike Mason michael.w.ma...@intel.com --- scripts/run-tests.sh | 15 +++ 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/scripts/run-tests.sh b/scripts/run-tests.sh index d0e0c67..580f777 100755 --- a/scripts/run-tests.sh +++ b/scripts/run-tests.sh @@ -59,6 +59,8 @@ function print_help { echo -v enable verbose mode echo -x regex exclude tests that match the regular expression echo (can be used more than once) + echo -R resume interrupted test where the partial results + echo are in the directory given by -r echo echo Useful patterns for test filtering are described in tests/NAMING-CONVENTION } @@ -73,7 +75,7 @@ function list_tests { done } -while getopts :dhlr:st:vx: opt; do +while getopts :dhlr:st:vx:R opt; do case $opt in d) download_piglit; exit ;; h) print_help; exit ;; @@ -83,6 +85,7 @@ while getopts :dhlr:st:vx: opt; do t) FILTER=$FILTER -t $OPTARG ;; v) VERBOSE=-v ;; x) EXCLUDE=$EXCLUDE -x $OPTARG ;; + R) RESUME=true ;; :) echo Option -$OPTARG requires an argument. exit 1 @@ -112,11 +115,15 @@ if [ ! -x $PIGLIT ]; then exit 1 fi -mkdir -p $RESULTS - -sudo IGT_TEST_ROOT=$IGT_TEST_ROOT $PIGLIT run igt $RESULTS $VERBOSE $EXCLUDE $FILTER +if [ x$RESUME != x ]; then + sudo IGT_TEST_ROOT=$IGT_TEST_ROOT $PIGLIT resume $RESULTS +else + mkdir -p $RESULTS + sudo IGT_TEST_ROOT=$IGT_TEST_ROOT $PIGLIT run igt $RESULTS $VERBOSE $EXCLUDE $FILTER +fi if [ $SUMMARY == html ]; then $PIGLIT summary html --overwrite $RESULTS/html $RESULTS echo HTML summary has been written to $RESULTS/html/index.html fi + -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: FBC flush nuke for BDW
On Tue, Aug 26, 2014 at 8:38 PM, Rodrigo Vivi rodrigo.v...@gmail.com wrote: On Tue, Aug 26, 2014 at 12:54 AM, Daniel Vetter dan...@ffwll.ch wrote: On Tue, Aug 26, 2014 at 2:39 AM, Rodrigo Vivi rodrigo.v...@gmail.com wrote: So I prefer to continue using the HW/ring version we have already working for HSW and merge this v3 to get FBC working at BDW. Well for that we first need to fix up the psr testcase. I really want that. fbc and psr are independend features and tasks prioritization should come from managers and program managers, right?! And then I also want fbc enabled by default, which means you need to rebase/review Ville's patch series to make that work. We have FBC working with issues and protected by parameter on all platforms. On BDW there is no fbc at all. This patch makes FBC state at least go to the same level as we already have FBC working on all other platforms. I don't see a dependency here between this fix and the big FBC rework-fix. This patch isn't enabling FBC by default. But allowing people that want and need to use FBC. It's going to be the same answer for both parts - I don't really like if we add features but don't complete them (so testcases and enabled by default). And in our long meeting today a lot of people asked my why I'm reluctant to merge patches so that we can clean them up in-tree (which I agree is often the much more efficient approach). Reactions like yours here are pretty much the reason for that - getting something in to make an internal customer happy or check of a box in our tracking or some other requirement and then move on right away. And I know that you yourself are in a very bad spot trenched between me and requests from management and project tracking. And this is also not to single out you at all, it's just what I see all over. -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