[Intel-gfx] ✓ Ro.CI.BAT: success for drm/i915: Hold irq uncore.lock when initialising fw_domains
== 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
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 = &dev_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);
> - opregion->acpi->cadl[i] = disp_id;
> - } while (++i < 8 && disp_id != 0);
> +
Re: [Intel-gfx] [PATCH] drm/i915: tidy up request alloc
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(&dev_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, &req->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, &req);
> > > > > > -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
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
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(&dev_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(&dev_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
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(&dev_priv->irq_lock);
- I915_WRITE(reg, dev_priv->pm_rps_events);
- I915_WRITE(reg, dev_priv->pm_rps_events);
+ assert_spin_locked(&dev_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(&dev_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(&dev_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)
+{
+ spin_lock_irq(&dev_priv->irq_lock);
+ gen6_reset_pm_i
Re: [Intel-gfx] [PATCH 05/14] drm/i915: Handle log buffer flush interrupt event from GuC
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 = &dev_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
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 = &dev_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
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
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 = &dev_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
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 = &dev_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
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
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(&dev_priv->irq_lock);
> - I915_WRITE(reg, dev_priv->pm_rps_events);
> - I915_WRITE(reg, dev_priv->pm_rps_events);
> + assert_spin_locked(&dev_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(&dev_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(&dev_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(&dev_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 really the set
of bits in pm_iir for rps.
>
Re: [Intel-gfx] [PATCH 04/14] drm/i915: Support for GuC interrupts
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
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 = &dev_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
