[Intel-gfx] ✓ Ro.CI.BAT: success for drm/i915: Hold irq uncore.lock when initialising fw_domains

2016-07-03 Thread Patchwork
== Series Details ==

Series: drm/i915: Hold irq uncore.lock when initialising fw_domains
URL   : https://patchwork.freedesktop.org/series/9432/
State : success

== Summary ==

Series 9432v1 drm/i915: Hold irq uncore.lock when initialising fw_domains
http://patchwork.freedesktop.org/api/1.0/series/9432/revisions/1/mbox

Test gem_exec_flush:
Subgroup basic-batch-kernel-default-cmd:
fail   -> PASS   (ro-byt-n2820)
Subgroup basic-batch-kernel-default-uc:
dmesg-fail -> PASS   (fi-skl-i5-6260u)
Test kms_pipe_crc_basic:
Subgroup hang-read-crc-pipe-a:
skip   -> PASS   (fi-skl-i5-6260u)

fi-kbl-qkkr  total:231  pass:160  dwarn:29  dfail:0   fail:2   skip:40 
fi-skl-i5-6260u  total:231  pass:204  dwarn:0   dfail:2   fail:0   skip:25 
fi-skl-i7-6700k  total:231  pass:190  dwarn:0   dfail:2   fail:0   skip:39 
fi-snb-i7-2600   total:231  pass:176  dwarn:0   dfail:0   fail:2   skip:53 
ro-bdw-i5-5250u  total:229  pass:204  dwarn:1   dfail:1   fail:0   skip:23 
ro-bdw-i7-5600u  total:229  pass:190  dwarn:0   dfail:1   fail:0   skip:38 
ro-bsw-n3050 total:229  pass:177  dwarn:0   dfail:1   fail:2   skip:49 
ro-byt-n2820 total:229  pass:181  dwarn:0   dfail:1   fail:2   skip:45 
ro-hsw-i3-4010u  total:229  pass:197  dwarn:0   dfail:1   fail:0   skip:31 
ro-hsw-i7-4770r  total:229  pass:197  dwarn:0   dfail:1   fail:0   skip:31 
ro-ilk-i7-620lm  total:229  pass:157  dwarn:0   dfail:1   fail:1   skip:70 
ro-ilk1-i5-650   total:224  pass:157  dwarn:0   dfail:1   fail:1   skip:65 
ro-ivb-i7-3770   total:229  pass:188  dwarn:0   dfail:1   fail:0   skip:40 
ro-skl3-i5-6260u total:229  pass:208  dwarn:1   dfail:1   fail:0   skip:19 
ro-snb-i7-2620M  total:229  pass:179  dwarn:0   dfail:1   fail:1   skip:48 
ro-bdw-i7-5557U failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_1389/

2fe5da8 drm-intel-nightly: 2016y-07m-02d-18h-31m-39s UTC integration manifest
f1e8235 drm/i915: Hold irq uncore.lock when initialising fw_domains

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


Re: [Intel-gfx] [PATCH v4 2/2] drm/i915/opregion: update cadl based on actually active outputs

