Re: [Intel-gfx] [PATCH 45/53] drm/i915/bdw: Do not call intel_runtime_pm_get() in an interrupt

2014-07-28 Thread Daniel Vetter
On Sat, Jul 26, 2014 at 11:27:38AM +0100, Chris Wilson wrote:
 On Wed, Jun 18, 2014 at 10:54:13PM +0200, Daniel Vetter wrote:
  On Fri, Jun 13, 2014 at 04:38:03PM +0100, oscar.ma...@intel.com wrote:
   From: Oscar Mateo oscar.ma...@intel.com
   
   Or with a spinlock grabbed, because it might sleep, which is not
   a nice thing to do. Instead, do the runtime_pm get/put together
   with the create/destroy request, and handle the forcewake get/put
   directly.
   
   Signed-off-by: Oscar Mateo oscar.ma...@intel.com
  
  Looks like a fixup that should be squashed into relevant earlier patches.
 
 The whole gen6_gt_force_wake_get() calling intel_runtime_pm_get() is
 broken due to this - we must be able to read registers in atomic
 context!
 
 Please revert c8c8fb33b37766acf6474784b0d5245dab9a1690

force_wake_get can't call runtime_pm_get becuase pm_get can sleep. So if
you want to read registers from atomic context you have to have a runtime
pm reference from someone else.
-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 22/40] drm/i915: Add chv port D TX wells

2014-07-28 Thread Daniel Vetter
On Fri, Jul 25, 2014 at 04:30:29PM +0300, Imre Deak wrote:
 On Sat, 2014-06-28 at 02:04 +0300, ville.syrj...@linux.intel.com wrote:
  From: Ville Syrjälä ville.syrj...@linux.intel.com
  
  Add the TX wells for port D. The Punit subsystem numbers are a total
  guess at this time. Also I'm not sure these even exist. Certainly the
  Punit in current hardware doesn't deal with these.
  
  Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
  ---
   drivers/gpu/drm/i915/i915_reg.h |  4 
   drivers/gpu/drm/i915/intel_pm.c | 23 +++
   2 files changed, 27 insertions(+)
  
  diff --git a/drivers/gpu/drm/i915/i915_reg.h 
  b/drivers/gpu/drm/i915/i915_reg.h
  index 3d1fef4..191df9e 100644
  --- a/drivers/gpu/drm/i915/i915_reg.h
  +++ b/drivers/gpu/drm/i915/i915_reg.h
  @@ -525,6 +525,10 @@ enum punit_power_well {
  PUNIT_POWER_WELL_DPIO_RX0   = 10,
  PUNIT_POWER_WELL_DPIO_RX1   = 11,
  PUNIT_POWER_WELL_DPIO_CMN_D = 12,
  +   /* FIXME: guesswork below */
  +   PUNIT_POWER_WELL_DPIO_TX_D_LANES_01 = 13,
  +   PUNIT_POWER_WELL_DPIO_TX_D_LANES_23 = 14,
  +   PUNIT_POWER_WELL_DPIO_RX2   = 15,
   
  PUNIT_POWER_WELL_NUM,
   };
  diff --git a/drivers/gpu/drm/i915/intel_pm.c 
  b/drivers/gpu/drm/i915/intel_pm.c
  index cae936c..55f3e6b 100644
  --- a/drivers/gpu/drm/i915/intel_pm.c
  +++ b/drivers/gpu/drm/i915/intel_pm.c
  @@ -6540,6 +6540,15 @@ EXPORT_SYMBOL_GPL(i915_release_power_well);
  BIT(POWER_DOMAIN_PORT_DDI_D_4_LANES) |  \
  BIT(POWER_DOMAIN_INIT))
   
  +#define CHV_DPIO_TX_D_LANES_01_POWER_DOMAINS ( \
  +   BIT(POWER_DOMAIN_PORT_DDI_D_2_LANES) |  \
  +   BIT(POWER_DOMAIN_PORT_DDI_D_4_LANES) |  \
  +   BIT(POWER_DOMAIN_INIT))
  +
  +#define CHV_DPIO_TX_D_LANES_23_POWER_DOMAINS ( \
  +   BIT(POWER_DOMAIN_PORT_DDI_D_4_LANES) |  \
 
 Atm, for all other ports we power up all lanes regardless of the actual
 configuration (until the PHY side setup is proved to work fine). So for
 consistency I'd do the same here too. With that change:
 
 Reviewed-by: Imre Deak imre.d...@intel.com

Pulled in all the power well patches Imre reviewed except this one.

Thanks, Daniel

 
  +   BIT(POWER_DOMAIN_INIT))
  +
   static const struct i915_power_well_ops i9xx_always_on_power_well_ops = {
  .sync_hw = i9xx_always_on_power_well_noop,
  .enable = i9xx_always_on_power_well_noop,
  @@ -6757,6 +6766,20 @@ static struct i915_power_well chv_power_wells[] = {
  .ops = vlv_dpio_power_well_ops,
  .data = PUNIT_POWER_WELL_DPIO_TX_C_LANES_23,
  },
  +   {
  +   .name = dpio-tx-d-01,
  +   .domains = CHV_DPIO_TX_D_LANES_01_POWER_DOMAINS |
  +  CHV_DPIO_TX_D_LANES_23_POWER_DOMAINS,
  +   .ops = vlv_dpio_power_well_ops,
  +   .data = PUNIT_POWER_WELL_DPIO_TX_D_LANES_01,
  +   },
  +   {
  +   .name = dpio-tx-d-23,
  +   .domains = CHV_DPIO_TX_D_LANES_01_POWER_DOMAINS |
  +  CHV_DPIO_TX_D_LANES_23_POWER_DOMAINS,
  +   .ops = vlv_dpio_power_well_ops,
  +   .data = PUNIT_POWER_WELL_DPIO_TX_D_LANES_23,
  +   },
   #endif
   };
   
 



 ___
 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 v2] drm/i915/bdw: BDW Software Turbo

2014-07-28 Thread Daniel Vetter
On Fri, Jul 25, 2014 at 05:47:11PM -0700, Sun, Daisy wrote:
 we have reconsidered good suggestions and evaluated performance and 
 complexity again.
 
 Timer Constant callback would continuously wake up CPU and entire
 package, results in lower CPU and package C-state and shorter battery life,
 especially for standby time.

If you shut down the timer when idle this shouldn't be a concern at all.
See the idle infrastructure we have and e.g. the retire work handler. We
could even put the software turbo into the retire work handler ...

 execbuf is a good one, and we had taken it into account too. execbuf
 can happen much more frequent than flips. Synchronization and calculation
 overhead were the main reasons that we tried to avoid using too much IA
 resource to benefit GT.

Yeah, running it at each execbuf is going to be too expensive. But with
the regular timer there should be no need to also do this in flips - it
might badly interfere with the missed-flip boosting patches from Chris
even.

 Here's is a revised version of software turbo for BDW, please take a
 look and see if there's any concern.

Doesn't seem to be attached.

 For software turbo, it can be tough to find out a perfect solution
 , may need some trade-off.

Well, but we've made already quite some nice improvments with the boosting
logic. So I think it can be done, but I agree it's not easy.

 Revised design:
 GT busyness will still be calculated when page_flip comes in, then GT 
 frequency
 will be adjusted accordingly. This point stays the same as previous design.
 For the cases no flip will happen(server or background task with no display 
 activity)
 which is a previous concern,  set GT frequency to RP0(no turbo algorithm 
 interfered in this
 case).
 
 Implementation details:
 1)  Driver start with RP0 as GT frequency.
 2)  When the flip comes, do the regular software turbo busyness calculation.
 Also set a timer with 250ms;  3)  If the flip keep coming in time, keep
 turbo algorithm, reset timer;
 4)  When the timer is fired, set RP frequency to RP0 so that the background 
 task will still be taken
 care of(the RPS boost and idle need to be disabled in this situation).
 5)If the flip comes again, go to 2).
 
 To recap,
 For most common cases, GT will run at a desired frequency as a
 result of software turbo algorithm;
 For background workloads or no flip environment, GT will be running at RP0 
 with
 shorter execution time to extend RC6 and pkg C state residency as long as 
 power
 is concerned.
 
 I'll start with the implementation if all concerns are ironed out.

Ah, I expected a patch ;-) Usually code diffs are much more efficient
communication as a replacement for when you'd use a whiteboard session
with a colocated team. It doesn't need to run nor even compile, just
speudo-code illustrating the main integration points.

Anyway, see above for my comments: No need to integrate with flips if we
do the timer thing correctly.
-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: Rework GPU reset sequence to match driver load thaw

2014-07-28 Thread Daniel Vetter
On Fri, Jul 25, 2014 at 06:05:29PM -0700, Ben Widawsky wrote:
 On Wed, Jul 16, 2014 at 04:05:59PM +0100, alistair.mcau...@intel.com wrote:
  From: McAulay, Alistair alistair.mcau...@intel.com
  
  This patch is to address Daniels concerns over different code during reset:
  
  http://lists.freedesktop.org/archives/intel-gfx/2014-June/047758.html
  
  The reason for aiming as hard as possible to use the exact same code for
  driver load, gpu reset and runtime pm/system resume is that we've simply
  seen too many bugs due to slight variations and unintended omissions.
  
  Tested using igt drv_hangman.
  
  Signed-off-by: McAulay, Alistair alistair.mcau...@intel.com
 
 2 quick comments before I actually do a real review.
 1. Did you actually run this with and without full ppgtt?
 2. I don't think this is actually fulfilling what Daniel is requesting,
 though we can let him comment.

Mostly looks like what I think we need. Comments below.

 3. Did you reall do #1?
 
 Assuming you satisifed #1, can you please post the igt results for the
 permutations (pre patch w/ and w/o ppgtt; post patch w/ and w/o ppgtt)
 
 I really want this data because I spent *a lot* of time with these
 specific areas in the PPGTT work, and I am somewhat skeptical enough of
 the code has changed that this will magically work. I also tried the
 trickiness with the ring handling functions, and never succeeded. Also,
 with the context stuff, I'm simply not convinced it can magically
 vanish. If igt looks good, and Daniel agrees that this is what he
 actually wanted, I will go fishing for corner cases and do the review.

I think the hack in ring_begin might explain why it never worked before.
But fully agreed, we really need to test this well (and fill gaps if igt
misses anything around resets - we don't have any systematic gpu reset
coverage anywhere outside of igt).

 
 Thanks.
 
  ---
   drivers/gpu/drm/i915/i915_gem.c |  2 -
   drivers/gpu/drm/i915/i915_gem_context.c | 42 +
   drivers/gpu/drm/i915/i915_gem_gtt.c | 67 
  +
   drivers/gpu/drm/i915/i915_gem_gtt.h |  3 +-
   drivers/gpu/drm/i915/intel_ringbuffer.c |  4 +-
   5 files changed, 14 insertions(+), 104 deletions(-)
  
  diff --git a/drivers/gpu/drm/i915/i915_gem.c 
  b/drivers/gpu/drm/i915/i915_gem.c
  index ef047bc..b38e086 100644
  --- a/drivers/gpu/drm/i915/i915_gem.c
  +++ b/drivers/gpu/drm/i915/i915_gem.c
  @@ -2590,8 +2590,6 @@ void i915_gem_reset(struct drm_device *dev)
  for_each_ring(ring, dev_priv, i)
  i915_gem_reset_ring_cleanup(dev_priv, ring);
   
  -   i915_gem_context_reset(dev);
  -
  i915_gem_restore_fences(dev);
   }
   
  diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
  b/drivers/gpu/drm/i915/i915_gem_context.c
  index de72a28..d96219f 100644
  --- a/drivers/gpu/drm/i915/i915_gem_context.c
  +++ b/drivers/gpu/drm/i915/i915_gem_context.c
  @@ -372,42 +372,6 @@ err_destroy:
  return ERR_PTR(ret);
   }
   
  -void i915_gem_context_reset(struct drm_device *dev)
  -{
  -   struct drm_i915_private *dev_priv = dev-dev_private;
  -   int i;
  -
  -   /* Prevent the hardware from restoring the last context (which hung) on
  -* the next switch */
  -   for (i = 0; i  I915_NUM_RINGS; i++) {
  -   struct intel_engine_cs *ring = dev_priv-ring[i];
  -   struct intel_context *dctx = ring-default_context;
  -   struct intel_context *lctx = ring-last_context;
  -
  -   /* Do a fake switch to the default context */
  -   if (lctx == dctx)
  -   continue;
  -
  -   if (!lctx)
  -   continue;
  -
  -   if (dctx-legacy_hw_ctx.rcs_state  i == RCS) {
  -   
  WARN_ON(i915_gem_obj_ggtt_pin(dctx-legacy_hw_ctx.rcs_state,
  - 
  get_context_alignment(dev), 0));
  -   /* Fake a finish/inactive */
  -   dctx-legacy_hw_ctx.rcs_state-base.write_domain = 0;
  -   dctx-legacy_hw_ctx.rcs_state-active = 0;
  -   }
  -
  -   if (lctx-legacy_hw_ctx.rcs_state  i == RCS)
  -   
  i915_gem_object_ggtt_unpin(lctx-legacy_hw_ctx.rcs_state);
  -
  -   i915_gem_context_unreference(lctx);
  -   i915_gem_context_reference(dctx);
  -   ring-last_context = dctx;
  -   }
  -}

I don't understand why we no longer need this - after reset we probably
have the default context loaded (if we resue the driver load sequence
exactly), so I expect that we must reset the software tracking
accordingly.

  -
   int i915_gem_context_init(struct drm_device *dev)
   {
  struct drm_i915_private *dev_priv = dev-dev_private;
  @@ -498,10 +462,6 @@ int i915_gem_context_enable(struct drm_i915_private 
  *dev_priv)
  ppgtt-enable(ppgtt);
  }
   
  -   /* FIXME: We should make this work, even in reset */
  -   if (i915_reset_in_progress(dev_priv-gpu_error))
 

Re: [Intel-gfx] [PATCH i-g-t] lib: avoid getopt value conflicts with tests

2014-07-28 Thread Thomas Wood
On 25 July 2014 17:57, Paulo Zanoni przan...@gmail.com wrote:
 2014-07-25 13:08 GMT-03:00 Thomas Wood thomas.w...@intel.com:
 Most tests use a printable character as the value for getopt to return,
 so avoid conflicts by using non-printing values for the standard options.

 Instead of this patch, isn't there any way to verify if the tests are
 using any character that is reserved to these general options?
 Having -r instead of --run-subtest is quite nice. That said, I'm
 not against your patch either.

