Re: [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915/guc: Release GuC interrupts in i915_guc_submission_disable

[Kamble, Sagar A]

On 3/12/2017 6:29 PM, Chris Wilson wrote:

On Sat, Mar 11, 2017 at 04:17:34AM -, Patchwork wrote:

== Series Details ==

Series: series starting with [1/3] drm/i915/guc: Release GuC interrupts in 
i915_guc_submission_disable
URL   : https://patchwork.freedesktop.org/series/21090/
State : success

== Summary ==

Series 21090v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/21090/revisions/1/mbox/

Test kms_pipe_crc_basic:
 Subgroup hang-read-crc-pipe-b:
 dmesg-warn -> PASS   (fi-byt-j1900)

Applied, thanks for the quick fix. It is looking much neater now as well
:)
-Chris


Thanks Chris.
I feel unmasking of ARAT_EXPIRED is hard coding the behavior with GuC.
Ideally rps enabling should happen post GuC load in reset path like in load 
time flow.
That way instead of hard coding interrupts to be kept as ARAT_EXPIRED we will 
be able to derive from bits unmasked by GuC in PMINTRMSK.
So before GuC load, we should be resetting RPS interrupts (making PMINRMSK=~0u) 
and then derive interrupts to be kept unmasked by Host.
And then enable RPS. Current state is fine as we know GuC isn't using other PM 
interrupts. (might use some of those in SLPC)





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

Re: [Intel-gfx] [PATCH] drm/i915: Rename REDIRECT_TO_GUC bit

[Kamble, Sagar A]

LGTM.
Reviewed-by: Sagar Arun Kamble

PS: Might need updating comments in the guc_interrupts_capture to align with 
new name and semantics of this bit
w.r.t pm_intrmsk_mbz.

On 3/12/2017 6:57 PM, Chris Wilson wrote:

The REDIRECT_TO_GUC bit is a strange beast as it is a disable bit -
setting the bit in the pm interrupt generation stops the interrupt going
to the guc (not sending it to the guc as the name implies). To help the
reader rename it to DISABLE_REDIRECT_TO_GUC so that we keep the bspec
greppable name without it being as confusing!

Signed-off-by: Chris Wilson 
Cc: Tvrtko Ursulin 
Cc: Oscar Mateo 
Cc: Radoslaw Szwichtenberg 
Cc: Sagar Arun Kamble 
---
  drivers/gpu/drm/i915/i915_guc_submission.c | 4 ++--
  drivers/gpu/drm/i915/i915_irq.c| 2 +-
  drivers/gpu/drm/i915/i915_reg.h| 2 +-
  3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c 
b/drivers/gpu/drm/i915/i915_guc_submission.c
index ca7723fd0f79..84fd49d5680e 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -975,7 +975,7 @@ static void guc_interrupts_capture(struct drm_i915_private 
*dev_priv)
 * result in the register bit being left SET!
 */
dev_priv->rps.pm_intrmsk_mbz |= ARAT_EXPIRED_INTRMSK;
-   dev_priv->rps.pm_intrmsk_mbz &= ~GEN8_PMINTR_REDIRECT_TO_GUC;
+   dev_priv->rps.pm_intrmsk_mbz &= ~GEN8_PMINTR_DISABLE_REDIRECT_TO_GUC;
  }
  
  int i915_guc_submission_enable(struct drm_i915_private *dev_priv)

@@ -1037,7 +1037,7 @@ static void guc_interrupts_release(struct 
drm_i915_private *dev_priv)
I915_WRITE(GUC_VCS2_VCS1_IER, 0);
I915_WRITE(GUC_WD_VECS_IER, 0);
  
-	dev_priv->rps.pm_intrmsk_mbz |= GEN8_PMINTR_REDIRECT_TO_GUC;

+   dev_priv->rps.pm_intrmsk_mbz |= GEN8_PMINTR_DISABLE_REDIRECT_TO_GUC;
dev_priv->rps.pm_intrmsk_mbz &= ~ARAT_EXPIRED_INTRMSK;
  
  }

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index a522da712cc8..89ccf3e1fda5 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -4282,7 +4282,7 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
dev_priv->rps.pm_intrmsk_mbz |= GEN6_PM_RP_UP_EI_EXPIRED;
  
  	if (INTEL_INFO(dev_priv)->gen >= 8)

