Re: [Intel-gfx] [PATCH] drm/i915/bdw: Render moot context reset and switch with Execlists

2014-08-26 Thread Chris Wilson
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

2014-08-26 Thread Chris Wilson
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

2014-08-26 Thread Chris Wilson
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

2014-08-26 Thread Wendy Wang
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

2014-08-26 Thread Daniel Vetter
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?

2014-08-26 Thread Daniel Vetter
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)

2014-08-26 Thread Daniel Vetter
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

2014-08-26 Thread Daniel Vetter
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()

2014-08-26 Thread Daniel Vetter
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

2014-08-26 Thread Daniel Vetter
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

2014-08-26 Thread Daniel Vetter
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

2014-08-26 Thread Daniel Vetter
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

2014-08-26 Thread Daniel Vetter
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

2014-08-26 Thread Daniel Vetter
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

2014-08-26 Thread Daniel Vetter
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

2014-08-26 Thread Daniel Vetter
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

2014-08-26 Thread Imre Deak
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()

2014-08-26 Thread Daniel Vetter
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

2014-08-26 Thread Siluvery, Arun

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

2014-08-26 Thread Arun Siluvery
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

2014-08-26 Thread Arun Siluvery
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

2014-08-26 Thread Arun Siluvery
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

2014-08-26 Thread Daniel Vetter
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

2014-08-26 Thread Daniel Vetter
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

2014-08-26 Thread Daniel Vetter
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

2014-08-26 Thread Daniel Vetter
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

2014-08-26 Thread Chris Wilson
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

2014-08-26 Thread Siluvery, Arun

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

2014-08-26 Thread Chris Wilson
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

2014-08-26 Thread Imre Deak
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

2014-08-26 Thread Chris Wilson
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

2014-08-26 Thread Daniel Vetter
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

2014-08-26 Thread Daniel Vetter
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)

2014-08-26 Thread Ville Syrjälä
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()

2014-08-26 Thread Ville Syrjälä
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

2014-08-26 Thread Thierry Reding
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

2014-08-26 Thread Siluvery, Arun

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

2014-08-26 Thread Jani Nikula
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

2014-08-26 Thread Chris Wilson
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()

2014-08-26 Thread Jani Nikula
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 Thread Paulo Zanoni
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

2014-08-26 Thread Jani Nikula
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)

2014-08-26 Thread Jani Nikula
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

2014-08-26 Thread Daniel Vetter
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

2014-08-26 Thread Daniel Vetter
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

2014-08-26 Thread Chris Wilson
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

2014-08-26 Thread Daniel Vetter
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

2014-08-26 Thread Daniel Vetter
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

2014-08-26 Thread Daniel Vetter
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

2014-08-26 Thread Siluvery, Arun

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

2014-08-26 Thread Ville Syrjälä
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 Thread Paulo Zanoni
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

2014-08-26 Thread Chris Wilson
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

2014-08-26 Thread Jani Nikula
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

2014-08-26 Thread Daniel Vetter
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 Thread Paulo Zanoni
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

2014-08-26 Thread Daniel Vetter
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

2014-08-26 Thread Ville Syrjälä
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

2014-08-26 Thread Daniel Vetter
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

2014-08-26 Thread Daniel Vetter
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

2014-08-26 Thread Daniel Vetter
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)

2014-08-26 Thread Eric Rannaud
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

2014-08-26 Thread Damien Lespiau
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

2014-08-26 Thread Damien Lespiau
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

2014-08-26 Thread Arun Siluvery
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

2014-08-26 Thread Arun Siluvery
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

2014-08-26 Thread Arun Siluvery
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

2014-08-26 Thread Chris Wilson
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

2014-08-26 Thread Arun Siluvery
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.

2014-08-26 Thread Daniel Vetter
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

2014-08-26 Thread Siluvery, Arun

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

2014-08-26 Thread Ville Syrjälä
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

2014-08-26 Thread Daniel Vetter
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

2014-08-26 Thread Siluvery, Arun

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

2014-08-26 Thread Ville Syrjälä
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)

2014-08-26 Thread Eric Rannaud
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)

2014-08-26 Thread Eric Rannaud
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

2014-08-26 Thread Ville Syrjälä
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

2014-08-26 Thread Chris Wilson
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

2014-08-26 Thread Scot Doyle
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

2014-08-26 Thread Charles Devereaux
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

2014-08-26 Thread Daniel Vetter
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

2014-08-26 Thread Damien Lespiau
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

2014-08-26 Thread Daniel Vetter
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

2014-08-26 Thread Daniel Vetter
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

2014-08-26 Thread Daniel Vetter
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

2014-08-26 Thread Daniel Vetter
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

2014-08-26 Thread Daniel Vetter
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

2014-08-26 Thread Daniel Vetter
---
 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

2014-08-26 Thread vedang . patel
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

2014-08-26 Thread Chris Wilson
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()

2014-08-26 Thread Oliver Hartkopp
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

2014-08-26 Thread Rodrigo Vivi
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

2014-08-26 Thread Jesse Barnes
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

2014-08-26 Thread Oliver Hartkopp
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

2014-08-26 Thread Jesse Barnes
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

2014-08-26 Thread Daniel Vetter
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

2014-08-26 Thread Scot Doyle
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

2014-08-26 Thread Mike Mason
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

2014-08-26 Thread Daniel Vetter
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


  1   2   >