The only standard short option is -h for --help, which needs to be
fixed in the patch. Adding a warning for conflicting short options and
getopt return values could be added, but probably in a separate patch.




 Signed-off-by: Thomas Wood thomas.w...@intel.com
 ---
  lib/igt_core.c | 23 +++
  1 file changed, 15 insertions(+), 8 deletions(-)

 diff --git a/lib/igt_core.c b/lib/igt_core.c
 index a0c9499..882614a 100644
 --- a/lib/igt_core.c
 +++ b/lib/igt_core.c
 @@ -218,6 +218,13 @@ int num_test_children;
  int test_children_sz;
  bool test_child;

 +enum {
 + OPT_LIST_SUBTESTS,
 + OPT_RUN_SUBTEST,
 + OPT_DEBUG,
 + OPT_HELP
 +};
 +
  __attribute__((format(printf, 1, 2)))
  static void kmsg(const char *format, ...)
  #define KERN_INFO 5
 @@ -320,10 +327,10 @@ static int common_init(int argc, char **argv,
  {
 int c, option_index = 0;
 static struct option long_options[] = {
 -   {list-subtests, 0, 0, 'l'},
 -   {run-subtest, 1, 0, 'r'},
 -   {debug, 0, 0, 'd'},
 -   {help, 0, 0, 'h'},
 +   {list-subtests, 0, 0, OPT_LIST_SUBTESTS},
 +   {run-subtest, 1, 0, OPT_RUN_SUBTEST},
 +   {debug, 0, 0, OPT_DEBUG},
 +   {help, 0, 0, OPT_HELP},
 };
 char *short_opts;
 struct option *combined_opts;
 @@ -370,18 +377,18 @@ static int common_init(int argc, char **argv,
 while ((c = getopt_long(argc, argv, short_opts, combined_opts,
option_index)) != -1) {
 switch(c) {
 -   case 'd':
 +   case OPT_DEBUG:
 igt_log_level = IGT_LOG_DEBUG;
 break;
 -   case 'l':
 +   case OPT_LIST_SUBTESTS:
 if (!run_single_subtest)
 list_subtests = true;
 break;
 -   case 'r':
 +   case OPT_RUN_SUBTEST:
 if (!list_subtests)
 run_single_subtest = strdup(optarg);
 break;
 -   case 'h':
 +   case OPT_HELP:
 print_usage(help_str, false);
 ret = -1;
 goto out;
 --
 1.9.3

 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx



 --
 Paulo Zanoni
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx
 -
 Intel Corporation (UK) Limited
 Registered No. 1134945 (England)
 Registered Office: Pipers Way, Swindon SN3 1RJ
 VAT No: 860 2173 47

 This e-mail and any attachments may contain confidential material for
 the sole use of the intended recipient(s). Any review or distribution
 by others is strictly prohibited. If you are not the intended
 recipient, please contact the sender and delete all copies.

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH] work around warning in i915_gem_gtt

2014-07-28 Thread Pavel Machek

Gcc warns that addr might be used uninitialized. It may not, but I see
why gcc gets confused.

Additionally, hiding code with side-effects inside WARN_ON() argument
seems uncool, so I moved it outside.

Signed-off-by: Pavel Machek pa...@ucw.cz

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 8b3cde7..8fcc974 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1448,7 +1448,7 @@ static void gen6_ggtt_insert_entries(struct 
i915_address_space *vm,
(gen6_gtt_pte_t __iomem *)dev_priv-gtt.gsm + first_entry;
int i = 0;
struct sg_page_iter sg_iter;
-   dma_addr_t addr;
+   dma_addr_t addr = 0;
 
for_each_sg_page(st-sgl, sg_iter, st-nents, 0) {
addr = sg_page_iter_dma_address(sg_iter);
@@ -1462,9 +1462,10 @@ static void gen6_ggtt_insert_entries(struct 
i915_address_space *vm,
 * of NUMA access patterns. Therefore, even with the way we assume
 * hardware should work, we must keep this posting read for paranoia.
 */
-   if (i != 0)
-   WARN_ON(readl(gtt_entries[i-1]) !=
-   vm-pte_encode(addr, level, true));
+   if (i != 0) {
+   unsigned long gtt = readl(gtt_entries[i-1]);
+   WARN_ON(gtt != vm-pte_encode(addr, level, true));
+   }
 
/* This next bit makes the above posting read even more important. We
 * want to flush the TLBs only after we're certain all the PTE updates

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] drm/i915: CONFIG_DRM_I915_UMS

2014-07-28 Thread Paul Bolle
On Sat, 2014-07-26 at 01:44 +0200, Daniel Vetter wrote:
 On Fri, Jul 25, 2014 at 2:14 PM, Paul Bolle pebo...@tiscali.nl wrote:
  Your commit 2225a28fd916 (drm/i915: Ditch UMS config option) is
  included in today's linux-next (ie, next-20140725). It removes the
  Kconfig symbol DRM_I915_UMS.
 
  It didn't remove the two (negative) checks for CONFIG_DRM_I915_UMS.
  These checks are superfluous as they now will always evaluate to true.
  Is the trivial cleanup to remove them already queued somewhere?
 
 No, and intentionally.

So this was not by mistake, which is good to know.

  Actually removing the code for
 user-mode-setting isn't just removing these two blocks,

Just to be clear: I'm only suggesting to remove the two lines reading
#ifndef CONFIG_DRM_I915_UMS

and their corresponding #endif lines.

  but requires
 the gutting of roughly 10k lines splattered all over the driver.
 Essentially all the code that checks for
 !drm_core_check_feature(DRIVER_MODESET) needs to go. That's not quite
 as trivial, and before I do that I want to make really sure that
 really no one misses this option.

 So probably after 3.17 is out the door for a bit.

None of what I brought up is urgent. But I do hope you don't mind me
sending a reminder if these few (preprocessor) lines are staying around
longer than expected.


Paul Bolle

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915: Introduce FBC False Color for debug purposes.

2014-07-28 Thread Ville Syrjälä
On Mon, Jul 07, 2014 at 11:42:04AM -0700, Rodrigo Vivi wrote:
 With this bit enabled, HW changes the color when compressing frames for
 debug purposes.
 
 ALthough the simple way to enable a single bit is over intel_reg_write,
 this value is overwriten on next update_fbc so depending on the workload
 it is not possible to set this bit with intel-gpu-tools. So this patch
 introduces a persistent way to enable false color over debugfs.
 
 v2: Use DEFINE_SIMPLE_ATTRIBUTE as Daniel suggested
 
 Cc: Daniel Vetter daniel.vet...@ffwll.ch
 Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
 ---
  drivers/gpu/drm/i915/i915_debugfs.c | 42 
 +
  drivers/gpu/drm/i915/i915_drv.h |  2 ++
  drivers/gpu/drm/i915/i915_reg.h |  1 +
  drivers/gpu/drm/i915/intel_pm.c |  6 ++
  4 files changed, 51 insertions(+)
 
 diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
 b/drivers/gpu/drm/i915/i915_debugfs.c
 index c1b88a8..b049fc5 100644
 --- a/drivers/gpu/drm/i915/i915_debugfs.c
 +++ b/drivers/gpu/drm/i915/i915_debugfs.c
 @@ -1510,6 +1510,47 @@ static int i915_fbc_status(struct seq_file *m, void 
 *unused)
   return 0;
  }
  
 +static int i915_fbc_fc_get(void *data, u64 *val)
 +{
 + struct drm_device *dev = data;
 + struct drm_i915_private *dev_priv = dev-dev_private;
 +
 + if (INTEL_INFO(dev)-gen  5)
 + return -ENODEV;

Did you test this on ILK/SNB? Bspec says the bit is MBZ before IVB.

-- 
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 03/40] drm/i915: Align chv rps min/max/rpe values

2014-07-28 Thread Ville Syrjälä
On Sat, Jul 12, 2014 at 07:16:15PM +0530, Deepak S wrote:
 
 On Saturday 28 June 2014 04:33 AM, ville.syrj...@linux.intel.com wrote:
  From: Ville Syrjälä ville.syrj...@linux.intel.com
 
  CHV wants even rps opcodes so make sure the min/max/rpe values are also
  even.
 
  Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
  ---
drivers/gpu/drm/i915/i915_debugfs.c |  8 
drivers/gpu/drm/i915/intel_pm.c | 19 ++-
2 files changed, 22 insertions(+), 5 deletions(-)
 
  diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
  b/drivers/gpu/drm/i915/i915_debugfs.c
  index 415010e..9b01e7c 100644
  --- a/drivers/gpu/drm/i915/i915_debugfs.c
  +++ b/drivers/gpu/drm/i915/i915_debugfs.c
  @@ -3566,6 +3566,10 @@ i915_max_freq_set(void *data, u64 val)
  if (IS_VALLEYVIEW(dev)) {
  val = vlv_freq_opcode(dev_priv, val);

  +   /* CHV needs even encode values */
  +   if (IS_CHERRYVIEW(dev))
  +   val = ~1;
  +
  hw_max = dev_priv-rps.max_freq;
  hw_min = dev_priv-rps.min_freq;
  } else {
  @@ -3647,6 +3651,10 @@ i915_min_freq_set(void *data, u64 val)
  if (IS_VALLEYVIEW(dev)) {
  val = vlv_freq_opcode(dev_priv, val);

  +   /* CHV needs even encode values */
  +   if (IS_CHERRYVIEW(dev))
  +   val = ALIGN(val, 2);
  +
  hw_max = dev_priv-rps.max_freq;
  hw_min = dev_priv-rps.min_freq;
  } else {
  diff --git a/drivers/gpu/drm/i915/intel_pm.c 
  b/drivers/gpu/drm/i915/intel_pm.c
  index 10c9c02..e3f23c2 100644
  --- a/drivers/gpu/drm/i915/intel_pm.c
  +++ b/drivers/gpu/drm/i915/intel_pm.c
  @@ -3924,21 +3924,30 @@ static void cherryview_init_gt_powersave(struct 
  drm_device *dev)
  mutex_lock(dev_priv-rps.hw_lock);

  dev_priv-rps.max_freq = cherryview_rps_max_freq(dev_priv);
  +   if (WARN_ON_ONCE(dev_priv-rps.max_freq  1))
  +   dev_priv-rps.max_freq = ~1;
 
 Cannot we use ALIGN Here?

The idea was to round max freq down and min freq up.

 
 Other than this it looks fine
 
 Reviewed-by: Deepak S deepa...@linux.intel.com
 
 
  dev_priv-rps.rp0_freq = dev_priv-rps.max_freq;
  DRM_DEBUG_DRIVER(max GPU freq: %d MHz (%u)\n,
   vlv_gpu_freq(dev_priv, dev_priv-rps.max_freq),
   dev_priv-rps.max_freq);

  -   dev_priv-rps.efficient_freq = cherryview_rps_rpe_freq(dev_priv);
  -   DRM_DEBUG_DRIVER(RPe GPU freq: %d MHz (%u)\n,
  -vlv_gpu_freq(dev_priv, dev_priv-rps.efficient_freq),
  -dev_priv-rps.efficient_freq);
  -
  dev_priv-rps.min_freq = cherryview_rps_min_freq(dev_priv);
  +   if (WARN_ON_ONCE(dev_priv-rps.min_freq  1))
  +   dev_priv-rps.min_freq = ALIGN(dev_priv-rps.min_freq, 2);
  DRM_DEBUG_DRIVER(min GPU freq: %d MHz (%u)\n,
   vlv_gpu_freq(dev_priv, dev_priv-rps.min_freq),
   dev_priv-rps.min_freq);

  +   dev_priv-rps.efficient_freq = cherryview_rps_rpe_freq(dev_priv);
  +   if (WARN_ON_ONCE(dev_priv-rps.min_freq  1))
  +   dev_priv-rps.efficient_freq = ~1;
  +   dev_priv-rps.efficient_freq = clamp(dev_priv-rps.efficient_freq,
  +dev_priv-rps.min_freq,
  +dev_priv-rps.max_freq);
  +   DRM_DEBUG_DRIVER(RPe GPU freq: %d MHz (%u)\n,
  +vlv_gpu_freq(dev_priv, dev_priv-rps.efficient_freq),
  +dev_priv-rps.efficient_freq);
  +
  /* Preserve min/max settings in case of re-init */
  if (dev_priv-rps.max_freq_softlimit == 0)
  dev_priv-rps.max_freq_softlimit = dev_priv-rps.max_freq;

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH i-g-t] lib: check test options for conflicts

2014-07-28 Thread Thomas Wood
Check any test specific options for conflicts with the standard set of
options.

Signed-off-by: Thomas Wood thomas.w...@intel.com
---
 lib/igt_core.c | 46 ++
 1 file changed, 42 insertions(+), 4 deletions(-)

diff --git a/lib/igt_core.c b/lib/igt_core.c
index 0e254e3..f4761ca 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -325,14 +325,16 @@ static int common_init(int argc, char **argv,
   const char *help_str,
   igt_opt_handler_t extra_opt_handler)
 {
-   int c, option_index = 0;
+   int c, option_index = 0, i, x;
static struct option long_options[] = {
{list-subtests, 0, 0, OPT_LIST_SUBTESTS},
{run-subtest, 1, 0, OPT_RUN_SUBTEST},
{debug, 0, 0, OPT_DEBUG},
{help, 0, 0, OPT_HELP},
+   {0, 0, 0, 0}
};
char *short_opts;
+   const char *std_short_opts = h;
struct option *combined_opts;
int extra_opt_count;
int all_opt_count;
@@ -356,10 +358,45 @@ static int common_init(int argc, char **argv,
 
/* First calculate space for all passed-in extra long options */
all_opt_count = 0;
-   while (extra_long_opts  extra_long_opts[all_opt_count].name)
+   while (extra_long_opts  extra_long_opts[all_opt_count].name) {
+
+   /* check for conflicts with standard long option values */
+   for (i = 0; long_options[i].name; i++)
+   if (extra_long_opts[all_opt_count].val == 
long_options[i].val)
+   igt_warn(Conflicting long option values 
between --%s and --%s\n,
+extra_long_opts[all_opt_count].name,
+long_options[i].name);
+
+   /* check for conflicts with short options */
+   if (extra_long_opts[all_opt_count].val != ':'
+strchr(std_short_opts, 
extra_long_opts[all_opt_count].val)) {
+   igt_warn(Conflicting long and short option values 
between --%s and -%s\n,
+extra_long_opts[all_opt_count].name,
+long_options[i].name);
+   }
+
+
all_opt_count++;
+   }
extra_opt_count = all_opt_count;
 
+   /* check for conflicts in extra short options*/
+   for (i = 0; extra_short_opts  extra_short_opts[i]; i++) {
+
+   if (extra_short_opts[i] == ':')
+   continue;
+
+   /* check for conflicts with standard short options */
+   if (strchr(std_short_opts, extra_short_opts[i]))
+   igt_warn(Conflicting short option: -%c\n, 
std_short_opts[i]);
+
+   /* check for conflicts with standard long option values */
+   for (x = 0; long_options[x].name; x++)
+   if (long_options[x].val == extra_short_opts[i])
+   igt_warn(Conflicting short option and long 
option value: --%s and -%c\n,
+long_options[x].name, 
extra_short_opts[i]);
+   }
+
all_opt_count += ARRAY_SIZE(long_options);
 
combined_opts = malloc(all_opt_count * sizeof(*combined_opts));
@@ -370,8 +407,9 @@ static int common_init(int argc, char **argv,
memcpy(combined_opts[extra_opt_count], long_options,
ARRAY_SIZE(long_options) * sizeof(*combined_opts));
 
-   ret = asprintf(short_opts, %sh,
-  extra_short_opts ? extra_short_opts : );
+   ret = asprintf(short_opts, %s%s,
+  extra_short_opts ? extra_short_opts : ,
+  std_short_opts);
assert(ret = 0);
 
while ((c = getopt_long(argc, argv, short_opts, combined_opts,
-- 
1.9.3

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 17/40] drm/i915: Add chv cmnlane power wells

2014-07-28 Thread Ville Syrjälä
On Fri, Jul 25, 2014 at 02:55:00PM +0300, Imre Deak wrote:
 On Sat, 2014-06-28 at 02:04 +0300, ville.syrj...@linux.intel.com wrote:
  From: Ville Syrjälä ville.syrj...@linux.intel.com
  
  CHV has two display PHYs so there are also two cmnlane power wells. Add
  the approriate code to power the wells up/down.
  
  Like on VLV we do the cmnreset assert/deassert and the DPLL refclock
  enabling at approriate times.
  
  This code actually works on my bsw.
  
  Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
  ---
   drivers/gpu/drm/i915/i915_reg.h |  1 +
   drivers/gpu/drm/i915/intel_pm.c | 89 
  +
   2 files changed, 90 insertions(+)
  
  diff --git a/drivers/gpu/drm/i915/i915_reg.h 
  b/drivers/gpu/drm/i915/i915_reg.h
  index d246609..19e68d6 100644
  --- a/drivers/gpu/drm/i915/i915_reg.h
  +++ b/drivers/gpu/drm/i915/i915_reg.h
  @@ -512,6 +512,7 @@ enum punit_power_well {
  PUNIT_POWER_WELL_DPIO_TX_C_LANES_23 = 9,
  PUNIT_POWER_WELL_DPIO_RX0   = 10,
  PUNIT_POWER_WELL_DPIO_RX1   = 11,
  +   PUNIT_POWER_WELL_DPIO_CMN_D = 12,
   
  PUNIT_POWER_WELL_NUM,
   };
  diff --git a/drivers/gpu/drm/i915/intel_pm.c 
  b/drivers/gpu/drm/i915/intel_pm.c
  index e2b956e..f88490b 100644
  --- a/drivers/gpu/drm/i915/intel_pm.c
  +++ b/drivers/gpu/drm/i915/intel_pm.c
  @@ -6200,6 +6200,64 @@ static void vlv_dpio_cmn_power_well_disable(struct 
  drm_i915_private *dev_priv,
  vlv_set_power_well(dev_priv, power_well, false);
   }
   
  +static void chv_dpio_cmn_power_well_enable(struct drm_i915_private 
  *dev_priv,
  +  struct i915_power_well *power_well)
  +{
  +   enum dpio_phy phy;
  +
  +   WARN_ON_ONCE(power_well-data != PUNIT_POWER_WELL_DPIO_CMN_BC 
  +power_well-data != PUNIT_POWER_WELL_DPIO_CMN_D);
  +
  +   /*
  +* Enable the CRI clock source so we can get at the
  +* display and the reference clock for VGA
  +* hotplug / manual detection.
  +*/
  +   if (power_well-data == PUNIT_POWER_WELL_DPIO_CMN_BC) {
  +   phy = DPIO_PHY0;
  +   I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) |
  +  DPLL_REFA_CLK_ENABLE_VLV);
  +   I915_WRITE(DPLL(PIPE_B), I915_READ(DPLL(PIPE_B)) |
  +  DPLL_REFA_CLK_ENABLE_VLV | 
  DPLL_INTEGRATED_CRI_CLK_VLV);
 
 Any reason the two clocks are enabled sequentially? For PHY1 you don't
 do this..

I think I meant to enable the ref clock for both pipes A and B. So the
first rmw should have hit DPLL(PIPE_A).

 In any case:
 Reviewed-by: Imre Deak imre.d...@intel.com
 
  +   } else {
  +   phy = DPIO_PHY1;
  +   I915_WRITE(DPLL(PIPE_C), I915_READ(DPLL(PIPE_C)) |
  +  DPLL_REFA_CLK_ENABLE_VLV | 
  DPLL_INTEGRATED_CRI_CLK_VLV);
  +   }
  +   udelay(1); /* 10ns for cmnreset, 0ns for sidereset */
  +   vlv_set_power_well(dev_priv, power_well, true);
  +
  +   /* Poll for phypwrgood signal */
  +   if (wait_for(I915_READ(DISPLAY_PHY_STATUS)  PHY_POWERGOOD(phy), 1))
  +   DRM_ERROR(Display PHY %d is not power up\n, phy);
  +
  +   I915_WRITE(DISPLAY_PHY_CONTROL,
  +  PHY_COM_LANE_RESET_DEASSERT(phy, 
  I915_READ(DISPLAY_PHY_CONTROL)));
  +}
  +
  +static void chv_dpio_cmn_power_well_disable(struct drm_i915_private 
  *dev_priv,
  +   struct i915_power_well *power_well)
  +{
  +   enum dpio_phy phy;
  +
  +   WARN_ON_ONCE(power_well-data != PUNIT_POWER_WELL_DPIO_CMN_BC 
  +power_well-data != PUNIT_POWER_WELL_DPIO_CMN_D);
  +
  +   if (power_well-data == PUNIT_POWER_WELL_DPIO_CMN_BC) {
  +   phy = DPIO_PHY0;
  +   assert_pll_disabled(dev_priv, PIPE_A);
  +   assert_pll_disabled(dev_priv, PIPE_B);
  +   } else {
  +   phy = DPIO_PHY1;
  +   assert_pll_disabled(dev_priv, PIPE_C);
  +   }
  +
  +   I915_WRITE(DISPLAY_PHY_CONTROL,
  +  PHY_COM_LANE_RESET_ASSERT(phy, 
  I915_READ(DISPLAY_PHY_CONTROL)));
  +
  +   vlv_set_power_well(dev_priv, power_well, false);
  +}
  +
   static void check_power_well_state(struct drm_i915_private *dev_priv,
 struct i915_power_well *power_well)
   {
  @@ -6369,6 +6427,18 @@ EXPORT_SYMBOL_GPL(i915_release_power_well);
  BIT(POWER_DOMAIN_PORT_DDI_C_4_LANES) |  \
  BIT(POWER_DOMAIN_INIT))
   
  +#define CHV_DPIO_CMN_BC_POWER_DOMAINS (\
  +   BIT(POWER_DOMAIN_PORT_DDI_B_2_LANES) |  \
  +   BIT(POWER_DOMAIN_PORT_DDI_B_4_LANES) |  \
  +   BIT(POWER_DOMAIN_PORT_DDI_C_2_LANES) |  \
  +   BIT(POWER_DOMAIN_PORT_DDI_C_4_LANES) |  \
  +   BIT(POWER_DOMAIN_INIT))
  +
  +#define CHV_DPIO_CMN_D_POWER_DOMAINS ( \
  +   BIT(POWER_DOMAIN_PORT_DDI_D_2_LANES) |  \
  +   BIT(POWER_DOMAIN_PORT_DDI_D_4_LANES) |  \
  +   BIT(POWER_DOMAIN_INIT))
  +
   static const struct i915_power_well_ops i9xx_always_on_power_well_ops = {
  .sync_hw = 

Re: [Intel-gfx] [PATCH 22/40] drm/i915: Add chv port D TX wells

2014-07-28 Thread Ville Syrjälä
On Fri, Jul 25, 2014 at 04:30:29PM +0300, Imre Deak wrote:
 On Sat, 2014-06-28 at 02:04 +0300, ville.syrj...@linux.intel.com wrote:
  From: Ville Syrjälä ville.syrj...@linux.intel.com
  
  Add the TX wells for port D. The Punit subsystem numbers are a total
  guess at this time. Also I'm not sure these even exist. Certainly the
  Punit in current hardware doesn't deal with these.
  
  Signed-off-by: Ville Syrjälä ville.syrj...@linux.intel.com
  ---
   drivers/gpu/drm/i915/i915_reg.h |  4 
   drivers/gpu/drm/i915/intel_pm.c | 23 +++
   2 files changed, 27 insertions(+)
  
  diff --git a/drivers/gpu/drm/i915/i915_reg.h 
  b/drivers/gpu/drm/i915/i915_reg.h
  index 3d1fef4..191df9e 100644
  --- a/drivers/gpu/drm/i915/i915_reg.h
  +++ b/drivers/gpu/drm/i915/i915_reg.h
  @@ -525,6 +525,10 @@ enum punit_power_well {
  PUNIT_POWER_WELL_DPIO_RX0   = 10,
  PUNIT_POWER_WELL_DPIO_RX1   = 11,
  PUNIT_POWER_WELL_DPIO_CMN_D = 12,
  +   /* FIXME: guesswork below */
  +   PUNIT_POWER_WELL_DPIO_TX_D_LANES_01 = 13,
  +   PUNIT_POWER_WELL_DPIO_TX_D_LANES_23 = 14,
  +   PUNIT_POWER_WELL_DPIO_RX2   = 15,
   
  PUNIT_POWER_WELL_NUM,
   };
  diff --git a/drivers/gpu/drm/i915/intel_pm.c 
  b/drivers/gpu/drm/i915/intel_pm.c
  index cae936c..55f3e6b 100644
  --- a/drivers/gpu/drm/i915/intel_pm.c
  +++ b/drivers/gpu/drm/i915/intel_pm.c
  @@ -6540,6 +6540,15 @@ EXPORT_SYMBOL_GPL(i915_release_power_well);
  BIT(POWER_DOMAIN_PORT_DDI_D_4_LANES) |  \
  BIT(POWER_DOMAIN_INIT))
   
  +#define CHV_DPIO_TX_D_LANES_01_POWER_DOMAINS ( \
  +   BIT(POWER_DOMAIN_PORT_DDI_D_2_LANES) |  \
  +   BIT(POWER_DOMAIN_PORT_DDI_D_4_LANES) |  \
  +   BIT(POWER_DOMAIN_INIT))
  +
  +#define CHV_DPIO_TX_D_LANES_23_POWER_DOMAINS ( \
  +   BIT(POWER_DOMAIN_PORT_DDI_D_4_LANES) |  \
 
 Atm, for all other ports we power up all lanes regardless of the actual
 configuration (until the PHY side setup is proved to work fine). So for
 consistency I'd do the same here too. With that change:

We do that here too. '.domains = 01 | 23' for both tx-d wells. Or am I
missing something?

 
 Reviewed-by: Imre Deak imre.d...@intel.com
 
  +   BIT(POWER_DOMAIN_INIT))
  +
   static const struct i915_power_well_ops i9xx_always_on_power_well_ops = {
  .sync_hw = i9xx_always_on_power_well_noop,
  .enable = i9xx_always_on_power_well_noop,
  @@ -6757,6 +6766,20 @@ static struct i915_power_well chv_power_wells[] = {
  .ops = vlv_dpio_power_well_ops,
  .data = PUNIT_POWER_WELL_DPIO_TX_C_LANES_23,
  },
  +   {
  +   .name = dpio-tx-d-01,
  +   .domains = CHV_DPIO_TX_D_LANES_01_POWER_DOMAINS |
  +  CHV_DPIO_TX_D_LANES_23_POWER_DOMAINS,
  +   .ops = vlv_dpio_power_well_ops,
  +   .data = PUNIT_POWER_WELL_DPIO_TX_D_LANES_01,
  +   },
  +   {
  +   .name = dpio-tx-d-23,
  +   .domains = CHV_DPIO_TX_D_LANES_01_POWER_DOMAINS |
  +  CHV_DPIO_TX_D_LANES_23_POWER_DOMAINS,
  +   .ops = vlv_dpio_power_well_ops,
  +   .data = PUNIT_POWER_WELL_DPIO_TX_D_LANES_23,
  +   },
   #endif
   };
   
 



-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH i-g-t] lib: don't abort if forcing the connector state fails

2014-07-28 Thread Thomas Wood
Ensure tests using igt_enable_connectors can still run even if the
relevant debugfs files are not available.

Signed-off-by: Thomas Wood thomas.w...@intel.com
---
 lib/igt_kms.c | 19 ++-
 lib/igt_kms.h |  2 +-
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 20370a9..740b5dd 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -429,9 +429,11 @@ static char* get_debugfs_connector_path(int drm_fd, 
drmModeConnector *connector,
  * @state: state to force on @connector
  *
  * Force the specified state on the specified connector.
+ *
+ * Returns: true on success
  */
-void kmstest_force_connector(int drm_fd, drmModeConnector *connector, enum
-kmstest_force_connector_state state)
+bool kmstest_force_connector(int drm_fd, drmModeConnector *connector,
+enum kmstest_force_connector_state state)
 {
char *path;
const char *value;
@@ -458,12 +460,15 @@ void kmstest_force_connector(int drm_fd, drmModeConnector 
*connector, enum
debugfs_fd = open(path, O_WRONLY | O_TRUNC);
free(path);
 
-   igt_assert(debugfs_fd != -1);
+   if (debugfs_fd == -1) {
+   return false;
+   }
 
ret = write(debugfs_fd, value, strlen(value));
close(debugfs_fd);
 
igt_assert(ret != -1);
+   return (ret == -1) ? false : true;
 }
 
 /**
@@ -1509,8 +1514,12 @@ void igt_enable_connectors(void)
continue;
 
/* just enable VGA for now */
-   if (c-connector_type == DRM_MODE_CONNECTOR_VGA)
-   kmstest_force_connector(drm_fd, c, FORCE_CONNECTOR_ON);
+   if (c-connector_type == DRM_MODE_CONNECTOR_VGA) {
+   if (!kmstest_force_connector(drm_fd, c, 
FORCE_CONNECTOR_ON))
+   igt_info(Unable to force state on %s-%d\n,
+
kmstest_connector_type_str(c-connector_type),
+c-connector_type_id);
+   }
 
drmModeFreeConnector(c);
}
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index fb0e66a..08b46ab 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -130,7 +130,7 @@ int kmstest_get_connector_default_mode(int drm_fd, 
drmModeConnector *connector,
 int kmstest_get_connector_config(int drm_fd, uint32_t connector_id,
 unsigned long crtc_idx_mask,
 struct kmstest_connector_config *config);
-void kmstest_force_connector(int fd, drmModeConnector *connector,
+bool kmstest_force_connector(int fd, drmModeConnector *connector,
 enum kmstest_force_connector_state state);
 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


Re: [Intel-gfx] [PATCH] work around warning in i915_gem_gtt

2014-07-28 Thread Daniel Vetter
On Mon, Jul 28, 2014 at 01:20:58PM +0200, Pavel Machek wrote:
 
 Gcc warns that addr might be used uninitialized. It may not, but I see
 why gcc gets confused.
 
 Additionally, hiding code with side-effects inside WARN_ON() argument
 seems uncool, so I moved it outside.
 
 Signed-off-by: Pavel Machek pa...@ucw.cz
 
 diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
 b/drivers/gpu/drm/i915/i915_gem_gtt.c
 index 8b3cde7..8fcc974 100644
 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
 +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
 @@ -1448,7 +1448,7 @@ static void gen6_ggtt_insert_entries(struct 
 i915_address_space *vm,
   (gen6_gtt_pte_t __iomem *)dev_priv-gtt.gsm + first_entry;
   int i = 0;
   struct sg_page_iter sg_iter;
 - dma_addr_t addr;
 + dma_addr_t addr = 0;

I want to have a /* shuts up gcc */ for these kinds of things, where we
need to help out the compiler. Added and merged, thanks.
-Daniel

  
   for_each_sg_page(st-sgl, sg_iter, st-nents, 0) {
   addr = sg_page_iter_dma_address(sg_iter);
 @@ -1462,9 +1462,10 @@ static void gen6_ggtt_insert_entries(struct 
 i915_address_space *vm,
* of NUMA access patterns. Therefore, even with the way we assume
* hardware should work, we must keep this posting read for paranoia.
*/
 - if (i != 0)
 - WARN_ON(readl(gtt_entries[i-1]) !=
 - vm-pte_encode(addr, level, true));
 + if (i != 0) {
 + unsigned long gtt = readl(gtt_entries[i-1]);
 + WARN_ON(gtt != vm-pte_encode(addr, level, true));
 + }
  
   /* This next bit makes the above posting read even more important. We
* want to flush the TLBs only after we're certain all the PTE updates
 
 -- 
 (english) http://www.livejournal.com/~pavelmachek
 (cesky, pictures) 
 http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
 ___
 dri-devel mailing list
 dri-de...@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
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: Add rotation_property to mode_config and creating it

2014-07-28 Thread Ville Syrjälä
On Tue, Jul 15, 2014 at 05:43:37PM +0530, sonika.jin...@intel.com wrote:
 From: Sonika Jindal sonika.jin...@intel.com
 
 v2: Adding creation of rotation_property here.
 
 Signed-off-by: Sonika Jindal sonika.jin...@intel.com
 ---
  drivers/gpu/drm/drm_crtc.c |3 ++-
  include/drm/drm_crtc.h |1 +
  2 files changed, 3 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
 index 787631e..49c0747 100644
 --- a/drivers/gpu/drm/drm_crtc.c
 +++ b/drivers/gpu/drm/drm_crtc.c
 @@ -1299,7 +1299,8 @@ static int 
 drm_mode_create_standard_plane_properties(struct drm_device *dev)
   type, drm_plane_type_enum_list,
   ARRAY_SIZE(drm_plane_type_enum_list));
   dev-mode_config.plane_type_property = type;
 -
 + dev-mode_config.rotation_property = 
 drm_mode_create_rotation_property(dev,
 + BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180));

This might not make sense for other (!i915) hardware. And that's the
reason why I had the driver create the property in the first place.

I think Daniel was thinking that we might want to expose all the bits
regardless of what the hardware supports, but I don't like that idea.
There are other properties (eg. alpha blending, csc stuff, etc.) that
have the same problem of hardware supporting only a (potentially small)
subset of the possible values. I'd rather we didn't make life harder
for userspace when the kernel can already report that certain values
will never work.

   return 0;
  }
  
 diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
 index ce6df4a..5545dd3 100644
 --- a/include/drm/drm_crtc.h
 +++ b/include/drm/drm_crtc.h
 @@ -819,6 +819,7 @@ struct drm_mode_config {
   struct drm_property *dpms_property;
   struct drm_property *path_property;
   struct drm_property *plane_type_property;
 + struct drm_property *rotation_property;
  
   /* DVI-I properties */
   struct drm_property *dvi_i_subconnector_property;
 -- 
 1.7.10.4
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH i-g-t] lib: don't abort if forcing the connector state fails

2014-07-28 Thread Daniel Vetter
On Mon, Jul 28, 2014 at 04:24:49PM +0100, Thomas Wood wrote:
 Ensure tests using igt_enable_connectors can still run even if the
 relevant debugfs files are not available.
 
 Signed-off-by: Thomas Wood thomas.w...@intel.com
 ---
  lib/igt_kms.c | 19 ++-
  lib/igt_kms.h |  2 +-
  2 files changed, 15 insertions(+), 6 deletions(-)
 
 diff --git a/lib/igt_kms.c b/lib/igt_kms.c
 index 20370a9..740b5dd 100644
 --- a/lib/igt_kms.c
 +++ b/lib/igt_kms.c
 @@ -429,9 +429,11 @@ static char* get_debugfs_connector_path(int drm_fd, 
 drmModeConnector *connector,
   * @state: state to force on @connector
   *
   * Force the specified state on the specified connector.
 + *
 + * Returns: true on success
   */
 -void kmstest_force_connector(int drm_fd, drmModeConnector *connector, enum
 -  kmstest_force_connector_state state)
 +bool kmstest_force_connector(int drm_fd, drmModeConnector *connector,
 +  enum kmstest_force_connector_state state)
  {
   char *path;
   const char *value;
 @@ -458,12 +460,15 @@ void kmstest_force_connector(int drm_fd, 
 drmModeConnector *connector, enum
   debugfs_fd = open(path, O_WRONLY | O_TRUNC);
   free(path);
  
 - igt_assert(debugfs_fd != -1);
 + if (debugfs_fd == -1) {
 + return false;
 + }

Aside: We have some neat debugfs helpers in igt_debugfs.c. Might convert
over to them while at it.
-Daniel

  
   ret = write(debugfs_fd, value, strlen(value));
   close(debugfs_fd);
  
   igt_assert(ret != -1);
 + return (ret == -1) ? false : true;
  }
  
  /**
 @@ -1509,8 +1514,12 @@ void igt_enable_connectors(void)
   continue;
  
   /* just enable VGA for now */
 - if (c-connector_type == DRM_MODE_CONNECTOR_VGA)
 - kmstest_force_connector(drm_fd, c, FORCE_CONNECTOR_ON);
 + if (c-connector_type == DRM_MODE_CONNECTOR_VGA) {
 + if (!kmstest_force_connector(drm_fd, c, 
 FORCE_CONNECTOR_ON))
 + igt_info(Unable to force state on %s-%d\n,
 +  
 kmstest_connector_type_str(c-connector_type),
 +  c-connector_type_id);
 + }
  
   drmModeFreeConnector(c);
   }
 diff --git a/lib/igt_kms.h b/lib/igt_kms.h
 index fb0e66a..08b46ab 100644
 --- a/lib/igt_kms.h
 +++ b/lib/igt_kms.h
 @@ -130,7 +130,7 @@ int kmstest_get_connector_default_mode(int drm_fd, 
 drmModeConnector *connector,
  int kmstest_get_connector_config(int drm_fd, uint32_t connector_id,
unsigned long crtc_idx_mask,
struct kmstest_connector_config *config);
 -void kmstest_force_connector(int fd, drmModeConnector *connector,
 +bool kmstest_force_connector(int fd, drmModeConnector *connector,
enum kmstest_force_connector_state state);
  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

-- 
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: s/seqno/request/ tracking inside objects

2014-07-28 Thread Daniel Vetter
On Fri, Jul 25, 2014 at 01:27:00PM +0100, Chris Wilson wrote:
 At the heart of this change is that the seqno is a too low level of an
 abstraction to handle the growing complexities of command tracking, both
 with the introduction of multiple command queues with execbuffer and the
 potential for reordering with a scheduler. On top of the seqno we have
 the request. Conceptually this is just a fence, but it also has
 substantial bookkeeping of its own in order to track the context and
 batch in flight, for example. It is the central structure upon which we
 can extend with dependency tracking et al.
 
 As regards the objects, they were using the seqno as a simple fence,
 upon which is check or even wait upon for command completion. This patch
 exchanges that seqno/ring pair with the request itself. For the
 majority, lifetime of the request is ordered by how we retire objects
 then requests. However, both the unlocked waits and probing elsewhere do
 not tie into the normal request lifetimes and so we need to introduce a
 kref. Extending the objects to use the request as the fence naturally
 extends to segregrating read/write fence tracking. This has significance
 for it reduces the number of semaphores we need to emit, reducing the
 likelihood of #54226, and improving performance overall.
 
 NOTE: this is not against bare drm-intel-nightly and is likely to
 conflict with execlists...
 
 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 Cc: Jesse Barnes jbar...@virtuousgeek.org
 Cc: Daniel Vetter daniel.vet...@ffwll.ch
 Cc: Oscar Mateo oscar.ma...@intel.com
 Cc: Brad Volkin bradley.d.vol...@intel.com

Ok, read through it and I like overall. Also, right now is the perfect
time to merge it since we're right before the merge window. But this here
needs to be split up a bit to cut out prep patches. I've noticed a few
things in-line, but there's also the mechanical stuff (like dropping the
drm_ prefix from requests).
-Daniel

 ---
  drivers/gpu/drm/i915/i915_debugfs.c  |  37 +-
  drivers/gpu/drm/i915/i915_drv.h  | 108 ++--
  drivers/gpu/drm/i915/i915_gem.c  | 769 
 ---
  drivers/gpu/drm/i915/i915_gem_context.c  |  19 +-
  drivers/gpu/drm/i915/i915_gem_exec.c |  10 +-
  drivers/gpu/drm/i915/i915_gem_execbuffer.c   |  37 +-
  drivers/gpu/drm/i915/i915_gem_render_state.c |   5 +-
  drivers/gpu/drm/i915/i915_gem_tiling.c   |   2 +-
  drivers/gpu/drm/i915/i915_gpu_error.c|  35 +-
  drivers/gpu/drm/i915/i915_irq.c  |   6 +-
  drivers/gpu/drm/i915/i915_perf.c |   6 +-
  drivers/gpu/drm/i915/i915_trace.h|   2 +-
  drivers/gpu/drm/i915/intel_display.c |  50 +-
  drivers/gpu/drm/i915/intel_drv.h |   3 +-
  drivers/gpu/drm/i915/intel_overlay.c | 118 ++--
  drivers/gpu/drm/i915/intel_ringbuffer.c  |  83 +--
  drivers/gpu/drm/i915/intel_ringbuffer.h  |  11 +-
  17 files changed, 745 insertions(+), 556 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
 b/drivers/gpu/drm/i915/i915_debugfs.c
 index 406e630..676d5f1 100644
 --- a/drivers/gpu/drm/i915/i915_debugfs.c
 +++ b/drivers/gpu/drm/i915/i915_debugfs.c
 @@ -122,10 +122,11 @@ static inline const char *get_global_flag(struct 
 drm_i915_gem_object *obj)
  static void
  describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj)
  {
 + struct i915_gem_request *rq = i915_gem_object_last_read(obj);
   struct i915_vma *vma;
   int pin_count = 0;
  
 - seq_printf(m, %pK: %s%s%s %8zdKiB %02x %02x %u %u %u%s%s%s,
 + seq_printf(m, %pK: %s%s%s %8zdKiB %02x %02x %x %x %x%s%s%s,
  obj-base,
  get_pin_flag(obj),
  get_tiling_flag(obj),
 @@ -133,9 +134,9 @@ describe_obj(struct seq_file *m, struct 
 drm_i915_gem_object *obj)
  obj-base.size / 1024,
  obj-base.read_domains,
  obj-base.write_domain,
 -obj-last_read_seqno,
 -obj-last_write_seqno,
 -obj-last_fenced_seqno,
 +i915_request_seqno(rq),
 +i915_request_seqno(obj-last_write.request),
 +i915_request_seqno(obj-last_fence.request),
  i915_cache_level_str(obj-cache_level),
  obj-dirty ?  dirty : ,
  obj-madv == I915_MADV_DONTNEED ?  purgeable : );
 @@ -168,8 +169,8 @@ describe_obj(struct seq_file *m, struct 
 drm_i915_gem_object *obj)
   *t = '\0';
   seq_printf(m,  (%s mappable), s);
   }
 - if (obj-ring != NULL)
 - seq_printf(m,  (%s), obj-ring-name);
 + if (rq)
 + seq_printf(m,  (%s), rq-ring-name);
   if (obj-frontbuffer_bits)
   seq_printf(m,  (frontbuffer: 0x%03x), obj-frontbuffer_bits);
  }
 @@ -336,7 +337,7 @@ static int per_file_stats(int id, void *ptr, void *data)
   if (ppgtt-ctx  ppgtt-ctx-file_priv != 
 

[Intel-gfx] [RFC] Move BDW workarounds to ring init fn

2014-07-28 Thread arun . siluvery
From: Arun Siluvery arun.siluv...@linux.intel.com

This patch moves BDW workarounds from init_clock_gating() to render ring
init fn otherwise they are lost when gpu is reset.
In case of execlists, some of the workarounds modify registers that are
part of register state context which doesn't get initialized until
init_clock_gating(); this results in default context with incorrect values
as it is restored and saved before updated by workarounds.

Open issue:
For Wa4x4STCOptimizationDisable, we set CACHE_MODE_1[6:6] = 1
At the time when HW contexts are enabled after rings are initialized with
default context this workaround is valid but followed by a context switch
this is getting reset, please see below log snippet.

...
[5.978209] [drm:i915_pages_create_for_stolen] offset=0x0, size=8294400
[5.978213] [drm:intel_alloc_plane_obj] plane fb obj 8801472e
[5.978215] [drm:i915_gem_setup_global_gtt] reserving preallocated space: 0 
+ 7e9000
[5.978216] [drm:i915_gem_setup_global_gtt] clearing unused GTT space: 
[7e9000, f000]
[5.979613] [drm:i915_gem_init] CACHE_MODE_1: 0x0180
[5.981372] [drm:gen8_ppgtt_init] Allocated 4 pages for page directories (0 
wasted)
[5.981373] [drm:gen8_ppgtt_init] Allocated 2048 pages for page tables (0 
wasted)
[5.981376] [drm:i915_gem_context_init] HW context support initialized
[5.981462] [drm:i915_gem_init_hw] CACHE_MODE_1: 0x0180
[5.981467] [drm:i915_gem_init_rings] CACHE_MODE_1: 0x0180
[5.981704] [drm:bdw_init_workarounds] CACHE_MODE_1: 0x01C0
[5.981716] [drm:init_status_page] bsd ring hws offset: 0x0081e000
[5.981792] [drm:init_status_page] blitter ring hws offset: 0x0083f000
[5.981910] [drm:init_status_page] video enhancement ring hws offset: 
0x0086
[5.982001] [drm:i915_gem_init_hw] CACHE_MODE_1: 0x01C0
[5.982104] [drm:i915_gem_context_enable] Switch render ring to 
default_context
[5.982106] [drm:i915_gem_render_state_init] render ring: Render state init
[5.982120] [drm:do_switch] render ring, CACHE_MODE_1: 0x01C0, 
uninitialized: 1
[5.982121] [drm:i915_gem_context_enable] Switch bsd ring to default_context
[5.982122] [drm:do_switch] bsd ring, CACHE_MODE_1: 0x01C0, 
uninitialized: 0
[5.982123] [drm:i915_gem_context_enable] Switch blitter ring to 
default_context
[5.982126] [drm:do_switch] blitter ring, CACHE_MODE_1: 0x01C0, 
uninitialized: 0
[5.982126] [drm:i915_gem_context_enable] Switch video enhancement ring to 
default_context
[5.982128] [drm:do_switch] video enhancement ring, CACHE_MODE_1: 
0x01C0, uninitialized: 0
[5.982133] [drm:i915_gem_init] CACHE_MODE_1: 0x01C0
[5.982258] [drm:intel_init_clock_gating]
...
[   10.037019] [drm:do_switch] blitter ring, CACHE_MODE_1: 0x0180, 
uninitialized: 0
...
[   10.488145] [drm:do_switch] render ring, CACHE_MODE_1: 0x0180, 
uninitialized: 0
...

I am currently testing this with an igt which triggers a gpu reset and compares
WA register contents before and after reset but the test fails because of
this register hence not sending it now.
Please let me know how to keep this WA valid after a context switch.


Arun Siluvery (1):
  drm/i915/bdw: Initialize BDW workarounds in render ring init fn

 drivers/gpu/drm/i915/i915_debugfs.c | 46 ++
 drivers/gpu/drm/i915/intel_pm.c | 59 
 drivers/gpu/drm/i915/intel_ringbuffer.c | 68 +
 3 files changed, 114 insertions(+), 59 deletions(-)

-- 
1.9.2

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [RFC] drm/i915/bdw: Initialize BDW workarounds in render ring init fn

2014-07-28 Thread arun . siluvery
From: Arun Siluvery arun.siluv...@linux.intel.com

The workarounds at the moment are initialized in init_clock_gating() but
they are lost during reset; In case of execlists some workarounds modify
registers that are part of register state context, since these are not
initialized until init_clock_gating() default context ends up with
incorrect values as render context is restored and saved before updated
by workarounds hence move them to render ring init fn. This should be
ok as these workarounds are not related to display clock gating.

Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com
---
 drivers/gpu/drm/i915/i915_debugfs.c | 46 ++
 drivers/gpu/drm/i915/intel_pm.c | 59 
 drivers/gpu/drm/i915/intel_ringbuffer.c | 68 +
 3 files changed, 114 insertions(+), 59 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 083683c..cf7da30 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2397,20 +2397,65 @@ 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 i915_workaround_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;
+   struct drm_i915_private *dev_priv = dev-dev_private;
+   int ret;
+
+   ret = mutex_lock_interruptible(dev-struct_mutex);
+   if (ret)
+   return ret;
+
+   if (IS_BROADWELL(dev)) {
+   seq_printf(m, GEN8_ROW_CHICKEN:\t0x%08x\n,
+  I915_READ(GEN8_ROW_CHICKEN));
+   seq_printf(m, HALF_SLICE_CHICKEN3:\t0x%08x\n,
+  I915_READ(HALF_SLICE_CHICKEN3));
+   seq_printf(m, GAMTARBMODE:\t0x%08x\n, I915_READ(GAMTARBMODE));
+   seq_printf(m, _3D_CHICKEN3:\t0x%08x\n,
+  I915_READ(_3D_CHICKEN3));
+   seq_printf(m, COMMON_SLICE_CHICKEN2:\t0x%08x\n,
+  I915_READ(COMMON_SLICE_CHICKEN2));
+   seq_printf(m, GEN7_HALF_SLICE_CHICKEN1:\t0x%08x\n,
+  I915_READ(GEN7_HALF_SLICE_CHICKEN1));
+   seq_printf(m, GEN7_ROW_CHICKEN2:\t0x%08x\n,
+  I915_READ(GEN7_ROW_CHICKEN2));
+   seq_printf(m, GAM_ECOCHK:\t0x%08x\n,
+  I915_READ(GAM_ECOCHK));
+   seq_printf(m, HDC_CHICKEN0:\t0x%08x\n,
+  I915_READ(HDC_CHICKEN0));
+   seq_printf(m, GEN7_FF_THREAD_MODE:\t0x%08x\n,
+  I915_READ(GEN7_FF_THREAD_MODE));
+   seq_printf(m, GEN8_UCGCTL6:\t0x%08x\n,
+  I915_READ(GEN8_UCGCTL6));
+   seq_printf(m, GEN6_RC_SLEEP_PSMI_CONTROL:\t0x%08x\n,
+  I915_READ(GEN6_RC_SLEEP_PSMI_CONTROL));
+   seq_printf(m, CACHE_MODE_1:\t0x%08x\n,
+  I915_READ(CACHE_MODE_1));
+   } else
+   DRM_DEBUG_DRIVER(Not available for Gen%d\n,
+INTEL_INFO(dev)-gen);
+
+   mutex_unlock(dev-struct_mutex);
+   return 0;
+}
+
 struct pipe_crc_info {
const char *name;
struct drm_device *dev;
enum pipe pipe;
 };
 
 static int i915_pipe_crc_open(struct inode *inode, struct file *filep)
 {
struct pipe_crc_info *info = inode-i_private;
struct drm_i915_private *dev_priv = info-dev-dev_private;
@@ -3904,20 +3949,21 @@ static const struct drm_info_list i915_debugfs_list[] = 
{
{i915_ppgtt_info, i915_ppgtt_info, 0},
{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_workaround_info, i915_workaround_info, 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/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c

Re: [Intel-gfx] [RFC] drm/i915/bdw: Initialize BDW workarounds in render ring init fn

2014-07-28 Thread Ville Syrjälä
On Mon, Jul 28, 2014 at 05:31:46PM +0100, arun.siluv...@linux.intel.com wrote:
 From: Arun Siluvery arun.siluv...@linux.intel.com
 
 The workarounds at the moment are initialized in init_clock_gating() but
 they are lost during reset; In case of execlists some workarounds modify
 registers that are part of register state context, since these are not
 initialized until init_clock_gating() default context ends up with
 incorrect values as render context is restored and saved before updated
 by workarounds hence move them to render ring init fn. This should be
 ok as these workarounds are not related to display clock gating.
 
 Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com
 ---
  drivers/gpu/drm/i915/i915_debugfs.c | 46 ++
  drivers/gpu/drm/i915/intel_pm.c | 59 
  drivers/gpu/drm/i915/intel_ringbuffer.c | 68 
 +
  3 files changed, 114 insertions(+), 59 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
 b/drivers/gpu/drm/i915/i915_debugfs.c
 index 083683c..cf7da30 100644
 --- a/drivers/gpu/drm/i915/i915_debugfs.c
 +++ b/drivers/gpu/drm/i915/i915_debugfs.c
 @@ -2397,20 +2397,65 @@ 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 i915_workaround_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;
 + struct drm_i915_private *dev_priv = dev-dev_private;
 + int ret;
 +
 + ret = mutex_lock_interruptible(dev-struct_mutex);
 + if (ret)
 + return ret;
 +
 + if (IS_BROADWELL(dev)) {
 + seq_printf(m, GEN8_ROW_CHICKEN:\t0x%08x\n,
 +I915_READ(GEN8_ROW_CHICKEN));
 + seq_printf(m, HALF_SLICE_CHICKEN3:\t0x%08x\n,
 +I915_READ(HALF_SLICE_CHICKEN3));
 + seq_printf(m, GAMTARBMODE:\t0x%08x\n, I915_READ(GAMTARBMODE));
 + seq_printf(m, _3D_CHICKEN3:\t0x%08x\n,
 +I915_READ(_3D_CHICKEN3));
 + seq_printf(m, COMMON_SLICE_CHICKEN2:\t0x%08x\n,
 +I915_READ(COMMON_SLICE_CHICKEN2));
 + seq_printf(m, GEN7_HALF_SLICE_CHICKEN1:\t0x%08x\n,
 +I915_READ(GEN7_HALF_SLICE_CHICKEN1));
 + seq_printf(m, GEN7_ROW_CHICKEN2:\t0x%08x\n,
 +I915_READ(GEN7_ROW_CHICKEN2));
 + seq_printf(m, GAM_ECOCHK:\t0x%08x\n,
 +I915_READ(GAM_ECOCHK));
 + seq_printf(m, HDC_CHICKEN0:\t0x%08x\n,
 +I915_READ(HDC_CHICKEN0));
 + seq_printf(m, GEN7_FF_THREAD_MODE:\t0x%08x\n,
 +I915_READ(GEN7_FF_THREAD_MODE));
 + seq_printf(m, GEN8_UCGCTL6:\t0x%08x\n,
 +I915_READ(GEN8_UCGCTL6));
 + seq_printf(m, GEN6_RC_SLEEP_PSMI_CONTROL:\t0x%08x\n,
 +I915_READ(GEN6_RC_SLEEP_PSMI_CONTROL));
 + seq_printf(m, CACHE_MODE_1:\t0x%08x\n,
 +I915_READ(CACHE_MODE_1));
 + } else
 + DRM_DEBUG_DRIVER(Not available for Gen%d\n,
 +  INTEL_INFO(dev)-gen);
 +
 + mutex_unlock(dev-struct_mutex);
 + return 0;
 +}
 +

This smells like a separate patch. But I'm not sure we want at all since
intel_reg_read will provide the same information.

  struct pipe_crc_info {
   const char *name;
   struct drm_device *dev;
   enum pipe pipe;
  };
  
  static int i915_pipe_crc_open(struct inode *inode, struct file *filep)
  {
   struct pipe_crc_info *info = inode-i_private;
   struct drm_i915_private *dev_priv = info-dev-dev_private;
 @@ -3904,20 +3949,21 @@ static const struct drm_info_list i915_debugfs_list[] 
 = {
   {i915_ppgtt_info, i915_ppgtt_info, 0},
   {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_workaround_info, i915_workaround_info, 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, 

Re: [Intel-gfx] [PATCH] drm/i915: Rework GPU reset sequence to match driver load thaw

2014-07-28 Thread Mcaulay, Alistair
Hi Ben / Daniel,
I agree that this needs to be properly tested. Are there any particular igt 
tests you would suggest I use?
I've been running:
drv_hangman, drv_suspend, gem_hangcheck_forcewake.

Also do you have a set of PPGTT Patches that should work with these tests. 
Michel sent me a set of patches to enable
PPGTT, but these 3 tests fail with the patches.

Thanks,
Alistair.

-Original Message-
From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter
Sent: Monday, July 28, 2014 10:27 AM
To: Ben Widawsky
Cc: Mcaulay, Alistair; intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915: Rework GPU reset sequence to match 
driver load  thaw

On Fri, Jul 25, 2014 at 06:05:29PM -0700, Ben Widawsky wrote:
 On Wed, Jul 16, 2014 at 04:05:59PM +0100, alistair.mcau...@intel.com wrote:
  From: McAulay, Alistair alistair.mcau...@intel.com
  
  This patch is to address Daniels concerns over different code during reset:
  
  http://lists.freedesktop.org/archives/intel-gfx/2014-June/047758.htm
  l
  
  The reason for aiming as hard as possible to use the exact same 
  code for driver load, gpu reset and runtime pm/system resume is that 
  we've simply seen too many bugs due to slight variations and unintended 
  omissions.
  
  Tested using igt drv_hangman.
  
  Signed-off-by: McAulay, Alistair alistair.mcau...@intel.com
 
 2 quick comments before I actually do a real review.
 1. Did you actually run this with and without full ppgtt?
 2. I don't think this is actually fulfilling what Daniel is 
 requesting, though we can let him comment.

Mostly looks like what I think we need. Comments below.

 3. Did you reall do #1?
 
 Assuming you satisifed #1, can you please post the igt results for the 
 permutations (pre patch w/ and w/o ppgtt; post patch w/ and w/o ppgtt)
 
 I really want this data because I spent *a lot* of time with these 
 specific areas in the PPGTT work, and I am somewhat skeptical enough 
 of the code has changed that this will magically work. I also tried 
 the trickiness with the ring handling functions, and never succeeded. 
 Also, with the context stuff, I'm simply not convinced it can 
 magically vanish. If igt looks good, and Daniel agrees that this is 
 what he actually wanted, I will go fishing for corner cases and do the review.

I think the hack in ring_begin might explain why it never worked before.
But fully agreed, we really need to test this well (and fill gaps if igt misses 
anything around resets - we don't have any systematic gpu reset coverage 
anywhere outside of igt).

 
 Thanks.
 
  ---
   drivers/gpu/drm/i915/i915_gem.c |  2 -
   drivers/gpu/drm/i915/i915_gem_context.c | 42 +
   drivers/gpu/drm/i915/i915_gem_gtt.c | 67 
  +
   drivers/gpu/drm/i915/i915_gem_gtt.h |  3 +-
   drivers/gpu/drm/i915/intel_ringbuffer.c |  4 +-
   5 files changed, 14 insertions(+), 104 deletions(-)
  
  diff --git a/drivers/gpu/drm/i915/i915_gem.c 
  b/drivers/gpu/drm/i915/i915_gem.c index ef047bc..b38e086 100644
  --- a/drivers/gpu/drm/i915/i915_gem.c
  +++ b/drivers/gpu/drm/i915/i915_gem.c
  @@ -2590,8 +2590,6 @@ void i915_gem_reset(struct drm_device *dev)
  for_each_ring(ring, dev_priv, i)
  i915_gem_reset_ring_cleanup(dev_priv, ring);
   
  -   i915_gem_context_reset(dev);
  -
  i915_gem_restore_fences(dev);
   }
   
  diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
  b/drivers/gpu/drm/i915/i915_gem_context.c
  index de72a28..d96219f 100644
  --- a/drivers/gpu/drm/i915/i915_gem_context.c
  +++ b/drivers/gpu/drm/i915/i915_gem_context.c
  @@ -372,42 +372,6 @@ err_destroy:
  return ERR_PTR(ret);
   }
   
  -void i915_gem_context_reset(struct drm_device *dev) -{
  -   struct drm_i915_private *dev_priv = dev-dev_private;
  -   int i;
  -
  -   /* Prevent the hardware from restoring the last context (which hung) on
  -* the next switch */
  -   for (i = 0; i  I915_NUM_RINGS; i++) {
  -   struct intel_engine_cs *ring = dev_priv-ring[i];
  -   struct intel_context *dctx = ring-default_context;
  -   struct intel_context *lctx = ring-last_context;
  -
  -   /* Do a fake switch to the default context */
  -   if (lctx == dctx)
  -   continue;
  -
  -   if (!lctx)
  -   continue;
  -
  -   if (dctx-legacy_hw_ctx.rcs_state  i == RCS) {
  -   
  WARN_ON(i915_gem_obj_ggtt_pin(dctx-legacy_hw_ctx.rcs_state,
  - 
  get_context_alignment(dev), 0));
  -   /* Fake a finish/inactive */
  -   dctx-legacy_hw_ctx.rcs_state-base.write_domain = 0;
  -   dctx-legacy_hw_ctx.rcs_state-active = 0;
  -   }
  -
  -   if (lctx-legacy_hw_ctx.rcs_state  i == RCS)
  -   
  i915_gem_object_ggtt_unpin(lctx-legacy_hw_ctx.rcs_state);
  -
  -   

Re: [Intel-gfx] [RFC] Move BDW workarounds to ring init fn

2014-07-28 Thread Ville Syrjälä
On Mon, Jul 28, 2014 at 05:31:45PM +0100, arun.siluv...@linux.intel.com wrote:
 From: Arun Siluvery arun.siluv...@linux.intel.com
 
 This patch moves BDW workarounds from init_clock_gating() to render ring
 init fn otherwise they are lost when gpu is reset.
 In case of execlists, some of the workarounds modify registers that are
 part of register state context which doesn't get initialized until
 init_clock_gating(); this results in default context with incorrect values
 as it is restored and saved before updated by workarounds.

I don't think it has to do with execlists. Many of the registers are
part of the context image even in ring buffer mode AFAIK.

 
 Open issue:
 For Wa4x4STCOptimizationDisable, we set CACHE_MODE_1[6:6] = 1
 At the time when HW contexts are enabled after rings are initialized with
 default context this workaround is valid but followed by a context switch
 this is getting reset, please see below log snippet.

This is a bit weird. The default context should have restore inhibit==1
so it shouldn't clobber the CACHE_MODE_1 register. There was a specific magic
dance you're supposed to do when accessing such registers with mmio, but here
we do the write even before the first context switch.

Apparently there was some kind of problem with CACHE_MODE_0 on snb too:
 commit 3a69ddd6f872180b6f61fda87152b37202118fbc
 Author: Kenneth Graunke kenn...@whitecape.org
 Date:   Fri Apr 27 12:44:41 2012 -0700

drm/i915: Set the Stencil Cache eviction policy to non-LRA mode.

but IIRC I wasn't able to reproduce it when I tried.

Maybe we need to delay these register writes until we've switched to the default
context?

 
 ...
 [5.978209] [drm:i915_pages_create_for_stolen] offset=0x0, size=8294400
 [5.978213] [drm:intel_alloc_plane_obj] plane fb obj 8801472e
 [5.978215] [drm:i915_gem_setup_global_gtt] reserving preallocated space: 
 0 + 7e9000
 [5.978216] [drm:i915_gem_setup_global_gtt] clearing unused GTT space: 
 [7e9000, f000]
 [5.979613] [drm:i915_gem_init] CACHE_MODE_1: 0x0180
 [5.981372] [drm:gen8_ppgtt_init] Allocated 4 pages for page directories 
 (0 wasted)
 [5.981373] [drm:gen8_ppgtt_init] Allocated 2048 pages for page tables (0 
 wasted)
 [5.981376] [drm:i915_gem_context_init] HW context support initialized
 [5.981462] [drm:i915_gem_init_hw] CACHE_MODE_1: 0x0180
 [5.981467] [drm:i915_gem_init_rings] CACHE_MODE_1: 0x0180
 [5.981704] [drm:bdw_init_workarounds] CACHE_MODE_1: 0x01C0
 [5.981716] [drm:init_status_page] bsd ring hws offset: 0x0081e000
 [5.981792] [drm:init_status_page] blitter ring hws offset: 0x0083f000
 [5.981910] [drm:init_status_page] video enhancement ring hws offset: 
 0x0086
 [5.982001] [drm:i915_gem_init_hw] CACHE_MODE_1: 0x01C0
 [5.982104] [drm:i915_gem_context_enable] Switch render ring to 
 default_context
 [5.982106] [drm:i915_gem_render_state_init] render ring: Render state init
 [5.982120] [drm:do_switch] render ring, CACHE_MODE_1: 0x01C0, 
 uninitialized: 1
 [5.982121] [drm:i915_gem_context_enable] Switch bsd ring to 
 default_context
 [5.982122] [drm:do_switch] bsd ring, CACHE_MODE_1: 0x01C0, 
 uninitialized: 0
 [5.982123] [drm:i915_gem_context_enable] Switch blitter ring to 
 default_context
 [5.982126] [drm:do_switch] blitter ring, CACHE_MODE_1: 0x01C0, 
 uninitialized: 0
 [5.982126] [drm:i915_gem_context_enable] Switch video enhancement ring to 
 default_context
 [5.982128] [drm:do_switch] video enhancement ring, CACHE_MODE_1: 
 0x01C0, uninitialized: 0
 [5.982133] [drm:i915_gem_init] CACHE_MODE_1: 0x01C0
 [5.982258] [drm:intel_init_clock_gating]
 ...
 [   10.037019] [drm:do_switch] blitter ring, CACHE_MODE_1: 0x0180, 
 uninitialized: 0
 ...
 [   10.488145] [drm:do_switch] render ring, CACHE_MODE_1: 0x0180, 
 uninitialized: 0
 ...
 
 I am currently testing this with an igt which triggers a gpu reset and 
 compares
 WA register contents before and after reset but the test fails because of
 this register hence not sending it now.
 Please let me know how to keep this WA valid after a context switch.
 
 
 Arun Siluvery (1):
   drm/i915/bdw: Initialize BDW workarounds in render ring init fn
 
  drivers/gpu/drm/i915/i915_debugfs.c | 46 ++
  drivers/gpu/drm/i915/intel_pm.c | 59 
  drivers/gpu/drm/i915/intel_ringbuffer.c | 68 
 +
  3 files changed, 114 insertions(+), 59 deletions(-)
 
 -- 
 1.9.2
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 1/1] drm/i915: Adding Gfx Clock, Wake and Gunit save/restore logic in PM suspend/resume paths.

2014-07-28 Thread sagar . a . kamble
From: Sagar Kamble sagar.a.kam...@intel.com

Sequence to get gfx clocks on/off, allow/disallow wake and save/restore of 
gunit registers need to be followed in
PM suspend and resume path similar to runtime suspend and resume.

Cc: Imre Deak imre.d...@intel.com
Cc: Paulo Zanoni paulo.r.zan...@intel.com
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Cc: Jani Nikula jani.nik...@linux.intel.com
Signed-off-by: Sagar Kamble sagar.a.kam...@intel.com
---
 drivers/gpu/drm/i915/i915_drv.c  | 84 
 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_drv.h |  7 
 3 files changed, 85 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 6c4b25c..fdfeccf 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -495,6 +495,26 @@ static int i915_drm_freeze(struct drm_device *dev)
struct drm_i915_private *dev_priv = dev-dev_private;
struct drm_crtc *crtc;
pci_power_t opregion_target_state;
+   u32 mask;
+   int err = 0;
+
+   /* Following sequence from vlv_runtime_suspend */
+   if (IS_VALLEYVIEW(dev)) {
+   /*
+* Bspec defines the following GT well on flags as debug only,
+* so don't treat them as hard failures.
+*/
+   (void)vlv_wait_for_gt_wells(dev_priv, false);
+
+   mask = VLV_GTLC_RENDER_CTX_EXISTS | VLV_GTLC_MEDIA_CTX_EXISTS;
+   WARN_ON((I915_READ(VLV_GTLC_WAKE_CTRL)  mask) != mask);
+
+   vlv_check_no_gt_access(dev_priv);
+
+   err = vlv_force_gfx_clock(dev_priv, true);
+   if (err)
+   goto err1;
+   }
 
/* ignore lid events during suspend */
mutex_lock(dev_priv-modeset_restore_lock);
@@ -542,6 +562,10 @@ static int i915_drm_freeze(struct drm_device *dev)
 
i915_gem_suspend_gtt_mappings(dev);
 
+   /* Save Gunit State */
+   if (IS_VALLEYVIEW(dev))
+   vlv_save_gunit_s0ix_state(dev_priv);
+
i915_save_state(dev);
 
opregion_target_state = PCI_D3cold;
@@ -562,7 +586,27 @@ static int i915_drm_freeze(struct drm_device *dev)
 
intel_display_set_init_power(dev_priv, false);
 
-   return 0;
+   /* Clear Allow Wake Bit so that none of the
+* force/demand wake requests
+*/
+   if (IS_VALLEYVIEW(dev)) {
+   err = vlv_allow_gt_wake(dev_priv, false);
+   if (err)
+   goto err2;
+
+   /* Release graphics clocks */
+   vlv_force_gfx_clock(dev_priv, false);
+   }
+   return err;
+err2:
+   /* For safety always re-enable waking and disable gfx clock forcing */
+   if (IS_VALLEYVIEW(dev))
+   vlv_allow_gt_wake(dev_priv, true);
+err1:
+   if (IS_VALLEYVIEW(dev))
+   vlv_force_gfx_clock(dev_priv, false);
+
+   return err;
 }
 
 int i915_suspend(struct drm_device *dev, pm_message_t state)
@@ -610,6 +654,26 @@ void intel_console_resume(struct work_struct *work)
 static int i915_drm_thaw_early(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = dev-dev_private;
+   int ret = 0;
+   int err;
+
+   /*
+* Following sequence from vlv_runtime_resume. Clock is released
+* in i915_drm_thaw.
+* If any of the steps fail just try to continue, that's the best we
+* can do at this point. Return the first error code (which will also
+* leave RPM permanently disabled).
+*/
+
+   if (IS_VALLEYVIEW(dev)) {
+   ret = vlv_force_gfx_clock(dev_priv, true);
+
+   vlv_restore_gunit_s0ix_state(dev_priv);
+
+   err = vlv_allow_gt_wake(dev_priv, true);
+   if (!ret)
+   ret = err;
+   }
 
if (IS_HASWELL(dev) || IS_BROADWELL(dev))
hsw_disable_pc8(dev_priv);
@@ -618,7 +682,7 @@ static int i915_drm_thaw_early(struct drm_device *dev)
intel_uncore_sanitize(dev);
intel_power_domains_init_hw(dev_priv);
 
-   return 0;
+   return ret;
 }
 
 static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
@@ -695,6 +759,9 @@ static int __i915_drm_thaw(struct drm_device *dev, bool 
restore_gtt_mappings)
 
intel_opregion_notify_adapter(dev, PCI_D0);
 
+   /* Release graphics clocks turned on in thaw_early*/
+   vlv_force_gfx_clock(dev_priv, false);
+
return 0;
 }
 
@@ -1028,7 +1095,7 @@ static int hsw_runtime_resume(struct drm_i915_private 
*dev_priv)
  * a black-box for the driver. Further investigation is needed to reduce the
  * saved/restored registers even further, by following the same 3 criteria.
  */
-static void vlv_save_gunit_s0ix_state(struct drm_i915_private *dev_priv)
+void vlv_save_gunit_s0ix_state(struct drm_i915_private *dev_priv)
 {
struct vlv_s0ix_state *s = 

Re: [Intel-gfx] [PATCH 1/1] drm/i915: Adding Gfx Clock, Wake and Gunit save/restore logic in PM suspend/resume paths.

2014-07-28 Thread Sagar Arun Kamble
On Mon, 2014-07-28 at 23:07 +0530, sagar.a.kam...@intel.com wrote:
 From: Sagar Kamble sagar.a.kam...@intel.com
 
 Sequence to get gfx clocks on/off, allow/disallow wake and save/restore of 
 gunit registers need to be followed in
 PM suspend and resume path similar to runtime suspend and resume.
 
 Cc: Imre Deak imre.d...@intel.com
 Cc: Paulo Zanoni paulo.r.zan...@intel.com
 Cc: Daniel Vetter daniel.vet...@ffwll.ch
 Cc: Jani Nikula jani.nik...@linux.intel.com
 Signed-off-by: Sagar Kamble sagar.a.kam...@intel.com
 ---
  drivers/gpu/drm/i915/i915_drv.c  | 84 
 
  drivers/gpu/drm/i915/i915_drv.h  |  1 +
  drivers/gpu/drm/i915/intel_drv.h |  7 
  3 files changed, 85 insertions(+), 7 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
 index 6c4b25c..fdfeccf 100644
 --- a/drivers/gpu/drm/i915/i915_drv.c
 +++ b/drivers/gpu/drm/i915/i915_drv.c
 @@ -495,6 +495,26 @@ static int i915_drm_freeze(struct drm_device *dev)
   struct drm_i915_private *dev_priv = dev-dev_private;
   struct drm_crtc *crtc;
   pci_power_t opregion_target_state;
 + u32 mask;
 + int err = 0;
 +
 + /* Following sequence from vlv_runtime_suspend */
 + if (IS_VALLEYVIEW(dev)) {
 + /*
 +  * Bspec defines the following GT well on flags as debug only,
 +  * so don't treat them as hard failures.
 +  */
 + (void)vlv_wait_for_gt_wells(dev_priv, false);
 +
 + mask = VLV_GTLC_RENDER_CTX_EXISTS | VLV_GTLC_MEDIA_CTX_EXISTS;
 + WARN_ON((I915_READ(VLV_GTLC_WAKE_CTRL)  mask) != mask);
 +
 + vlv_check_no_gt_access(dev_priv);
Above piece of code may not be at the right place since during suspend
operation there might be workload on wells. Added to make it analogous
to runtime suspend. Moving it below gem_suspend will be proper way. Is
my understanding correct?
 +
 + err = vlv_force_gfx_clock(dev_priv, true);
 + if (err)
 + goto err1;
 + }
  
   /* ignore lid events during suspend */
   mutex_lock(dev_priv-modeset_restore_lock);
 @@ -542,6 +562,10 @@ static int i915_drm_freeze(struct drm_device *dev)
  
   i915_gem_suspend_gtt_mappings(dev);
  
 + /* Save Gunit State */
 + if (IS_VALLEYVIEW(dev))
 + vlv_save_gunit_s0ix_state(dev_priv);
 +
   i915_save_state(dev);
  
   opregion_target_state = PCI_D3cold;
 @@ -562,7 +586,27 @@ static int i915_drm_freeze(struct drm_device *dev)
  
   intel_display_set_init_power(dev_priv, false);
  
 - return 0;
 + /* Clear Allow Wake Bit so that none of the
 +  * force/demand wake requests
 +  */
 + if (IS_VALLEYVIEW(dev)) {
 + err = vlv_allow_gt_wake(dev_priv, false);
 + if (err)
 + goto err2;
 +
 + /* Release graphics clocks */
 + vlv_force_gfx_clock(dev_priv, false);
 + }
 + return err;
 +err2:
 + /* For safety always re-enable waking and disable gfx clock forcing */
 + if (IS_VALLEYVIEW(dev))
 + vlv_allow_gt_wake(dev_priv, true);
 +err1:
 + if (IS_VALLEYVIEW(dev))
 + vlv_force_gfx_clock(dev_priv, false);
 +
 + return err;
  }
  
  int i915_suspend(struct drm_device *dev, pm_message_t state)
 @@ -610,6 +654,26 @@ void intel_console_resume(struct work_struct *work)
  static int i915_drm_thaw_early(struct drm_device *dev)
  {
   struct drm_i915_private *dev_priv = dev-dev_private;
 + int ret = 0;
 + int err;
 +
 + /*
 +  * Following sequence from vlv_runtime_resume. Clock is released
 +  * in i915_drm_thaw.
 +  * If any of the steps fail just try to continue, that's the best we
 +  * can do at this point. Return the first error code (which will also
 +  * leave RPM permanently disabled).
 +  */
 +
 + if (IS_VALLEYVIEW(dev)) {
 + ret = vlv_force_gfx_clock(dev_priv, true);
 +
 + vlv_restore_gunit_s0ix_state(dev_priv);
 +
 + err = vlv_allow_gt_wake(dev_priv, true);
 + if (!ret)
 + ret = err;
 + }
  
   if (IS_HASWELL(dev) || IS_BROADWELL(dev))
   hsw_disable_pc8(dev_priv);
 @@ -618,7 +682,7 @@ static int i915_drm_thaw_early(struct drm_device *dev)
   intel_uncore_sanitize(dev);
   intel_power_domains_init_hw(dev_priv);
  
 - return 0;
 + return ret;
  }
  
  static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
 @@ -695,6 +759,9 @@ static int __i915_drm_thaw(struct drm_device *dev, bool 
 restore_gtt_mappings)
  
   intel_opregion_notify_adapter(dev, PCI_D0);
  
 + /* Release graphics clocks turned on in thaw_early*/
 + vlv_force_gfx_clock(dev_priv, false);
 +
   return 0;
  }
  
 @@ -1028,7 +1095,7 @@ static int hsw_runtime_resume(struct drm_i915_private 
 *dev_priv)
   * a black-box for the driver. Further 

[Intel-gfx] [PATCH 0/3] Fixes for runtime PM on planes APIs

2014-07-28 Thread Paulo Zanoni
From: Paulo Zanoni paulo.r.zan...@intel.com

Hi

This series fixes some bugs that happen when we're runtime suspended and try to
use the planes APIs. I also wrote IGT test cases for the bugs, so we will be
able to detect future regressions.

The controversial part of these patches is that we had previously defined that
we wanted to get/put runtime PM in the highest level of the stack, wrapping as
much code as possible, but Daniel asked me to only get/put runtime PM around the
functions that pin the objects (still on the highest level, but only around the
pin functions). This series implements Daniel's suggestions.

Thanks,
Paulo


Paulo Zanoni (3):
  drm/i915: fix cursor handling when runtime suspended
  drm/i915: get runtime PM when pinning sprite objects
  drm/i915: get runtime PM when pinning primary plane objects

 drivers/gpu/drm/i915/intel_display.c | 9 +
 drivers/gpu/drm/i915/intel_sprite.c  | 3 +++
 2 files changed, 12 insertions(+)

-- 
2.0.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 1/2] tests/pm_rpm: add cursor subtests

2014-07-28 Thread Paulo Zanoni
From: Paulo Zanoni paulo.r.zan...@intel.com

These tests currently trigger WARNs on our Kernel. Let's make sure we
fix the bugs and they never come back.

v2: Reorganize the code a little bit, and improve the tests.

Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
 tests/pm_rpm.c | 123 -
 1 file changed, 113 insertions(+), 10 deletions(-)

diff --git a/tests/pm_rpm.c b/tests/pm_rpm.c
index 869e6f3..f720f86 100644
--- a/tests/pm_rpm.c
+++ b/tests/pm_rpm.c
@@ -277,13 +277,15 @@ static uint32_t get_fb(struct mode_set_data *data, int 
width, int height)
igt_assert(false);
 }
 
-static bool enable_one_screen_with_type(struct mode_set_data *data,
-   enum screen_type type)
+static bool find_connector_for_modeset(struct mode_set_data *data,
+  enum screen_type type,
+  uint32_t *connector_id,
+  drmModeModeInfoPtr *mode)
 {
-   uint32_t crtc_id = 0, buffer_id = 0, connector_id = 0;
-   drmModeModeInfoPtr mode = NULL;
-   int i, rc;
+   int i;
 
+   *connector_id = 0;
+   *mode = NULL;
for (i = 0; i  data-res-count_connectors; i++) {
drmModeConnectorPtr c = data-connectors[i];
 
@@ -296,13 +298,23 @@ static bool enable_one_screen_with_type(struct 
mode_set_data *data,
continue;
 
if (c-connection == DRM_MODE_CONNECTED  c-count_modes) {
-   connector_id = c-connector_id;
-   mode = c-modes[0];
+   *connector_id = c-connector_id;
+   *mode = c-modes[0];
break;
}
}
 
-   if (connector_id == 0)
+   return (*connector_id != 0);
+}
+
+static bool enable_one_screen_with_type(struct mode_set_data *data,
+   enum screen_type type)
+{
+   uint32_t crtc_id = 0, buffer_id = 0, connector_id;
+   drmModeModeInfoPtr mode;
+   int rc;
+
+   if (!find_connector_for_modeset(data, type, connector_id, mode))
return false;
 
crtc_id = data-res-crtcs[0];
@@ -310,8 +322,6 @@ static bool enable_one_screen_with_type(struct 
mode_set_data *data,
 
igt_assert(crtc_id);
igt_assert(buffer_id);
-   igt_assert(connector_id);
-   igt_assert(mode);
 
rc = drmModeSetCrtc(drm_fd, crtc_id, buffer_id, 0, 0, connector_id,
1, mode);
@@ -1407,6 +1417,93 @@ static void dpms_mode_unset_subtest(enum screen_type 
type)
igt_assert(wait_for_suspended());
 }
 
+static void fill_igt_fb(struct igt_fb *fb, uint32_t color)
+{
+   int i;
+   uint32_t *ptr;
+
+   ptr = gem_mmap__cpu(drm_fd, fb-gem_handle, fb-size, 0);
+   for (i = 0; i  fb-size/sizeof(uint32_t); i++)
+   ptr[i] = color;
+   igt_assert(munmap(ptr, fb-size) == 0);
+}
+
+/* At some point, this test triggered WARNs in the Kernel. */
+static void cursor_subtest(bool dpms)
+{
+   uint32_t crtc_id = 0, connector_id;
+   drmModeModeInfoPtr mode;
+   int rc;
+   struct igt_fb scanout_fb, cursor_fb1, cursor_fb2;
+
+   disable_all_screens(ms_data);
+   igt_assert(wait_for_suspended());
+
+   igt_require(find_connector_for_modeset(ms_data, SCREEN_TYPE_ANY,
+  connector_id, mode));
+
+   crtc_id = ms_data.res-crtcs[0];
+   igt_assert(crtc_id);
+
+   igt_create_fb(drm_fd, mode-hdisplay, mode-vdisplay,
+ DRM_FORMAT_XRGB, false, scanout_fb);
+   igt_create_fb(drm_fd, 64, 64, DRM_FORMAT_ARGB, false, cursor_fb1);
+   igt_create_fb(drm_fd, 64, 64, DRM_FORMAT_ARGB, false, cursor_fb2);
+
+   fill_igt_fb(scanout_fb, 0xFF);
+   fill_igt_fb(cursor_fb1, 0xFF00);
+   fill_igt_fb(cursor_fb2, 0xFF00FF00);
+
+   rc = drmModeSetCrtc(drm_fd, crtc_id, scanout_fb.fb_id, 0, 0,
+   connector_id, 1, mode);
+   igt_assert(rc == 0);
+   igt_assert(wait_for_active());
+
+   rc = drmModeSetCursor(drm_fd, crtc_id, cursor_fb1.gem_handle,
+ cursor_fb1.width, cursor_fb1.height);
+   igt_assert(rc == 0);
+   rc = drmModeMoveCursor(drm_fd, crtc_id, 0, 0);
+   igt_assert(rc == 0);
+   igt_assert(wait_for_active());
+
+   if (dpms)
+   disable_all_screens_dpms(ms_data);
+   else
+   disable_all_screens(ms_data);
+   igt_assert(wait_for_suspended());
+
+   /* First, just move the cursor. */
+   rc = drmModeMoveCursor(drm_fd, crtc_id, 1, 1);
+   igt_assert(rc == 0);
+   igt_assert(wait_for_suspended());
+
+   /* Then unset it, and set a new one. */
+   rc = drmModeSetCursor(drm_fd, crtc_id, 0, 0, 0);
+   igt_assert(rc == 0);
+   

[Intel-gfx] [PATCH 1/3] drm/i915: fix cursor handling when runtime suspended

2014-07-28 Thread Paulo Zanoni
From: Paulo Zanoni paulo.r.zan...@intel.com

If we're runtime suspended and try to use the cursor interfaces, we
will get a lot of WARNs saying we did the wrong thing.

For intel_crtc_update_cursor(), all we need to do is return if the
CRTC is not active, since writing the registers won't really have any
effect if the screen is not visible, and we will write the registers
later when enabling the screen.

For intel_crtc_cursor_set_obj(), we just need to wake up the hardware
then pinning the display plane.

v2: - Narrow the put/get calls on intel_crtc_cursor_set_obj() (Daniel)

Testcase: igt/pm_rpm/cursor
Testcase: igt/pm_rpm/cursor-dpms
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=81645
Cc: sta...@vger.kernel.org
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
 drivers/gpu/drm/i915/intel_display.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 1edfd1a..f1a9b5c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8144,6 +8144,9 @@ static void intel_crtc_update_cursor(struct drm_crtc 
*crtc,
if (base == 0  intel_crtc-cursor_base == 0)
return;
 
+   if (!intel_crtc-active)
+   return;
+
I915_WRITE(CURPOS(pipe), pos);
 
if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev) || IS_BROADWELL(dev))
@@ -8217,7 +8220,9 @@ static int intel_crtc_cursor_set_obj(struct drm_crtc 
*crtc,
if (need_vtd_wa(dev))
alignment = 64*1024;
 
+   intel_runtime_pm_get(dev_priv);
ret = i915_gem_object_pin_to_display_plane(obj, alignment, 
NULL);
+   intel_runtime_pm_put(dev_priv);
if (ret) {
DRM_DEBUG_KMS(failed to move cursor bo into the 
GTT\n);
goto fail_locked;
-- 
2.0.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 2/2] tests/pm_rpm: add planes subtests

2014-07-28 Thread Paulo Zanoni
From: Paulo Zanoni paulo.r.zan...@intel.com

Just like the cursor subtests, these also trigger WARNs on the current
Kernel.

Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
 tests/pm_rpm.c | 212 -
 1 file changed, 211 insertions(+), 1 deletion(-)

diff --git a/tests/pm_rpm.c b/tests/pm_rpm.c
index f720f86..048d9ad 100644
--- a/tests/pm_rpm.c
+++ b/tests/pm_rpm.c
@@ -51,6 +51,9 @@
 #include igt_kms.h
 #include igt_debugfs.h
 
+/* One day, this will be on your libdrm. */
+#define DRM_CLIENT_CAP_UNIVERSAL_PLANES 2
+
 #define MSR_PC8_RES0x630
 #define MSR_PC9_RES0x631
 #define MSR_PC10_RES   0x632
@@ -72,6 +75,12 @@ enum screen_type {
SCREEN_TYPE_ANY,
 };
 
+enum plane_type {
+   PLANE_OVERLAY,
+   PLANE_PRIMARY,
+   PLANE_CURSOR,
+};
+
 /* Wait flags */
 #define DONT_WAIT  0
 #define WAIT_STATUS1
@@ -1504,6 +1513,199 @@ static void cursor_subtest(bool dpms)
igt_assert(wait_for_suspended());
 }
 
+static enum plane_type get_plane_type(uint32_t plane_id)
+{
+   drmModeObjectPropertiesPtr props;
+   int i, j;
+   enum plane_type type;
+   bool found = false;
+
+   props = drmModeObjectGetProperties(drm_fd, plane_id,
+  DRM_MODE_OBJECT_PLANE);
+   igt_assert(props);
+
+   for (i = 0; i  props-count_props  !found; i++) {
+   drmModePropertyPtr prop;
+   const char *enum_name = NULL;
+
+   prop = drmModeGetProperty(drm_fd, props-props[i]);
+   igt_assert(prop);
+
+   if (strcmp(prop-name, type) == 0) {
+   igt_assert(prop-flags  DRM_MODE_PROP_ENUM);
+   igt_assert(props-prop_values[i]  prop-count_enums);
+
+   for (j = 0; j  prop-count_enums; j++) {
+   if (prop-enums[j].value ==
+   props-prop_values[i]) {
+   enum_name = prop-enums[j].name;
+   break;
+   }
+   }
+   igt_assert(enum_name);
+
+   if (strcmp(enum_name, Overlay) == 0)
+   type = PLANE_OVERLAY;
+   else if (strcmp(enum_name, Primary) == 0)
+   type = PLANE_PRIMARY;
+   else if (strcmp(enum_name, Cursor) == 0)
+   type = PLANE_CURSOR;
+   else
+   igt_assert(0);
+
+   found = true;
+   }
+
+   drmModeFreeProperty(prop);
+   }
+   igt_assert(found);
+
+   drmModeFreeObjectProperties(props);
+   return type;
+}
+
+static void test_one_plane(bool dpms, uint32_t plane_id,
+  enum plane_type plane_type)
+{
+   int rc;
+   uint32_t plane_format, plane_w, plane_h;
+   uint32_t crtc_id, connector_id;
+   struct igt_fb scanout_fb, plane_fb1, plane_fb2;
+   drmModeModeInfoPtr mode;
+   int32_t crtc_x = 0, crtc_y = 0;
+
+   disable_all_screens(ms_data);
+   igt_assert(wait_for_suspended());
+
+   igt_require(find_connector_for_modeset(ms_data, SCREEN_TYPE_ANY,
+  connector_id, mode));
+
+   crtc_id = ms_data.res-crtcs[0];
+   igt_assert(crtc_id);
+
+   igt_create_fb(drm_fd, mode-hdisplay, mode-vdisplay,
+ DRM_FORMAT_XRGB, false, scanout_fb);
+
+   fill_igt_fb(scanout_fb, 0xFF);
+
+   switch (plane_type) {
+   case PLANE_OVERLAY:
+   plane_format = DRM_FORMAT_XRGB;
+   plane_w = 64;
+   plane_h = 64;
+   break;
+   case PLANE_PRIMARY:
+   plane_format = DRM_FORMAT_XRGB;
+   plane_w = mode-hdisplay;
+   plane_h = mode-vdisplay;
+   break;
+   case PLANE_CURSOR:
+   plane_format = DRM_FORMAT_ARGB;
+   plane_w = 64;
+   plane_h = 64;
+   break;
+   default:
+   igt_assert(0);
+   break;
+   }
+
+   igt_create_fb(drm_fd, plane_w, plane_h, plane_format, false,
+ plane_fb1);
+   igt_create_fb(drm_fd, plane_w, plane_h, plane_format, false,
+ plane_fb2);
+   fill_igt_fb(plane_fb1, 0xFF00);
+   fill_igt_fb(plane_fb2, 0xFF00FF00);
+
+   rc = drmModeSetCrtc(drm_fd, crtc_id, scanout_fb.fb_id, 0, 0,
+   connector_id, 1, mode);
+   igt_assert(rc == 0);
+   igt_assert(wait_for_active());
+
+   rc = drmModeSetPlane(drm_fd, plane_id, crtc_id, plane_fb1.fb_id, 0,
+0, 0, plane_fb1.width, plane_fb1.height,
+0  16, 0  16, plane_fb1.width  16,
+   

[Intel-gfx] [PATCH 3/3] drm/i915: get runtime PM when pinning primary plane objects

2014-07-28 Thread Paulo Zanoni
From: Paulo Zanoni paulo.r.zan...@intel.com

Otherwise we may get WARNs saying we're writing registers while
runtime suspended.

Testcase: igt/pm_rpm/universal-planes
Testcase: igt/pm_rpm/universal-planes-dpms
Cc: sta...@vger.kernel.org
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
 drivers/gpu/drm/i915/intel_display.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index f1a9b5c..5948207 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11531,7 +11531,9 @@ intel_primary_plane_setplane(struct drm_plane *plane, 
struct drm_crtc *crtc,
  INTEL_FRONTBUFFER_PRIMARY(intel_crtc-pipe));
 
/* Pin and return without programming hardware */
+   intel_runtime_pm_get(dev_priv);
ret = intel_pin_and_fence_fb_obj(dev, obj, NULL);
+   intel_runtime_pm_put(dev_priv);
mutex_unlock(dev-struct_mutex);
 
return ret;
@@ -11553,7 +11555,9 @@ intel_primary_plane_setplane(struct drm_plane *plane, 
struct drm_crtc *crtc,
 * fail.
 */
if (plane-fb != fb) {
+   intel_runtime_pm_get(dev_priv);
ret = intel_pin_and_fence_fb_obj(dev, obj, NULL);
+   intel_runtime_pm_put(dev_priv);
if (ret) {
mutex_unlock(dev-struct_mutex);
return ret;
-- 
2.0.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 2/3] drm/i915: get runtime PM when pinning sprite objects

2014-07-28 Thread Paulo Zanoni
From: Paulo Zanoni paulo.r.zan...@intel.com

Otherwise we may get WARNs saying we're writing registers while
runtime suspended.

Testcase: igt/pm_rpm/legacy-planes
Testcase: igt/pm_rpm/legacy-planes-dpms
Cc: sta...@vger.kernel.org
Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com
---
 drivers/gpu/drm/i915/intel_sprite.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
b/drivers/gpu/drm/i915/intel_sprite.c
index d34a569..8c5a8f7 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -821,6 +821,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc 
*crtc,
   uint32_t src_w, uint32_t src_h)
 {
struct drm_device *dev = plane-dev;
+   struct drm_i915_private *dev_priv = dev-dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
struct intel_plane *intel_plane = to_intel_plane(plane);
enum pipe pipe = intel_crtc-pipe;
@@ -1009,7 +1010,9 @@ intel_update_plane(struct drm_plane *plane, struct 
drm_crtc *crtc,
 * primary plane requires 256KiB alignment with 64 PTE padding,
 * the sprite planes only require 128KiB alignment and 32 PTE padding.
 */
+   intel_runtime_pm_get(dev_priv);
ret = intel_pin_and_fence_fb_obj(dev, obj, NULL);
+   intel_runtime_pm_put(dev_priv);
 
i915_gem_track_fb(old_obj, obj,
  INTEL_FRONTBUFFER_SPRITE(pipe));
-- 
2.0.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm: Add rotation_property to mode_config and creating it

2014-07-28 Thread Daniel Vetter
On Mon, Jul 28, 2014 at 06:29:41PM +0300, Ville Syrjälä wrote:
 On Tue, Jul 15, 2014 at 05:43:37PM +0530, sonika.jin...@intel.com wrote:
  From: Sonika Jindal sonika.jin...@intel.com
  
  v2: Adding creation of rotation_property here.
  
  Signed-off-by: Sonika Jindal sonika.jin...@intel.com
  ---
   drivers/gpu/drm/drm_crtc.c |3 ++-
   include/drm/drm_crtc.h |1 +
   2 files changed, 3 insertions(+), 1 deletion(-)
  
  diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
  index 787631e..49c0747 100644
  --- a/drivers/gpu/drm/drm_crtc.c
  +++ b/drivers/gpu/drm/drm_crtc.c
  @@ -1299,7 +1299,8 @@ static int 
  drm_mode_create_standard_plane_properties(struct drm_device *dev)
  type, drm_plane_type_enum_list,
  ARRAY_SIZE(drm_plane_type_enum_list));
  dev-mode_config.plane_type_property = type;
  -
  +   dev-mode_config.rotation_property = 
  drm_mode_create_rotation_property(dev,
  +   BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_180));
 
 This might not make sense for other (!i915) hardware. And that's the
 reason why I had the driver create the property in the first place.
 
 I think Daniel was thinking that we might want to expose all the bits
 regardless of what the hardware supports, but I don't like that idea.
 There are other properties (eg. alpha blending, csc stuff, etc.) that
 have the same problem of hardware supporting only a (potentially small)
 subset of the possible values. I'd rather we didn't make life harder
 for userspace when the kernel can already report that certain values
 will never work.

Well I'd like the property to be in some generic place so that fbcon can
unroate and that with the atomic stuff we can have rotation support in the
core structures. Which should help with argument checking.

But for rotation I don't think we should set it up in generic code, but in
i915. So the place where we keep it would be generic, the values would be
the sames, but the allowed set would differ depending upon platform or
driver.
-Daniel

 
  return 0;
   }
   
  diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
  index ce6df4a..5545dd3 100644
  --- a/include/drm/drm_crtc.h
  +++ b/include/drm/drm_crtc.h
  @@ -819,6 +819,7 @@ struct drm_mode_config {
  struct drm_property *dpms_property;
  struct drm_property *path_property;
  struct drm_property *plane_type_property;
  +   struct drm_property *rotation_property;
   
  /* DVI-I properties */
  struct drm_property *dvi_i_subconnector_property;
  -- 
  1.7.10.4
  
  ___
  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

-- 
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/1] drm/i915: Adding Gfx Clock, Wake and Gunit save/restore logic in PM suspend/resume paths.

2014-07-28 Thread Daniel Vetter
On Mon, Jul 28, 2014 at 11:07:10PM +0530, sagar.a.kam...@intel.com wrote:
 From: Sagar Kamble sagar.a.kam...@intel.com
 
 Sequence to get gfx clocks on/off, allow/disallow wake and save/restore of 
 gunit registers need to be followed in
 PM suspend and resume path similar to runtime suspend and resume.
 
 Cc: Imre Deak imre.d...@intel.com
 Cc: Paulo Zanoni paulo.r.zan...@intel.com
 Cc: Daniel Vetter daniel.vet...@ffwll.ch
 Cc: Jani Nikula jani.nik...@linux.intel.com
 Signed-off-by: Sagar Kamble sagar.a.kam...@intel.com

Whereever that code currently lives, I want it shared instead of runtime
pm and system s/r diverging even more. Runtime pm is fully platform
agnostic, so this should be possible without add a single IS_VLV check
anywhere in the code.
-Daniel

 ---
  drivers/gpu/drm/i915/i915_drv.c  | 84 
 
  drivers/gpu/drm/i915/i915_drv.h  |  1 +
  drivers/gpu/drm/i915/intel_drv.h |  7 
  3 files changed, 85 insertions(+), 7 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
 index 6c4b25c..fdfeccf 100644
 --- a/drivers/gpu/drm/i915/i915_drv.c
 +++ b/drivers/gpu/drm/i915/i915_drv.c
 @@ -495,6 +495,26 @@ static int i915_drm_freeze(struct drm_device *dev)
   struct drm_i915_private *dev_priv = dev-dev_private;
   struct drm_crtc *crtc;
   pci_power_t opregion_target_state;
 + u32 mask;
 + int err = 0;
 +
 + /* Following sequence from vlv_runtime_suspend */
 + if (IS_VALLEYVIEW(dev)) {
 + /*
 +  * Bspec defines the following GT well on flags as debug only,
 +  * so don't treat them as hard failures.
 +  */
 + (void)vlv_wait_for_gt_wells(dev_priv, false);
 +
 + mask = VLV_GTLC_RENDER_CTX_EXISTS | VLV_GTLC_MEDIA_CTX_EXISTS;
 + WARN_ON((I915_READ(VLV_GTLC_WAKE_CTRL)  mask) != mask);
 +
 + vlv_check_no_gt_access(dev_priv);
 +
 + err = vlv_force_gfx_clock(dev_priv, true);
 + if (err)
 + goto err1;
 + }
  
   /* ignore lid events during suspend */
   mutex_lock(dev_priv-modeset_restore_lock);
 @@ -542,6 +562,10 @@ static int i915_drm_freeze(struct drm_device *dev)
  
   i915_gem_suspend_gtt_mappings(dev);
  
 + /* Save Gunit State */
 + if (IS_VALLEYVIEW(dev))
 + vlv_save_gunit_s0ix_state(dev_priv);
 +
   i915_save_state(dev);
  
   opregion_target_state = PCI_D3cold;
 @@ -562,7 +586,27 @@ static int i915_drm_freeze(struct drm_device *dev)
  
   intel_display_set_init_power(dev_priv, false);
  
 - return 0;
 + /* Clear Allow Wake Bit so that none of the
 +  * force/demand wake requests
 +  */
 + if (IS_VALLEYVIEW(dev)) {
 + err = vlv_allow_gt_wake(dev_priv, false);
 + if (err)
 + goto err2;
 +
 + /* Release graphics clocks */
 + vlv_force_gfx_clock(dev_priv, false);
 + }
 + return err;
 +err2:
 + /* For safety always re-enable waking and disable gfx clock forcing */
 + if (IS_VALLEYVIEW(dev))
 + vlv_allow_gt_wake(dev_priv, true);
 +err1:
 + if (IS_VALLEYVIEW(dev))
 + vlv_force_gfx_clock(dev_priv, false);
 +
 + return err;
  }
  
  int i915_suspend(struct drm_device *dev, pm_message_t state)
 @@ -610,6 +654,26 @@ void intel_console_resume(struct work_struct *work)
  static int i915_drm_thaw_early(struct drm_device *dev)
  {
   struct drm_i915_private *dev_priv = dev-dev_private;
 + int ret = 0;
 + int err;
 +
 + /*
 +  * Following sequence from vlv_runtime_resume. Clock is released
 +  * in i915_drm_thaw.
 +  * If any of the steps fail just try to continue, that's the best we
 +  * can do at this point. Return the first error code (which will also
 +  * leave RPM permanently disabled).
 +  */
 +
 + if (IS_VALLEYVIEW(dev)) {
 + ret = vlv_force_gfx_clock(dev_priv, true);
 +
 + vlv_restore_gunit_s0ix_state(dev_priv);
 +
 + err = vlv_allow_gt_wake(dev_priv, true);
 + if (!ret)
 + ret = err;
 + }
  
   if (IS_HASWELL(dev) || IS_BROADWELL(dev))
   hsw_disable_pc8(dev_priv);
 @@ -618,7 +682,7 @@ static int i915_drm_thaw_early(struct drm_device *dev)
   intel_uncore_sanitize(dev);
   intel_power_domains_init_hw(dev_priv);
  
 - return 0;
 + return ret;
  }
  
  static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
 @@ -695,6 +759,9 @@ static int __i915_drm_thaw(struct drm_device *dev, bool 
 restore_gtt_mappings)
  
   intel_opregion_notify_adapter(dev, PCI_D0);
  
 + /* Release graphics clocks turned on in thaw_early*/
 + vlv_force_gfx_clock(dev_priv, false);
 +
   return 0;
  }
  
 @@ -1028,7 +1095,7 @@ static int hsw_runtime_resume(struct drm_i915_private 
 *dev_priv)
   * a black-box for the driver. 

[Intel-gfx] [PATCH] drm/i915: Fix read back of plane stride register

2014-07-28 Thread rafael . barbalho
From: Rafael Barbalho rafael.barba...@intel.com

According to the specifications bit 6 is actually valid in the stride register.

Cc: Jesse Barnes jbar...@virtuousgeek.org
Cc: Ville Syrjälä ville.syrj...@linux.intel.com
Signed-off-by: Rafael Barbalho rafael.barba...@intel.com
---
 drivers/gpu/drm/i915/intel_display.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 99eb7ca..52dab31 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6221,7 +6221,7 @@ static void i9xx_get_plane_config(struct intel_crtc *crtc,
crtc-base.primary-fb-height = ((val  0)  0xfff) + 1;
 
val = I915_READ(DSPSTRIDE(pipe));
-   crtc-base.primary-fb-pitches[0] = val  0xff80;
+   crtc-base.primary-fb-pitches[0] = val  0xffc0;
 
aligned_height = intel_align_height(dev, crtc-base.primary-fb-height,
plane_config-tiled);
@@ -7241,7 +7241,7 @@ static void ironlake_get_plane_config(struct intel_crtc 
*crtc,
crtc-base.primary-fb-height = ((val  0)  0xfff) + 1;
 
val = I915_READ(DSPSTRIDE(pipe));
-   crtc-base.primary-fb-pitches[0] = val  0xff80;
+   crtc-base.primary-fb-pitches[0] = val  0xffc0;
 
aligned_height = intel_align_height(dev, crtc-base.primary-fb-height,
plane_config-tiled);
-- 
2.0.3

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [RFC] drm/i915/bdw: Initialize BDW workarounds in render ring init fn

2014-07-28 Thread Daniel Vetter
On Mon, Jul 28, 2014 at 08:00:39PM +0300, Ville Syrjälä wrote:
 On Mon, Jul 28, 2014 at 05:31:46PM +0100, arun.siluv...@linux.intel.com wrote:
  From: Arun Siluvery arun.siluv...@linux.intel.com
  
  The workarounds at the moment are initialized in init_clock_gating() but
  they are lost during reset; In case of execlists some workarounds modify
  registers that are part of register state context, since these are not
  initialized until init_clock_gating() default context ends up with
  incorrect values as render context is restored and saved before updated
  by workarounds hence move them to render ring init fn. This should be
  ok as these workarounds are not related to display clock gating.
  
  Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com
  ---
   drivers/gpu/drm/i915/i915_debugfs.c | 46 ++
   drivers/gpu/drm/i915/intel_pm.c | 59 
   drivers/gpu/drm/i915/intel_ringbuffer.c | 68 
  +
   3 files changed, 114 insertions(+), 59 deletions(-)
  
  diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
  b/drivers/gpu/drm/i915/i915_debugfs.c
  index 083683c..cf7da30 100644
  --- a/drivers/gpu/drm/i915/i915_debugfs.c
  +++ b/drivers/gpu/drm/i915/i915_debugfs.c
  @@ -2397,20 +2397,65 @@ 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 i915_workaround_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;
  +   struct drm_i915_private *dev_priv = dev-dev_private;
  +   int ret;
  +
  +   ret = mutex_lock_interruptible(dev-struct_mutex);
  +   if (ret)
  +   return ret;
  +
  +   if (IS_BROADWELL(dev)) {
  +   seq_printf(m, GEN8_ROW_CHICKEN:\t0x%08x\n,
  +  I915_READ(GEN8_ROW_CHICKEN));
  +   seq_printf(m, HALF_SLICE_CHICKEN3:\t0x%08x\n,
  +  I915_READ(HALF_SLICE_CHICKEN3));
  +   seq_printf(m, GAMTARBMODE:\t0x%08x\n, I915_READ(GAMTARBMODE));
  +   seq_printf(m, _3D_CHICKEN3:\t0x%08x\n,
  +  I915_READ(_3D_CHICKEN3));
  +   seq_printf(m, COMMON_SLICE_CHICKEN2:\t0x%08x\n,
  +  I915_READ(COMMON_SLICE_CHICKEN2));
  +   seq_printf(m, GEN7_HALF_SLICE_CHICKEN1:\t0x%08x\n,
  +  I915_READ(GEN7_HALF_SLICE_CHICKEN1));
  +   seq_printf(m, GEN7_ROW_CHICKEN2:\t0x%08x\n,
  +  I915_READ(GEN7_ROW_CHICKEN2));
  +   seq_printf(m, GAM_ECOCHK:\t0x%08x\n,
  +  I915_READ(GAM_ECOCHK));
  +   seq_printf(m, HDC_CHICKEN0:\t0x%08x\n,
  +  I915_READ(HDC_CHICKEN0));
  +   seq_printf(m, GEN7_FF_THREAD_MODE:\t0x%08x\n,
  +  I915_READ(GEN7_FF_THREAD_MODE));
  +   seq_printf(m, GEN8_UCGCTL6:\t0x%08x\n,
  +  I915_READ(GEN8_UCGCTL6));
  +   seq_printf(m, GEN6_RC_SLEEP_PSMI_CONTROL:\t0x%08x\n,
  +  I915_READ(GEN6_RC_SLEEP_PSMI_CONTROL));
  +   seq_printf(m, CACHE_MODE_1:\t0x%08x\n,
  +  I915_READ(CACHE_MODE_1));
  +   } else
  +   DRM_DEBUG_DRIVER(Not available for Gen%d\n,
  +INTEL_INFO(dev)-gen);
  +
  +   mutex_unlock(dev-struct_mutex);
  +   return 0;
  +}
  +
 
 This smells like a separate patch. But I'm not sure we want at all since
 intel_reg_read will provide the same information.

Yeah, debugfs files that just do what intel_reg_read does are just an
additional maintaince burden. I know that we have a few that dump lots of
registers, but most of them dump a lot of other information, too.
-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] [RFC] drm/i915/bdw: Initialize BDW workarounds in render ring init fn

2014-07-28 Thread Siluvery, Arun

On 28/07/2014 20:22, Daniel Vetter wrote:

On Mon, Jul 28, 2014 at 08:00:39PM +0300, Ville Syrjälä wrote:

On Mon, Jul 28, 2014 at 05:31:46PM +0100, arun.siluv...@linux.intel.com wrote:

From: Arun Siluvery arun.siluv...@linux.intel.com

The workarounds at the moment are initialized in init_clock_gating() but
they are lost during reset; In case of execlists some workarounds modify
registers that are part of register state context, since these are not
initialized until init_clock_gating() default context ends up with
incorrect values as render context is restored and saved before updated
by workarounds hence move them to render ring init fn. This should be
ok as these workarounds are not related to display clock gating.

Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com
---
  drivers/gpu/drm/i915/i915_debugfs.c | 46 ++
  drivers/gpu/drm/i915/intel_pm.c | 59 
  drivers/gpu/drm/i915/intel_ringbuffer.c | 68 +
  3 files changed, 114 insertions(+), 59 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 083683c..cf7da30 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2397,20 +2397,65 @@ 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 i915_workaround_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;
+   struct drm_i915_private *dev_priv = dev-dev_private;
+   int ret;
+
+   ret = mutex_lock_interruptible(dev-struct_mutex);
+   if (ret)
+   return ret;
+
+   if (IS_BROADWELL(dev)) {
+   seq_printf(m, GEN8_ROW_CHICKEN:\t0x%08x\n,
+  I915_READ(GEN8_ROW_CHICKEN));
+   seq_printf(m, HALF_SLICE_CHICKEN3:\t0x%08x\n,
+  I915_READ(HALF_SLICE_CHICKEN3));
+   seq_printf(m, GAMTARBMODE:\t0x%08x\n, I915_READ(GAMTARBMODE));
+   seq_printf(m, _3D_CHICKEN3:\t0x%08x\n,
+  I915_READ(_3D_CHICKEN3));
+   seq_printf(m, COMMON_SLICE_CHICKEN2:\t0x%08x\n,
+  I915_READ(COMMON_SLICE_CHICKEN2));
+   seq_printf(m, GEN7_HALF_SLICE_CHICKEN1:\t0x%08x\n,
+  I915_READ(GEN7_HALF_SLICE_CHICKEN1));
+   seq_printf(m, GEN7_ROW_CHICKEN2:\t0x%08x\n,
+  I915_READ(GEN7_ROW_CHICKEN2));
+   seq_printf(m, GAM_ECOCHK:\t0x%08x\n,
+  I915_READ(GAM_ECOCHK));
+   seq_printf(m, HDC_CHICKEN0:\t0x%08x\n,
+  I915_READ(HDC_CHICKEN0));
+   seq_printf(m, GEN7_FF_THREAD_MODE:\t0x%08x\n,
+  I915_READ(GEN7_FF_THREAD_MODE));
+   seq_printf(m, GEN8_UCGCTL6:\t0x%08x\n,
+  I915_READ(GEN8_UCGCTL6));
+   seq_printf(m, GEN6_RC_SLEEP_PSMI_CONTROL:\t0x%08x\n,
+  I915_READ(GEN6_RC_SLEEP_PSMI_CONTROL));
+   seq_printf(m, CACHE_MODE_1:\t0x%08x\n,
+  I915_READ(CACHE_MODE_1));
+   } else
+   DRM_DEBUG_DRIVER(Not available for Gen%d\n,
+INTEL_INFO(dev)-gen);
+
+   mutex_unlock(dev-struct_mutex);
+   return 0;
+}
+


This smells like a separate patch. But I'm not sure we want at all since
intel_reg_read will provide the same information.


Yeah, debugfs files that just do what intel_reg_read does are just an
additional maintaince burden. I know that we have a few that dump lots of
registers, but most of them dump a lot of other information, too.
-Daniel



I've added this mainly for testing workarounds which can be extended 
further as we move WAs for other chipsets but I agree it can be done 
with intel_reg_read.


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: s/seqno/request/ tracking inside objects

2014-07-28 Thread Jesse Barnes
On Fri, 25 Jul 2014 13:27:00 +0100
Chris Wilson ch...@chris-wilson.co.uk wrote:

 @@ -614,12 +615,12 @@ static int i915_gem_pageflip_info(struct
 seq_file *m, void *data) seq_printf(m, Flip pending (waiting for
 vsync) on pipe %c (plane %c)\n, pipe, plane);
   }
 - if (work-ring)
 + if (work-flip_queued_request) {
 + struct i915_gem_request *rq =
 work-flip_queued_request; seq_printf(m, Flip queued on %s at seqno
 %u, now %u\n,
 - work-ring-name,
 -
 work-flip_queued_seqno,
 -
 work-ring-get_seqno(work-ring, true));
 - else
 + rq-ring-name,
 rq-seqno,
 +
 rq-ring-get_seqno(rq-ring, true));
 + } else
   seq_printf(m, Flip not associated

I wonder if the overlay, flip queue and unlocked wait changes could be
split out somehow; I think they're the trickiest part of the change...

It does look like you're doing get/puts in the right places, though I
didn't check the error paths (and I'm not familiar at all with the
overlay bits, I guess Daniel will have to look at that).

 with any ring\n); seq_printf(m, Flip queued on frame %d, (was ready
 on frame %d), now %d\n, work-flip_queued_vblank,
 @@ -656,7 +657,7 @@ static int i915_gem_request_info(struct seq_file
 *m, void *data) struct drm_device *dev = node-minor-dev;
   struct drm_i915_private *dev_priv = dev-dev_private;
   struct intel_engine_cs *ring;
 - struct drm_i915_gem_request *gem_request;
 + struct i915_gem_request *rq;
   int ret, count, i;
  
   ret = mutex_lock_interruptible(dev-struct_mutex);
 @@ -669,12 +670,10 @@ static int i915_gem_request_info(struct
 seq_file *m, void *data) continue;
  
   seq_printf(m, %s requests:\n, ring-name);
 - list_for_each_entry(gem_request,
 - ring-request_list,
 - list) {
 + list_for_each_entry(rq, ring-request_list, list) {
   seq_printf(m, %d @ %d\n,
 -gem_request-seqno,
 -(int) (jiffies -
 gem_request-emitted_jiffies));
 +rq-seqno,
 +(int)(jiffies -
 rq-emitted_jiffies)); }
   count++;
   }

Semi gratuitous rename (though I know you renamed the struct to catch
all the users).

 diff --git a/drivers/gpu/drm/i915/i915_drv.h
 b/drivers/gpu/drm/i915/i915_drv.h index 9837b0f..5794d096 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -187,6 +187,7 @@ enum hpd_pin {
  struct drm_i915_private;
  struct i915_mm_struct;
  struct i915_mmu_object;
 +struct i915_gem_request;
  
  enum intel_dpll_id {
   DPLL_ID_PRIVATE = -1, /* non-shared dpll in use */
 @@ -1720,16 +1721,15 @@ struct drm_i915_gem_object {
   struct drm_mm_node *stolen;
   struct list_head global_list;
  
 - struct list_head ring_list;
   /** Used in execbuf to temporarily hold a ref */
   struct list_head obj_exec_link;
  
   /**
* This is set if the object is on the active lists (has
 pending
 -  * rendering and so a non-zero seqno), and is not set if it
 i s on
 -  * inactive (ready to be unbound) list.
 +  * rendering and so a submitted request), and is not set if
 it is on
 +  * inactive (ready to be unbound) list. We track activity
 per engine. */
 - unsigned int active:1;
 + unsigned int active:3;

I wonder if it would be better to be explicit and have a num_rings
sized array here then.  We need 4 bits anyway for the second video ring
right?  We'd need a wrapper to check for active then though...

 @@ -1850,7 +1850,9 @@ void i915_gem_track_fb(struct
 drm_i915_gem_object *old,
   * sequence-number comparisons on buffer last_rendering_seqnos, and
 associate
   * an emission time with seqnos for tracking how far ahead of the
 GPU we are. */
 -struct drm_i915_gem_request {
 +struct i915_gem_request {
 + struct kref kref;
 +
   /** On Which ring this request was generated */
   struct intel_engine_cs *ring;
  
 @@ -1878,8 +1880,60 @@ struct drm_i915_gem_request {
   struct drm_i915_file_private *file_priv;
   /** file_priv list entry for this request */
   struct list_head client_list;
 +
 + bool completed:1;
  };

If Daniel pulls in Greg's tree, the kref could turn into a fence and
the completed could be removed.

  
 +static inline struct intel_engine_cs *i915_request_ring(struct
 i915_gem_request *rq) +{
 + return rq ? rq-ring : NULL;
 +}
 +
 +static inline int i915_request_ring_id(struct i915_gem_request *rq)
 +{
 + return rq ? rq-ring-id : -1;
 +}
 +
 +static inline u32 i915_request_seqno(struct i915_gem_request *rq)
 +{
 + return rq ? rq-seqno : 0;
 +}
 +
 +/**
 + * Returns true if seq1 

Re: [Intel-gfx] [RFC] drm/i915/bdw: Initialize BDW workarounds in render ring init fn

2014-07-28 Thread Siluvery, Arun

On 28/07/2014 18:00, Ville Syrjälä wrote:

On Mon, Jul 28, 2014 at 05:31:46PM +0100, arun.siluv...@linux.intel.com wrote:

From: Arun Siluvery arun.siluv...@linux.intel.com

The workarounds at the moment are initialized in init_clock_gating() but
they are lost during reset; In case of execlists some workarounds modify
registers that are part of register state context, since these are not
initialized until init_clock_gating() default context ends up with
incorrect values as render context is restored and saved before updated
by workarounds hence move them to render ring init fn. This should be
ok as these workarounds are not related to display clock gating.

Signed-off-by: Arun Siluvery arun.siluv...@linux.intel.com
---
  drivers/gpu/drm/i915/i915_debugfs.c | 46 ++
  drivers/gpu/drm/i915/intel_pm.c | 59 
  drivers/gpu/drm/i915/intel_ringbuffer.c | 68 +
  3 files changed, 114 insertions(+), 59 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 083683c..cf7da30 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2397,20 +2397,65 @@ 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 i915_workaround_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;
+   struct drm_i915_private *dev_priv = dev-dev_private;
+   int ret;
+
+   ret = mutex_lock_interruptible(dev-struct_mutex);
+   if (ret)
+   return ret;
+
+   if (IS_BROADWELL(dev)) {
+   seq_printf(m, GEN8_ROW_CHICKEN:\t0x%08x\n,
+  I915_READ(GEN8_ROW_CHICKEN));
+   seq_printf(m, HALF_SLICE_CHICKEN3:\t0x%08x\n,
+  I915_READ(HALF_SLICE_CHICKEN3));
+   seq_printf(m, GAMTARBMODE:\t0x%08x\n, I915_READ(GAMTARBMODE));
+   seq_printf(m, _3D_CHICKEN3:\t0x%08x\n,
+  I915_READ(_3D_CHICKEN3));
+   seq_printf(m, COMMON_SLICE_CHICKEN2:\t0x%08x\n,
+  I915_READ(COMMON_SLICE_CHICKEN2));
+   seq_printf(m, GEN7_HALF_SLICE_CHICKEN1:\t0x%08x\n,
+  I915_READ(GEN7_HALF_SLICE_CHICKEN1));
+   seq_printf(m, GEN7_ROW_CHICKEN2:\t0x%08x\n,
+  I915_READ(GEN7_ROW_CHICKEN2));
+   seq_printf(m, GAM_ECOCHK:\t0x%08x\n,
+  I915_READ(GAM_ECOCHK));
+   seq_printf(m, HDC_CHICKEN0:\t0x%08x\n,
+  I915_READ(HDC_CHICKEN0));
+   seq_printf(m, GEN7_FF_THREAD_MODE:\t0x%08x\n,
+  I915_READ(GEN7_FF_THREAD_MODE));
+   seq_printf(m, GEN8_UCGCTL6:\t0x%08x\n,
+  I915_READ(GEN8_UCGCTL6));
+   seq_printf(m, GEN6_RC_SLEEP_PSMI_CONTROL:\t0x%08x\n,
+  I915_READ(GEN6_RC_SLEEP_PSMI_CONTROL));
+   seq_printf(m, CACHE_MODE_1:\t0x%08x\n,
+  I915_READ(CACHE_MODE_1));
+   } else
+   DRM_DEBUG_DRIVER(Not available for Gen%d\n,
+INTEL_INFO(dev)-gen);
+
+   mutex_unlock(dev-struct_mutex);
+   return 0;
+}
+


This smells like a separate patch. But I'm not sure we want at all since
intel_reg_read will provide the same information.


  struct pipe_crc_info {
const char *name;
struct drm_device *dev;
enum pipe pipe;
  };

  static int i915_pipe_crc_open(struct inode *inode, struct file *filep)
  {
struct pipe_crc_info *info = inode-i_private;
struct drm_i915_private *dev_priv = info-dev-dev_private;
@@ -3904,20 +3949,21 @@ static const struct drm_info_list i915_debugfs_list[] = 
{
{i915_ppgtt_info, i915_ppgtt_info, 0},
{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_workaround_info, i915_workaround_info, 0},
  };
  #define I915_DEBUGFS_ENTRIES ARRAY_SIZE(i915_debugfs_list)

  static const struct i915_debugfs_files {
const char *name;
const struct 

Re: [Intel-gfx] [PATCH] mutex: Export an interface to wrap a mutex lock

2014-07-28 Thread Ben Widawsky
On Sat, Jul 26, 2014 at 09:01:55AM +0100, Chris Wilson wrote:
 In i915, we have a big mutex around our device struct - every time before
 we attempt to communicate with the GPU, we acquire the mutex. This makes
 it a convenient juncture to place our GPU error handling - before we take
 the mutex we first check whether the GPU is hung or whether we are in
 the process of recovering from a GPU hang. So we wrap the call to
 mutex_lock() alongside our additional error handling routines.
 
 The downside of using a wrapper around mutex_lock() is that lockdep and
 lockstat cannot discriminate the true callers of mutex_lock(). Unless we
 provide a means for the wrapper to pass that information down.
 
 It also appears that i915 is unique in this manner of wrapping
 mutex_lock().
 
 Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk
 Cc: Ben Widawsky benjamin.widaw...@intel.com

With the one fix inline, take your pick:

Reported-and-tested-by: Ben Widawsky b...@bwidawsk.net
Reviewed-by: Ben Widawsky b...@bwidawsk.net

I find it very strange that i915 is unique here. It makes me feel like
we're doin' in wrong. BTW, a cleaned up version of the macro version I
sent you might meet with less resistance given that no other driver
needs it [currently].

Also, looking at this code now, we could probably replace a bunch of
mutex_lock()'s in i915 with mutex_lock_wrapper(..., TASK_KILLABLE, ...)


 ---
  drivers/gpu/drm/i915/i915_gem.c |  4 +++-
  include/linux/mutex.h   |  9 +
  kernel/locking/mutex.c  | 36 
  3 files changed, 48 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
 index ae3c7b6162f5..888acd01db44 100644
 --- a/drivers/gpu/drm/i915/i915_gem.c
 +++ b/drivers/gpu/drm/i915/i915_gem.c
 @@ -233,7 +233,9 @@ int i915_mutex_lock_interruptible(struct drm_device *dev)
   if (ret)
   return ret;
  
 - ret = mutex_lock_interruptible(dev-struct_mutex);
 + ret = mutex_lock_wrapper(dev-struct_mutex,
 +  TASK_INTERRUPTIBLE,
 +  _RET_IP_);
   if (ret)
   return ret;
  
 diff --git a/include/linux/mutex.h b/include/linux/mutex.h
 index 42aa9b9ecd5f..b88255a390a2 100644
 --- a/include/linux/mutex.h
 +++ b/include/linux/mutex.h
 @@ -143,10 +143,15 @@ extern int __must_check 
 mutex_lock_interruptible_nested(struct mutex *lock,
   unsigned int subclass);
  extern int __must_check mutex_lock_killable_nested(struct mutex *lock,
   unsigned int subclass);
 +extern int __must_check mutex_lock_wrapper_nested(struct mutex *lock,
 +   unsigned int subclass,
 +   long state,
 +   unsigned long ip);
  
  #define mutex_lock(lock) mutex_lock_nested(lock, 0)
  #define mutex_lock_interruptible(lock) mutex_lock_interruptible_nested(lock, 
 0)
  #define mutex_lock_killable(lock) mutex_lock_killable_nested(lock, 0)
 +#define mutex_lock_wrapper(lock, state, ip) mutex_lock_killable_nested(lock, 
 0, state, ip)

s/mutex_lock_killable_nested/mutex_lock_wrapper_nested/

  
  #define mutex_lock_nest_lock(lock, nest_lock)
 \
  do { \
 @@ -158,10 +163,14 @@ do {
 \
  extern void mutex_lock(struct mutex *lock);
  extern int __must_check mutex_lock_interruptible(struct mutex *lock);
  extern int __must_check mutex_lock_killable(struct mutex *lock);
 +extern int __must_check mutex_lock_wrapper(struct mutex *lock,
 +long state,
 +unsigned long ip);
  
  # define mutex_lock_nested(lock, subclass) mutex_lock(lock)
  # define mutex_lock_interruptible_nested(lock, subclass) 
 mutex_lock_interruptible(lock)
  # define mutex_lock_killable_nested(lock, subclass) mutex_lock_killable(lock)
 +# define mutex_lock_wrapper_nested(lock, subclass, state, ip) 
 mutex_lock_wrapper(lock, state, ip)
  # define mutex_lock_nest_lock(lock, nest_lock) mutex_lock(lock)
  #endif
  
 diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
 index acca2c1a3c5e..243ee5fc5dc3 100644
 --- a/kernel/locking/mutex.c
 +++ b/kernel/locking/mutex.c
 @@ -619,6 +619,17 @@ mutex_lock_interruptible_nested(struct mutex *lock, 
 unsigned int subclass)
  
  EXPORT_SYMBOL_GPL(mutex_lock_interruptible_nested);
  
 +int __sched
 +mutex_lock_wrapper_nested(struct mutex *lock, unsigned int subclass,
 +   long state, unsigned long ip)
 +{
 + might_sleep();
 + return __mutex_lock_common(lock, state,
 +subclass, NULL, ip, NULL, 0);
 +}
 +
 +EXPORT_SYMBOL_GPL(mutex_lock_wrapper_nested);
 +
  

[Intel-gfx] [PATCH 1/2] drm/i915: Collect gtier properly on HSW.

2014-07-28 Thread Rodrigo Vivi
GTIER and DEIER doesn't have same interface on HSW so this or operation
makes the information provided useless.

Cc: Paulo Zanoni paulo.r.zan...@intel.com
Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com
---
 drivers/gpu/drm/i915/i915_drv.h   |  1 +
 drivers/gpu/drm/i915/i915_gpu_error.c | 16 ++--
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ef38c3b..ccb97f1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -314,6 +314,7 @@ struct drm_i915_error_state {
u32 eir;
u32 pgtbl_er;
u32 ier;
+   u32 gtier;
u32 ccid;
u32 derrmr;
u32 forcewake;
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c 
b/drivers/gpu/drm/i915/i915_gpu_error.c
index 0b3f694..372fea3 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -359,6 +359,8 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf 
*m,
err_printf(m, PCI ID: 0x%04x\n, dev-pdev-device);
err_printf(m, EIR: 0x%08x\n, error-eir);
err_printf(m, IER: 0x%08x\n, error-ier);
+   if (IS_HASWELL(dev))
+   err_printf(m, GTIER: 0x%08x\n, error-gtier);
err_printf(m, PGTBL_ER: 0x%08x\n, error-pgtbl_er);
err_printf(m, FORCEWAKE: 0x%08x\n, error-forcewake);
err_printf(m, DERRMR: 0x%08x\n, error-derrmr);
@@ -1135,13 +1137,15 @@ static void i915_capture_reg_state(struct 
drm_i915_private *dev_priv,
if (HAS_HW_CONTEXTS(dev))
error-ccid = I915_READ(CCID);
 
-   if (HAS_PCH_SPLIT(dev))
+   if (IS_HASWELL(dev)) {
+   error-ier = I915_READ(DEIER);
+   error-gtier = I915_READ(GTIER);
+   } else if (HAS_PCH_SPLIT(dev)) {
error-ier = I915_READ(DEIER) | I915_READ(GTIER);
-   else {
-   if (IS_GEN2(dev))
-   error-ier = I915_READ16(IER);
-   else
-   error-ier = I915_READ(IER);
+   } else if (IS_GEN2(dev)) {
+   error-ier = I915_READ16(IER);
+   } else {
+   error-ier = I915_READ(IER);
}
 
/* 4: Everything else */
-- 
1.9.3

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 0/3] Fixes for runtime PM on planes APIs

2014-07-28 Thread Matt Roper
On Mon, Jul 28, 2014 at 03:37:11PM -0300, Paulo Zanoni wrote:
 From: Paulo Zanoni paulo.r.zan...@intel.com
 
 Hi
 
 This series fixes some bugs that happen when we're runtime suspended and try 
 to
 use the planes APIs. I also wrote IGT test cases for the bugs, so we will be
 able to detect future regressions.
 
 The controversial part of these patches is that we had previously defined that
 we wanted to get/put runtime PM in the highest level of the stack, wrapping as
 much code as possible, but Daniel asked me to only get/put runtime PM around 
 the
 functions that pin the objects (still on the highest level, but only around 
 the
 pin functions). This series implements Daniel's suggestions.

These look pretty straightforward to me, so for all three:
Reviewed-by: Matt Roper matthew.d.ro...@intel.com

However as a side note on the runtime PM stuff I'll admit that it's not
an area I'd previously paid too much attention to and my first reaction
looking at the patches was I wonder if intel_runtime_pm_{get,put} could
be pushed down into intel_pin_and_fence_fb_obj().  Until I read your
cover letter I wasn't aware of the goal of get/put runtime PM in the
highest level of the stack, wrapping as much code as possible and I
don't think that's really explained anywhere in the code or in the
commit log today.  Maybe we could add a comment in intel_pm.c explaining
that design goal for future contributors/reviewers?  


Matt

 
 Thanks,
 Paulo
 
 
 Paulo Zanoni (3):
   drm/i915: fix cursor handling when runtime suspended
   drm/i915: get runtime PM when pinning sprite objects
   drm/i915: get runtime PM when pinning primary plane objects
 
  drivers/gpu/drm/i915/intel_display.c | 9 +
  drivers/gpu/drm/i915/intel_sprite.c  | 3 +++
  2 files changed, 12 insertions(+)
 
 -- 
 2.0.1
 
 ___
 Intel-gfx mailing list
 Intel-gfx@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling  Development
Intel Corporation
(916) 356-2795
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 2/2] tests/pm_rpm: add planes subtests

2014-07-28 Thread Matt Roper
On Mon, Jul 28, 2014 at 03:37:15PM -0300, Paulo Zanoni wrote:
 From: Paulo Zanoni paulo.r.zan...@intel.com
 
 Just like the cursor subtests, these also trigger WARNs on the current
 Kernel.
 
 Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com

I feel like a lot of the setup you have to do here is duplicating logic
we have in the igt_kms library.  Was there functionality lacking from
that library that prevented you from using it to write this test?  If
so, I can look into adding it.

Anyway, the test still looks good, just one or two minor comments inline
below.

 ---
  tests/pm_rpm.c | 212 
 -
  1 file changed, 211 insertions(+), 1 deletion(-)
 
 diff --git a/tests/pm_rpm.c b/tests/pm_rpm.c
 index f720f86..048d9ad 100644
 --- a/tests/pm_rpm.c
 +++ b/tests/pm_rpm.c
 @@ -51,6 +51,9 @@
  #include igt_kms.h
  #include igt_debugfs.h
  
 +/* One day, this will be on your libdrm. */
 +#define DRM_CLIENT_CAP_UNIVERSAL_PLANES 2

I think there was just an official release of libdrm today that finally
includes this and the plane type enum below...maybe we can just bump the
libdrm requirement for igt and stop including these by hand?

 +
  #define MSR_PC8_RES  0x630
  #define MSR_PC9_RES  0x631
  #define MSR_PC10_RES 0x632
 @@ -72,6 +75,12 @@ enum screen_type {
   SCREEN_TYPE_ANY,
  };
  
 +enum plane_type {
 + PLANE_OVERLAY,
 + PLANE_PRIMARY,
 + PLANE_CURSOR,
 +};
 +
  /* Wait flags */
  #define DONT_WAIT0
  #define WAIT_STATUS  1
 @@ -1504,6 +1513,199 @@ static void cursor_subtest(bool dpms)
   igt_assert(wait_for_suspended());
  }
  
 +static enum plane_type get_plane_type(uint32_t plane_id)
 +{
 + drmModeObjectPropertiesPtr props;
 + int i, j;
 + enum plane_type type;
 + bool found = false;
 +
 + props = drmModeObjectGetProperties(drm_fd, plane_id,
 +DRM_MODE_OBJECT_PLANE);
 + igt_assert(props);
 +
 + for (i = 0; i  props-count_props  !found; i++) {
 + drmModePropertyPtr prop;
 + const char *enum_name = NULL;
 +
 + prop = drmModeGetProperty(drm_fd, props-props[i]);
 + igt_assert(prop);
 +
 + if (strcmp(prop-name, type) == 0) {
 + igt_assert(prop-flags  DRM_MODE_PROP_ENUM);
 + igt_assert(props-prop_values[i]  prop-count_enums);
 +
 + for (j = 0; j  prop-count_enums; j++) {
 + if (prop-enums[j].value ==
 + props-prop_values[i]) {
 + enum_name = prop-enums[j].name;
 + break;
 + }
 + }
 + igt_assert(enum_name);
 +
 + if (strcmp(enum_name, Overlay) == 0)
 + type = PLANE_OVERLAY;
 + else if (strcmp(enum_name, Primary) == 0)
 + type = PLANE_PRIMARY;
 + else if (strcmp(enum_name, Cursor) == 0)
 + type = PLANE_CURSOR;
 + else
 + igt_assert(0);
 +
 + found = true;
 + }
 +
 + drmModeFreeProperty(prop);
 + }
 + igt_assert(found);
 +
 + drmModeFreeObjectProperties(props);
 + return type;
 +}
 +
 +static void test_one_plane(bool dpms, uint32_t plane_id,
 +enum plane_type plane_type)
 +{
 + int rc;
 + uint32_t plane_format, plane_w, plane_h;
 + uint32_t crtc_id, connector_id;
 + struct igt_fb scanout_fb, plane_fb1, plane_fb2;
 + drmModeModeInfoPtr mode;
 + int32_t crtc_x = 0, crtc_y = 0;
 +
 + disable_all_screens(ms_data);
 + igt_assert(wait_for_suspended());
 +
 + igt_require(find_connector_for_modeset(ms_data, SCREEN_TYPE_ANY,
 +connector_id, mode));
 +
 + crtc_id = ms_data.res-crtcs[0];
 + igt_assert(crtc_id);
 +
 + igt_create_fb(drm_fd, mode-hdisplay, mode-vdisplay,
 +   DRM_FORMAT_XRGB, false, scanout_fb);
 +
 + fill_igt_fb(scanout_fb, 0xFF);
 +
 + switch (plane_type) {
 + case PLANE_OVERLAY:
 + plane_format = DRM_FORMAT_XRGB;
 + plane_w = 64;
 + plane_h = 64;
 + break;
 + case PLANE_PRIMARY:
 + plane_format = DRM_FORMAT_XRGB;
 + plane_w = mode-hdisplay;
 + plane_h = mode-vdisplay;
 + break;
 + case PLANE_CURSOR:
 + plane_format = DRM_FORMAT_ARGB;
 + plane_w = 64;
 + plane_h = 64;
 + break;
 + default:
 + igt_assert(0);
 + break;
 + }
 +
 + igt_create_fb(drm_fd, plane_w, plane_h, plane_format, false,
 +   plane_fb1);
 + igt_create_fb(drm_fd, plane_w, 

Re: [Intel-gfx] [PATCH] drm/i915: Rework GPU reset sequence to match driver load thaw

2014-07-28 Thread Ben Widawsky
On Mon, Jul 28, 2014 at 05:12:59PM +, Mcaulay, Alistair wrote:
 Hi Ben / Daniel,
 I agree that this needs to be properly tested. Are there any particular igt 
 tests you would suggest I use?
 I've been running:
 drv_hangman, drv_suspend, gem_hangcheck_forcewake.

I thought IGT had added some random reset stuff in the way that we have
for signals. However, it looks like I imagined it. I guess you can add
gem_hang, gem_reset_stats, and tests/debugfs_wedged to that list. Daniel
can probably provide any I might have missed.

The way that igt quiesces everything these days really hurts the ability
to test multi-process. If every tests starts off with no work, and
running the default context, things are pretty trivial. Similarly,
running these tests in isolation, even if it isn't quiescing doesn't
help the situation. The way I wrote the code originally was through
debugging hangs on a desktop as I developed patches, and not with IGT
(though drv_hangman could catch many issues). I'd definitely recommend
trying to invoke hangs on a running desktop. I'd advise doing this by
modifying mesa to submit a crap batch, I can provide you more details on
how to do this if you need it. Also try to disable the quiescing in IGT
and run more than these tests in isolation.

 
 Also do you have a set of PPGTT Patches that should work with these tests. 
 Michel sent me a set of patches to enable
 PPGTT, but these 3 tests fail with the patches.

I will try to reproduce this on my patch series when I have some time
and if nothing else, that should be a good preparation/refresher for the
patch review anyway. The patches I wrote shouldn't have touched much on
these paths - not sure if Michel changed anything there.

With patch on top of what Michel sent you, everything passes?

 
 Thanks,
 Alistair.
 
 -Original Message-
 From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter
 Sent: Monday, July 28, 2014 10:27 AM
 To: Ben Widawsky
 Cc: Mcaulay, Alistair; intel-gfx@lists.freedesktop.org
 Subject: Re: [Intel-gfx] [PATCH] drm/i915: Rework GPU reset sequence to match 
 driver load  thaw
 
 On Fri, Jul 25, 2014 at 06:05:29PM -0700, Ben Widawsky wrote:
  On Wed, Jul 16, 2014 at 04:05:59PM +0100, alistair.mcau...@intel.com wrote:
   From: McAulay, Alistair alistair.mcau...@intel.com
   
   This patch is to address Daniels concerns over different code during 
   reset:
   
   http://lists.freedesktop.org/archives/intel-gfx/2014-June/047758.htm
   l
   
   The reason for aiming as hard as possible to use the exact same 
   code for driver load, gpu reset and runtime pm/system resume is that 
   we've simply seen too many bugs due to slight variations and unintended 
   omissions.
   
   Tested using igt drv_hangman.
   
   Signed-off-by: McAulay, Alistair alistair.mcau...@intel.com
  
  2 quick comments before I actually do a real review.
  1. Did you actually run this with and without full ppgtt?
  2. I don't think this is actually fulfilling what Daniel is 
  requesting, though we can let him comment.
 
 Mostly looks like what I think we need. Comments below.
 
  3. Did you reall do #1?
  
  Assuming you satisifed #1, can you please post the igt results for the 
  permutations (pre patch w/ and w/o ppgtt; post patch w/ and w/o ppgtt)
  
  I really want this data because I spent *a lot* of time with these 
  specific areas in the PPGTT work, and I am somewhat skeptical enough 
  of the code has changed that this will magically work. I also tried 
  the trickiness with the ring handling functions, and never succeeded. 
  Also, with the context stuff, I'm simply not convinced it can 
  magically vanish. If igt looks good, and Daniel agrees that this is 
  what he actually wanted, I will go fishing for corner cases and do the 
  review.
 
 I think the hack in ring_begin might explain why it never worked before.
 But fully agreed, we really need to test this well (and fill gaps if igt 
 misses anything around resets - we don't have any systematic gpu reset 
 coverage anywhere outside of igt).
 
  
  Thanks.
  
   ---
drivers/gpu/drm/i915/i915_gem.c |  2 -
drivers/gpu/drm/i915/i915_gem_context.c | 42 +
drivers/gpu/drm/i915/i915_gem_gtt.c | 67 
   +
drivers/gpu/drm/i915/i915_gem_gtt.h |  3 +-
drivers/gpu/drm/i915/intel_ringbuffer.c |  4 +-
5 files changed, 14 insertions(+), 104 deletions(-)
   
   diff --git a/drivers/gpu/drm/i915/i915_gem.c 
   b/drivers/gpu/drm/i915/i915_gem.c index ef047bc..b38e086 100644
   --- a/drivers/gpu/drm/i915/i915_gem.c
   +++ b/drivers/gpu/drm/i915/i915_gem.c
   @@ -2590,8 +2590,6 @@ void i915_gem_reset(struct drm_device *dev)
 for_each_ring(ring, dev_priv, i)
 i915_gem_reset_ring_cleanup(dev_priv, ring);

   - i915_gem_context_reset(dev);
   -
 i915_gem_restore_fences(dev);
}

   diff --git a/drivers/gpu/drm/i915/i915_gem_context.c