-   dev_priv->rps.pm_intrmsk_mbz |= GEN8_PMINTR_REDIRECT_TO_GUC;
+   dev_priv->rps.pm_intrmsk_mbz |= 
GEN8_PMINTR_DISABLE_REDIRECT_TO_GUC;
  
  	if (IS_GEN2(dev_priv)) {

/* Gen2 doesn't have a hardware frame counter */
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 19d42e8813c4..5d88c35c41cd 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7453,7 +7453,7 @@ enum {
  #define VLV_RCEDATA   _MMIO(0xA0BC)
  #define GEN6_RC6pp_THRESHOLD  _MMIO(0xA0C0)
  #define GEN6_PMINTRMSK_MMIO(0xA168)
-#define   GEN8_PMINTR_REDIRECT_TO_GUC  (1<<31)
+#define   GEN8_PMINTR_DISABLE_REDIRECT_TO_GUC  (1<<31)
  #define   ARAT_EXPIRED_INTRMSK(1<<9)
  #define GEN8_MISC_CTRL0   _MMIO(0xA180)
  #define VLV_PWRDWNUPCTL   _MMIO(0xA294)


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

[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: make context status notifier head be per engine (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915: make context status notifier head be per engine (rev3)
URL   : https://patchwork.freedesktop.org/series/20552/
State : failure

== Summary ==

Series 20552v3 drm/i915: make context status notifier head be per engine
https://patchwork.freedesktop.org/api/1.0/series/20552/revisions/3/mbox/

Test drv_module_reload:
Subgroup basic-reload-inject:
pass   -> INCOMPLETE (fi-hsw-4770r)
Test gem_exec_flush:
Subgroup basic-batch-kernel-default-uc:
pass   -> FAIL   (fi-snb-2600) fdo#17
Test kms_force_connector_basic:
Subgroup prune-stale-modes:
skip   -> PASS   (fi-snb-2520m)

fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17

fi-bdw-5557u total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  
time: 454s
fi-bsw-n3050 total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39  
time: 624s
fi-bxt-j4205 total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  
time: 535s
fi-bxt-t5700 total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  
time: 579s
fi-byt-j1900 total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27  
time: 505s
fi-byt-n2820 total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31  
time: 504s
fi-hsw-4770  total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  
time: 441s
fi-hsw-4770r total:276  pass:260  dwarn:0   dfail:0   fail:0   skip:15  
time: 0s
fi-ilk-650   total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  
time: 445s
fi-ivb-3520m total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  
time: 495s
fi-ivb-3770  total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  
time: 491s
fi-kbl-7500u total:278  pass:259  dwarn:1   dfail:0   fail:0   skip:18  
time: 474s
fi-skl-6260u total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  
time: 502s
fi-skl-6700hqtotal:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  
time: 585s
fi-skl-6700k total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  
time: 496s
fi-skl-6770hqtotal:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  
time: 543s
fi-snb-2520m total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  
time: 567s
fi-snb-2600  total:278  pass:248  dwarn:0   dfail:0   fail:1   skip:29  
time: 419s

2cb12884655eab41d4992b33ccb36c609c4537d3 drm-tip: 2017y-03m-12d-13h-00m-05s UTC 
integration manifest
2bd4f8a drm/i915: make context status notifier head be per engine

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4147/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2] drm/i915: make context status notifier head be per engine

[Zhenyu Wang]

On 2017.03.13 10:47:11 +0800, changbin...@intel.com wrote:
> From: Changbin Du 
> 
> GVTg has introduced the context status notifier to schedule the GVTg
> workload. At that time, the notifier is bound to GVTg context only,
> so GVTg is not aware of host workloads.
> 
> Now we are going to improve GVTg's guest workload scheduler policy,
> and add Guc emulation support for new Gen graphics. Both these two
> features require acknowledgment for all contexts running on hardware.
> (But will not alter host workload.) So here try to make some change.
> 
> The change is simple:
>   1. Move the context status notifier head from i915_gem_context to
>  intel_engine_cs. Which means there is a notifier head per engine
>  instead of per context. Execlist driver still call notifier for
>  each context sched-in/out events of current engine.
>   2. At GVTg side, it binds a notifier_block for each physical engine
>  at GVTg initialization period. Then GVTg can hear all context
>  status events.
> 
> In this patch, GVTg do nothing for host context event, but later
> will add a function there. But in any case, the notifier callback is
> a noop if this is no active vGPU.
> 
> Since intel_gvt_init() is called at early initialization stage and
> require the status notifier head has been initiated, I initiate it in
> intel_engine_setup().
> 
> v2: remove a redundant newline. (chris)
> 
> Signed-off-by: Changbin Du 
> Reviewed-by: Chris Wilson 

Applied to gvt-next, thanks!

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


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

[Intel-gfx] [PATCH v2] drm/i915: make context status notifier head be per engine

[changbin . du]

From: Changbin Du 

GVTg has introduced the context status notifier to schedule the GVTg
workload. At that time, the notifier is bound to GVTg context only,
so GVTg is not aware of host workloads.

Now we are going to improve GVTg's guest workload scheduler policy,
and add Guc emulation support for new Gen graphics. Both these two
features require acknowledgment for all contexts running on hardware.
(But will not alter host workload.) So here try to make some change.

The change is simple:
  1. Move the context status notifier head from i915_gem_context to
 intel_engine_cs. Which means there is a notifier head per engine
 instead of per context. Execlist driver still call notifier for
 each context sched-in/out events of current engine.
  2. At GVTg side, it binds a notifier_block for each physical engine
 at GVTg initialization period. Then GVTg can hear all context
 status events.

In this patch, GVTg do nothing for host context event, but later
will add a function there. But in any case, the notifier callback is
a noop if this is no active vGPU.

Since intel_gvt_init() is called at early initialization stage and
require the status notifier head has been initiated, I initiate it in
intel_engine_setup().

v2: remove a redundant newline. (chris)

Signed-off-by: Changbin Du 
Reviewed-by: Chris Wilson 
---
 drivers/gpu/drm/i915/gvt/gvt.h  |  2 +-
 drivers/gpu/drm/i915/gvt/scheduler.c| 45 ++---
 drivers/gpu/drm/i915/i915_gem_context.c |  1 -
 drivers/gpu/drm/i915/i915_gem_context.h |  3 ---
 drivers/gpu/drm/i915/intel_engine_cs.c  |  2 ++
 drivers/gpu/drm/i915/intel_lrc.c|  3 ++-
 drivers/gpu/drm/i915/intel_ringbuffer.h |  3 +++
 7 files changed, 27 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
index 2379192..6dfc48b 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.h
+++ b/drivers/gpu/drm/i915/gvt/gvt.h
@@ -162,7 +162,6 @@ struct intel_vgpu {
atomic_t running_workload_num;
DECLARE_BITMAP(tlb_handle_pending, I915_NUM_ENGINES);
struct i915_gem_context *shadow_ctx;
-   struct notifier_block shadow_ctx_notifier_block;
 
 #if IS_ENABLED(CONFIG_DRM_I915_GVT_KVMGT)
struct {
@@ -233,6 +232,7 @@ struct intel_gvt {
struct intel_gvt_gtt gtt;
struct intel_gvt_opregion opregion;
struct intel_gvt_workload_scheduler scheduler;
+   struct notifier_block shadow_ctx_notifier_block[I915_NUM_ENGINES];
DECLARE_HASHTABLE(cmd_table, GVT_CMD_HASH_BITS);
struct intel_vgpu_type *types;
unsigned int num_types;
diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c 
b/drivers/gpu/drm/i915/gvt/scheduler.c
index d3a56c9..48e5088 100644
--- a/drivers/gpu/drm/i915/gvt/scheduler.c
+++ b/drivers/gpu/drm/i915/gvt/scheduler.c
@@ -130,12 +130,10 @@ static int populate_shadow_context(struct 
intel_vgpu_workload *workload)
 static int shadow_context_status_change(struct notifier_block *nb,
unsigned long action, void *data)
 {
-   struct intel_vgpu *vgpu = container_of(nb,
-   struct intel_vgpu, shadow_ctx_notifier_block);
-   struct drm_i915_gem_request *req =
-   (struct drm_i915_gem_request *)data;
-   struct intel_gvt_workload_scheduler *scheduler =
-   >gvt->scheduler;
+   struct drm_i915_gem_request *req = (struct drm_i915_gem_request *)data;
+   struct intel_gvt *gvt = container_of(nb, struct intel_gvt,
+   shadow_ctx_notifier_block[req->engine->id]);
+   struct intel_gvt_workload_scheduler *scheduler = >scheduler;
struct intel_vgpu_workload *workload =
scheduler->current_workload[req->engine->id];
 
@@ -513,15 +511,16 @@ void intel_gvt_wait_vgpu_idle(struct intel_vgpu *vgpu)
 void intel_gvt_clean_workload_scheduler(struct intel_gvt *gvt)
 {
struct intel_gvt_workload_scheduler *scheduler = >scheduler;
-   int i;
+   struct intel_engine_cs *engine;
+   enum intel_engine_id i;
 
gvt_dbg_core("clean workload scheduler\n");
 
-   for (i = 0; i < I915_NUM_ENGINES; i++) {
-   if (scheduler->thread[i]) {
-   kthread_stop(scheduler->thread[i]);
-   scheduler->thread[i] = NULL;
-   }
+   for_each_engine(engine, gvt->dev_priv, i) {
+   atomic_notifier_chain_unregister(
+   >context_status_notifier,
+   >shadow_ctx_notifier_block[i]);
+   kthread_stop(scheduler->thread[i]);
}
 }
 
@@ -529,18 +528,15 @@ int intel_gvt_init_workload_scheduler(struct intel_gvt 
*gvt)
 {
struct intel_gvt_workload_scheduler *scheduler = >scheduler;
struct workload_thread_param *param = NULL;
+   struct intel_engine_cs *engine;
+   enum 

Re: [Intel-gfx] [PATCH 2/2] drm/i915/scheduler: emulate a scheduler for guc

[Dong, Chuanxiao]

> -Original Message-
> From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf
> Of Tvrtko Ursulin
> Sent: Wednesday, February 15, 2017 8:34 PM
> To: Chris Wilson ; intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 2/2] drm/i915/scheduler: emulate a scheduler
> for guc
> 
> 
> On 15/02/2017 12:20, Chris Wilson wrote:
> > On Wed, Feb 15, 2017 at 11:56:28AM +, Tvrtko Ursulin wrote:
> >>
> >> On 14/02/2017 11:44, Chris Wilson wrote:
> >>> diff --git a/drivers/gpu/drm/i915/i915_irq.c
> >>> b/drivers/gpu/drm/i915/i915_irq.c index cdc7da60d37a..aa886b5fb2cd
> >>> 100644
> >>> --- a/drivers/gpu/drm/i915/i915_irq.c
> >>> +++ b/drivers/gpu/drm/i915/i915_irq.c
> >>> @@ -1350,13 +1350,20 @@ static void snb_gt_irq_handler(struct
> >>> drm_i915_private *dev_priv, static __always_inline void
> >>> gen8_cs_irq_handler(struct intel_engine_cs *engine, u32 iir, int
> >>> test_shift) {
> >>> - if (iir & (GT_RENDER_USER_INTERRUPT << test_shift))
> >>> - notify_ring(engine);
> >>> + bool tasklet = false;
> >>>
> >>>   if (iir & (GT_CONTEXT_SWITCH_INTERRUPT << test_shift)) {
> >>>   set_bit(ENGINE_IRQ_EXECLIST, >irq_posted);
> >>> - tasklet_hi_schedule(>irq_tasklet);
> >>> + tasklet = true;
> >>>   }
> >>> +
> >>> + if (iir & (GT_RENDER_USER_INTERRUPT << test_shift)) {
> >>> + notify_ring(engine);
> >>> + tasklet |= i915.enable_guc_submission;
> >>
> >> Hm.. noticed that BXT was quite unhappy in CI?
> >>
> >> I wonder if we need to set ENGINE_IRQ_EXECLISTS here (and clear it in
> >> the irq tasklet as well) to be fully identical between the two
> >> backends.
> >>
> >> Not saying that was the reason for the CI unhappiness, just an
> >> observation at this stage.
> >
> > I don't think we want to. Note that the bit will always be unset, so
> > that shouldn't be causing the intel_execlists_idle() timeout. I was
> 
> Yeah agreed, but when you added the bit test in intel_execlists_idle, the
> commit message said it is a sanity check before suspend. So presumably it
> would make sense to have the same for the GuC tasklet.
> 
> > fearing a missed interrupt as we only idle once the request list is
> > empty, but that means we didn't process the interrupt. Or something
> > scary like that. (I do recall seeing missed interrupts on my skl with
> > guc btw, don't know if that is still a feature.)
> 
> No idea and no BXT to test on. But it is something new compared to the
> previous CI run. So something changed in the tree in the meantime to cause
> it.

Seems recently community has some bug fixing for the GuC CI testing. And this 
patch already cannot directly be applied on the latest intel-gfx tree. Is there 
any plan to re-submit this patch? I am asking because the current i915 host GuC 
scheduling doesn't provide any chance for GVT to do the notification like using 
execlist submit. But with this implementation, GuC can provide the right point 
to do the notification for GVT as well.

Thanks
Chuanxiao

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

Re: [Intel-gfx] [PATCH] drm/i915: make context status notifier head be per engine

[Du, Changbin]

hi, chris,

On Fri, Mar 10, 2017 at 05:17:17PM +, Chris Wilson wrote:
> On Thu, Mar 09, 2017 at 07:27:24PM +0800, changbin...@intel.com wrote:
> > From: Changbin Du 
> > 
> > GVTg has introduced the context status notifier to schedule the GVTg
> > workload. At that time, the notifier is bound to GVTg context only,
> > so GVTg is not aware of host workloads.
> > 
> > Now we are going to improve GVTg's guest workload scheduler policy,
> > and add Guc emulation support for new Gen graphics. Both these two
> > features require acknowledgment for all contexts running on hardware.
> > (But will not alter host workload.) So here try to make some change.
> > 
> > The change is simple:
> >   1. Move the context status notifier head from i915_gem_context to
> >  intel_engine_cs. Which means there is a notifier head per engine
> >  instead of per context. Execlist driver still call notifier for
> >  each context sched-in/out events of current engine.
> >   2. At GVTg side, it binds a notifier_block for each physical engine
> >  at GVTg initialization period. Then GVTg can hear all context
> >  status events.
> > 
> > In this patch, GVTg do nothing for host context event, but later
> > will add a function there. But in any case, the notifier callback is
> > a noop if this is no active vGPU.
> > 
> > Since intel_gvt_init() is called at early initialization stage and
> > require the status notifier head has been initiated, I initiate it in
> > intel_engine_setup().
> > 
> > Signed-off-by: Changbin Du 
> Reviewed-by: Chris Wilson 
> 
> I presume you will apply this via gvt?
>
Sure, I'll sync with zhenyu about this. Then update patch with below
'bonus newline' fixed. Thanks.

> > diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c 
> > b/drivers/gpu/drm/i915/gvt/scheduler.c
> > index d3a56c9..64875ec 100644
> > --- a/drivers/gpu/drm/i915/gvt/scheduler.c
> > +++ b/drivers/gpu/drm/i915/gvt/scheduler.c
> > @@ -127,15 +127,14 @@ static int populate_shadow_context(struct 
> > intel_vgpu_workload *workload)
> > return 0;
> >  }
> >  
> > +
> 
> Bonus newline
> 
> >  static int shadow_context_status_change(struct notifier_block *nb,
> > unsigned long action, void *data)
> >  {
> > -   struct intel_vgpu *vgpu = container_of(nb,
> > -   struct intel_vgpu, shadow_ctx_notifier_block);
> > -   struct drm_i915_gem_request *req =
> > -   (struct drm_i915_gem_request *)data;
> > -   struct intel_gvt_workload_scheduler *scheduler =
> > -   >gvt->scheduler;
> > +   struct drm_i915_gem_request *req = (struct drm_i915_gem_request *)data;
> > +   struct intel_gvt *gvt = container_of(nb, struct intel_gvt,
> > +   shadow_ctx_notifier_block[req->engine->id]);
> > +   struct intel_gvt_workload_scheduler *scheduler = >scheduler;
> > struct intel_vgpu_workload *workload =
> > scheduler->current_workload[req->engine->id];
> >  
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

-- 
Thanks,
Changbin Du


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

Re: [Intel-gfx] [libdrm PATCH] intel: Make unsynchronized GTT mappings work on systems with snooping.

[Kenneth Graunke]

On Sunday, March 12, 2017 6:21:12 AM PDT Chris Wilson wrote:
> On Fri, Mar 10, 2017 at 05:14:32PM -0800, Kenneth Graunke wrote:
> > On systems without LLC, drm_intel_gem_bo_map_unsynchronized() has
> > had the surprising behavior of doing a synchronized GTT mapping.
> > This is obviously not what the user of the API wanted.
> > 
> > Eric left a comment indicating a valid concern: if the CPU and GPU
> > caches are incoherent, we don't keep track of where the user last
> > mapped the buffer, and what caches might contain relevant data.
> 
> Note this is an issue in libdrm_intel not tracking the cache domain
> transitions. Even just a switch between cpu and coherent would solve the
> majority of that - the caveat being shared bo where the tracking is
> incomplete.

Hmm.  Given the API we have today, I suppose we could make libdrm track
that - it just doesn't currently...

>  
> > Modern Atom systems still don't have LLC, but they do offer snooping,
> > which effectively makes the caches coherent.  The kernel appears to
> > set up the PTE/PPAT to enable snooping for everything where the cache
> > level is not I915_CACHE_NONE.  As far as I know, only scanout buffers
> > are marked as uncached.
> 
> Byt, bsw beg to differ. I don't have a bxt to know the results of the
> igt/kernel tests.

They do?  I was reading i915_gem_gtt.c, and see in byt_pte_encode:

if (level != I915_CACHE_NONE)
pte |= BYT_PTE_SNOOPED_BY_CPU_CACHES;

and also in chv_setup_private_ppat, a whole lot of CHV_PPAT_SNOOP.
It sure looked to me that we were setting up snooping for all non-UC
buffers.

i915_gem_object_is_coherent() also appears to indicate that LLC or
cache_level != I915_CACHE_NONE guarantees coherency.

Am I missing something?

> > Any buffers used by scanout should be flagged as non-reusable with
> > drm_intel_bo_disable_reuse(), prime export, or flink.  So, we can
> > assume that any reusable buffer should be snooped.
> 
> Not really, there is no reason why scanout buffers can't be reused.

Yes, we could make them reusable.  However, as far as I can tell, libdrm
doesn't consider the cacheability settings and just adds all reusable
BOs back into the cache.

As I understand it, the kernel marks a BO uncached when it's used for
display, and that BO remains uncached forever.  So that would mean that
libdrm's buffer cache would have random uncached buffers in it, which
would be really undesirable.  So I believe libdrm (and its users) mark
all scanout buffers non-reusable.

It would probably be better to check I915_GEM_GET_CACHING !=
I915_CACHING_NONE, but I was hoping to avoid the extra ioctl.

> > This patch enables unsynchronized mappings for reusable buffers
> > on all Gen6+ hardware (which have either LLC or snooping).
> > 
> > On Broxton, this improves the performance of Unigine Valley 1.0
> > on Low settings at 1280x720 by about 45%, and Unigine Heaven 4.0
> > (same settings) by about 53%.
> 
> Does anyone have figures for gtt performance on bxt - does it cover over
> the same performance penalty from earler atoms? Basically why bother to
> enable this over wc mapping (no stalls for a contended, limited
> resource) + detiling. (Just note that for detiling Y to WC you need to
> use a temporary cacheable page, or rearrange the code to make sure the
> reads/writes are in 64 byte chunks.) 

I'd like to use WC mappings eventually, but to me, fixing the
lack-of-async seems kind of orthogonal to changing GTT vs. WC.
Even if we want to use WC, why not fix this too?

> > Signed-off-by: Kenneth Graunke 
> > Cc: Chris Wilson 
> > Cc: mesa-...@lists.freedesktop.org
> > ---
> >  intel/intel_bufmgr_gem.c | 8 +---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> > 
> > It looks like Mesa and Beignet are the only callers of this function
> > (SNA and Anvil don't use libdrm, UXA and vaapi don't use this function.)
> > 
> > This passed our full barrage of Piglit/dEQP/GLCTS/GLESCTS testing.
> > gnome-shell still works, as does Unigine, and GLBenchmark.
> > 
> > I haven't tested any OpenCL workloads.
> > 
> > diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
> > index e260f2dc..f53f1fcc 100644
> > --- a/intel/intel_bufmgr_gem.c
> > +++ b/intel/intel_bufmgr_gem.c
> > @@ -1630,9 +1630,7 @@ int
> >  drm_intel_gem_bo_map_unsynchronized(drm_intel_bo *bo)
> >  {
> > drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *) bo->bufmgr;
> > -#ifdef HAVE_VALGRIND
> > drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *) bo;
> > -#endif
> > int ret;
> >  
> > /* If the CPU cache isn't coherent with the GTT, then use a
> > @@ -1641,8 +1639,12 @@ drm_intel_gem_bo_map_unsynchronized(drm_intel_bo *bo)
> >  * terms of drm_intel_bo_map vs drm_intel_gem_bo_map_gtt, so
> >  * we would potentially corrupt the buffer even when the user
> >  * does reasonable things.
> > +*
> > +* The caches are coherent on LLC platforms or snooping is 

Re: [Intel-gfx] The i915 stable patch marking is totally broken

[Greg KH]

On Sun, Mar 12, 2017 at 09:46:21PM +0100, Daniel Vetter wrote:
> On Sun, Mar 12, 2017 at 08:44:40PM +0100, Greg KH wrote:
> > Hi Daniel and Jani and other members of the i915-commit-cabal,
> > 
> > I've mentioned this a few times to Daniel in the past (like at the last
> > kernel summit), but the way you all are handling the tagging of patches
> > for inclusion in stable kernel releases is totally broken and causing me
> > no end of headaches.
> > 
> > It's due to your "you have a branch, you have a branch, you all have a
> > branch!" model of development.  You have patches that end up flowing in
> > to Linus's trees multiple times for different releases.  Now git is
> > smart enough to handle most of this, and you end up doing a lot of
> > hand-fixing for other merge issues, but when it comes to doing the
> > stable kernel work, none of that means squat.  All I get to see is when
> > a patch lands in Linus's tree, and if that patch has a "cc: stable@"
> > marking, then I look at it.
> > 
> > But, when a patch hits Linus through multiple branches, that means I see
> > it multiple times, usually months apart in timeframe.  This is
> > especially bad during the -rc1 merge window when all of the old work you
> > all did for the past 3 months hits Linus, which includes all of the old
> > bugfixes that were already in the previous kernel release.
> > 
> > I think there were over 25 different patches in -rc1 that hit this
> > issue.  Maybe more, maybe less, I don't know, I'm giving up at this
> > point, I wasted over 2 hours trying to figure out a way to do this in an
> > automated way, or by hand, or something else to deal with all of these
> > rejections or "fuzz merge" which really was a duplicate.
> > 
> > And the joy of your "cherry-picked from 12345..." messages, with git
> > commit ids that are no where to be found in Linus's tree at all, is
> > totally annoying as well, why even have this if it doesn't mean
> > anything?  I sure can't use it.
> > 
> > Not to mention all of the build-breakages, and normal "fails to apply"
> > that you all end up with, your driver subsystem has the largest instance
> > of those as well, and build-breakages are the worst, as they cause my
> > process to come to a screeching halt while I have to bisect to find the
> > broken patch.  Given the lack of patches that people actually send me
> > when I send in the "FAILED" emails, I'm guessing that you all don't care
> > that much about stable kernels either, which is fine, as again, I'm
> > giving up on your driver.
> > 
> > So, either you all only mark a patch _ONCE_.  Or come up with some way
> > for me to determine, in an automated way that a patch is already in
> > Linus's older release, or just don't mark anything and send me a
> > hand-curated list of git commit ids, or patches in mbox form (like DaveM
> > does), or something else you all come up with.  What is happening now is
> > not working at all, and as of now, I'm going to just drop all i915
> > patches with a cc: stable marking on the floor.
> > 
> > Congrats on being the first subsystem that I've had to blacklist from my
> > stable patch workflow :(
> > 
> > greg "35k feet above India and grumpy" k-h
> 
> So I blame this on flight level 350, but we discussed this at kernel
> summit. Every patch we cherry-pick over comes with a "cherry-picked from
> $sha1" line, as long as you ignore any such sha1 as duplicate you won't
> see the same patch twice.

I tried that, but that cherry-pick number doesn't seem to match up with
anything in Linus's tree.  Where are those numbers coming from?

Or there aren't numbers at all.  Look at commit:
8726f2faa371514fba2f594d799db95203dfeee0.  It just showed up in Linus's
tree, and there's no "cherry-pick" number in there.  It ended up in
4.9.7.

Hm, ok, you want me to look at the commit id and then search to see if
it's already been merged "before".  Ah, that's crazy.  So I need to do
that for every i915 patch?  Search backwards?  Ugh, that's a mess, no
wonder I couldn't figure it out...

> Iirc you said you'll implement this in your scripts, and as long as we
> never break this rule, you'll be fine. Since you seemed to have agreed to
> a solution that would solve all your headaches I didn't bother doing
> any changes on our side here.

So if a commit says "cherry-pick", I guess I can always assume it's safe
to add, right?  If not, _then_ I have to run the "search backwards"
logic, right?

Ok, let me think about this a bit to see if that's possible to script...

> what happened? Has bashing drm become the cool thing to do somehow?

I don't understand, I haven't bashed drm in years :)

thanks,

greg "jet lagged" k-h
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] The i915 stable patch marking is totally broken

[Greg KH]

On Mon, Mar 13, 2017 at 06:11:12AM +1000, Dave Airlie wrote:
> On 13 March 2017 at 05:44, Greg KH  wrote:
> > Hi Daniel and Jani and other members of the i915-commit-cabal,
> >
> > I've mentioned this a few times to Daniel in the past (like at the last
> > kernel summit), but the way you all are handling the tagging of patches
> > for inclusion in stable kernel releases is totally broken and causing me
> > no end of headaches.
> >
> > It's due to your "you have a branch, you have a branch, you all have a
> > branch!" model of development.  You have patches that end up flowing in
> > to Linus's trees multiple times for different releases.  Now git is
> > smart enough to handle most of this, and you end up doing a lot of
> > hand-fixing for other merge issues, but when it comes to doing the
> > stable kernel work, none of that means squat.  All I get to see is when
> > a patch lands in Linus's tree, and if that patch has a "cc: stable@"
> > marking, then I look at it.
> >
> > But, when a patch hits Linus through multiple branches, that means I see
> > it multiple times, usually months apart in timeframe.  This is
> > especially bad during the -rc1 merge window when all of the old work you
> > all did for the past 3 months hits Linus, which includes all of the old
> > bugfixes that were already in the previous kernel release.
> >
> > I think there were over 25 different patches in -rc1 that hit this
> > issue.  Maybe more, maybe less, I don't know, I'm giving up at this
> > point, I wasted over 2 hours trying to figure out a way to do this in an
> > automated way, or by hand, or something else to deal with all of these
> > rejections or "fuzz merge" which really was a duplicate.
> >
> > And the joy of your "cherry-picked from 12345..." messages, with git
> > commit ids that are no where to be found in Linus's tree at all, is
> > totally annoying as well, why even have this if it doesn't mean
> > anything?  I sure can't use it.
> >
> > Not to mention all of the build-breakages, and normal "fails to apply"
> > that you all end up with, your driver subsystem has the largest instance
> > of those as well, and build-breakages are the worst, as they cause my
> > process to come to a screeching halt while I have to bisect to find the
> > broken patch.  Given the lack of patches that people actually send me
> > when I send in the "FAILED" emails, I'm guessing that you all don't care
> > that much about stable kernels either, which is fine, as again, I'm
> > giving up on your driver.
> >
> > So, either you all only mark a patch _ONCE_.  Or come up with some way
> > for me to determine, in an automated way that a patch is already in
> > Linus's older release, or just don't mark anything and send me a
> > hand-curated list of git commit ids, or patches in mbox form (like DaveM
> > does), or something else you all come up with.  What is happening now is
> > not working at all, and as of now, I'm going to just drop all i915
> > patches with a cc: stable marking on the floor.
> >
> > Congrats on being the first subsystem that I've had to blacklist from my
> > stable patch workflow :(
> >
> > greg "35k feet above India and grumpy" k-h
> 
> What happened to, I won't ask subsystem maintainers to do any extra work :-)

Well, when they cause me extra work, or problems, I'm allowed to complain :)

> But I'm not sure why the model doesn't break for others, surely some 
> subsystems
> realise after committing patches to -next, they really are more urgent
> so put them into a -fixes pull earlier, but can't rebase -next. The
> cherry-pick tag should have the info vs the -next tree that Linus is
> pulling in the next merge window, it would be a bit difficult to do a
> cherry-pick to -fixes from a branch Linus has already pulled.

Nope, no other subsystem does it like the i915 driver does.  Only rarely
does a patch for stable come in multiple times.  The i915 driver does it
way too often.

Why don't the maintainers know which tree to put them in when they are
submitted?  As an example, if I get a patch that needs to go to Linus, I
put it in my usb-linus branch, and when it hits a -rc release, I then
merge that -rc back into my usb-next branch.  So I end up with about 2-3
merges to -rc every release, which isn't bad and doesn't cause any
duplication issues.

Seems that most other subsystems also do this as well.

> I don't think there is anything incorrect in the model, and it
> certainly has nothing to do with "the branch model" or whatever you
> are alluding to there.  The branches are pretty simple, a -next and a
> -fixes with occasional -next-fixes for merge window, is it just the
> sheer number of patches that go to -next and get pulled into -fixes
> that overwhelms you?

25-30 patches marked for stable ended up in 4.11-rc1 that were already
in 4.10.0.  And I think this was way less than what happened in
4.12-rc1.  It's hard to determine that a patch really is a duplicate, or
if it just doesn't apply 

[Intel-gfx] ✗ Fi.CI.BAT: warning for drm/i915: annote drop_caches debugfs interface with lockdep (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: annote drop_caches debugfs interface with lockdep (rev2)
URL   : https://patchwork.freedesktop.org/series/21114/
State : warning

== Summary ==

Series 21114v2 drm/i915: annote drop_caches debugfs interface with lockdep
https://patchwork.freedesktop.org/api/1.0/series/21114/revisions/2/mbox/

Test gem_exec_gttfill:
Subgroup basic:
pass   -> DMESG-WARN (fi-byt-n2820)
pass   -> DMESG-WARN (fi-kbl-7500u)
pass   -> DMESG-WARN (fi-snb-2600)
pass   -> DMESG-WARN (fi-bxt-j4205)
pass   -> DMESG-WARN (fi-skl-6700hq)
pass   -> DMESG-WARN (fi-skl-6260u)
pass   -> DMESG-WARN (fi-skl-6770hq)
pass   -> DMESG-WARN (fi-hsw-4770r)
pass   -> DMESG-WARN (fi-ilk-650)
pass   -> DMESG-WARN (fi-snb-2520m)
pass   -> DMESG-WARN (fi-skl-6700k)
pass   -> DMESG-WARN (fi-ivb-3770)
pass   -> DMESG-WARN (fi-byt-j1900)
pass   -> DMESG-WARN (fi-bdw-5557u)
pass   -> DMESG-WARN (fi-hsw-4770)
pass   -> DMESG-WARN (fi-ivb-3520m)
Test gem_exec_suspend:
Subgroup basic-s3:
dmesg-warn -> PASS   (fi-kbl-7500u) fdo#100124
Subgroup basic-s4-devices:
pass   -> DMESG-WARN (fi-bxt-t5700) fdo#100125
Test kms_force_connector_basic:
Subgroup prune-stale-modes:
skip   -> PASS   (fi-snb-2520m)

fdo#100124 https://bugs.freedesktop.org/show_bug.cgi?id=100124
fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125

fi-bdw-5557u total:278  pass:266  dwarn:1   dfail:0   fail:0   skip:11  
time: 458s
fi-bsw-n3050 total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39  
time: 590s
fi-bxt-j4205 total:278  pass:258  dwarn:1   dfail:0   fail:0   skip:19  
time: 514s
fi-bxt-t5700 total:278  pass:257  dwarn:1   dfail:0   fail:0   skip:20  
time: 546s
fi-byt-j1900 total:278  pass:250  dwarn:1   dfail:0   fail:0   skip:27  
time: 476s
fi-byt-n2820 total:278  pass:246  dwarn:1   dfail:0   fail:0   skip:31  
time: 478s
fi-hsw-4770  total:278  pass:261  dwarn:1   dfail:0   fail:0   skip:16  
time: 432s
fi-hsw-4770r total:278  pass:261  dwarn:1   dfail:0   fail:0   skip:16  
time: 423s
fi-ilk-650   total:278  pass:227  dwarn:1   dfail:0   fail:0   skip:50  
time: 551s
fi-ivb-3520m total:278  pass:259  dwarn:1   dfail:0   fail:0   skip:18  
time: 504s
fi-ivb-3770  total:278  pass:259  dwarn:1   dfail:0   fail:0   skip:18  
time: 482s
fi-kbl-7500u total:278  pass:259  dwarn:1   dfail:0   fail:0   skip:18  
time: 469s
fi-skl-6260u total:278  pass:267  dwarn:1   dfail:0   fail:0   skip:10  
time: 483s
fi-skl-6700hqtotal:278  pass:260  dwarn:1   dfail:0   fail:0   skip:17  
time: 587s
fi-skl-6700k total:278  pass:255  dwarn:5   dfail:0   fail:0   skip:18  
time: 483s
fi-skl-6770hqtotal:278  pass:267  dwarn:1   dfail:0   fail:0   skip:10  
time: 530s
fi-snb-2520m total:278  pass:249  dwarn:1   dfail:0   fail:0   skip:28  
time: 540s
fi-snb-2600  total:278  pass:248  dwarn:1   dfail:0   fail:0   skip:29  
time: 407s

2cb12884655eab41d4992b33ccb36c609c4537d3 drm-tip: 2017y-03m-12d-13h-00m-05s UTC 
integration manifest
0cfa335 drm/i915: annote drop_caches debugfs interface with lockdep

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4146/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: annote drop_caches debugfs interface with lockdep

[Daniel Vetter]

The trouble we have is that we can't really test all the shrinker
recursion stuff exhaustively in BAT because any kind of thrashing
stress test just takes too long.

But that leaves a really big gap open, since shrinker recursions are
one of the most annoying bugs. Now lockdep already has support for
checking allocation deadlocks:

- Direct reclaim paths are marked up with
  lockdep_set_current_reclaim_state() and
  lockdep_clear_current_reclaim_state().

- Any allocation paths are marked with lockdep_trace_alloc().

If we simply mark up our debugfs with the reclaim annotations, any
code and locks taken in there will automatically complete the picture
with any allocation paths we already have, as long as we have a simple
testcase in BAT which throws out a few objects using this interface.
Not stress test or thrashing needed at all.

v2: Need to EXPORT_SYMBOL_GPL to make it compile as a module.

v3: Fixup rebase fail (spotted by Chris).

Cc: Chris Wilson 
Cc: Peter Zijlstra 
Cc: Ingo Molnar 
Cc: linux-ker...@vger.kernel.org
Signed-off-by: Daniel Vetter 

--

Peter/Ingo,

We want this to validate the i915 shrinker locking in our fast tests
without thrashing badly (that takes too long, we can only thrash in
the extended runs). Can you pls take a look and if it's ok ack for
merging through drm-intel.git?

Thanks, Daniel
---
 drivers/gpu/drm/i915/i915_debugfs.c | 2 ++
 kernel/locking/lockdep.c| 2 ++
 2 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 82fb005a5e22..fbe761a3f5bd 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -4273,6 +4273,7 @@ i915_drop_caches_set(void *data, u64 val)
if (val & (DROP_RETIRE | DROP_ACTIVE))
i915_gem_retire_requests(dev_priv);
 
+   lockdep_set_current_reclaim_state(GFP_KERNEL);
if (val & DROP_BOUND)
i915_gem_shrink(dev_priv, LONG_MAX, I915_SHRINK_BOUND);
 
@@ -4281,6 +4282,7 @@ i915_drop_caches_set(void *data, u64 val)
 
if (val & DROP_SHRINK_ALL)
i915_gem_shrink_all(dev_priv);
+   lockdep_clear_current_reclaim_state();
 
 unlock:
mutex_unlock(>struct_mutex);
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 12e38c213b70..508cbf31d43e 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -3856,11 +3856,13 @@ void lockdep_set_current_reclaim_state(gfp_t gfp_mask)
 {
current->lockdep_reclaim_gfp = gfp_mask;
 }
+EXPORT_SYMBOL_GPL(lockdep_set_current_reclaim_state);
 
 void lockdep_clear_current_reclaim_state(void)
 {
current->lockdep_reclaim_gfp = 0;
 }
+EXPORT_SYMBOL_GPL(lockdep_clear_current_reclaim_state);
 
 #ifdef CONFIG_LOCK_STAT
 static int
-- 
2.11.0

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

Re: [Intel-gfx] The i915 stable patch marking is totally broken

[Daniel Vetter]

On Sun, Mar 12, 2017 at 08:44:40PM +0100, Greg KH wrote:
> Hi Daniel and Jani and other members of the i915-commit-cabal,
> 
> I've mentioned this a few times to Daniel in the past (like at the last
> kernel summit), but the way you all are handling the tagging of patches
> for inclusion in stable kernel releases is totally broken and causing me
> no end of headaches.
> 
> It's due to your "you have a branch, you have a branch, you all have a
> branch!" model of development.  You have patches that end up flowing in
> to Linus's trees multiple times for different releases.  Now git is
> smart enough to handle most of this, and you end up doing a lot of
> hand-fixing for other merge issues, but when it comes to doing the
> stable kernel work, none of that means squat.  All I get to see is when
> a patch lands in Linus's tree, and if that patch has a "cc: stable@"
> marking, then I look at it.
> 
> But, when a patch hits Linus through multiple branches, that means I see
> it multiple times, usually months apart in timeframe.  This is
> especially bad during the -rc1 merge window when all of the old work you
> all did for the past 3 months hits Linus, which includes all of the old
> bugfixes that were already in the previous kernel release.
> 
> I think there were over 25 different patches in -rc1 that hit this
> issue.  Maybe more, maybe less, I don't know, I'm giving up at this
> point, I wasted over 2 hours trying to figure out a way to do this in an
> automated way, or by hand, or something else to deal with all of these
> rejections or "fuzz merge" which really was a duplicate.
> 
> And the joy of your "cherry-picked from 12345..." messages, with git
> commit ids that are no where to be found in Linus's tree at all, is
> totally annoying as well, why even have this if it doesn't mean
> anything?  I sure can't use it.
> 
> Not to mention all of the build-breakages, and normal "fails to apply"
> that you all end up with, your driver subsystem has the largest instance
> of those as well, and build-breakages are the worst, as they cause my
> process to come to a screeching halt while I have to bisect to find the
> broken patch.  Given the lack of patches that people actually send me
> when I send in the "FAILED" emails, I'm guessing that you all don't care
> that much about stable kernels either, which is fine, as again, I'm
> giving up on your driver.
> 
> So, either you all only mark a patch _ONCE_.  Or come up with some way
> for me to determine, in an automated way that a patch is already in
> Linus's older release, or just don't mark anything and send me a
> hand-curated list of git commit ids, or patches in mbox form (like DaveM
> does), or something else you all come up with.  What is happening now is
> not working at all, and as of now, I'm going to just drop all i915
> patches with a cc: stable marking on the floor.
> 
> Congrats on being the first subsystem that I've had to blacklist from my
> stable patch workflow :(
> 
> greg "35k feet above India and grumpy" k-h

So I blame this on flight level 350, but we discussed this at kernel
summit. Every patch we cherry-pick over comes with a "cherry-picked from
$sha1" line, as long as you ignore any such sha1 as duplicate you won't
see the same patch twice.

Iirc you said you'll implement this in your scripts, and as long as we
never break this rule, you'll be fine. Since you seemed to have agreed to
a solution that would solve all your headaches I didn't bother doing
any changes on our side here.

what happened? Has bashing drm become the cool thing to do somehow?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] The i915 stable patch marking is totally broken

[Dave Airlie]

On 13 March 2017 at 05:44, Greg KH  wrote:
> Hi Daniel and Jani and other members of the i915-commit-cabal,
>
> I've mentioned this a few times to Daniel in the past (like at the last
> kernel summit), but the way you all are handling the tagging of patches
> for inclusion in stable kernel releases is totally broken and causing me
> no end of headaches.
>
> It's due to your "you have a branch, you have a branch, you all have a
> branch!" model of development.  You have patches that end up flowing in
> to Linus's trees multiple times for different releases.  Now git is
> smart enough to handle most of this, and you end up doing a lot of
> hand-fixing for other merge issues, but when it comes to doing the
> stable kernel work, none of that means squat.  All I get to see is when
> a patch lands in Linus's tree, and if that patch has a "cc: stable@"
> marking, then I look at it.
>
> But, when a patch hits Linus through multiple branches, that means I see
> it multiple times, usually months apart in timeframe.  This is
> especially bad during the -rc1 merge window when all of the old work you
> all did for the past 3 months hits Linus, which includes all of the old
> bugfixes that were already in the previous kernel release.
>
> I think there were over 25 different patches in -rc1 that hit this
> issue.  Maybe more, maybe less, I don't know, I'm giving up at this
> point, I wasted over 2 hours trying to figure out a way to do this in an
> automated way, or by hand, or something else to deal with all of these
> rejections or "fuzz merge" which really was a duplicate.
>
> And the joy of your "cherry-picked from 12345..." messages, with git
> commit ids that are no where to be found in Linus's tree at all, is
> totally annoying as well, why even have this if it doesn't mean
> anything?  I sure can't use it.
>
> Not to mention all of the build-breakages, and normal "fails to apply"
> that you all end up with, your driver subsystem has the largest instance
> of those as well, and build-breakages are the worst, as they cause my
> process to come to a screeching halt while I have to bisect to find the
> broken patch.  Given the lack of patches that people actually send me
> when I send in the "FAILED" emails, I'm guessing that you all don't care
> that much about stable kernels either, which is fine, as again, I'm
> giving up on your driver.
>
> So, either you all only mark a patch _ONCE_.  Or come up with some way
> for me to determine, in an automated way that a patch is already in
> Linus's older release, or just don't mark anything and send me a
> hand-curated list of git commit ids, or patches in mbox form (like DaveM
> does), or something else you all come up with.  What is happening now is
> not working at all, and as of now, I'm going to just drop all i915
> patches with a cc: stable marking on the floor.
>
> Congrats on being the first subsystem that I've had to blacklist from my
> stable patch workflow :(
>
> greg "35k feet above India and grumpy" k-h

What happened to, I won't ask subsystem maintainers to do any extra work :-)

But I'm not sure why the model doesn't break for others, surely some subsystems
realise after committing patches to -next, they really are more urgent
so put them
into a -fixes pull earlier, but can't rebase -next. The cherry-pick tag should
have the info vs the -next tree that Linus is pulling in the next merge window,
it would be a bit difficult to do a cherry-pick to -fixes from a
branch Linus has
already pulled.

I don't think there is anything incorrect in the model, and it
certainly has nothing
to do with "the branch model" or whatever you are alluding to there.
The branches
are pretty simple, a -next and a -fixes with occasional -next-fixes
for merge window,
is it just the sheer number of patches that go to -next and get pulled
into -fixes that
overwhelms you?

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

[Intel-gfx] The i915 stable patch marking is totally broken

[Greg KH]

Hi Daniel and Jani and other members of the i915-commit-cabal,

I've mentioned this a few times to Daniel in the past (like at the last
kernel summit), but the way you all are handling the tagging of patches
for inclusion in stable kernel releases is totally broken and causing me
no end of headaches.

It's due to your "you have a branch, you have a branch, you all have a
branch!" model of development.  You have patches that end up flowing in
to Linus's trees multiple times for different releases.  Now git is
smart enough to handle most of this, and you end up doing a lot of
hand-fixing for other merge issues, but when it comes to doing the
stable kernel work, none of that means squat.  All I get to see is when
a patch lands in Linus's tree, and if that patch has a "cc: stable@"
marking, then I look at it.

But, when a patch hits Linus through multiple branches, that means I see
it multiple times, usually months apart in timeframe.  This is
especially bad during the -rc1 merge window when all of the old work you
all did for the past 3 months hits Linus, which includes all of the old
bugfixes that were already in the previous kernel release.

I think there were over 25 different patches in -rc1 that hit this
issue.  Maybe more, maybe less, I don't know, I'm giving up at this
point, I wasted over 2 hours trying to figure out a way to do this in an
automated way, or by hand, or something else to deal with all of these
rejections or "fuzz merge" which really was a duplicate.

And the joy of your "cherry-picked from 12345..." messages, with git
commit ids that are no where to be found in Linus's tree at all, is
totally annoying as well, why even have this if it doesn't mean
anything?  I sure can't use it.

Not to mention all of the build-breakages, and normal "fails to apply"
that you all end up with, your driver subsystem has the largest instance
of those as well, and build-breakages are the worst, as they cause my
process to come to a screeching halt while I have to bisect to find the
broken patch.  Given the lack of patches that people actually send me
when I send in the "FAILED" emails, I'm guessing that you all don't care
that much about stable kernels either, which is fine, as again, I'm
giving up on your driver.

So, either you all only mark a patch _ONCE_.  Or come up with some way
for me to determine, in an automated way that a patch is already in
Linus's older release, or just don't mark anything and send me a
hand-curated list of git commit ids, or patches in mbox form (like DaveM
does), or something else you all come up with.  What is happening now is
not working at all, and as of now, I'm going to just drop all i915
patches with a cc: stable marking on the floor.

Congrats on being the first subsystem that I've had to blacklist from my
stable patch workflow :(

greg "35k feet above India and grumpy" k-h
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: annote drop_caches debugfs interface with lockdep

[Chris Wilson]

On Sun, Mar 12, 2017 at 08:27:16PM +0100, Daniel Vetter wrote:
> The trouble we have is that we can't really test all the shrinker
> recursion stuff exhaustively in BAT because any kind of thrashing
> stress test just takes too long.
> 
> But that leaves a really big gap open, since shrinker recursions are
> one of the most annoying bugs. Now lockdep already has support for
> checking allocation deadlocks:
> 
> - Direct reclaim paths are marked up with
>   lockdep_set_current_reclaim_state() and
>   lockdep_clear_current_reclaim_state().
> 
> - Any allocation paths are marked with lockdep_trace_alloc().
> 
> If we simply mark up our debugfs with the reclaim annotations, any
> code and locks taken in there will automatically complete the picture
> with any allocation paths we already have, as long as we have a simple
> testcase in BAT which throws out a few objects using this interface.
> Not stress test or thrashing needed at all.
> 
> v2: Need to EXPORT_SYMBOL_GPL to make it compile as a module.
> 
> Cc: Chris Wilson 
> Cc: Peter Zijlstra 
> Cc: Ingo Molnar 
> Cc: linux-ker...@vger.kernel.org
> Reviewed-by: Chris Wilson  (v1)
> Signed-off-by: Daniel Vetter 
> 
> --
> 
> Peter/Ingo,
> 
> We want this to validate the i915 shrinker locking in our fast tests
> without thrashing badly (that takes too long, we can only thrash in
> the extended runs). Can you pls take a look and if it's ok ack for
> merging through drm-intel.git?
> 
> Thanks, Daniel
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 2 ++
>  kernel/locking/lockdep.c| 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 82fb005a5e22..0f1d6c4a212b 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -4273,11 +4273,13 @@ i915_drop_caches_set(void *data, u64 val)
>   if (val & (DROP_RETIRE | DROP_ACTIVE))
>   i915_gem_retire_requests(dev_priv);
>  
> + lockdep_set_current_reclaim_state(GFP_KERNEL);
>   if (val & DROP_BOUND)
>   i915_gem_shrink(dev_priv, LONG_MAX, I915_SHRINK_BOUND);
>  
>   if (val & DROP_UNBOUND)
>   i915_gem_shrink(dev_priv, LONG_MAX, I915_SHRINK_UNBOUND);
> + lockdep_clear_current_reclaim_state();
>  
>   if (val & DROP_SHRINK_ALL)
>   i915_gem_shrink_all(dev_priv);

Best to move the clear to here.
-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

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: annote drop_caches debugfs interface with lockdep

[Patchwork]

== Series Details ==

Series: drm/i915: annote drop_caches debugfs interface with lockdep
URL   : https://patchwork.freedesktop.org/series/21114/
State : success

== Summary ==

Series 21114v1 drm/i915: annote drop_caches debugfs interface with lockdep
https://patchwork.freedesktop.org/api/1.0/series/21114/revisions/1/mbox/

Test kms_force_connector_basic:
Subgroup prune-stale-modes:
skip   -> PASS   (fi-snb-2520m)

fi-bdw-5557u total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  
time: 465s
fi-bsw-n3050 total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39  
time: 614s
fi-bxt-j4205 total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  
time: 532s
fi-bxt-t5700 total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  
time: 585s
fi-byt-j1900 total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27  
time: 503s
fi-byt-n2820 total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31  
time: 501s
fi-hsw-4770  total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  
time: 436s
fi-hsw-4770r total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  
time: 436s
fi-ilk-650   total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  
time: 443s
fi-ivb-3520m total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  
time: 506s
fi-ivb-3770  total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  
time: 495s
fi-kbl-7500u total:278  pass:259  dwarn:1   dfail:0   fail:0   skip:18  
time: 479s
fi-skl-6260u total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  
time: 507s
fi-skl-6700hqtotal:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  
time: 591s
fi-skl-6700k total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  
time: 501s
fi-skl-6770hqtotal:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  
time: 540s
fi-snb-2520m total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  
time: 550s
fi-snb-2600  total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  
time: 426s

2cb12884655eab41d4992b33ccb36c609c4537d3 drm-tip: 2017y-03m-12d-13h-00m-05s UTC 
integration manifest
bbcfeae drm/i915: annote drop_caches debugfs interface with lockdep

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4145/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: annote drop_caches debugfs interface with lockdep

[Daniel Vetter]

The trouble we have is that we can't really test all the shrinker
recursion stuff exhaustively in BAT because any kind of thrashing
stress test just takes too long.

But that leaves a really big gap open, since shrinker recursions are
one of the most annoying bugs. Now lockdep already has support for
checking allocation deadlocks:

- Direct reclaim paths are marked up with
  lockdep_set_current_reclaim_state() and
  lockdep_clear_current_reclaim_state().

- Any allocation paths are marked with lockdep_trace_alloc().

If we simply mark up our debugfs with the reclaim annotations, any
code and locks taken in there will automatically complete the picture
with any allocation paths we already have, as long as we have a simple
testcase in BAT which throws out a few objects using this interface.
Not stress test or thrashing needed at all.

v2: Need to EXPORT_SYMBOL_GPL to make it compile as a module.

Cc: Chris Wilson 
Cc: Peter Zijlstra 
Cc: Ingo Molnar 
Cc: linux-ker...@vger.kernel.org
Reviewed-by: Chris Wilson  (v1)
Signed-off-by: Daniel Vetter 

--

Peter/Ingo,

We want this to validate the i915 shrinker locking in our fast tests
without thrashing badly (that takes too long, we can only thrash in
the extended runs). Can you pls take a look and if it's ok ack for
merging through drm-intel.git?

Thanks, Daniel
---
 drivers/gpu/drm/i915/i915_debugfs.c | 2 ++
 kernel/locking/lockdep.c| 2 ++
 2 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 82fb005a5e22..0f1d6c4a212b 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -4273,11 +4273,13 @@ i915_drop_caches_set(void *data, u64 val)
if (val & (DROP_RETIRE | DROP_ACTIVE))
i915_gem_retire_requests(dev_priv);
 
+   lockdep_set_current_reclaim_state(GFP_KERNEL);
if (val & DROP_BOUND)
i915_gem_shrink(dev_priv, LONG_MAX, I915_SHRINK_BOUND);
 
if (val & DROP_UNBOUND)
i915_gem_shrink(dev_priv, LONG_MAX, I915_SHRINK_UNBOUND);
+   lockdep_clear_current_reclaim_state();
 
if (val & DROP_SHRINK_ALL)
i915_gem_shrink_all(dev_priv);
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 12e38c213b70..508cbf31d43e 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -3856,11 +3856,13 @@ void lockdep_set_current_reclaim_state(gfp_t gfp_mask)
 {
current->lockdep_reclaim_gfp = gfp_mask;
 }
+EXPORT_SYMBOL_GPL(lockdep_set_current_reclaim_state);
 
 void lockdep_clear_current_reclaim_state(void)
 {
current->lockdep_reclaim_gfp = 0;
 }
+EXPORT_SYMBOL_GPL(lockdep_clear_current_reclaim_state);
 
 #ifdef CONFIG_LOCK_STAT
 static int
-- 
2.11.0

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

Re: [Intel-gfx] [libdrm PATCH] intel: Make unsynchronized GTT mappings work on systems with snooping.

[David Weinehall]

On Sun, Mar 12, 2017 at 01:21:12PM +, Chris Wilson wrote:
> On Fri, Mar 10, 2017 at 05:14:32PM -0800, Kenneth Graunke wrote:
> > On systems without LLC, drm_intel_gem_bo_map_unsynchronized() has
> > had the surprising behavior of doing a synchronized GTT mapping.
> > This is obviously not what the user of the API wanted.
> > 
> > Eric left a comment indicating a valid concern: if the CPU and GPU
> > caches are incoherent, we don't keep track of where the user last
> > mapped the buffer, and what caches might contain relevant data.
> 
> Note this is an issue in libdrm_intel not tracking the cache domain
> transitions. Even just a switch between cpu and coherent would solve the
> majority of that - the caveat being shared bo where the tracking is
> incomplete.
>  
> > Modern Atom systems still don't have LLC, but they do offer snooping,
> > which effectively makes the caches coherent.  The kernel appears to
> > set up the PTE/PPAT to enable snooping for everything where the cache
> > level is not I915_CACHE_NONE.  As far as I know, only scanout buffers
> > are marked as uncached.
> 
> Byt, bsw beg to differ. I don't have a bxt to know the results of the
> igt/kernel tests.

Just give me a list of the tests to run (and, if any, what patches
to apply and the debugging level you want enabled) and I'll provide
the necessary results.

> > Any buffers used by scanout should be flagged as non-reusable with
> > drm_intel_bo_disable_reuse(), prime export, or flink.  So, we can
> > assume that any reusable buffer should be snooped.
> 
> Not really, there is no reason why scanout buffers can't be reused.
>  
> > This patch enables unsynchronized mappings for reusable buffers
> > on all Gen6+ hardware (which have either LLC or snooping).
> > 
> > On Broxton, this improves the performance of Unigine Valley 1.0
> > on Low settings at 1280x720 by about 45%, and Unigine Heaven 4.0
> > (same settings) by about 53%.
> 
> Does anyone have figures for gtt performance on bxt - does it cover over
> the same performance penalty from earler atoms? Basically why bother to
> enable this over wc mapping (no stalls for a contended, limited
> resource) + detiling. (Just note that for detiling Y to WC you need to
> use a temporary cacheable page, or rearrange the code to make sure the
> reads/writes are in 64 byte chunks.) 

Again, I can run any tests you'd like to get numbers from,
just give me a list.


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

[Intel-gfx] ✗ Fi.CI.BAT: warning for drm/i915: Inline gen6_sanitize_rps_pm_mask()

[Patchwork]

== Series Details ==

Series: drm/i915: Inline gen6_sanitize_rps_pm_mask()
URL   : https://patchwork.freedesktop.org/series/21107/
State : warning

== Summary ==

Series 21107v1 drm/i915: Inline gen6_sanitize_rps_pm_mask()
https://patchwork.freedesktop.org/api/1.0/series/21107/revisions/1/mbox/

Test gem_exec_flush:
Subgroup basic-batch-kernel-default-uc:
pass   -> FAIL   (fi-snb-2600) fdo#17
Test kms_cursor_legacy:
Subgroup basic-flip-before-cursor-legacy:
pass   -> DMESG-WARN (fi-byt-n2820)
Test kms_force_connector_basic:
Subgroup prune-stale-modes:
skip   -> PASS   (fi-snb-2520m)

fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17

fi-bdw-5557u total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  
time: 465s
fi-bsw-n3050 total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39  
time: 604s
fi-bxt-j4205 total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  
time: 532s
fi-byt-j1900 total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27  
time: 505s
fi-byt-n2820 total:278  pass:246  dwarn:1   dfail:0   fail:0   skip:31  
time: 506s
fi-hsw-4770  total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  
time: 437s
fi-hsw-4770r total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  
time: 429s
fi-ilk-650   total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  
time: 452s
fi-ivb-3520m total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  
time: 510s
fi-ivb-3770  total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  
time: 499s
fi-kbl-7500u total:278  pass:259  dwarn:1   dfail:0   fail:0   skip:18  
time: 469s
fi-skl-6260u total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  
time: 502s
fi-skl-6700hqtotal:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  
time: 587s
fi-skl-6700k total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  
time: 496s
fi-skl-6770hqtotal:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  
time: 540s
fi-snb-2520m total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  
time: 547s
fi-snb-2600  total:278  pass:248  dwarn:0   dfail:0   fail:1   skip:29  
time: 415s
fi-bxt-t5700 failed to collect. IGT log at Patchwork_4144/fi-bxt-t5700/igt.log

2cb12884655eab41d4992b33ccb36c609c4537d3 drm-tip: 2017y-03m-12d-13h-00m-05s UTC 
integration manifest
888da0c drm/i915: Inline gen6_sanitize_rps_pm_mask()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4144/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Rename REDIRECT_TO_GUC bit

[Patchwork]

== Series Details ==

Series: drm/i915: Rename REDIRECT_TO_GUC bit
URL   : https://patchwork.freedesktop.org/series/21104/
State : success

== Summary ==

Series 21104v1 drm/i915: Rename REDIRECT_TO_GUC bit
https://patchwork.freedesktop.org/api/1.0/series/21104/revisions/1/mbox/

Test gem_exec_flush:
Subgroup basic-batch-kernel-default-uc:
pass   -> FAIL   (fi-snb-2600) fdo#17
Test kms_force_connector_basic:
Subgroup prune-stale-modes:
skip   -> PASS   (fi-snb-2520m)

fdo#17 https://bugs.freedesktop.org/show_bug.cgi?id=17

fi-bdw-5557u total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  
time: 458s
fi-bsw-n3050 total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39  
time: 612s
fi-bxt-j4205 total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  
time: 540s
fi-bxt-t5700 total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  
time: 588s
fi-byt-j1900 total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27  
time: 505s
fi-byt-n2820 total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31  
time: 509s
fi-hsw-4770  total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  
time: 438s
fi-hsw-4770r total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  
time: 435s
fi-ilk-650   total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  
time: 446s
fi-ivb-3520m total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  
time: 501s
fi-ivb-3770  total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  
time: 496s
fi-kbl-7500u total:278  pass:259  dwarn:1   dfail:0   fail:0   skip:18  
time: 469s
fi-skl-6260u total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  
time: 496s
fi-skl-6700hqtotal:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  
time: 595s
fi-skl-6700k total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  
time: 488s
fi-skl-6770hqtotal:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  
time: 530s
fi-snb-2520m total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  
time: 551s
fi-snb-2600  total:278  pass:248  dwarn:0   dfail:0   fail:1   skip:29  
time: 420s

2cb12884655eab41d4992b33ccb36c609c4537d3 drm-tip: 2017y-03m-12d-13h-00m-05s UTC 
integration manifest
1691433 drm/i915: Rename REDIRECT_TO_GUC bit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4143/
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: Inline gen6_sanitize_rps_pm_mask()

[Chris Wilson]

gen6_sanitize_rps_pm_mask() is small enough that inlining it shrinks the
object code.

Signed-off-by: Chris Wilson 
Cc: Tvrtko Ursulin 
---
 drivers/gpu/drm/i915/i915_irq.c  | 5 -
 drivers/gpu/drm/i915/intel_drv.h | 8 +++-
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 89ccf3e1fda5..b8a0afdc26a2 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -389,11 +389,6 @@ void gen6_enable_rps_interrupts(struct drm_i915_private 
*dev_priv)
spin_unlock_irq(_priv->irq_lock);
 }
 
-u32 gen6_sanitize_rps_pm_mask(struct drm_i915_private *dev_priv, u32 mask)
-{
-   return (mask & ~dev_priv->rps.pm_intrmsk_mbz);
-}
-
 void gen6_disable_rps_interrupts(struct drm_i915_private *dev_priv)
 {
if (!READ_ONCE(dev_priv->rps.interrupts_enabled))
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 578f7d20501f..4e38682d22b5 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1188,7 +1188,13 @@ void gen6_disable_pm_irq(struct drm_i915_private 
*dev_priv, uint32_t mask);
 void gen6_reset_rps_interrupts(struct drm_i915_private *dev_priv);
 void gen6_enable_rps_interrupts(struct drm_i915_private *dev_priv);
 void gen6_disable_rps_interrupts(struct drm_i915_private *dev_priv);
-u32 gen6_sanitize_rps_pm_mask(struct drm_i915_private *dev_priv, u32 mask);
+
+static inline u32 gen6_sanitize_rps_pm_mask(struct drm_i915_private *i915,
+   u32 mask)
+{
+   return mask & ~i915->rps.pm_intrmsk_mbz;
+}
+
 void intel_runtime_pm_disable_interrupts(struct drm_i915_private *dev_priv);
 void intel_runtime_pm_enable_interrupts(struct drm_i915_private *dev_priv);
 static inline bool intel_irqs_enabled(struct drm_i915_private *dev_priv)
-- 
2.11.0

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

[Intel-gfx] [PATCH] drm/i915: Rename REDIRECT_TO_GUC bit

[Chris Wilson]

The REDIRECT_TO_GUC bit is a strange beast as it is a disable bit -
setting the bit in the pm interrupt generation stops the interrupt going
to the guc (not sending it to the guc as the name implies). To help the
reader rename it to DISABLE_REDIRECT_TO_GUC so that we keep the bspec
greppable name without it being as confusing!

Signed-off-by: Chris Wilson 
Cc: Tvrtko Ursulin 
Cc: Oscar Mateo 
Cc: Radoslaw Szwichtenberg 
Cc: Sagar Arun Kamble 
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 4 ++--
 drivers/gpu/drm/i915/i915_irq.c| 2 +-
 drivers/gpu/drm/i915/i915_reg.h| 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c 
b/drivers/gpu/drm/i915/i915_guc_submission.c
index ca7723fd0f79..84fd49d5680e 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -975,7 +975,7 @@ static void guc_interrupts_capture(struct drm_i915_private 
*dev_priv)
 * result in the register bit being left SET!
 */
dev_priv->rps.pm_intrmsk_mbz |= ARAT_EXPIRED_INTRMSK;
-   dev_priv->rps.pm_intrmsk_mbz &= ~GEN8_PMINTR_REDIRECT_TO_GUC;
+   dev_priv->rps.pm_intrmsk_mbz &= ~GEN8_PMINTR_DISABLE_REDIRECT_TO_GUC;
 }
 
 int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
@@ -1037,7 +1037,7 @@ static void guc_interrupts_release(struct 
drm_i915_private *dev_priv)
I915_WRITE(GUC_VCS2_VCS1_IER, 0);
I915_WRITE(GUC_WD_VECS_IER, 0);
 
-   dev_priv->rps.pm_intrmsk_mbz |= GEN8_PMINTR_REDIRECT_TO_GUC;
+   dev_priv->rps.pm_intrmsk_mbz |= GEN8_PMINTR_DISABLE_REDIRECT_TO_GUC;
dev_priv->rps.pm_intrmsk_mbz &= ~ARAT_EXPIRED_INTRMSK;
 
 }
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index a522da712cc8..89ccf3e1fda5 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -4282,7 +4282,7 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
dev_priv->rps.pm_intrmsk_mbz |= GEN6_PM_RP_UP_EI_EXPIRED;
 
if (INTEL_INFO(dev_priv)->gen >= 8)
-   dev_priv->rps.pm_intrmsk_mbz |= GEN8_PMINTR_REDIRECT_TO_GUC;
+   dev_priv->rps.pm_intrmsk_mbz |= 
GEN8_PMINTR_DISABLE_REDIRECT_TO_GUC;
 
if (IS_GEN2(dev_priv)) {
/* Gen2 doesn't have a hardware frame counter */
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 19d42e8813c4..5d88c35c41cd 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7453,7 +7453,7 @@ enum {
 #define VLV_RCEDATA_MMIO(0xA0BC)
 #define GEN6_RC6pp_THRESHOLD   _MMIO(0xA0C0)
 #define GEN6_PMINTRMSK _MMIO(0xA168)
-#define   GEN8_PMINTR_REDIRECT_TO_GUC  (1<<31)
+#define   GEN8_PMINTR_DISABLE_REDIRECT_TO_GUC  (1<<31)
 #define   ARAT_EXPIRED_INTRMSK (1<<9)
 #define GEN8_MISC_CTRL0_MMIO(0xA180)
 #define VLV_PWRDWNUPCTL_MMIO(0xA294)
-- 
2.11.0

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

Re: [Intel-gfx] [libdrm PATCH] intel: Make unsynchronized GTT mappings work on systems with snooping.

[Chris Wilson]

On Fri, Mar 10, 2017 at 05:14:32PM -0800, Kenneth Graunke wrote:
> On systems without LLC, drm_intel_gem_bo_map_unsynchronized() has
> had the surprising behavior of doing a synchronized GTT mapping.
> This is obviously not what the user of the API wanted.
> 
> Eric left a comment indicating a valid concern: if the CPU and GPU
> caches are incoherent, we don't keep track of where the user last
> mapped the buffer, and what caches might contain relevant data.

Note this is an issue in libdrm_intel not tracking the cache domain
transitions. Even just a switch between cpu and coherent would solve the
majority of that - the caveat being shared bo where the tracking is
incomplete.
 
> Modern Atom systems still don't have LLC, but they do offer snooping,
> which effectively makes the caches coherent.  The kernel appears to
> set up the PTE/PPAT to enable snooping for everything where the cache
> level is not I915_CACHE_NONE.  As far as I know, only scanout buffers
> are marked as uncached.

Byt, bsw beg to differ. I don't have a bxt to know the results of the
igt/kernel tests.
 
> Any buffers used by scanout should be flagged as non-reusable with
> drm_intel_bo_disable_reuse(), prime export, or flink.  So, we can
> assume that any reusable buffer should be snooped.

Not really, there is no reason why scanout buffers can't be reused.
 
> This patch enables unsynchronized mappings for reusable buffers
> on all Gen6+ hardware (which have either LLC or snooping).
> 
> On Broxton, this improves the performance of Unigine Valley 1.0
> on Low settings at 1280x720 by about 45%, and Unigine Heaven 4.0
> (same settings) by about 53%.

Does anyone have figures for gtt performance on bxt - does it cover over
the same performance penalty from earler atoms? Basically why bother to
enable this over wc mapping (no stalls for a contended, limited
resource) + detiling. (Just note that for detiling Y to WC you need to
use a temporary cacheable page, or rearrange the code to make sure the
reads/writes are in 64 byte chunks.) 

> Signed-off-by: Kenneth Graunke 
> Cc: Chris Wilson 
> Cc: mesa-...@lists.freedesktop.org
> ---
>  intel/intel_bufmgr_gem.c | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> It looks like Mesa and Beignet are the only callers of this function
> (SNA and Anvil don't use libdrm, UXA and vaapi don't use this function.)
> 
> This passed our full barrage of Piglit/dEQP/GLCTS/GLESCTS testing.
> gnome-shell still works, as does Unigine, and GLBenchmark.
> 
> I haven't tested any OpenCL workloads.
> 
> diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
> index e260f2dc..f53f1fcc 100644
> --- a/intel/intel_bufmgr_gem.c
> +++ b/intel/intel_bufmgr_gem.c
> @@ -1630,9 +1630,7 @@ int
>  drm_intel_gem_bo_map_unsynchronized(drm_intel_bo *bo)
>  {
>   drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *) bo->bufmgr;
> -#ifdef HAVE_VALGRIND
>   drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *) bo;
> -#endif
>   int ret;
>  
>   /* If the CPU cache isn't coherent with the GTT, then use a
> @@ -1641,8 +1639,12 @@ drm_intel_gem_bo_map_unsynchronized(drm_intel_bo *bo)
>* terms of drm_intel_bo_map vs drm_intel_gem_bo_map_gtt, so
>* we would potentially corrupt the buffer even when the user
>* does reasonable things.
> +  *
> +  * The caches are coherent on LLC platforms or snooping is enabled
> +  * for the BO.  The kernel enables snooping for non-scanout (reusable)
> +  * buffers on modern non-LLC systems.

gen >= 9; the snoop was reinvented

Not enabled by default on !llc. By default object and PTE are set to
uncached, and mocs default is to follow PTE.

I would just do the unsync map and leave it to the caller to ensure it
is used correctly. Or just use the raw interfaces that leave the domain
tracking to the caller.

>*/
> - if (!bufmgr_gem->has_llc)
> + if (bufmgr_gem->gen < 6 || !bo_gem->reusable)
>   return drm_intel_gem_bo_map_gtt(bo);
>  
>   pthread_mutex_lock(_gem->lock);
> -- 
> 2.12.0
> 

-- 
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] ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915/guc: Release GuC interrupts in i915_guc_submission_disable

[Chris Wilson]

On Sat, Mar 11, 2017 at 04:17:34AM -, Patchwork wrote:
> == Series Details ==
> 
> Series: series starting with [1/3] drm/i915/guc: Release GuC interrupts in 
> i915_guc_submission_disable
> URL   : https://patchwork.freedesktop.org/series/21090/
> State : success
> 
> == Summary ==
> 
> Series 21090v1 Series without cover letter
> https://patchwork.freedesktop.org/api/1.0/series/21090/revisions/1/mbox/
> 
> Test kms_pipe_crc_basic:
> Subgroup hang-read-crc-pipe-b:
> dmesg-warn -> PASS   (fi-byt-j1900)

Applied, thanks for the quick fix. It is looking much neater now as well
:)
-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

[Intel-gfx] [PULL] drm-misc-next

[Daniel Vetter]

Hi Dave,

drm-misc-next-2017-03-12:
More drm-misc stuff for 4.12:

- drm_platform removal from Laurent
- more dw-hdmi bridge driver updates (Laurent, Kieran, Neil)
- more header cleanup and documentation
- more drm_debugs_remove_files removal (Noralf)
- minor qxl updates (Gerd)
- edp crc support in helper + analogix_dp (Tomeu) for more igt
  testing!
- old/new iterator roll-out (Maarten)
- new bridge drivers: lvds (Laurent), megachips-something (Peter
  Senna)

Cheers, Daniel


The following changes since commit ca39b449f6d03e8235969f12f5dd25b8eb4304d6:

  drm/vc4: Fix OOPSes from trying to cache a partially constructed BO. 
(2017-03-02 09:57:23 -0800)

are available in the git repository at:

  git://anongit.freedesktop.org/git/drm-misc tags/drm-misc-next-2017-03-12

for you to fetch changes up to a45216547e8925078b18b2a6b539100c3814e973:

  Merge branch 'drm/next/platform' of git://linuxtv.org/pinchartl/media into 
drm-misc-next (2017-03-11 11:46:03 +0100)


More drm-misc stuff for 4.12:

- drm_platform removal from Laurent
- more dw-hdmi bridge driver updates (Laurent, Kieran, Neil)
- more header cleanup and documentation
- more drm_debugs_remove_files removal (Noralf)
- minor qxl updates (Gerd)
- edp crc support in helper + analogix_dp (Tomeu) for more igt
  testing!
- old/new iterator roll-out (Maarten)
- new bridge drivers: lvds (Laurent), megachips-something (Peter
  Senna)


Daniel Vetter (10):
  drm/doc: Add todo about connector_list_iter
  drm: Extract drm_prime.h
  drm: Move drm_lock_data out of drmP.h
  drm: Extract drm_pci.h
  drm: Remove drmP.h include from drm_kms_helper_common.c
  drm/doc: document fallback behaviour for atomic events
  drm: rename drm_fops.c to drm_file.c
  drm: Remove DRM_MINOR_CNT
  drm: Extract drm_file.h
  Merge branch 'drm/next/platform' of git://linuxtv.org/pinchartl/media 
into drm-misc-next

Gabriel Krisman Bertazi (1):
  drm: qxl: Don't alloc fbdev if emulation is not supported

Gerd Hoffmann (5):
  qxl: drop mode_info.modes & related code.
  qxl: limit monitor config read retries
  qxl: read monitors config at boot
  qxl: fix qxl_conn_get_modes
  drm: virtio: use kmem_cache

Kieran Bingham (2):
  drm: bridge: dw-hdmi: Add support for custom PHY configuration
  drm: bridge: dw-hdmi: Remove device type from platform data

Laurent Pinchart (14):
  drm: shmobile: Perform initialization/cleanup at probe/remove time
  drm: exynos: Perform initialization/cleanup at probe/remove time
  drm: Remove unused drm_platform midlayer
  drm: Remove the struct drm_device platformdev field
  devicetree/bindings: display: bridge: Add LVDS encoder DT bindings
  drm: bridge: Add LVDS encoder driver
  drm: bridge: vga-dac: Add adi,adv7123 compatible string
  drm: bridge: lvds-encoder: Add thine,thc63lvdm83d compatible string
  drm: bridge: dw-hdmi: Remove unused functions
  drm: bridge: dw-hdmi: Move CSC configuration out of PHY code
  drm: bridge: dw-hdmi: Fix the PHY power down sequence
  drm: bridge: dw-hdmi: Fix the PHY power up sequence
  drm: bridge: dw-hdmi: Create PHY operations
  drm: bridge: dw-hdmi: Move the driver to a separate directory.

Maarten Lankhorst (5):
  drm/atomic: Fix atomic helpers to use the new iterator macros, v3.
  drm/atomic: Make drm_atomic_plane_disabling easier to understand.
  drm/atomic: Add macros to access existing old/new state, v2.
  drm/atomic: Convert get_existing_state callers to get_old/new_state, v4.
  drm/blend: Use new atomic iterator macros.

Neil Armstrong (2):
  drm: bridge: dw-hdmi: Enable CSC even for DVI
  drm: bridge: dw-hdmi: Switch to regmap for register access

Noralf Trønnes (3):
  drm/msm: Remove msm_debugfs_cleanup()
  drm/debugfs: Remove the drm_driver.debugfs_cleanup callback
  drm/qxl: Remove qxl_debugfs_remove_files()

Peter Senna Tschudin (3):
  dt-bindings: display: megachips-stdp-ge-b850v3-fw
  MAINTAINERS: Add entry for megachips-stdp-ge-b850v3-fw
  drm/bridge: Drivers for megachips-stdp-ge-b850v3-fw (LVDS-DP++)

Sean Paul (2):
  drm: Fix compilation error when CONFIG_DEBUG_FS is undefined
  drm/rockchip: Fix link error when CONFIG_DRM_ANALOGIX_DP undefined

Tomeu Vizoso (5):
  drm/dp: add crtc backpointer to drm_dp_aux
  drm/dp: add helpers for capture of frame CRCs
  drm/bridge: analogix_dp: add helpers for capture of frame CRCs
  drm/rockchip: Implement CRC debugfs API
  drm/dp: Add missing description to parameter

 .../bindings/display/bridge/lvds-transmitter.txt   |  64 +++
 .../bridge/megachips-stdp-ge-b850v3-fw.txt |  94 
 .../devicetree/bindings/vendor-prefixes.txt|   1 +
 Documentation/gpu/drm-internals.rst|   7 +-