2016-07-03 Thread Maarten Lankhorst
Op 29-06-16 om 17:36 schreef Jani Nikula:
> Previously we've just shoved the first eight devices in DIDL to CADL
> (list of active outputs). Some of the active outputs may have been left
> outside of CADL. The problem is, some BIOS implementations prevent
> laptop brightness hotkey propagation if the flat panel is not active.
>
> Now that we have connector to acpi device id mapping covered, we can
> update CADL based on which outputs are actually active.
>
> v3: actually git add the dev->dev_priv change.
>
> v4: update cadl in intel_shared_dpll_commit() if intel_state->modeset
> (Maarten)
>
> Cc: Maarten Lankhorst 
> Reviewed-and-tested-by: Peter Wu 
> Signed-off-by: Jani Nikula 
> ---
>  drivers/gpu/drm/i915/i915_drv.h   |  2 +
>  drivers/gpu/drm/i915/intel_display.c  |  4 ++
>  drivers/gpu/drm/i915/intel_opregion.c | 70 
> ++-
>  3 files changed, 43 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 724d34b00196..64ab52529be8 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3692,6 +3692,7 @@ extern int intel_opregion_notify_encoder(struct 
> intel_encoder *intel_encoder,
>  extern int intel_opregion_notify_adapter(struct drm_i915_private *dev_priv,
>pci_power_t state);
>  extern int intel_opregion_get_panel_type(struct drm_i915_private *dev_priv);
> +extern void intel_opregion_update_cadl(struct drm_i915_private *dev_priv);
>  #else
>  static inline int intel_opregion_setup(struct drm_i915_private *dev) { 
> return 0; }
>  static inline void intel_opregion_register(struct drm_i915_private 
> *dev_priv) { }
> @@ -3713,6 +3714,7 @@ static inline int intel_opregion_get_panel_type(struct 
> drm_i915_private *dev)
>  {
>   return -ENODEV;
>  }
> +static inline void intel_opregion_update_cadl(struct drm_i915_private 
> *dev_priv) { }
>  #endif
>  
>  /* intel_acpi.c */
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index d902a70edb84..4f404900f610 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13953,6 +13953,10 @@ static int intel_atomic_commit(struct drm_device 
> *dev,
>   dev_priv->wm.distrust_bios_wm = false;
>   dev_priv->wm.skl_results = intel_state->wm_results;
>   intel_shared_dpll_commit(state);
> +
> + if (intel_state->modeset)
> + intel_opregion_update_cadl(dev_priv);
> +
>   intel_atomic_track_fbs(state);
>  
>   if (nonblock)
> diff --git a/drivers/gpu/drm/i915/intel_opregion.c 
> b/drivers/gpu/drm/i915/intel_opregion.c
> index 632f0178c2b0..8b3f7e6ae4bb 100644
> --- a/drivers/gpu/drm/i915/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> @@ -642,24 +642,6 @@ static struct notifier_block intel_opregion_notifier = {
>   * (version 3)
>   */
>  
> -static u32 get_did(struct intel_opregion *opregion, int i)
> -{
> - u32 did;
> -
> - if (i < ARRAY_SIZE(opregion->acpi->didl)) {
> - did = opregion->acpi->didl[i];
> - } else {
> - i -= ARRAY_SIZE(opregion->acpi->didl);
> -
> - if (WARN_ON(i >= ARRAY_SIZE(opregion->acpi->did2)))
> - return 0;
> -
> - did = opregion->acpi->did2[i];
> - }
> -
> - return did;
> -}
> -
>  static void set_did(struct intel_opregion *opregion, int i, u32 val)
>  {
>   if (i < ARRAY_SIZE(opregion->acpi->didl)) {
> @@ -674,6 +656,14 @@ static void set_did(struct intel_opregion *opregion, int 
> i, u32 val)
>   }
>  }
>  
> +static void set_cad(struct intel_opregion *opregion, int i, u32 val)
> +{
> + if (WARN_ON(i >= ARRAY_SIZE(opregion->acpi->cadl)))
> + return;
> +
> + opregion->acpi->cadl[i] = val;
> +}
> +
>  static u32 acpi_display_type(struct intel_connector *connector)
>  {
>   u32 display_type;
> @@ -759,22 +749,36 @@ static void intel_didl_outputs(struct drm_i915_private 
> *dev_priv)
>   set_did(opregion, i, 0);
>  }
>  
> -static void intel_setup_cadls(struct drm_i915_private *dev_priv)
> +/* Update CADL to reflect active outputs. */
> +void intel_opregion_update_cadl(struct drm_i915_private *dev_priv)
>  {
>   struct intel_opregion *opregion = _priv->opregion;
> - int i = 0;
> - u32 disp_id;
> -
> - /* Initialize the CADL field by duplicating the DIDL values.
> -  * Technically, this is not always correct as display outputs may exist,
> -  * but not active. This initialization is necessary for some Clevo
> -  * laptops that check this field before processing the brightness and
> -  * display switching hotkeys. Just like DIDL, CADL is NULL-terminated if
> -  * there are less than eight devices. */
> - do {
> - disp_id = get_did(opregion, i);
> - 

Re: [Intel-gfx] [PATCH] drm/i915: tidy up request alloc

2016-07-03 Thread Liu, Hong
On Fri, 2016-07-01 at 19:34 +0100, Chris Wilson wrote:
> On Fri, Jul 01, 2016 at 05:58:18PM +0100, Dave Gordon wrote:
> > On 30/06/16 13:49, Tvrtko Ursulin wrote:
> > > 
> > > On 30/06/16 11:22, Chris Wilson wrote:
> > > > On Thu, Jun 30, 2016 at 09:50:20AM +0100, Tvrtko Ursulin wrote:
> > > > > 
> > > > > On 30/06/16 02:35, Hong Liu wrote:
> > > > > > Return the allocated request pointer directly to remove
> > > > > > the double pointer parameter.
> > > > > > 
> > > > > > Signed-off-by: Hong Liu 
> > > > > > ---
> > > > > >  drivers/gpu/drm/i915/i915_gem.c | 25 +++
> > > > > > --
> > > > > >  1 file changed, 7 insertions(+), 18 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c
> > > > > > b/drivers/gpu/drm/i915/i915_gem.c
> > > > > > index 1d98782..9881455 100644
> > > > > > --- a/drivers/gpu/drm/i915/i915_gem.c
> > > > > > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > > > > > @@ -2988,32 +2988,26 @@ void i915_gem_request_free(struct
> > > > > > kref
> > > > > > *req_ref)
> > > > > >  kmem_cache_free(req->i915->requests, req);
> > > > > >  }
> > > > > > 
> > > > > > -static inline int
> > > > > > +static inline struct drm_i915_gem_request *
> > > > > >  __i915_gem_request_alloc(struct intel_engine_cs *engine,
> > > > > > - struct i915_gem_context *ctx,
> > > > > > - struct drm_i915_gem_request **req_out)
> > > > > > + struct i915_gem_context *ctx)
> > > > > >  {
> > > > > >  struct drm_i915_private *dev_priv = engine->i915;
> > > > > >  unsigned reset_counter =
> > > > > > i915_reset_counter(_priv->gpu_error);
> > > > > >  struct drm_i915_gem_request *req;
> > > > > >  int ret;
> > > > > > 
> > > > > > -if (!req_out)
> > > > > > -return -EINVAL;
> > > > > > -
> > > > > > -*req_out = NULL;
> > > > > > -
> > > > > >  /* ABI: Before userspace accesses the GPU (e.g.
> > > > > > execbuffer),
> > > > > > report
> > > > > >   * EIO if the GPU is already wedged, or EAGAIN to drop
> > > > > > the
> > > > > > struct_mutex
> > > > > >   * and restart.
> > > > > >   */
> > > > > >  ret = i915_gem_check_wedge(reset_counter,
> > > > > > dev_priv->mm.interruptible);
> > > > > >  if (ret)
> > > > > > -return ret;
> > > > > > +return ERR_PTR(ret);
> > > > > > 
> > > > > >  req = kmem_cache_zalloc(dev_priv->requests,
> > > > > > GFP_KERNEL);
> > > > > >  if (req == NULL)
> > > > > > -return -ENOMEM;
> > > > > > +return ERR_PTR(-ENOMEM);
> > > > > > 
> > > > > >  ret = i915_gem_get_seqno(engine->i915, >seqno);
> > > > > >  if (ret)
> > > > > > @@ -3041,14 +3035,13 @@ __i915_gem_request_alloc(struct
> > > > > > intel_engine_cs *engine,
> > > > > >  if (ret)
> > > > > >  goto err_ctx;
> > > > > > 
> > > > > > -*req_out = req;
> > > > > > -return 0;
> > > > > > +return req;
> > > > > > 
> > > > > >  err_ctx:
> > > > > >  i915_gem_context_unreference(ctx);
> > > > > >  err:
> > > > > >  kmem_cache_free(dev_priv->requests, req);
> > > > > > -return ret;
> > > > > > +return ERR_PTR(ret);
> > > > > >  }
> > > > > > 
> > > > > >  /**
> > > > > > @@ -3067,13 +3060,9 @@ struct drm_i915_gem_request *
> > > > > >  i915_gem_request_alloc(struct intel_engine_cs *engine,
> > > > > > struct i915_gem_context *ctx)
> > > > > >  {
> > > > > > -struct drm_i915_gem_request *req;
> > > > > > -int err;
> > > > > > -
> > > > > >  if (ctx == NULL)
> > > > > >  ctx = engine->i915->kernel_context;
> > > > > > -err = __i915_gem_request_alloc(engine, ctx, );
> > > > > > -return err ? ERR_PTR(err) : req;
> > > > > > +return __i915_gem_request_alloc(engine, ctx);
> > > > > >  }
> > > > > > 
> > > > > >  struct drm_i915_gem_request *
> > > > > > 
> > > > > 
> > > > > Looks good to me. And have this feeling I've seen this
> > > > > somewhere before.
> > > > 
> > > > Several times. This is not the full tidy, nor does it realise
> > > > the
> > > > ramifactions of request alloc through the stack.
> > > 
> > > Hm I can't spot that it is doing anything wrong or making
> > > anything
> > > worse. You don't want to let the small cleanup in?
> > > 
> > > Regards,
> > > Tvrtko
> > 
> > It ought to make almost no difference, because the *only* place the
> > inner function is called is from the outer one, which passes a
> > pointer to a local for the returned object; and the inner one is
> > then inlined, so the compiler doesn't actually put it on the stack
> > and call to the inner allocator anyway.
> > 
> > Strangely, however, with this change the code becomes ~400 bytes
> > bigger!
> > 
> > Disassembly reveals that while the code for the externally-callable
> > outer function is indeed almost identical, a second copy of it has
> > also been inlined at the one callsite in this file:
> > 
> > __i915_gem_object_sync() ...
> > req = i915_gem_request_alloc(to, NULL);

[Intel-gfx] [Regression report] Weekly regression report WW27

2016-07-03 Thread Jairo Miramontes

Last week regressions.
+---+---+++
| BugId | Summary   | Created on | 
Bisect |

+---+---+++
| 96736 | kernel 4.6 regression: PSR causes screen to f | 2016-06-29 | 
No |
| 96704 | kernel 4.6 regression: PSR on Haswell causes  | 2016-06-28 | 
No |
| 96675 | [Regression][BISECT]After commit f21a21983ef1 | 2016-06-25 | 
Yes|

+---+---+++


Previous Regressions.
+---+---+++
| BugId | Summary   | Created on | 
Bisect |

+---+---+++
| 90112 | [BSW bisected] OglGSCloth/Lightsmark/CS/ Port | 2015-04-20 | 
Yes|
| 94590 | [KBL/BXT] igt/kms_fbcon_fbt/psr-suspend regre | 2016-03-17 | 
No |
| 93263 | 945GM regression since 4.3| 2015-12-05 | 
No |
| 72782 | [945GM bisected] screen blank on S3 resume on | 2013-12-17 | 
Yes|
| 92414 | [Intel-gfx] As of kernel 4.3-rc1 system will  | 2015-10-10 | 
Yes|
| 92050 | [regression]/bug introduced by commit [0e572f | 2015-09-19 | 
No |
| 93393 | Regression for Skylake modesetting in kernel  | 2015-12-16 | 
No |
| 93802 | [IVB bisected] switching to tty1 causes fifo  | 2016-01-20 | 
Yes|
| 95197 | [i915] regression in 4.6-rc5: GPU HANG: ecode | 2016-04-28 | 
No |
| 94587 | [KBL] igt/kms_plane/plane-panning-bottom-righ | 2016-03-17 | 
No |
| 96591 | [4.6 regression] FBC doesn't notice writes to | 2016-06-19 | 
No |
| 96584 | [regression] [i915] DMAR Errors Spamming Logs | 2016-06-19 | 
No |
| 95736 | [IVB bisected] *ERROR* uncleared fifo underru | 2016-05-24 | 
Yes|
| 89728 | [HSW/BDW/BYT bisected] igt / pm_rps / reset f | 2015-03-23 | 
Yes|
| 89629 | [i965 regression]igt/kms_rotation_crc/sprite- | 2015-03-18 | 
No |
| 92502 | [SKL] [Regression] igt/kms_flip/2x-flip-vs-ex | 2015-10-16 | 
No |
| 84855 | [ILK regression]igt kms_rotation_crc/sprite-r | 2014-10-10 | 
No |
| 87131 | [PNV regression] igt/gem_exec_lut_handle take | 2014-12-09 | 
No |
| 87726 | [BDW Bisected] OglDrvCtx performance reduced  | 2014-12-26 | 
Yes|
| 91974 | [bisected] unrecoverable black screen after k | 2015-09-11 | 
Yes|
| 96645 | [regression 4.7] [BISECT]Low package c-states | 2016-06-22 | 
Yes|
| 94430 | [HSW+nvidia] regression: display becomes "dis | 2016-03-07 | 
No |
| 93971 | video framerate performance regression with U | 2016-02-02 | 
No |
| 89872 | [ HSW Bisected ] VGA was white screen when re | 2015-04-02 | 
Yes|
| 96428 | [IVB bisected] [drm:intel_dp_aux_ch] *ERROR*  | 2016-06-07 | 
Yes|
| 91959 | [865g Linux regression] GPU hang and disabled | 2015-09-10 | 
No |
| 95097 | [hdmi regression ilk] no signal to TV | 2016-04-24 | 
No |
| 89334 | [945 regression] 4.0-rc1 kernel GPU hang:  ec | 2015-02-26 | 
No |
| 94748 | Black screen on Skylake (mouse position relat | 2016-03-29 | 
No |
| 92972 | Black screen on Intel NUC hardware (i915) pos | 2015-11-16 | 
No |
| 87725 | [BDW Bisected] OglBatch7 performance reduced  | 2014-12-26 | 
Yes|
| 94676 | Possible kernel regression for gen3 and earli | 2016-03-23 | 
No |
| 96608 | [KBL] igt / kms_sink_crc_basic [regression]   | 2016-06-20 | 
No |
| 94337 | Linux 4.5 regression: FIFO underruns on Skyla | 2016-02-29 | 
No |
| 90368 | [SNB BSW SKL BXT KBL] bisected igt/kms_3d has | 2015-05-08 | 
Yes|
| 94588 | [KBL/BSW/BXT] igt/gem_reloc_overflow regressi | 2016-03-17 | 
No |
| 90732 | [BDW/BSW Bisected]igt/gem_reloc_vs_gpu/forked | 2015-05-29 | 
Yes|
| 90134 | [BSW Bisected]GFXBench3_gl_driver/GFXBench3_g | 2015-04-22 | 
Yes|
| 93782 | [i9xx TV][BISECT] vblank wait timeout on crtc | 2016-01-19 | 
Yes|
| 89632 | [i965 regression]igt/kms_universal_plane/univ | 2015-03-18 | 
No |
| 88439 | [BDW Bisected]igt/gem_reloc_vs_gpu/forked-fau | 2015-01-15 | 
Yes|
| 92237 | [SNB]Horrible noise (audio) via DisplayPort [ | 2015-10-02 | 
No |
| 93122 | [SNB BAT IGT regression] pm_rpm started skipp | 2015-11-26 | 
No |

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


[Intel-gfx] [PATCH] drm/i915: Hold irq uncore.lock when initialising fw_domains

2016-07-03 Thread Chris Wilson
Acquiring the forcewake domain asserts that it is in an atomic section
(as we always expect to under the uncore.lock). This true expect for
initialising the domains on Ivybridge, and so we generate a warning.
Wrap the manual usage of fw_domains inside the spin_lock.

Signed-off-by: Chris Wilson 
Cc: Tvrtko Ursulin 
Cc: Mika Kuoppala 
---
 drivers/gpu/drm/i915/intel_uncore.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c 
b/drivers/gpu/drm/i915/intel_uncore.c
index 7da3906badf3..1d65209c0998 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1299,9 +1299,11 @@ static void intel_uncore_fw_domains_init(struct 
drm_i915_private *dev_priv)
fw_domain_init(dev_priv, FW_DOMAIN_ID_RENDER,
   FORCEWAKE_MT, FORCEWAKE_MT_ACK);
 
+   spin_lock_irq(_priv->uncore.lock);
fw_domains_get_with_thread_status(dev_priv, FORCEWAKE_ALL);
ecobus = __raw_i915_read32(dev_priv, ECOBUS);
fw_domains_put_with_fifo(dev_priv, FORCEWAKE_ALL);
+   spin_unlock_irq(_priv->uncore.lock);
 
if (!(ecobus & FORCEWAKE_MT_ENABLE)) {
DRM_INFO("No MT forcewake available on Ivybridge, this 
can result in issues\n");
-- 
2.8.1

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


Re: [Intel-gfx] [PATCH 03/14] drm/i915: Add low level set of routines for programming PM IER/IIR/IMR register set

2016-07-03 Thread Goel, Akash



On 7/3/2016 3:08 PM, Chris Wilson wrote:

On Sun, Jul 03, 2016 at 12:21:20AM +0530, akash.g...@intel.com wrote:

From: Akash Goel 

So far PM IER/IIR/IMR registers were being used only for Turbo related
interrupts. But interrupts coming from GuC also use the same set.
As a precursor to supporting GuC interrupts, added new low level routines
so as to allow sharing the programming of PM IER/IIR/IMR registers between
Turbo & GuC.
Also similar to PM IMR, maintaining a bitmask for PM IER register, to allow
easy sharing of it between Turbo & GuC without involving a rmw operation.

v2:
- For appropriateness & avoid any ambiguity, rename old functions
  enable/disable pm_irq to mask/unmask pm_irq and rename new functions
  enable/disable pm_interrupts to enable/disable pm_irq. (Tvrtko)
- Use u32 in place of uint32_t. (Tvrtko)

Suggested-by: Chris Wilson 
Signed-off-by: Akash Goel 
---
 drivers/gpu/drm/i915/i915_drv.h |  1 +
 drivers/gpu/drm/i915/i915_irq.c | 63 -
 drivers/gpu/drm/i915/intel_drv.h|  3 ++
 drivers/gpu/drm/i915/intel_ringbuffer.c |  4 +--
 4 files changed, 53 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9ef4919..85a7103 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1806,6 +1806,7 @@ struct drm_i915_private {
};
u32 gt_irq_mask;
u32 pm_irq_mask;
+   u32 pm_ier_mask;


Oops. u32 pm_imr; and u32 pm_ier;


Fine, will rename.


u32 pm_rps_events;
u32 pipestat_irq_mask[I915_MAX_PIPES];

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 4378a65..dd5ae6d 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -314,7 +314,7 @@ static void snb_update_pm_irq(struct drm_i915_private 
*dev_priv,
}
 }

-void gen6_enable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask)
+void gen6_unmask_pm_irq(struct drm_i915_private *dev_priv, u32 mask)
 {
if (WARN_ON(!intel_irqs_enabled(dev_priv)))
return;
@@ -322,28 +322,62 @@ void gen6_enable_pm_irq(struct drm_i915_private 
*dev_priv, uint32_t mask)
snb_update_pm_irq(dev_priv, mask, mask);
 }

-static void __gen6_disable_pm_irq(struct drm_i915_private *dev_priv,
- uint32_t mask)
+static void __gen6_mask_pm_irq(struct drm_i915_private *dev_priv, u32 mask)
 {
snb_update_pm_irq(dev_priv, mask, 0);
 }

-void gen6_disable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask)
+void gen6_mask_pm_irq(struct drm_i915_private *dev_priv, u32 mask)
 {
if (WARN_ON(!intel_irqs_enabled(dev_priv)))
return;

-   __gen6_disable_pm_irq(dev_priv, mask);
+   __gen6_mask_pm_irq(dev_priv, mask);
 }

-void gen6_reset_rps_interrupts(struct drm_i915_private *dev_priv)
+void gen6_reset_pm_irq(struct drm_i915_private *dev_priv, u32 reset_mask)


reset_pm_iir


Thanks, will update.


 {
i915_reg_t reg = gen6_pm_iir(dev_priv);

-   spin_lock_irq(_priv->irq_lock);
-   I915_WRITE(reg, dev_priv->pm_rps_events);
-   I915_WRITE(reg, dev_priv->pm_rps_events);
+   assert_spin_locked(_priv->irq_lock);
+
+   I915_WRITE(reg, reset_mask);
+   I915_WRITE(reg, reset_mask);
POSTING_READ(reg);
+}
+
+void gen6_enable_pm_irq(struct drm_i915_private *dev_priv, u32 enable_mask)
+{
+   u32 new_val;
+
+   assert_spin_locked(_priv->irq_lock);
+
+   new_val = dev_priv->pm_ier_mask;
+   new_val |= enable_mask;
+
+   dev_priv->pm_ier_mask = new_val;


dev_priv->pm_ier |= new_val;


Sorry, my bad.



+   I915_WRITE(gen6_pm_ier(dev_priv), dev_priv->pm_ier_mask);
+   gen6_unmask_pm_irq(dev_priv, enable_mask);


What barrier do you need between the hw and the caller? I presume there
is a POSTING_READ in this callchain, would be nice to document it.

/* unmask_pm_irq provides a POSTING_READ */


Thanks, will add the comment.
So will assume that POSTING_READ is good enough here.


+}
+
+void gen6_disable_pm_irq(struct drm_i915_private *dev_priv, u32 disable_mask)
+{
+   u32 new_val;
+
+   assert_spin_locked(_priv->irq_lock);
+
+   new_val = dev_priv->pm_ier_mask;
+   new_val &= ~disable_mask;
+
+   dev_priv->pm_ier_mask = new_val;


dev_priv->pm_ier &= ~disable_mask;


+   __gen6_mask_pm_irq(dev_priv, disable_mask);
+   I915_WRITE(gen6_pm_ier(dev_priv), dev_priv->pm_ier_mask);


Do we need a barrier upon disabling? (Usually we need a stronger barrier
on enabling to ensure we don't miss an interrupt when enabling, but for
disabling we don't care.)

So no modification needed here, as you mentioned that we don't need to 
care about the register update getting completed in the disabling case.



+}
+
+void gen6_reset_rps_interrupts(struct drm_i915_private *dev_priv)
+{
+   

Re: [Intel-gfx] [PATCH 05/14] drm/i915: Handle log buffer flush interrupt event from GuC

2016-07-03 Thread Chris Wilson
On Sun, Jul 03, 2016 at 05:51:35PM +0530, Goel, Akash wrote:
> 
> 
> On 7/3/2016 2:45 PM, Chris Wilson wrote:
> >On Sun, Jul 03, 2016 at 12:21:22AM +0530, akash.g...@intel.com wrote:
> >>+static void guc_read_update_log_buffer(struct drm_device *dev, bool 
> >>capture_all)
> >>+{
> >>+   struct drm_i915_private *dev_priv = dev->dev_private;
> >>+   struct intel_guc *guc = _priv->guc;
> >>+   struct guc_log_buffer_state *log_buffer_state;
> >>+   struct guc_log_buffer_state *log_buffer_copy_state;
> >>+   void *src_ptr, *dst_ptr;
> >>+   u32 num_pages_to_copy;
> >>+   int i;
> >>+
> >>+   if (!guc->log.obj)
> >>+   return;
> >>+
> >>+   num_pages_to_copy = guc->log.obj->base.size / PAGE_SIZE;
> >>+   /* Don't really need to copy crash buffer area in regular cases as there
> >>+* won't be any unread data there.
> >>+*/
> >>+   if (!capture_all)
> >>+   num_pages_to_copy -= (GUC_LOG_CRASH_PAGES + 1);
> >>+
> >>+   log_buffer_state = src_ptr =
> >>+   kmap_atomic(i915_gem_object_get_page(guc->log.obj, 0));
> >
> >So why not use i915_gem_object_pin_map() from the start?
> >
> >That will cut down on the churn later.
> 
> Fine, will reorder the series and squash the other patch 'drm/i915:
> Use uncached(WC) mapping for accessing the GuC log buffer' with this
> patch.

I would keep the pin_map(false -> true) as a separate step so that it is
clearly documented (and reversible).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 05/14] drm/i915: Handle log buffer flush interrupt event from GuC

2016-07-03 Thread Chris Wilson
On Sun, Jul 03, 2016 at 05:55:14PM +0530, Goel, Akash wrote:
> 
> 
> On 7/3/2016 5:51 PM, Goel, Akash wrote:
> >
> >
> >On 7/3/2016 2:45 PM, Chris Wilson wrote:
> >>On Sun, Jul 03, 2016 at 12:21:22AM +0530, akash.g...@intel.com wrote:
> >>>+static void guc_read_update_log_buffer(struct drm_device *dev, bool
> >>>capture_all)
> >>>+{
> >>>+struct drm_i915_private *dev_priv = dev->dev_private;
> >>>+struct intel_guc *guc = _priv->guc;
> >>>+struct guc_log_buffer_state *log_buffer_state;
> >>>+struct guc_log_buffer_state *log_buffer_copy_state;
> >>>+void *src_ptr, *dst_ptr;
> >>>+u32 num_pages_to_copy;
> >>>+int i;
> >>>+
> >>>+if (!guc->log.obj)
> >>>+return;
> >>>+
> >>>+num_pages_to_copy = guc->log.obj->base.size / PAGE_SIZE;
> >>>+/* Don't really need to copy crash buffer area in regular cases
> >>>as there
> >>>+ * won't be any unread data there.
> >>>+ */
> >>>+if (!capture_all)
> >>>+num_pages_to_copy -= (GUC_LOG_CRASH_PAGES + 1);
> >>>+
> >>>+log_buffer_state = src_ptr =
> >>>+kmap_atomic(i915_gem_object_get_page(guc->log.obj, 0));
> >>
> >>So why not use i915_gem_object_pin_map() from the start?
> >>
> >>That will cut down on the churn later.
> >
> >Fine, will reorder the series and squash the other patch 'drm/i915: Use
> >uncached(WC) mapping for accessing the GuC log buffer' with this patch.
> >
> Sorry got confused, will use the i915_gem_object_pin_map() here
> instead of kmap and keep the WC mapping patch at the end of series
> only. Then will just have to modify the call to
> i915_gem_object_pin_map() to pass the WC flag.

Yup.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 13/14] drm/i915: Add stats for GuC log buffer flush interrupts

2016-07-03 Thread Goel, Akash



On 7/3/2016 3:14 PM, Chris Wilson wrote:

On Sun, Jul 03, 2016 at 12:21:30AM +0530, akash.g...@intel.com wrote:

From: Akash Goel 

GuC firmware sends an interrupt to flush the log buffer when it
becomes half full. GuC firmware also tracks how many times the
buffer overflowed.
It would be useful to maintain a statistics of how many flush


For what purpose? Would not tracepoints be even more useful?
Having a stats would be useful to get an idea of the volume & the rate 
at which logs are being generated from GuC side and whether Driver is 
quick enough to capture all of them.


Yes tracepoint would also be very useful.

Please see below the logging related stats, in the output of
‘i915_guc_info’ on execution of ‘gem_exec_nop’ IGT.

GuC total action count: 623531
GuC action failure count: 0
GuC last action command: 0x30
GuC last action status: 0xf000
GuC last action error code: 0

GuC submissions:
render ring :9019910, last seqno 0x01a4390b
blitter ring:6188291, last seqno 0x01a4390d
bsd ring:6179075, last seqno 0x01a4390c
video enhancement ring  :6156547, last seqno 0x01a4390e
Total: 27543823

GuC execbuf client @ 8801659fb100:
Priority 2, GuC ctx index: 0, PD offset 0x800
Doorbell id 0, offset: 0x0, cookie 0x1a4490f
WQ size 8192, offset: 0x1000, tail 4336
Work queue full: 0
Failed to queue: 0
Failed doorbell: 0
Last submission result: 0
Submissions: 9019910 render ring
Submissions: 6188291 blitter ring
Submissions: 6179075 bsd ring
Submissions: 6156547 video enhancement ring
Total: 27543823

GuC logging stats:
ISR: flush count 321718, overflow count0
DPC: flush count 303788, overflow count1
CRASH:   flush count  0, overflow count0
Total flush interrupt count: 625511


Best regards
Akash

-Chris


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


Re: [Intel-gfx] [PATCH 05/14] drm/i915: Handle log buffer flush interrupt event from GuC

2016-07-03 Thread Goel, Akash



On 7/3/2016 5:51 PM, Goel, Akash wrote:



On 7/3/2016 2:45 PM, Chris Wilson wrote:

On Sun, Jul 03, 2016 at 12:21:22AM +0530, akash.g...@intel.com wrote:

+static void guc_read_update_log_buffer(struct drm_device *dev, bool
capture_all)
+{
+struct drm_i915_private *dev_priv = dev->dev_private;
+struct intel_guc *guc = _priv->guc;
+struct guc_log_buffer_state *log_buffer_state;
+struct guc_log_buffer_state *log_buffer_copy_state;
+void *src_ptr, *dst_ptr;
+u32 num_pages_to_copy;
+int i;
+
+if (!guc->log.obj)
+return;
+
+num_pages_to_copy = guc->log.obj->base.size / PAGE_SIZE;
+/* Don't really need to copy crash buffer area in regular cases
as there
+ * won't be any unread data there.
+ */
+if (!capture_all)
+num_pages_to_copy -= (GUC_LOG_CRASH_PAGES + 1);
+
+log_buffer_state = src_ptr =
+kmap_atomic(i915_gem_object_get_page(guc->log.obj, 0));


So why not use i915_gem_object_pin_map() from the start?

That will cut down on the churn later.


Fine, will reorder the series and squash the other patch 'drm/i915: Use
uncached(WC) mapping for accessing the GuC log buffer' with this patch.

Sorry got confused, will use the i915_gem_object_pin_map() here instead 
of kmap and keep the WC mapping patch at the end of series only. Then 
will just have to modify the call to i915_gem_object_pin_map() to pass 
the WC flag.




Best regards
Akash

-Chris


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


Re: [Intel-gfx] [PATCH 05/14] drm/i915: Handle log buffer flush interrupt event from GuC

2016-07-03 Thread Goel, Akash



On 7/3/2016 2:45 PM, Chris Wilson wrote:

On Sun, Jul 03, 2016 at 12:21:22AM +0530, akash.g...@intel.com wrote:

+static void guc_read_update_log_buffer(struct drm_device *dev, bool 
capture_all)
+{
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   struct intel_guc *guc = _priv->guc;
+   struct guc_log_buffer_state *log_buffer_state;
+   struct guc_log_buffer_state *log_buffer_copy_state;
+   void *src_ptr, *dst_ptr;
+   u32 num_pages_to_copy;
+   int i;
+
+   if (!guc->log.obj)
+   return;
+
+   num_pages_to_copy = guc->log.obj->base.size / PAGE_SIZE;
+   /* Don't really need to copy crash buffer area in regular cases as there
+* won't be any unread data there.
+*/
+   if (!capture_all)
+   num_pages_to_copy -= (GUC_LOG_CRASH_PAGES + 1);
+
+   log_buffer_state = src_ptr =
+   kmap_atomic(i915_gem_object_get_page(guc->log.obj, 0));


So why not use i915_gem_object_pin_map() from the start?

That will cut down on the churn later.


Fine, will reorder the series and squash the other patch 'drm/i915: Use 
uncached(WC) mapping for accessing the GuC log buffer' with this patch.


Best regards
Akash

-Chris


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


Re: [Intel-gfx] [PATCH 13/14] drm/i915: Add stats for GuC log buffer flush interrupts

2016-07-03 Thread Chris Wilson
On Sun, Jul 03, 2016 at 12:21:30AM +0530, akash.g...@intel.com wrote:
> From: Akash Goel 
> 
> GuC firmware sends an interrupt to flush the log buffer when it
> becomes half full. GuC firmware also tracks how many times the
> buffer overflowed.
> It would be useful to maintain a statistics of how many flush

For what purpose? Would not tracepoints be even more useful?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 03/14] drm/i915: Add low level set of routines for programming PM IER/IIR/IMR register set

2016-07-03 Thread Chris Wilson
On Sun, Jul 03, 2016 at 12:21:20AM +0530, akash.g...@intel.com wrote:
> From: Akash Goel 
> 
> So far PM IER/IIR/IMR registers were being used only for Turbo related
> interrupts. But interrupts coming from GuC also use the same set.
> As a precursor to supporting GuC interrupts, added new low level routines
> so as to allow sharing the programming of PM IER/IIR/IMR registers between
> Turbo & GuC.
> Also similar to PM IMR, maintaining a bitmask for PM IER register, to allow
> easy sharing of it between Turbo & GuC without involving a rmw operation.
> 
> v2:
> - For appropriateness & avoid any ambiguity, rename old functions
>   enable/disable pm_irq to mask/unmask pm_irq and rename new functions
>   enable/disable pm_interrupts to enable/disable pm_irq. (Tvrtko)
> - Use u32 in place of uint32_t. (Tvrtko)
> 
> Suggested-by: Chris Wilson 
> Signed-off-by: Akash Goel 
> ---
>  drivers/gpu/drm/i915/i915_drv.h |  1 +
>  drivers/gpu/drm/i915/i915_irq.c | 63 
> -
>  drivers/gpu/drm/i915/intel_drv.h|  3 ++
>  drivers/gpu/drm/i915/intel_ringbuffer.c |  4 +--
>  4 files changed, 53 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 9ef4919..85a7103 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1806,6 +1806,7 @@ struct drm_i915_private {
>   };
>   u32 gt_irq_mask;
>   u32 pm_irq_mask;
> + u32 pm_ier_mask;

Oops. u32 pm_imr; and u32 pm_ier;

>   u32 pm_rps_events;
>   u32 pipestat_irq_mask[I915_MAX_PIPES];
>  
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 4378a65..dd5ae6d 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -314,7 +314,7 @@ static void snb_update_pm_irq(struct drm_i915_private 
> *dev_priv,
>   }
>  }
>  
> -void gen6_enable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask)
> +void gen6_unmask_pm_irq(struct drm_i915_private *dev_priv, u32 mask)
>  {
>   if (WARN_ON(!intel_irqs_enabled(dev_priv)))
>   return;
> @@ -322,28 +322,62 @@ void gen6_enable_pm_irq(struct drm_i915_private 
> *dev_priv, uint32_t mask)
>   snb_update_pm_irq(dev_priv, mask, mask);
>  }
>  
> -static void __gen6_disable_pm_irq(struct drm_i915_private *dev_priv,
> -   uint32_t mask)
> +static void __gen6_mask_pm_irq(struct drm_i915_private *dev_priv, u32 mask)
>  {
>   snb_update_pm_irq(dev_priv, mask, 0);
>  }
>  
> -void gen6_disable_pm_irq(struct drm_i915_private *dev_priv, uint32_t mask)
> +void gen6_mask_pm_irq(struct drm_i915_private *dev_priv, u32 mask)
>  {
>   if (WARN_ON(!intel_irqs_enabled(dev_priv)))
>   return;
>  
> - __gen6_disable_pm_irq(dev_priv, mask);
> + __gen6_mask_pm_irq(dev_priv, mask);
>  }
>  
> -void gen6_reset_rps_interrupts(struct drm_i915_private *dev_priv)
> +void gen6_reset_pm_irq(struct drm_i915_private *dev_priv, u32 reset_mask)

reset_pm_iir

>  {
>   i915_reg_t reg = gen6_pm_iir(dev_priv);
>  
> - spin_lock_irq(_priv->irq_lock);
> - I915_WRITE(reg, dev_priv->pm_rps_events);
> - I915_WRITE(reg, dev_priv->pm_rps_events);
> + assert_spin_locked(_priv->irq_lock);
> +
> + I915_WRITE(reg, reset_mask);
> + I915_WRITE(reg, reset_mask);
>   POSTING_READ(reg);
> +}
> +
> +void gen6_enable_pm_irq(struct drm_i915_private *dev_priv, u32 enable_mask)
> +{
> + u32 new_val;
> +
> + assert_spin_locked(_priv->irq_lock);
> +
> + new_val = dev_priv->pm_ier_mask;
> + new_val |= enable_mask;
> +
> + dev_priv->pm_ier_mask = new_val;

dev_priv->pm_ier |= new_val;

> + I915_WRITE(gen6_pm_ier(dev_priv), dev_priv->pm_ier_mask);
> + gen6_unmask_pm_irq(dev_priv, enable_mask);

What barrier do you need between the hw and the caller? I presume there
is a POSTING_READ in this callchain, would be nice to document it.

/* unmask_pm_irq provides a POSTING_READ */

> +}
> +
> +void gen6_disable_pm_irq(struct drm_i915_private *dev_priv, u32 disable_mask)
> +{
> + u32 new_val;
> +
> + assert_spin_locked(_priv->irq_lock);
> +
> + new_val = dev_priv->pm_ier_mask;
> + new_val &= ~disable_mask;
> +
> + dev_priv->pm_ier_mask = new_val;

dev_priv->pm_ier &= ~disable_mask;

> + __gen6_mask_pm_irq(dev_priv, disable_mask);
> + I915_WRITE(gen6_pm_ier(dev_priv), dev_priv->pm_ier_mask);

Do we need a barrier upon disabling? (Usually we need a stronger barrier
on enabling to ensure we don't miss an interrupt when enabling, but for
disabling we don't care.)

> +}
> +
> +void gen6_reset_rps_interrupts(struct drm_i915_private *dev_priv)
> +{
> + spin_lock_irq(_priv->irq_lock);
> + gen6_reset_pm_irq(dev_priv, dev_priv->pm_rps_events);
>   dev_priv->rps.pm_iir = 0;

Hmm. That's slightly confusing, but passable since it is 

Re: [Intel-gfx] [PATCH 04/14] drm/i915: Support for GuC interrupts

2016-07-03 Thread Chris Wilson
On Sun, Jul 03, 2016 at 12:21:21AM +0530, akash.g...@intel.com wrote:
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 85a7103..20c701c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1808,6 +1808,7 @@ struct drm_i915_private {
>   u32 pm_irq_mask;
>   u32 pm_ier_mask;
>   u32 pm_rps_events;
> + u32 guc_events;

pm_guc_events so we have some clue as to which register/control block it
is associated within the massive drm_i915_private.

>   u32 pipestat_irq_mask[I915_MAX_PIPES];
>  
>   struct i915_hotplug hotplug;
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c 
> b/drivers/gpu/drm/i915/i915_guc_submission.c
> index fedf82b..b441951 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -1038,6 +1038,8 @@ int intel_guc_suspend(struct drm_device *dev)
>   if (guc->guc_fw.guc_fw_load_status != GUC_FIRMWARE_SUCCESS)
>   return 0;
>  
> + gen9_disable_guc_interrupts(dev_priv);

Also upon sanitize?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 05/14] drm/i915: Handle log buffer flush interrupt event from GuC

2016-07-03 Thread Chris Wilson
On Sun, Jul 03, 2016 at 12:21:22AM +0530, akash.g...@intel.com wrote:
> +static void guc_read_update_log_buffer(struct drm_device *dev, bool 
> capture_all)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_guc *guc = _priv->guc;
> + struct guc_log_buffer_state *log_buffer_state;
> + struct guc_log_buffer_state *log_buffer_copy_state;
> + void *src_ptr, *dst_ptr;
> + u32 num_pages_to_copy;
> + int i;
> +
> + if (!guc->log.obj)
> + return;
> +
> + num_pages_to_copy = guc->log.obj->base.size / PAGE_SIZE;
> + /* Don't really need to copy crash buffer area in regular cases as there
> +  * won't be any unread data there.
> +  */
> + if (!capture_all)
> + num_pages_to_copy -= (GUC_LOG_CRASH_PAGES + 1);
> +
> + log_buffer_state = src_ptr =
> + kmap_atomic(i915_gem_object_get_page(guc->log.obj, 0));

So why not use i915_gem_object_pin_map() from the start?

That will cut down on the churn later.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx